Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SecretsManager] Secret.fromSecretArn(): Error: invalid ARN format; no secret name provided (#10309) #10520

Closed
rrrix opened this issue Sep 25, 2020 · 3 comments · Fixed by #10568
Assignees
Labels
@aws-cdk/aws-secretsmanager Related to AWS Secrets Manager bug This issue is a bug. effort/small Small work item – less than a day of effort in-progress This issue is being actively worked on. p1

Comments

@rrrix
Copy link

rrrix commented Sep 25, 2020

When using Secret.fromSecretArn(scope, id, secretArn) to retrieve an ARN from another stack, and the secretArn variable is an unresolved token (as it has not been created yet) instead of a real / concrete value, the functionality in PR #10309 causes an error of Error: invalid ARN format; no secret name provided

Reproduction Steps

I think this should do it:

const app = new App();
const stackA = new Stack(app, 'stackA')
const stackB = new Stack(app, 'stackB')

const secret1 = new Secret(stackA, 'secret1');

// Boom 💥
const secret2 = Secret.fromSecretArn(stackB, 'secret2', secret1.secretArn);

What did you expect to happen?

Work as it did before Release 1.64.0 (which worked with Tokens just fine).

What actually happened?

/Users/me/project/cdk/node_modules/@aws-cdk/aws-secretsmanager/lib/secret.ts:605
  throw new Error('invalid ARN format; no secret name provided');
        ^
Error: invalid ARN format; no secret name provided
    at parseSecretName (/Users/me/project/cdk/node_modules/@aws-cdk/aws-secretsmanager/lib/secret.ts:605:9)
    at new Import (/Users/me/project/cdk/node_modules/@aws-cdk/aws-secretsmanager/lib/secret.ts:282:36)
    at Function.fromSecretAttributes (/Users/me/project/cdk/node_modules/@aws-cdk/aws-secretsmanager/lib/secret.ts:286:12)
    at Function.fromSecretArn (/Users/me/project/cdk/node_modules/@aws-cdk/aws-secretsmanager/lib/secret.ts:245:19)
    at new APIService (/Users/me/project/cdk/src/services/api.ts:77:44)
    at main (/Users/me/project/cdk/src/index.ts:190:22)

Environment

  • CLI Version : 1.64.0 (build 9510201)
  • Framework Version: 1.64.0
  • Node.js Version: v14.8.0
  • OS : MacOS Catalina
  • Language (Version): TypeScript 3.8.5

Other

I'll follow up in the PR thread too if that's better. #10309 (comment)

I'll stare at the offending piece of code tomorrow to see if I can come up with a suggestion.

/** Returns the secret name if defined, otherwise attempts to parse it from the ARN. */
export function parseSecretName(construct: IConstruct, secretArn: string, secretName?: string) {
if (secretName) { return secretName; }
const resourceName = Stack.of(construct).parseArn(secretArn).resourceName;
if (resourceName) {
// Secret resource names are in the format `${secretName}-${SecretsManager suffix}`
const secretNameFromArn = resourceName.substr(0, resourceName.lastIndexOf('-'));
if (secretNameFromArn) { return secretNameFromArn; }
}
throw new Error('invalid ARN format; no secret name provided');
}


This is 🐛 Bug Report

@rrrix rrrix added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 25, 2020
@github-actions github-actions bot added the @aws-cdk/aws-secretsmanager Related to AWS Secrets Manager label Sep 25, 2020
@Cloudrage
Copy link

+1, since 1.64.0 :

invalid ARN format; no secret name provided
Subprocess exited with error 1

@rrrix rrrix changed the title [SecretsManager] #10309 Broke using Secret.fromSecretArn() if the ARN is a Token (does not exist yet) [SecretsManager] Secret.fromSecretArn(): Error: invalid ARN format; no secret name provided (#10309) Sep 25, 2020
@rrrix
Copy link
Author

rrrix commented Sep 25, 2020

Here's a unit test to reproduce the problem.

Works with 1.63.0, breaks with 1.64.0.

import { Secret } from '@aws-cdk/aws-secretsmanager';
import { App, Stack } from '@aws-cdk/core';

describe('Cross-Stack Secrets', () => {
  it('Can import a Secret ARN that is a Token', () => {
    const app = new App();
    const stackA = new Stack(app, 'stackA');
    const stackB = new Stack(app, 'stackB');
    const secret1 = new Secret(stackA, 'secret1');
    const secret2 = Secret.fromSecretArn(stackB, 'secret2', secret1.secretArn);
    expect(secret2.secretArn).toEqual(secret1.secretArn);
  });
});

With 1.63.0

$ cdk --version ; yarn list @aws-cdk/aws-secretsmanager
yarn list v1.22.4
warning Filtering by arguments is deprecated. Please use the pattern option instead.
└─ @aws-cdk/aws-secretsmanager@1.63.0
1.63.0 (build 7a68125)

$ jest --silent test/secret.fromSecretArn.test.ts
 PASS  test/secret.fromSecretArn.test.ts (7.819 s)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        8.64 s, estimated 9 s
✨  Done in 9.70s.

With 1.64.0

$ cdk --version ; yarn list @aws-cdk/aws-secretsmanager
1.64.0 (build 9510201)
yarn list v1.22.4
warning Filtering by arguments is deprecated. Please use the pattern option instead.
└─ @aws-cdk/aws-secretsmanager@1.64.0
✨  Done in 0.82s.

$ yarn test test/secret.fromSecretArn.test.ts
yarn run v1.22.4

$ jest --silent test/secret.fromSecretArn.test.ts
 FAIL  test/secret.fromSecretArn.test.ts (7.591 s)
  ● Cross-Stack Secrets › Can import a Secret ARN that is a Token

    invalid ARN format; no secret name provided

       8 |     const stackB = new Stack(app, 'stackB');
       9 |     const secret1 = new Secret(stackA, 'secret1');
    > 10 |     const secret2 = Secret.fromSecretArn(stackB, 'secret2', secret1.secretArn);
         |                            ^
      11 |     expect(secret2.secretArn).toEqual(secret1.secretArn);
      12 |   });
      13 | });

      at parseSecretName (node_modules/@aws-cdk/aws-secretsmanager/lib/secret.ts:605:9)
      at new Import (node_modules/@aws-cdk/aws-secretsmanager/lib/secret.ts:282:36)
      at Function.fromSecretAttributes (node_modules/@aws-cdk/aws-secretsmanager/lib/secret.ts:286:12)
      at Function.fromSecretArn (node_modules/@aws-cdk/aws-secretsmanager/lib/secret.ts:245:19)
      at Object.<anonymous> (test/secret.fromSecretArn.test.ts:10:28)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        8.394 s

