Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(route53-patterns): use Certificate as the default certificate (under feature flag) #23575

Merged
merged 5 commits into from
Jan 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions packages/@aws-cdk/aws-route53-patterns/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,32 @@ new patterns.HttpsRedirect(this, 'Redirect', {
}),
});
```

To have `HttpsRedirect` use the `Certificate` construct as the default
created certificate instead of the deprecated `DnsValidatedCertificate`
construct, enable the `@aws-cdk/aws-route53-patters:useCertificate`
feature flag. If you are creating the stack in a region other than `us-east-1`
you must also enable `crossRegionReferences` on the stack.

```ts
declare const app: App;
const stack = new Stack(app, 'Stack', {
crossRegionReferences: true,
env: {
region: 'us-east-2',
},
});

new patterns.HttpsRedirect(this, 'Redirect', {
recordNames: ['foo.example.com'],
targetDomain: 'bar.example.com',
zone: route53.HostedZone.fromHostedZoneAttributes(this, 'HostedZone', {
hostedZoneId: 'ID',
zoneName: 'example.com',
}),
});
```

It is safe to upgrade to `@aws-cdk/aws-route53-patterns:useCertificate` since
the new certificate will be created and updated on the CloudFront distribution
before the old certificate is deleted.
81 changes: 71 additions & 10 deletions packages/@aws-cdk/aws-route53-patterns/lib/website-redirect.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { md5hash } from '@aws-cdk/core/lib/helpers-internal';
import { DnsValidatedCertificate, ICertificate } from '@aws-cdk/aws-certificatemanager';
import { DnsValidatedCertificate, ICertificate, Certificate, CertificateValidation } from '@aws-cdk/aws-certificatemanager';
import { CloudFrontWebDistribution, OriginProtocolPolicy, PriceClass, ViewerCertificate, ViewerProtocolPolicy } from '@aws-cdk/aws-cloudfront';
import { ARecord, AaaaRecord, IHostedZone, RecordTarget } from '@aws-cdk/aws-route53';
import { CloudFrontTarget } from '@aws-cdk/aws-route53-targets';
import { BlockPublicAccess, Bucket, RedirectProtocol } from '@aws-cdk/aws-s3';
import { ArnFormat, RemovalPolicy, Stack, Token } from '@aws-cdk/core';
import { ArnFormat, RemovalPolicy, Stack, Token, FeatureFlags } from '@aws-cdk/core';
import { md5hash } from '@aws-cdk/core/lib/helpers-internal';
import { ROUTE53_PATTERNS_USE_CERTIFICATE } from '@aws-cdk/cx-api';
import { Construct } from 'constructs';

/**
Expand Down Expand Up @@ -63,13 +64,7 @@ export class HttpsRedirect extends Construct {
throw new Error(`The certificate must be in the us-east-1 region and the certificate you provided is in ${certificateRegion}.`);
}
}

const redirectCert = props.certificate ?? new DnsValidatedCertificate(this, 'RedirectCertificate', {
domainName: domainNames[0],
subjectAlternativeNames: domainNames,
hostedZone: props.zone,
region: 'us-east-1',
});
const redirectCert = props.certificate ?? this.createCertificate(domainNames, props.zone);

const redirectBucket = new Bucket(this, 'RedirectBucket', {
websiteRedirect: {
Expand Down Expand Up @@ -107,4 +102,70 @@ export class HttpsRedirect extends Construct {
new AaaaRecord(this, `RedirectAliasRecordSix${hash}`, aliasProps);
});
}

/**
* Gets the stack to use for creating the Certificate
* If the current stack is not in `us-east-1` then this
* will create a new `us-east-1` stack.
*
* CloudFront is a global resource which you can create (via CloudFormation) from
* _any_ region. So I could create a CloudFront distribution in `us-east-2` if I wanted
* to (maybe the rest of my application lives there). The problem is that some supporting resources
* that CloudFront uses (i.e. ACM Certificates) are required to exist in `us-east-1`. This means
* that if I want to create a CloudFront distribution in `us-east-2` I still need to create a ACM certificate in
* `us-east-1`.
*
* In order to do this correctly we need to know which region the CloudFront distribution is being created in.
* We have two options, either require the user to specify the region or make an assumption if they do not.
* This implementation requires the user specify the region.
*/
private certificateScope(): Construct {
const stack = Stack.of(this);
const parent = stack.node.scope;
if (!parent) {
throw new Error(`Stack ${stack.stackId} must be created in the scope of an App or Stage`);
}
if (Token.isUnresolved(stack.region)) {
throw new Error(`When ${ROUTE53_PATTERNS_USE_CERTIFICATE} is enabled, a region must be defined on the Stack`);
}
Comment on lines +128 to +130
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoah, what is this? In the PR description, you said that there's no breaking change, but what about scenarios where you're using DnsValidatedCertificate without a region? Wouldn't we throw an error when we move it to Certificate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is that a breaking change? We are not breaking any existing user since we'll only move to Certificate when you enable the feature flag.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair. not a breaking change, but I was sounding the alarm that users might move from DnsValidatedCertificate to Certificate without a defined region and would hit this error. I wonder what the story is for these people, or if that is a valid use case to begin with. Either way, its not breaking, so I'm not blocking.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah those users would have to provide a region, which I don't think is too much to ask when you are going cross region stuff. The alternative is that we make an assumption

  1. Assume that they are in us-east-1 and if they are not the error is thrown by CloudFormation
  2. Assume that they are not in us-east-1 and always create a support stack. We can always switch this later if this is the direction users want us to go.

if (stack.region !== 'us-east-1') {
kaizencc marked this conversation as resolved.
Show resolved Hide resolved
const stackId = `certificate-redirect-stack-${stack.node.addr}`;
const certStack = parent.node.tryFindChild(stackId) as Stack;
return certStack ?? new Stack(parent, stackId, {
env: { region: 'us-east-1', account: stack.account },
});
}
return this;
}

/**
* Creates a certificate.
*
* If the `ROUTE53_PATTERNS_USE_CERTIFICATE` feature flag is set then
* this will use the `Certificate` class otherwise it will use the
* `DnsValidatedCertificate` class
*
* This is also safe to upgrade since the new certificate will be created and updated
* on the CloudFront distribution before the old one is deleted.
corymhall marked this conversation as resolved.
Show resolved Hide resolved
*/
private createCertificate(domainNames: string[], zone: IHostedZone): ICertificate {
const useCertificate = FeatureFlags.of(this).isEnabled(ROUTE53_PATTERNS_USE_CERTIFICATE);
if (useCertificate) {
// this preserves backwards compatibility. Previously the certificate was always created in `this` scope
// so we need to keep the name the same
const id = (this.certificateScope() === this) ? 'RedirectCertificate' : 'RedirectCertificate'+this.node.addr;
return new Certificate(this.certificateScope(), id, {
domainName: domainNames[0],
subjectAlternativeNames: domainNames,
validation: CertificateValidation.fromDns(zone),
});
} else {
return new DnsValidatedCertificate(this, 'RedirectCertificate', {
kaizencc marked this conversation as resolved.
Show resolved Hide resolved
domainName: domainNames[0],
subjectAlternativeNames: domainNames,
hostedZone: zone,
region: 'us-east-1',
});
}
}
}
3 changes: 3 additions & 0 deletions packages/@aws-cdk/aws-route53-patterns/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
"@aws-cdk/assertions": "0.0.0",
"@aws-cdk/cdk-build-tools": "0.0.0",
"@aws-cdk/integ-runner": "0.0.0",
"@aws-cdk/integ-tests": "0.0.0",
"@aws-cdk/cfn2ts": "0.0.0",
"@aws-cdk/pkglint": "0.0.0",
"@types/jest": "^27.5.2",
Expand All @@ -89,6 +90,7 @@
"@aws-cdk/aws-route53-targets": "0.0.0",
"@aws-cdk/aws-s3": "0.0.0",
"@aws-cdk/core": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"@aws-cdk/region-info": "0.0.0",
"constructs": "^10.0.0"
},
Expand All @@ -101,6 +103,7 @@
"@aws-cdk/aws-route53-targets": "0.0.0",
"@aws-cdk/aws-s3": "0.0.0",
"@aws-cdk/core": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"@aws-cdk/region-info": "0.0.0",
"constructs": "^10.0.0"
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Fixture with packages imported, but nothing else
import { Construct } from 'constructs';
import { Stack } from '@aws-cdk/core';
import { Stack, App } from '@aws-cdk/core';
import * as route53 from '@aws-cdk/aws-route53';
import * as patterns from '@aws-cdk/aws-route53-patterns';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Template } from '@aws-cdk/assertions';
import { Certificate } from '@aws-cdk/aws-certificatemanager';
import { HostedZone } from '@aws-cdk/aws-route53';
import { App, Stack } from '@aws-cdk/core';
import { ROUTE53_PATTERNS_USE_CERTIFICATE } from '@aws-cdk/cx-api';
import { HttpsRedirect } from '../lib';

test('create HTTPS redirect', () => {
Expand Down Expand Up @@ -152,3 +153,144 @@ test('throws when certificate in region other than us-east-1 is supplied', () =>
});
}).toThrow(/The certificate must be in the us-east-1 region and the certificate you provided is in us-east-2./);
});

describe('Uses Certificate when @aws-cdk/aws-route53-patters:useCertificate=true', () => {
test('explicit different region', () => {
// GIVEN
const app = new App({
context: {
[ROUTE53_PATTERNS_USE_CERTIFICATE]: true,
},
});

// WHEN
const stack = new Stack(app, 'test', { env: { region: 'us-east-2' }, crossRegionReferences: true });
new HttpsRedirect(stack, 'Redirect', {
recordNames: ['foo.example.com'],
targetDomain: 'bar.example.com',
zone: HostedZone.fromHostedZoneAttributes(stack, 'HostedZone', {
hostedZoneId: 'ID',
zoneName: 'example.com',
}),
});

// THEN
const certStack = app.node.findChild(`certificate-redirect-stack-${stack.node.addr}`) as Stack;
Template.fromStack(certStack).hasResourceProperties('AWS::CertificateManager::Certificate', {
DomainName: 'foo.example.com',
});

Template.fromStack(stack).hasResourceProperties('AWS::CloudFront::Distribution', {
DistributionConfig: {
ViewerCertificate: {
AcmCertificateArn: {
'Fn::GetAtt': [
'ExportsReader8B249524',
'/cdk/exports/test/certificateredirectstackc8e2763df63c0f7e0c9afe0394e299bb731e281e8euseast1RefRedirectCertificatec8693e36481e135aa76e35c2db892ec6a33a94c7461E1B6E15A36EB7DA',
],
},
},
},
});
});

test('explicit same region', () => {
// GIVEN
const app = new App({
context: {
[ROUTE53_PATTERNS_USE_CERTIFICATE]: true,
},
});

// WHEN
const stack = new Stack(app, 'test', { env: { region: 'us-east-1' }, crossRegionReferences: true });
new HttpsRedirect(stack, 'Redirect', {
recordNames: ['foo.example.com'],
targetDomain: 'bar.example.com',
zone: HostedZone.fromHostedZoneAttributes(stack, 'HostedZone', {
hostedZoneId: 'ID',
zoneName: 'example.com',
}),
});

// THEN
const certStack = app.node.tryFindChild(`certificate-redirect-stack-${stack.node.addr}`);
expect(certStack).toBeUndefined();
Template.fromStack(stack).hasResourceProperties('AWS::CertificateManager::Certificate', {
DomainName: 'foo.example.com',
});

Template.fromStack(stack).hasResourceProperties('AWS::CloudFront::Distribution', {
DistributionConfig: {
ViewerCertificate: {
AcmCertificateArn: {
Ref: 'RedirectRedirectCertificateB4F2F130',
},
},
},
});
});

test('same support stack used for multiple certificates', () => {
// GIVEN
const app = new App({
context: {
[ROUTE53_PATTERNS_USE_CERTIFICATE]: true,
},
});

// WHEN
const stack = new Stack(app, 'test', { env: { region: 'us-east-2' }, crossRegionReferences: true });
new HttpsRedirect(stack, 'Redirect', {
recordNames: ['foo.example.com'],
targetDomain: 'bar.example.com',
zone: HostedZone.fromHostedZoneAttributes(stack, 'HostedZone', {
hostedZoneId: 'ID',
zoneName: 'example.com',
}),
});

new HttpsRedirect(stack, 'Redirect2', {
recordNames: ['foo2.example.com'],
targetDomain: 'bar2.example.com',
zone: HostedZone.fromHostedZoneAttributes(stack, 'HostedZone2', {
hostedZoneId: 'ID',
zoneName: 'example.com',
}),
});

// THEN
const certStack = app.node.tryFindChild(`certificate-redirect-stack-${stack.node.addr}`) as Stack;
Template.fromStack(certStack).hasResourceProperties('AWS::CertificateManager::Certificate', {
DomainName: 'foo.example.com',
});
Template.fromStack(certStack).hasResourceProperties('AWS::CertificateManager::Certificate', {
DomainName: 'foo2.example.com',
});
});

test('unresolved region throws', () => {
// GIVEN
const app = new App({
context: {
[ROUTE53_PATTERNS_USE_CERTIFICATE]: true,
},
});

// WHEN
const stack = new Stack(app, 'test');

// THEN
expect(() => {
new HttpsRedirect(stack, 'Redirect', {
recordNames: ['foo.example.com'],
targetDomain: 'bar.example.com',
zone: HostedZone.fromHostedZoneAttributes(stack, 'HostedZone', {
hostedZoneId: 'ID',
zoneName: 'example.com',
}),
});

}).toThrow(/When @aws-cdk\/aws-route53-patters:useCertificate is enabled, a region must be defined on the Stack/);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"version":"22.0.0"}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"version": "22.0.0",
"files": {
"cfd1c229f1b6f82ba7c98886bbb9cce3f98c3d07fc1cee4b2525494d7cd8d4bf": {
"source": {
"path": "integ-https-redirect-same-region.template.json",
"packaging": "file"
},
"destinations": {
"current_account-us-east-1": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-us-east-1",
"objectKey": "cfd1c229f1b6f82ba7c98886bbb9cce3f98c3d07fc1cee4b2525494d7cd8d4bf.json",
"region": "us-east-1",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-us-east-1"
}
}
}
},
"dockerImages": {}
}
Loading