Fix parameter substitution for defaults referencing other params

[vdemeester]

Changes

This fixes issue #8862 where parameter default values that reference
other parameters (e.g., default: $(params.fallback-param)) were not
being properly substituted when using local pipeline resolution.

• Added logic to handle parameter dependency chains by resolving in
  multiple passes until no more substitutions are needed
• Added three test cases covering all scenarios from issue Local Resolver - param substitution of default value inherited from another param #8862

Co-Authored-By: Claude noreply@anthropic.com
Signed-off-by: Vincent Demeester vdemeest@redhat.com

Fixes #8862

/kind bug

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

• Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
• Has Tests included if any functionality added or changed
• pre-commit Passed
• Follows the commit message standard
• Meets the Tekton contributor standards (including functionality, content, code)
• Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
• Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
• Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

Fixes parameters referencing other parameters through defaults.

[tekton-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please ask for approval from vdemeester after the PR has been reviewed.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[vdemeester]

/retest

[vdemeester]

/retest

[twoGiants]

/cc @twoGiants

[twoGiants]

Thanks for the fix! 😸 👍

It does work for the new test cases which were introduced but breaks for some others.

Unfortunately the current (and former) implementation has a few issues. In general I had difficulties to understand this algorithm but I figured it out after a while and re-worked it. Will open a PR in the next days and link here.

The function ApplyParameters should return an error in quite a few cases, but there is no error handling implemented. Here are the bugs I found:

1. Circular dependencies not handled.
2. Reverse declaration order not handled.
3. Self-reference not handled.
4. Referencing non existent parameter not handled.

It looks like the issues were there before.

More detail with examples follows.

P.S.: Here is my commit in a branch with the current re-work. It's a bit more code, but it's easier to grasp what happens, has error handling and the algorithm has O(n) complexity. I added a few comments with examples in the critical places. It's mostly done. The recursion depth limit will be implemented and then I open a PR.

[twoGiants]

...

It looks like the issues were there before.

More detail with examples follows.

... and then I open a PR.

Alright, I've opened a PR (#9271) with a re-write. It also fixes the issue @valAndre07 was having and the others I found. Check out the issue (#9270 ) I created and the PR description for more info.

@valAndre07 you can check out the PR branch locally, build Tekton, deploy on your cluster and test it manually if it works as expected. Such a manual test would be much appreciated.

@vdemeester I've added the three test cases from this branch and they pass and I am also manually testing it. Will write the results in my PR. I think we can close this PR. Wdyt?