refactor with tests

aws · Jul 2, 2024 · 6e3b430 · 6e3b430
1 parent 7f5ce4b
commit 6e3b430
Show file tree

Hide file tree

Showing 2 changed files with 160 additions and 12 deletions.
diff --git a/packages/aws-cdk-lib/aws-appconfig/lib/configuration.ts b/packages/aws-cdk-lib/aws-appconfig/lib/configuration.ts
@@ -512,7 +512,7 @@ export interface SourcedConfigurationProps extends ConfigurationProps {
   /**
    * The IAM role to retrieve the configuration.
    *
-   * @default - A role is generated.
+   * @default - Auto generated if location type is not ConfigurationSourceType.CODE_PIPELINE otherwise no role specified.
    */
   readonly retrievalRole?: iam.IRole;
 }
@@ -564,16 +564,7 @@ export class SourcedConfiguration extends ConfigurationBase {
     this.locationUri = this.location.locationUri;
     this.versionNumber = props.versionNumber;
     this.sourceKey = this.location.key;
-    this.retrievalRole = props.retrievalRole || this.location.type != ConfigurationSourceType.CODE_PIPELINE
-      ? new iam.Role(this, 'Role', {
-        roleName: PhysicalName.GENERATE_IF_NEEDED,
-        assumedBy: new iam.ServicePrincipal('appconfig.amazonaws.com'),
-        inlinePolicies: {
-          ['AllowAppConfigReadFromSourcePolicy']: this.getPolicyForRole(),
-        },
-      })
-      : undefined;
-
+    this.retrievalRole = props.retrievalRole ?? this.getRetrievalRole();
     this._cfnConfigurationProfile = new CfnConfigurationProfile(this, 'Resource', {
       applicationId: this.applicationId,
       locationUri: this.locationUri,
@@ -596,6 +587,22 @@ export class SourcedConfiguration extends ConfigurationBase {
     this.deployConfigToEnvironments();
   }
 
+  private getRetrievalRole(): iam.Role | undefined {
+    // Check if the configuration source is not from CodePipeline
+    if (this.location.type != ConfigurationSourceType.CODE_PIPELINE) {
+      return new iam.Role(this, 'Role', {
+        roleName: PhysicalName.GENERATE_IF_NEEDED,
+        assumedBy: new iam.ServicePrincipal('appconfig.amazonaws.com'),
+        inlinePolicies: {
+          ['AllowAppConfigReadFromSourcePolicy']: this.getPolicyForRole(),
+        },
+      });
+    } else {
+      // No role is needed if the configuration source is from CodePipeline
+      return undefined;
+    }
+  }
+
   private getPolicyForRole(): iam.PolicyDocument {
     const policy = new iam.PolicyStatement({
       effect: iam.Effect.ALLOW,

diff --git a/packages/aws-cdk-lib/aws-appconfig/test/configuration.test.ts b/packages/aws-cdk-lib/aws-appconfig/test/configuration.test.ts
@@ -1,4 +1,4 @@
-import { Template } from '../../assertions';
+import { Template, Match } from '../../assertions';
 import { Artifact, Pipeline } from '../../aws-codepipeline';
 import { S3DeployAction, S3SourceAction } from '../../aws-codepipeline-actions';
 import * as iam from '../../aws-iam';
@@ -283,6 +283,147 @@ describe('configuration', () => {
     Template.fromStack(stack).resourceCountIs('AWS::AppConfig::Deployment', 2);
   });
 
+  test('configuration with retrievalRole undefined from bucket source should create a new role', () => {
+    // GIVEN
+    const stack = new cdk.Stack();
+    const app = new Application(stack, 'MyAppConfig', {
+      applicationName: 'MyApplication',
+    });
+    const bucket = new Bucket(stack, 'MyBucket');
+
+    // WHEN
+    new SourcedConfiguration(stack, 'MySourcedConfig', {
+      versionNumber: '1',
+      location: ConfigurationSource.fromBucket(bucket, 'path/to/object'),
+      application: app,
+    });
+
+    // THEN
+    // should have a new role provisioned with AllowAppConfigReadFromSourcePolicy
+    Template.fromStack(stack).hasResourceProperties('AWS::IAM::Role', {
+      AssumeRolePolicyDocument: {
+        Statement: [
+          {
+            Action: 'sts:AssumeRole',
+            Effect: 'Allow',
+            Principal: {
+              Service: 'appconfig.amazonaws.com',
+            },
+          },
+        ],
+        Version: '2012-10-17',
+      },
+      Policies: [
+        {
+          PolicyName: 'AllowAppConfigReadFromSourcePolicy',
+        },
+      ],
+    });
+  });
+
+  test('configuration with retrievalRole defined should NOT create a new role', () => {
+    // GIVEN
+    const stack = new cdk.Stack();
+    const app = new Application(stack, 'MyAppConfig', {
+      applicationName: 'MyApplication',
+    });
+    const bucket = new Bucket(stack, 'MyBucket');
+
+    // WHEN
+    new SourcedConfiguration(stack, 'MySourcedConfig', {
+      versionNumber: '1',
+      location: ConfigurationSource.fromBucket(bucket, 'path/to/object'),
+      application: app,
+      retrievalRole: new iam.Role(stack, 'MyRole', {
+        assumedBy: new iam.ServicePrincipal('appconfig.amazonaws.com'),
+      }),
+    });
+
+    // THEN
+    // should have a new role provisioned with AllowAppConfigReadFromSourcePolicy
+    Template.fromStack(stack).hasResourceProperties('AWS::IAM::Role', Match.not({
+      AssumeRolePolicyDocument: {
+        Statement: [
+          {
+            Action: 'sts:AssumeRole',
+            Effect: 'Allow',
+            Principal: {
+              Service: 'appconfig.amazonaws.com',
+            },
+          },
+        ],
+        Version: '2012-10-17',
+      },
+      Policies: [
+        {
+          PolicyName: 'AllowAppConfigReadFromSourcePolicy',
+        },
+      ],
+    }));
+  });
+
+  test('configuration with retrievalRole undefined from CodePipeline source should NOT create a new role', () => {
+    // GIVEN
+    const stack = new cdk.Stack();
+    const app = new Application(stack, 'MyAppConfig', {
+      applicationName: 'MyApplication',
+    });
+    const bucket = new Bucket(stack, 'MyBucket');
+    const sourceAction = new S3SourceAction({
+      actionName: 'Source',
+      bucket: bucket,
+      bucketKey: 'hello/world/codepipeline.txt',
+      output: new Artifact('SourceOutput'),
+    });
+    const deployAction = new S3DeployAction({
+      actionName: 'Deploy',
+      input: Artifact.artifact('SourceOutput'),
+      bucket: bucket,
+      extract: true,
+    });
+    const pipeline = new Pipeline(stack, 'MyPipeline', {
+      stages: [
+        {
+          stageName: 'beta',
+          actions: [sourceAction],
+        },
+        {
+          stageName: 'prod',
+          actions: [deployAction],
+        },
+      ],
+    });
+
+    // WHEN
+    new SourcedConfiguration(stack, 'MySourcedConfig', {
+      versionNumber: '1',
+      location: ConfigurationSource.fromPipeline(pipeline),
+      application: app,
+    });
+
+    // THEN
+    // should NOT have a new role provisioned with AllowAppConfigReadFromSourcePolicy
+    Template.fromStack(stack).hasResourceProperties('AWS::IAM::Role', Match.not({
+      AssumeRolePolicyDocument: {
+        Statement: [
+          {
+            Action: 'sts:AssumeRole',
+            Effect: 'Allow',
+            Principal: {
+              Service: 'appconfig.amazonaws.com',
+            },
+          },
+        ],
+        Version: '2012-10-17',
+      },
+      Policies: [
+        {
+          PolicyName: 'AllowAppConfigReadFromSourcePolicy',
+        },
+      ],
+    }));
+  });
+
   test('configuration with two configurations and no deployment strategy specified', () => {
     const stack = new cdk.Stack();
     const app = new Application(stack, 'MyAppConfig', {