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

feat(s3): export bucket websiteURL (#1521) #1522

Closed
wants to merge 26 commits into from
Closed

feat(s3): export bucket websiteURL (#1521) #1522

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
497620d
feat(s3): export bucket websiteURL
AlexChesters Jan 11, 2019
f6a3bfa
fix
AlexChesters Jan 11, 2019
c2aeebd
rename bucketWebsiteURL to bucketWebsiteUrl
AlexChesters Jan 14, 2019
a36ac08
update implementation to return WebsiteURL value
AlexChesters Jan 14, 2019
7bafbff
add unit test
AlexChesters Jan 14, 2019
1724161
feat(apigateway): add tracingEnabled property to APIGW Stage (#1482)
workeitel Jan 6, 2019
a4645f2
feat(aws-ecs): add support Amazon Linux 2 (#1484)
cohalz Jan 6, 2019
b22cf0c
feat(aws-kms): allow tagging kms keys (#1485)
rsmogura Jan 7, 2019
19c9392
chore: Document return behavior of cdk diff (#1446)
RomainMuller Jan 7, 2019
5096201
fix(elbv2): unable to specify load balancer name (#1486)
Jan 7, 2019
9bae09b
feat(diff): Better diff of random objects (#1488)
RomainMuller Jan 7, 2019
be19cc5
feat(aws-codepipeline): Jenkins build and test Actions. (#1216)
skinny85 Jan 7, 2019
81a5a07
feat(route53): support CNAME records (#1487)
Jan 8, 2019
ed5a38b
feat: no more generated attribute types in CFN layer (L1) (#1489)
Jan 8, 2019
ccd01e2
feat: stop generating legacy cloudformation resources (#1493)
Jan 8, 2019
b1f5b2e
feat: cloudformation condition chaining (#1494)
Jan 8, 2019
9cb842f
feat(step-functions): support parameters option (#1492)
guisrx Jan 8, 2019
f393a26
feat(cdk): transparently use constructs from another stack
rix0rrr Jan 9, 2019
6ad8c5e
fix(lambda): use IRole instead of Role to allow imports (#1509)
Jan 10, 2019
0fb62c8
fix(core): automatic cross-stack refs for CFN resources (#1510)
Jan 10, 2019
3961ad7
feat(aws-cdk): better stack dependency handling (#1511)
rix0rrr Jan 10, 2019
01d72cf
v0.22.0 (#1518)
Jan 11, 2019
948b5df
add packages.txt to github pages (#1520)
Jan 11, 2019
4443d5c
chore(scripts): Add Jetbrains node_modules exclusion script (#1507)
costleya Jan 13, 2019
582c8b1
feat(ecs): environment variables for LoadBalancedXxxService (#1537)
PaulMaddox Jan 14, 2019
c79d234
chore(cdk): make `isInstance` static methods more concrete (#1519)
rix0rrr Jan 14, 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
10 changes: 9 additions & 1 deletion packages/@aws-cdk/aws-s3/lib/bucket.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export interface IBucket extends cdk.IConstruct {
*/
export interface BucketImportProps {
/**
* The ARN fo the bucket. At least one of bucketArn or bucketName must be
* The ARN of the bucket. At least one of bucketArn or bucketName must be
* defined in order to initialize a bucket ref.
*/
bucketArn?: string;
Expand All @@ -199,6 +199,13 @@ export interface BucketImportProps {
* @default Inferred from bucket name
*/
bucketDomainName?: string;

/**
* The website URL of the bucket (if static web hosting is enabled).
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'm not sure how useful this description is, I tried a couple of different variations but couldn't come up with anything better than this.

Suggestions welcome!

*
* @default Inferred from bucket name
*/
bucketWebsiteURL?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename to bucketWebsiteUrl

Copy link
Contributor

Choose a reason for hiding this comment

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

What's the difference between this and domainName? I also can't seem to find the actual implementation in Bucket.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review @eladb 😄

I've pushed the change to rename to bucketWebsiteUrl now.

Ah, I've just spotted that this actually will be identical to domainName. Good spot, I'll push a fix for that.

}

/**
Expand Down Expand Up @@ -620,6 +627,7 @@ export class Bucket extends BucketBase {
bucketArn: new cdk.Output(this, 'BucketArn', { value: this.bucketArn }).makeImportValue().toString(),
bucketName: new cdk.Output(this, 'BucketName', { value: this.bucketName }).makeImportValue().toString(),
bucketDomainName: new cdk.Output(this, 'DomainName', { value: this.domainName }).makeImportValue().toString(),
bucketWebsiteURL: new cdk.Output(this, 'WebsiteURL', { value: this.domainName }).makeImportValue().toString()
};
}

Expand Down
28 changes: 26 additions & 2 deletions packages/@aws-cdk/aws-s3/test/test.bucket.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,8 @@ export = {
test.deepEqual(cdk.resolve(bucketRef), {
bucketArn: { 'Fn::ImportValue': 'MyStack:MyBucketBucketArnE260558C' },
bucketName: { 'Fn::ImportValue': 'MyStack:MyBucketBucketName8A027014' },
bucketDomainName: { 'Fn::ImportValue': 'MyStack:MyBucketDomainNameF76B9A7A' }
bucketDomainName: { 'Fn::ImportValue': 'MyStack:MyBucketDomainNameF76B9A7A' },
bucketWebsiteURL: { 'Fn::ImportValue': 'MyStack:MyBucketWebsiteURL9C222788' }
});
test.done();
},
Expand All @@ -346,7 +347,8 @@ export = {
test.deepEqual(cdk.resolve(bucketRef), {
bucketArn: { 'Fn::ImportValue': 'MyStack:MyBucketBucketArnE260558C' },
bucketName: { 'Fn::ImportValue': 'MyStack:MyBucketBucketName8A027014' },
bucketDomainName: { 'Fn::ImportValue': 'MyStack:MyBucketDomainNameF76B9A7A' }
bucketDomainName: { 'Fn::ImportValue': 'MyStack:MyBucketDomainNameF76B9A7A' },
bucketWebsiteURL: { 'Fn::ImportValue': 'MyStack:MyBucketWebsiteURL9C222788' }
});
test.done();
},
Expand Down Expand Up @@ -465,6 +467,17 @@ export = {
"Export": {
"Name": "S1:MyBucketDomainNameF76B9A7A"
}
},
"MyBucketWebsiteURL9C222788": {
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 found the logical ID here (i.e. MyBucketWebsiteURL9C222788) from running the tests, watching them fail and observing the output.

I'm not sure if I was meant to have determined the output from somewhere else?

"Value": {
"Fn::GetAtt": [
"MyBucketF68F3FF0",
"DomainName"
]
},
"Export": {
"Name": "S1:MyBucketWebsiteURL9C222788"
}
}
}
});
Expand Down Expand Up @@ -898,6 +911,17 @@ export = {
"Export": {
"Name": "MyBucketDomainNameF76B9A7A"
}
},
"MyBucketWebsiteURL9C222788": {
"Value": {
"Fn::GetAtt": [
"MyBucketF68F3FF0",
"DomainName"
]
},
"Export": {
"Name": "MyBucketWebsiteURL9C222788"
}
}
}
});
Expand Down