Skip to content

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

Description

@mkushakov

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 <repo>:<env>: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:

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)) // -> "<repo>:<env>:0"
...
if err := d.Set("policy_id", policy.GetID()); err != nil {                                  // -> 0

The API documents 303 when the same branch name pattern already exists. 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 <repository>:<environment>:<policy_id>", 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 404d 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

  • GitHub Enterprise Cloud with Personal Accounts (github.com)

Affected Resource(s)

  • github_repository_environment_deployment_policy

Terraform Configuration Files

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

# 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

  • I agree to follow this project's Code of Conduct

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions