-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat(secretsmanager): import secrets by name #10309
Conversation
Adds the ability to import secrets by name, including without the SecretsManager assigned suffix. As long as a secret with the same name has been created in each region with the same name, this allows for the same `fromSecretName` usage in stacks across regions. Oddly enough, most CloudFormation templates that take references to secrets accept either the full-form ARN, including the suffix or just the base secret name (not in ARN format). The one place where a full ARN format is needed is in IAM policy statements, where the wildcard is necessary to account for the suffix. Tested this manually against an existing secret with a CodeBuild project; per the CloudFormation docs, this should work equally well with other SecretsManager-integrated services. fixes #7444 fixes #7949 fixes #7994
15f05aa
to
890af00
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved provided small reconsiderations
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
In #10309, secretName was added to SecretAttributes, but given the ARN is always required, it's fairly redundant. Removing to reduce public API surface area. Not a breaking change, as #10309 has not yet been released. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Uhg. This totally broke my production stack. I'm importing a secret using from another stack by ARN, like this: const dbSecret = secretsmanager.Secret.fromSecretArn(this, 'dbSecret', props.dbSecretArn); I get this error:
@njlynch What's supposed to happen if the /** 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');
} $ cdk --version
1.64.0 (build 9510201) |
Same here since update in 1.64.0 :
|
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*
This also breaks our production stack. |
To prevent that you have to set a fix version of CDK. I'm using pipelines in most cases and only with a valid/fixed CDK version; in that case you don't have any surprise. |
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 introduces an option to the `fromSecretName` factory to control this behavior, so users can set up the secret properly for the service they are integrating with. *Disclaimer:* - I don't *love* this, and am very open to feedback on alternative approaches that would also be backwards compatible. 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'.
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 introduces an option to the `fromSecretName` factory to control this behavior, so users can set up the secret properly for the service they are integrating with. *Disclaimer:* - I don't *love* this, and am very open to feedback on alternative approaches that would also be backwards compatible. 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'. fixes #10519
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
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
) 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 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Adds the ability to import secrets by name, including without the SecretsManager
assigned suffix. As long as a secret with the same name has been created in each
region with the same name, this allows for the same
fromSecretName
usage instacks across regions.
Oddly enough, most CloudFormation templates that take references to secrets
accept either the full-form ARN, including the suffix or just the base secret
name (not in ARN format). The one place where a full ARN format is needed is in
IAM policy statements, where the wildcard is necessary to account for the
suffix.
Tested this manually against an existing secret with a CodeBuild project; per
the CloudFormation docs, this should work equally well with other
SecretsManager-integrated services.
fixes #7444
fixes #7949
fixes #7994
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license