go-github: Error when creating branch protection

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: https://github.com/integrations/terraform-provider-github/issues/1147

About this issue

Original URL
State: closed
Created 2 years ago
Reactions: 2
Comments: 17 (7 by maintainers)

Commits related to this issue

Always send 'checks' field when creating branch protection (#2468) Fixes: #2467 — committed to google/go-github by luisdavim 2 years ago

Most upvoted comments

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

gmlewis on Sep 19, 2022

Awesome, thanks. Really appreciate it 👍

luisdavim on Sep 19, 2022

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?

luisdavim on Sep 19, 2022

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 https://github.com/google/go-github/pull/2468 to remove the omitempty.

luisdavim on Sep 17, 2022

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.

luisdavim on Dec 5, 2022

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!

TheQueenIsDead on Dec 4, 2022

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.

gmlewis on Dec 2, 2022

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

luisdavim on Dec 2, 2022

When attempting to update I ran into the same issue as https://github.com/integrations/terraform-provider-github/issues/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. []

alock on Dec 1, 2022

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.

gmlewis on Sep 19, 2022