fix: detect out-of-band removal of environment reviewers on refresh - #3616
Draft
nitinjain999 wants to merge 1 commit into
Draft
fix: detect out-of-band removal of environment reviewers on refresh#3616nitinjain999 wants to merge 1 commit into
nitinjain999 wants to merge 1 commit into
Conversation
reviewers and prevent_self_review were only written to state inside the required_reviewers case of the protection_rules loop in resourceGithubRepositoryEnvironmentRead. Once the last reviewer is removed outside Terraform, the API stops returning a required_reviewers entry, so that case never runs and the stale state value survives the refresh. Reset both fields before the loop, the way wait_timer already is. Fixes integrations#3609
|
👋 Hi, and thank you for this contribution! This repo is maintained by GitHub and community members on a best-effort basis. We'll get to this as soon as we can. You can help us prioritize by joining the discussion on open issues and PRs, sharing details on the changes you need, and reviewing other contributions. 🤖 This is an automated message. |
Contributor
Author
|
This PR is functionally ready for review — it's still marked as a draft only because this account is currently at the repo's open-PR limit (#3464, #3462 also open). Happy to flip it to "Ready for review" as soon as a slot frees up, or a maintainer is welcome to do so directly. @deiga @stevehipwell |
Collaborator
|
I think this is a duplicate of #3583 Please coordinate there instead of opening a new PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3609.
reviewersandprevent_self_reviewinresourceGithubRepositoryEnvironmentReadwere only ever written to state insidecase "required_reviewers":of theprotection_rulesloop. Once the last required reviewer is removed outside Terraform, the GitHub API stops returning arequired_reviewersentry inprotection_rulesat all, so that case never runs,d.Setis never called for either field, and the prior state value silently survives the refresh.terraform planthen reportsNo changeseven though the reviewer is gone.wait_timeralready avoids this exact problem by being reset tonilbefore the loop runs. This PR applies the same pattern toreviewersandprevent_self_review.Changes
reviewersto[]any{}andprevent_self_reviewtofalsebefore theprotection_rulesloop ingithub/resource_github_repository_environment.go, so a rule missing from the API response is correctly treated as absent.github/resource_github_repository_environment_unit_test.go) that mocks aGetEnvironmentresponse withprotection_rules: [](the exact shape GitHub returns once the last reviewer is removed, per the debug logs in the issue) against a prior state that has reviewers set, and asserts both fields refresh to empty/false.Test plan
go build ./...,go vet ./github/...,gofmt -lall clean.go test ./github/... -run '^Test[^A]|^TestA[^c]|^TestAc[^c]').