Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion modules/common_repository/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,40 @@ resource "github_branch_protection" "repo_protection" {
}

required_status_checks {
strict = true
strict = var.strict_status_checks
contexts = var.required_status_checks
}
Comment on lines +101 to 103

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Automatically disable strict status checks when a merge queue is configured.

GitHub does not allow requiring branches to be up to date before merging when a merge queue is active. To prevent API errors and reduce the configuration burden on module consumers, consider automatically enforcing strict = false when var.merge_queue is provided.

♻️ Proposed refactor
   required_status_checks {
-    strict   = var.strict_status_checks
+    strict   = var.merge_queue != null ? false : var.strict_status_checks
     contexts = var.required_status_checks
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
strict = var.strict_status_checks
contexts = var.required_status_checks
}
required_status_checks {
strict = var.merge_queue != null ? false : var.strict_status_checks
contexts = var.required_status_checks
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/common_repository/main.tf` around lines 101 - 103, Update the strict
status-check assignment in the branch protection configuration to force false
whenever var.merge_queue is configured, while preserving
var.strict_status_checks when no merge queue is provided. Keep the existing
required status-check contexts unchanged.


depends_on = [github_repository.repo, github_repository_collaborators.repo_collaborators]
}

resource "github_repository_ruleset" "merge_queue" {
count = var.merge_queue != null ? 1 : 0
name = "merge-queue"
repository = github_repository.repo.name
target = "branch"
enforcement = "active"

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

rules {
merge_queue {
merge_method = var.merge_queue.merge_method
min_entries_to_merge = var.merge_queue.min_entries_to_merge
max_entries_to_merge = var.merge_queue.max_entries_to_merge
check_response_timeout_minutes = var.merge_queue.check_response_timeout_minutes
grouping_strategy = var.merge_queue.grouping_strategy
}
}
Comment on lines +136 to +144

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that if var.merge_queue == null the count will be zero. But, will TF also skip trying to make these assignments?

Suggested change
rules {
merge_queue {
merge_method = var.merge_queue.merge_method
min_entries_to_merge = var.merge_queue.min_entries_to_merge
max_entries_to_merge = var.merge_queue.max_entries_to_merge
check_response_timeout_minutes = var.merge_queue.check_response_timeout_minutes
grouping_strategy = var.merge_queue.grouping_strategy
}
}
rules {
# Use a dynamic block to only generate the merge_queue configuration
# if var.merge_queue is not null.
dynamic "merge_queue" {
for_each = var.merge_queue != null ? [var.merge_queue] : []
content {
merge_method = merge_queue.value.merge_method
min_entries_to_merge = merge_queue.value.min_entries_to_merge
max_entries_to_merge = merge_queue.value.max_entries_to_merge
check_response_timeout_minutes = merge_queue.value.check_response_timeout_minutes
grouping_strategy = merge_queue.value.grouping_strategy
}
}
}


depends_on = [github_repository.repo]
}

resource "github_repository_environment" "env" {
for_each = {
for env in var.environments :
Expand Down
18 changes: 18 additions & 0 deletions modules/common_repository/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ variable "required_status_checks" {
default = []
}

variable "strict_status_checks" {
description = "Require the PR branch to be up to date with the base branch before merging. Disable when using merge queues, which handle this automatically."
type = bool
default = true
}

variable "visibility" {
description = "Repository visibility (public or private)"
type = string
Expand Down Expand Up @@ -175,6 +181,18 @@ variable "environments" {
}
}

variable "merge_queue" {
description = "Enable GitHub merge queue for the default branch. When set, a repository ruleset is created that requires PRs to pass through the merge queue before merging."
type = object({
merge_method = optional(string, "SQUASH")
min_entries_to_merge = optional(number, 1)
max_entries_to_merge = optional(number, 5)
check_response_timeout_minutes = optional(number, 90)
grouping_strategy = optional(string, "ALLGREEN")
})
default = null
}

variable "all_members_permission" {
description = "Permission for all organization members"
type = string
Expand Down
30 changes: 20 additions & 10 deletions repositories.tf
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,10 @@ module "repo_fulfillment_service" {
"ci/prow/unit",
"e2e-vmaas-full-install / e2e"
]
push_allowances = ["/openshift-merge-robot", "osac-project/wg-infra", "osac-project/org-admins"]
environments = [{ name = "e2e-test" }]
push_allowances = ["/openshift-merge-robot", "osac-project/wg-infra", "osac-project/org-admins"]
environments = [{ name = "e2e-test" }]
merge_queue = {}
strict_status_checks = false
pages = {
build_type = "workflow"
source = {
Expand Down Expand Up @@ -141,8 +143,10 @@ module "repo_cloudkit_operator" {
"ci/prow/temp",
"e2e-vmaas-full-install / e2e"
]
push_allowances = ["/openshift-merge-robot", "osac-project/wg-infra", "osac-project/org-admins"]
environments = [{ name = "e2e-test" }]
push_allowances = ["/openshift-merge-robot", "osac-project/wg-infra", "osac-project/org-admins"]
environments = [{ name = "e2e-test" }]
merge_queue = {}
strict_status_checks = false
}

module "repo_cloudkit_aap" {
Expand All @@ -165,8 +169,10 @@ module "repo_cloudkit_aap" {
"ci/prow/temp",
"e2e-vmaas-full-install / e2e"
]
push_allowances = ["/openshift-merge-robot", "osac-project/wg-infra", "osac-project/org-admins"]
environments = [{ name = "e2e-test" }]
push_allowances = ["/openshift-merge-robot", "osac-project/wg-infra", "osac-project/org-admins"]
environments = [{ name = "e2e-test" }]
merge_queue = {}
strict_status_checks = false
}

module "repo_cloudkit_aap_ee" {
Expand Down Expand Up @@ -216,8 +222,10 @@ module "repo_osac_installer" {
]

required_approvals = null
push_allowances = ["/openshift-merge-robot", "osac-project/wg-infra", "osac-project/org-admins"]
environments = [{ name = "e2e-test" }]
push_allowances = ["/openshift-merge-robot", "osac-project/wg-infra", "osac-project/org-admins"]
environments = [{ name = "e2e-test" }]
merge_queue = {}
strict_status_checks = false
}

module "repo_enhancement_proposals" {
Expand Down Expand Up @@ -245,8 +253,10 @@ module "repo_osac_test_infra" {
}
]
required_approvals = null
push_allowances = ["/openshift-merge-robot", "osac-project/wg-infra", "osac-project/org-admins"]
environments = [{ name = "e2e-test" }]
push_allowances = ["/openshift-merge-robot", "osac-project/wg-infra", "osac-project/org-admins"]
environments = [{ name = "e2e-test" }]
merge_queue = {}
strict_status_checks = false
}

module "repo_massopencloud_templates" {
Expand Down
Loading