Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unable to update branch_protections for github repos #1147

Closed
balusarakesh opened this issue May 12, 2022 · 21 comments
Closed

unable to update branch_protections for github repos #1147

balusarakesh opened this issue May 12, 2022 · 21 comments
Labels
Status: Up for grabs Issues that are ready to be worked on by anyone Type: Bug Something isn't working as documented

Comments

@balusarakesh
Copy link

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
@mnsanfilippo-caylent
Copy link

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

@stanislavsapov
Copy link

@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
Copy link

iniinikoski commented May 17, 2022

@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
Copy link
Author

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
Copy link

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
Copy link

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

@highb
Copy link

highb commented Jun 15, 2022

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
Copy link

highb commented Jun 17, 2022

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
Copy link

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

@luisdavim
Copy link

luisdavim commented Sep 18, 2022

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 (google/go-github#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
Copy link

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

@luisdavim
Copy link

I think 4.22.0 should work.

@grrywlsn
Copy link

grrywlsn commented Oct 4, 2022

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

@cfisher281
Copy link

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
Copy link
Member

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?

@nickfloyd nickfloyd added the Status: Triage This is being looked at and prioritized label Oct 31, 2022
@c0mput3rj0n3s
Copy link

c0mput3rj0n3s commented Nov 4, 2022

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.

@nickfloyd nickfloyd added Type: Bug Something isn't working as documented Status: Triage This is being looked at and prioritized and removed Status: Needs info Full requirements are not yet known, so implementation should not be started labels Nov 4, 2022
@kfcampbell
Copy link
Member

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
Copy link

hollow commented Nov 14, 2022

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
Copy link

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.

@kfcampbell kfcampbell added Status: Up for grabs Issues that are ready to be worked on by anyone and removed Status: Triage This is being looked at and prioritized labels Nov 28, 2022
@TheQueenIsDead
Copy link
Contributor

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
Copy link
Member

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 kfcampbell closed this as not planned Won't fix, can't repro, duplicate, stale Jan 5, 2023
charlesmmarshall added a commit to ministryofjustice/opg-terraform-github-repository that referenced this issue Feb 16, 2023
…s workaround before changing the provider version
charlesmmarshall added a commit to ministryofjustice/opg-terraform-github-repository that referenced this issue Feb 16, 2023
* known issue (integrations/terraform-provider-github#1147), trying this workaround before changing the provider version
* change to v4 graphql branch protection
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Up for grabs Issues that are ready to be worked on by anyone Type: Bug Something isn't working as documented
Projects
None yet
Development

No branches or pull requests