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

Cannot create github_branch_protection_v3 after update to v5.3.0 #1307

Closed
jtsaito opened this issue Sep 28, 2022 · 8 comments
Closed

Cannot create github_branch_protection_v3 after update to v5.3.0 #1307

jtsaito opened this issue Sep 28, 2022 · 8 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

@jtsaito
Copy link
Contributor

jtsaito commented Sep 28, 2022

Terraform Version

v5.3.0

Affected Resource(s)

  • github_branch_protection_v3
  • github_branch_default
  • github_branch_default

The issue occures when creating a new github_branch_protection_v3 resource with attributes referenced from github_branch_default and github_branch_protection_v3 resources.

Debug Output

github_branch_protection_v3.<MY_REPO_NAME_REDACTED>: Creating...
╷
│ Error: PUT https://api.github.com/repos/<MY_ORG_REDACTED>/<MY_REPO_NAME_REDACTED>/branches/main/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, "contexts"=>["build"], "checks"=>nil} is not a null. []

Expected Behavior

The branch protection v3 resource should be created.

Actual Behavior

Terraform errors out with message above.

Steps to Reproduce

Step 1

Create a repo with terraform apply.

resource "github_repository" "test-repo" {
  name        = "test-repo"
  description = "This repo is a test"

  has_downloads = true
  has_issues    = false
  has_projects  = false
  has_wiki      = false

  visibility = "private"

  auto_init = true

  vulnerability_alerts = false
}

resource "github_branch_default" "test-repo" {
  repository = github_repository.test-repo.name
  branch     = "main"
}

Step 2

Terraform apply the following change:

resource "github_branch_protection_v3" "test-repo" {
  repository     = github_repository.test-repo.name
  branch         = github_branch_default.test-repo.branch
  enforce_admins = true

  required_status_checks {
    strict   = true
    contexts = ["build"]
  }

  required_pull_request_reviews {
    dismiss_stale_reviews      = true
    require_code_owner_reviews = true
  }
}

Important Factoids

This works up to GitHub provider version v5.2.0 without errors and errors in v5.3.0.

This seems to be linked to the go client update in the release diff where the API Client is updated as follows.

Checks []*RequiredStatusCheck `json:"checks"`

From

Checks []*RequiredStatusCheck `json:"checks,omitempty"

To

Checks []*RequiredStatusCheck `json:"checks"`

References

@iniinikoski
Copy link

iniinikoski commented Sep 29, 2022

Can confirm this. Thanks for reporting @jtsaito !

Update: and we've also locked ourselves to v5.2.0 and this one works supersmooth...

@jtsaito
Copy link
Contributor Author

jtsaito commented Sep 29, 2022

@iniinikoski Thanks for looking into it and confirming the bug.

@soerenmartius
Copy link

same here

@kfcampbell
Copy link
Member

Oh no! What a hassle. It looks like that was introduced to google/go-github in this PR, based off of this issue. Interestingly enough, that issue references #1147.

Looking at our relevant code here and here, I'm not sure what we could do to fix this on our side...perhaps the appropriate course of action is an upstream revert?

Perhaps experimenting with setting something other than []interface{}{} when protection.GetRequiredStatusChecks is nil could prove fruitful as well.

Is anybody interested in exploring this further in a PR?

@TheQueenIsDead
Copy link
Contributor

TheQueenIsDead commented Dec 4, 2022

@kfcampbell I've opened a draft PR with a potential solution here:
#1409

It simply adds in an empty for the missing Checks attribute, which should resolve the issue as proposed by the issue reporter in google/go-github#2467

If this seems like a viable path forwards, I'll look at building the module and testing it when I get the chance :-) (Untested currently hence the draft)

@kfcampbell
Copy link
Member

I responded on the PR, which looks good! I can't wait to resolve this issue.

@jtsaito
Copy link
Contributor Author

jtsaito commented Jan 23, 2023

@TheQueenIsDead and everyone else who contributed. The issue was fixed by release 5.16.0 which included #1415.

@jtsaito jtsaito closed this as completed Jan 23, 2023
@ekhaydarov
Copy link

ekhaydarov commented Jan 30, 2023

EDIT: was not using _v3. the pr was fine

im not so sure that this works? I am on 5.16.0 and hit the nil issue, adding checks just gave me this

Error: Unsupported argument 
...
An argument named "checks" is not expected here.

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
7 participants