feature/985 branch protection checks (integrations#1415)

* 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 (integrations#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 <[email protected]> * feat: adds new branch protection options for last reviewer and locking branch (integrations#1407) Co-authored-by: Keegan Campbell <[email protected]> * feat(github_release): adding github_release resource and tests (integrations#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 <[email protected]> * 🚧 Workflows have changed Workflow changes have been made in the Octokit org repo. This PR is propagating those changes. * Issue template tweak (integrations#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 <[email protected]> Co-authored-by: Keegan Campbell <[email protected]> Co-authored-by: Sean Smith <[email protected]> Co-authored-by: Trent Millar <[email protected]> Co-authored-by: Nick Floyd <[email protected]>
avidspartan1 · Jan 20, 2023 · a782a23 · a782a23
1 parent a655e2a
commit a782a23
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 10 deletions.
diff --git 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
@@ -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
@@ -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