From c5cfce0a4c8820ea26c72a9dfa7d4e434907e0e2 Mon Sep 17 00:00:00 2001 From: TheQueenIsDead <30706552+TheQueenIsDead@users.noreply.github.com> Date: Sat, 21 Jan 2023 09:02:02 +1300 Subject: [PATCH] feature/985 branch protection `checks` (#1415) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add new schema for check block resource under required_status_checks * feat: iterate provided `check` blocks and build array of RequiredStatusCheck * feat: set default app_id to -1 * feat: implement checks flattening for required status checks * Add resource github_app_installation_repositories (#1376) * Add resource github_app_installation_repositories This resource allows multiple repositories to be passed in; which greatly improves the performance of the resource compared to the single repository version when needing to control state of multiple app installations with multiple repos, required in larger organisations. The optimisation occurs as only a single call to get the list of repos is required per installation per read, regardless of the number of respositories being added. - Add resource_github_app_installation_repositories - Add tests * Update docs and link Co-authored-by: Keegan Campbell * feat: adds new branch protection options for last reviewer and locking branch (#1407) Co-authored-by: Keegan Campbell * feat(github_release): adding github_release resource and tests (#1122) * feat(github_release): adding github_release resource and tests * feat(docs) adding github_release page to website docs * chore: update changelog with this pr's new resource * fix: adding node_id and release_id to resource attributes * Update CHANGELOG.md * Fix broken merge/build Co-authored-by: Keegan Campbell * 🚧 Workflows have changed Workflow changes have been made in the Octokit org repo. This PR is propagating those changes. * Issue template tweak (#1422) * Don't link to a real PR * Wording tweak * feat: allow branch protection check app_id to be null * chore: change branch protection flatten function to use GetAppID sdk method * feat: change branch protection v3 utils to flatten and expand contexts into checks * feat: change checks from it's own resource to a list of strings * chore: resolve incorrect merge of main * chore: update deprecation notice on contexts array * chore(docs): Update branch_protection_v3 docs to mention the new `checks` functionality * fix: Initialise literal empty slice of RequiredStatusCheck to mitigate errors when passing nil to the sdk * chore(lint): resolve gosimple S1082 violation (errors.New => fmt.Errorf) * chore: remove unused code comment Co-authored-by: David Bain <97858950+david-bain@users.noreply.github.com> Co-authored-by: Keegan Campbell Co-authored-by: Sean Smith Co-authored-by: Trent Millar Co-authored-by: Nick Floyd <139819+nickfloyd@users.noreply.github.com> --- .../resource_github_branch_protection_v3.go | 8 ++ ...ource_github_branch_protection_v3_utils.go | 78 +++++++++++++++++-- .../docs/r/branch_protection_v3.html.markdown | 11 ++- 3 files changed, 87 insertions(+), 10 deletions(-) diff --git a/github/resource_github_branch_protection_v3.go b/github/resource_github_branch_protection_v3.go index cb0bc6b95f..f425bbf56b 100644 --- a/github/resource_github_branch_protection_v3.go +++ b/github/resource_github_branch_protection_v3.go @@ -53,6 +53,14 @@ func resourceGithubBranchProtectionV3() *schema.Resource { Default: false, }, "contexts": { + Type: schema.TypeSet, + Optional: true, + Deprecated: "GitHub is deprecating the use of `contexts`. Use a `checks` array instead.", + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + "checks": { Type: schema.TypeSet, Optional: true, Elem: &schema.Schema{ diff --git a/github/resource_github_branch_protection_v3_utils.go b/github/resource_github_branch_protection_v3_utils.go index 5e1fb07ac5..23d1f6fa22 100644 --- a/github/resource_github_branch_protection_v3_utils.go +++ b/github/resource_github_branch_protection_v3_utils.go @@ -4,11 +4,11 @@ import ( "context" "errors" "fmt" - "log" - "strings" - "github.com/google/go-github/v49/github" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "log" + "strconv" + "strings" ) func buildProtectionRequest(d *schema.ResourceData) (*github.ProtectionRequest, error) { @@ -40,16 +40,34 @@ func buildProtectionRequest(d *schema.ResourceData) (*github.ProtectionRequest, func flattenAndSetRequiredStatusChecks(d *schema.ResourceData, protection *github.Protection) error { rsc := protection.GetRequiredStatusChecks() + if rsc != nil { - contexts := make([]interface{}, 0, len(rsc.Contexts)) + + // Contexts and Checks arrays to flatten into + var contexts []interface{} + var checks []interface{} + + // TODO: Remove once contexts is fully deprecated. + // Flatten contexts for _, c := range rsc.Contexts { + // Parse into contexts contexts = append(contexts, c) + checks = append(contexts, c) + } + + // Flatten checks + for _, chk := range rsc.Checks { + // Parse into contexts + contexts = append(contexts, chk.Context) + checks = append(contexts, fmt.Sprintf("%s:%d", chk.Context, chk.AppID)) } return d.Set("required_status_checks", []interface{}{ map[string]interface{}{ - "strict": rsc.Strict, + "strict": rsc.Strict, + // TODO: Remove once contexts is fully deprecated. "contexts": schema.NewSet(schema.HashString, contexts), + "checks": schema.NewSet(schema.HashString, checks), }, }) } @@ -191,8 +209,56 @@ func expandRequiredStatusChecks(d *schema.ResourceData) (*github.RequiredStatusC m := v.(map[string]interface{}) rsc.Strict = m["strict"].(bool) + // Initialise empty literal to ensure an empty array is passed mitigating schema errors like so: + // For 'anyOf/1', {"strict"=>true, "checks"=>nil} is not a null. [] + rscChecks := []*github.RequiredStatusCheck{} + + // TODO: Remove once contexts is deprecated + // Iterate and parse contexts into checks using -1 as default to allow checks from all apps. contexts := expandNestedSet(m, "contexts") - rsc.Contexts = contexts + for _, c := range contexts { + appID := int64(-1) // Default + rscChecks = append(rscChecks, &github.RequiredStatusCheck{ + Context: c, + AppID: &appID, + }) + } + + // Iterate and parse checks + checks := expandNestedSet(m, "checks") + for _, c := range checks { + + // Expect a string of "context:app_id", allowing for the absence of "app_id" + parts := strings.SplitN(c, ":", 2) + var cContext, cAppId string + switch len(parts) { + case 1: + cContext, cAppId = parts[0], "" + case 2: + cContext, cAppId = parts[0], parts[1] + default: + return nil, fmt.Errorf("Could not parse check '%s'. Expected `context:app_id` or `context`", c) + } + + var rscCheck *github.RequiredStatusCheck + if cAppId != "" { + // If we have a valid app_id, include it in the RSC + rscAppId, err := strconv.Atoi(cAppId) + if err != nil { + return nil, fmt.Errorf("Could not parse %v as valid app_id", cAppId) + } + rscAppId64 := int64(rscAppId) + rscCheck = &github.RequiredStatusCheck{Context: cContext, AppID: &rscAppId64} + } else { + // Else simply provide the context + rscCheck = &github.RequiredStatusCheck{Context: cContext} + } + + // Append + rscChecks = append(rscChecks, rscCheck) + } + // Assign after looping both checks and contexts + rsc.Checks = rscChecks } return rsc, nil } diff --git a/website/docs/r/branch_protection_v3.html.markdown b/website/docs/r/branch_protection_v3.html.markdown index 12a7c25efd..4194fba8e7 100644 --- a/website/docs/r/branch_protection_v3.html.markdown +++ b/website/docs/r/branch_protection_v3.html.markdown @@ -29,8 +29,8 @@ resource "github_branch_protection_v3" "example" { ```hcl # Protect the main branch of the foo repository. Additionally, require that -# the "ci/travis" context to be passing and only allow the engineers team merge -# to the branch. +# the "ci/check" check ran by the Github Actions app is passing and only allow +# the engineers team merge to the branch. resource "github_branch_protection_v3" "example" { repository = github_repository.example.name @@ -39,7 +39,9 @@ resource "github_branch_protection_v3" "example" { required_status_checks { strict = false - contexts = ["ci/travis"] + checks = [ + "ci/check:824642007264" + ] } required_pull_request_reviews { @@ -88,7 +90,8 @@ The following arguments are supported: `required_status_checks` supports the following arguments: * `strict`: (Optional) Require branches to be up to date before merging. Defaults to `false`. -* `contexts`: (Optional) The list of status checks to require in order to merge into this branch. No status checks are required by default. +* `contexts`: [**DEPRECATED**] (Optional) The list of status checks to require in order to merge into this branch. No status checks are required by default. +* `checks`: (Optional) The list of status checks to require in order to merge into this branch. No status checks are required by default. Checks should be strings containing the context and app_id like so "context:app_id". ### Required Pull Request Reviews