diff --git a/CHANGELOG.md b/CHANGELOG.md index 1df98207..cf3268b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,12 @@ A `Migration` section is added to any release that bumps `schema_version`. and a component promotion save could synthesize a phantom marker for a component that had never released +- **config:** Validate matrix dimension keys and axes at validation time: each + `matrix.dimensions` key must be a valid GitHub Actions matrix identifier + (start with a letter or underscore; letters, digits, hyphens, and + underscores only) and each axis must list at least one value, instead of an + invalid key or empty axis passing validation and failing at the first + workflow run; the manifest JSON Schema enforces the same constraints - **config:** Validate `run_policy`, `on_failure`, and `retries` on the `validate` block the same way as builds and deploys: `run_policy` and `on_failure` must be one of their documented values and `retries` must stay diff --git a/docs/public/manifest.schema.json b/docs/public/manifest.schema.json index af758080..5384f229 100644 --- a/docs/public/manifest.schema.json +++ b/docs/public/manifest.schema.json @@ -403,11 +403,13 @@ "properties": { "dimensions": { "type": "object", + "propertyNames": { "pattern": "^[A-Za-z_][A-Za-z0-9_-]*$" }, "additionalProperties": { "type": "array", - "items": { "type": "string" } + "items": { "type": "string" }, + "minItems": 1 }, - "description": "Cross-product axes (for example os, arch)." + "description": "Cross-product axes (for example os, arch). Each key must start with a letter or underscore and contain only letters, digits, hyphens, and underscores; each axis must list at least one value." }, "max_parallel": { "type": "integer", "description": "Cap on concurrent matrix legs (0 means the platform default)." }, "fail_fast": { "type": "boolean" } diff --git a/docs/src/content/docs/reference/manifest.md b/docs/src/content/docs/reference/manifest.md index 12904a03..24855f23 100644 --- a/docs/src/content/docs/reference/manifest.md +++ b/docs/src/content/docs/reference/manifest.md @@ -392,7 +392,7 @@ The build's `artifact_id` output (if declared) is captured into state automatica | Sub-field | Status | Type | Description | |-----------|--------|------|-------------| -| `dimensions` | emitted | map | The cross-product axes (for example `os: [linux, darwin]`, `arch: [amd64, arm64]`). | +| `dimensions` | emitted | map | The cross-product axes (for example `os: [linux, darwin]`, `arch: [amd64, arm64]`). Each key must start with a letter or underscore and contain only letters, digits, hyphens, and underscores (the identifier set GitHub Actions accepts for matrix keys and `matrix.` references), and each axis must list at least one value; validation rejects anything else. | | `max_parallel` | emitted | int | Caps concurrent matrix legs (0 uses the GitHub Actions default). | | `fail_fast` | emitted | bool | Whether a failing leg cancels the rest. Unset applies the GitHub Actions default (true for matrix builds). | diff --git a/e2e/scenarios/63-build-matrix.yaml b/e2e/scenarios/63-build-matrix.yaml new file mode 100644 index 00000000..73af6e9a --- /dev/null +++ b/e2e/scenarios/63-build-matrix.yaml @@ -0,0 +1,72 @@ +name: "Build Matrix Fan-Out" +description: | + A build that declares matrix dimensions must render a strategy block in the + generated orchestrate workflow: each axis as a YAML mapping key with its + value list, max-parallel and fail-fast from the manifest knobs, and the + dimension value threaded to the callback via with: as ${{ matrix. }} + when the callback declares a matching input. The hyphenated go-version axis + proves the full identifier set GitHub Actions accepts for matrix keys + (letters, digits, hyphens, underscores, starting with a letter or + underscore) round-trips through generation; validation rejects anything + outside that set up front. + + Generation-only. The runtime fan-out behavior (one job per matrix leg, + fail-fast cancellation, max-parallel throttling) is not reliably observable + in act, so this cell asserts the generated strategy block and the matrix + input threading, labeled as a ceiling rather than runtime coverage. + +config: + trunk_branch: main + environments: [dev, prod] + builds: + - name: app + workflow: build.yaml + triggers: ["src/**"] + matrix: + dimensions: + goarch: ["amd64", "arm64"] + go-version: ["1.22"] + max_parallel: 2 + fail_fast: false + +# The build callback declares the goarch input so the generated orchestrate +# workflow threads the current matrix value through with:, keeping the +# generated reusable-workflow call valid. The go-version axis is deliberately +# left undeclared to pin the silent-skip behavior for unknown inputs. +setup_workflows: + ".github/workflows/build.yaml": | + name: build + on: + workflow_call: + inputs: + environment: + required: false + type: string + goarch: + required: false + type: string + jobs: + build: + runs-on: ubuntu-latest + steps: + - run: echo "build ${{ inputs.goarch }}" + +steps: + - name: "Seed a minimal source tree; assert the orchestrate strategy block" + action: commit + commit: + message: "seed source" + files: + src/main.go: | + package main + + func main() {} + expect: + workflow_files: + - path: ".github/workflows/orchestrate.yaml" + contains: + - 'goarch: ["amd64", "arm64"]' + - 'go-version: ["1.22"]' + - "max-parallel: 2" + - "fail-fast: false" + - "goarch: ${{ matrix.goarch }}" diff --git a/internal/config/parse.go b/internal/config/parse.go index fe4ef291..51ee869d 100644 --- a/internal/config/parse.go +++ b/internal/config/parse.go @@ -232,6 +232,12 @@ func Validate(cfg *TrunkConfig) []string { // Validate run_policy, on_failure, and retries errors = append(errors, validateCallbackPolicy(fmt.Sprintf("builds[%d]", i), b.RunPolicy, b.OnFailure, b.Retries)...) + // Matrix dimension keys are emitted raw as YAML mapping keys and inside + // ${{ matrix. }} expressions; enforce the GHA matrix identifier + // grammar and non-empty axes here so generation cannot emit a workflow + // that fails at the first run. + errors = append(errors, validateMatrix(fmt.Sprintf("builds[%d]", i), b.Matrix)...) + // Validate depends_on references using ResolveDependency for _, dep := range b.DependsOn { if _, err := cfg.ResolveDependency(dep, CallbackTypeBuild); err != nil { diff --git a/internal/config/validate_matrix_test.go b/internal/config/validate_matrix_test.go new file mode 100644 index 00000000..a3d46ec1 --- /dev/null +++ b/internal/config/validate_matrix_test.go @@ -0,0 +1,134 @@ +package config + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +// matrixBaseConfig is a minimal valid manifest with one build, used to isolate +// the matrix dimension validation rules. +func matrixBaseConfig() *TrunkConfig { + return &TrunkConfig{ + TrunkBranch: "main", + Environments: EnvNames("dev", "prod"), + Builds: []BuildConfig{ + {Name: "app", Workflow: ".github/workflows/build.yaml"}, + }, + } +} + +// TestValidate_MatrixDimensions covers the matrix dimension key and axis +// rules. Dimension keys are emitted raw as YAML mapping keys +// (strategy.matrix.), as with: input keys, and inside +// ${{ matrix. }} expressions, so a key GitHub Actions cannot parse must +// be rejected at validation time instead of at the first workflow run. GitHub +// accepts a matrix key that starts with a letter or underscore and contains +// only letters, digits, hyphens, and underscores; hyphens are valid in both a +// matrix axis name and a matrix. context dereference. An axis with no +// values fails at run time with an empty matrix vector, so it is likewise +// rejected up front. +func TestValidate_MatrixDimensions(t *testing.T) { + tests := []struct { + name string + dimensions map[string][]string + wantErrs bool + wantSubstr []string + }{ + { + name: "key with a space rejected", + dimensions: map[string][]string{"go version": {"1.22"}}, + wantErrs: true, + wantSubstr: []string{"go version", "matrix.dimensions"}, + }, + { + name: "key with a leading digit rejected", + dimensions: map[string][]string{"2fast": {"a"}}, + wantErrs: true, + wantSubstr: []string{"2fast", "matrix.dimensions"}, + }, + { + name: "key with a leading hyphen rejected", + dimensions: map[string][]string{"-os": {"linux"}}, + wantErrs: true, + wantSubstr: []string{"-os", "matrix.dimensions"}, + }, + { + name: "key with a colon rejected", + dimensions: map[string][]string{"os: x": {"linux"}}, + wantErrs: true, + wantSubstr: []string{"matrix.dimensions"}, + }, + { + name: "key with a dot rejected", + dimensions: map[string][]string{"go.version": {"1.22"}}, + wantErrs: true, + wantSubstr: []string{"go.version", "matrix.dimensions"}, + }, + { + name: "key with a newline rejected", + dimensions: map[string][]string{"os\nx": {"linux"}}, + wantErrs: true, + wantSubstr: []string{"matrix.dimensions"}, + }, + { + name: "empty key rejected", + dimensions: map[string][]string{"": {"linux"}}, + wantErrs: true, + wantSubstr: []string{"matrix.dimensions"}, + }, + { + name: "empty axis rejected", + dimensions: map[string][]string{"os": {}}, + wantErrs: true, + wantSubstr: []string{"os", "at least one value"}, + }, + { + name: "valid identifier keys pass", + dimensions: map[string][]string{ + "os": {"ubuntu-22.04", "macos-14"}, + "go_version": {"1.22"}, + "_arch": {"amd64"}, + }, + wantErrs: false, + }, + { + // GitHub allows hyphens in a matrix axis name and in the + // matrix. dot dereference (for example matrix.node-version), + // so a hyphenated key must keep validating. + name: "hyphenated key passes", + dimensions: map[string][]string{"go-version": {"1.22", "1.23"}}, + wantErrs: false, + }, + { + name: "empty dimensions map passes", + dimensions: map[string][]string{}, + wantErrs: false, + }, + { + name: "nil matrix passes", + dimensions: nil, + wantErrs: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := matrixBaseConfig() + if tt.dimensions != nil || tt.name == "empty dimensions map passes" { + cfg.Builds[0].Matrix = &MatrixConfig{Dimensions: tt.dimensions} + } + errs := Validate(cfg) + if !tt.wantErrs { + assert.Empty(t, errs, "expected no validation errors, got %v", errs) + return + } + assert.NotEmpty(t, errs, "expected validation errors") + joined := strings.Join(errs, "\n") + for _, substr := range tt.wantSubstr { + assert.Contains(t, joined, substr) + } + }) + } +} diff --git a/internal/config/validate_v1.go b/internal/config/validate_v1.go index df1b3899..b1e4fe73 100644 --- a/internal/config/validate_v1.go +++ b/internal/config/validate_v1.go @@ -143,6 +143,49 @@ func validateOutputKeyCollisions(section string, names []string) []string { return errs } +// matrixDimensionKeyRe matches a matrix dimension key GitHub Actions accepts +// both as a matrix axis name and as a ${{ matrix. }} context dereference: +// it must start with a letter or underscore and contain only letters, digits, +// hyphens, and underscores. The generator emits the key raw as a YAML mapping +// key under strategy.matrix, as a with: input key, and inside a +// ${{ matrix. }} expression, so a key outside this set either fails +// GitHub's workflow parse (a space, a leading digit) or silently restructures +// the emitted YAML (a colon, a "#", a newline). Hyphens stay allowed because +// GitHub permits them in axis names and dot dereferences (matrix.node-version). +var matrixDimensionKeyRe = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_-]*$`) + +// validateMatrix checks a build's matrix block: every dimension key must be a +// valid GitHub Actions matrix identifier and every axis must list at least one +// value. Both failures otherwise validate clean and surface only at the first +// workflow run (parse rejection for a bad key, "matrix vector does not contain +// any values" for an empty axis), so they are rejected at validation time like +// every other emitted identifier. Keys are rejected, not sanitized, for the +// same reason validateJobIDSafeName rejects: rewriting could collapse two +// distinct axes. A nil matrix or an empty dimensions map is valid (no fan-out). +func validateMatrix(prefix string, m *MatrixConfig) []string { + if m == nil { + return nil + } + var errs []string + keys := make([]string, 0, len(m.Dimensions)) + for k := range m.Dimensions { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + if !matrixDimensionKeyRe.MatchString(k) { + errs = append(errs, fmt.Sprintf( + "%s.matrix.dimensions key %q must start with a letter or underscore and contain only letters, digits, hyphens, and underscores", + prefix, k)) + } + if len(m.Dimensions[k]) == 0 { + errs = append(errs, fmt.Sprintf( + "%s.matrix.dimensions[%q] must list at least one value", prefix, k)) + } + } + return errs +} + // validateWorkflowRunXOR enforces that callbacks are reusable-workflow only. // Inline run: and shell: are no longer supported, so each is rejected with an // actionable error, and workflow: is required. diff --git a/internal/schema/manifest.schema.json b/internal/schema/manifest.schema.json index af758080..5384f229 100644 --- a/internal/schema/manifest.schema.json +++ b/internal/schema/manifest.schema.json @@ -403,11 +403,13 @@ "properties": { "dimensions": { "type": "object", + "propertyNames": { "pattern": "^[A-Za-z_][A-Za-z0-9_-]*$" }, "additionalProperties": { "type": "array", - "items": { "type": "string" } + "items": { "type": "string" }, + "minItems": 1 }, - "description": "Cross-product axes (for example os, arch)." + "description": "Cross-product axes (for example os, arch). Each key must start with a letter or underscore and contain only letters, digits, hyphens, and underscores; each axis must list at least one value." }, "max_parallel": { "type": "integer", "description": "Cap on concurrent matrix legs (0 means the platform default)." }, "fail_fast": { "type": "boolean" } diff --git a/schema/manifest.schema.json b/schema/manifest.schema.json index af758080..5384f229 100644 --- a/schema/manifest.schema.json +++ b/schema/manifest.schema.json @@ -403,11 +403,13 @@ "properties": { "dimensions": { "type": "object", + "propertyNames": { "pattern": "^[A-Za-z_][A-Za-z0-9_-]*$" }, "additionalProperties": { "type": "array", - "items": { "type": "string" } + "items": { "type": "string" }, + "minItems": 1 }, - "description": "Cross-product axes (for example os, arch)." + "description": "Cross-product axes (for example os, arch). Each key must start with a letter or underscore and contain only letters, digits, hyphens, and underscores; each axis must list at least one value." }, "max_parallel": { "type": "integer", "description": "Cap on concurrent matrix legs (0 means the platform default)." }, "fail_fast": { "type": "boolean" }