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

Generate IAM Policy with all required Actions #740

Merged
merged 6 commits into from
Jul 7, 2022

Conversation

Zvikan
Copy link
Contributor

@Zvikan Zvikan commented Jul 6, 2022

What does this PR do?

  • Generates IAM policy using iamlive
    • During e2e parallel workflow, each example will have iamlive running and it will print out the result, once done will upload the result to s3 bucket
    • At the end of the workflow, the workflow will run a python script reading each of the files in the bucket (each policy generated ) and will merge it into a single policy, and will print it out.

Motivation

More

Note: Not all the PRs required examples and docs except a new pattern or add-on added.

For Moderators

  • E2E Test successfully complete before merge?

Additional Notes

Copy link
Contributor

@askulkarni2 askulkarni2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Can we add a doc page that shows the policy for a few key examples?

kill ${{ env.iamlive_pid }}
while $(kill -0 ${{ env.iamlive_pid }} 2>/dev/null); do sleep 1; done;
cat ${HOME}/policy.json
aws s3 cp ${HOME}/policy.json s3://eks-blueprints-iam-policies/${{ matrix.example_path }}.json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. We should make the bucket name either a workflow global env variable or a github secret to ensure its defined in one place correctly (is it terraform-eks-blueprints-iam-policies-examples or eks-blueprints-iam-policies ?)
  2. do we need to add in a git ref or something so that PRs aren't clobbering one another? adding something like /${{ github.ref }}/ into the prefix?
  3. If we do that, do we need/want to set a lifecycle policy on these

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. set as global env var, correct bucket name was set (good catch!)
  2. currently the python script merges all the files seeing in the bucket, as of now we cant run multiple parallel e2e flows at the same time.
    I made the suggested change but then thought about it, the current script is merging all the files seeing in this bucket, as we go we can start tweak this around according to use case, I did enable versioning on the bucket for now just in-case.

@Zvikan Zvikan had a problem deploying to EKS Blueprints Test July 7, 2022 15:11 Failure
@Zvikan Zvikan had a problem deploying to EKS Blueprints Test July 7, 2022 15:32 Failure
@Zvikan Zvikan had a problem deploying to EKS Blueprints Test July 7, 2022 15:35 Failure
Copy link
Contributor

@askulkarni2 askulkarni2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

@bryantbiggs bryantbiggs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great - should turn this into a GitHub action

@Zvikan Zvikan merged commit c60003d into main Jul 7, 2022
@Zvikan Zvikan deleted the issue-562/min-iam-permissions branch July 7, 2022 15:47
allamand pushed a commit to allamand/terraform-aws-eks-blueprints that referenced this pull request Dec 15, 2022
* feat: generate iam policy per example

* feat: add final policy generator script

* chore: clean code - workflow env, one liner syntax

* chore: bucket name as global env

* fix: place policy per gh ref

* fix: revert bb18d5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants