integrations / terraform-provider-github

Terraform GitHub provider
https://www.terraform.io/docs/providers/github/
MIT License
888 stars 731 forks source link

unable to update branch_protections for github repos #1147

Open balusarakesh opened 2 years ago

balusarakesh commented 2 years ago

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

Affected Resource(s)

Please list the resources as a list, for example:

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

Actual Behavior

Steps to Reproduce

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

  1. terraform apply

Important Factoids

mnsanfilippo-caylent commented 2 years ago

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

stanislavsapov commented 2 years ago

@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
  }

}
iniinikoski commented 2 years ago

@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...

balusarakesh commented 2 years ago

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. []
TheJackson commented 2 years ago

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?

geancarlo commented 2 years ago

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

highb commented 2 years ago

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 commented 2 years ago

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. :)

luisdavim commented 2 years ago

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

luisdavim commented 2 years ago

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)

gfoligna-nyshex commented 1 year ago

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

luisdavim commented 1 year ago

I think 4.22.0 should work.

grrywlsn commented 1 year ago

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

cfisher281 commented 1 year ago

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

kfcampbell commented 1 year ago

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?

c0mput3rj0n3s commented 1 year ago

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.

kfcampbell commented 1 year ago

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?

hollow commented 1 year ago

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.

odormond commented 1 year ago

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.

TheQueenIsDead commented 1 year ago

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/

kfcampbell commented 1 year ago

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.

ljw4010 commented 4 months ago

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. []```