From 3e1ff268c859d0d1d5a1a85667dcbe997c78366c Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Tue, 16 Jun 2026 12:25:00 -0400 Subject: [PATCH] fix: consolidate matrix build passthrough artifacts before upload A matrix build's cascade-owned -upload post-job ran upload-artifact on a fresh runner that never held the matrix legs' files, so the upload found an empty path, produced no build- artifact, and a downstream consumer's download failed with "Artifact not found". The post-job now collects the per-leg artifacts first when the build declares a matrix. cascade cannot inject upload steps into a reusable callback's legs, so each leg is expected to upload an artifact named -; the post-job downloads pattern -* with merge-multiple into the upload directory, then uploads the single consolidated build-. Non-matrix builds keep uploading directly. Signed-off-by: Joshua Temple --- internal/generate/generator.go | 48 ++++++++++ internal/generate/generator_test.go | 135 ++++++++++++++++++++++++++++ 2 files changed, 183 insertions(+) diff --git a/internal/generate/generator.go b/internal/generate/generator.go index 6858c8ab..e96b8aa0 100644 --- a/internal/generate/generator.go +++ b/internal/generate/generator.go @@ -1876,14 +1876,51 @@ func (g *Generator) writePassthroughDownloadJob(sb *strings.Builder, info Callba } } +// passthroughLegPattern returns the download-artifact pattern that a matrix +// build's per-leg artifacts must match to be collected into the consolidated +// build-{name} upload. cascade cannot inject upload steps into a reusable +// callback's matrix legs, so each leg is expected to upload an artifact named +// "{build-name}-{leg-suffix}" (e.g. image-linux-amd64 for a build named +// "image"). The "{build-name}-*" pattern collects every leg deterministically +// without hardcoding any dimension values. +func passthroughLegPattern(buildName string) string { + return fmt.Sprintf("%s-*", buildName) +} + +// passthroughUploadDir converts an upload path into a directory suitable as the +// download-artifact destination. download-artifact writes into a directory, so +// a trailing recursive glob ("dist/**", "dist/", "dist") is reduced to the +// base directory ("dist"). A bare "**" (upload the whole workspace) maps to "." +// so the merged legs land back at the workspace root. +func passthroughUploadDir(upload string) string { + dir := strings.TrimSuffix(upload, "**") + dir = strings.TrimSuffix(dir, "/") + if dir == "" { + return "." + } + return dir +} + // writePassthroughUploadJob emits a cascade-owned post-job that runs // actions/upload-artifact after info's reusable-workflow callback completes. // The artifact is named "build-{job-name}". // Used for reusable-workflow callbacks where steps cannot be injected into the // jobs..uses block. +// +// For matrix builds the callback fans out across legs that each run on their +// own runner, so the upload-path files only ever exist on those leg runners, +// never on this fresh post-job runner. Uploading directly here would find an +// empty path and produce no artifact, and a downstream consumer's download then +// fails with "Artifact not found". To consolidate, this job first downloads the +// per-leg artifacts (which the legs are expected to upload under the +// "{build-name}-*" convention; see passthroughLegPattern), merges them into the +// upload directory, and only then uploads the single consolidated build-{name} +// artifact consumers download. Non-matrix builds run on one runner whose files +// already live at the upload path, so they upload directly with no collect step. func (g *Generator) writePassthroughUploadJob(sb *strings.Builder, info CallbackInfo) { postJobID := fmt.Sprintf("%s-upload", info.JobID) name := passthroughArtifactName(info.Name) + isMatrix := info.Matrix != nil && len(info.Matrix.Dimensions) > 0 fmt.Fprintf(sb, " %s:\n", postJobID) fmt.Fprintf(sb, " name: Upload artifact %s\n", name) fmt.Fprintf(sb, " needs: [%s]\n", info.JobID) @@ -1891,6 +1928,17 @@ func (g *Generator) writePassthroughUploadJob(sb *strings.Builder, info Callback sb.WriteString(" runs-on: ubuntu-latest\n") g.writeOwnedTimeout(sb, " ") sb.WriteString(" steps:\n") + if isMatrix { + // Collect the per-leg artifacts before consolidating. The legs upload + // "{build-name}-*" artifacts; merge them into the upload directory. + dir := passthroughUploadDir(info.PassthroughArtifact.Upload) + sb.WriteString(" - name: Collect matrix leg artifacts\n") + writeActionUses(sb, g.config, " ", actionDownloadArtifact) + sb.WriteString(" with:\n") + fmt.Fprintf(sb, " pattern: %s\n", passthroughLegPattern(info.Name)) + fmt.Fprintf(sb, " path: %s\n", dir) + sb.WriteString(" merge-multiple: true\n") + } fmt.Fprintf(sb, " - name: Upload artifact %s\n", name) writeActionUses(sb, g.config, " ", actionUploadArtifact) sb.WriteString(" with:\n") diff --git a/internal/generate/generator_test.go b/internal/generate/generator_test.go index 56bff2af..346c4361 100644 --- a/internal/generate/generator_test.go +++ b/internal/generate/generator_test.go @@ -3,6 +3,7 @@ package generate import ( "os" "path/filepath" + "regexp" "strings" "testing" @@ -2024,6 +2025,140 @@ func TestGenerator_PassthroughArtifact_DownloadNeedsUploadJob(t *testing.T) { "download pre-job must depend on the producer's -upload job, otherwise it races the upload and fails with 'Artifact not found'") } +// uploadJobBody returns the text of the " :\n" block from a generated +// workflow, spanning until the next two-space-indented job header (or EOF). It +// lets a test assert on the step ordering inside a single owned job. +func uploadJobBody(t *testing.T, workflow, jobID string) string { + t.Helper() + header := "\n " + jobID + ":\n" + idx := strings.Index(workflow, header) + require.NotEqual(t, -1, idx, "job %q not found in generated workflow", jobID) + start := idx + len(header) + rest := workflow[start:] + // Find the next job header at two-space indentation (a line " word:"). + re := regexp.MustCompile(`(?m)^ [A-Za-z0-9_-]+:\n`) + if loc := re.FindStringIndex(rest); loc != nil { + return rest[:loc[0]] + } + return rest +} + +// TestGenerator_PassthroughArtifact_MatrixUploadCollectsLegs asserts that a +// matrix build's cascade-owned -upload post-job collects the per-leg artifacts +// before consolidating them into the single build- artifact consumers +// download. cascade cannot inject upload steps into the reusable callback's +// matrix legs, so each leg uploads a per-leg artifact following the +// "-*" convention, and the upload post-job downloads that pattern +// (merge-multiple) into the upload path, then uploads the consolidated +// build-. The manifest mirrors 2env: a matrix "image" build feeding a +// "bundle" consumer, where the legs upload image--. +func TestGenerator_PassthroughArtifact_MatrixUploadCollectsLegs(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".github/workflows"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, ".github/workflows/build-image.yaml"), []byte("on:\n workflow_call:\n"), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, ".github/workflows/build-bundle.yaml"), []byte("on:\n workflow_call:\n"), 0644)) + + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: []string{"staging"}, + Builds: []config.BuildConfig{ + { + Name: "image", + Workflow: ".github/workflows/build-image.yaml", + Triggers: []string{"src/**"}, + Matrix: &config.MatrixConfig{ + Dimensions: map[string][]string{ + "os": {"linux"}, + "arch": {"amd64", "arm64"}, + }, + }, + PassthroughArtifact: &config.PassthroughArtifact{ + Upload: "dist/**", + }, + }, + { + Name: "bundle", + Workflow: ".github/workflows/build-bundle.yaml", + Triggers: []string{"src/**"}, + PassthroughArtifact: &config.PassthroughArtifact{ + Downloads: []string{"image"}, + }, + }, + }, + } + + gen := NewGenerator(cfg, tmpDir) + result, err := gen.Generate() + require.NoError(t, err) + + body := uploadJobBody(t, result, "build-image-upload") + + // The post-job must collect the per-leg artifacts first. + assert.Contains(t, body, "uses: actions/download-artifact@v4", + "matrix upload post-job must download per-leg artifacts before uploading") + assert.Contains(t, body, "pattern: image-*", + "collect step must use the -* convention pattern") + assert.Contains(t, body, "merge-multiple: true", + "collect step must merge per-leg artifacts into one path") + assert.Contains(t, body, "path: dist", + "collect step must download into the upload directory (glob stripped)") + + // The collect step must precede the upload step. + collectIdx := strings.Index(body, "actions/download-artifact") + uploadIdx := strings.Index(body, "actions/upload-artifact") + require.NotEqual(t, -1, collectIdx) + require.NotEqual(t, -1, uploadIdx) + assert.Less(t, collectIdx, uploadIdx, + "collect (download) step must come before the consolidating upload step") + + // The consolidation still uploads build-image, and the bundle download + // references the same name: end-to-end name consistency. + assert.Contains(t, body, "name: build-image", + "consolidation must upload the build- artifact consumers download") + assert.Contains(t, result, "build-bundle-download:", + "consumer must emit its download pre-job") + + downloadBody := uploadJobBody(t, result, "build-bundle-download") + assert.Contains(t, downloadBody, "name: build-image", + "bundle download must reference the consolidated build-image artifact") +} + +// TestGenerator_PassthroughArtifact_NonMatrixUploadNoCollect asserts that a +// non-matrix build's -upload post-job uploads directly with no collect step, +// since its callback runs on a single runner whose artifacts already live at +// the upload path. A spurious download-artifact in a non-matrix upload job +// would look for per-leg artifacts that never exist. +func TestGenerator_PassthroughArtifact_NonMatrixUploadNoCollect(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".github/workflows"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, ".github/workflows/build.yaml"), []byte("on:\n workflow_call:\n"), 0644)) + + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: []string{"dev"}, + Builds: []config.BuildConfig{ + { + Name: "compile", + Workflow: ".github/workflows/build.yaml", + Triggers: []string{"src/**"}, + PassthroughArtifact: &config.PassthroughArtifact{ + Upload: "dist/", + }, + }, + }, + } + + gen := NewGenerator(cfg, tmpDir) + result, err := gen.Generate() + require.NoError(t, err) + + body := uploadJobBody(t, result, "build-compile-upload") + assert.NotContains(t, body, "actions/download-artifact", + "non-matrix upload job must not emit a per-leg collect step") + assert.Contains(t, body, "uses: actions/upload-artifact@v4", + "non-matrix upload job must still upload directly") +} + // TestGenerator_DispatchInputs_StringType asserts that a string dispatch_input // is emitted correctly in the workflow_dispatch.inputs block. func TestGenerator_DispatchInputs_StringType(t *testing.T) {