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

[BUG] github_branch_protection_v3 tries to delete the contexts every time #1701

Open
julioagos opened this issue May 25, 2023 · 18 comments · May be fixed by #2232
Open

[BUG] github_branch_protection_v3 tries to delete the contexts every time #1701

julioagos opened this issue May 25, 2023 · 18 comments · May be fixed by #2232
Labels
hacktoberfest Issues for participation in Hacktoberfest Status: Pinned A way to keep old or long lived issues around Status: Up for grabs Issues that are ready to be worked on by anyone Type: Bug Something isn't working as documented vNext

Comments

@julioagos
Copy link

Problem

I created some branch protection rules for some branches. These rules included required checks. After I applied them, it created matching contexts and a new check that is just one of the required checks with an app_id that always changes (First 4 digits remain the same, the rest changes). Now, every time I ran plan, it says that it is going to delete them again. The branch protections are working just as intended but, running plan always generates meaningless noise that get's in the way especially since we have a lot of branch protection rules once we fully migrate it over to Terraform. The applies are all run by Atlantis on PRs. This is what an example output of what terraform state show looks like for one of the branches:

    required_status_checks {
        checks         = [
            "test 1",
            "test 2",
            "test 1:<random-numbers>",
        ]
        contexts       = [
            "test 1",
            "test 2",
        ]
    }

Running terraform plan returns something like this

      ~ required_status_checks {
          ~ checks         = [
              - "test 1:<random-numbers>",
                # (2 unchanged elements hidden)
            ]
          ~ contexts       = [
              - "test 1",
              - "test 2",
            ]
            # (2 unchanged attributes hidden)
        }

There are no error messages at any point and the apply works as intended. It states that all plans have been applied.

Versions

Terraform: v1.4.4
GitHub Provider: Originally 5.23.0 but also happens in 5.25.1

@nickfloyd nickfloyd moved this from 🆕 Triage to 🔥 Backlog in 🧰 Octokit Active May 26, 2023
@kfcampbell kfcampbell added Type: Bug Something isn't working as documented Status: Up for grabs Issues that are ready to be worked on by anyone Priority: Normal labels May 31, 2023
@svalevka
Copy link

svalevka commented Jun 2, 2023

We have a very similiar issue with 5.25.1 provider.
Using github app for authentication

  # module.github_repository.github_branch_protection_v3.branch["redacted_name.master"] will be updated in-place
  ~ resource "github_branch_protection_v3" "branch" {
        id                              = "redacted_name:master"
        # (6 unchanged attributes hidden)

      ~ required_status_checks {
          ~ checks         = [
              - "Naming conventions / Pull request:0",
                # (1 unchanged element hidden)
            ]
          ~ contexts       = [
              - "Naming conventions / Pull request",
            ]
            # (2 unchanged attributes hidden)
        }

@julioagos
Copy link
Author

Yea it is probably worth mentioning, I am also using github app for authentication

@0x46616c6b
Copy link
Contributor

I have the same problem. Since #1774 (v5.31.0) the checks diffs are gone but every plan produce a diff for contexts (regardless if they are set or not).

@stefanschulte
Copy link

For what it's worth, if you require a workable provider again now, you could also switch to the GraphQL based API endpoint for branch protections. The behaviour of that one wasn't changed, so there are no superfluous changes for terraform plans.

Which would be this resource: https://registry.terraform.io/providers/integrations/github/latest/docs/resources/branch_protection

@0x46616c6b
Copy link
Contributor

You are correct; the github_branch_protection works better with required_status_checks. However, you need to access apps when using pull_request_bypassers. Public apps should work, but if you reference an internal one, it will fail when using app authentication. App authentication for the provider does not make this possible; see #1248.

Sad times to use the provider :-(

@kfcampbell
Copy link
Member

@0x46616c6b if you have ideas about how we can handle this better, we're very accepting of new issues and PRs!

@0x46616c6b
Copy link
Contributor

Would it be possible to remove the deprecated field contexts from the branch protection v3? How the actual policy for removing a deprecated field/feature looks in the provider right now?

@kfcampbell
Copy link
Member

kfcampbell commented Jul 25, 2023

I think it's reasonable to do a major version release in the near future! It's been quite a while since we last released a breaking change (September 14th). There's a few other changes I wouldn't mind batching in there as well, such as #1029, #448, #1704, and maybe even #1780.

In general, Hashicorp recommends not doing more than one breaking change a year if we can help it.

@entropitor
Copy link

entropitor commented Aug 16, 2023

Is there any progress on this? This kinda makes github_branch_protection_v3 unusable.

@yaakov-h yaakov-h mentioned this issue Aug 16, 2023
5 tasks
@xiaoyanli-lyft
Copy link

@kfcampbell any timeline we have to officially remove contexts from the branch protection v3? this issue had been on for a while

@kfcampbell
Copy link
Member

Unfortunately GitHub's SDK team doesn't have the bandwidth to work on this ourselves right now, though PRs are appreciated!

@nickfloyd nickfloyd added the hacktoberfest Issues for participation in Hacktoberfest label Sep 20, 2023
@yorik
Copy link
Contributor

yorik commented Oct 30, 2023

We have several issues related to that:

  ~ resource "github_branch_protection_v3" "this" {
        id                              = "foo:master"
      ~ require_signed_commits          = false -> true
        # (5 unchanged attributes hidden)

      ~ required_status_checks {
          ~ checks         = [
              - "foo",
              + "foo:-1",
            ]
          ~ contexts       = [
              - "foo",
            ]
            # (2 unchanged attributes hidden)
        }

        # (2 unchanged blocks hidden)
    }

  ~ resource "github_branch_protection_v3" "this" {
        id                              = "bar:main"
        # (6 unchanged attributes hidden)

      ~ required_status_checks {
          ~ contexts       = [
              - "TruffleHog",
            ]
            # (3 unchanged attributes hidden)
        }

        # (2 unchanged blocks hidden)
    }

So there should be added special check for app_id == -1 and probably it's possible to hack that if checks do exist in terraform state just ignore contexts completely.

Would that work?

Copy link

👋 Hey Friends, this issue 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 Jul 27, 2024
@yorik
Copy link
Contributor

yorik commented Jul 29, 2024

This is still very annoying issue: it shows huge diff, while actual change could be small, so it's easy to miss something.

@kfcampbell kfcampbell added Status: Pinned A way to keep old or long lived issues around and removed Status: Stale Used by stalebot to clean house labels Jul 29, 2024
@rsi-mrobinson
Copy link

Also facing this, tried adding the contexts to my terraform as I was not using the field since it's been marked as deprecated. So instead of useless plan output showing changes that don't matter, now I have GitHub is deprecating the use of contexts. Use a checks array instead. yippee...

@tanjarinne
Copy link

Still happening in 6.2.0 and 6.3.x

We just had to replace github_branch_protection_v3 with github_branch_protection to get rid of this bug.

@rsi-mrobinson
Copy link

I love that #2232 potentially fixes this issue and it's been sitting open and unreviewed since APRIL.

@stevehipwell
Copy link
Contributor

I think this issue has been caused by the lack of having Computed: true set for contexts & checks. I'm working through the acceptance tests in #2476 and have fixed this by adding Computed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest Issues for participation in Hacktoberfest Status: Pinned A way to keep old or long lived issues around Status: Up for grabs Issues that are ready to be worked on by anyone Type: Bug Something isn't working as documented vNext
Projects
None yet