@njlynch njlynch added p0 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Sep 28, 2020
njlynch added a commit that referenced this issue Sep 28, 2020
The feature to support importing secrets by name (#10309) failed to handle
scenarios where the secret ARN is a token, due to parsing the ARN to retrieve
the secret name.

fixes #10520
@njlynch
Copy link
Contributor

njlynch commented Sep 28, 2020

Whoops! So sorry for the breakage! Thanks for the bug report, repo steps and example test!

I've posted #10568 to fix the issue.

@njlynch njlynch added in-progress This issue is being actively worked on. p1 and removed p0 labels Sep 28, 2020
njlynch added a commit that referenced this issue Sep 28, 2020
The feature to support importing secrets by name (#10309) failed to handle
scenarios where the secret ARN is a token, due to parsing the ARN to retrieve
the secret name.

fixes #10520
@mergify mergify bot closed this as completed in #10568 Sep 28, 2020
mergify bot pushed a commit that referenced this issue Sep 28, 2020
The feature to support importing secrets by name (#10309) failed to handle
scenarios where the secret ARN is a token, due to parsing the ARN to retrieve
the secret name.

fixes #10520


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-secretsmanager Related to AWS Secrets Manager bug This issue is a bug. effort/small Small work item – less than a day of effort in-progress This issue is being actively worked on. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants