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

fix(pipelines): policy size too large at around ~70 actions #20189

Closed
wants to merge 20 commits into from

Conversation

rix0rrr
Copy link
Contributor

@rix0rrr rix0rrr commented May 3, 2022

Two changes:

  • Collapse CodeBuild action Roles: each CodeBuild step used to create a
    fresh Role to run the CodeBuild action. Change to use one Role for all
    CodeBuild actions. This saves a lot of resources and policy space when
    using a lot of CodeBuild steps, and doesn't appreciably change the
    security posture of the Pipeline (note: this is not about the
    Execution Role of the CodeBuild projects, this is about the Role
    assumed by the Pipeline to initiate execution of the Project).
  • If inline policies grow bigger than 10k, split additional statements
    off into ManagedPolicies.

Since we want to do the splitting post-merging (to get the most bang for
our buck), we now need to do statement merging during the prepare
phase (that is, pre-rendering, instead of post-rendering). That means it
had to be modified to work on PolicyStatement objects, instead of on
raw IAM JSON documents. This requires being able to deduplicate principals
at the object graph level as well, so we need to be able to compare them.

Closes #19276, closes #19939, closes #19835.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

Two changes:

- Collapse CodeBuild action Roles: each CodeBuild step used to create a
  fresh Role to run the CodeBuild action. Change to use one Role for all
  CodeBuild actions. This saves a lot of resources and policy space when
  using a lot of CodeBuild steps, and doesn't appreciably change the
  security posture of the Pipeline (note: this is *not* about the
  Execution Role of the CodeBuild projects, this is about the Role
  assumed by the Pipeline to initiate execution of the Project).
- If inline policies grow bigger than 10k, split additional statements
  off into ManagedPolicies.

Since we want to do the splitting post-merging (to get the most bang for
our buck), we now need to do statement merging during the `prepare`
phase (that is, pre-rendering, instead of post-rendering). That means it
had to be modified to work on `PolicyStatement` objects, instead of on
raw IAM JSON documents.

Closes #19276, closes #19939, closes #19835.
@rix0rrr rix0rrr requested a review from a team May 3, 2022 12:08
@rix0rrr rix0rrr self-assigned this May 3, 2022
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label May 3, 2022
@gitpod-io
Copy link

gitpod-io bot commented May 3, 2022

@aws-cdk-automation aws-cdk-automation requested a review from a team May 3, 2022 12:08
@github-actions github-actions bot added bug This issue is a bug. p1 labels May 3, 2022
@github-actions github-actions bot added the effort/medium Medium work item – several days of effort label May 5, 2022
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 276220f
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

rix0rrr added a commit that referenced this pull request May 18, 2022
(This change has been split off from #20189 because that PR was growing
too big)

Collapse CodeBuild action Roles: each CodeBuild step used to create a
fresh Role to run the CodeBuild action. Change to use one Role for all
CodeBuild actions. This saves a lot of resources and policy space when
using many CodeBuild steps, and doesn't appreciably change the
security posture of the Pipeline (note: this is not about the
Execution Role of the CodeBuild projects, this is about the Role
assumed by the Pipeline to initiate execution of the Project).

Relates to #19276, #19939, #19835.
@rix0rrr
Copy link
Contributor Author

rix0rrr commented May 18, 2022

This PR was getting too big and has been split into 4 other PRs.

@rix0rrr rix0rrr closed this May 18, 2022
mergify bot pushed a commit that referenced this pull request May 24, 2022
(This change has been split off from #20189 because that PR was growing
too big)

Collapse CodeBuild action Roles: each CodeBuild step used to create a
fresh Role to run the CodeBuild action. Change to use one Role for all
CodeBuild actions. This saves a lot of resources and policy space when
using many CodeBuild steps, and doesn't appreciably change the
security posture of the Pipeline (note: this is not about the
Execution Role of the CodeBuild projects, this is about the Role
assumed by the Pipeline to initiate execution of the Project).

Relates to #19276, #19939, #19835.


----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@TheRealAmazonKendra TheRealAmazonKendra deleted the huijbers/pipelines-policy-sizes branch February 14, 2023 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. contribution/core This is a PR that came from AWS. effort/medium Medium work item – several days of effort p1
Projects
None yet
2 participants