terraform-provider-github icon indicating copy to clipboard operation
terraform-provider-github copied to clipboard

unable to update branch_protections for github repos

Open balusarakesh opened this issue 2 years ago • 22 comments

Hi there,

We were trying to enable enforce_admins = true and we see a TF plan like the following but when we apply we get an error. As you can see below there are no changes to required_status_checks but we are not able to apply it.

  # module.repositories.github_branch_protection_v3.managed["REPO_NAME:BRANCH_NAME"] will be updated in-place
  ~ resource "github_branch_protection_v3" "managed" {
      ~ enforce_admins                  = false -> true
        id                              = "REPO_NAME:BRANCH_NAME"
        # (5 unchanged attributes hidden)


        # (2 unchanged blocks hidden)
    }

Terraform Version

  • 1.0.9
  • terraform-provider-github: v4.24.1

Affected Resource(s)

Please list the resources as a list, for example:

  • github_branch_protection_v3

If this issue appears to affect multiple resources, it may be an issue with Terraform's core, so please mention this.

Terraform Configuration Files

resource "github_branch_protection_v3" "example" {
  repository     = github_repository.example.name
  branch         = "main"
  enforce_admins = true

  required_status_checks {
    strict   = true
    contexts = []
  }

}

Debug Output

unfortunately I'm not allowed to share full debug output due to company policy, here's some relevant debug logs:

2022-05-12T09:24:17.198-0700 [DEBUG] provider.terraform-provider-github_v4.24.1: {
2022-05-12T09:24:17.198-0700 [DEBUG] provider.terraform-provider-github_v4.24.1:  "message": "Invalid request.\n\nNo subschema in \"anyOf\" matched.\nNo subschema in \"oneOf\" matched.\nNot all subschemas of \"allOf\" matched.\nFor 'anyOf/1', {\"strict\"=>true} is not a null.",
2022-05-12T09:24:17.198-0700 [DEBUG] provider.terraform-provider-github_v4.24.1:  "documentation_url": "https://docs.github.com/rest/reference/repos#update-branch-protection"
2022-05-12T09:24:17.198-0700 [DEBUG] provider.terraform-provider-github_v4.24.1: }

