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]: integration_id value returns difference that can't be ignored #2317

Open
1 task done
strachg opened this issue Jul 15, 2024 · 4 comments · May be fixed by #2529
Open
1 task done

[BUG]: integration_id value returns difference that can't be ignored #2317

strachg opened this issue Jul 15, 2024 · 4 comments · May be fixed by #2529
Labels
Status: Up for grabs Issues that are ready to be worked on by anyone Type: Bug Something isn't working as documented

Comments

@strachg
Copy link

strachg commented Jul 15, 2024

Expected Behavior

When using the github_repository_ruleset and enabling the required_status_checks with a required_check config block, the integration_id is an optional value. When a required_check is configured with only the context value provided, the integration_id should be ignored.

    required_status_checks {
      required_check {
        context        = "context1"
      }
    }
  }

Actual Behavior

When using the github_repository_ruleset and enabling the required_status_checks, if the integration_id is not supplied, once the value is updated on the github side (if you have something like Github Advanced Security), the terraform plan results show that the integration_id has been updated. Limitation in Lifecycle ignore_changes means that this value can't be ignored without ignoring all required_check blocks that have been configured.

      ~ rules {
            # (7 unchanged attributes hidden)

          ~ required_status_checks {
                # (1 unchanged attribute hidden)

              - required_check {
                  - context        = "CodeQL" -> null
                  - integration_id = 57789 -> null
                }
              + required_check {
                  + context        = "CodeQL"
                  + integration_id = 0
                }
            }

            # (1 unchanged block hidden)
        }

        # (2 unchanged blocks hidden)

If the value returned on subsequent terraform plan executions is used initially, when creating the required_check, the following error is returned.

module.[redacted].github_repository_ruleset.main[0]: Modifying... [id=1154071]
╷
│ Error: PUT https://api.github.com/repos/[org]/[redacted]/rulesets/1154071: 422 Validation Failed [{Resource: Field: Code: Message:Invalid rules: 'Required status checks'}]
│ 
│   with module.[redacted].github_repository_ruleset.main[0],
│   on modules/github-repository/main.tf line 48, in resource "github_repository_ruleset" "main":
│   48: resource "github_repository_ruleset" "main" {
│ 
╵

Terraform Version

Terraform v1.9.2
on darwin_arm64

  • provider registry.terraform.io/hashicorp/aws v5.51.1
  • provider registry.terraform.io/integrations/github v6.2.1

Affected Resource(s)

  • github_repository, github_repository_ruleset.required_status_checks

Terraform Configuration Files

variable "branch_protection_required_status_checks" {
  type        = map(string)
  description = "List of strings mapping to status checks that must pass for a protected branch merge"
  default = {
    CodeQL = 0
  }
}

variable "branch_protection_enabled" {
  type        = bool
  description = "Protects the main branch from a number of possible sources of corruption"
  default     = false
}


resource "github_repository" "main" {
  name                   = var.name
  description            = var.description
  visibility             = "private"
  delete_branch_on_merge = var.delete_branch_on_merge
  vulnerability_alerts   = true
  has_downloads          = false // Deprecated
  has_issues             = var.has_issues
  has_projects           = var.has_projects
  has_wiki               = var.has_wiki
}



resource "github_repository_ruleset" "main" {
  count       = var.branch_protection_enabled ? 1 : 0
  name        = "main-branch-protection"
  repository  = github_repository.main.name
  target      = "branch"
  enforcement = "active"

  conditions {
    ref_name {
      include = ["~DEFAULT_BRANCH"]
      exclude = []
    }
  }

  bypass_actors {
    actor_id    =[redacted]
    actor_type  = "Team"
    bypass_mode = "always"
  }

  rules {
    creation                = true
    non_fast_forward        = true
    update                  = false
    deletion                = true
    required_linear_history = true
    required_signatures     = true

    pull_request {
      dismiss_stale_reviews_on_push     = false
      require_code_owner_review         = false
      require_last_push_approval        = false
      required_approving_review_count   = 1
      required_review_thread_resolution = true
    }

    required_status_checks {
      strict_required_status_checks_policy = true

      dynamic "required_check" {
        for_each = var.branch_protection_required_status_checks
        content {
          context        = required_check.key
          integration_id = required_check.value
        }
      }
    }
  }

}

Steps to Reproduce

No response

Debug Output

No response

Panic Output

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@strachg strachg added Status: Triage This is being looked at and prioritized Type: Bug Something isn't working as documented labels Jul 15, 2024
@leonfibal
Copy link

I have a problem which seems to be connected with yours. I can't apply a plan with integration_id other than 0, because of the same error (422 error code). Were you able to use non-0 integration_id @strachg?

@kfcampbell kfcampbell added Status: Up for grabs Issues that are ready to be worked on by anyone and removed Status: Triage This is being looked at and prioritized labels Jul 15, 2024
@kfcampbell kfcampbell moved this from 🆕 Triage to 🔥 Backlog in 🧰 Octokit Active Jul 15, 2024
@norbertwnuk
Copy link

For me the 422 error code was caused by missing repository access in the configuration the GH App. This can be fixed by either allowing APP to read all repositories or using github_app_installation_repository.

@Danielku15
Copy link
Contributor

I also ran into this problem. In my case I want integration_id to be unset but it is always passed as 0 causing the validation error (having all permissions as org admin).

This fully prevents me from creating ruleset with required checks. I also checked the API directly and I get the same error when setting integration_id to 0 instead of skipping it.

This code part looks suspicious:

for _, check := range params.RequiredStatusChecks {
integrationID := int64(0)
if check.IntegrationID != nil {
integrationID = *check.IntegrationID
}
requiredStatusChecksSlice = append(requiredStatusChecksSlice, map[string]interface{}{
"context": check.Context,
"integration_id": integrationID,
})
}

Feels like it should be:

    for _, check := range params.RequiredStatusChecks {
        integrationID := int64(0)
        
        checkMap := make(map[string]interface{}{})
        checkMap["context"] = check.Context
        if check.IntegrationID != nil {
            checkMap["integration_id"] = *check.IntegrationID
        }
        requiredStatusChecksSlice = append(requiredStatusChecksSlice, checkMap)
    }

@Danielku15 Danielku15 linked a pull request Jan 6, 2025 that will close this issue
4 tasks
@Danielku15
Copy link
Contributor

Danielku15 commented Jan 6, 2025

I attempted to fix this problem in #2529 In the tests it looks promising that zero and 0 values are handled correctly after the change. In my local tests I could successfully create rulesets afterwards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Up for grabs Issues that are ready to be worked on by anyone Type: Bug Something isn't working as documented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants