workflows: read protected owners and drift reviewers from deployment environments

.github/actions/drift-pr/action.yaml

@@ -102,7 +102,9 @@ runs:
             --head "$BRANCH")
           echo "Opened new drift PR: $pr_url"
           if [[ -n "${REVIEWERS}" ]]; then
-            gh pr edit "$pr_url" --add-reviewer "${REVIEWERS}" || echo "Warning: could not request reviewers '${REVIEWERS}'"
+            gh pr edit "$pr_url" --add-reviewer "${REVIEWERS}" || echo "::warning::Could not request reviewers '${REVIEWERS}' on $pr_url"
+          else
+            echo "::warning::No reviewers configured, drift PR $pr_url is unassigned. Set DRIFT_REVIEWERS on the schedule environment."
           fi
         else
           echo "Updated existing drift PR #$existing_pr."

.github/workflows/drift-check.yaml

@@ -12,11 +12,6 @@ on:
         type: string
         description: "Terraform Cloud organization"
         required: true
-      reviewers:
-        type: string
-        description: "Comma-separated reviewers (users or org/team slugs) to request on the drift PR"
-        required: false
-        default: ""
     secrets:
       tfc_token:
         description: "Terraform Cloud API token"
@@ -40,9 +35,10 @@ jobs:
     runs-on: ubuntu-latest
     # The 'schedule' environment must provide APP_ID + app_private_key (management
     # app, which needs org-wide "All repositories" access so new repos are visible
-    # to the importer), plus WORKSPACE for the plan. Because this runs unattended
-    # on a schedule, the environment MUST NOT have required-reviewer/wait-timer
-    # protection rules, or every run will stall pending manual approval.
+    # to the importer), WORKSPACE for the plan, and DRIFT_REVIEWERS for the drift
+    # PR. Because this runs unattended on a schedule, the environment MUST NOT
+    # have required-reviewer/wait-timer protection rules, or every run will stall
+    # pending manual approval.
     environment: schedule
     permissions:
       contents: write
@@ -124,7 +120,7 @@ jobs:
         uses: ./.github/actions/drift-pr
         with:
           branch-name: "drift/detected-changes"
-          reviewers: ${{ inputs.reviewers }}
+          reviewers: ${{ vars.DRIFT_REVIEWERS }}
           github-token: ${{ steps.generate-token.outputs.token }}
           # terraform is the authority on whether drift is real (avoids false
           # positives from config differences it treats as no-ops).

.github/workflows/tf-plan.yaml

@@ -21,11 +21,6 @@ on:
         description: "Workspace-relative path to the fallback JSON schema for repos/*.yaml validation, used when the config repo provides no override (e.g. 'gcss_config/.schemas/my.schema.json'). When omitted, the schema bundled with github-terraformer is used."
         required: false
         default: ""
-      protected_owners:
-        type: string
-        description: "Comma-separated org logins that must stay owners in organisation/members.yaml (never removed or demoted). Sourced from deployment config (e.g. a repo variable), not the config repo."
-        required: false
-        default: ""
     secrets:
       app_private_key:
         required: true
@@ -37,6 +32,7 @@ on:
 jobs:
   validate:
     runs-on: ubuntu-latest
+    environment: plan
     permissions:
       contents: read
     steps:
@@ -65,7 +61,7 @@ jobs:
         uses: ./.github/actions/validate-org-configs
         with:
           config-path: gcss_config
-          protected-owners: ${{ inputs.protected_owners }}
+          protected-owners: ${{ vars.PROTECTED_OWNERS }}
 
   terraform-plan:
     needs: validate

docs/renaming-teams-and-members.md

@@ -139,7 +139,7 @@ Use `terraform state rm` instead. The recreate is safe because the provider crea
 
 ### 1. Pre-flight: protected owners
 
-If the member is listed in the `protected_owners` input passed to `tf-plan.yaml` (a deployment-side repository variable, not config-repo content), **update that variable to the new username first**. Otherwise `validate-org` fails the PR because a protected owner is missing from `members.yaml`.
+If the member is listed in `PROTECTED_OWNERS` — a variable on the config repo's `plan` deployment environment, not config-repo content — **update that variable to the new username first**. Otherwise `validate-org` fails the PR because a protected owner is missing from `members.yaml`.
 
 ### 2. Open the PR and get it approved
 

