fix(secretsmanager): Secret.fromSecretName doesn't work with ECS

The ability to import and reference a Secret purely by the secret name was
introduced in #10309. One of the original requests was modelled after the
integration with CodeBuild, where either the secret name or the full ARN
-- including the SecretsManager-provided suffix -- were accepted, but not a
"partial" ARN without the suffix. To ease integrations with other services
in this case, the `secretArn` was defined as returning the `secretName` for
these secrets imported by name.

However, other services -- like ECS -- require that an ARN format is provided,
even as a partial ARN. This introduces a dual behavior where sometimes the
secretName works and partial ARN fails, and other times the partial ARN works
and the secretName fails.

This change deprecates `fromSecretName` and introduces a new, better-behaved
`fromSecretNameV2` that sets the ARN to a "partial" ARN without the Secrets
Manager suffix. It also introduces a `secretFullArn` that is an optional version
of `secretArn` that will be undefined for secrets imported by name.

Related changes
* I improved the suffix-strippiung logic of `parseSecretName` to only strip a
  suffix if it's exactly 6 characters long, as all SecretsManager
  suffixes are 6 characters. This prevents accidentally stripping the last word
  off of a hyphenated secret name like 'github-token'.
* Updated the CodeBuild integration and added CodeBuild tests.

fixes #10519

packages/@aws-cdk/aws-codebuild/lib/project.ts

@@ -956,14 +956,15 @@ export class Project extends ProjectBase {
       this.buildImage.secretsManagerCredentials?.grantRead(this);
     }
 
+    const secret = this.buildImage.secretsManagerCredentials;
     return {
       type: this.buildImage.type,
       image: this.buildImage.imageId,
       imagePullCredentialsType: imagePullPrincipalType,
-      registryCredential: this.buildImage.secretsManagerCredentials
+      registryCredential: secret
         ? {
           credentialProvider: 'SECRETS_MANAGER',
-          credential: this.buildImage.secretsManagerCredentials.secretArn,
+          credential: secret.secretFullArn ?? secret.secretName,
         }
         : undefined,
       privilegedMode: env.privileged || false,

packages/@aws-cdk/aws-codebuild/test/test.project.ts

@@ -1,7 +1,8 @@
-import { countResources, expect, haveResource, haveResourceLike, not, ResourcePart } from '@aws-cdk/assert';
+import { countResources, expect, haveResource, haveResourceLike, objectLike, not, ResourcePart } from '@aws-cdk/assert';
 import * as ec2 from '@aws-cdk/aws-ec2';
 import * as iam from '@aws-cdk/aws-iam';
 import * as s3 from '@aws-cdk/aws-s3';
+import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
 import * as cdk from '@aws-cdk/core';
 import { Test } from 'nodeunit';
 import * as codebuild from '../lib';
@@ -540,4 +541,64 @@ export = {
       test.done();
     },
   },
+
+  'Environment': {
+    'build image - can use secret to access build image'(test: Test) {
+      // GIVEN
+      const stack = new cdk.Stack();
+      const secret = new secretsmanager.Secret(stack, 'Secret');
+
+      // WHEN
+      new codebuild.Project(stack, 'Project', {
+        source: codebuild.Source.s3({
+          bucket: new s3.Bucket(stack, 'Bucket'),
+          path: 'path',
+        }),
+        environment: {
+          buildImage: codebuild.LinuxBuildImage.fromDockerRegistry('myimage', { secretsManagerCredentials: secret }),
+        },
+      });
+
+      // THEN
+      expect(stack).to(haveResourceLike('AWS::CodeBuild::Project', {
+        Environment: objectLike({
+          RegistryCredential: {
+            CredentialProvider: 'SECRETS_MANAGER',
+            Credential: { 'Ref': 'SecretA720EF05' },
+          },
+        }),
+      }));
+
+      test.done();
+    },
+
+    'build image - can use imported secret by name'(test: Test) {
+      // GIVEN
+      const stack = new cdk.Stack();
+      const secret = secretsmanager.Secret.fromSecretNameV2(stack, 'Secret', 'MySecretName');
+
+      // WHEN
+      new codebuild.Project(stack, 'Project', {
+        source: codebuild.Source.s3({
+          bucket: new s3.Bucket(stack, 'Bucket'),
+          path: 'path',
+        }),
+        environment: {
+          buildImage: codebuild.LinuxBuildImage.fromDockerRegistry('myimage', { secretsManagerCredentials: secret }),
+        },
+      });
+
+      // THEN
+      expect(stack).to(haveResourceLike('AWS::CodeBuild::Project', {
+        Environment: objectLike({
+          RegistryCredential: {
+            CredentialProvider: 'SECRETS_MANAGER',
+            Credential: 'MySecretName',
+          },
+        }),
+      }));
+
+      test.done();
+    },
+  },
 };

