Skip to content

Commit

Permalink
fix(CodeBuild): add resource only once per secret (aws#14510)
Browse files Browse the repository at this point in the history
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*
  • Loading branch information
Kruspe authored and hollanddd committed Aug 26, 2021
1 parent d4fd1d1 commit 16c1932
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 5 deletions.
10 changes: 5 additions & 5 deletions packages/@aws-cdk/aws-codebuild/lib/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down
51 changes: 51 additions & 0 deletions packages/@aws-cdk/aws-codebuild/test/test.project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 16c1932

Please sign in to comment.