-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat(codepipeline): support environment variables for actions #35741
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
base: main
Are you sure you want to change the base?
Conversation
/** | ||
* The environment variables for the action. | ||
* | ||
* @default - no environment variables | ||
*/ | ||
readonly actionEnvironmentVariables?: EnvironmentVariable[]; |
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.
Avoided the name environmentVariables
because CodeBuildActionProps
that extends codepipeline.CommonAwsActionProps
and this CommonActionProps
has already used a property with the same name and an error occurs if the name is used in this props.
// Don't use `grantRead` method for the secret because `secretsmanager:DescribeSecret` is not required. | ||
// See https://docs.aws.amazon.com/codepipeline/latest/userguide/action-reference-Commands.html#action-reference-Commands-envvars | ||
options.role.addToPrincipalPolicy(new iam.PolicyStatement({ | ||
actions: ['secretsmanager:GetSecretValue'], | ||
resources: [secretArn], | ||
})); |
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.
To use the SecretsManager, you must add the following permissions to your pipeline service role:
{
"Effect": "Allow",
"Action": [
"secretsmanager:GetSecretValue"
],
"Resource": [
"SECRET_ARN"
]
}
/** | ||
* Bind the environment variable to the action. | ||
* @internal | ||
*/ | ||
public abstract _bind(scope: Construct, actionProperties: ActionProperties, options: ActionBindOptions): void; |
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.
Originally, this abstract method was not made, but the concrete bind method was only created for SecretsManagerEnvironmentVariable
. However, considering that support for PARAMETER_STORE
may be added in the future, such as the one for CodeBuild, it is better to create the abstract method, so I did it this way. Otherwise, it would be necessary to implement the bind calling as follows. If more types are added in the future, the code will become even more complicated.
envVars?.forEach(envVar => {
if (envVar instanceof SecretsManagerEnvironmentVariable || envVar instanceof ParameterStoreEnvironmentVariable) {
envVar._bind(scope, this.actionProperties, options);
}
});
Now we can write like:
envVars?.forEach(envVar => {
envVar._bind(scope, this.actionProperties, options);
});
Issue # (if applicable)
N/A
Reason for this change
CodePipeline supports the Environment Variables in actions, but L2 Pipeline with actions doesn't support it.
SECRETS_MANAGER
type is only supported for the Commands action.Description of changes
Added
EnvironmentVariable
withPlaintextEnvironmentVariable
andSecretsManagerEnvironmentVariable
.Describe any new or updated permissions being added
Description of how you validated changes
Both unit tests and integ tests.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license