packages/@aws-cdk/aws-secretsmanager/README.md

@@ -160,7 +160,7 @@ credentials generation and rotation is integrated.
 ### Importing Secrets
 
 Existing secrets can be imported by ARN, name, and other attributes (including the KMS key used to encrypt the secret).
-Secrets imported by name can used the short-form of the name (without the SecretsManager-provided suffx);
+Secrets imported by name can use the short-form of the name (without the SecretsManager-provided suffx);
 the secret name must exist in the same account and region as the stack.
 Importing by name makes it easier to reference secrets created in different regions, each with their own suffix and ARN.
 
@@ -170,7 +170,7 @@ import * as kms from '@aws-cdk/aws-kms';
 const secretArn = 'arn:aws:secretsmanager:eu-west-1:111111111111:secret:MySecret-f3gDy9';
 const encryptionKey = kms.Key.fromKeyArn(stack, 'MyEncKey', 'arn:aws:kms:eu-west-1:111111111111:key/21c4b39b-fde2-4273-9ac0-d9bb5c0d0030');
 const mySecretFromArn = secretsmanager.Secret.fromSecretArn(stack, 'SecretFromArn', secretArn);
-const mySecretFromName = secretsmanager.Secret.fromSecretName(stack, 'SecretFromName', 'MySecret') // Note: the -f3gDy9 suffix is optional
+const mySecretFromName = secretsmanager.Secret.fromSecretNameV2(stack, 'SecretFromName', 'MySecret')
 const mySecretFromAttrs = secretsmanager.Secret.fromSecretAttributes(stack, 'SecretFromAttributes', {
   secretArn,
   encryptionKey,

packages/@aws-cdk/aws-secretsmanager/lib/secret.ts

@@ -17,11 +17,18 @@ export interface ISecret extends IResource {
   readonly encryptionKey?: kms.IKey;
 
   /**
-   * The ARN of the secret in AWS Secrets Manager.
+   * The ARN of the secret in AWS Secrets Manager. Will return the full ARN if available, otherwise a partial arn.
+   * For secrets imported by the deprecated `fromSecretName`, it will return the `secretName`.
    * @attribute
    */
   readonly secretArn: string;
 
+  /**
+   * The full ARN of the secret in AWS Secrets Manager, which is the ARN including the Secrets Manager-supplied 6-character suffix.
+   * This is equal to `secretArn` in most cases, but is undefined when a full ARN is not available (e.g., secrets imported by name).
+   */
+  readonly secretFullArn?: string;
+
   /**
    * The name of the secret
    */
@@ -152,6 +159,8 @@ abstract class SecretBase extends Resource implements ISecret {
 
   private policy?: ResourcePolicy;
 
+  public get secretFullArn(): string | undefined { return this.secretArn; }
+
   public grantRead(grantee: iam.IGrantable, versionStages?: string[]): iam.Grant {
     // @see https://docs.aws.amazon.com/secretsmanager/latest/userguide/auth-and-access_identity-based-policies.html
 
@@ -277,13 +286,15 @@ export class Secret extends SecretBase {
   /**
    * Imports a secret by secret name; the ARN of the Secret will be set to the secret name.
    * A secret with this name must exist in the same account & region.
+   * @deprecated use `fromSecretNameV2`
    */
   public static fromSecretName(scope: Construct, id: string, secretName: string): ISecret {
     return new class extends SecretBase {
       public readonly encryptionKey = undefined;
       public readonly secretArn = secretName;
       public readonly secretName = secretName;
       protected readonly autoCreatePolicy = false;
+      public get secretFullArn() { return undefined; }
       // Overrides the secretArn for grant* methods, where the secretArn must be in ARN format.
       // Also adds a wildcard to the resource name to support the SecretsManager-provided suffix.
       protected get arnForPolicies() {
@@ -297,6 +308,35 @@ export class Secret extends SecretBase {
     }(scope, id);
   }
 
+  /**
+   * Imports a secret by secret name.
+   * A secret with this name must exist in the same account & region.
+   * Replaces the deprecated `fromSecretName`.
+   */
+  public static fromSecretNameV2(scope: Construct, id: string, secretName: string): ISecret {
+    return new class extends SecretBase {
+      public readonly encryptionKey = undefined;
+      public readonly secretName = secretName;
+      public readonly secretArn = this.partialArn;
+      protected readonly autoCreatePolicy = false;
+      public get secretFullArn() { return undefined; }
+      // Overrides the secretArn for grant* methods, where the secretArn must be in ARN format.
+      // Also adds a wildcard to the resource name to support the SecretsManager-provided suffix.
+      protected get arnForPolicies(): string {
+        return this.partialArn + '-??????';
+      }
+      // Creates a "partial" ARN from the secret name. The "full" ARN would include the SecretsManager-provided suffix.
+      private get partialArn(): string {
+        return Stack.of(this).formatArn({
+          service: 'secretsmanager',
+          resource: 'secret',
+          resourceName: secretName,
+          sep: ':',
+        });
+      }
+    }(scope, id);
+  }
+
   /**
    * Import an existing secret into the Stack.
    *
@@ -612,9 +652,11 @@ function parseSecretName(construct: IConstruct, secretArn: string) {
       return resourceName;
     }
 
-    // Secret resource names are in the format `${secretName}-${SecretsManager suffix}`
-    // If there is no hyphen, assume no suffix was provided, and return the whole name.
-    return resourceName.substr(0, resourceName.lastIndexOf('-')) || resourceName;
+    // Secret resource names are in the format `${secretName}-${6-character SecretsManager suffix}`
+    // If there is no hyphen (or 6-character suffix) assume no suffix was provided, and return the whole name.
+    const lastHyphenIndex = resourceName.lastIndexOf('-');
+    const hasSecretsSuffix = lastHyphenIndex !== -1 && resourceName.substr(lastHyphenIndex + 1).length === 6;
+    return hasSecretsSuffix ? resourceName.substr(0, lastHyphenIndex) : resourceName;
   }
   throw new Error('invalid ARN format; no secret name provided');
 }

packages/@aws-cdk/aws-secretsmanager/package.json

@@ -103,6 +103,9 @@
   "awslint": {
     "exclude": [
       "attribute-tag:@aws-cdk/aws-secretsmanager.Secret.secretName",
+      "attribute-tag:@aws-cdk/aws-secretsmanager.Secret.secretFullArn",
+      "from-signature:@aws-cdk/aws-secretsmanager.Secret.fromSecretNameV2",
+      "from-signature:@aws-cdk/aws-secretsmanager.Secret.fromSecretNameV2.params[2]",
       "from-signature:@aws-cdk/aws-secretsmanager.SecretTargetAttachment.fromSecretTargetAttachmentSecretArn",
       "from-attributes:fromSecretTargetAttachmentAttributes",
       "props-physical-name:@aws-cdk/aws-secretsmanager.RotationScheduleProps",

packages/@aws-cdk/aws-secretsmanager/test/secret.test.ts

@@ -434,6 +434,7 @@ test('import by secretArn', () => {
 
   // THEN
   expect(secret.secretArn).toBe(secretArn);
+  expect(secret.secretFullArn).toBe(secretArn);
   expect(secret.secretName).toBe('MySecret');
   expect(secret.encryptionKey).toBeUndefined();
   expect(stack.resolve(secret.secretValue)).toEqual(`{{resolve:secretsmanager:${secretArn}:SecretString:::}}`);
@@ -460,6 +461,18 @@ test('import by secretArn supports secret ARNs without suffixes', () => {
   expect(secret.secretName).toBe('MySecret');
 });
 
+test('import by secretArn does not strip suffixes unless the suffix length is six', () => {
+  // GIVEN
+  const arnWith5CharacterSuffix = 'arn:aws:secretsmanager:eu-west-1:111111111111:secret:github-token';
+  const arnWith6CharacterSuffix = 'arn:aws:secretsmanager:eu-west-1:111111111111:secret:github-token-f3gDy9';
+  const arnWithMultiple6CharacterSuffix = 'arn:aws:secretsmanager:eu-west-1:111111111111:secret:github-token-f3gDy9-acb123';
+
+  // THEN
+  expect(secretsmanager.Secret.fromSecretArn(stack, 'Secret5', arnWith5CharacterSuffix).secretName).toEqual('github-token');
+  expect(secretsmanager.Secret.fromSecretArn(stack, 'Secret6', arnWith6CharacterSuffix).secretName).toEqual('github-token');
+  expect(secretsmanager.Secret.fromSecretArn(stack, 'Secret6Twice', arnWithMultiple6CharacterSuffix).secretName).toEqual('github-token-f3gDy9');
+});
+
 test('import by secretArn supports tokens for ARNs', () => {
   // GIVEN
   const app = new cdk.App();
@@ -491,6 +504,7 @@ test('import by attributes', () => {
 
   // THEN
   expect(secret.secretArn).toBe(secretArn);
+  expect(secret.secretFullArn).toBe(secretArn);
   expect(secret.secretName).toBe('MySecret');
   expect(secret.encryptionKey).toBe(encryptionKey);
   expect(stack.resolve(secret.secretValue)).toBe(`{{resolve:secretsmanager:${secretArn}:SecretString:::}}`);
@@ -507,6 +521,7 @@ test('import by secret name', () => {
   // THEN
   expect(secret.secretArn).toBe(secretName);
   expect(secret.secretName).toBe(secretName);
+  expect(secret.secretFullArn).toBeUndefined();
   expect(stack.resolve(secret.secretValue)).toBe(`{{resolve:secretsmanager:${secretName}:SecretString:::}}`);
   expect(stack.resolve(secret.secretValueFromJson('password'))).toBe(`{{resolve:secretsmanager:${secretName}:SecretString:password::}}`);
 });
@@ -555,6 +570,74 @@ test('import by secret name with grants', () => {
   });
 });
 
+test('import by secret name v2', () => {
+  // GIVEN
+  const secretName = 'MySecret';
+
+  // WHEN
+  const secret = secretsmanager.Secret.fromSecretNameV2(stack, 'Secret', secretName);
+
+  // THEN
+  expect(secret.secretArn).toBe(`arn:${stack.partition}:secretsmanager:${stack.region}:${stack.account}:secret:MySecret`);
+  expect(secret.secretName).toBe(secretName);
+  expect(secret.secretFullArn).toBeUndefined();
+  expect(stack.resolve(secret.secretValue)).toEqual({
+    'Fn::Join': ['', [
+      '{{resolve:secretsmanager:arn:',
+      { Ref: 'AWS::Partition' },
+      ':secretsmanager:',
+      { Ref: 'AWS::Region' },
+      ':',
+      { Ref: 'AWS::AccountId' },
+      ':secret:MySecret:SecretString:::}}',
+    ]],
+  });
+});
+
+test('import by secret name v2 with grants', () => {
+  // GIVEN
+  const role = new iam.Role(stack, 'Role', { assumedBy: new iam.AccountRootPrincipal() });
+  const secret = secretsmanager.Secret.fromSecretNameV2(stack, 'Secret', 'MySecret');
+
+  // WHEN
+  secret.grantRead(role);
+  secret.grantWrite(role);
+
+  // THEN
+  const expectedSecretReference = {
+    'Fn::Join': ['', [
+      'arn:',
+      { Ref: 'AWS::Partition' },
+      ':secretsmanager:',
+      { Ref: 'AWS::Region' },
+      ':',
+      { Ref: 'AWS::AccountId' },
+      ':secret:MySecret-??????',
+    ]],
+  };
+  expect(stack).toHaveResource('AWS::IAM::Policy', {
+    PolicyDocument: {
+      Version: '2012-10-17',
+      Statement: [{
+        Action: [
+          'secretsmanager:GetSecretValue',
+          'secretsmanager:DescribeSecret',
+        ],
+        Effect: 'Allow',
+        Resource: expectedSecretReference,
+      },
+      {
+        Action: [
+          'secretsmanager:PutSecretValue',
+          'secretsmanager:UpdateSecret',
+        ],
+        Effect: 'Allow',
+        Resource: expectedSecretReference,
+      }],
+    },
+  });
+});
+
 test('can attach a secret with attach()', () => {
   // GIVEN
   const secret = new secretsmanager.Secret(stack, 'Secret');