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

aws-s3-deployment: Wrong URL in BucketDeployment.deployedBucket.bucketWebsiteUrl #23354

Assignees
Labels
@aws-cdk/aws-s3-deployment bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@iwasinnam
Copy link

Describe the bug

I deploy a static website using s3 deployment, and print the endpoint from BucketDeployment.deployedBucket.bucketWebsiteUrl. When deploying to Frankfurt region the URL I get http://my-sweet-stack-123456789-xxxxxxxxxx.s3-website-eu-central-1.amazonaws.com/

That is, the URL I received has a hyphen before the region name, but the actual endpoint is actually with a dot (...s3-website.eu-central-1...)

Expected Behavior

The actual URL is http://my-sweet-stack-123456789-xxxxxxxxxx.s3-website.eu-central-1.amazonaws.com/

As is indeed documented here: Amazon S3 website endpoints

Current Behavior

The URL in BucketDeployment.deployedBucket.bucketWebsiteUrl is wrong and leads nowhere.

Reproduction Steps

  1. Deploy a static website to eu-central-1 region
  2. Print .deployedBucket.bucketWebsiteUrl of the bucket you just deployed
  3. Compare to the URL that appears in the console of S3 (bucket -> Properties -> Static web hosting)
  4. They're different!

Possible Solution

If had to guess I'd say that either there's a typo in the table that maps regions to static hosting endpoint URL format, or there is no such table.

Additional Information/Context

No response

CDK CLI Version

2.54.0 (build 9f41881)

Framework Version

No response

Node.js Version

v12.22.9

OS

Ubuntu 22.04.1

Language

Typescript

Language Version

No response

Other information

No response

@iwasinnam iwasinnam added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 15, 2022
@peterwoodworth
Copy link
Contributor

When we return the deployedBucket prop, we are returning the following:

public get deployedBucket(): s3.IBucket {
this.requestDestinationArn = true;
this._deployedBucket = this._deployedBucket ?? s3.Bucket.fromBucketArn(this, 'DestinationBucket', cdk.Token.asString(this.cr.getAtt('DestinationBucketArn')));
return this._deployedBucket;
}

I can't see anywhere else where we are setting _deployedBucket, so this should always return an import Bucket.fromBucketArn

fromBucketArn will call fromBucketAttributes under the hood, only supplying the bucketArn attribute. You can see here how fromBucketAttributes calculates its other properties by default:

const newUrlFormat = attrs.bucketWebsiteNewUrlFormat === undefined
? false
: attrs.bucketWebsiteNewUrlFormat;
const websiteDomain = newUrlFormat
? `${bucketName}.s3-website.${region}.${urlSuffix}`
: `${bucketName}.s3-website-${region}.${urlSuffix}`;
class Import extends BucketBase {
public readonly bucketName = bucketName!;
public readonly bucketArn = parseBucketArn(scope, attrs);
public readonly bucketDomainName = attrs.bucketDomainName || `${bucketName}.s3.${urlSuffix}`;
public readonly bucketWebsiteUrl = attrs.bucketWebsiteUrl || `http://${websiteDomain}`;

websiteDomain will be set to ${bucketName}.s3-website-${region}.${urlSuffix} because newUrlFormat is disabled by default. Then bucketWebsiteUrl will be set to this websiteDomain: http://${websiteDomain}

We should be able to calculate this ourselves based on the region using the docs you linked. And if not, we should offer a way to override this setting. Thanks for reporting!

@peterwoodworth peterwoodworth added p1 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Dec 22, 2022
@mrgrain
Copy link
Contributor

mrgrain commented Feb 1, 2023

@peterwoodworth I don't think we can calculate this just from the ARN. Unfortunately Bucket ARNs do not contain a region. Wo do you think?
https://docs.aws.amazon.com/AmazonS3/latest/userguide/s3-arn-format.html

@mrgrain
Copy link
Contributor

mrgrain commented Feb 7, 2023

#23919 will fix this for scenarios where the destination bucket is in the same region as the stack.

mrgrain added a commit to mrgrain/aws-cdk that referenced this issue Feb 7, 2023
mrgrain added a commit to mrgrain/aws-cdk that referenced this issue Feb 7, 2023
mrgrain added a commit to mrgrain/aws-cdk that referenced this issue Feb 8, 2023
mrgrain added a commit to mrgrain/aws-cdk that referenced this issue Feb 8, 2023
mrgrain added a commit to mrgrain/aws-cdk that referenced this issue Feb 8, 2023
@mergify mergify bot closed this as completed in #24055 Feb 8, 2023
mergify bot pushed a commit that referenced this issue Feb 8, 2023
…etWebsiteUrl (#24055)

Fixes #23354

Without pass-through of all attribute values, it is currently not possible to automatically force a dependency on the deployment for every attribute.
This change merely sets the bucket's region & account, so that all computed website/domain attributes will now include the correct values. Other attributes, and manually set website/domain attributes are not supported.
Additionally the documentation has been extended to highlight the above issue and provide a workaround.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Feb 8, 2023

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment