fix(backend): iterate placeholder substitution to fixpoint in launcher_v2

[vizansh]

Fixes #13977

Thanks to @ahn132 for the issue report and root cause breakdown.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[google-oss-prow]

Hi @vizansh. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[dobbydobap]

Nice catch adding the getPlaceholders error check on top of the reported bug. That one was hiding the real cause behind a confusing failed to convert ExecutorInput into JSON.

The loop needs a cap though:

for {
    prev := s
    for placeholder, replacement := range placeholders {
        s = strings.ReplaceAll(s, placeholder, replacement)
    }
    if s == prev {
        break
    }
}

It stops at a fixpoint, but a value containing its own placeholder never reaches one. Set parameter a to x{{$.inputs.parameters['a']}} and each pass re-injects the placeholder with one more x in front, so s never equals prev. Two parameters referencing each other do the same. Pipeline parameters are user supplied, so this hangs the launcher on ordinary input instead of erroring. A counted loop with a small cap keeps the nesting and drops the hang; anything unresolved after that passes through literally, same as today.

The SDK files look accidental. Removing the _KFP_RUNTIME guard is your #13896, not this fix, so the branch may have been cut from there rather than master. launcher_v2_test.go also lost its trailing newline.

On the test, it runs once over two placeholders, so map order gives it roughly a 50% pass rate against the unfixed code. Looping it makes the regression reproducible.

Worth a look at #12811 too, which resolves placeholders inside list and struct constants in the driver. If that lands, some of these nested cases would be resolved before the launcher sees them. Not a blocker, just worth settling which layer owns it.

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign chensun for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• backend/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[vizansh]

Thanks @dobbydobap for the thorough review! These were excellent points:

1. Infinite Loop Cap: Added a maxPasses cap (10) to substitutePlaceholders to prevent infinite loops in the event of self-referential or cyclic user parameters.
2. Flaky Test Fix: Updated TestCompileCmdAndArgs_NestedPlaceholders to run 100 iterations per test run to guarantee map iteration order randomization doesn't hide regressions.

Updated and force-pushed!

[vizansh]

/assign @chensun

[dobbydobap]

Cap logic is right, and dropping the SDK files cleaned it up nicely.

One thing that will bite CI though: the new substitutePlaceholders body is indented with spaces where the rest of the file uses tabs. At head 6fa2aa19b lines 1074-1085 are space-indented, while getPlaceholders right below at 1090 is tabs. .golangci.yml enables gofmt and goimports, and pre-commit runs golangci-lint with --new-from-merge-base, so it lints exactly the lines you changed. gofmt -w backend/src/v2/component/launcher_v2.go should clear it.