fix(servicecatalog): support nested stacks in product stacks

[colifran]

This PR fixes a bug that prevented a nested stack from being used within a product stack. The logic in the addFileAsset method as part of the ProductStackSynthesizer was updated to forward all assets to the parent stack and uses the FileAssetLocation returned from the parent stack synthesizer to create a bucket Source from the bucketName and objectKey.

Closes #24317

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[gitpod-io]

[mrgrain]

This okay in principle, but we tend to prefer props objects.
Because I have just approved an other PR that also adds stuff to this: https://github.com/aws/aws-cdk/pull/26303/files#diff-26862b8f0be4955057aed48a0d3dfabefaa27d96e95d3c5c99a3a85cc10ce770R18

Can you rebase your PR and then turn this whole thing into a prop object please?

[colifran]

This has been updated. I created an interface named ProductStackSynthesizerProps and I moved all constructor properties inside of that.

[mrgrain]

Suggested change

	constructor(private readonly parentDeployment: cdk.IStackSynthesizer, private readonly assetBucket?: IBucket) {
	constructor(private readonly parentStack: cdk.Stack, private readonly assetBucket?: IBucket) {

I know this is only an internal class, but I wonder if passing the parent stack is a bit cleaner as an API. Passing a synthesizer feels like we are implying that any synthesizer could be passed in here.

[colifran]

I've updated the code to pass the parentStack as part of the newly added ProductStackSynthesizerProps

[mrgrain]

I'm sure this is needed.... but why?

[colifran]

Good question.

TL;DR - this is needed to ensure the nested stack template file is added as a Source to the BucketDeployment.

Take this code block as an example:

class SampleNestedStack extends cdk.NestedStack {
      constructor(scope: Construct, id: string) {
        super(scope, id);

        new lambda.Function(this, 'HelloHandler1', {
          runtime: lambda.Runtime.PYTHON_3_9,
          code: lambda.Code.fromAsset(path.join(__dirname, 'assets')),
          handler: 'index.handler',
        });
        
        new lambda.Function(this, 'HelloHandler2', {
          runtime: lambda.Runtime.PYTHON_3_9,
          code: lambda.Code.fromAsset(path.join(__dirname, 'assetsv2')),
          handler: 'index.handler',
        });
      }
    }

Note: I'm not a Lazy expert, but the process is something like this. I had some help from @rix0rrr to figure this part out.

First, the Code assets from the two lambda Functions will be added as Sources to the BucketDeployment. Since these are evaluated with Lazy they are added to a cache before evaluation is carried out. At the point when the NestedStack is ready to add its own template, the Lazy logic has already assumed all assets have been added to the cache. This results in the template file for NestedStack being left out when SourceBucketNames, SourceObjectKeys, and SourceMarkers is evaluated. Lazy.uncachedList is basically saying don't only rely on the cache, there may be more items to evaluate.

[rix0rrr]

Btw, probably not necessary for the markers. Doesn’t really harm, but maybe put it back? And add a comment saying that its uncached because the contents may change for nestedstacks in a productstacj

[colifran]

I didn't think so either, but it actually is. An error is thrown if SourceObjectKeys length isn't equal to SourceMarkers length and it'll be off by 1 if List.any is used.

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 1107ef2
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).