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.

Sign up for GitHub

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

fix(route53): use static s3 website endpoint #4250

Merged
merged 4 commits into from
Sep 26, 2019
Merged

fix(route53): use static s3 website endpoint #4250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1b83b56
fix(route53): use static s3 website endpoint
Sep 26, 2019
1f91ebd
chore: detailed s3 target example
Sep 26, 2019
0704176
chore: match test bucketName with generated records
Sep 26, 2019
8901eb5
Merge branch 'master' into 4248
mergify[bot] Sep 26, 2019
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
28 changes: 21 additions & 7 deletions packages/@aws-cdk/aws-route53-targets/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,6 @@ This library contains Route53 Alias Record targets for:
target: route53.RecordTarget.fromAlias(new alias.CloudFrontTarget(distribution)),
});
```
* S3 Bucket WebSite
```ts
new route53.ARecord(this, 'AliasRecord', {
zone,
target: route53.RecordTarget.fromAlias(new alias.BucketWebsiteTarget(bucket)),
});
```
* ELBv2 load balancers
```ts
new route53.ARecord(this, 'AliasRecord', {
Expand All @@ -48,5 +41,26 @@ This library contains Route53 Alias Record targets for:
// or - route53.RecordTarget.fromAlias(new alias.ApiGatewayDomainName(domainName)),
});
```
* S3 Bucket WebSite:

**Important:** The Bucket name must strictly match the full DNS name.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd like to further improve the user experience and check in aws-route53 that the Bucket name matches RecordSet and HostedZone. How do you guys feel about that?

Copy link
Contributor

Choose a reason for hiding this comment

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

I like it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll make another PR for it 👍

See [the Developer Guide](https://docs.aws.amazon.com/Route53/latest/DeveloperGuide/getting-started.html) for more info.
```ts
const [recordName, domainName] = ['www', 'example.com'];

const bucketWebsite = new Bucket(this, 'BucketWebsite', {
bucketName: [recordName, domainName].join('.'), // www.example.com
publicReadAccess: true,
websiteIndexDocument: 'index.html',
});

const zone = HostedZone.fromLookup(this, 'Zone', {domainName}); // example.com

new route53.ARecord(this, 'AliasRecord', {
zone,
recordName, // www
target: route53.RecordTarget.fromAlias(new alias.BucketWebsiteTarget(bucket)),
});
```

See the documentation of `@aws-cdk/aws-route53` for more information.
9 changes: 3 additions & 6 deletions packages/@aws-cdk/aws-route53-targets/lib/bucket-website-target.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,12 @@ export class BucketWebsiteTarget implements route53.IAliasRecordTarget {
].join(' '));
}

const hostedZoneId = RegionInfo.get(region).s3StaticWebsiteHostedZoneId;
const {s3StaticWebsiteHostedZoneId: hostedZoneId, s3StaticWebsiteEndpoint: dnsName} = RegionInfo.get(region);

if (!hostedZoneId) {
if (!hostedZoneId || !dnsName) {
throw new Error(`Bucket website target is not supported for the "${region}" region`);
}

return {
hostedZoneId,
dnsName: this.bucket.bucketWebsiteDomainName,
};
return {hostedZoneId, dnsName};
}
}
54 changes: 16 additions & 38 deletions packages/@aws-cdk/aws-route53-targets/test/bucket-website-target.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,40 +4,28 @@ import s3 = require('@aws-cdk/aws-s3');
import { App, Stack } from '@aws-cdk/core';
import targets = require('../lib');

const [recordName, zoneName] = ['foo', 'test.public'];
const bucketName = [recordName, zoneName].join('.');

test('use S3 bucket website as record target', () => {
// GIVEN
const app = new App();
const stack = new Stack(app, 'test', {env: {region: 'us-east-1'}});

const bucketWebsite = new s3.Bucket(stack, 'Bucket');
const bucketWebsite = new s3.Bucket(stack, 'Bucket', { bucketName });

// WHEN
const zone = new route53.PublicHostedZone(stack, 'HostedZone', { zoneName: 'test.public' });
const zone = new route53.PublicHostedZone(stack, 'HostedZone', { zoneName });
new route53.ARecord(zone, 'Alias', {
zone,
recordName: '_foo',
recordName,
target: route53.RecordTarget.fromAlias(new targets.BucketWebsiteTarget(bucketWebsite))
});

// THEN
expect(stack).toHaveResource('AWS::Route53::RecordSet', {
AliasTarget: {
DNSName: {
"Fn::Select": [
2,
{
"Fn::Split": [
"/",
{
"Fn::GetAtt": [
"Bucket83908E77",
"WebsiteURL"
]
}
]
}
]
},
DNSName: "s3-website-us-east-1.amazonaws.com",
HostedZoneId: "Z3AQBSTGFYJSTF"
},
});
Expand All @@ -48,30 +36,20 @@ test('use S3 bucket website as record target (fromBucketName)', () => {
const app = new App();
const stack = new Stack(app, 'test', {env: {region: 'us-east-1'}});

const bucketWebsite = s3.Bucket.fromBucketName(stack, 'Bucket', 'test');
const bucketWebsite = s3.Bucket.fromBucketName(stack, 'Bucket', bucketName);

// WHEN
const zone = new route53.PublicHostedZone(stack, 'HostedZone', { zoneName: 'test.public' });
const zone = new route53.PublicHostedZone(stack, 'HostedZone', { zoneName });
new route53.ARecord(zone, 'Alias', {
zone,
recordName: '_foo',
recordName,
target: route53.RecordTarget.fromAlias(new targets.BucketWebsiteTarget(bucketWebsite))
});

// THEN
expect(stack).toHaveResource('AWS::Route53::RecordSet', {
AliasTarget: {
DNSName: {
"Fn::Join": [
"",
[
"test.s3-website-us-east-1.",
{
Ref: "AWS::URLSuffix"
}
]
]
},
DNSName: "s3-website-us-east-1.amazonaws.com",
HostedZoneId: "Z3AQBSTGFYJSTF"
},
});
Expand All @@ -81,16 +59,16 @@ test('throws if region agnostic', () => {
// GIVEN
const stack = new Stack();

const bucketWebsite = new s3.Bucket(stack, 'Bucket');
const bucketWebsite = new s3.Bucket(stack, 'Bucket', { bucketName });

// WHEN
const zone = new route53.PublicHostedZone(stack, 'HostedZone', { zoneName: 'test.public' });
const zone = new route53.PublicHostedZone(stack, 'HostedZone', { zoneName });

// THEN
expect(() => {
new route53.ARecord(zone, 'Alias', {
zone,
recordName: '_foo',
recordName,
target: route53.RecordTarget.fromAlias(new targets.BucketWebsiteTarget(bucketWebsite))
});
}).toThrow(/Cannot use an S3 record alias in region-agnostic stacks/);
Expand All @@ -104,13 +82,13 @@ test('throws if bucket website hosting is unavailable (cn-northwest-1)', () => {
const bucketWebsite = new s3.Bucket(stack, 'Bucket');

// WHEN
const zone = new route53.PublicHostedZone(stack, 'HostedZone', { zoneName: 'test.public' });
const zone = new route53.PublicHostedZone(stack, 'HostedZone', { zoneName });

// THEN
expect(() => {
new route53.ARecord(zone, 'Alias', {
zone,
recordName: '_foo',
recordName,
target: route53.RecordTarget.fromAlias(new targets.BucketWebsiteTarget(bucketWebsite))
});
}).toThrow(/Bucket website target is not supported/);
Expand Down