diff --git a/scripts/generate-federation-iac.go b/scripts/generate-federation-iac.go index a671601c0..d3a9e6c83 100644 --- a/scripts/generate-federation-iac.go +++ b/scripts/generate-federation-iac.go @@ -470,6 +470,11 @@ func validateOIDCSubjectClaim(claim string, mode subjectClaimMode) error { // validFederationTargets in internal/api/handler_federation.go. var validTargets = map[string]bool{"aws": true, "azure": true, "gcp": true} +// validSources is the allowlist of source clouds. Without it a typo such as +// --source Azure fell through every source switch, rendering an empty OIDC +// issuer next to a populated subject claim. +var validSources = map[string]bool{"aws": true, "azure": true, "gcp": true} + // sourceAccountIDRE matches an AWS account ID: exactly 12 ASCII digits, no // whitespace padding, leading sign, or Unicode digit look-alikes. AWS account // IDs are always 12 digits; anything else could not be a real one and would @@ -513,13 +518,25 @@ func validateAzureAPIURL(baseURL string) error { return nil } +// validateCloudFlags checks --target before --source, for the diagnostic +// ordering described in populateData. +func validateCloudFlags(target, source string) error { + if !validTargets[target] { + return fmt.Errorf("--target must be aws, azure, or gcp (got %q)", target) + } + if !validSources[source] { + return fmt.Errorf("--source must be aws, azure, or gcp (got %q)", source) + } + return nil +} + // Validation precedes rendering so rejected input cannot overwrite an artifact. func populateData(data *iacData, target, source, tenantID, projectID, saEmail, oidcSubjectClaim, sourceAccountID string) error { // --target is checked first so that a typo there is reported as a bad // --target rather than as an inapplicable --oidc-subject-claim, which would // send the operator off to drop a flag that was never the problem. - if !validTargets[target] { - return fmt.Errorf("--target must be aws, azure, or gcp (got %q)", target) + if err := validateCloudFlags(target, source); err != nil { + return err } // The claim is then validated outside the switch, so the check covers the // targets that must NOT carry a subject claim as well as the one that must. diff --git a/scripts/generate_federation_iac_test.go b/scripts/generate_federation_iac_test.go index 9fcf50c87..3de1661fa 100644 --- a/scripts/generate_federation_iac_test.go +++ b/scripts/generate_federation_iac_test.go @@ -312,6 +312,51 @@ func TestGenerator_InvalidTargetReportedAsTarget(t *testing.T) { } } +// TestGenerator_InvalidSourceReportedAsSource is the --source mirror of the +// target test above. Before the source allowlist, --source Azure rendered a +// WIF tfvars with oidc_issuer_url = "" beside a populated subject claim. +func TestGenerator_InvalidSourceReportedAsSource(t *testing.T) { + for _, target := range []string{"aws", "gcp"} { + t.Run(target, func(t *testing.T) { + res := runGenerator(t, + "--target", target, + "--source", "Azure", // capitalised: a plausible typo + "--account-name", "prod", + "--account-id", "123456789012", + "--oidc-subject-claim", "123456789012345678901", + "--output", "-", + ) + if res.exitCode == 0 { + t.Fatalf("--source Azure was accepted (exit 0):\n%s", res.stdout) + } + if !strings.Contains(res.stderr, "--source must be aws, azure, or gcp") { + t.Errorf("a bad --source must be reported as a --source problem, got:\n%s", res.stderr) + } + if res.stdout != "" { + t.Errorf("a rejected --source must not render an artifact, got:\n%s", res.stdout) + } + }) + } +} + +// TestGenerator_BadTargetAndSourceReportsTargetFirst pins the check order in +// validateCloudFlags: with both flags wrong, --target is the one reported. +func TestGenerator_BadTargetAndSourceReportsTargetFirst(t *testing.T) { + res := runGenerator(t, + "--target", "bad", + "--source", "Azure", + "--account-name", "prod", + "--account-id", "123456789012", + "--output", "-", + ) + if res.exitCode == 0 { + t.Fatalf("bad --target and --source were accepted (exit 0)") + } + if !strings.Contains(res.stderr, "--target must be aws, azure, or gcp") { + t.Errorf("a bad --target must be reported before a bad --source, got:\n%s", res.stderr) + } +} + // TestGenerator_OverlongClaimIsNotEchoed checks that no rejection path echoes a // multi-kilobyte argument back at the operator, whichever diagnosis it reports. // Every branch that formats the claim goes through displayClaim, so this holds