Skip to content

Conversation

@bzarboni1
Copy link

@bzarboni1 bzarboni1 commented Apr 8, 2024

Resolves #I2077


Before the change?

  • For a repository update, there's a bug in the GitHub 2022-11-28 version, that throws a 422 error whenever the web_commit_signoff_required is set to true, even when it is already true.

After the change?

  • On a repository update, we check if the web_commit_signoff_required has been modified. If it has, it is passed along in the request. If it hasn't, it is silently dropped from the request.

Pull request checklist

  • 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!

  • No

@TylerMizuyabu TylerMizuyabu force-pushed the fix-5.43-upgrade-signoff branch from 7c379b7 to b8f15a9 Compare April 9, 2024 13:59
TylerMizuyabu and others added 2 commits April 9, 2024 10:50
* fix: 5.43 upgrade signoff

---------
Co-authored-by: Ben Zarboni <[email protected]>
@kfcampbell
Copy link
Contributor

Thanks for the contribution! The newly-added test is failing for me:

--- FAIL: TestAccGithubRepositoryWebCommitSignoffRequired (11.15s)
    --- FAIL: TestAccGithubRepositoryWebCommitSignoffRequired/changes_the_web_commit_signoff_required_attribute_for_a_repository (5.67s)
        --- SKIP: TestAccGithubRepositoryWebCommitSignoffRequired/changes_the_web_commit_signoff_required_attribute_for_a_repository/with_an_anonymous_account (0.00s)
        --- FAIL: TestAccGithubRepositoryWebCommitSignoffRequired/changes_the_web_commit_signoff_required_attribute_for_a_repository/with_an_individual_account (5.67s)
        --- SKIP: TestAccGithubRepositoryWebCommitSignoffRequired/changes_the_web_commit_signoff_required_attribute_for_a_repository/with_an_organization_account (0.00s)
    --- FAIL: TestAccGithubRepositoryWebCommitSignoffRequired/changes_a_non_web_commit_signoff_required_attribute_for_a_repository (5.49s)
        --- SKIP: TestAccGithubRepositoryWebCommitSignoffRequired/changes_a_non_web_commit_signoff_required_attribute_for_a_repository/with_an_anonymous_account (0.00s)
        --- FAIL: TestAccGithubRepositoryWebCommitSignoffRequired/changes_a_non_web_commit_signoff_required_attribute_for_a_repository/with_an_individual_account (5.49s)
        --- SKIP: TestAccGithubRepositoryWebCommitSignoffRequired/changes_a_non_web_commit_signoff_required_attribute_for_a_repository/with_an_organization_account (0.00s)
FAIL

with the following error:

    resource_github_repository_test.go:1648: Step 1/2 error: After applying this test step, the plan was not empty.
        stdout:
        
        
        Terraform used the selected providers to generate the following execution
        plan. Resource actions are indicated with the following symbols:
          ~ update in-place
        
        Terraform will perform the following actions:
        
          # github_repository.test will be updated in-place
          ~ resource "github_repository" "test" {
                id                          = "tf-acc-4apm1"
                name                        = "tf-acc-4apm1"
              - vulnerability_alerts        = true -> null
                # (32 unchanged attributes hidden)
        
                # (1 unchanged block hidden)
            }
        
        Plan: 0 to add, 1 to change, 0 to destroy.

Is this something you can reproduce?

@bzarboni1
Copy link
Author

Thanks for the contribution! The newly-added test is failing for me:

--- FAIL: TestAccGithubRepositoryWebCommitSignoffRequired (11.15s)
    --- FAIL: TestAccGithubRepositoryWebCommitSignoffRequired/changes_the_web_commit_signoff_required_attribute_for_a_repository (5.67s)
        --- SKIP: TestAccGithubRepositoryWebCommitSignoffRequired/changes_the_web_commit_signoff_required_attribute_for_a_repository/with_an_anonymous_account (0.00s)
        --- FAIL: TestAccGithubRepositoryWebCommitSignoffRequired/changes_the_web_commit_signoff_required_attribute_for_a_repository/with_an_individual_account (5.67s)
        --- SKIP: TestAccGithubRepositoryWebCommitSignoffRequired/changes_the_web_commit_signoff_required_attribute_for_a_repository/with_an_organization_account (0.00s)
    --- FAIL: TestAccGithubRepositoryWebCommitSignoffRequired/changes_a_non_web_commit_signoff_required_attribute_for_a_repository (5.49s)
        --- SKIP: TestAccGithubRepositoryWebCommitSignoffRequired/changes_a_non_web_commit_signoff_required_attribute_for_a_repository/with_an_anonymous_account (0.00s)
        --- FAIL: TestAccGithubRepositoryWebCommitSignoffRequired/changes_a_non_web_commit_signoff_required_attribute_for_a_repository/with_an_individual_account (5.49s)
        --- SKIP: TestAccGithubRepositoryWebCommitSignoffRequired/changes_a_non_web_commit_signoff_required_attribute_for_a_repository/with_an_organization_account (0.00s)
FAIL

with the following error:

    resource_github_repository_test.go:1648: Step 1/2 error: After applying this test step, the plan was not empty.
        stdout:
        
        
        Terraform used the selected providers to generate the following execution
        plan. Resource actions are indicated with the following symbols:
          ~ update in-place
        
        Terraform will perform the following actions:
        
          # github_repository.test will be updated in-place
          ~ resource "github_repository" "test" {
                id                          = "tf-acc-4apm1"
                name                        = "tf-acc-4apm1"
              - vulnerability_alerts        = true -> null
                # (32 unchanged attributes hidden)
        
                # (1 unchanged block hidden)
            }
        
        Plan: 0 to add, 1 to change, 0 to destroy.

Is this something you can reproduce?

Apologies for the late reply.
I've tried it here on my end, and had a colleague try as well. The test are successful for us.

@bzarboni1
Copy link
Author

Bump

@jkroepke
Copy link

@kfcampbell do you think is ready to merge?

@github-actions
Copy link

👋 Hey Friends, this pull request has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!

@github-actions github-actions bot added the Status: Stale Used by stalebot to clean house label Aug 25, 2025
@github-actions github-actions bot closed this Sep 1, 2025
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in 🧰 Octokit Active Sep 1, 2025
@steveteuber
Copy link

Hey @kfcampbell, is there any chance to reopen this pull request? I can confirm that the issue #2077 still exists...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Stale Used by stalebot to clean house

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants