diff --git a/scripts/generate-federation-iac.go b/scripts/generate-federation-iac.go index 7e1852c8..aab02e98 100644 --- a/scripts/generate-federation-iac.go +++ b/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, diff --git a/scripts/generate_federation_iac_drift_test.go b/scripts/generate_federation_iac_drift_test.go new file mode 100644 index 00000000..96f456f6 --- /dev/null +++ b/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) +} diff --git a/scripts/generate_federation_iac_escape_test.go b/scripts/generate_federation_iac_escape_test.go index ab69012f..e4175d4e 100644 --- a/scripts/generate_federation_iac_escape_test.go +++ 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 diff --git a/scripts/generate_federation_iac_test.go b/scripts/generate_federation_iac_test.go index d3f4c403..69280350 100644 --- a/scripts/generate_federation_iac_test.go +++ b/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) } }