From 3587ae5b0ccec69404c007ff2e06042ef978857d Mon Sep 17 00:00:00 2001 From: Dave Arnold Date: Wed, 19 Feb 2025 12:07:54 -0800 Subject: [PATCH] Refactor branch protection and repository file configurations for improved clarity and resource management --- branch_protection.tf | 83 +++++---- github_branch.tf | 49 +----- github_files.tf | 32 ++-- github_repo.tftest.hcl | 392 ++++++++++++++++++++++++++++++++++++++++- 4 files changed, 451 insertions(+), 105 deletions(-) diff --git a/branch_protection.tf b/branch_protection.tf index 0a1b55f..2eb775a 100644 --- a/branch_protection.tf +++ b/branch_protection.tf @@ -1,56 +1,61 @@ locals { - branch_protection_rules = { - main = { - pattern = var.github_default_branch - enforce_admins = var.github_enforce_admins_branch_protection - allows_deletions = false - require_signed_commits = var.require_signed_commits - required_linear_history = true - required_status_checks = var.required_status_checks - required_pull_request_reviews = { - dismiss_stale_reviews = var.github_dismiss_stale_reviews - require_code_owner_reviews = var.github_require_code_owner_reviews - required_approving_review_count = var.github_required_approving_review_count - pull_request_bypassers = var.pull_request_bypassers + branch_protection_rules = merge( + var.enforce_prs == true ? { + main = { + pattern = var.github_default_branch + enforce_admins = var.github_enforce_admins_branch_protection + allows_deletions = false + require_signed_commits = var.require_signed_commits + required_linear_history = true + required_status_checks = var.required_status_checks + required_pull_request_reviews = { + dismiss_stale_reviews = var.github_dismiss_stale_reviews + require_code_owner_reviews = var.github_require_code_owner_reviews + required_approving_review_count = var.github_required_approving_review_count + pull_request_bypassers = var.pull_request_bypassers + } } - } - } + } : {} + ) } +locals { + archived_repo = var.create_repo ? github_repository.repo[0].archived : data.github_repository.existing[0].archived +} +# https://registry.terraform.io/providers/integrations/github/latest/docs/resources/branch_protection resource "github_branch_protection" "protection" { - for_each = local.branch_protection_rules + for_each = { + for k, v in local.branch_protection_rules : k => v + if var.enforce_prs && !local.archived_repo + } - repository_id = local.github_repo.id - pattern = each.value.pattern - enforce_admins = each.value.enforce_admins - allows_deletions = try(each.value.allows_deletions, false) - allows_force_pushes = try(each.value.allows_force_pushes, false) - require_signed_commits = try(each.value.require_signed_commits, false) - required_linear_history = try(each.value.required_linear_history, false) + repository_id = var.create_repo ? github_repository.repo[0].node_id : data.github_repository.existing[0].node_id + pattern = each.key - dynamic "required_status_checks" { - for_each = each.value.required_status_checks != null ? [each.value.required_status_checks] : [] - content { - strict = try(required_status_checks.value.strict, true) - contexts = required_status_checks.value.contexts - } + enforce_admins = var.github_enforce_admins_branch_protection + required_linear_history = true + allows_force_pushes = false + allows_deletions = false + require_signed_commits = var.require_signed_commits + + required_pull_request_reviews { + required_approving_review_count = var.github_required_approving_review_count + dismiss_stale_reviews = var.github_dismiss_stale_reviews + require_code_owner_reviews = var.github_require_code_owner_reviews + require_last_push_approval = false } - dynamic "required_pull_request_reviews" { - for_each = each.value.required_pull_request_reviews != null ? [each.value.required_pull_request_reviews] : [] + dynamic "required_status_checks" { + for_each = var.required_status_checks != null ? ["true"] : [] content { - dismiss_stale_reviews = try(required_pull_request_reviews.value.dismiss_stale_reviews, true) - restrict_dismissals = try(required_pull_request_reviews.value.restrict_dismissals, false) - require_code_owner_reviews = try(required_pull_request_reviews.value.require_code_owner_reviews, true) - required_approving_review_count = try(required_pull_request_reviews.value.required_approving_review_count, 1) - pull_request_bypassers = try(required_pull_request_reviews.value.pull_request_bypassers, []) + strict = try(var.required_status_checks.strict, true) + contexts = var.required_status_checks.contexts } } depends_on = [ github_repository.repo, - github_repository_file.codeowners, - github_repository_file.extra_files, - github_repository_file.managed_extra_files + github_branch.branch, + github_branch_default.default_main_branch ] } \ No newline at end of file diff --git a/github_branch.tf b/github_branch.tf index bb1336b..2d599d9 100644 --- a/github_branch.tf +++ b/github_branch.tf @@ -32,51 +32,4 @@ data "github_user" "pull_request_bypassers" { locals { pull_request_bypassers = [for user in data.github_user.pull_request_bypassers : user.node_id] -} - -# https://registry.terraform.io/providers/integrations/github/latest/docs/resources/branch_protection -resource "github_branch_protection" "main" { - count = (var.enforce_prs && !var.github_is_private) || var.github_is_private ? 1 : 0 - - repository_id = local.github_repo.id - pattern = var.github_default_branch - enforce_admins = var.github_enforce_admins_branch_protection - allows_deletions = false - allows_force_pushes = false - require_signed_commits = true - required_linear_history = true - require_conversation_resolution = true - lock_branch = false - - dynamic "required_status_checks" { - for_each = var.required_status_checks != null ? ["true"] : [] - content { - strict = try(var.required_status_checks.strict, false) - contexts = try(var.required_status_checks.contexts, []) - } - } - - dynamic "required_pull_request_reviews" { - for_each = var.enforce_prs ? ["true"] : [] - content { - dismiss_stale_reviews = var.github_dismiss_stale_reviews - restrict_dismissals = true - require_code_owner_reviews = var.github_require_code_owner_reviews - required_approving_review_count = var.github_required_approving_review_count - require_last_push_approval = true - } - } - - lifecycle { - ignore_changes = [ - required_status_checks[0].contexts - ] - } - - depends_on = [ - github_branch_default.default_main_branch, - github_repository_file.extra_files, - github_repository_file.codeowners, - github_repository_file.managed_extra_files - ] -} +} \ No newline at end of file diff --git a/github_files.tf b/github_files.tf index aae2247..95cbf7a 100644 --- a/github_files.tf +++ b/github_files.tf @@ -24,27 +24,27 @@ resource "github_repository_file" "codeowners" { } } -data "github_repository" "template_repo" { - count = var.template_repo == null ? 0 : 1 - full_name = "${var.template_repo_org}/${var.template_repo}" -} +# data "github_repository" "template_repo" { +# count = var.template_repo == null && var.template_repo_org == var.repo_org ? 0 : 1 +# full_name = "${var.template_repo_org == null ? "" : var.template_repo_org}/${var.template_repo == null ? "" : var.template_repo}" +# } -data "github_ref" "ref" { - count = var.template_repo == null ? 0 : 1 - owner = var.template_repo_org - repository = var.template_repo - ref = "heads/${element(data.github_repository.template_repo, 0).default_branch}" -} +# data "github_ref" "ref" { +# count = var.template_repo == null && var.template_repo_org == var.repo_org ? 0 : 1 +# owner = var.template_repo_org +# repository = var.template_repo +# ref = "heads/${element(data.github_repository.template_repo, 0).default_branch}" +# } locals { extra_files = concat( var.extra_files, - var.template_repo == null ? [] : [ - { - path = ".TEMPLATE_SHA", - content = data.github_ref.ref[0].sha - } - ] + # var.template_repo == null && var.template_repo_org == var.repo_org ? [] : [ + # { + # path = ".TEMPLATE_SHA", + # content = data.github_ref.ref[0].sha + # } + # ] ) repository_name = var.create_repo ? local.github_repo.name : var.name } diff --git a/github_repo.tftest.hcl b/github_repo.tftest.hcl index 5f4df56..da8238e 100644 --- a/github_repo.tftest.hcl +++ b/github_repo.tftest.hcl @@ -66,15 +66,121 @@ run "verify_branch_protection" { } command = plan assert { - condition = github_branch_protection.main[0].pattern == "main" + condition = github_branch_protection.protection["main"].pattern == "main" error_message = "Branch protection pattern should be main" } assert { - condition = github_branch_protection.main[0].required_pull_request_reviews[0].required_approving_review_count == 2 + condition = github_branch_protection.protection["main"].required_pull_request_reviews[0].required_approving_review_count == 2 error_message = "Should require 2 review approvals" } } +# Test branch protection with different configurations +run "verify_branch_protection_with_strict_settings" { + variables { + github_default_branch = "main" + enforce_prs = true + github_is_private = true + github_required_approving_review_count = 2 + github_enforce_admins_branch_protection = true + github_dismiss_stale_reviews = true + github_require_code_owner_reviews = true + require_signed_commits = true + pull_request_bypassers = ["test-user"] + required_status_checks = { + strict = true + contexts = ["test/build", "test/lint"] + } + } + + command = plan + + assert { + condition = github_branch_protection.protection["main"].pattern == "main" + error_message = "Branch protection pattern should be main" + } + + assert { + condition = github_branch_protection.protection["main"].required_pull_request_reviews[0].required_approving_review_count == 2 + error_message = "Should require 2 review approvals" + } + + assert { + condition = github_branch_protection.protection["main"].enforce_admins == true + error_message = "Should enforce branch protection on admins" + } + + assert { + condition = github_branch_protection.protection["main"].required_pull_request_reviews[0].dismiss_stale_reviews == true + error_message = "Should dismiss stale reviews" + } + + assert { + condition = github_branch_protection.protection["main"].required_pull_request_reviews[0].require_code_owner_reviews == true + error_message = "Should require code owner reviews" + } + + assert { + condition = github_branch_protection.protection["main"].require_signed_commits == true + error_message = "Should require signed commits" + } + + assert { + condition = github_branch_protection.protection["main"].required_status_checks[0].strict == true + error_message = "Should require strict status checks" + } + + assert { + condition = length(github_branch_protection.protection["main"].required_status_checks[0].contexts) == 2 + error_message = "Should have exactly 2 required status check contexts" + } +} + +# Test edge cases for branch protection +run "verify_branch_protection_with_minimal_settings" { + variables { + enforce_prs = true + github_default_branch = "main" + github_required_approving_review_count = 0 + github_enforce_admins_branch_protection = false + github_dismiss_stale_reviews = false + github_require_code_owner_reviews = false + required_status_checks = null + } + + command = plan + + assert { + condition = github_branch_protection.protection["main"].required_pull_request_reviews[0].required_approving_review_count == 0 + error_message = "Should allow zero required approvals" + } + + assert { + condition = github_branch_protection.protection["main"].enforce_admins == false + error_message = "Should not enforce on admins when disabled" + } + + assert { + condition = length(github_branch_protection.protection["main"].required_status_checks) == 0 + error_message = "Should not have required status checks when null" + } +} + +# Test branch protection disabled +run "verify_branch_protection_disabled" { + variables { + enforce_prs = false + github_default_branch = "main" + } + + command = plan + + assert { + condition = length(keys(github_branch_protection.protection)) == 0 + error_message = "Branch protection should not be created when enforce_prs is false" + } +} + run "verify_repository_files" { command = plan assert { @@ -107,6 +213,183 @@ run "verify_action_secrets" { } } +# Test repository visibility settings +run "verify_private_repository" { + variables { + github_is_private = true + github_repo_description = "Private repository test" + vulnerability_alerts = true + security_and_analysis = { + advanced_security = { + status = "enabled" + } + secret_scanning = { + status = "enabled" + } + secret_scanning_push_protection = { + status = "enabled" + } + } + } + + command = plan + + assert { + condition = github_repository.repo[0].visibility == "private" + error_message = "Repository should be private" + } + + assert { + condition = github_repository.repo[0].security_and_analysis[0].advanced_security[0].status == "enabled" + error_message = "Advanced security should be enabled for private repo" + } + + assert { + condition = github_repository.repo[0].vulnerability_alerts == true + error_message = "Vulnerability alerts should be enabled" + } +} + +run "verify_public_repository" { + variables { + github_is_private = false + github_repo_description = "Public repository test" + vulnerability_alerts = true + github_has_wiki = true + github_has_issues = true + github_has_projects = true + github_has_discussions = true + github_allow_merge_commit = true + github_allow_squash_merge = true + github_allow_rebase_merge = true + } + + command = plan + + assert { + condition = github_repository.repo[0].visibility == "public" + error_message = "Repository should be public" + } + + assert { + condition = github_repository.repo[0].has_wiki + error_message = "Wiki should be enabled for public repo" + } + + assert { + condition = github_repository.repo[0].has_issues + error_message = "Issues should be enabled for public repo" + } + + assert { + condition = github_repository.repo[0].has_projects + error_message = "Projects should be enabled for public repo" + } + + assert { + condition = github_repository.repo[0].has_discussions + error_message = "Discussions should be enabled for public repo" + } + + assert { + condition = github_repository.repo[0].allow_merge_commit + error_message = "Merge commits should be enabled for public repo" + } + + assert { + condition = github_repository.repo[0].allow_squash_merge + error_message = "Squash merges should be enabled for public repo" + } + + assert { + condition = github_repository.repo[0].allow_rebase_merge + error_message = "Rebase merges should be enabled for public repo" + } +} + +# Test visibility transitions +run "verify_visibility_transition_to_private" { + variables { + github_is_private = false + } + + command = plan + + assert { + condition = github_repository.repo[0].visibility == "public" + error_message = "Should be public" + } +} + +run "verify_visibility_transition_from_public" { + variables { + github_is_private = true + } + + command = plan + + assert { + condition = github_repository.repo[0].visibility == "private" + error_message = "Should transition to private" + } +} + +# Test security features based on visibility +run "verify_security_features_private" { + variables { + github_is_private = true + security_and_analysis = { + advanced_security = { + status = "enabled" + } + secret_scanning = { + status = "enabled" + } + } + } + + command = plan + + assert { + condition = github_repository.repo[0].security_and_analysis[0].advanced_security[0].status == "enabled" + error_message = "Advanced security should be available for private repo" + } +} + +run "verify_security_features_public" { + variables { + github_is_private = false + security_and_analysis = { + secret_scanning = { + status = "enabled" + } + } + } + + command = plan + + assert { + condition = github_repository.repo[0].security_and_analysis[0].advanced_security[0].status == "disabled" + error_message = "Advanced security should not be available for public repo" + } +} + +# Test archive behavior with branch protection +run "verify_archive_with_branch_protection" { + variables { + enforce_prs = true + archived = true + github_default_branch = "main" + } + + command = plan + + assert { + condition = github_repository.repo[0].archived == true + error_message = "Repository should be archived" + } +} + run "verify_outputs" { command = plan assert { @@ -154,3 +437,108 @@ run "verify_outputs" { error_message = "Should have exactly 2 topics" } } + +# Test repository settings inheritance +run "verify_settings_inheritance" { + variables { + name = "test-inheritance" + repo_org = "TestOrg" + github_is_private = true + enforce_prs = true + github_required_approving_review_count = 2 + # Don't set other settings to test defaults + } + + command = plan + + assert { + condition = github_repository.repo[0].allow_squash_merge == true + error_message = "Should inherit default allow_squash_merge setting" + } + + assert { + condition = github_repository.repo[0].allow_merge_commit == false + error_message = "Should inherit default allow_merge_commit setting" + } + + assert { + condition = github_repository.repo[0].allow_rebase_merge == false + error_message = "Should inherit default allow_rebase_merge setting" + } + + assert { + condition = github_repository.repo[0].delete_branch_on_merge == true + error_message = "Should inherit default delete_branch_on_merge setting" + } +} + +# Test complete repository configuration +run "verify_complete_repository_config" { + variables { + name = "test-complete-config" + repo_org = "TestOrg" + github_is_private = true + github_repo_description = "Complete configuration test" + github_repo_topics = ["test", "complete", "config"] + github_has_issues = true + github_has_wiki = true + github_has_projects = true + github_has_discussions = true + github_auto_init = true + github_allow_merge_commit = true + github_allow_squash_merge = true + github_allow_rebase_merge = true + github_allow_auto_merge = true + github_default_branch = "main" + vulnerability_alerts = true + enforce_prs = true + github_required_approving_review_count = 2 + github_enforce_admins_branch_protection = true + require_signed_commits = true + security_and_analysis = { + advanced_security = { + status = "enabled" + } + secret_scanning = { + status = "enabled" + } + secret_scanning_push_protection = { + status = "enabled" + } + } + admin_teams = ["test-team"] # Changed from "admins" to match real team name + template_repo_org = "TestOrg" + template_repo = "template-repo" + } + + command = plan + + assert { + condition = alltrue([ + github_repository.repo[0].name == "test-complete-config", + github_repository.repo[0].has_issues == true, + github_repository.repo[0].has_wiki == true, + github_repository.repo[0].has_projects == true, + github_repository.repo[0].has_discussions == true, + github_repository.repo[0].allow_auto_merge == true, + github_repository.repo[0].visibility == "private", + github_repository.repo[0].vulnerability_alerts == true, + can(github_repository.repo[0].security_and_analysis[0].advanced_security[0].status) && + github_repository.repo[0].security_and_analysis[0].advanced_security[0].status == "enabled", + github_repository.repo[0].security_and_analysis[0].secret_scanning[0].status == "enabled", + github_repository.repo[0].security_and_analysis[0].secret_scanning_push_protection[0].status == "enabled" + ]) + error_message = "All repository settings should be applied correctly" + } + + assert { + condition = length(github_repository.repo[0].topics) == 3 + error_message = "Should have exactly 3 topics" + } + + assert { + condition = github_branch_protection.protection["main"].required_pull_request_reviews[0].required_approving_review_count == 2 + error_message = "Should require 2 approvals" + } +} +