v3.1.0 github_branch_protection does not apply required_status_checks when set to false

[wilmer-mendez-globant]

Terraform Version

v0.13.3

Affected Resource(s)

Please list the resources as a list, for example:

• github_branch_protection

Terraform Configuration Files

locals {
  repo_settings    = jsondecode(file(var.filename))
  branches_created = toset(local.repo_settings.additional_branches)
  repo_perms       = merge(var.org_perms, local.repo_settings.teams)
}

resource "github_branch_protection" "set_rules" {
  for_each = local.repo_settings.branch_protection

  repository_id          = github_repository.create_repo.node_id
  pattern                = each.key
  enforce_admins         = each.value.enforce_admins
  require_signed_commits = each.value.require_signed_commits

  required_pull_request_reviews {
    required_approving_review_count = each.value.required_pull_request_reviews.required_approving_review_count
    dismiss_stale_reviews           = each.value.required_pull_request_reviews.dismiss_stale_reviews
    require_code_owner_reviews      = each.value.required_pull_request_reviews.require_code_owner_reviews
  }

  required_status_checks {
    strict   = "false"
  }
}

Expected Behavior

Terraform should have updated the statefile with the required_status_checks attribute set to false.

Actual Behavior

Terraform does not update the statefile so tf plan or tf apply show required_status_checks add on each run.

[onurg]

basically we have templates which is where read this as json and strict can be true or false - rules are applied based on that and status_context is passed in as well. Right now, it modifies it in each run unless there are contexts set ! Behavior started after upgrade from 2.9.0 to 3.1.0

[bharathkkb]

I am also seeing this. There is a permadiff for the required_status_checks block.

Terraform v0.13.4
+ provider registry.terraform.io/hashicorp/github v3.1.0

[jcudit]

Thanks for reporting and confirming this behaviour. The switch over from the REST implementation to the GraphQL implementation for this resource has been problematic on various fronts. A revert is being discussed, but it seems the feature of having a wildcard pattern is very much wanted by the community.

Will keep this issue in mind as we discuss next steps on making it less buggy.

[guillermotti]

Any news or actions to be taken on this? It's so annoying this issue. Also happens when we try to add an App as push_restrictions

module.ops-terraform-aws.github_branch_protection.some_branch_protection["master"] will be updated in-place
  ~ resource "github_branch_protection" "some_branch_protection" {
        enforce_admins         = true
        id                     = "MDIwOkJyYW5jaFByb3RlY3Rpb25SdWxlMTgzNDYzMjA="
        pattern                = "master"
      ~ push_restrictions      = [
          + "MDM6QXBwODI5NjQ=",
            "MDQ6VGVhbTMzNjEyMzU=",
        ]
        repository_id          = "ops-terraform-aws"
        require_signed_commits = false

        required_pull_request_reviews {
            dismiss_stale_reviews           = true
            dismissal_restrictions          = []
            require_code_owner_reviews      = true
            required_approving_review_count = 1
        }

        required_status_checks {
            contexts = [
                "atlantis/plan",
            ]
            strict   = true
        }
    }

Plan: 0 to add, 1 to change, 0 to destroy.

In this case for the v4.0.1 so I guess the error is being propagated throw versions.

I can solve the problem if I add contexts with strict. But if I don't add contexts, it fails with strict as well.

[patrickmarabeas]

The issue is the GraphQL API returns requiresStrictStatusChecks as true when there are no contexts. I'm tantalisingly close to a fix using a DiffSuppressFunc, but a couple edge cases are throwing a spanner in the works.

@guillermotti app support hasn't been added - that's easy enough.

[patrickmarabeas]

I've looked at this for too long to trust my judgment that it indeed works in all cases: https://github.com/terraform-providers/terraform-provider-github/pull/614

[patrickmarabeas]

Adding Apps as actor types in branch protection push restrictions: https://github.com/terraform-providers/terraform-provider-github/pull/615