Skip to content

Commit

Permalink
fix(cloudfront): propagate originAccessControlId CloudFront Origin …
Browse files Browse the repository at this point in the history
…property to CloudFormation templates (#32020)

### Issue

Closes #32018.

### Reason for this change

The originAccessControlId property of CloudFront Origin has not been propagated to CloudFormation templates.

### Description of changes

Propagate the property to render function.

### Description of how you validated changes

have run the [run build over the whole repo](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md#setup) - my computer almost exploded.

I have also run tests for the aws-cloudfront which run successfully.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
ivanbarlog authored Nov 28, 2024
1 parent 9f3d09b commit f9708a6
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 5 deletions.
3 changes: 3 additions & 0 deletions packages/aws-cdk-lib/aws-cloudfront/lib/origin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ export abstract class OriginBase implements IOrigin {
private readonly originShieldRegion?: string;
private readonly originShieldEnabled: boolean;
private readonly originId?: string;
private readonly originAccessControlId?: string;

protected constructor(domainName: string, props: OriginProps = {}) {
validateIntInRangeOrUndefined('connectionTimeout', 1, 10, props.connectionTimeout?.toSeconds());
Expand All @@ -163,6 +164,7 @@ export abstract class OriginBase implements IOrigin {
this.originShieldRegion = props.originShieldRegion;
this.originId = props.originId;
this.originShieldEnabled = props.originShieldEnabled ?? true;
this.originAccessControlId = props.originAccessControlId;
}

/**
Expand All @@ -187,6 +189,7 @@ export abstract class OriginBase implements IOrigin {
s3OriginConfig,
customOriginConfig,
originShield: this.renderOriginShield(this.originShieldEnabled, this.originShieldRegion),
originAccessControlId: this.originAccessControlId,
},
};
}
Expand Down
32 changes: 31 additions & 1 deletion packages/aws-cdk-lib/aws-cloudfront/test/distribution.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { defaultOrigin, defaultOriginGroup } from './test-origin';
import { defaultOrigin, defaultOriginGroup, defaultOriginWithOriginAccessControl } from './test-origin';
import { Annotations, Match, Template } from '../../assertions';
import * as acm from '../../aws-certificatemanager';
import * as cloudwatch from '../../aws-cloudwatch';
Expand Down Expand Up @@ -1282,6 +1282,36 @@ test('with publish additional metrics', () => {
});
});

test('with origin access control id', () => {
const origin = defaultOriginWithOriginAccessControl();
new Distribution(stack, 'MyDist', {
defaultBehavior: { origin },
publishAdditionalMetrics: true,
});

Template.fromStack(stack).hasResourceProperties('AWS::CloudFront::Distribution', {
DistributionConfig: {
DefaultCacheBehavior: {
CachePolicyId: '658327ea-f89d-4fab-a63d-7e88639e58f6',
Compress: true,
TargetOriginId: 'StackMyDistOrigin1D6D5E535',
ViewerProtocolPolicy: 'allow-all',
},
Enabled: true,
HttpVersion: 'http2',
IPV6Enabled: true,
Origins: [{
DomainName: 'www.example.com',
Id: 'StackMyDistOrigin1D6D5E535',
CustomOriginConfig: {
OriginProtocolPolicy: 'https-only',
},
OriginAccessControlId: 'test-origin-access-control-id',
}],
},
});
});

describe('Distribution metrics tests', () => {
const additionalMetrics = [
{ name: 'OriginLatency', method: 'metricOriginLatency', statistic: 'Average', additionalMetricsRequired: true, errorMetricName: 'Origin latency' },
Expand Down
35 changes: 31 additions & 4 deletions packages/aws-cdk-lib/aws-cloudfront/test/test-origin.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,31 @@
import { Construct } from 'constructs';
import { CfnDistribution, IOrigin, OriginBase, OriginBindConfig, OriginBindOptions, OriginProps, OriginProtocolPolicy } from '../lib';
import {
CfnDistribution,
IOrigin,
OriginBase,
OriginBindConfig,
OriginBindOptions,
OriginProps,
OriginProtocolPolicy,
} from '../lib';

/** Used for testing common Origin functionality */
export class TestOrigin extends OriginBase {
constructor(domainName: string, props: OriginProps = {}) { super(domainName, props); }
protected renderCustomOriginConfig(): CfnDistribution.CustomOriginConfigProperty | undefined {
constructor(domainName: string, props: OriginProps = {}) {
super(domainName, props);
}
protected renderCustomOriginConfig():
| CfnDistribution.CustomOriginConfigProperty
| undefined {
return { originProtocolPolicy: OriginProtocolPolicy.HTTPS_ONLY };
}
}

export class TestOriginGroup implements IOrigin {
constructor(private readonly primaryDomainName: string, private readonly secondaryDomainName: string) { }
constructor(
private readonly primaryDomainName: string,
private readonly secondaryDomainName: string,
) {}
/* eslint-disable @cdklabs/no-core-construct */
public bind(scope: Construct, options: OriginBindOptions): OriginBindConfig {
const primaryOrigin = new TestOrigin(this.primaryDomainName);
Expand All @@ -35,3 +50,15 @@ export function defaultOrigin(domainName?: string, originId?: string): IOrigin {
export function defaultOriginGroup(): IOrigin {
return new TestOriginGroup('www.example.com', 'foo.example.com');
}

export function defaultOriginWithOriginAccessControl(
domainName?: string,
originId?: string,
originAccessControlId?: string,
): IOrigin {
return new TestOrigin(domainName ?? 'www.example.com', {
originId,
originAccessControlId:
originAccessControlId ?? 'test-origin-access-control-id',
});
}

0 comments on commit f9708a6

Please sign in to comment.