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

Scope down unnecessary permissions #23

Closed
wants to merge 13 commits into from
Closed

Conversation

Almenon
Copy link
Contributor

@Almenon Almenon commented Mar 18, 2023

What does this PR do?

  • Unpins version for more flexibility - AWS version pinning #21
  • Tightens permissions for better security
  • Add secret_prefix var one can use to grant MWAA access to secrets under a certain prefix.

Motivation

My workplace pays me to do devops, and I can use that money to buy donuts. On a more specific level, these changes are needed for our security requirements at iSpot. The version pinning is needed to avoid conflicts.

More

  • Yes, I have tested the PR using my local account setup (Provide any test evidence report under Additional Notes)
  • Yes, I ran pre-commit run -a with this PR

For Moderators

  • E2E Test successfully complete before merge?

Additional Notes

@Almenon Almenon marked this pull request as ready for review March 19, 2023 18:06
@Almenon Almenon requested a review from vara-bonthu as a code owner March 19, 2023 18:06
@askulkarni2
Copy link
Collaborator

@Almenon thank you for the PR. Can you please open separate out the PR for version pinning, secrets, and scoped down permissions? Also please create issues for the secret and permissions so we can examine it closely.

@vara-bonthu
Copy link
Collaborator

vara-bonthu commented Mar 30, 2023

@Almenon thanks for creating the issues. Would you mind splitting the PR? One for TF versions and other one for permissions?

We will merge the versions ASAP and investigate the permissions after.

Thanks 🙏

@Almenon Almenon changed the title Pin version, tighten security, and add secret_prefix Tighten security, and add secret_prefix Apr 6, 2023
Copy link
Collaborator

@vara-bonthu vara-bonthu left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @Almenon ! Left few comments needs addressing other than that we are good to go.

data.tf Outdated Show resolved Hide resolved
variables.tf Outdated Show resolved Hide resolved
One can use iam_role_additional_policies instead
@Almenon Almenon requested a review from a team as a code owner May 11, 2023 15:23
Almenon added 2 commits May 11, 2023 08:24
one can use iam_role_additional_policies instead
no longer needed
@Almenon Almenon changed the title Tighten security, and add secret_prefix Scope down unnecessary permissions May 11, 2023
@Almenon
Copy link
Contributor Author

Almenon commented May 11, 2023

@vara-bonthu done, and good point. Thanks.

@Almenon
Copy link
Contributor Author

Almenon commented Jun 26, 2023

@vara-bonthu following up on this, good to merge?

@vara-bonthu
Copy link
Collaborator

@vara-bonthu following up on this, good to merge?

@Almenon Sorry for the delay. We are planning to test this feature and include this as a part of any upgrades required. I will merge it once its tested

Yuriy Kirbaba and others added 2 commits January 5, 2024 14:36
@Chili-Man
Copy link
Contributor

Chili-Man commented Feb 19, 2024

@vara-bonthu any updates on this? It's important to have the reduced permissions.

@Almenon any interest in updating this PR to resolve the branch conflicts? if not, I'm happy to port over these changes to a new pr that is up to date with the main branch

@Almenon
Copy link
Contributor Author

Almenon commented Feb 19, 2024

Updated. I opened a new PR with conflicts resolved. See #49

@Almenon Almenon closed this Feb 19, 2024
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.

4 participants