Skip to content

Commit

Permalink
fix(cloudfront): fromOriginAccessIdentityName is a misnomer (aws#20772)
Browse files Browse the repository at this point in the history
fixes aws#20141

----

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
dbartholomae authored and daschaa committed Jul 9, 2022
1 parent 41df699 commit 0294ef9
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 14 deletions.
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-cloudfront-origins/lib/s3-origin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,6 @@ class S3BucketOrigin extends cloudfront.OriginBase {
}

protected renderS3OriginConfig(): cloudfront.CfnDistribution.S3OriginConfigProperty | undefined {
return { originAccessIdentity: `origin-access-identity/cloudfront/${this.originAccessIdentity.originAccessIdentityName}` };
return { originAccessIdentity: `origin-access-identity/cloudfront/${this.originAccessIdentity.originAccessIdentityId}` };
}
}
65 changes: 55 additions & 10 deletions packages/@aws-cdk/aws-cloudfront/lib/origin-access-identity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,35 @@ export interface OriginAccessIdentityProps {
*/
export interface IOriginAccessIdentity extends cdk.IResource, iam.IGrantable {
/**
* The Origin Access Identity Name
* The Origin Access Identity Id (physical id)
* It is misnamed and superseded by the correctly named originAccessIdentityId
*
* @deprecated use originAccessIdentityId instead
*/
readonly originAccessIdentityName: string;

/**
* The Origin Access Identity Id (physical id)
* This was called originAccessIdentityName before
*/
readonly originAccessIdentityId: string;
}

abstract class OriginAccessIdentityBase extends cdk.Resource {
/**
* The Origin Access Identity Name (physical id)
* The Origin Access Identity Id (physical id)
* It is misnamed and superseded by the correctly named originAccessIdentityId
*
* @deprecated use originAccessIdentityId instead
*/
public abstract readonly originAccessIdentityName: string;

/**
* The Origin Access Identity Id (physical id)
* This was called originAccessIdentityName before
*/
public abstract readonly originAccessIdentityId: string;

/**
* Derived principal value for bucket access
*/
Expand All @@ -45,7 +64,7 @@ abstract class OriginAccessIdentityBase extends cdk.Resource {
region: '', // global
account: 'cloudfront',
resource: 'user',
resourceName: `CloudFront Origin Access Identity ${this.originAccessIdentityName}`,
resourceName: `CloudFront Origin Access Identity ${this.originAccessIdentityId}`,
},
);
}
Expand All @@ -60,18 +79,32 @@ abstract class OriginAccessIdentityBase extends cdk.Resource {
*/
export class OriginAccessIdentity extends OriginAccessIdentityBase implements IOriginAccessIdentity {
/**
* Creates a OriginAccessIdentity by providing the OriginAccessIdentityName
* Creates a OriginAccessIdentity by providing the OriginAccessIdentityId.
* It is misnamed and superseded by the correctly named fromOriginAccessIdentityId.
*
* @deprecated use `fromOriginAccessIdentityId`
*/
public static fromOriginAccessIdentityName(
scope: Construct,
id: string,
originAccessIdentityName: string): IOriginAccessIdentity {
return OriginAccessIdentity.fromOriginAccessIdentityId(scope, id, originAccessIdentityName);
}

/**
* Creates a OriginAccessIdentity by providing the OriginAccessIdentityId.
*/
public static fromOriginAccessIdentityId(
scope: Construct,
id: string,
originAccessIdentityId: string): IOriginAccessIdentity {

class Import extends OriginAccessIdentityBase {
public readonly originAccessIdentityName = originAccessIdentityName;
public readonly originAccessIdentityId = originAccessIdentityId;
public readonly originAccessIdentityName = originAccessIdentityId;
public readonly grantPrincipal = new iam.ArnPrincipal(this.arn());
constructor(s: Construct, i: string) {
super(s, i, { physicalName: originAccessIdentityName });
super(s, i, { physicalName: originAccessIdentityId });
}
}

Expand All @@ -93,11 +126,23 @@ export class OriginAccessIdentity extends OriginAccessIdentityBase implements IO
public readonly grantPrincipal: iam.IPrincipal;

/**
* The Origin Access Identity Name (physical id)
* The Origin Access Identity Id (physical id)
* It is misnamed and superseded by the correctly named originAccessIdentityId
*
* @attribute
* @deprecated use originAccessIdentityId instead
*/
public get originAccessIdentityName() {
return this.originAccessIdentityId;
}

/**
* The Origin Access Identity Id (physical id)
* This was called originAccessIdentityName before
*
* @attribute
*/
public readonly originAccessIdentityName: string;
public readonly originAccessIdentityId: string;

/**
* CDK L1 resource
Expand All @@ -112,8 +157,8 @@ export class OriginAccessIdentity extends OriginAccessIdentityBase implements IO
this.resource = new CfnCloudFrontOriginAccessIdentity(this, 'Resource', {
cloudFrontOriginAccessIdentityConfig: { comment },
});
// physical id - OAI name
this.originAccessIdentityName = this.getResourceNameAttribute(this.resource.ref);
// physical id - OAI Id
this.originAccessIdentityId = this.getResourceNameAttribute(this.resource.ref);

// Canonical user to grant access to in the S3 Bucket Policy
this.cloudFrontOriginAccessIdentityS3CanonicalUserId = this.resource.attrS3CanonicalUserId;
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-cloudfront/lib/web-distribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,7 @@ export class CloudFrontWebDistribution extends cdk.Resource implements IDistribu
}));

s3OriginConfig = {
originAccessIdentity: `origin-access-identity/cloudfront/${originConfig.s3OriginSource.originAccessIdentity.originAccessIdentityName}`,
originAccessIdentity: `origin-access-identity/cloudfront/${originConfig.s3OriginSource.originAccessIdentity.originAccessIdentityId}`,
};
} else {
s3OriginConfig = {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const oai = new cloudfront.CfnCloudFrontOriginAccessIdentity(stack, 'OAI', {
},
});

const oaiImported = cloudfront.OriginAccessIdentity.fromOriginAccessIdentityName(
const oaiImported = cloudfront.OriginAccessIdentity.fromOriginAccessIdentityId(
stack,
'OAIImported',
oai.ref,
Expand Down
11 changes: 10 additions & 1 deletion packages/@aws-cdk/aws-cloudfront/test/oai.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Template } from '@aws-cdk/assertions';
import * as cdk from '@aws-cdk/core';
import { testDeprecated } from '@aws-cdk/cdk-build-tools';
import { OriginAccessIdentity } from '../lib';

describe('Origin Access Identity', () => {
Expand Down Expand Up @@ -61,11 +62,19 @@ describe('Origin Access Identity', () => {
});
});

test('Builds ARN of CloudFront user', () => {
testDeprecated('Builds ARN of CloudFront user for fromOriginAccessIdentityName', () => {
const stack = new cdk.Stack();

const oai = OriginAccessIdentity.fromOriginAccessIdentityName(stack, 'OAI', 'OAITest');

expect(oai.grantPrincipal.policyFragment.principalJson.AWS[0]).toMatch(/:iam::cloudfront:user\/CloudFront Origin Access Identity OAITest$/);
});

test('Builds ARN of CloudFront user for fromOriginAccessIdentityId', () => {
const stack = new cdk.Stack();

const oai = OriginAccessIdentity.fromOriginAccessIdentityId(stack, 'OAI', 'OAITest');

expect(oai.grantPrincipal.policyFragment.principalJson.AWS[0]).toMatch(/:iam::cloudfront:user\/CloudFront Origin Access Identity OAITest$/);
});
});

0 comments on commit 0294ef9

Please sign in to comment.