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

Update github_organization_settings to only need RO permissions at plan time #1321

Conversation

avgalani
Copy link
Contributor

@avgalani avgalani commented Oct 11, 2022

This PR aims to fix #1322.

As part of #1125, a new resource was added that allows the manipulation of Github Organization Settings.

In our setup, we mint Github tokens with read-only permissions in the CI for every terraform plan (which occurs on:pull_request) and we mint tokens with read-write permissions only on runs occuring on the main branch (after merging)

However, we found that at plan time, the github_organization_settings resource requires "organization_administration": "write" permissions, which doesn't meet the Principle of least privilege.

We've verified that the provider still works correctly with the changes proposed in this PR by building the provider locally & validating against it. We've also run all existing tests for this resource & they still pass as well.

The Organizations.Get method returns the same object as the Organizations.Edit method, however using the Get method meets least privilege criteria on terraform plan time.

I would love to provide any more information if required or produce changes deemed necessary by the maintainers of the provider to get this change in.

Thank you!

Copy link
Contributor

@kuhlman-labs kuhlman-labs left a comment

Choose a reason for hiding this comment

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

LGTM. 👍🏻

@kuhlman-labs
Copy link
Contributor

@kfcampbell I have tested this and all looks good.

@avgalani avgalani force-pushed the fix/github_organization_settings_plan_permissions branch from a1c7969 to 02d3272 Compare October 20, 2022 07:24
Copy link
Member

@kfcampbell kfcampbell left a comment

Choose a reason for hiding this comment

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

Interesting fix. Thanks!

@kfcampbell kfcampbell merged commit f3640e6 into integrations:main Oct 24, 2022
kazaker pushed a commit to auto1-oss/terraform-provider-github that referenced this pull request Dec 28, 2022
avidspartan1 pushed a commit to avidspartan1/terraform-provider-github that referenced this pull request Feb 5, 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.

github_organizations_settings requires write permissions at plan time
3 participants