integrations / integrations/terraform-provider-github

[BUG]: github_repository_environment_deployment_policy silently stores policy_id = 0 when the branch policy already exists (303)

Open
#3,611 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
1.2k
Forks
1k
Avg merge
1d 14h
Merged PRs (30d)
8

Description

### Expected Behavior

When creating a `github_repository_environment_deployment_policy` whose branch pattern already exists in GitHub, the provider should either adopt the existing policy or fail with an actionable error telling the user to import it.

It should never persist `policy_id = 0`.

### Actual Behavior

`Create` succeeds silently and writes `policy_id = 0` with a resource ID of `::0`. Nothing surfaces at apply time.

On the next refresh-enabled plan the resource enters a permanent loop:

1. `Read` issues `GET /repos/{owner}/{repo}/environments/{env}/deployment-branch-policies/0`
2. GitHub returns `404`
3. The provider logs `Deployment branch policy not found, removing from state.` and calls `d.SetId("")`
4. Terraform plans the resource for creation again
5. The create hits the same already-exists path and stores `policy_id = 0` again

Every refresh-enabled run therefore reports the same phantom `+ create`, and every apply reports success without fixing anything.

#### Root cause

`Create` never validates the ID returned by the API:

```go
policy, _, err := client.Repositories.CreateDeploymentBranchPolicy(ctx, owner, repoName, url.PathEscape(envName), &createData)
if err != nil {
return diag.FromErr(err)
}

id, err := buildID(repoName, escapeIDPart(envName), strconv.FormatInt(policy.GetID(), 10)) // -> "::0"
...
if err := d.Set("policy_id", policy.GetID()); err != nil { // -> 0
```

The API [documents `303` when the same branch name pattern already exists](https://docs.github.com/en/rest/deployments/branch-policies#create-a-deployment-branch-policy). I verified the resulting behaviour directly against `api.github.com` (see Debug Output):

* The `303` carries `Location` pointing at the **collection** endpoint, not at the existing policy.
* go-github uses a default `http.Client`, so the `303` is auto-followed and converted to a `GET`, returning `200` with `{"total_count": N, "branch_policies": [...]}`. `CheckResponse` even notes that its redirect branch "should never happen with the default `CheckRedirect`".
* Decoding that payload into `*DeploymentBranchPolicy` leaves every field nil, so `GetID()` returns `0` and `err` is `nil`.

This is deterministic, not a race: any create against an already-existing pattern yields `0`.

Corroborating detail from a corrupted state file: `repository_id` is populated correctly on the broken instances while `policy_id` is `0`, which proves execution passed the `err != nil` check and the subsequent `Repositories.Get` succeeded.

#### Why this matters

Adopting pre-existing branch policies is a normal scenario, not an edge case:

* GitHub Pages auto-provisions the `github-pages` environment together with deployment branch policies.
* Policies created in the UI before the repository was brought under Terraform.
* Any state loss or re-adoption of a repository that already had policies.

In one workspace, 8 resources were stuck in this state and reappeared as phantom creates in every refresh-enabled run. Recovery required `terraform state rm` plus `terraform import` with the real policy ID looked up from the list endpoint.

#### Suggested fix

1. **Minimum:** reject a zero ID in `Create`, e.g. `if policy.GetID() == 0 { return diag.Errorf("deployment branch policy %q already exists in environment %q; import it as ::", pattern, envName) }`. Failing loudly is strictly better than persisting an ID that can never be read back.
2. **Better:** treat `303` as already-exists, resolve the policy via `ListDeploymentBranchPolicies`, and adopt its real ID. The `303` already hands you the collection URL, so this is cheap.
3. **Test:** create a branch policy out of band, apply the matching resource, then refresh and assert there is no diff.

#### Possible regression

This used to fail loudly. Before #2993 (v6.9.1), `Create` called `Read`, which `404`d on ID `0` and produced `Provider produced inconsistent result after apply: Root object was present, but now absent` — see #2843, where multiple people reported plans recreating deployment policies that already existed. Removing the post-create read fixed that crash but converted the failure mode into silent state corruption that only appears on refresh.

### Terraform Version

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

### GitHub Installation Type

- [x] GitHub Enterprise Cloud with Personal Accounts (github.com)

### Affected Resource(s)

- `github_repository_environment_deployment_policy`

### Terraform Configuration Files

```hcl
resource "github_repository_environment" "example" {
repository = "REPO"
environment = "my-env"

deployment_branch_policy {
protected_branches = false
custom_branch_policies = true
}
}

# A branch policy with pattern "main" already exists in this environment,
# for example because it was created in the UI or auto-provisioned by GitHub.
resource "github_repository_environment_deployment_policy" "example" {
repository = github_repository_environment.example.repository
environment = github_repository_environment.example.environment
branch_pattern = "main"
}
```

### Steps to Reproduce

1. Create a deployment branch policy with pattern `main` out of band (via the UI or the REST API), or let GitHub Pages provision the `github-pages` environment, which creates branch policies automatically.
2. Declare the equivalent `github_repository_environment_deployment_policy` with `branch_pattern = "main"`.
3. `terraform apply` — reports success. State now contains `policy_id = 0` and an ID ending in `:0`.
4. `terraform plan -refresh=true` — the resource is planned for creation again.
5. Repeat 3 and 4 indefinitely.

### Debug Output

```shell
# 1. Create a policy whose pattern already exists. Redirect NOT followed.
# Placeholders substituted for the real repository and IDs.
$ curl -i -X POST \
-H "Accept: application/vnd.github+json" \
-H "X-GitHub-Api-Version: 2022-11-28" \
-d '{"name":"main","type":"branch"}' \
https://api.github.com/repos/OWNER/REPO/environments/my-env/deployment-branch-policies

HTTP/2 303
location: https://api.github.com/repositories/1234567890/environments/my-env/deployment-branch-policies
(empty body)

# 2. Same request with the redirect followed as a GET, which is what
# go-github does with the default http.Client.
final_status=200
final_url=https://api.github.com/repositories/1234567890/environments/my-env/deployment-branch-policies

# Body returned by the followed request:
{"total_count":5,"branch_policies":[{"id":11111111,"node_id":"...","name":"canary","type":"branch"}, ...]}

# Decoding that body into *DeploymentBranchPolicy:
top-level .id -> ABSENT => GetID() == 0
top-level .name -> ABSENT => GetName() == ""
err -> nil

# Resulting state, before any refresh:
# id = "REPO:my-env:0"
# policy_id = 0
# repository_id = 1234567890 <- correctly populated, so Create ran to completion
```

### Code of Conduct

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

Contributor guide

Open the contributing guide

Research direction

Start at the github_repository_environment_deployment_policy resource's Create and Read paths, especially the CreateDeploymentBranchPolicy call and buildID handling. Reproduce an existing branch-pattern policy, then run the suggested apply and refresh scenario. Done means the resource either adopts the existing policy with its real ID or fails with an actionable import error, without persisting policy_id = 0.

Written by the indexing model from the issue text.

Assessment

Tech stack
github, go
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.