Error when creating branch protection

[luisdavim]

Since we updated our code to use the latest version of this library, we can't create a branch protection that requires branches to be up to date before merging but don't require any status checks:

func (r *RepoSettings) protectionRequestOptions() *github.ProtectionRequest {
	return &github.ProtectionRequest{
		EnforceAdmins: r.EnforceAdmins,
		RequiredPullRequestReviews: &github.PullRequestReviewsEnforcementRequest{
			DismissStaleReviews:          r.DismissStaleReviews,
			RequiredApprovingReviewCount: r.RequiredApprovingReviewCount,
			RequireCodeOwnerReviews:      r.RequireCodeOwnerReviews,
		},
		RequiredStatusChecks: &github.RequiredStatusChecks{
			Strict: true,
		},
	}
}

The above results in an error:

PUT https://api.github.com/repos/****/****/branches/main/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. []

Such rule is possible to be created from the UI:

This is also affecting the Github Terraform provider: integrations/terraform-provider-github#1147

[luisdavim]

Looking at https://github.com/orgs/community/discussions/24642 removing omitempty from the Checks property could solve the problem but I guess that for that you'll have to fully deprecate the Contexts or maybe you could force Checks to be null if len(Contexts) > 0 && len(Contexts) == 0

[luisdavim]

I've tested this locally, removing omitempty from the Checks property in https://github.com/google/go-github/blob/master/github/repos.go#L908 and passing:

github.ProtectionRequest{
	EnforceAdmins: r.EnforceAdmins,
	RequiredPullRequestReviews: &github.PullRequestReviewsEnforcementRequest{
		DismissStaleReviews:          r.DismissStaleReviews,
		RequiredApprovingReviewCount: r.RequiredApprovingReviewCount,
		RequireCodeOwnerReviews:      r.RequireCodeOwnerReviews,
	},
	RequiredStatusChecks: &github.RequiredStatusChecks{
		Strict: true,
		Checks: []*github.RequiredStatusCheck{},
	},
}

Works, and you might not need to fully deprecate Contexts as if Checks is not set it's passed as null and that should be ok if Contexts is set...

I've opened #2468 to remove the omitempty.

[luisdavim]

Thank you very much for the quick turnaround on this @gmlewis 🚀
Any chances of getting a new tag released so we can get the fix downstream?

[gmlewis]

Thank you very much for the quick turnaround on this @gmlewis rocket Any chances of getting a new tag released so we can get the fix downstream?

Yes, I can work on a new release today.

[luisdavim]

Awesome, thanks. Really appreciate it 👍

[gmlewis]

@luisdavim - this issue has now been covered in the latest release:
https://github.com/google/go-github/releases/tag/v47.1.0

[alock]

When attempting to update I ran into the same issue as integrations/terraform-provider-github#1307. It appears that golang is setting the value to nil and not null as the github api requires. Is there a simple way to override that before the value is sent?

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

[gmlewis]

@luisdavim - do you have any insights to this new issue?

@alock - just for the record, this repo is now sending an empty [] which that error message is reporting as nil, as evidenced by our unit test here:
23a2636#diff-1da0ffca0653d8b270e435e5e5ef6615b547c752c60b45df54c7e4307ece89e9R1589

[luisdavim]

I think the terraform provider still needs to be updated to use a version of this lib that contains the fix....

[gmlewis]

I think the terraform provider still needs to be updated to use a version of this lib that contains the fix....

Ah, thank you, @luisdavim !

Then I'll close this issue to hopefully reduce the confusion... but please report back if you feel we introduced a regression.

[TheQueenIsDead]

I think the terraform provider still needs to be updated to use a version of this lib that contains the fix....

It looks like the Terraform provider updated to go-github v48.1.0 17 days ago, and published those changes in their v5.11.0 release

However I still receive the same issue attempting to apply:

