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

(pipelines): missing SecretsManager permissions for asset action role #15628

Closed
hoegertn opened this issue Jul 17, 2021 · 1 comment · Fixed by #15718
Closed

(pipelines): missing SecretsManager permissions for asset action role #15628

hoegertn opened this issue Jul 17, 2021 · 1 comment · Fixed by #15718
Assignees
Labels
@aws-cdk/aws-secretsmanager Related to AWS Secrets Manager @aws-cdk/pipelines CDK Pipelines library bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@hoegertn
Copy link
Contributor

If you specify environment variables of the SecretsManager for the CodeBuild projects, only the synth and the self-mutate action have the appropriate IAM permissions but all actions get the environment config.

Reproduction Steps

    const pipeline = new pipelines.CodePipeline(this, 'pipeline', {
      synth: new pipelines.ShellStep('Synth', {
        input: pipelines.CodePipelineSource.connection('test/test', 'main', { connectionArn: 'arn' }),
        commands: ['npm ci', 'npx cdk synth'],
      }),
      codeBuildDefaults: {
        buildEnvironment: {
          environmentVariables: {
            FOOBAR: {
              value: 'FoobarSecret',
              type: cb.BuildEnvironmentVariableType.SECRETS_MANAGER,
            },
          },
        },
      },
    });

If the stack/stage contains assets the asset upload project has the environment config

"EnvironmentVariables": [
  {
    "Name": "FOOBAR",
    "Type": "SECRETS_MANAGER",
    "Value": "FoobarSecret"
  }
],

but the role does not have the needed secretsmanager:GetSecretValue permissions. The roles for synth and self-mutate do get the permissions.

What did you expect to happen?

The asset upload role has the needed permissions

What actually happened?

No permissions are granted

Environment

  • CDK CLI Version : 1.114.0
  • Framework Version: 1.114.0
  • Node.js Version: v16.4.2
  • OS : macOS
  • Language (Version): latest TS

Other


This is 🐛 Bug Report

@hoegertn hoegertn added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 17, 2021
@github-actions github-actions bot added the @aws-cdk/aws-secretsmanager Related to AWS Secrets Manager label Jul 17, 2021
@github-actions github-actions bot added the @aws-cdk/pipelines CDK Pipelines library label Jul 17, 2021
rix0rrr added a commit that referenced this issue Jul 22, 2021
(Longer description here...)

(Still needs testing...)

(Still needs the KMS key hack taken out...)

Fixes #15628.
@mergify mergify bot closed this as completed in #15718 Jul 23, 2021
mergify bot pushed a commit that referenced this issue Jul 23, 2021
…ts (#15718)

We used to use an immutable singleton role with `*` permissions for the assets projects, because if there were many different destinations in a pipeline, and each asset build had to publish to each destination, the policy could grow too long and exceed the maximum policy size.

However, this also disabled the automatic policy wrangling that CodeBuild would do for us, like automatically adding permissions to bind to a VPC, and adding permissions to read Secrets Manager secrets. This especially becoming relevant since that now in the modern API, it's possible to modify build the environment in a way that normally automatically adds SecretsManager permission, but now won't (and it's not possible to fix either).

Replace the immutable singleton role with a mutable singleton role, but in such a way that it won't add permissions statements for which it already has a `*` statement (to cut down on duplication), and have the CB project do the automatic VPC bind permissions again.

Fixes #15628.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Aug 3, 2021
…ts (aws#15718)

We used to use an immutable singleton role with `*` permissions for the assets projects, because if there were many different destinations in a pipeline, and each asset build had to publish to each destination, the policy could grow too long and exceed the maximum policy size.

However, this also disabled the automatic policy wrangling that CodeBuild would do for us, like automatically adding permissions to bind to a VPC, and adding permissions to read Secrets Manager secrets. This especially becoming relevant since that now in the modern API, it's possible to modify build the environment in a way that normally automatically adds SecretsManager permission, but now won't (and it's not possible to fix either).

Replace the immutable singleton role with a mutable singleton role, but in such a way that it won't add permissions statements for which it already has a `*` statement (to cut down on duplication), and have the CB project do the automatic VPC bind permissions again.

Fixes aws#15628.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
…ts (aws#15718)

We used to use an immutable singleton role with `*` permissions for the assets projects, because if there were many different destinations in a pipeline, and each asset build had to publish to each destination, the policy could grow too long and exceed the maximum policy size.

However, this also disabled the automatic policy wrangling that CodeBuild would do for us, like automatically adding permissions to bind to a VPC, and adding permissions to read Secrets Manager secrets. This especially becoming relevant since that now in the modern API, it's possible to modify build the environment in a way that normally automatically adds SecretsManager permission, but now won't (and it's not possible to fix either).

Replace the immutable singleton role with a mutable singleton role, but in such a way that it won't add permissions statements for which it already has a `*` statement (to cut down on duplication), and have the CB project do the automatic VPC bind permissions again.

Fixes aws#15628.


----

*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 @aws-cdk/pipelines CDK Pipelines library bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants