fix: resolve cross-repo reusable-workflow callbacks without local read - #172
Merged
Merged
Conversation
Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Generator.discoverOutputsAndInputsread every callback's workflow file from local disk. For a cross-repo callback whoseworkflow:isorg/repo/.github/workflows/file.yaml@ref,normalizeWorkflowPathpasses the@refthrough unchanged, sofilepath.Joinproduces a bogus local path ending in a literal@refandos.ReadFilehard-fails, aborting generation. This blocked generating any manifest that calls a reusable workflow in another repository.stablekernel/cascade-example-primaryhits this exactly viabuilds[].sharedlib.workflow: stablekernel/cascade-example-artifact-a/.github/workflows/build-shared.yaml@main.Fix
In
discoverOutputsAndInputs, detect cross-repo refs with the existingconfig.IsExternalWorkflowhelper and skip the local read for them. Instead of parsing the unreachable file, seed the callback's contract surface:environment,sha,dry_run) plus any operator-declared manifest inputs (inputs:/env_inputs:), sowriteWithInputsstill emits the correctwith:wiring.artifact_idoutput, so state capture and downstream chaining keep working.Correctness of the generated
with:blockThe caller job for the cross-repo callback still carries the full callback-contract
with:block. Verified against the live committedorchestrate.yamlincascade-example-primary: the regeneratedbuild-sharedlibjob is byte-identical inuses:andwith:(environment+sha), and theartifact_idoutput wiring (state capture, finalize summary, downstream env) matches as well. The build-shared callback's optionalversioninput is not passed because the primary manifest declares no value for it (the workflow defaults it), matching the committed output; when an operator does declare such an input, it passes through (covered by a test).Verification
go build ./...,go test ./...(1347 pass),go vet ./...,golangci-lint run ./...: all green.go test -race ./internal/generate/: clean. e2e module:go build ./...andgo vet ./...clean.reading workflow stablekernel/cascade-example-artifact-a/.github/workflows/build-shared.yaml@main: ... no such file or directory.uses: stablekernel/cascade-example-artifact-a/.github/workflows/build-shared.yaml@mainwithwith: { environment, sha }, equivalent to the committedorchestrate.yaml.actionlinton the regenerated orchestrate: clean for the cross-repo job (only pre-existing SC2129 style suggestions in cascade-owned finalize scripts, also present in the committed file).Tests
Generator-level (
internal/generate/cross_repo_callback_test.go):TestCrossRepoBuildCallback_GeneratesWithoutLocalRead: feeds a manifest shaped like primary's (local + cross-repo@refbuild), asserts generation no longer errors, the caller job uses the external workflow verbatim, thewith:block carries the contract inputs, andartifact_idflows downstream. Written first as a failing test reproducing the exact local-read error.TestCrossRepoCallback_OperatorInputsPassThrough: asserts operator-declared manifest inputs still reach the cross-repo caller.End-to-end (
e2e/scenarios/21-cross-repo-callback.yaml):builds[]entry whoseworkflow:is a cross-repo@ref, asserting the generatedorchestrate.yamlcontains theuses: ...@refcaller, itsenvironment/shawith:lines, and the downstreamneeds.build-sharedlib.outputs.artifact_idwiring (with anot_containsguard that the literal@refnever leaks into a job id). The harness intentionally does not seed a local stub for cross-repo refs (normalizeCallbackStubPathskips@ref), so the scenario runs against the same no-local-file conditions the live fleet hits. This uses the harness's staticworkflow_filesassertion mode (as in scenarios 14 and 17) and does not require act to execute the cross-repo call. The scenario was verified to parse and discover viaDiscoverMultiStepScenarios; the full act/Docker run is left to CI.