│ Error: PUT https://api.github.com/repos/ORG_NAME/REPO_NAME/branches/BRANCH_NAME/protection: 422 Invalid request.
│
│ No subschema in "anyOf" matched.
│ No subschema in "oneOf" matched.
│ Not all subschemas of "allOf" matched.
│ For 'anyOf/1', {"strict"=>true} is not a null. []
│
│   with module.repositories.github_branch_protection_v3.managed["REPO_NAME:BRANCH_NAME"],
│   on ../../../modules/repositories/repository-branch-protection.tf line 1, in resource "github_branch_protection_v3" "managed":
│    1: resource "github_branch_protection_v3" "managed" {
│
╵

Expected Behavior

  • able to update branch protection

Actual Behavior

  • seeing above errors

Steps to Reproduce

Please list the steps required to reproduce the issue, for example:

  1. terraform apply

Important Factoids

  • we use atlantis but the same error happens locally as well

balusarakesh avatar May 12 '22 16:05 balusarakesh

I was facing the same issue a few moments ago. Try adding context. required_status_checks { strict = true contexts = [ "acontext" ] }

mnsanfilippo-caylent avatar May 16 '22 16:05 mnsanfilippo-caylent

@balusarakesh if you don't need to enable checks, then you can try to provide null instead of an empty list.

resource "github_branch_protection_v3" "example" {
  repository     = github_repository.example.name
  branch         = "main"
  enforce_admins = true

  required_status_checks {
    strict   = true
    contexts = null
  }

}

stanislavsapov avatar May 17 '22 13:05 stanislavsapov

@balusarakesh if you don't need to enable checks, then you can try to provide null instead of an empty list.

resource "github_branch_protection_v3" "example" {
  repository     = github_repository.example.name
  branch         = "main"
  enforce_admins = true

  required_status_checks {
    strict   = true
    contexts = null
  }

}

I'm not sure if this works, here's the full (non-working) example:

resource "github_repository" "example" {
  name        = "terraform-test-repo"
  description = "Terraform acceptance tests"

  auto_init = true
}

resource "github_branch_protection_v3" "example" {
  repository = github_repository.example.name
  # branch         = "main"
  branch         = github_repository.example.default_branch
  enforce_admins = true

  required_status_checks {
    strict   = true
    contexts = null
  }

}

I'm also a bit puzzled what broke here (why did this surface now; did GitHub update their API on this or what). The best option for us to get this sorted seems to just create the required_status_checks block with dynamic now - but only when the "contexts" have been configured - as otherwise - for reason X - GitHub gives the error.

But, we should be able to set the "strict" without the "contexts" - so - it seems to be a bug...

iniinikoski avatar May 17 '22 15:05 iniinikoski

FYI: it's not the contexts variable, it's strict variable which is having issues as you ca see from the logs

│ For 'anyOf/1', {"strict"=>true} is not a null. []

balusarakesh avatar May 17 '22 18:05 balusarakesh

I'm also experiencing this issue at my organisation, out of no-where much like @iniinikoski.

Has anyone had much success at a workaround or fix?

Unsure if this is related, but in the GitHub API docs under the "Body Parameters" section, it states that the contexts property is both REQUIRED and DEPRECATED (which seems contradictory) and has been replaced by checks. Is this a recent change?

TheJackson avatar May 25 '22 22:05 TheJackson

Having the same issue over here. Apparently it worked fine 21 days ago from our terraform/CI logs...

geancarlo avatar Jun 10 '22 21:06 geancarlo

I'm also seeing this issue. My data point that I can contribute is that updated my pinned version for the provider from version ~> 4.19.2 to ~> 4.26.1. I'm not sure if that is directly related or a red herring.

highb avatar Jun 15 '22 20:06 highb

We downgraded the provider to 4.22.0 and it appears to have resolved the issue for us. Hopefully that helps you git bisect it. :)

highb avatar Jun 17 '22 21:06 highb

This seems to be a problem with github.com/google/go-github/v47

luisdavim avatar Sep 17 '22 11:09 luisdavim

This seems to be a problem with github.com/google/go-github/v47

Actually, it seems like github.com/google/go-github/v47 is used in other parts of this provider, but the branch protection uses github.com/shurcooL/githubv4, I've opened a PR to fix the issue on github.com/google/go-github/v47 (https://github.com/google/go-github/pull/2468) the fix on the other lib is probably similar, removing omitempty from https://github.com/shurcooL/githubv4/blob/master/input.go#L604

