From d17416d499d863b82f86d5818bb30921859d77f9 Mon Sep 17 00:00:00 2001 From: Trevor Wood Date: Fri, 2 Sep 2022 15:23:11 -0400 Subject: [PATCH 01/10] feat: add v4 branch protections Signed-off-by: Trevor Wood --- main.tf | 94 ++++++++++++++++++++++++++++++++++++++++------------ variables.tf | 47 ++++++++++++++++++++++++++ versions.tf | 2 +- 3 files changed, 120 insertions(+), 23 deletions(-) diff --git a/main.tf b/main.tf index 547e0e7..f1cc984 100644 --- a/main.tf +++ b/main.tf @@ -27,7 +27,17 @@ locals { topics = concat(local.standard_topics, var.extra_topics) template = var.template == null ? [] : [var.template] issue_labels_create = var.issue_labels_create == null ? lookup(var.defaults, "issue_labels_create", local.issue_labels_create_computed) : var.issue_labels_create - branch_protections_v3 = var.branch_protections_v3 == null ? var.branch_protections : var.branch_protections_v3 + branch_protections_v3 = [ + for b in coalesce(var.branch_protections_v3, var.branch_protections, []) : merge({ + branch = null + enforce_admins = null + require_conversation_resolution = null + require_signed_commits = null + required_status_checks = {} + required_pull_request_reviews = {} + restrictions = {} + }, b) + ] issue_labels_create_computed = local.has_issues || length(var.issue_labels) > 0 @@ -41,20 +51,8 @@ locals { } locals { - branch_protections = try([ - for b in local.branch_protections_v3 : merge({ - branch = null - enforce_admins = null - require_conversation_resolution = null - require_signed_commits = null - required_status_checks = {} - required_pull_request_reviews = {} - restrictions = {} - }, b) - ], []) - required_status_checks = [ - for b in local.branch_protections : + for b in local.branch_protections_v3 : length(keys(b.required_status_checks)) > 0 ? [ merge({ strict = null @@ -63,7 +61,7 @@ locals { ] required_pull_request_reviews = [ - for b in local.branch_protections : + for b in local.branch_protections_v3 : length(keys(b.required_pull_request_reviews)) > 0 ? [ merge({ dismiss_stale_reviews = true @@ -75,7 +73,7 @@ locals { ] restrictions = [ - for b in local.branch_protections : + for b in local.branch_protections_v3 : length(keys(b.restrictions)) > 0 ? [ merge({ users = [] @@ -137,6 +135,7 @@ resource "github_repository" "repository" { lifecycle { ignore_changes = [ auto_init, + branches, license_template, gitignore_template, template, @@ -177,12 +176,63 @@ resource "github_branch_default" "default" { } # --------------------------------------------------------------------------------------------------------------------- -# Branch Protection +# v4 Branch Protection +# https://registry.terraform.io/providers/integrations/github/latest/docs/resources/branch_protection +# --------------------------------------------------------------------------------------------------------------------- + +resource "github_branch_protection" "this" { + for_each = coalesce(var.branch_protections_v4, {}) + + # ensure we have all members and collaborators added before applying + # any configuration for them + depends_on = [ + github_repository_collaborator.collaborator, + github_team_repository.team_repository, + github_team_repository.team_repository_by_slug, + github_branch.branch, + ] + + repository_id = github_repository.repository.node_id + pattern = each.key + allows_deletions = each.value.allows_deletions + allows_force_pushes = each.value.allows_force_pushes + blocks_creations = each.value.blocks_creations + enforce_admins = each.value.enforce_admins + push_restrictions = each.value.push_restrictions + require_conversation_resolution = each.value.require_conversation_resolution + require_signed_commits = each.value.require_signed_commits + required_linear_history = each.value.required_linear_history + + dynamic "required_pull_request_reviews" { + for_each = each.value.required_pull_request_reviews != null ? [true] : [] + + content { + dismiss_stale_reviews = each.value.required_pull_request_reviews.dismiss_stale_reviews + dismissal_restrictions = each.value.required_pull_request_reviews.dismissal_restrictions + pull_request_bypassers = each.value.required_pull_request_reviews.pull_request_bypassers + require_code_owner_reviews = each.value.required_pull_request_reviews.require_code_owner_reviews + required_approving_review_count = each.value.required_pull_request_reviews.required_approving_review_count + restrict_dismissals = length(each.value.required_pull_request_reviews.dismissal_restrictions) > 0 + } + } + + dynamic "required_status_checks" { + for_each = each.value.required_status_checks != null ? [true] : [] + + content { + strict = each.value.required_status_checks.strict + contexts = each.value.required_status_checks.contexts + } + } +} + +# --------------------------------------------------------------------------------------------------------------------- +# v3 Branch Protection # https://registry.terraform.io/providers/integrations/github/latest/docs/resources/branch_protection_v3 # --------------------------------------------------------------------------------------------------------------------- resource "github_branch_protection_v3" "branch_protection" { - count = length(local.branch_protections) + count = var.branch_protections_v4 == null ? length(local.branch_protections_v3) : 0 # ensure we have all members and collaborators added before applying # any configuration for them @@ -194,10 +244,10 @@ resource "github_branch_protection_v3" "branch_protection" { ] repository = github_repository.repository.name - branch = local.branch_protections[count.index].branch - enforce_admins = local.branch_protections[count.index].enforce_admins - require_conversation_resolution = local.branch_protections[count.index].require_conversation_resolution - require_signed_commits = local.branch_protections[count.index].require_signed_commits + branch = local.branch_protections_v3[count.index].branch + enforce_admins = local.branch_protections_v3[count.index].enforce_admins + require_conversation_resolution = local.branch_protections_v3[count.index].require_conversation_resolution + require_signed_commits = local.branch_protections_v3[count.index].require_signed_commits dynamic "required_status_checks" { for_each = local.required_status_checks[count.index] diff --git a/variables.tf b/variables.tf index 3eac082..fe4084a 100644 --- a/variables.tf +++ b/variables.tf @@ -351,6 +351,53 @@ variable "branch_protections_v3" { # ] } +variable "branch_protections_v4" { + description = "(Optional) A list of v4 branch protections to apply to the repository. Default is {}." + type = map( + object( + { + allows_deletions = optional(bool, false) + allows_force_pushes = optional(bool, false) + blocks_creations = optional(bool, false) + enforce_admins = optional(bool, false) + push_restrictions = optional(list(string), []) + require_conversation_resolution = optional(bool, false) + require_signed_commits = optional(bool, false) + required_linear_history = optional(bool, false) + required_pull_request_reviews = optional(object( + { + dismiss_stale_reviews = optional(bool, false) + dismissal_restrictions = optional(list(string), []) + pull_request_bypassers = optional(list(string), []) + require_code_owner_reviews = optional(bool, false) + required_approving_review_count = optional(number, 0) + } + )) + required_status_checks = optional(object( + { + strict = optional(bool, false) + contexts = optional(list(string), []) + } + )) + } + ) + ) + default = null + + validation { + condition = alltrue( + [ + for cfg in coalesce(var.branch_protections_v4, {}) : try( + cfg.required_pull_request_reviews.required_approving_review_count >= 0 + && cfg.required_pull_request_reviews.required_approving_review_count <= 6, + true + ) + ] + ) + error_message = "The value for branch_protections_v4.required_pull_request_reviews.required_approving_review_count must be between 0 and 6, inclusively." + } +} + variable "issue_labels_merge_with_github_labels" { description = "(Optional) Specify if you want to merge and control githubs default set of issue labels." type = bool diff --git a/versions.tf b/versions.tf index 01e52ac..6eaeefd 100644 --- a/versions.tf +++ b/versions.tf @@ -3,7 +3,7 @@ # --------------------------------------------------------------------------------------------------------------------- terraform { - required_version = "~> 1.0" + required_version = "~> 1.3" # branch_protections_v3 are broken in >= 5.3 required_providers { From f300c19e81e1d03d63e8441f2518b3b79f55a5e8 Mon Sep 17 00:00:00 2001 From: Trevor Wood Date: Mon, 31 Oct 2022 17:34:20 -0400 Subject: [PATCH 02/10] chore: guard against empty map Signed-off-by: Trevor Wood --- main.tf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main.tf b/main.tf index f1cc984..4b8a085 100644 --- a/main.tf +++ b/main.tf @@ -181,7 +181,7 @@ resource "github_branch_default" "default" { # --------------------------------------------------------------------------------------------------------------------- resource "github_branch_protection" "this" { - for_each = coalesce(var.branch_protections_v4, {}) + for_each = length(coalesce(var.branch_protections_v4, {})) > 0 ? var.branch_protections_v4 : {} # ensure we have all members and collaborators added before applying # any configuration for them @@ -232,7 +232,7 @@ resource "github_branch_protection" "this" { # --------------------------------------------------------------------------------------------------------------------- resource "github_branch_protection_v3" "branch_protection" { - count = var.branch_protections_v4 == null ? length(local.branch_protections_v3) : 0 + count = length(coalesce(var.branch_protections_v4, {})) == 0 ? length(local.branch_protections_v3) : 0 # ensure we have all members and collaborators added before applying # any configuration for them From 2969be6feb91e25c76ee8ab475e8bb874d1ebd44 Mon Sep 17 00:00:00 2001 From: Trevor Wood Date: Tue, 1 Nov 2022 21:54:21 -0400 Subject: [PATCH 03/10] fix: review comments Signed-off-by: Trevor Wood --- main.tf | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/main.tf b/main.tf index 4b8a085..9f36b51 100644 --- a/main.tf +++ b/main.tf @@ -135,7 +135,6 @@ resource "github_repository" "repository" { lifecycle { ignore_changes = [ auto_init, - branches, license_template, gitignore_template, template, @@ -180,7 +179,7 @@ resource "github_branch_default" "default" { # https://registry.terraform.io/providers/integrations/github/latest/docs/resources/branch_protection # --------------------------------------------------------------------------------------------------------------------- -resource "github_branch_protection" "this" { +resource "github_branch_protection" "branch_protection" { for_each = length(coalesce(var.branch_protections_v4, {})) > 0 ? var.branch_protections_v4 : {} # ensure we have all members and collaborators added before applying From 8d7a72f9ccb068e4bbd367943e906da0cebb9ce5 Mon Sep 17 00:00:00 2001 From: Trevor Wood Date: Tue, 1 Nov 2022 22:38:01 -0400 Subject: [PATCH 04/10] chore(docs): include v4 branch protections --- README.md | 136 +++++++++++++++++++++++++++++++++++++- README.tfdoc.hcl | 165 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 296 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index df15cc4..01b436f 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,8 @@ A [Terraform] module for creating a public or private repository on [Github]. - [Collaborator Configuration](#collaborator-configuration) - [Branches Configuration](#branches-configuration) - [Deploy Keys Configuration](#deploy-keys-configuration) - - [Branch Protections Configuration](#branch-protections-configuration) + - [Branch Protections v3 Configuration](#branch-protections-v3-configuration) + - [Branch Protections v4 Configuration](#branch-protections-v4-configuration) - [Issue Labels Configuration](#issue-labels-configuration) - [Projects Configuration](#projects-configuration) - [Webhooks Configuration](#webhooks-configuration) @@ -528,11 +529,11 @@ This is due to some terraform limitation and we will update the module once terr Default is `"md5(key)"`. -#### Branch Protections Configuration +#### Branch Protections v3 Configuration - [**`branch_protections_v3`**](#var-branch_protections_v3): *(Optional `list(branch_protection_v3)`)* - This resource allows you to configure branch protection for repositories in your organization. + This resource allows you to configure v3 branch protection for repositories in your organization. When applied, the branch will be protected from forced pushes and deletion. Additional constraints, such as required status checks or restrictions on users and teams, can also be configured. @@ -652,6 +653,135 @@ This is due to some terraform limitation and we will update the module once terr Default is `[]`. +#### Branch Protections v4 Configuration + +- [**`branch_protections_v4`**](#var-branch_protections_v4): *(Optional `map(branch_protection_v4)`)* + + This map allows you to configure v4 branch protection for repositories in your organization. + + Each element in the map is a branch to be protected and the value the corresponding to the desired configuration for the branch. + + When applied, the branch will be protected from forced pushes and deletion. + Additional constraints, such as required status checks or restrictions on users and teams, can also be configured. + + **_NOTE_** This will take precedence over v3 branch protections. + + Default is `null`. + + Each `branch_protection_v4` object in the map accepts the following attributes: + + - [**`allows_deletions`**](#attr-branch_protections_v4-allows_deletions): *(Optional `bool`)* + + Setting this to true to allow the branch to be deleted. + + Default is `false`. + + - [**`allows_force_pushes`**](#attr-branch_protections_v4-allows_force_pushes): *(Optional `bool`)* + + Setting this to true to allow force pushes on the branch. + + Default is `false`. + + - [**`blocks_creations`**](#attr-branch_protections_v4-blocks_creations): *(Optional `bool`)* + + Setting this to true will block creating the branch. + + Default is `false`. + + - [**`enforce_admins`**](#attr-branch_protections_v4-enforce_admins): *(Optional `bool`)* + + Setting this to true enforces status checks for repository administrators. + + Default is `false`. + + - [**`push_restrictions`**](#attr-branch_protections_v4-push_restrictions): *(Optional `list(string)`)* + + The list of actor Names/IDs that may push to the branch. + Actor names must either begin with a "/" for users or the organization name followed by a "/" for teams. + + Default is `[]`. + + - [**`require_conversation_resolution`**](#attr-branch_protections_v4-require_conversation_resolution): *(Optional `bool`)* + + Setting this to true requires all conversations on code must be resolved before a pull request can be merged. + + Default is `false`. + + - [**`require_signed_commits`**](#attr-branch_protections_v4-require_signed_commits): *(Optional `bool`)* + + Setting this to true requires all commits to be signed with GPG. + + Default is `false`. + + - [**`required_linear_history`**](#attr-branch_protections_v4-required_linear_history): *(Optional `bool`)* + + Setting this to true enforces a linear commit Git history, which prevents anyone from pushing merge commits to a branch. + + Default is `false`. + + - [**`required_pull_request_reviews`**](#attr-branch_protections_v4-required_pull_request_reviews): *(Optional `object(required_pull_request_reviews)`)* + + Enforce restrictions for pull request reviews. + + Default is `null`. + + The `required_pull_request_reviews` object accepts the following attributes: + + - [**`dismiss_stale_reviews`**](#attr-branch_protections_v4-required_pull_request_reviews-dismiss_stale_reviews): *(Optional `bool`)* + + Dismiss approved reviews automatically when a new commit is pushed. + + Default is `true`. + + - [**`dismissal_restrictions`**](#attr-branch_protections_v4-required_pull_request_reviews-dismissal_restrictions): *(Optional `list(string)`)* + + The list of actor Names/IDs with dismissal access. + If not empty, restrict_dismissals is ignored. + Actor names must either begin with a "/" for users or the organization name followed by a "/" for teams. + + Default is `[]`. + + - [**`pull_request_bypassers`**](#attr-branch_protections_v4-required_pull_request_reviews-pull_request_bypassers): *(Optional `list(string)`)* + + The list of actor Names/IDs that are allowed to bypass pull request requirements. + Actor names must either begin with a "/" for users or the organization name followed by a "/" for teams. + + Default is `[]`. + + - [**`require_code_owner_reviews`**](#attr-branch_protections_v4-required_pull_request_reviews-require_code_owner_reviews): *(Optional `bool`)* + + Require an approved review in pull requests including files with a designated code owner. + + Default is `false`. + + - [**`required_approving_review_count`**](#attr-branch_protections_v4-required_pull_request_reviews-required_approving_review_count): *(Optional `number`)* + + Require x number of approvals to satisfy branch protection requirements. + If this is specified it must be a number between 0-6. + + Default is `0`. + + - [**`required_status_checks`**](#attr-branch_protections_v4-required_status_checks): *(Optional `object(required_status_checks)`)* + + Enforce restrictions for required status checks. + See Required Status Checks below for details. + + Default is `null`. + + The `required_status_checks` object accepts the following attributes: + + - [**`strict`**](#attr-branch_protections_v4-required_status_checks-strict): *(Optional `bool`)* + + Require branches to be up to date before merging. + + Default is `false`. + + - [**`contexts`**](#attr-branch_protections_v4-required_status_checks-contexts): *(Optional `list(string)`)* + + The list of status checks to require in order to merge into this branch. If default is `[]` no status checks are required. + + Default is `[]`. + #### Issue Labels Configuration - [**`issue_labels`**](#var-issue_labels): *(Optional `list(issue_label)`)* diff --git a/README.tfdoc.hcl b/README.tfdoc.hcl index ab43b59..b6e97a7 100644 --- a/README.tfdoc.hcl +++ b/README.tfdoc.hcl @@ -677,13 +677,13 @@ section { } section { - title = "Branch Protections Configuration" + title = "Branch Protections v3 Configuration" variable "branch_protections_v3" { type = list(branch_protection_v3) default = [] description = <<-END - This resource allows you to configure branch protection for repositories in your organization. + This resource allows you to configure v3 branch protection for repositories in your organization. When applied, the branch will be protected from forced pushes and deletion. Additional constraints, such as required status checks or restrictions on users and teams, can also be configured. END @@ -832,6 +832,167 @@ section { } } + section { + title = "Branch Protections v4 Configuration" + + variable "branch_protections_v4" { + type = map(branch_protection_v4) + default = null + description = <<-END + This map allows you to configure v4 branch protection for repositories in your organization. + + Each element in the map is a branch to be protected and the value the corresponding to the desired configuration for the branch. + + When applied, the branch will be protected from forced pushes and deletion. + Additional constraints, such as required status checks or restrictions on users and teams, can also be configured. + + **_NOTE_** This will take precedence over v3 branch protections. + END + + attribute "allows_deletions" { + type = bool + default = false + description = <<-END + Setting this to true to allow the branch to be deleted. + END + } + + attribute "allows_force_pushes" { + type = bool + default = false + description = <<-END + Setting this to true to allow force pushes on the branch. + END + } + + attribute "blocks_creations" { + type = bool + default = false + description = <<-END + Setting this to true will block creating the branch. + END + } + + attribute "enforce_admins" { + type = bool + default = false + description = <<-END + Setting this to true enforces status checks for repository administrators. + END + } + + attribute "push_restrictions" { + type = list(string) + default = [] + description = <<-END + The list of actor Names/IDs that may push to the branch. + Actor names must either begin with a "/" for users or the organization name followed by a "/" for teams. + END + } + + attribute "require_conversation_resolution" { + type = bool + default = false + description = <<-END + Setting this to true requires all conversations on code must be resolved before a pull request can be merged. + END + } + + attribute "require_signed_commits" { + type = bool + default = false + description = <<-END + Setting this to true requires all commits to be signed with GPG. + END + } + + attribute "required_linear_history" { + type = bool + default = false + description = <<-END + Setting this to true enforces a linear commit Git history, which prevents anyone from pushing merge commits to a branch. + END + } + + attribute "required_pull_request_reviews" { + type = object(required_pull_request_reviews) + default = null + description = <<-END + Enforce restrictions for pull request reviews. + END + + attribute "dismiss_stale_reviews" { + type = bool + default = true + description = <<-END + Dismiss approved reviews automatically when a new commit is pushed. + END + } + + attribute "dismissal_restrictions" { + type = list(string) + default = [] + description = <<-END + The list of actor Names/IDs with dismissal access. + If not empty, restrict_dismissals is ignored. + Actor names must either begin with a "/" for users or the organization name followed by a "/" for teams. + END + } + + attribute "pull_request_bypassers" { + type = list(string) + default = [] + description = <<-END + The list of actor Names/IDs that are allowed to bypass pull request requirements. + Actor names must either begin with a "/" for users or the organization name followed by a "/" for teams. + END + } + + attribute "require_code_owner_reviews" { + type = bool + default = false + description = <<-END + Require an approved review in pull requests including files with a designated code owner. + END + } + + attribute "required_approving_review_count" { + type = number + default = 0 + description = <<-END + Require x number of approvals to satisfy branch protection requirements. + If this is specified it must be a number between 0-6. + END + } + } + + attribute "required_status_checks" { + type = object(required_status_checks) + default = null + description = <<-END + Enforce restrictions for required status checks. + See Required Status Checks below for details. + END + + attribute "strict" { + type = bool + default = false + description = <<-END + Require branches to be up to date before merging. + END + } + + attribute "contexts" { + type = list(string) + default = [] + description = <<-END + The list of status checks to require in order to merge into this branch. If default is `[]` no status checks are required. + END + } + } + } + } + section { title = "Issue Labels Configuration" From 00556ebef8e7b9de6542dd81540bf17ef8c1d519 Mon Sep 17 00:00:00 2001 From: Marius Tolzmann Date: Thu, 3 Nov 2022 17:34:36 +0100 Subject: [PATCH 05/10] chore: restore compatibility with tf 1.0+ --- variables.tf | 58 ++++++++++++++++++++++++++-------------------------- versions.tf | 2 +- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/variables.tf b/variables.tf index fe4084a..868b70a 100644 --- a/variables.tf +++ b/variables.tf @@ -353,35 +353,35 @@ variable "branch_protections_v3" { variable "branch_protections_v4" { description = "(Optional) A list of v4 branch protections to apply to the repository. Default is {}." - type = map( - object( - { - allows_deletions = optional(bool, false) - allows_force_pushes = optional(bool, false) - blocks_creations = optional(bool, false) - enforce_admins = optional(bool, false) - push_restrictions = optional(list(string), []) - require_conversation_resolution = optional(bool, false) - require_signed_commits = optional(bool, false) - required_linear_history = optional(bool, false) - required_pull_request_reviews = optional(object( - { - dismiss_stale_reviews = optional(bool, false) - dismissal_restrictions = optional(list(string), []) - pull_request_bypassers = optional(list(string), []) - require_code_owner_reviews = optional(bool, false) - required_approving_review_count = optional(number, 0) - } - )) - required_status_checks = optional(object( - { - strict = optional(bool, false) - contexts = optional(list(string), []) - } - )) - } - ) - ) + # type = map( + # object( + # { + # allows_deletions = optional(bool, false) + # allows_force_pushes = optional(bool, false) + # blocks_creations = optional(bool, false) + # enforce_admins = optional(bool, false) + # push_restrictions = optional(list(string), []) + # require_conversation_resolution = optional(bool, false) + # require_signed_commits = optional(bool, false) + # required_linear_history = optional(bool, false) + # required_pull_request_reviews = optional(object( + # { + # dismiss_stale_reviews = optional(bool, false) + # dismissal_restrictions = optional(list(string), []) + # pull_request_bypassers = optional(list(string), []) + # require_code_owner_reviews = optional(bool, false) + # required_approving_review_count = optional(number, 0) + # } + # )) + # required_status_checks = optional(object( + # { + # strict = optional(bool, false) + # contexts = optional(list(string), []) + # } + # )) + # } + # ) + # ) default = null validation { diff --git a/versions.tf b/versions.tf index 6eaeefd..01e52ac 100644 --- a/versions.tf +++ b/versions.tf @@ -3,7 +3,7 @@ # --------------------------------------------------------------------------------------------------------------------- terraform { - required_version = "~> 1.3" + required_version = "~> 1.0" # branch_protections_v3 are broken in >= 5.3 required_providers { From 31c249296176b08b59f607b2fc15557bb502312e Mon Sep 17 00:00:00 2001 From: Marius Tolzmann Date: Thu, 3 Nov 2022 18:21:16 +0100 Subject: [PATCH 06/10] refactor!: allow computed values in v4 branch protections --- .pre-commit-config.yaml | 5 --- README.md | 54 +++++++++++++++--------------- README.tfdoc.hcl | 62 ++++++++++++++++++++-------------- main.tf | 73 ++++++++++++++++++++++------------------- variables.tf | 18 ++++------ 5 files changed, 112 insertions(+), 100 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index bba3315..6561e23 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -8,10 +8,5 @@ repos: - id: tflint - id: phony-targets - id: terradoc-validate - - id: golangci-lint - id: terradoc-fmt - id: terradoc-generate - # - id: terramate-generate - - id: markdown-link-check - args: ['-p'] # When adding the -p flag, markdown-link-check will always with an exit code 0, even if dead links are found - verbose: true # Forces the output of the hook to be printed even when the hook passes. diff --git a/README.md b/README.md index 01b436f..e7a3950 100644 --- a/README.md +++ b/README.md @@ -646,53 +646,55 @@ This is due to some terraform limitation and we will update the module once terr Default is `[]`. -- [**`branch_protections`**](#var-branch_protections): *(Optional `list(branch_protection_v3)`)* - - **_DEPRECATED_** To ensure compatibility with future versions of this module, please use `branch_protections_v3`. - This argument is ignored if `branch_protections_v3` is used. Please see `branch_protections_v3` for supported attributes. - - Default is `[]`. - #### Branch Protections v4 Configuration -- [**`branch_protections_v4`**](#var-branch_protections_v4): *(Optional `map(branch_protection_v4)`)* +- [**`branch_protections_v4`**](#var-branch_protections_v4): *(Optional `list(branch_protection_v4)`)* - This map allows you to configure v4 branch protection for repositories in your organization. + This resource allows you to configure v4 branch protection for repositories in your organization. - Each element in the map is a branch to be protected and the value the corresponding to the desired configuration for the branch. + Each element in the list is a branch to be protected and the value the corresponding to the desired configuration for the branch. When applied, the branch will be protected from forced pushes and deletion. Additional constraints, such as required status checks or restrictions on users and teams, can also be configured. - **_NOTE_** This will take precedence over v3 branch protections. + **_NOTE:_** May conflict with v3 branch protections if used for the same branch. + + Default is `[]`. + + Each `branch_protection_v4` object in the list accepts the following attributes: + + - [**`pattern`**](#attr-branch_protections_v4-pattern): *(**Required** `string`)* + + Identifies the protection rule pattern. - Default is `null`. + - [**`_key`**](#attr-branch_protections_v4-_key): *(Optional `string`)* - Each `branch_protection_v4` object in the map accepts the following attributes: + An alternative key to use in `for_each` resource creation. + Defaults to the value of `var.pattern`. - [**`allows_deletions`**](#attr-branch_protections_v4-allows_deletions): *(Optional `bool`)* - Setting this to true to allow the branch to be deleted. + Setting this to `true` to allow the branch to be deleted. Default is `false`. - [**`allows_force_pushes`**](#attr-branch_protections_v4-allows_force_pushes): *(Optional `bool`)* - Setting this to true to allow force pushes on the branch. + Setting this to `true` to allow force pushes on the branch. Default is `false`. - [**`blocks_creations`**](#attr-branch_protections_v4-blocks_creations): *(Optional `bool`)* - Setting this to true will block creating the branch. + Setting this to `true` will block creating the branch. Default is `false`. - [**`enforce_admins`**](#attr-branch_protections_v4-enforce_admins): *(Optional `bool`)* - Setting this to true enforces status checks for repository administrators. + Keeping this as `true` enforces status checks for repository administrators. - Default is `false`. + Default is `true`. - [**`push_restrictions`**](#attr-branch_protections_v4-push_restrictions): *(Optional `list(string)`)* @@ -723,8 +725,6 @@ This is due to some terraform limitation and we will update the module once terr Enforce restrictions for pull request reviews. - Default is `null`. - The `required_pull_request_reviews` object accepts the following attributes: - [**`dismiss_stale_reviews`**](#attr-branch_protections_v4-required_pull_request_reviews-dismiss_stale_reviews): *(Optional `bool`)* @@ -733,18 +733,22 @@ This is due to some terraform limitation and we will update the module once terr Default is `true`. + - [**`restrict_dismissals`**](#attr-branch_protections_v4-required_pull_request_reviews-restrict_dismissals): *(Optional `bool`)* + + Restrict pull request review dismissals. + - [**`dismissal_restrictions`**](#attr-branch_protections_v4-required_pull_request_reviews-dismissal_restrictions): *(Optional `list(string)`)* The list of actor Names/IDs with dismissal access. - If not empty, restrict_dismissals is ignored. - Actor names must either begin with a "/" for users or the organization name followed by a "/" for teams. + If not empty, `restrict_dismissals` is ignored + Actor names must either begin with a `/` for users or the organization name followed by a `/` for teams. Default is `[]`. - [**`pull_request_bypassers`**](#attr-branch_protections_v4-required_pull_request_reviews-pull_request_bypassers): *(Optional `list(string)`)* The list of actor Names/IDs that are allowed to bypass pull request requirements. - Actor names must either begin with a "/" for users or the organization name followed by a "/" for teams. + Actor names must either begin with a `/` for users or the organization name followed by a `/` for teams. Default is `[]`. @@ -752,7 +756,7 @@ This is due to some terraform limitation and we will update the module once terr Require an approved review in pull requests including files with a designated code owner. - Default is `false`. + Default is `true`. - [**`required_approving_review_count`**](#attr-branch_protections_v4-required_pull_request_reviews-required_approving_review_count): *(Optional `number`)* @@ -766,8 +770,6 @@ This is due to some terraform limitation and we will update the module once terr Enforce restrictions for required status checks. See Required Status Checks below for details. - Default is `null`. - The `required_status_checks` object accepts the following attributes: - [**`strict`**](#attr-branch_protections_v4-required_status_checks-strict): *(Optional `bool`)* diff --git a/README.tfdoc.hcl b/README.tfdoc.hcl index b6e97a7..078679c 100644 --- a/README.tfdoc.hcl +++ b/README.tfdoc.hcl @@ -821,39 +821,46 @@ section { } } } - - variable "branch_protections" { - type = list(branch_protection_v3) - default = [] - description = <<-END - **_DEPRECATED_** To ensure compatibility with future versions of this module, please use `branch_protections_v3`. - This argument is ignored if `branch_protections_v3` is used. Please see `branch_protections_v3` for supported attributes. - END - } } section { title = "Branch Protections v4 Configuration" variable "branch_protections_v4" { - type = map(branch_protection_v4) - default = null + type = list(branch_protection_v4) + default = [] description = <<-END - This map allows you to configure v4 branch protection for repositories in your organization. + This resource allows you to configure v4 branch protection for repositories in your organization. - Each element in the map is a branch to be protected and the value the corresponding to the desired configuration for the branch. + Each element in the list is a branch to be protected and the value the corresponding to the desired configuration for the branch. When applied, the branch will be protected from forced pushes and deletion. Additional constraints, such as required status checks or restrictions on users and teams, can also be configured. - **_NOTE_** This will take precedence over v3 branch protections. + **_NOTE:_** May conflict with v3 branch protections if used for the same branch. END + attribute "pattern" { + type = string + required = true + description = <<-END + Identifies the protection rule pattern. + END + } + + attribute "_key" { + type = string + description = <<-END + An alternative key to use in `for_each` resource creation. + Defaults to the value of `var.pattern`. + END + } + attribute "allows_deletions" { type = bool default = false description = <<-END - Setting this to true to allow the branch to be deleted. + Setting this to `true` to allow the branch to be deleted. END } @@ -861,7 +868,7 @@ section { type = bool default = false description = <<-END - Setting this to true to allow force pushes on the branch. + Setting this to `true` to allow force pushes on the branch. END } @@ -869,15 +876,15 @@ section { type = bool default = false description = <<-END - Setting this to true will block creating the branch. + Setting this to `true` will block creating the branch. END } attribute "enforce_admins" { type = bool - default = false + default = true description = <<-END - Setting this to true enforces status checks for repository administrators. + Keeping this as `true` enforces status checks for repository administrators. END } @@ -916,7 +923,6 @@ section { attribute "required_pull_request_reviews" { type = object(required_pull_request_reviews) - default = null description = <<-END Enforce restrictions for pull request reviews. END @@ -929,13 +935,20 @@ section { END } + attribute "restrict_dismissals" { + type = bool + description = <<-END + Restrict pull request review dismissals. + END + } + attribute "dismissal_restrictions" { type = list(string) default = [] description = <<-END The list of actor Names/IDs with dismissal access. - If not empty, restrict_dismissals is ignored. - Actor names must either begin with a "/" for users or the organization name followed by a "/" for teams. + If not empty, `restrict_dismissals` is ignored + Actor names must either begin with a `/` for users or the organization name followed by a `/` for teams. END } @@ -944,13 +957,13 @@ section { default = [] description = <<-END The list of actor Names/IDs that are allowed to bypass pull request requirements. - Actor names must either begin with a "/" for users or the organization name followed by a "/" for teams. + Actor names must either begin with a `/` for users or the organization name followed by a `/` for teams. END } attribute "require_code_owner_reviews" { type = bool - default = false + default = true description = <<-END Require an approved review in pull requests including files with a designated code owner. END @@ -968,7 +981,6 @@ section { attribute "required_status_checks" { type = object(required_status_checks) - default = null description = <<-END Enforce restrictions for required status checks. See Required Status Checks below for details. diff --git a/main.tf b/main.tf index 9f36b51..587e1bb 100644 --- a/main.tf +++ b/main.tf @@ -27,17 +27,6 @@ locals { topics = concat(local.standard_topics, var.extra_topics) template = var.template == null ? [] : [var.template] issue_labels_create = var.issue_labels_create == null ? lookup(var.defaults, "issue_labels_create", local.issue_labels_create_computed) : var.issue_labels_create - branch_protections_v3 = [ - for b in coalesce(var.branch_protections_v3, var.branch_protections, []) : merge({ - branch = null - enforce_admins = null - require_conversation_resolution = null - require_signed_commits = null - required_status_checks = {} - required_pull_request_reviews = {} - restrictions = {} - }, b) - ] issue_labels_create_computed = local.has_issues || length(var.issue_labels) > 0 @@ -51,6 +40,18 @@ locals { } locals { + branch_protections_v3 = [ + for b in var.branch_protections_v3 : merge({ + branch = null + enforce_admins = null + require_conversation_resolution = null + require_signed_commits = null + required_status_checks = {} + required_pull_request_reviews = {} + restrictions = {} + }, b) + ] + required_status_checks = [ for b in local.branch_protections_v3 : length(keys(b.required_status_checks)) > 0 ? [ @@ -179,8 +180,12 @@ resource "github_branch_default" "default" { # https://registry.terraform.io/providers/integrations/github/latest/docs/resources/branch_protection # --------------------------------------------------------------------------------------------------------------------- +locals { + branch_protections_v4_map = { for idx, e in var.branch_protections_v4 : try(e._key, e.pattern) => idx } +} + resource "github_branch_protection" "branch_protection" { - for_each = length(coalesce(var.branch_protections_v4, {})) > 0 ? var.branch_protections_v4 : {} + for_each = local.branch_protections_v4_map # ensure we have all members and collaborators added before applying # any configuration for them @@ -191,36 +196,38 @@ resource "github_branch_protection" "branch_protection" { github_branch.branch, ] - repository_id = github_repository.repository.node_id - pattern = each.key - allows_deletions = each.value.allows_deletions - allows_force_pushes = each.value.allows_force_pushes - blocks_creations = each.value.blocks_creations - enforce_admins = each.value.enforce_admins - push_restrictions = each.value.push_restrictions - require_conversation_resolution = each.value.require_conversation_resolution - require_signed_commits = each.value.require_signed_commits - required_linear_history = each.value.required_linear_history + repository_id = github_repository.repository.node_id + + pattern = var.branch_protections_v4[each.value].pattern + + allows_deletions = try(var.branch_protections_v4[each.value].allows_deletions, false) + allows_force_pushes = try(var.branch_protections_v4[each.value].allows_force_pushes, false) + blocks_creations = try(var.branch_protections_v4[each.value].blocks_creations, false) + enforce_admins = try(var.branch_protections_v4[each.value].enforce_admins, true) + push_restrictions = try(var.branch_protections_v4[each.value].push_restrictions, []) + require_conversation_resolution = try(var.branch_protections_v4[each.value].require_conversation_resolution, false) + require_signed_commits = try(var.branch_protections_v4[each.value].require_signed_commits, false) + required_linear_history = try(var.branch_protections_v4[each.value].required_linear_history, false) dynamic "required_pull_request_reviews" { - for_each = each.value.required_pull_request_reviews != null ? [true] : [] + for_each = try([var.branch_protections_v4[each.value].required_pull_request_reviews], []) content { - dismiss_stale_reviews = each.value.required_pull_request_reviews.dismiss_stale_reviews - dismissal_restrictions = each.value.required_pull_request_reviews.dismissal_restrictions - pull_request_bypassers = each.value.required_pull_request_reviews.pull_request_bypassers - require_code_owner_reviews = each.value.required_pull_request_reviews.require_code_owner_reviews - required_approving_review_count = each.value.required_pull_request_reviews.required_approving_review_count - restrict_dismissals = length(each.value.required_pull_request_reviews.dismissal_restrictions) > 0 + dismiss_stale_reviews = try(required_pull_request_reviews.value.dismiss_stale_reviews, true) + restrict_dismissals = try(required_pull_request_reviews.value.restrict_dismissals, null) + dismissal_restrictions = try(required_pull_request_reviews.value.dismissal_restrictions, []) + pull_request_bypassers = try(required_pull_request_reviews.value.pull_request_bypassers, []) + 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, 0) } } dynamic "required_status_checks" { - for_each = each.value.required_status_checks != null ? [true] : [] + for_each = try([var.branch_protections_v4[each.value].required_status_checks], []) content { - strict = each.value.required_status_checks.strict - contexts = each.value.required_status_checks.contexts + strict = try(required_status_checks.value.strict, false) + contexts = try(required_status_checks.value.contexts, []) } } } @@ -231,7 +238,7 @@ resource "github_branch_protection" "branch_protection" { # --------------------------------------------------------------------------------------------------------------------- resource "github_branch_protection_v3" "branch_protection" { - count = length(coalesce(var.branch_protections_v4, {})) == 0 ? length(local.branch_protections_v3) : 0 + count = length(local.branch_protections_v3) # ensure we have all members and collaborators added before applying # any configuration for them diff --git a/variables.tf b/variables.tf index 868b70a..debfbaa 100644 --- a/variables.tf +++ b/variables.tf @@ -287,12 +287,6 @@ variable "maintain_teams" { default = [] } -variable "branch_protections" { - description = "DEPRECATED: use branch_protections_v3 instead. Default is []." - type = any - default = null -} - variable "branch_protections_v3" { description = "(Optional) A list of branch protections to apply to the repository. Default is [] unless branch_protections is set." type = any @@ -321,7 +315,7 @@ variable "branch_protections_v3" { # }) # })) - default = null + default = [] # Example: # branch_protections = [ @@ -352,10 +346,12 @@ variable "branch_protections_v3" { } variable "branch_protections_v4" { - description = "(Optional) A list of v4 branch protections to apply to the repository. Default is {}." - # type = map( + description = "(Optional) A list of v4 branch protections to apply to the repository. Default is []." + type = any + # type = list( # object( # { + # pattern = string # allows_deletions = optional(bool, false) # allows_force_pushes = optional(bool, false) # blocks_creations = optional(bool, false) @@ -382,12 +378,12 @@ variable "branch_protections_v4" { # } # ) # ) - default = null + default = [] validation { condition = alltrue( [ - for cfg in coalesce(var.branch_protections_v4, {}) : try( + for cfg in var.branch_protections_v4 : try( cfg.required_pull_request_reviews.required_approving_review_count >= 0 && cfg.required_pull_request_reviews.required_approving_review_count <= 6, true From 6e5763c031bafe6ee96ce2dc3d093b0fa3da3b07 Mon Sep 17 00:00:00 2001 From: Marius Tolzmann Date: Sun, 6 Nov 2022 14:20:00 +0100 Subject: [PATCH 07/10] chore: Add test for v4 branch protections --- test/unit-complete/main.tf | 26 +++++++++++++++++++++++++- test/unit-complete/provider.tf | 5 +++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/test/unit-complete/main.tf b/test/unit-complete/main.tf index ef708a4..f307097 100644 --- a/test/unit-complete/main.tf +++ b/test/unit-complete/main.tf @@ -86,7 +86,31 @@ module "repository" { secret = var.webhook_secret }] - branch_protections = [ + branch_protections_v4 = [ + { + pattern = "staging" + + allows_deletions = false + allows_force_pushes = false + blocks_creations = false + enforce_admins = true + require_conversation_resolution = true + require_signed_commits = true + required_linear_history = true + + required_pull_request_reviews = { + dismiss_stale_reviews = true + require_code_owner_reviews = true + required_approving_review_count = 1 + } + + required_status_checks = { + strict = true + } + } + ] + + branch_protections_v3 = [ { branch = "main" enforce_admins = true diff --git a/test/unit-complete/provider.tf b/test/unit-complete/provider.tf index 7cd89ca..e80c4cc 100644 --- a/test/unit-complete/provider.tf +++ b/test/unit-complete/provider.tf @@ -5,8 +5,9 @@ terraform { required_providers { github = { - source = "integrations/github" - version = "~> 5.0, !=5.3.0, !=5.4.0, !=5.5.0" + source = "integrations/github" + # mask providers with broken branch protection v3 imlementation + version = "~> 5.0, !=5.3.0, !=5.4.0, !=5.5.0, !=5.6.0, !=5.7.0" } tls = { source = "hashicorp/tls" From 6d601b121f7066549dc0a9db42d2ae1aeae54df0 Mon Sep 17 00:00:00 2001 From: Marius Tolzmann Date: Sun, 6 Nov 2022 15:28:11 +0100 Subject: [PATCH 08/10] chore: prepare 0.18.0 release --- CHANGELOG.md | 13 ++++++++++++- README.md | 15 +-------------- README.tfdoc.hcl | 15 +-------------- 3 files changed, 14 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27f4708..b8e16b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.18.0] + +### Added + +- Add support for v4 branch protections. + +### Removed + +- BREAKING CHANGE: Remove deprectated variable `branch_protections` please use `branch_protections_v3` instead. + ## [0.17.0] ### Added @@ -385,7 +395,8 @@ Please review plans and report regressions and issues asap so we can improve doc - This is the initial release of our GitHub Repository module with support for creating and managing GitHub Repositories for Organizations. -[unreleased]: https://github.com/mineiros-io/terraform-github-repository/compare/v0.17.0...HEAD +[unreleased]: https://github.com/mineiros-io/terraform-github-repository/compare/v0.18.0...HEAD +[0.18.0]: https://github.com/mineiros-io/terraform-github-repository/compare/v0.17.0...v0.18.0 [0.17.0]: https://github.com/mineiros-io/terraform-github-repository/compare/v0.16.2...v0.17.0 [0.16.2]: https://github.com/mineiros-io/terraform-github-repository/compare/v0.16.1...v0.16.2 [0.16.1]: https://github.com/mineiros-io/terraform-github-repository/compare/v0.16.0...v0.16.1 diff --git a/README.md b/README.md index e7a3950..b923150 100644 --- a/README.md +++ b/README.md @@ -104,25 +104,12 @@ Most basic usage creating a new private github repository. ```hcl module "repository" { source = "mineiros-io/repository/github" - version = "~> 0.16.0" + version = "~> 0.18.0" name = "terraform-github-repository" license_template = "apache-2.0" gitignore_template = "Terraform" } - -provider "github" {} - -terraform { - required_version = "~> 1.0" - - required_providers { - github = { - source = "integrations/github" - version = "~> 4.0" - } - } -} ``` ## Module Argument Reference diff --git a/README.tfdoc.hcl b/README.tfdoc.hcl index 078679c..9ea35cd 100644 --- a/README.tfdoc.hcl +++ b/README.tfdoc.hcl @@ -109,25 +109,12 @@ section { ```hcl module "repository" { source = "mineiros-io/repository/github" - version = "~> 0.16.0" + version = "~> 0.18.0" name = "terraform-github-repository" license_template = "apache-2.0" gitignore_template = "Terraform" } - - provider "github" {} - - terraform { - required_version = "~> 1.0" - - required_providers { - github = { - source = "integrations/github" - version = "~> 4.0" - } - } - } ``` END } From 629cafb2eb40b432a74c35344ed0a4b2b2ab0662 Mon Sep 17 00:00:00 2001 From: Marius Tolzmann Date: Sun, 6 Nov 2022 15:44:04 +0100 Subject: [PATCH 09/10] chore: Mark defaults as deprecated. --- README.md | 30 ++++-------------------------- README.tfdoc.hcl | 30 ++++-------------------------- variables.tf | 25 ++----------------------- 3 files changed, 10 insertions(+), 75 deletions(-) diff --git a/README.md b/README.md index b923150..eed6135 100644 --- a/README.md +++ b/README.md @@ -124,32 +124,10 @@ See [variables.tf] and [examples/] for details and use-cases. - [**`defaults`**](#var-defaults): *(Optional `object(defaults)`)* - A object of default settings to use instead of module defaults for top-level arguments. - See below for a list of supported arguments. - - This is a special argument to set various defaults to be reused for multiple repositories. - - The following top-level arguments can be set as defaults: - `homepage_url`, - `visibility`, - `has_issues`, - `has_projects`, - `has_wiki`, - `allow_merge_commit`, - `allow_rebase_merge`, - `allow_squash_merge`, - `allow_auto_merge`, - `has_downloads`, - `auto_init`, - `gitignore_template`, - `license_template`, - `default_branch`, - `topics`, - `issue_labels_create`, - `issue_labels_merge_with_github_labels`. - - Module defaults are used for all arguments that are not set in `defaults`. - Using top level arguments override defaults set by this argument. + DEPRECATED: + This variable will be removed in future releases. + It was needed in times when Terraform Module for each was not available to provide default values for multiple repositories. + Please convert your code accordingly to stay compatible with future releases. Default is `{}`. diff --git a/README.tfdoc.hcl b/README.tfdoc.hcl index 9ea35cd..1f75765 100644 --- a/README.tfdoc.hcl +++ b/README.tfdoc.hcl @@ -140,32 +140,10 @@ section { type = object(defaults) default = {} description = <<-END - A object of default settings to use instead of module defaults for top-level arguments. - See below for a list of supported arguments. - - This is a special argument to set various defaults to be reused for multiple repositories. - - The following top-level arguments can be set as defaults: - `homepage_url`, - `visibility`, - `has_issues`, - `has_projects`, - `has_wiki`, - `allow_merge_commit`, - `allow_rebase_merge`, - `allow_squash_merge`, - `allow_auto_merge`, - `has_downloads`, - `auto_init`, - `gitignore_template`, - `license_template`, - `default_branch`, - `topics`, - `issue_labels_create`, - `issue_labels_merge_with_github_labels`. - - Module defaults are used for all arguments that are not set in `defaults`. - Using top level arguments override defaults set by this argument. + DEPRECATED: + This variable will be removed in future releases. + It was needed in times when Terraform Module for each was not available to provide default values for multiple repositories. + Please convert your code accordingly to stay compatible with future releases. END } diff --git a/variables.tf b/variables.tf index debfbaa..7599034 100644 --- a/variables.tf +++ b/variables.tf @@ -25,30 +25,9 @@ variable "branches" { } variable "defaults" { - description = "(Optional) Overwrite defaults for various repository settings" + description = "(Deprecated) DEPRECATED. Please convert defaults to Terraform Module for_each" type = any - - # Example: - # defaults = { - # homepage_url = "https://mineiros.io/" - # visibility = "private" - # has_issues = false - # has_projects = false - # has_wiki = false - # delete_branch_on_merge = true - # allow_merge_commit = true - # allow_rebase_merge = false - # allow_squash_merge = false - # allow_auto_merge = false - # has_downloads = false - # auto_init = true - # gitignore_template = "terraform" - # license_template = "mit" - # default_branch = "main" - # topics = ["topic-1", "topic-2"] - # } - - default = {} + default = {} } variable "description" { From 3e4e6e2ab47a5ca8d73d2c877a2b38bdd69de6c7 Mon Sep 17 00:00:00 2001 From: Marius Tolzmann Date: Sun, 6 Nov 2022 15:45:25 +0100 Subject: [PATCH 10/10] chore: Update Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8e16b7..46065d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - BREAKING CHANGE: Remove deprectated variable `branch_protections` please use `branch_protections_v3` instead. +### Deprecated + +- Mark `var.defaults` as deprecated. This variable was introduced and used before Terraform Module `for_each` was available. + ## [0.17.0] ### Added