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

Fix 5.43 upgrade signoff #2222

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

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

@kfcampbell
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants