From 16c193281d9a736f7fde41233e53b9b443c572a6 Mon Sep 17 00:00:00 2001
From: Maximilian Prusch <maximilian.prusch@outlook.com>
Date: Mon, 3 May 2021 23:26:21 +0200
Subject: [PATCH] fix(CodeBuild): add resource only once per secret (#14510)

When using the same secret as token with different json keys only add
the resource once to the policy


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
---
 .../@aws-cdk/aws-codebuild/lib/project.ts     | 10 ++--
 .../aws-codebuild/test/test.project.ts        | 51 +++++++++++++++++++
 2 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/packages/@aws-cdk/aws-codebuild/lib/project.ts b/packages/@aws-cdk/aws-codebuild/lib/project.ts
index 14d435e8e8cbc..307db08241ff5 100644
--- a/packages/@aws-cdk/aws-codebuild/lib/project.ts
+++ b/packages/@aws-cdk/aws-codebuild/lib/project.ts
@@ -720,7 +720,7 @@ export class Project extends ProjectBase {
 
     const ret = new Array<CfnProject.EnvironmentVariableProperty>();
     const ssmIamResources = new Array<string>();
-    const secretsManagerIamResources = new Array<string>();
+    const secretsManagerIamResources = new Set<string>();
     const kmsIamResources = new Set<string>();
 
     for (const [name, envVariable] of Object.entries(environmentVariables)) {
@@ -771,7 +771,7 @@ export class Project extends ProjectBase {
 
             // if we are passed a Token, we should assume it's the ARN of the Secret
             // (as the name would not work anyway, because it would be the full name, which CodeBuild does not support)
-            secretsManagerIamResources.push(secretArn);
+            secretsManagerIamResources.add(secretArn);
           } else {
             // check if the provided value is a full ARN of the Secret
             let parsedArn: ArnComponents | undefined;
@@ -791,7 +791,7 @@ export class Project extends ProjectBase {
               // If we were given just a name, it must be partial, as CodeBuild doesn't support providing full names.
               // In this case, we need to accommodate for the generated suffix in the IAM resource name
               : `${secretName}-??????`;
-            secretsManagerIamResources.push(stack.formatArn({
+            secretsManagerIamResources.add(stack.formatArn({
               service: 'secretsmanager',
               resource: 'secret',
               resourceName: secretIamResourceName,
@@ -828,10 +828,10 @@ export class Project extends ProjectBase {
         resources: ssmIamResources,
       }));
     }
-    if (secretsManagerIamResources.length !== 0) {
+    if (secretsManagerIamResources.size !== 0) {
       principal?.grantPrincipal.addToPrincipalPolicy(new iam.PolicyStatement({
         actions: ['secretsmanager:GetSecretValue'],
-        resources: secretsManagerIamResources,
+        resources: Array.from(secretsManagerIamResources),
       }));
     }
     if (kmsIamResources.size !== 0) {
diff --git a/packages/@aws-cdk/aws-codebuild/test/test.project.ts b/packages/@aws-cdk/aws-codebuild/test/test.project.ts
index e1ecb633aa3ff..7ed5fafccbbd3 100644
--- a/packages/@aws-cdk/aws-codebuild/test/test.project.ts
+++ b/packages/@aws-cdk/aws-codebuild/test/test.project.ts
@@ -1231,6 +1231,57 @@ export = {
         test.done();
       },
 
+      'when the same new secret is provided with different JSON keys, only adds the resource once'(test: Test) {
+        // GIVEN
+        const stack = new cdk.Stack();
+
+        // WHEN
+        const secret = new secretsmanager.Secret(stack, 'Secret');
+        new codebuild.PipelineProject(stack, 'Project', {
+          environmentVariables: {
+            'ENV_VAR1': {
+              type: codebuild.BuildEnvironmentVariableType.SECRETS_MANAGER,
+              value: `${secret.secretArn}:json-key1`,
+            },
+            'ENV_VAR2': {
+              type: codebuild.BuildEnvironmentVariableType.SECRETS_MANAGER,
+              value: `${secret.secretArn}:json-key2`,
+            },
+          },
+        });
+
+        // THEN
+        expect(stack).to(haveResourceLike('AWS::CodeBuild::Project', {
+          'Environment': {
+            'EnvironmentVariables': [
+              {
+                'Name': 'ENV_VAR1',
+                'Type': 'SECRETS_MANAGER',
+                'Value': { 'Fn::Join': ['', [{ 'Ref': 'SecretA720EF05' }, ':json-key1']] },
+              },
+              {
+                'Name': 'ENV_VAR2',
+                'Type': 'SECRETS_MANAGER',
+                'Value': { 'Fn::Join': ['', [{ 'Ref': 'SecretA720EF05' }, ':json-key2']] },
+              },
+            ],
+          },
+        }));
+
+        // THEN
+        expect(stack).to(haveResourceLike('AWS::IAM::Policy', {
+          'PolicyDocument': {
+            'Statement': arrayWith({
+              'Action': 'secretsmanager:GetSecretValue',
+              'Effect': 'Allow',
+              'Resource': { 'Ref': 'SecretA720EF05' },
+            }),
+          },
+        }));
+
+        test.done();
+      },
+
       'can be provided as the ARN attribute of a new Secret, followed by a JSON key'(test: Test) {
         // GIVEN
         const stack = new cdk.Stack();