-
Notifications
You must be signed in to change notification settings - Fork 4k
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
fix(codebuild): allow passing the ARN of the Secret in environment variables #13706
fix(codebuild): allow passing the ARN of the Secret in environment variables #13706
Conversation
…riables In the SecretsManager-typed environment variables in CodeBuild, the code in the Project class assumed those would be passed as names. As it turns out, CodeBuild also allows passing there entire ARNs of secrets (both partial, and full), and also optional qualifiers, separated by colons, that specify SecretsManager attributes like the JSON key, or the secret version. Add handling of all of these cases. Fixes aws#12703
resourceName: `${envVariable.value}-??????`, | ||
sep: ':', | ||
})); | ||
if (Token.isUnresolved(envVariableValue)) { |
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.
Is there a test for this condition? I'm not seeing it.
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.
Yep - those are the can be provided as the ARN attribute of a new Secret
and can be provided as the ARN attribute of a new Secret, followed by a JSON key
tests.
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.
Ah, I guess I was looking for a test where a Fn:Split
call was expected, as we might be splitting a token. Now I see that we're assuming the input is partially an unresolved token (e.g., ${secretArn}:jsonKey
).
If I understand correctly, you're supporting something like the above, but not a "full" token'ed value. So if I create a custom resource which returns back my-secret-name:jsonKey
as a Token, then we will be passing the Token as a whole to secretsManagerIamResources
, and the grant won't work. Am I reading that correctly now?
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.
Yep, you're reading it correctly.
What's the behavior that you would want to see here?
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.
Frankly, I'm not sure. I don't know that there's a good solution here. I'm hoping this is a relatively edge-of-the-corner case. :D
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). |
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). |
…riables (aws#13706) In the SecretsManager-typed environment variables in CodeBuild, the code in the Project class assumed those would be passed as names. As it turns out, CodeBuild also allows passing there entire ARNs of secrets (both partial, and full), and also optional qualifiers, separated by colons, that specify SecretsManager attributes like the JSON key, or the secret version. Add handling of all of these cases. Fixes aws#12703 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…riables (aws#13706) In the SecretsManager-typed environment variables in CodeBuild, the code in the Project class assumed those would be passed as names. As it turns out, CodeBuild also allows passing there entire ARNs of secrets (both partial, and full), and also optional qualifiers, separated by colons, that specify SecretsManager attributes like the JSON key, or the secret version. Add handling of all of these cases. Fixes aws#12703 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
In the SecretsManager-typed environment variables in CodeBuild,
the code in the Project class assumed those would be passed as names.
As it turns out, CodeBuild also allows passing there entire ARNs of secrets
(both partial, and full), and also optional qualifiers,
separated by colons, that specify SecretsManager attributes like the JSON key,
or the secret version.
Add handling of all of these cases.
Fixes #12703
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license