diff --git a/packages/@aws-cdk/aws-codebuild/lib/project.ts b/packages/@aws-cdk/aws-codebuild/lib/project.ts index 307db08241ff5..85cf2cbe75789 100644 --- a/packages/@aws-cdk/aws-codebuild/lib/project.ts +++ b/packages/@aws-cdk/aws-codebuild/lib/project.ts @@ -7,7 +7,7 @@ import * as iam from '@aws-cdk/aws-iam'; import * as kms from '@aws-cdk/aws-kms'; import * as s3 from '@aws-cdk/aws-s3'; import * as secretsmanager from '@aws-cdk/aws-secretsmanager'; -import { ArnComponents, Aws, Duration, IResource, Lazy, Names, PhysicalName, Resource, SecretValue, Stack, Token, TokenComparison, Tokenization } from '@aws-cdk/core'; +import { Aws, Duration, IResource, Lazy, Names, PhysicalName, Reference, Resource, SecretValue, Stack, Token, TokenComparison, Tokenization } from '@aws-cdk/core'; import { Construct } from 'constructs'; import { IArtifacts } from './artifacts'; import { BuildSpec } from './build-spec'; @@ -764,46 +764,34 @@ export class Project extends ProjectBase { // save SecretsManager env variables if (envVariable.type === BuildEnvironmentVariableType.SECRETS_MANAGER) { - if (Token.isUnresolved(envVariableValue)) { - // the value of the property can be a complex string, separated by ':'; - // see https://docs.aws.amazon.com/codebuild/latest/userguide/build-spec-ref.html#build-spec.env.secrets-manager - const secretArn = envVariableValue.split(':')[0]; - - // 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.add(secretArn); - } else { - // check if the provided value is a full ARN of the Secret - let parsedArn: ArnComponents | undefined; - try { - parsedArn = stack.parseArn(envVariableValue, ':'); - } catch (e) {} - const secretSpecifier: string = parsedArn ? parsedArn.resourceName : envVariableValue; + // We have 3 basic cases here of what envVariableValue can be: + // 1. A string that starts with 'arn:' (and might contain Token fragments). + // 2. A Token. + // 3. A simple value, like 'secret-id'. + if (envVariableValue.startsWith('arn:')) { + const parsedArn = stack.parseArn(envVariableValue, ':'); + if (!parsedArn.resourceName) { + throw new Error('SecretManager ARN is missing the name of the secret: ' + envVariableValue); + } // the value of the property can be a complex string, separated by ':'; // see https://docs.aws.amazon.com/codebuild/latest/userguide/build-spec-ref.html#build-spec.env.secrets-manager - const secretName = secretSpecifier.split(':')[0]; - const secretIamResourceName = parsedArn - // If we were given an ARN, we don't' know whether the name is full, or partial, - // as CodeBuild supports both ARN forms. - // Because of that, follow the name with a '*', which works for both - ? `${secretName}*` - // 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}-??????`; + const secretName = parsedArn.resourceName.split(':')[0]; secretsManagerIamResources.add(stack.formatArn({ service: 'secretsmanager', resource: 'secret', - resourceName: secretIamResourceName, + // since we don't know whether the ARN was full, or partial + // (CodeBuild supports both), + // stick a "*" at the end, which makes it work for both + resourceName: `${secretName}*`, sep: ':', - // if we were given an ARN, we need to use the provided partition/account/region - partition: parsedArn?.partition, - account: parsedArn?.account, - region: parsedArn?.region, + partition: parsedArn.partition, + account: parsedArn.account, + region: parsedArn.region, })); // if secret comes from another account, SecretsManager will need to access // KMS on the other account as well to be able to get the secret - if (parsedArn && parsedArn.account && Token.compareStrings(parsedArn.account, stack.account) === TokenComparison.DIFFERENT) { + if (parsedArn.account && Token.compareStrings(parsedArn.account, stack.account) === TokenComparison.DIFFERENT) { kmsIamResources.add(stack.formatArn({ service: 'kms', resource: 'key', @@ -811,12 +799,61 @@ export class Project extends ProjectBase { // the key policies have to allow this access, so a wildcard is safe here resourceName: '*', sep: '/', - // if we were given an ARN, we need to use the provided partition/account/region partition: parsedArn.partition, account: parsedArn.account, region: parsedArn.region, })); } + } else if (Token.isUnresolved(envVariableValue)) { + // the value of the property can be a complex string, separated by ':'; + // see https://docs.aws.amazon.com/codebuild/latest/userguide/build-spec-ref.html#build-spec.env.secrets-manager + let secretArn = envVariableValue.split(':')[0]; + + // parse the Token, and see if it represents a single resource + // (we will assume it's a Secret from SecretsManager) + const fragments = Tokenization.reverseString(envVariableValue); + if (fragments.tokens.length === 1) { + const resolvable = fragments.tokens[0]; + if (Reference.isReference(resolvable)) { + // check the Stack the resource owning the reference belongs to + const resourceStack = Stack.of(resolvable.target); + if (Token.compareStrings(stack.account, resourceStack.account) === TokenComparison.DIFFERENT) { + // since this is a cross-account access, + // add the appropriate KMS permissions + kmsIamResources.add(stack.formatArn({ + service: 'kms', + resource: 'key', + // We do not know the ID of the key, but since this is a cross-account access, + // the key policies have to allow this access, so a wildcard is safe here + resourceName: '*', + sep: '/', + partition: resourceStack.partition, + account: resourceStack.account, + region: resourceStack.region, + })); + + // Work around a bug in SecretsManager - + // when the access is cross-environment, + // Secret.secretArn returns a partial ARN! + // So add a "*" at the end, so that the permissions work + secretArn = `${secretArn}-??????`; + } + } + } + + // 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.add(secretArn); + } else { + // the value of the property can be a complex string, separated by ':'; + // see https://docs.aws.amazon.com/codebuild/latest/userguide/build-spec-ref.html#build-spec.env.secrets-manager + const secretName = envVariableValue.split(':')[0]; + secretsManagerIamResources.add(stack.formatArn({ + service: 'secretsmanager', + resource: 'secret', + resourceName: `${secretName}-??????`, + sep: ':', + })); } } } diff --git a/packages/@aws-cdk/aws-codebuild/test/test.project.ts b/packages/@aws-cdk/aws-codebuild/test/test.project.ts index 7ed5fafccbbd3..2dc652e64f39d 100644 --- a/packages/@aws-cdk/aws-codebuild/test/test.project.ts +++ b/packages/@aws-cdk/aws-codebuild/test/test.project.ts @@ -1466,6 +1466,242 @@ export = { test.done(); }, + + 'can be provided as a SecretArn of a new Secret, with its physical name set, created in a different account'(test: Test) { + // GIVEN + const app = new cdk.App(); + const secretStack = new cdk.Stack(app, 'SecretStack', { + env: { account: '012345678912' }, + }); + const stack = new cdk.Stack(app, 'ProjectStack', { + env: { account: '123456789012' }, + }); + + // WHEN + const secret = new secretsmanager.Secret(secretStack, 'Secret', { secretName: 'secret-name' }); + new codebuild.PipelineProject(stack, 'Project', { + environmentVariables: { + 'ENV_VAR1': { + type: codebuild.BuildEnvironmentVariableType.SECRETS_MANAGER, + value: secret.secretArn, + }, + }, + }); + + // THEN + expect(stack).to(haveResourceLike('AWS::CodeBuild::Project', { + 'Environment': { + 'EnvironmentVariables': [ + { + 'Name': 'ENV_VAR1', + 'Type': 'SECRETS_MANAGER', + 'Value': { + 'Fn::Join': ['', [ + 'arn:', + { 'Ref': 'AWS::Partition' }, + ':secretsmanager:', + { 'Ref': 'AWS::Region' }, + ':012345678912:secret:secret-name', + ]], + }, + }, + ], + }, + })); + + expect(stack).to(haveResourceLike('AWS::IAM::Policy', { + 'PolicyDocument': { + 'Statement': arrayWith({ + 'Action': 'secretsmanager:GetSecretValue', + 'Effect': 'Allow', + 'Resource': { + 'Fn::Join': ['', [ + 'arn:', + { 'Ref': 'AWS::Partition' }, + ':secretsmanager:', + { 'Ref': 'AWS::Region' }, + ':012345678912:secret:secret-name-??????', + ]], + }, + }), + }, + })); + + expect(stack).to(haveResourceLike('AWS::IAM::Policy', { + 'PolicyDocument': { + 'Statement': arrayWith({ + 'Action': 'kms:Decrypt', + 'Effect': 'Allow', + 'Resource': { + 'Fn::Join': ['', [ + 'arn:', + { 'Ref': 'AWS::Partition' }, + ':kms:', + { 'Ref': 'AWS::Region' }, + ':012345678912:key/*', + ]], + }, + }), + }, + })); + + test.done(); + }, + + 'can be provided as a SecretArn of a Secret imported by name in a different account'(test: Test) { + // GIVEN + const app = new cdk.App(); + const secretStack = new cdk.Stack(app, 'SecretStack', { + env: { account: '012345678912' }, + }); + const stack = new cdk.Stack(app, 'ProjectStack', { + env: { account: '123456789012' }, + }); + + // WHEN + const secret = secretsmanager.Secret.fromSecretNameV2(secretStack, 'Secret', 'secret-name'); + new codebuild.PipelineProject(stack, 'Project', { + environmentVariables: { + 'ENV_VAR1': { + type: codebuild.BuildEnvironmentVariableType.SECRETS_MANAGER, + value: `${secret.secretArn}:json-key`, + }, + }, + }); + + // THEN + expect(stack).to(haveResourceLike('AWS::CodeBuild::Project', { + 'Environment': { + 'EnvironmentVariables': [ + { + 'Name': 'ENV_VAR1', + 'Type': 'SECRETS_MANAGER', + 'Value': { + 'Fn::Join': ['', [ + 'arn:', + { 'Ref': 'AWS::Partition' }, + ':secretsmanager:', + { 'Ref': 'AWS::Region' }, + ':012345678912:secret:secret-name:json-key', + ]], + }, + }, + ], + }, + })); + + expect(stack).to(haveResourceLike('AWS::IAM::Policy', { + 'PolicyDocument': { + 'Statement': arrayWith({ + 'Action': 'secretsmanager:GetSecretValue', + 'Effect': 'Allow', + 'Resource': { + 'Fn::Join': ['', [ + 'arn:', + { 'Ref': 'AWS::Partition' }, + ':secretsmanager:', + { 'Ref': 'AWS::Region' }, + ':012345678912:secret:secret-name*', + ]], + }, + }), + }, + })); + + expect(stack).to(haveResourceLike('AWS::IAM::Policy', { + 'PolicyDocument': { + 'Statement': arrayWith({ + 'Action': 'kms:Decrypt', + 'Effect': 'Allow', + 'Resource': { + 'Fn::Join': ['', [ + 'arn:', + { 'Ref': 'AWS::Partition' }, + ':kms:', + { 'Ref': 'AWS::Region' }, + ':012345678912:key/*', + ]], + }, + }), + }, + })); + + test.done(); + }, + + 'can be provided as a SecretArn of a Secret imported by complete ARN from a different account'(test: Test) { + // GIVEN + const app = new cdk.App(); + const stack = new cdk.Stack(app, 'ProjectStack', { + env: { account: '123456789012' }, + }); + const secretArn = 'arn:aws:secretsmanager:us-west-2:901234567890:secret:mysecret-123456'; + + // WHEN + const secret = secretsmanager.Secret.fromSecretCompleteArn(stack, 'Secret', secretArn); + new codebuild.PipelineProject(stack, 'Project', { + environmentVariables: { + 'ENV_VAR1': { + type: codebuild.BuildEnvironmentVariableType.SECRETS_MANAGER, + value: secret.secretArn, + }, + }, + }); + + // THEN + expect(stack).to(haveResourceLike('AWS::CodeBuild::Project', { + 'Environment': { + 'EnvironmentVariables': [ + { + 'Name': 'ENV_VAR1', + 'Type': 'SECRETS_MANAGER', + 'Value': secretArn, + }, + ], + }, + })); + + expect(stack).to(haveResourceLike('AWS::IAM::Policy', { + 'PolicyDocument': { + 'Statement': arrayWith({ + 'Action': 'secretsmanager:GetSecretValue', + 'Effect': 'Allow', + 'Resource': `${secretArn}*`, + }), + }, + })); + + expect(stack).to(haveResourceLike('AWS::IAM::Policy', { + 'PolicyDocument': { + 'Statement': arrayWith({ + 'Action': 'kms:Decrypt', + 'Effect': 'Allow', + 'Resource': 'arn:aws:kms:us-west-2:901234567890:key/*', + }), + }, + })); + + test.done(); + }, + + 'should fail when the parsed Arn does not contain a secret name'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + + // WHEN + test.throws(() => { + new codebuild.PipelineProject(stack, 'Project', { + environmentVariables: { + 'ENV_VAR1': { + type: codebuild.BuildEnvironmentVariableType.SECRETS_MANAGER, + value: 'arn:aws:secretsmanager:us-west-2:123456789012:secret', + }, + }, + }); + }, /SecretManager ARN is missing the name of the secret:/); + + test.done(); + }, }, 'should fail creating when using a secret value in a plaintext variable'(test: Test) { diff --git a/packages/@aws-cdk/aws-secretsmanager/lib/secret.ts b/packages/@aws-cdk/aws-secretsmanager/lib/secret.ts index f438c5fd21e14..01dbba9a64775 100644 --- a/packages/@aws-cdk/aws-secretsmanager/lib/secret.ts +++ b/packages/@aws-cdk/aws-secretsmanager/lib/secret.ts @@ -425,7 +425,7 @@ export class Secret extends SecretBase { public readonly secretName = parseSecretName(scope, secretArn); protected readonly autoCreatePolicy = false; public get secretFullArn() { return secretArnIsPartial ? undefined : secretArn; } - }(scope, id); + }(scope, id, { environmentFromArn: secretArn }); } public readonly encryptionKey?: kms.IKey; diff --git a/packages/@aws-cdk/aws-secretsmanager/test/secret.test.ts b/packages/@aws-cdk/aws-secretsmanager/test/secret.test.ts index df07bcd7f2534..eb93f827b27c2 100644 --- a/packages/@aws-cdk/aws-secretsmanager/test/secret.test.ts +++ b/packages/@aws-cdk/aws-secretsmanager/test/secret.test.ts @@ -807,6 +807,19 @@ describe('fromSecretAttributes', () => { secretCompleteArn: 'arn:aws:secretsmanager:eu-west-1:111111111111:secret:MySecret', })).toThrow(/does not appear to be complete/); }); + + test('parses environment from secretArn', () => { + // GIVEN + const secretAccount = '222222222222'; + + // WHEN + const secret = secretsmanager.Secret.fromSecretAttributes(stack, 'Secret', { + secretCompleteArn: `arn:aws:secretsmanager:eu-west-1:${secretAccount}:secret:MySecret-f3gDy9`, + }); + + // THEN + expect(secret.env.account).toBe(secretAccount); + }); }); test('import by secret name', () => { diff --git a/packages/@aws-cdk/core/lib/secret-value.ts b/packages/@aws-cdk/core/lib/secret-value.ts index 33622a7cdc0ea..2cd1b772f3207 100644 --- a/packages/@aws-cdk/core/lib/secret-value.ts +++ b/packages/@aws-cdk/core/lib/secret-value.ts @@ -34,7 +34,7 @@ export class SecretValue extends Intrinsic { * @param secretId The ID or ARN of the secret * @param options Options */ - public static secretsManager(secretId: string, options: SecretsManagerSecretOptions = { }): SecretValue { + public static secretsManager(secretId: string, options: SecretsManagerSecretOptions = {}): SecretValue { if (!secretId) { throw new Error('secretId cannot be empty'); } @@ -43,6 +43,10 @@ export class SecretValue extends Intrinsic { throw new Error(`secret id "${secretId}" is not an ARN but contains ":"`); } + if (options.versionStage && options.versionId) { + throw new Error(`verionStage: '${options.versionStage}' and versionId: '${options.versionId}' were both provided but only one is allowed`); + } + const parts = [ secretId, 'SecretString', @@ -103,7 +107,7 @@ export class SecretValue extends Intrinsic { */ export interface SecretsManagerSecretOptions { /** - * Specified the secret version that you want to retrieve by the staging label attached to the version. + * Specifies the secret version that you want to retrieve by the staging label attached to the version. * * Can specify at most one of `versionId` and `versionStage`. * diff --git a/packages/@aws-cdk/core/test/secret-value.test.ts b/packages/@aws-cdk/core/test/secret-value.test.ts index 155a0d371aab0..7a119d3534a9b 100644 --- a/packages/@aws-cdk/core/test/secret-value.test.ts +++ b/packages/@aws-cdk/core/test/secret-value.test.ts @@ -21,12 +21,11 @@ nodeunitShim({ // WHEN const v = SecretValue.secretsManager('secret-id', { jsonField: 'json-key', - versionId: 'version-id', versionStage: 'version-stage', }); // THEN - test.deepEqual(stack.resolve(v), '{{resolve:secretsmanager:secret-id:SecretString:json-key:version-stage:version-id}}'); + test.deepEqual(stack.resolve(v), '{{resolve:secretsmanager:secret-id:SecretString:json-key:version-stage:}}'); test.done(); }, @@ -47,6 +46,18 @@ nodeunitShim({ test.done(); }, + 'secretsManager with versionStage and versionId'(test: Test) { + test.throws(() => { + SecretValue.secretsManager('secret-id', + { + versionStage: 'version-stage', + versionId: 'version-id', + }); + }, /were both provided but only one is allowed/); + + test.done(); + }, + 'secretsManager with a non-ARN ID that has colon'(test: Test) { test.throws(() => SecretValue.secretsManager('not:an:arn'), /is not an ARN but contains ":"/); test.done(); @@ -98,5 +109,4 @@ nodeunitShim({ test.throws(() => SecretValue.cfnParameter(p), /CloudFormation parameter must be configured with "NoEcho"/); test.done(); }, - });