docs/workflows.md

@@ -26,6 +26,37 @@
     5. Upon PR merge, Terraform Cloud plans and applies the configuration
     6. Configuration file is then sanitized (ids removed) and moved to the appropriate directory `feature/github-repo-provisioning/repo_configs/{branch}/{organization}`
 
+### ✅ `Validate and Plan` Workflow
+
+- **Trigger**: `workflow_run` from the config repo, on every pull request.
+- **Behavior**:
+    1. `validate` checks `repos/*.yaml` against the repository schema and `organisation/*.yaml` against the teams/members schemas and cross-file rules — including the protected-owner rule, which fails the PR if a protected identity is removed from or demoted in `members.yaml`.
+    2. `terraform-plan` (`needs: validate`, so it is skipped when validation fails) runs `terraform plan` on Terraform Cloud and posts the result as a PR comment and a check-run.
+
+#### Consumer setup requirements
+
+Both jobs run in the **`plan`** environment. Variables are read from that environment by the workflow itself:
+
+| Name | Read by | Purpose |
+|---|---|---|
+| `PROTECTED_OWNERS` | `validate` | Comma-separated org logins that must stay owners in `organisation/members.yaml` |
+| `APP_ID` | `terraform-plan` | GitHub App used to post the plan comment and check-run |
+| `WORKSPACE` | `terraform-plan` | Terraform Cloud workspace |
+
+The caller must also pass all three `workflow_call` secrets — these are declared on the reusable workflow, not looked up by the jobs:
+
+| Name | Purpose |
+|---|---|
+| `app_private_key` | Private key for the App named by `APP_ID` |
+| `gh_token` | Checks the config repo out at the commit under review |
+| `tfc_token` | Terraform Cloud API token |
+
+> [!IMPORTANT]
+> `PROTECTED_OWNERS` is deployment config and is deliberately read from the environment rather than passed in by the caller, so that a pull request cannot weaken the rule it is validated against. Leaving it unset is not a way to opt out: whenever any `organisation/` config is present, `validate-org` fails. A config repo with no organisation config at all needs no list and passes.
+
+> [!WARNING]
+> The `plan` environment **must not** have required-reviewer or wait-timer protection rules. Protection rules apply per job, and `validate` now runs in this environment too — so an approval gate costs **two** approvals per pull request, and the author sees no validation feedback until the first one lands. Consumers that had reviewers on `plan` for the `terraform-plan` job need to remove them, or accept that cost.
+
 ### 🔍 `Drift Check` Workflow
 
 - **Trigger**: Scheduled (cron) from the config repo.
@@ -45,12 +76,11 @@ The reusable `drift-check.yaml` runs in the **`schedule`** environment. That env
 | `app_private_key` | secret | Private key for that App |
 | `WORKSPACE` | variable | Terraform Cloud workspace |
 | `tfc_token` | secret | Terraform Cloud API token |
+| `DRIFT_REVIEWERS` | variable | Comma-separated users or `org/team` slugs to request as reviewers on the drift PR. When unset the drift PR still opens, unassigned, and the run logs a warning |
 
 > [!IMPORTANT]
 > The `schedule` environment **must not** have required-reviewer or wait-timer protection rules. The workflow runs unattended on a schedule, so any approval gate makes every run stall forever.
 
-Caller also passes `reviewers` (comma-separated users or `org/team` slugs) to request on the drift PR.
-
 #### Notes / limitations
 
 - **Scale**: each run does a **full org import** plus a plan. On large orgs this is the dominant cost (several API calls per repo). Pick a cron interval that comfortably exceeds a run's duration — overlapping runs are queued (`cancel-in-progress: false`), so too-frequent scheduling lags detection.
@@ -105,4 +135,4 @@ To import an **existing GitHub repository** into Terraform:
     - The name of the YAML file will be the same as the repository name
     - Create a PR against the `prod` branch
 4. Review, approve, and merge the PR