╷
│ Error: PUT https://api.github.com/repos/<org>/status/branches/<b>/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"=>false, "checks"=>nil} is not a null. []
│ 
│   with module.repository["<service>"].github_branch_protection_v3.branch_protection[1],
│   on .terraform/modules/repository/main.tf line 240, in resource "github_branch_protection_v3" "branch_protection":
│  240: resource "github_branch_protection_v3" "branch_protection" {
│ 
╵
╷
│ Error: PUT https://api.github.com/repos/<org>/status/branches/<a>/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"=>false, "contexts"=>["go / Build"], "checks"=>nil} is not a null. []
│ 
│   with module.repository["<service>"].github_branch_protection_v3.branch_protection[0],
│   on .terraform/modules/repository/main.tf line 240, in resource "github_branch_protection_v3" "branch_protection":
│  240: resource "github_branch_protection_v3" "branch_protection" {
│ 
╵

Versions as such, is there something I'm missing here?

$ terraform version
Terraform v1.3.5
on linux_amd64
+ provider registry.terraform.io/integrations/github v5.11.0

Any help appreciated 🙏

[gmlewis]

Reopening but need assistance on this one from Terraform users.

[luisdavim]

The fix will have to be on the terraform provider side, to pass an empty array instead of nothing, the alternative could be to add logic to this library to set an empty array when it's nil....

[TheQueenIsDead]

I've tested this locally, removing omitempty from the Checks property in https://github.com/google/go-github/blob/master/github/repos.go#L908 and passing:

github.ProtectionRequest{
	EnforceAdmins: r.EnforceAdmins,
	RequiredPullRequestReviews: &github.PullRequestReviewsEnforcementRequest{
		DismissStaleReviews:          r.DismissStaleReviews,
		RequiredApprovingReviewCount: r.RequiredApprovingReviewCount,
		RequireCodeOwnerReviews:      r.RequireCodeOwnerReviews,
	},
	RequiredStatusChecks: &github.RequiredStatusChecks{
		Strict: true,
		Checks: []*github.RequiredStatusCheck{},
	},
}

Works, and you might not need to fully deprecate Contexts as if Checks is not set it's passed as null and that should be ok if Contexts is set...

I've opened #2468 to remove the omitempty.

Oh I see, per your comment above. Apologies for missing that. I'll open a PR downstream to try adding an empty array to the request in their utility file 👍
https://github.com/integrations/terraform-provider-github/blob/main/github/resource_github_branch_protection_v3_utils.go#L178

The idea to handle the nil gracefully upstream is a nice one, though you'd know best if there might be any interesting ramifications of that. Thanks for the response!

[luisdavim]

Do keep on mind that if contexts is set then checks should be null. Using contexts is deprecated and terraform shouldn't be using it but I don't know if it is.

[TheQueenIsDead]

Duly noted. The Terraform provider has not yet been switched over to use checks instead of contexts, per this utility function that builds the branch protection objects:
https://github.com/integrations/terraform-provider-github/blob/main/github/resource_github_branch_protection_v3_utils.go#L41

It seems there is a downstream issue to track this upgrade:
integrations/terraform-provider-github#985
With a WIP PR here:
integrations/terraform-provider-github#1051

In the meantime I'll see about merging the PR to include an empty Checks array to resolve the immediate issue, though it might require a little more through than I initially realised.

I'm happy to close this issue knowing that there is a downstream ticket to track deprecating Contexts. I suppose the only other reason to keep this open would be if we wanted the client to handle nil values as empty arrays per your suggestion.

I'll leave that decision to you 👍

[gmlewis]

OK, then I think we are good to go and I'll close this issue (again).
Hopefully we can leave it closed this time.

[emmahsax]

I'm trying to run v57.0.0 of this, and I'm still getting this issue:

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

This is what our branch protection looks like:

defaultBranchProtection := &github.ProtectionRequest{
    RequiredStatusChecks: &github.RequiredStatusChecks{
	Checks: []*github.RequiredStatusCheck{},
	Strict: true,

    },
}

[gmlewis]

Reopening.

[emmahsax]

I did find that v57.0.0 has the omitempty on Checks, while v56.0.0 does not.

[KatharinaSick]

fyi: I'm getting the same issue. I tried to work around it but couldn't find a way.

[gmlewis]

This issue is apparently in direct conflict with #2976.

We would appreciate it if one of our amazing contributors would please volunteer to take a deep dive here and see why #2467 and #2976 are at opposition with one another and see if they can come up with a fix that solves both issues.

[KatharinaSick]

Thank you! :)
Is there anything I can do to work around this in the meantime?

[gmlewis]

Thank you! :) Is there anything I can do to work around this in the meantime?

I would recommend using the version that works for you for your particular use case, since we appear to have different use cases apparently.

This difference in use cases will be part of what needs to be determined with a deep dive into fixing this issue.

[gmlewis]

Please check out this comment: #2976 (comment)

[emmahsax]

@KatharinaSick I've come with two potential workarounds that could work for your situation. If neither of them work, maybe I can try to help you find a solution. The workarounds are written here: #2976 (comment)

[gmlewis]

Fixed by: #3070.