Addressed comments

packages/@aws-cdk/aws-servicecatalog/README.md

@@ -209,7 +209,7 @@ class LambdaProduct extends servicecatalog.ProductStack {
 }
 
 const userDefinedBucket = new Bucket(this, `UserDefinedBucket`, {
-  assetBucketName: 'user-defined-bucket-for-product-stack-assets',
+  bucketName: 'user-defined-bucket-for-product-stack-assets',
 });
 
 const product = new servicecatalog.CloudFormationProduct(this, 'Product', {
@@ -231,6 +231,39 @@ will automatically grant read permissions to the spoke account.
 Note, it is not recommended using a referenced bucket as permissions cannot be added from CDK. 
 In this case, it will be your responsibility to grant read permissions for the asset bucket to
 the spoke account.
+If you want to provide your own bucket policy or scope down your bucket policy further to only allow 
+reads from a specific launch role, refer to the following example policy:
+
+```ts
+new iam.PolicyStatement({
+	actions: [
+		's3:GetObject*',
+		's3:GetBucket*',
+		's3:List*', ],
+	effect: iam.Effect.ALLOW,
+	resources: [
+		bucket.bucketArn,
+		bucket.arnForObjects('*'),
+	],
+	principals: [
+		new iam.ArnPrincipal(cdk.Stack.of(this).formatArn({
+        			service: 'iam',
+        			region: '',
+        			sharedAccount,
+        			resource: 'role',
+        			resourceName: launchRoleName,
+        		}))
+	],
+	conditions: {
+		'ForAnyValue:StringEquals': {
+			'aws:CalledVia': ['cloudformation.amazonaws.com'],
+		},
+		'Bool': {
+			'aws:ViaAWSService': true,
+		},
+	},
+});
+```
 
 Furthermore, in order for a spoke account to provision a product with an asset, the role launching 
 the product needs permissions to read from the asset bucket.

packages/@aws-cdk/aws-servicecatalog/lib/private/product-stack-synthesizer.ts

@@ -1,4 +1,5 @@
 import { CfnBucket, IBucket } from '@aws-cdk/aws-s3';
+import { BucketDeployment, ISource, Source } from '@aws-cdk/aws-s3-deployment';
 import * as cdk from '@aws-cdk/core';
 import { ProductStack } from '../product-stack';
 
@@ -9,10 +10,12 @@ import { ProductStack } from '../product-stack';
  */
 export class ProductStackSynthesizer extends cdk.StackSynthesizer {
   private readonly assetBucket?: IBucket;
+  private readonly assets: ISource[];
 
   constructor(assetBucket?: IBucket) {
     super();
     this.assetBucket = assetBucket;
+    this.assets = [];
   }
 
   public addFileAsset(asset: cdk.FileAssetSource): cdk.FileAssetLocation {
@@ -22,7 +25,7 @@ export class ProductStackSynthesizer extends cdk.StackSynthesizer {
 
     const physicalName = this.physicalNameOfBucket(this.assetBucket);
 
-    (this.boundStack as ProductStack)._addAsset(asset);
+    this._addAsset(asset);
 
     const bucketName = physicalName;
     const s3Filename = asset.fileName?.split('.')[1] + '.zip';
@@ -34,7 +37,12 @@ export class ProductStackSynthesizer extends cdk.StackSynthesizer {
   }
 
   private physicalNameOfBucket(bucket: IBucket) {
-    const resolvedName = cdk.Stack.of(bucket).resolve((bucket.node.defaultChild as CfnBucket).bucketName);
+    let resolvedName;
+    if (cdk.Resource.isOwnedResource(bucket)) {
+      resolvedName = cdk.Stack.of(bucket).resolve((bucket.node.defaultChild as CfnBucket).bucketName);
+    } else {
+      resolvedName = bucket.bucketName;
+    }
     if (resolvedName === undefined) {
       throw new Error('A bucketName must be provided to use Assets');
     }
@@ -45,9 +53,40 @@ export class ProductStackSynthesizer extends cdk.StackSynthesizer {
     throw new Error('Service Catalog Product Stacks cannot use Assets');
   }
 
+  /**
+   * Asset are prepared for bulk deployment to S3.
+   * @internal
+   */
+  public _addAsset(asset: cdk.FileAssetSource): void {
+    const outdir = cdk.App.of(this.boundStack)?.outdir ?? 'cdk.out';
+    const assetPath = `./${outdir}/${asset.fileName}`;
+    this.assets.push(Source.asset(assetPath));
+  }
+
+  /**
+   * Deploy all assets to S3.
+   * @internal
+   */
+  public _deployAssets() {
+    const parentStack = (this.boundStack as ProductStack)._getParentStack();
+    if (this.assetBucket && this.assets.length > 0) {
+      if (!cdk.Resource.isOwnedResource(this.assetBucket)) {
+        cdk.Annotations.of(parentStack).addWarning('[WARNING] Bucket Policy Permissions cannot be added to' +
+          ' referenced Bucket. Please make sure your bucket has the correct permissions');
+      }
+      new BucketDeployment(parentStack, 'AssetsBucketDeployment', {
+        sources: this.assets,
+        destinationBucket: this.assetBucket,
+        extract: false,
+        prune: false,
+      });
+    }
+  }
+
   public synthesize(session: cdk.ISynthesisSession): void {
     // Synthesize the template, but don't emit as a cloud assembly artifact.
     // It will be registered as an S3 asset of its parent instead.
+    this._deployAssets();
     this.synthesizeTemplate(session);
   }
 }

packages/@aws-cdk/aws-servicecatalog/lib/product-stack.ts

@@ -2,7 +2,6 @@ import * as crypto from 'crypto';
 import * as fs from 'fs';
 import * as path from 'path';
 import { IBucket } from '@aws-cdk/aws-s3';
-import { BucketDeployment, ISource, Source } from '@aws-cdk/aws-s3-deployment';
 import * as cdk from '@aws-cdk/core';
 import { Construct } from 'constructs';
 import { ProductStackSynthesizer } from './private/product-stack-synthesizer';
@@ -34,7 +33,6 @@ export class ProductStack extends cdk.Stack {
   private _templateUrl?: string;
   private _parentStack: cdk.Stack;
 
-  private readonly assets: ISource[];
   private assetBucket?: IBucket;
 
   constructor(scope: Construct, id: string, props: ProductStackProps = {}) {
@@ -47,7 +45,6 @@ export class ProductStack extends cdk.Stack {
     // this is the file name of the synthesized template file within the cloud assembly
     this.templateFile = `${cdk.Names.uniqueId(this)}.product.template.json`;
 
-    this.assets = [];
     this.assetBucket = props.assetBucket;
   }
 
@@ -79,32 +76,12 @@ export class ProductStack extends cdk.Stack {
   }
 
   /**
-   * Asset are prepared for bulk deployment to S3.
-   * @internal
-   */
-  public _addAsset(asset: cdk.FileAssetSource): void {
-    const assetPath = './cdk.out/' + asset.fileName;
-    this.assets.push(Source.asset(assetPath));
-  }
-
-  /**
-   * Deploy all assets to S3.
+   * Fetch the parent Stack.
+   *
    * @internal
    */
-  private _deployAssets() {
-    if (this.assetBucket && this.assets.length > 0) {
-      if (!cdk.Resource.isOwnedResource(this.assetBucket)) {
-        // eslint-disable-next-line no-console
-        console.warn('[WARNING]', 'Bucket Policy Permissions cannot be added to' +
-          ' referenced Bucket. Please make sure your bucket has the correct permissions');
-      }
-      new BucketDeployment(this._parentStack, 'AssetsBucketDeployment', {
-        sources: this.assets,
-        destinationBucket: this.assetBucket,
-        extract: false,
-        prune: false,
-      });
-    }
+  public _getParentStack(): cdk.Stack {
+    return this._parentStack;
   }
 
   /**
@@ -129,8 +106,6 @@ export class ProductStack extends cdk.Stack {
       this._parentProductStackHistory._writeTemplateToSnapshot(cfn);
     }
 
-    this._deployAssets();
-
     fs.writeFileSync(path.join(session.assembly.outdir, this.templateFile), cfn);
   }
 }

packages/@aws-cdk/aws-servicecatalog/package.json

@@ -85,10 +85,12 @@
     "@aws-cdk/integ-runner": "0.0.0",
     "@aws-cdk/cfn2ts": "0.0.0",
     "@aws-cdk/pkglint": "0.0.0",
+    "@aws-cdk/integ-tests": "0.0.0",
     "@types/jest": "^27.5.2"
   },
   "dependencies": {
     "@aws-cdk/aws-iam": "0.0.0",
+    "@aws-cdk/aws-lambda": "0.0.0",
     "@aws-cdk/aws-s3": "0.0.0",
     "@aws-cdk/aws-s3-assets": "0.0.0",
     "@aws-cdk/aws-s3-deployment": "0.0.0",
@@ -99,6 +101,7 @@
   "homepage": "https://github.com/aws/aws-cdk",
   "peerDependencies": {
     "@aws-cdk/aws-iam": "0.0.0",
+    "@aws-cdk/aws-lambda": "0.0.0",
     "@aws-cdk/aws-s3": "0.0.0",
     "@aws-cdk/aws-s3-assets": "0.0.0",
     "@aws-cdk/aws-s3-deployment": "0.0.0",

packages/@aws-cdk/aws-servicecatalog/test/assets/index.py

@@ -0,0 +1,7 @@
+import json
+
+def handler(event, context):
+    return {
+        'statusCode': 200,
+        'body': json.dumps('Hello from Lambda!')
+    }

packages/@aws-cdk/aws-servicecatalog/test/integ.product.js.snapshot/asset.6412a5f4524c6b41d26fbeee226c68c2dad735393940a51008d77e6f8b1038f5.json

@@ -0,0 +1,98 @@
+{
+  "AWSTemplateFormatVersion" : "2010-09-09",
+
+  "Description" : "AWS Service Catalog sample template. Creates an Amazon EC2 instance running the Amazon Linux AMI. The AMI is chosen based on the region in which the stack is run. This example creates an EC2 security group for the instance to give you SSH access. **WARNING** This template creates an Amazon EC2 instance. You will be billed for the AWS resources used if you create a stack from this template.",
+
+  "Parameters" : {
+    "KeyName": {
+      "Description" : "Name of an existing EC2 key pair for SSH access to the EC2 instance.",
+      "Type": "AWS::EC2::KeyPair::KeyName"
+    },
+
+    "InstanceType" : {
+      "Description" : "EC2 instance type.",
+      "Type" : "String",
+      "Default" : "t2.micro",
+      "AllowedValues" : [ "t2.micro", "t2.small", "t2.medium"]
+    },
+
+    "SSHLocation" : {
+      "Description" : "The IP address range that can SSH to the EC2 instance.",
+      "Type": "String",
+      "MinLength": "9",
+      "MaxLength": "18",
+      "Default": "0.0.0.0/0",
+      "AllowedPattern": "(\\d{1,3})\\.(\\d{1,3})\\.(\\d{1,3})\\.(\\d{1,3})/(\\d{1,2})",
+      "ConstraintDescription": "Must be a valid IP CIDR range of the form x.x.x.x/x."
+   }
+  },
+
+  "Metadata" : {
+    "AWS::CloudFormation::Interface" : {
+      "ParameterGroups" : [{
+        "Label" : {"default": "Instance configuration"},
+        "Parameters" : ["InstanceType"]
+      },{
+        "Label" : {"default": "Security configuration"},
+        "Parameters" : ["KeyName", "SSHLocation"]
+      }],
+      "ParameterLabels" : {
+        "InstanceType": {"default": "Server size:"},
+        "KeyName": {"default": "Key pair:"},
+        "SSHLocation": {"default": "CIDR range:"}
+      }
+    }
+  },
+
+  "Mappings" : {
+    "AWSRegionArch2AMI" : {
+      "us-east-1"      : { "HVM64" : "ami-08842d60" },
+      "us-west-2"      : { "HVM64" : "ami-8786c6b7" },
+      "us-west-1"      : { "HVM64" : "ami-cfa8a18a" },
+      "eu-west-1"      : { "HVM64" : "ami-748e2903" },
+      "ap-southeast-1" : { "HVM64" : "ami-d6e1c584" },
+      "ap-northeast-1" : { "HVM64" : "ami-35072834" },
+      "ap-southeast-2" : { "HVM64" : "ami-fd4724c7" },
+      "sa-east-1"      : { "HVM64" : "ami-956cc688" },
+      "cn-north-1"     : { "HVM64" : "ami-ac57c595" },
+      "eu-central-1"   : { "HVM64" : "ami-b43503a9" }
+    }
+
+  },
+
+  "Resources" : {
+    "EC2Instance" : {
+      "Type" : "AWS::EC2::Instance",
+      "Properties" : {
+        "InstanceType" : { "Ref" : "InstanceType" },
+        "SecurityGroups" : [ { "Ref" : "InstanceSecurityGroup" } ],
+        "KeyName" : { "Ref" : "KeyName" },
+        "ImageId" : { "Fn::FindInMap" : [ "AWSRegionArch2AMI", { "Ref" : "AWS::Region" }, "HVM64" ] }
+      }
+    },
+
+    "InstanceSecurityGroup" : {
+      "Type" : "AWS::EC2::SecurityGroup",
+      "Properties" : {
+        "GroupDescription" : "Enable SSH access via port 22",
+        "SecurityGroupIngress" : [ {
+          "IpProtocol" : "tcp",
+          "FromPort" : "22",
+          "ToPort" : "22",
+          "CidrIp" : { "Ref" : "SSHLocation"}
+        } ]
+      }
+    }
+  },
+
+  "Outputs" : {
+    "PublicDNSName" : {
+      "Description" : "Public DNS name of the new EC2 instance",
+      "Value" : { "Fn::GetAtt" : [ "EC2Instance", "PublicDnsName" ] }
+    },
+    "PublicIPAddress" : {
+      "Description" : "Public IP address of the new EC2 instance",
+      "Value" : { "Fn::GetAtt" : [ "EC2Instance", "PublicIp" ] }
+    }
+  }
+}