-5. Terraform Cloud will detect and apply the changes
+5. Terraform Cloud will detect and apply the changes

feature/github-repo-importer/cmd/validate-org.go

@@ -44,6 +44,11 @@ Files may be absent. An absent members.yaml is treated as an empty member list,
 matching how Terraform reads it, so protected owners are enforced even when the
 file is deleted outright.
 
+--protected-owners is required whenever any organisation config is present:
+validating organisation membership without it would report success while
+enforcing nothing. A config directory with no organisation config at all needs
+no list and passes.
+
 Unlike repository config, the schema is never taken from the config repository:
 both the schema and the protected-owner list are deployment config, so a pull
 request cannot weaken the rules it is validated against.`,
@@ -74,18 +79,22 @@ func runValidateOrg(cmd *cobra.Command, configDir, protectedOwnersCSV, fallbackT
 	)
 
 	protectedOwners := splitCSV(protectedOwnersCSV)
+	hasOrgConfig := len(teamsFiles) > 0 || len(membersFiles) > 0
+
 	if len(protectedOwners) > 0 {
-		cmd.Printf("Enforcing %d protected owner(s): %s\n", len(protectedOwners), strings.Join(protectedOwners, ", "))
-	} else {
-		cmd.PrintErrln("WARNING: no protected owners configured, owner removal and demotion are not enforced")
+		cmd.Printf("Enforcing %d protected owner(s)\n", len(protectedOwners))
 	}
 
-	if len(teamsFiles) == 0 && len(membersFiles) == 0 {
+	if !hasOrgConfig {
 		cmd.Println("No organisation teams.yaml or members.yaml found, validating as an empty organisation config")
 	}
 
 	var failures []string
 
+	if len(protectedOwners) == 0 && hasOrgConfig {
+		failures = append(failures, "no protected owners configured: organisation config is present, so owner removal and demotion must be enforced. Set PROTECTED_OWNERS on the deployment environment that runs validation")
+	}
+
 	teamNames, teamsOK := validateTeamsFiles(cmd, teamsFiles, fallbackTeams, &failures)
 	validateMembersFiles(cmd, membersFiles, fallbackMembers, teamNames, teamsOK, protectedOwners, &failures)
 

feature/github-repo-importer/cmd/validate-org_test.go

@@ -242,15 +242,37 @@ func TestValidateOrg_ProtectedOwnersCSVParsed(t *testing.T) {
 	assert.Contains(t, out, "Enforcing 2 protected owner(s)")
 }
 
-func TestValidateOrg_WarnsWhenNoProtectedOwnersConfigured(t *testing.T) {
+func TestValidateOrg_FailsWhenOrgConfigPresentAndNoProtectedOwners(t *testing.T) {
 	dir := newOrgConfigDir(t, map[string]string{
 		"members.yaml": "members:\n  - username: alice\n    role: owner\n",
 	})
 
 	out, err := runValidateOrgCmd(t, dir, "")
 
-	assert.NoError(t, err)
-	assert.Contains(t, out, "WARNING: no protected owners configured")
+	require.Error(t, err)
+	assert.Contains(t, out, "no protected owners configured")
+}
+
+func TestValidateOrg_FailsWhenOnlyTeamsPresentAndNoProtectedOwners(t *testing.T) {
+	dir := newOrgConfigDir(t, map[string]string{
+		"teams.yaml": validTeamsYAML,
+	})
+
+	_, err := runValidateOrgCmd(t, dir, "")
+
+	require.Error(t, err)
+}
+
+func TestValidateOrg_ProtectedOwnerLoginsAreNotEchoed(t *testing.T) {
+	dir := newOrgConfigDir(t, map[string]string{
+		"members.yaml": "members:\n  - username: alice\n    role: owner\n",
+	})
+
+	out, err := runValidateOrgCmd(t, dir, "alice")
+
+	require.NoError(t, err)
+	assert.Contains(t, out, "Enforcing 1 protected owner(s)")
+	assert.NotContains(t, out, "alice")
 }
 
 func TestValidateOrg_FileBasedSchemasAreUsed(t *testing.T) {