update: actually, there are 2 versions of the resource, one uses github.com/google/go-github/v47 (https://github.com/integrations/terraform-provider-github/blob/main/github/resource_github_branch_protection_v3.go) the other uses github.com/shurcooL/githubv4 (https://github.com/integrations/terraform-provider-github/blob/main/github/resource_github_branch_protection.go)

luisdavim avatar Sep 18 '22 15:09 luisdavim

@luisdavim which version can we use meanwhile this gets resolved? I'm using github_branch_protection_v3 with provider version ~> 4.0

gfoligna-nyshex avatar Oct 03 '22 17:10 gfoligna-nyshex

I think 4.22.0 should work.

luisdavim avatar Oct 03 '22 22:10 luisdavim

Can confirm, we're using 4.22.0 and the branch protection error doesn't appear.

grrywlsn avatar Oct 04 '22 09:10 grrywlsn

Downgrading to a lower version doesn't seem like a good long term solution. Are there any plans to fix this in later versions?

cfisher281 avatar Oct 20 '22 21:10 cfisher281

This should already be fixed in github_branch_protection_v3 since this PR was merged in and released.

@luisdavim for github_branch_protection, the githubv4 schema has been updated a couple of times recently. Do you mind checking if the relevant omitempty still exists?

kfcampbell avatar Oct 27 '22 21:10 kfcampbell

This is not fixed. Passing null for contexts still throws the same error as passing an empty list. This was working in~> 5.0 previously but broke again in v5.7.0.

EDIT: when passing [] as the value for contexts, the following error is returned by the API:

<REDACTED URL>/branches/master/protection: 422 Invalid request.

No subschema in "anyOf" matched.
For 'properties/checks', nil is not an array.
Not all subschemas of "allOf" matched.
For 'anyOf/1', {"strict"=>true, "checks"=>nil} is not a null. []

Seems like the provider is perhaps converting the empty list to nil? I tried as well by explicitly setting contexts = null and the same error is returned.:

No subschema in "anyOf" matched.
For 'properties/checks', nil is not an array.
Not all subschemas of "allOf" matched.
For 'anyOf/1', {"strict"=>true, "checks"=>nil} is not a null. []

This is on v5.7.0 of the provider using the "github_branch_protection_v3" resource.

c0mput3rj0n3s avatar Nov 04 '22 00:11 c0mput3rj0n3s

Looks like this is related to #1307, which was caused by an upstream change in google/go-github attempting to fix this issue in the provider.

Perhaps an upstream revert is our best bet?

kfcampbell avatar Nov 10 '22 19:11 kfcampbell

We have resolved this issue on a few dozen repositories by switching from github_branch_protection_v3 to github_branch_protection (v4) in https://github.com/mineiros-io/terraform-github-repository. No changes except renaming branch to pattern were necessary to get all our branch protections working again via Terraform.

hollow avatar Nov 14 '22 05:11 hollow

I'm facing the same issue using v5.2.0 and digging it I found out an important point that I haven't seen mentioned in this ticket: the etag stored in the terraform state of this resource is wrongly updated despite the failure. As a consequence, the next terraform apply/plan will not show any changes but the enforce_admin is still not properly set on the branch protection rule.

Trace logging the terraform run, I can see an initial GET on the branch protection rule returning 200 OK, an Etag header and the json state showing "enforce_admins": {"url": "...", "enabled": false}. Then the PUT request passes "enforce_admins": true and the buggy "required_status_checks": {"strict": true} and so receives back a 422 Unprocessable Entity. After that the state is saved with both "enforce_admins": true and the etag received at the time of the GET!

Any following terraform plan/apply will then receive an answer to the GET on the branch protection rule that says 304 Not Modified and so terraform think that nothing needs to be done.

So, whether github.com/google/go-github/v47 is to blame or not for the buggy required_status_check that is passed along, the github provider must be blamed for updating the terraform state with the wrong etag and a lying enforce_admins.

odormond avatar Nov 15 '22 15:11 odormond

We have resolved this issue on a few dozen repositories by switching from github_branch_protection_v3 to github_branch_protection (v4) in https://github.com/mineiros-io/terraform-github-repository. No changes except renaming branch to pattern were necessary to get all our branch protections working again via Terraform.

This is a good workaround, though it's worth noting that the V4 (GraphQL) API does not support fine-grained tokens at the moment, which is an issue for those that have enabled the recent "Disallow classic PAT's" toggle: https://github.blog/2022-10-18-introducing-fine-grained-personal-access-tokens-for-github/

TheQueenIsDead avatar Dec 06 '22 22:12 TheQueenIsDead

This seems to be a dupe of #1307, which a fix is in-progress for. I'm going to close this out and we should continue further discourse there.

kfcampbell avatar Jan 05 '23 23:01 kfcampbell

still get the same issue in v6.2.1

│ No subschema in "oneOf" matched.
│ Not all subschemas of "allOf" matched.
│ For 'anyOf/1', {"strict"=>true} is not a null. []```

ljw4010 avatar May 14 '24 11:05 ljw4010