integrations / integrations/terraform-provider-github

[BUG]: Out-of-band removal of environment reviewers is not detected on refresh

Open Beginner friendly
#3,609 3 comments 1 reaction 0 assignees View on GitHub
r/repository_environment Type: Bug
Dominant language
Go
Stars
1.2k
Forks
1k
Avg merge
1d 14h
Merged PRs (30d)
8

Description

### Expected Behavior

When a required reviewer is removed from an environment outside Terraform, the next `terraform plan` should report a change, because the configuration still declares the reviewer and GitHub no longer has it.

### Actual Behavior

`terraform plan` reports `No changes`, and the reviewer stays removed. The drift is invisible for as long as the configuration is left alone.

I checked whether the API is at fault and it is not. `GET /repos/{owner}/{repo}/environments/{name}` reports the removal faithfully: once the last reviewer is gone, the response's `protection_rules` array simply has no `required_reviewers` entry in it. The information the provider needs is in the response.

The cause is in `resourceGithubRepositoryEnvironmentRead`, in `github/resource_github_repository_environment.go`. Five fields are populated by three different patterns, and only two of them survive an entry disappearing from `protection_rules`:

| Field | How it is populated | Refreshes on removal? |
|---|---|---|
| `wait_timer` | Reset to `nil` before the loop | Yes |
| `can_admins_bypass` | Read from the top-level `env` object | Yes |
| `deployment_branch_policy` | `if env.DeploymentBranchPolicy != nil { … } else { d.Set(…, []any{}) }` | Yes |
| `reviewers` | Set **only** inside `case "required_reviewers":` | **No** |
| `prevent_self_review` | Set **only** inside the same case | **No** |

`wait_timer` gets it right, and it is worth reading as the fix already present in the same function:

```go
if err := d.Set("wait_timer", nil); err != nil {
return diag.FromErr(err)
}
```

That pre-loop reset means a vanished `wait_timer` entry leaves `nil` behind. `reviewers` and `prevent_self_review` have no equivalent, so when the `required_reviewers` case never executes, `d.Set` is never called for either and the prior state value survives the refresh untouched. `terraform plan` then compares the configuration against stale state and correctly reports no changes about a state that is wrong.

`prevent_self_review` is caught by the same defect for the same reason, since it is set inside that case as well.

This looks like a two-line fix: reset both fields before the loop, the way `wait_timer` already is.

Note this is not #3156, which was about removing `reviewers` through configuration and is fixed. This is the reverse direction: a removal made outside Terraform is never detected.

### Terraform Version

```
Terraform v1.15.8
on darwin_arm64
+ provider registry.terraform.io/integrations/github v6.13.0
```

Also read at `main`, where `github/resource_github_repository_environment.go` is byte-identical to `v6.13.0` apart from a `go-github` v88 to v89 bump and a type-assertion style change. Neither touches this logic.

### GitHub Installation Type

GitHub.com (Free, Pro, or Team)

### Affected Resource(s)

- `github_repository_environment`

### Terraform Configuration Files

```hcl
resource "github_repository_environment" "example" {
repository = "example-repo"
environment = "production"

reviewers {
users = [1234567]
}
}
```

### Steps to Reproduce

1. `terraform apply` the configuration above, on a public repository or a plan tier that allows required reviewers.
2. In the GitHub UI, go to Settings > Environments > production and remove the reviewer, so no reviewers remain.
3. `terraform plan`.

Expected: a change is reported, restoring the reviewer. Actual: `No changes`.

Comparing the API response before and after step 2 shows the `required_reviewers` entry present, then absent, with the rest of the object unchanged.

### Code of Conduct

- [X] I agree to follow this project's Code of Conduct

Contributor guide

Open the contributing guide

Research direction

Read resourceGithubRepositoryEnvironmentRead in github/resource_github_repository_environment.go, starting with the existing wait_timer reset before the protection_rules loop. Verify the refresh behavior when required_reviewers is absent, and consider the issue's described reset pattern for reviewers and prevent_self_review; done means terraform plan detects the externally removed reviewer and restores the declared state.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, terraform
Domain
infrastructure
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.