fix: check if organization and requires sign off commits

[steveteuber]

This is a follow-up pull request to fix the issue described in #2077.

---

Before the change?

There is a bug in the GitHub API 2022-11-28 version that causes a 422 error when updating a repository within an organization that has "Require sign off on web-based commits" enabled.

After the change?

On a repository update, we check if the organization has "Require sign off on web-based commits" enabled.
If it's enabled, we do not send the web_commit_signoff_required field in the update request to avoid the 422 error.

Pull request checklist

• Schema migrations have been created if needed (example)
• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

• Yes
• No

---

[ArlonAntonius]

Hi @steveteuber,

I was reading into this issue and was wondering the following:
At this moment only changes to the update method have been provided, does that mean that this issue does not seem to happen on the creation of a GitHub repository?

[steveteuber]

@ArlonAntonius Yes, correct. After #2763 was merged, this issue now occurs only when updating a repository.

[nickfloyd]

Let me know what you think about my comment and thank you for this change ❤️

[steveteuber]

@nickfloyd Did you read my comment in the discussion above? It seems like you might have missed it.

[IaroslavSapak]

currently I have to play around lifecycle{ ignore_changes = all} or reimporting updated repo. So this PR would greatly imrove workflow. So it would be great go with proposed patch instead of waiting v7

[deiga]

@steveteuber
We've been discussing internally, that modifying the Owner struct isn't something we'd like to do. And we want to move the provider to be more explicit instead of implicit, the proposed solution doesn't look like one we want to adopt.

And for the changes to github/resource_github_repository.go I'm having trouble understading how it changes any behaviour. There seem to be a few more cases where we send the field, but that doesn't sound like it would fix the issue you're reporting.

Could you add a testcase which fails with the current code and works with your code?

Edit: Once you have a testcase, could you change the logic to use d.GetOkExists so that the field is unset, if it's unset in the TF config and otherwise the value of the TF config?

[steveteuber]

@deiga Thanks for the feedback. Regarding the changes to github/resource_github_repository.go, the main change is to check if the owner is an organization and has "Require sign off on web-based commits" enabled before sending the web_commit_signoff_required field in the update request.

To clarify the behavior, here are some scenarios:

1. Owner is an individual user account:
   The web_commit_signoff_required field can be sent without any issues.

2. Owner is an organization:
   The web_commit_signoff_required field can be sent without any issues.

3. Owner is an organization and has "Require sign off on web-based commits" enabled:
   The web_commit_signoff_required field cannot be sent, otherwise it will cause a 422 error. To avoid this, check if the owner is an organization and has "Require sign off on web-based commits" enabled. When this is the case, do not send the web_commit_signoff_required field in the update request.

[steveteuber]

@deiga I've created a new PR #3165 for an alternative approach that only sets the web_commit_signoff_required field if it's explicitly configured in the repository configuration.

I'll close this PR here because the alternative approach doesn't modifying the Owner struct, and it's more in line with making the provider more explicit.