fix: make e2e localizer uses-rewrite idempotent - #143
Merged
Merged
Conversation
The harness reusable-workflow localizer prefixed any `uses:` value matching `[^/]...\.yaml` with `./`. Once the generator began emitting fully-qualified local callback paths (`uses: ./.github/workflows/build.yaml`), the leading `.` matched `[^/]` and the rewrite produced `uses: ././...`, which act cannot resolve. Tighten the capture group to a bare filename (no leading `.`, `/`, or `@` and no embedded `/`) so already-qualified local paths and cross-repo `owner/repo/...@ref` values are left unchanged while a bare `build.yaml` still gains its `./` prefix. Extract the two sed expressions to named constants and add an idempotency test that runs the real expression and asserts no `././` is ever produced. 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
1 participant
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
After the generator began emitting fully-qualified local callback paths (
uses: ./.github/workflows/build.yaml), the harness reusable-workflow localizer ine2e/harness/harness.gokept running a stalesed(s|uses: \([^/][^@]*\.yaml\)|uses: ./\1|g). Its leading[^/]matched the already-present., so it prepended a second./and produceduses: ././.github/workflows/build.yaml, which act cannot resolve.This breaks three scenarios on main:
Inline_Run_Callback(12) andInline_Job_Attributes(10) fail theiruses: ./.github/workflows/build.yamlsubstring assertion.Promote_rolls_back_a_successful_deploy_when_a_sibling_deploy_failscorrupts the reusable-deployuses:the same way, so theappdeploy/infradeployjobs never run and step 6 sees empty conclusions.Fix
Tighten the capture group to a bare filename:
s|uses: \([^./@][^/@]*\.yaml\)|uses: ./\1|g. This rewrite is now idempotent. It skips values already starting with.or/, anything containing@(cross-repoowner/repo/...@ref), and any path with an embedded/, while still localizing a barebuild.yamlto./build.yaml. The two sed expressions are extracted to named constants and the stale comment is updated.Verification (local Docker is down, so the act suite could not run)
go build ./...,cd e2e && go vet ./..., andgolangci-lint run ./...are all clean.TestUsesLocalizeSedExpr_Idempotent_LeavesQualifiedPathsUnchangedruns the exact expression and asserts no././is produced and a second pass is a no-op.uses: ././.github/workflows/build.yamland././.github/workflows/deploy-{infra,app}.yamluses: ./.github/workflows/build.yamland./.github/workflows/deploy-{infra,app}.yamlThe post-merge Orchestrate e2e run is the final confirmation. This is expected to resolve all three failing scenarios (10, 12, and the rollback scenario via the corrected reusable-deploy
uses:).