fix(scripts): add OIDCIssuerHost to generate-federation-iac and test for drift

scripts/generate-federation-iac.go

@@ -108,8 +108,13 @@ import (
 	"unicode"
 )
 
-// iacData mirrors internal/api/handler_federation.go:federationIaCData.
-// Keep in sync with any template variable changes.
+// iacData is this script's copy of federationIaCData in
+// internal/api/handler_federation.go: both render the same templates in
+// internal/iacfiles/templates. The script cannot import that type (it would
+// pull internal/api into a //go:build ignore dev tool), so the two are held in
+// step by tests in scripts/generate_federation_iac_drift_test.go instead: they
+// fail when the two structs stop having the same fields, or when any template
+// references a field iacData lacks (#153).
 type iacData struct {
 	AccountName       string
 	AccountExternalID string
@@ -123,7 +128,11 @@ type iacData struct {
 	SourceAccountID string
 	// AWS WIF / cross-account
 	OIDCIssuerURL string
-	OIDCAudience  string
+	// OIDCIssuerHost is OIDCIssuerURL without its https:// prefix, the form
+	// IAM uses in condition keys; aws-cfn-deploy.sh.tmpl passes it to
+	// CloudFormation. Derived the same way as the server does.
+	OIDCIssuerHost string
+	OIDCAudience   string
 	// OIDCSubjectClaim restricts the AWS trust policy to a single workload
 	// subject. Empty unless --oidc-subject-claim is passed; every AWS-WIF
 	// template requires it (no working subject-less default, see #1640).
@@ -664,6 +673,7 @@ func populateData(data *iacData, target, source, tenantID, projectID, saEmail, o
 	switch target {
 	case "aws":
 		data.OIDCIssuerURL = awsOIDCIssuer(source, tenantID)
+		data.OIDCIssuerHost = strings.TrimPrefix(data.OIDCIssuerURL, "https://")
 		data.OIDCAudience = awsOIDCAudience(source)
 		data.OIDCSubjectClaim = oidcSubjectClaim
 		if err := requireSourceAccountID(data, source, sourceAccountID,

scripts/generate_federation_iac_drift_test.go

@@ -0,0 +1,209 @@
+package main
+
+// Drift guards for #153. generate-federation-iac.go renders the same templates
+// as the server's GET /api/federation/iac, from its own copy of the template
+// data struct (iacData, a copy of internal/api's federationIaCData). A comment
+// asking people to keep the two in sync did not: iacData went without
+// OIDCIssuerHost, which aws-cfn-deploy.sh.tmpl references, so
+// `--format bundle` failed for every AWS target with a non-AWS source.
+//
+// The script carries //go:build ignore and internal/api is a large package, so
+// neither struct is imported here. Both are read from source with go/parser,
+// and every template is parsed with text/template/parse, so a field added to
+// one side or used by a new template fails these tests without coupling the
+// script to the server package.
+
+import (
+	"archive/zip"
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"io"
+	"os"
+	"path/filepath"
+	"sort"
+	"strings"
+	"testing"
+	"text/template"
+	"text/template/parse"
+
+	"github.com/stretchr/testify/require"
+)
+
+// structFieldNames returns the field names of the struct type typeName
+// declared in the Go source file at path, sorted.
+func structFieldNames(t *testing.T, path, typeName string) []string {
+	t.Helper()
+	file, err := parser.ParseFile(token.NewFileSet(), path, nil, parser.SkipObjectResolution)
+	require.NoError(t, err)
+	var names []string
+	found := false
+	ast.Inspect(file, func(n ast.Node) bool {
+		spec, ok := n.(*ast.TypeSpec)
+		if !ok || spec.Name.Name != typeName {
+			return true
+		}
+		st, ok := spec.Type.(*ast.StructType)
+		require.True(t, ok, "%s in %s is not a struct type", typeName, path)
+		found = true
+		for _, field := range st.Fields.List {
+			require.NotEmpty(t, field.Names, "embedded field in %s is not supported by this check", typeName)
+			for _, name := range field.Names {
+				names = append(names, name.Name)
+			}
+		}
+		return false
+	})
+	require.True(t, found, "type %s not found in %s", typeName, path)
+	sort.Strings(names)
+	return names
+}
+
+// templateFieldRefs adds to refs every top-level field that node reads from
+// the template's data, mapped to the template that reads it. The templates
+// never rebind dot, so .X always names a field of the data struct; a with or
+// range block would break that assumption, so it fails the test instead of
+// being guessed at.
+func templateFieldRefs(t *testing.T, tmpl string, node parse.Node, refs map[string]string) {
+	t.Helper()
+	switch n := node.(type) {
+	case nil:
+	case *parse.ListNode:
+		if n == nil {
+			return
+		}
+		for _, child := range n.Nodes {
+			templateFieldRefs(t, tmpl, child, refs)
+		}
+	case *parse.ActionNode:
+		templateFieldRefs(t, tmpl, n.Pipe, refs)
+	case *parse.IfNode:
+		templateFieldRefs(t, tmpl, n.Pipe, refs)
+		templateFieldRefs(t, tmpl, n.List, refs)
+		templateFieldRefs(t, tmpl, n.ElseList, refs)
+	case *parse.PipeNode:
+		if n == nil {
+			return
+		}
+		for _, cmd := range n.Cmds {
+			for _, arg := range cmd.Args {
+				templateFieldRefs(t, tmpl, arg, refs)
+			}
+		}
+	case *parse.FieldNode:
+		refs[n.Ident[0]] = tmpl
+	case *parse.VariableNode:
+		if n.Ident[0] == "$" && len(n.Ident) > 1 {
+			refs[n.Ident[1]] = tmpl
+		}
+	case *parse.TextNode, *parse.CommentNode, *parse.StringNode, *parse.NumberNode,
+		*parse.BoolNode, *parse.NilNode, *parse.DotNode, *parse.IdentifierNode:
+	default:
+		t.Fatalf("%s: template node %T (%s) is not handled by this check; extend templateFieldRefs", tmpl, node, node)
+	}
+}
+
+// iacTemplateFieldRefs parses every template in internal/iacfiles/templates
+// and returns the data fields they reference, each mapped to one template
+// that uses it.
+func iacTemplateFieldRefs(t *testing.T) map[string]string {
+	t.Helper()
+	paths, err := filepath.Glob(filepath.Join(repoRoot, "internal", "iacfiles", "templates", "*.tmpl"))
+	require.NoError(t, err)
+	require.NotEmpty(t, paths, "no templates found")
+	refs := map[string]string{}
+	for _, path := range paths {
+		raw, err := os.ReadFile(path)
+		require.NoError(t, err)
+		// Parsed the way the script and the server parse them, so the
+		// builtin functions such as eq are defined.
+		tmpl, err := template.New(filepath.Base(path)).Parse(string(raw))
+		require.NoError(t, err, "parse %s", path)
+		for _, tt := range tmpl.Templates() {
+			templateFieldRefs(t, filepath.Base(path), tt.Root, refs)
+		}
+	}
+	return refs
+}
+
+// TestIaCData_MatchesServerTemplateData fails when iacData and the server's
+// federationIaCData stop having the same fields. Templates are shared, so a
+// field the server fills is one a template can start using at any time.
+func TestIaCData_MatchesServerTemplateData(t *testing.T) {
+	script := structFieldNames(t, scriptFile, "iacData")
+	server := structFieldNames(t, filepath.Join(repoRoot, "internal", "api", "handler_federation.go"), "federationIaCData")
+	require.Equal(t, server, script,
+		"iacData in scripts/%s and federationIaCData in internal/api/handler_federation.go render the same "+
+			"templates and must have the same fields (#153)", scriptFile)
+}
+
+// TestIaCData_CoversEveryTemplateField fails as soon as any template in
+// internal/iacfiles/templates references a field iacData lacks. text/template
+// treats a missing struct field as an execution error, so such a template
+// cannot be rendered by the script at all.
+func TestIaCData_CoversEveryTemplateField(t *testing.T) {
+	have := map[string]bool{}
+	for _, name := range structFieldNames(t, scriptFile, "iacData") {
+		have[name] = true
+	}
+	var missing []string
+	for field, tmpl := range iacTemplateFieldRefs(t) {
+		if !have[field] {
+			missing = append(missing, field+" (used by "+tmpl+")")
+		}
+	}
+	sort.Strings(missing)
+	require.Empty(t, missing, "iacData lacks fields that the templates reference: %s", strings.Join(missing, ", "))
+}
+
+// TestIaCData_FieldWalkerSeesKnownFields is the positive control for the two
+// tests above: a parser that found no fields would make both pass vacuously.
+func TestIaCData_FieldWalkerSeesKnownFields(t *testing.T) {
+	refs := iacTemplateFieldRefs(t)
+	for _, field := range []string{"AccountName", "CUDlyAPIURL", "OIDCIssuerHost", "SourceAccountID"} {
+		require.Contains(t, refs, field)
+	}
+	require.Contains(t, structFieldNames(t, scriptFile, "iacData"), "AccountName")
+}
+
+// TestGenerator_AWSWIFBundleRendersDeployScript runs the real
+// aws-cfn-deploy.sh.tmpl through --format bundle for each non-AWS source. It
+// only reads the rendered script; running it would call the AWS CLI.
+func TestGenerator_AWSWIFBundleRendersDeployScript(t *testing.T) {
+	const tenant = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"
+	cases := []struct {
+		source, wantHost string
+		extra            []string
+	}{
+		{"gcp", "accounts.google.com", nil},
+		{"azure", "login.microsoftonline.com/" + tenant + "/v2.0", []string{"--tenant-id", tenant}},
+	}
+	for _, tc := range cases {
+		t.Run(tc.source, func(t *testing.T) {
+			bundle := filepath.Join(t.TempDir(), "bundle.zip")
+			args := append([]string{
+				"--target", "aws", "--source", tc.source, "--format", "bundle", "--output", bundle,
+				"--account-name", "prod", "--account-id", validAccountIDFor("aws"),
+				"--oidc-subject-claim", "123456789012345678901",
+			}, tc.extra...)
+			res := runGenerator(t, args...)
+			require.Zero(t, res.exitCode, "stderr: %s", res.stderr)
+			script := readZipEntry(t, bundle, "cloudformation/deploy-cfn.sh")
+			require.Contains(t, script, `"OIDCIssuerHost=`+tc.wantHost+`"`)
+		})
+	}
+}
+
+// readZipEntry returns the contents of the named entry in the zip at path.
+func readZipEntry(t *testing.T, path, name string) string {
+	t.Helper()
+	archive, err := zip.OpenReader(path)
+	require.NoError(t, err)
+	defer archive.Close()
+	entry, err := archive.Open(name)
+	require.NoError(t, err)
+	defer entry.Close()
+	b, err := io.ReadAll(entry)
+	require.NoError(t, err)
+	return string(b)
+}

scripts/generate_federation_iac_escape_test.go

@@ -227,11 +227,11 @@ func TestGenerator_CFParamsValuesCannotChangeStructure(t *testing.T) {
 	}, got)
 }
 
-// probeDeployScript stands in for aws-cfn-deploy.sh.tmpl. The real template
-// cannot be rendered by this script today (iacData lacks OIDCIssuerHost,
-// #153), so this probe exercises the same .sh.tmpl escaping path with the
-// quoting shapes the real Bash templates use, and prints what Bash made of
-// each value instead of calling any cloud CLI.
+// probeDeployScript stands in for aws-cfn-deploy.sh.tmpl. Running the real
+// script would call the AWS CLI, so this probe exercises the same .sh.tmpl
+// escaping path with the quoting shapes the real Bash templates use, and
+// prints what Bash made of each value instead of calling any cloud CLI.
+// TestGenerator_AWSWIFBundleRendersDeployScript renders the real template.
 const probeDeployScript = `#!/usr/bin/env bash
 # Target: {{.AccountName}} ({{.AccountExternalID}})
 set -euo pipefail

scripts/generate_federation_iac_test.go

@@ -434,13 +434,9 @@ func TestGenerator_ValidationPrecedesTemplateRead(t *testing.T) {
 }
 
 // TestGenerator_AcceptsValidOIDCSubjectClaim is the positive control: without
-// it, a validator that rejected everything would pass every test above.
-//
-// It uses --format cf-params rather than the default tfvars output because the
-// script's iacData is missing the CUDlyAPIURL / SourceAccountID / ContactEmail
-// fields the tfvars templates reference, so every tfvars path fails to render
-// on main today. That drift predates this change and is tracked in #1690; the
-// cf-params path renders end to end.
+// it, a validator that rejected everything would pass every test above. It
+// checks the cf-params output because that is where the claim is easiest to
+// read back exactly; the tfvars and bundle paths are covered elsewhere.
 func TestGenerator_AcceptsValidOIDCSubjectClaim(t *testing.T) {
 	cases := []struct {
 		name  string
@@ -482,23 +478,22 @@ func TestGenerator_AcceptsValidOIDCSubjectClaim(t *testing.T) {
 // the new requirement. target=aws with source=aws is the cross-account
 // (external ID) path: it renders aws-cross-account.tfvars.tmpl, which has no
 // OIDC trust policy at all, so demanding a subject claim there would break a
-// working combination.
-//
-// It asserts only on stderr, not on the exit code, because that invocation
-// currently fails for an unrelated pre-existing reason: iacData is missing the
-// SourceAccountID / CUDlyAPIURL / ContactEmail / OIDCIssuerHost fields the
-// tfvars and deploy-script templates reference, so no tfvars path of this
-// script renders on main today. That drift is tracked in #1690; once it is
-// fixed this can also assert exitCode == 0.
+// working combination. It asserts a successful render, not just the absence of
+// a complaint, so a regression that breaks the path outright also fails (#153).
 func TestGenerator_CrossAccountDoesNotRequireSubjectClaim(t *testing.T) {
 	res := runGenerator(t,
 		"--target", "aws",
 		"--source", "aws",
 		"--account-name", "tgt",
 		"--account-id", "999888777666",
+		"--source-account-id", "111122223333",
 		"--output", "-",
 	)
-	if strings.Contains(res.stderr, "--oidc-subject-claim") {
-		t.Errorf("the aws->aws cross-account path must not require --oidc-subject-claim, got:\n%s", res.stderr)
+	if res.exitCode != 0 {
+		t.Fatalf("the aws->aws cross-account path must render without --oidc-subject-claim (exit %d); stderr:\n%s",
+			res.exitCode, res.stderr)
+	}
+	if !strings.Contains(res.stdout, "999888777666") {
+		t.Errorf("rendered cross-account tfvars must carry the target account ID, got:\n%s", res.stdout)
 	}
 }