chore(bootstrap): split file/image publishing roles (#8403)

For security purposes, we decided that it would be lower risk to assume a different role when we publish S3 assets and when we publish ECR assets. The reason is that ECR publishers execute `docker build` which can potentially execute 3rd party code (via a base docker image).

This change modifies the conventional name for the publishing roles as well as adds a set of properties to the `DefaultStackSynthesizer` to allow customization as needed.

This is a resubmission of #8319. That one was failing backwards regression tests... and for good reason! However in this case, the regression was intended (and deemed acceptable since we haven't officially "released" the feature we're breaking yet).

Unfortunately the mechanism to skip integration tests during the regression tests has been broken recently, so had to be reintroduced here.

----

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

allowed-breaking-changes.txt

@@ -1 +1,5 @@
+removed:@aws-cdk/core.BootstraplessSynthesizer.DEFAULT_ASSET_PUBLISHING_ROLE_ARN
+removed:@aws-cdk/core.DefaultStackSynthesizer.DEFAULT_ASSET_PUBLISHING_ROLE_ARN
+removed:@aws-cdk/core.DefaultStackSynthesizerProps.assetPublishingExternalId
+removed:@aws-cdk/core.DefaultStackSynthesizerProps.assetPublishingRoleArn
 

packages/@aws-cdk/core/lib/stack-synthesizers/default-synthesizer.ts

@@ -13,6 +13,11 @@ import { IStackSynthesizer } from './types';
 
 export const BOOTSTRAP_QUALIFIER_CONTEXT = '@aws-cdk/core:bootstrapQualifier';
 
+/**
+ * The minimum bootstrap stack version required by this app.
+ */
+const MIN_BOOTSTRAP_STACK_VERSION = 2;
+
 /**
  * Configuration properties for DefaultStackSynthesizer
  */
@@ -44,24 +49,44 @@ export interface DefaultStackSynthesizerProps {
   readonly imageAssetsRepositoryName?: string;
 
   /**
-   * The role to use to publish assets to this environment
+   * The role to use to publish file assets to the S3 bucket in this environment
    *
    * You must supply this if you have given a non-standard name to the publishing role.
    *
    * The placeholders `${Qualifier}`, `${AWS::AccountId}` and `${AWS::Region}` will
    * be replaced with the values of qualifier and the stack's account and region,
    * respectively.
    *
-   * @default DefaultStackSynthesizer.DEFAULT_ASSET_PUBLISHING_ROLE_ARN
+   * @default DefaultStackSynthesizer.DEFAULT_FILE_ASSET_PUBLISHING_ROLE_ARN
    */
-  readonly assetPublishingRoleArn?: string;
+  readonly fileAssetPublishingRoleArn?: string;
 
   /**
-   * External ID to use when assuming role for asset publishing
+   * External ID to use when assuming role for file asset publishing
    *
    * @default - No external ID
    */
-  readonly assetPublishingExternalId?: string;
+  readonly fileAssetPublishingExternalId?: string;
+
+  /**
+   * The role to use to publish image assets to the ECR repository in this environment
+   *
+   * You must supply this if you have given a non-standard name to the publishing role.
+   *
+   * The placeholders `${Qualifier}`, `${AWS::AccountId}` and `${AWS::Region}` will
+   * be replaced with the values of qualifier and the stack's account and region,
+   * respectively.
+   *
+   * @default DefaultStackSynthesizer.DEFAULT_IMAGE_ASSET_PUBLISHING_ROLE_ARN
+   */
+  readonly imageAssetPublishingRoleArn?: string;
+
+  /**
+   * External ID to use when assuming role for image asset publishing
+   *
+   * @default - No external ID
+   */
+  readonly imageAssetPublishingExternalId?: string;
 
   /**
    * The role to assume to initiate a deployment in this environment
@@ -126,9 +151,14 @@ export class DefaultStackSynthesizer implements IStackSynthesizer {
   public static readonly DEFAULT_DEPLOY_ROLE_ARN = 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-${Qualifier}-deploy-role-${AWS::AccountId}-${AWS::Region}';
 
   /**
-   * Default asset publishing role ARN.
+   * Default asset publishing role ARN for file (S3) assets.
+   */
+  public static readonly DEFAULT_FILE_ASSET_PUBLISHING_ROLE_ARN = 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-${Qualifier}-file-publishing-role-${AWS::AccountId}-${AWS::Region}';
+
+  /**
+   * Default asset publishing role ARN for image (ECR) assets.
    */
-  public static readonly DEFAULT_ASSET_PUBLISHING_ROLE_ARN = 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-${Qualifier}-publishing-role-${AWS::AccountId}-${AWS::Region}';
+  public static readonly DEFAULT_IMAGE_ASSET_PUBLISHING_ROLE_ARN = 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-${Qualifier}-image-publishing-role-${AWS::AccountId}-${AWS::Region}';
 
   /**
    * Default image assets repository name
@@ -145,7 +175,8 @@ export class DefaultStackSynthesizer implements IStackSynthesizer {
   private repositoryName?: string;
   private _deployRoleArn?: string;
   private _cloudFormationExecutionRoleArn?: string;
-  private assetPublishingRoleArn?: string;
+  private fileAssetPublishingRoleArn?: string;
+  private imageAssetPublishingRoleArn?: string;
 
   private readonly files: NonNullable<asset_schema.ManifestFile['files']> = {};
   private readonly dockerImages: NonNullable<asset_schema.ManifestFile['dockerImages']> = {};
@@ -178,7 +209,8 @@ export class DefaultStackSynthesizer implements IStackSynthesizer {
     this.repositoryName = specialize(this.props.imageAssetsRepositoryName ?? DefaultStackSynthesizer.DEFAULT_IMAGE_ASSETS_REPOSITORY_NAME);
     this._deployRoleArn = specialize(this.props.deployRoleArn ?? DefaultStackSynthesizer.DEFAULT_DEPLOY_ROLE_ARN);
     this._cloudFormationExecutionRoleArn = specialize(this.props.cloudFormationExecutionRole ?? DefaultStackSynthesizer.DEFAULT_CLOUDFORMATION_ROLE_ARN);
-    this.assetPublishingRoleArn = specialize(this.props.assetPublishingRoleArn ?? DefaultStackSynthesizer.DEFAULT_ASSET_PUBLISHING_ROLE_ARN);
+    this.fileAssetPublishingRoleArn = specialize(this.props.fileAssetPublishingRoleArn ?? DefaultStackSynthesizer.DEFAULT_FILE_ASSET_PUBLISHING_ROLE_ARN);
+    this.imageAssetPublishingRoleArn = specialize(this.props.imageAssetPublishingRoleArn ?? DefaultStackSynthesizer.DEFAULT_IMAGE_ASSET_PUBLISHING_ROLE_ARN);
     // tslint:enable:max-line-length
   }
 
@@ -199,8 +231,8 @@ export class DefaultStackSynthesizer implements IStackSynthesizer {
           bucketName: this.bucketName,
           objectKey,
           region: resolvedOr(this.stack.region, undefined),
-          assumeRoleArn: this.assetPublishingRoleArn,
-          assumeRoleExternalId: this.props.assetPublishingExternalId,
+          assumeRoleArn: this.fileAssetPublishingRoleArn,
+          assumeRoleExternalId: this.props.fileAssetPublishingExternalId,
         },
       },
     };
@@ -237,8 +269,8 @@ export class DefaultStackSynthesizer implements IStackSynthesizer {
           repositoryName: this.repositoryName,
           imageTag,
           region: resolvedOr(this.stack.region, undefined),
-          assumeRoleArn: this.assetPublishingRoleArn,
-          assumeRoleExternalId: this.props.assetPublishingExternalId,
+          assumeRoleArn: this.imageAssetPublishingRoleArn,
+          assumeRoleExternalId: this.props.imageAssetPublishingExternalId,
         },
       },
     };
@@ -262,7 +294,7 @@ export class DefaultStackSynthesizer implements IStackSynthesizer {
       assumeRoleArn: this._deployRoleArn,
       cloudFormationExecutionRoleArn: this._cloudFormationExecutionRoleArn,
       stackTemplateAssetObjectUrl: templateManifestUrl,
-      requiresBootstrapStackVersion: 1,
+      requiresBootstrapStackVersion: MIN_BOOTSTRAP_STACK_VERSION,
     }, [artifactId]);
   }
 
@@ -344,7 +376,7 @@ export class DefaultStackSynthesizer implements IStackSynthesizer {
       type: cxschema.ArtifactType.ASSET_MANIFEST,
       properties: {
         file: manifestFile,
-        requiresBootstrapStackVersion: 1,
+        requiresBootstrapStackVersion: MIN_BOOTSTRAP_STACK_VERSION,
       },
     });
 

packages/@aws-cdk/core/test/stack-synthesis/test.new-style-synthesis.ts

@@ -2,7 +2,7 @@ import * as asset_schema from '@aws-cdk/cdk-assets-schema';
 import * as cxapi from '@aws-cdk/cx-api';
 import * as fs from 'fs';
 import { Test } from 'nodeunit';
-import { App, CfnResource, FileAssetPackaging, Stack } from '../../lib';
+import { App, CfnResource, DefaultStackSynthesizer, FileAssetPackaging, Stack } from '../../lib';
 import { evaluateCFN } from '../evaluate-cfn';
 
 const CFN_CONTEXT = {
@@ -50,7 +50,7 @@ export = {
         'current_account-current_region': {
           bucketName: 'cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}',
           objectKey: '4bdae6e3b1b15f08c889d6c9133f24731ee14827a9a9ab9b6b6a9b42b6d34910',
-          assumeRoleArn: 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-publishing-role-${AWS::AccountId}-${AWS::Region}',
+          assumeRoleArn: 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}',
         },
       },
     });
@@ -106,22 +106,75 @@ export = {
     const asm = app.synth();
 
     // THEN - we have an asset manifest with both assets and the stack template in there
-    const manifestArtifact = asm.artifacts.filter(isAssetManifest)[0];
-    test.ok(manifestArtifact);
-    const manifest: asset_schema.ManifestFile = JSON.parse(fs.readFileSync(manifestArtifact.file, { encoding: 'utf-8' }));
+    const manifest = readAssetManifest(asm);
 
     test.equals(Object.keys(manifest.files || {}).length, 2);
     test.equals(Object.keys(manifest.dockerImages || {}).length, 1);
 
     // THEN - every artifact has an assumeRoleArn
-    for (const file of Object.values({...manifest.files, ...manifest.dockerImages})) {
+    for (const file of Object.values(manifest.files ?? {})) {
+      for (const destination of Object.values(file.destinations)) {
+        test.deepEqual(destination.assumeRoleArn, 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}');
+      }
+    }
+
+    for (const file of Object.values(manifest.dockerImages ?? {})) {
       for (const destination of Object.values(file.destinations)) {
-        test.ok(destination.assumeRoleArn);
+        test.deepEqual(destination.assumeRoleArn, 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-image-publishing-role-${AWS::AccountId}-${AWS::Region}');
       }
     }
 
     test.done();
   },
+
+  'customize publishing resources'(test: Test) {
+    // GIVEN
+    const myapp = new App();
+
+    // WHEN
+    const mystack = new Stack(myapp, 'mystack', {
+      synthesizer: new DefaultStackSynthesizer({
+        fileAssetsBucketName: 'file-asset-bucket',
+        fileAssetPublishingRoleArn: 'file:role:arn',
+        fileAssetPublishingExternalId: 'file-external-id',
+
+        imageAssetsRepositoryName: 'image-ecr-repository',
+        imageAssetPublishingRoleArn: 'image:role:arn',
+        imageAssetPublishingExternalId: 'image-external-id',
+      }),
+    });
+
+    mystack.synthesizer.addFileAsset({
+      fileName: __filename,
+      packaging: FileAssetPackaging.FILE,
+      sourceHash: 'file-asset-hash',
+    });
+
+    mystack.synthesizer.addDockerImageAsset({
+      directoryName: '.',
+      sourceHash: 'docker-asset-hash',
+    });
+
+    // THEN
+    const asm = myapp.synth();
+    const manifest = readAssetManifest(asm);
+
+    test.deepEqual(manifest.files?.['file-asset-hash']?.destinations?.['current_account-current_region'], {
+      bucketName: 'file-asset-bucket',
+      objectKey: 'file-asset-hash',
+      assumeRoleArn: 'file:role:arn',
+      assumeRoleExternalId: 'file-external-id',
+    });
+
+    test.deepEqual(manifest.dockerImages?.['docker-asset-hash']?.destinations?.['current_account-current_region'] , {
+      repositoryName: 'image-ecr-repository',
+      imageTag: 'docker-asset-hash',
+      assumeRoleArn: 'image:role:arn',
+      assumeRoleExternalId: 'image-external-id',
+    });
+
+    test.done();
+  },
 };
 
 /**
@@ -135,4 +188,11 @@ function evalCFN(value: any) {
 
 function isAssetManifest(x: cxapi.CloudArtifact): x is cxapi.AssetManifestArtifact {
   return x instanceof cxapi.AssetManifestArtifact;
+}
+
+function readAssetManifest(asm: cxapi.CloudAssembly): asset_schema.ManifestFile {
+  const manifestArtifact = asm.artifacts.filter(isAssetManifest)[0];
+  if (!manifestArtifact) { throw new Error('no asset manifest in assembly'); }
+
+  return JSON.parse(fs.readFileSync(manifestArtifact.file, { encoding: 'utf-8' }));
 }

packages/aws-cdk/.gitignore

@@ -32,5 +32,6 @@ cdk.context.json
 # as the subdirs contain .js files that should be committed)
 test/integ/cli/*.js
 test/integ/cli/*.d.ts
-!test/integ/cli/jest.setup.js
-!test/integ/cli/jest.config.js
+!test/integ/cli-regression-patches/**/*
+
+.DS_Store

packages/aws-cdk/.npmignore

@@ -25,3 +25,6 @@ tsconfig.json
 jest.config.js
 !lib/init-templates/**/jest.config.js
 !test/integ/cli/jest.config.js
+!test/integ/cli-regression-patches/**/*
+
+.DS_Store

packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml

@@ -106,7 +106,7 @@ Resources:
             Effect: Allow
             Principal:
               AWS:
-                Fn::Sub: "${PublishingRole.Arn}"
+                Fn::Sub: "${FilePublishingRole.Arn}"
             Resource: "*"
     Condition: CreateNewKey
   StagingBucket:
@@ -158,7 +158,7 @@ Resources:
           - HasCustomContainerAssetsRepositoryName
           - Fn::Sub: "${ContainerAssetsRepositoryName}"
           - Fn::Sub: cdk-${Qualifier}-container-assets-${AWS::AccountId}-${AWS::Region}
-  PublishingRole:
+  FilePublishingRole:
     Type: AWS::IAM::Role
     Properties:
       AssumeRolePolicyDocument:
@@ -177,8 +177,28 @@ Resources:
                     Ref: TrustedAccounts
               - Ref: AWS::NoValue
       RoleName:
-        Fn::Sub: cdk-${Qualifier}-publishing-role-${AWS::AccountId}-${AWS::Region}
-  PublishingRoleDefaultPolicy:
+        Fn::Sub: cdk-${Qualifier}-file-publishing-role-${AWS::AccountId}-${AWS::Region}
+  ImagePublishingRole:
+    Type: AWS::IAM::Role
+    Properties:
+      AssumeRolePolicyDocument:
+        Statement:
+          - Action: sts:AssumeRole
+            Effect: Allow
+            Principal:
+              AWS:
+                Ref: AWS::AccountId
+          - Fn::If:
+              - HasTrustedAccounts
+              - Action: sts:AssumeRole
+                Effect: Allow
+                Principal:
+                  AWS:
+                    Ref: TrustedAccounts
+              - Ref: AWS::NoValue
+      RoleName:
+        Fn::Sub: cdk-${Qualifier}-image-publishing-role-${AWS::AccountId}-${AWS::Region}
+  FilePublishingRoleDefaultPolicy:
     Type: AWS::IAM::Policy
     Properties:
       PolicyDocument:
@@ -206,6 +226,16 @@ Resources:
                 - CreateNewKey
                 - Fn::Sub: "${FileAssetsBucketEncryptionKey.Arn}"
                 - Fn::Sub: arn:${AWS::Partition}:kms:${AWS::Region}:${AWS::AccountId}:key/${FileAssetsBucketKmsKeyId}
+        Version: '2012-10-17'
+      Roles:
+        - Ref: FilePublishingRole
+      PolicyName:
+        Fn::Sub: cdk-${Qualifier}-file-publishing-role-default-policy-${AWS::AccountId}-${AWS::Region}
+  ImagePublishingRoleDefaultPolicy:
+    Type: AWS::IAM::Policy
+    Properties:
+      PolicyDocument:
+        Statement:
           - Action:
               - ecr:PutImage
               - ecr:InitiateLayerUpload
@@ -223,9 +253,9 @@ Resources:
             Effect: Allow
         Version: '2012-10-17'
       Roles:
-        - Ref: PublishingRole
+        - Ref: ImagePublishingRole
       PolicyName:
-        Fn::Sub: cdk-${Qualifier}-publishing-role-default-policy-${AWS::AccountId}-${AWS::Region}
+        Fn::Sub: cdk-${Qualifier}-image-publishing-role-default-policy-${AWS::AccountId}-${AWS::Region}
   DeploymentActionRole:
     Type: AWS::IAM::Role
     Properties:
@@ -317,10 +347,14 @@ Outputs:
     Description: The domain name of the S3 bucket owned by the CDK toolkit stack
     Value:
       Fn::Sub: "${StagingBucket.RegionalDomainName}"
+  ImageRepositoryName:
+    Description: The name of the ECR repository which hosts docker image assets
+    Value:
+      Fn::Sub: "${ContainerAssetsRepository}"
   BootstrapVersion:
     Description: The version of the bootstrap resources that are currently mastered
       in this stack
-    Value: '1'
+    Value: '2'
     Export:
       Name:
         Fn::Sub: CdkBootstrap-${Qualifier}-Version