fix(branch_protection_v3): send each required status check only once

github/resource_github_branch_protection_v3_test.go

@@ -6,6 +6,9 @@ import (
 
 	"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
+	"github.com/hashicorp/terraform-plugin-testing/knownvalue"
+	"github.com/hashicorp/terraform-plugin-testing/statecheck"
+	"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
 )
 
 func TestAccGithubBranchProtectionV3_required_pull_request_reviews(t *testing.T) {
@@ -360,6 +363,64 @@ func TestAccGithubBranchProtectionV3_computed_status_contexts_no_churn(t *testin
 	})
 }
 
+func TestAccGithubBranchProtectionV3_update_with_status_checks(t *testing.T) {
+	t.Parallel()
+
+	// A read populates both `contexts` and `checks` from the API response, so updating
+	// any other setting used to send every required check twice, which GitHub rejects
+	// with "Context must be unique per branch protection".
+	for _, statusChecksField := range []string{"contexts", "checks"} {
+		t.Run(fmt.Sprintf("updates other settings when %s is set", statusChecksField), func(t *testing.T) {
+			t.Parallel()
+
+			repo := mustCreateTestRepository(t)
+
+			config := func(enforceAdmins bool) string {
+				return fmt.Sprintf(`
+				resource "github_branch_protection_v3" "test" {
+					repository     = "%s"
+					branch         = "main"
+					enforce_admins = %t
+
+					required_status_checks {
+						strict = true
+						%s = [
+							"ci/test",
+							"ci/build"
+						]
+					}
+				}
+			`, repo.GetName(), enforceAdmins, statusChecksField)
+			}
+
+			resource.Test(t, resource.TestCase{
+				PreCheck:          func() { skipUnauthenticated(t) },
+				ProviderFactories: providerFactories,
+				Steps: []resource.TestStep{
+					{
+						Config: config(false),
+					},
+					{
+						Config: config(true),
+						ConfigStateChecks: []statecheck.StateCheck{
+							statecheck.ExpectKnownValue(
+								"github_branch_protection_v3.test",
+								tfjsonpath.New("enforce_admins"),
+								knownvalue.Bool(true),
+							),
+							statecheck.ExpectKnownValue(
+								"github_branch_protection_v3.test",
+								tfjsonpath.New("required_status_checks").AtSliceIndex(0).AtMapKey(statusChecksField),
+								knownvalue.SetSizeExact(2),
+							),
+						},
+					},
+				},
+			})
+		})
+	}
+}
+
 func TestAccGithubBranchProtectionV3(t *testing.T) {
 	t.Parallel()
 

github/resource_github_branch_protection_v3_utils.go

@@ -261,17 +261,7 @@ func expandRequiredStatusChecks(d *schema.ResourceData) (*github.RequiredStatusC
 			// 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")
-			for _, c := range contexts {
-				appID := int64(-1) // Default
-				rscChecks = append(rscChecks, &github.RequiredStatusCheck{
-					Context: c,
-					AppID:   &appID,
-				})
-			}
+			rscChecksByContext := make(map[string]*github.RequiredStatusCheck)
 
 			// Iterate and parse checks
 			checks := expandNestedSet(m, "checks")
@@ -303,6 +293,29 @@ func expandRequiredStatusChecks(d *schema.ResourceData) (*github.RequiredStatusC
 
 				// Append
 				rscChecks = append(rscChecks, rscCheck)
+				rscChecksByContext[cContext] = rscCheck
+			}
+
+			// TODO: Remove once contexts is deprecated
+			// Iterate and parse contexts into checks using -1 as default to allow checks from all apps.
+			// A context can appear in both fields even though configuration cannot set both, because
+			// the API returns every required check under `contexts` as well as `checks`, so a read
+			// populates both. GitHub rejects a payload that repeats a context, so skip the duplicates.
+			contexts := expandNestedSet(m, "contexts")
+			for _, c := range contexts {
+				appID := int64(-1) // Default
+				if rscCheck, ok := rscChecksByContext[c]; ok {
+					if rscCheck.AppID == nil {
+						// `app_id: null` was read back, meaning any app is allowed. Omitting app_id
+						// instead would let GitHub pick an app, so keep it explicit.
+						rscCheck.AppID = &appID
+					}
+					continue
+				}
+				rscChecks = append(rscChecks, &github.RequiredStatusCheck{
+					Context: c,
+					AppID:   &appID,
+				})
 			}
 			// Assign after looping both checks and contexts
 			rsc.Checks = &rscChecks

github/resource_github_branch_protection_v3_utils_test.go

@@ -0,0 +1,98 @@
+package github
+
+import (
+	"testing"
+
+	"github.com/google/go-github/v89/github"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+)
+
+func TestExpandRequiredStatusChecks(t *testing.T) {
+	appID := int64(15368)
+	anyApp := int64(-1)
+
+	testCases := map[string]struct {
+		contexts []any
+		checks   []any
+		want     []*github.RequiredStatusCheck
+	}{
+		"contexts only": {
+			contexts: []any{"ci/test"},
+			want: []*github.RequiredStatusCheck{
+				{Context: "ci/test", AppID: &anyApp},
+			},
+		},
+		"checks only": {
+			checks: []any{"ci/test:15368"},
+			want: []*github.RequiredStatusCheck{
+				{Context: "ci/test", AppID: &appID},
+			},
+		},
+		// A read populates both fields from the API response, so state can hold the
+		// same context twice even though configuration cannot set both fields.
+		"same context in both fields is sent once": {
+			contexts: []any{"ci/test", "ci/build"},
+			checks:   []any{"ci/test", "ci/build"},
+			want: []*github.RequiredStatusCheck{
+				{Context: "ci/test", AppID: &anyApp},
+				{Context: "ci/build", AppID: &anyApp},
+			},
+		},
+		"app_id from checks wins over the context duplicate": {
+			contexts: []any{"ci/test"},
+			checks:   []any{"ci/test:15368"},
+			want: []*github.RequiredStatusCheck{
+				{Context: "ci/test", AppID: &appID},
+			},
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			statusChecks := map[string]any{"strict": true}
+			if tc.contexts != nil {
+				statusChecks["contexts"] = schema.NewSet(schema.HashString, tc.contexts)
+			}
+			if tc.checks != nil {
+				statusChecks["checks"] = schema.NewSet(schema.HashString, tc.checks)
+			}
+
+			d := schema.TestResourceDataRaw(t, resourceGithubBranchProtectionV3().Schema, map[string]any{
+				"repository": "test",
+				"branch":     "main",
+			})
+			if err := d.Set("required_status_checks", []any{statusChecks}); err != nil {
+				t.Fatalf("failed to set required_status_checks: %v", err)
+			}
+
+			rsc, err := expandRequiredStatusChecks(d)
+			if err != nil {
+				t.Fatalf("expected no error, got: %v", err)
+			}
+			if !rsc.Strict {
+				t.Error("expected strict to be true")
+			}
+
+			got := map[string]*int64{}
+			for _, check := range *rsc.Checks {
+				if _, ok := got[check.Context]; ok {
+					t.Fatalf("context %q sent more than once, GitHub rejects duplicates", check.Context)
+				}
+				got[check.Context] = check.AppID
+			}
+
+			if len(got) != len(tc.want) {
+				t.Fatalf("expected %d checks, got %d: %v", len(tc.want), len(got), got)
+			}
+			for _, want := range tc.want {
+				gotAppID, ok := got[want.Context]
+				if !ok {
+					t.Fatalf("expected context %q to be present", want.Context)
+				}
+				if gotAppID == nil || *gotAppID != *want.AppID {
+					t.Errorf("context %q: expected app_id %d, got %v", want.Context, *want.AppID, gotAppID)
+				}
+			}
+		})
+	}
+}