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

feat: add organizations as readonly access #106

Merged
merged 22 commits into from Oct 31, 2023
Merged

Conversation

dragosmc
Copy link
Contributor

what

  • Add the ability to have organizations as trustees (read-only) for the ECR repository

why

references

@Gowiem
Copy link
Member

Gowiem commented May 25, 2023

@dragosmc we're reworking these policies in #105 -- Let's hold off on this type of work until that is merged and then you can take another crack at this PR. The way policies were originally (or additionally) implemented is leading to some funky code that I want to put a stop to.

@Gowiem
Copy link
Member

Gowiem commented May 26, 2023

@dragosmc the rework of the funky way we were handling policies has merged (#105). Mind updating this to reflect the new pattern that we create a policy document for each type of policy?

@adamantike
Copy link

It would be great to see this merged!
As a follow up after this one, we can add support for access to specific Organizational Units.

@Gowiem
Copy link
Member

Gowiem commented Jun 29, 2023

@dragosmc friendly ping here. Mind fixing conflicts + reworking after #105?

@dragosmc
Copy link
Contributor Author

Ready for a review 👍

main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
main.tf Show resolved Hide resolved
Signed-off-by: Dragos Ciupureanu <dragos.ciupureanu@gmail.com>
@dragosmc
Copy link
Contributor Author

dragosmc commented Jul 19, 2023

Thanks for the review @adamantike - everythng should be fixed now.

LE: about the docs, is that an automated job that runs in CI to update them? I couldn't find a target in makefile for that

main.tf Outdated Show resolved Hide resolved
Signed-off-by: Dragos Ciupureanu <dragos.ciupureanu@gmail.com>
@dragosmc dragosmc requested a review from adamantike July 20, 2023 09:42
@dragosmc
Copy link
Contributor Author

Hi team, this is ready for another round of review. 🎉

@joe-niland joe-niland requested review from joe-niland and removed request for adamantike August 9, 2023 21:42
@adamantike
Copy link

@joe-niland, gentle ping. Can this be reviewed?

@Gowiem
Copy link
Member

Gowiem commented Oct 24, 2023

@dragosmc generally, this looks good and seems to follow the pattern we've set out. Thanks for the updates. To move this forward, please run the following commands, commit, and push the changes:

make init
make github/init
make readme

I'll give it another review once that is done -- Thanks!

@Gowiem Gowiem self-requested a review October 24, 2023 16:19
@dragosmc
Copy link
Contributor Author

dragosmc commented Oct 24, 2023

@dragosmc generally, this looks good and seems to follow the pattern we've set out. Thanks for the updates. To move this forward, please run the following commands, commit, and push the changes:

make init
make github/init
make readme

I'll give it another review once that is done -- Thanks!

Trying to run the above I get the following error:

* Installing terraform-docs to /Users/dragos/opensource/terraform-aws-ecr/build-harness/vendor
mkdir -p -m a+rX /Users/dragos/opensource/terraform-aws-ecr/build-harness/vendor
[ -n "/tmp" ] && [ -n "terraform-docs" ] && rm -rf "/tmp/terraform-docs"
mkdir -p /tmp/terraform-docs
curl --retry 3 --retry-delay 5 --fail -sSL -o - https://github.com/segmentio/terraform-docs/releases/download/v0.16.0/terraform-docs-v0.16.0-darwin-arm64.tar.gz | tar -zx -C /tmp/terraform-docs
find /tmp/terraform-docs -type f -name terraform-docs | xargs -I {} cp -f {} /Users/dragos/opensource/terraform-aws-ecr/build-harness/vendor/terraform-docs
chmod +x /Users/dragos/opensource/terraform-aws-ecr/build-harness/vendor/terraform-docs
[ -n "/tmp" ] && [ -n "terraform-docs" ] && rm -rf "/tmp/terraform-docs"
make: gomplate: No such file or directory
make: *** [readme/build] Error 1

@Gowiem
Copy link
Member

Gowiem commented Oct 24, 2023

@dragosmc I believe the gomplate installation is broken via make. Can you please install that tool via whatever platform you're on and try again. If you're on Mac, then brew install gomplate will do.

@max-lobur
Copy link
Contributor

brew install make
make --version
GNU Make 4.4.1

make precommit/terraform # Run dockerized version of precommit hook that we use in CI.

@max-lobur
Copy link
Contributor

Ensuring make version alone might fix the issue too.

Signed-off-by: Dragos Ciupureanu <dragos.ciupureanu@gmail.com>
@dragosmc
Copy link
Contributor Author

Thanks @max-lobur @Gowiem . Ensure the correct make version works fine. I see the tests for the readme still fail so not sure what the problem is. Could you have a look please and let me know if there's anything else I need to run?

@@ -10,7 +10,7 @@

| Name | Version |
|------|---------|
| <a name="provider_aws"></a> [aws](#provider\_aws) | >= 4.22.0 |
| <a name="provider_aws"></a> [aws](#provider\_aws) | 5.8.0 |
Copy link
Member

Choose a reason for hiding this comment

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

@dragosmc this change here is what is causing your README failures. I think this may be an issue with an older version of terraform-docs. Maybe try upgrading that and then seeing if this gets the changes when you run make readme again?

Choose a reason for hiding this comment

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

I have experienced this issue in the past, where terraform-docs resolves the current provider version instead of maintaining the version constraint. I have always just reverted those changed lines back, if I'm not touching anything related to providers or dependencies.

Signed-off-by: Dragos Ciupureanu <dragos.ciupureanu@gmail.com>
@Gowiem
Copy link
Member

Gowiem commented Oct 30, 2023

/terratest

@max-lobur max-lobur merged commit 69cd569 into cloudposse:main Oct 31, 2023
10 checks passed
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.

Allow the whole AWS Org for readonly
5 participants