feat: add file asset support #106
Conversation
| private physicalNameOfBucket(bucket: IBucket) { | ||
| let resolvedName; | ||
| if (Resource.isOwnedResource(bucket)) { | ||
| resolvedName = Stack.of(bucket).resolve((bucket.node.defaultChild as CfnBucket).bucketName); |
There was a problem hiding this comment.
the service catalog L2 had this but i'm not sure why it is needed vs just using bucket.bucketName. this part fails an integration test i had when creating a bucket and passing to the construct vs doing a bucket import. it wasn't able to get the name from the token - but this logic works in an actual cdk deploy.
| import { Construct } from 'constructs'; | ||
|
|
||
| export const fileAssetResourceName = 'StackSetAssetsBucketDeployment'; |
There was a problem hiding this comment.
this is needed so we can export the construct name so we can find it after creating our StackSet resource so we can add a dependency. the bucket deployment must finish before the StackSet resource is created. Open to ideas on better naming conventions and/or better ways to accomplish this.
There was a problem hiding this comment.
We could make the bucketDeployment property public and then we could access
it.
(Stack.of(this).synthesizer as StackSetStackSynthesizer).bucketDeployment
|
|
||
| // the file asset bucket deployment needs to complete before the stackset can deploy | ||
| const fileAssetResource = scope.node.tryFindChild(fileAssetResourceName); | ||
| fileAssetResource && stackSet.node.addDependency(fileAssetResource); |
There was a problem hiding this comment.
i was thinking about forcing a dependency on all sibling resources to the StackSet resource here. but opted for this more targeted approach. but i still like the idea of forcing the StackSet resource to always be last in the deployment if there is a clean way to do that.
| @@ -5,12 +5,12 @@ const project = new CdklabsConstructLibrary({ | |||
| projenrcTs: true, | |||
| author: 'AWS', | |||
| authorAddress: 'aws-cdk-dev@amazon.com', | |||
| cdkVersion: '2.45.0', | |||
| cdkVersion: '2.77.0', | |||
There was a problem hiding this comment.
the bucket deployment process complained about the addSource call when building the bucket deployment object. upgrading the CDK version fixed that - but i'm not sure if this is truly required. but didn't think it would cause other harm.
|
Perhaps request @otaviomacedo and @corymhall to review? |
| import { Construct } from 'constructs'; | ||
|
|
||
| export const fileAssetResourceName = 'StackSetAssetsBucketDeployment'; |
There was a problem hiding this comment.
We could make the bucketDeployment property public and then we could access
it.
(Stack.of(this).synthesizer as StackSetStackSynthesizer).bucketDeployment
Fixes #58
Mimics file asset support from the Service Catalog L2 construct. Uses the S3 bucket deployment construct called in the parent stack to send artifacts to an S3 bucket passed a prop to the StackSet stack. The bucket deployment and resources needed to faciliate it are siblings to the
AWS::CloudFormation::StackSetresource in the synthesized CloudFormation template. The asset bucket must be outside the CDK-owned asset bucket process and is passed as a prop.We also need to ensure the bucket deployment process finishes before we try to run the StackSet so there is a dependency added after the creation of the StackSet resource. This is done using a construct name shared across the synthesis and StackSet creation processes. I'm open to feedback on if there is a better way to do this. But we do need to ensure the bucket deployment finishes before CloudFormation creates/updates the StackSet resource.
There is also potential to create another type of
StackSynthesizerand let StackSet and Service Catalog share it since this logic is very similar to how service catalog does it.I also plan to add some additional integration tests to validate more features of the file asset process but wanted to open the pull request to get eyes and feedback on the approach while finishing up the tests. I also think more scenarios could be added to the README to clear up any complexity.