Skip to content

Commit

Permalink
chore(core): fix bug introduced in recent PR (#12880)
Browse files Browse the repository at this point in the history
In #12765, a bug was introduced where there was a mismatch between
the file entry written to the asset manifest for the stack template,
and the template location written to the cloud assembly, thereby
causing all deployments to fail.

The error was "Access Denied" (because that's what S3 says when it
doesn't want to tell you "File not found").

Fix the inconsistency.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
rix0rrr authored Feb 5, 2021
1 parent 22eb7e1 commit b4e1b68
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,12 @@ export class DefaultStackSynthesizer extends StackSynthesizer {
//
// Instead, we'll have a protocol with the CLI that we put an 's3://.../...' URL here, and the CLI
// is going to resolve it to the correct 'https://.../' URL before it gives it to CloudFormation.
return `s3://${this.bucketName}/${this.bucketPrefix}${sourceHash}`;
//
// ALSO: it would be great to reuse the return value of `addFileAsset()` here, except those contain
// CloudFormation REFERENCES to locations, not actual locations (can contain `{ Ref: AWS::Region }` and
// `{ Ref: SomeParameter }` etc). We therefore have to duplicate some logic here :(.
const extension = path.extname(this.stack.templateFile);
return `s3://${this.bucketName}/${this.bucketPrefix}${sourceHash}${extension}`;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ nodeunitShim({
// THEN -- the S3 url is advertised on the stack artifact
const stackArtifact = asm.getStackArtifact('Stack');

const templateHash = last(stackArtifact.stackTemplateAssetObjectUrl?.split('/'));
const templateObjectKey = last(stackArtifact.stackTemplateAssetObjectUrl?.split('/'));

test.equals(stackArtifact.stackTemplateAssetObjectUrl, `s3://cdk-hnb659fds-assets-\${AWS::AccountId}-\${AWS::Region}/${templateHash}`);
test.equals(stackArtifact.stackTemplateAssetObjectUrl, `s3://cdk-hnb659fds-assets-\${AWS::AccountId}-\${AWS::Region}/${templateObjectKey}`);

// THEN - the template is in the asset manifest
const manifestArtifact = asm.artifacts.filter(isAssetManifest)[0];
Expand All @@ -52,7 +52,7 @@ nodeunitShim({
destinations: {
'current_account-current_region': {
bucketName: 'cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}',
objectKey: templateHash + '.json',
objectKey: templateObjectKey,
assumeRoleArn: 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}',
},
},
Expand Down

0 comments on commit b4e1b68

Please sign in to comment.