Pipeline: Tekton variable expansion seems to have issues.

[n3wscott]

Found two things:

Given,

params:
    - name: FOO
      value: foo
    - name: FOOBAR
      value: $(params.FOO)-bar
    - name: FOOBARBAZ
      value: $(params.FOOBAR)-baz

1. Templates that have nested refs do not fully resolve.

ie, the value of FOOBARBAZ will not fully resolve some times, and will be $(params.FOOBAR)-baz

1. Order seems to matter. Sometimes a run will have the value of FOOBAR as foo-bar and other times it will be $(params.FOO)-bar like params.FOO has not been processed yet.

[skaegi]

/assign

[vdemeester]

Thanks @n3wscott.

The first one is definitely something we do not support at that point of time.
The second seems to be a bug as it should work.

Few questions:

• which version of tekton are you using ? of the api (I guess v1alpha1) ?
• is this only on Pipeline / PipelineRun objects or also on Task/TaskRun ?

/kind bug
/kind feature

[n3wscott]

I am using the latest release yaml, v1alpha1

It seems to be easer to hit when using Pipelines, but I can get it to happen with Task too but less.

[vdemeester]

@n3wscott can you share your yamls to reproduce it ?

[n3wscott]

yeah, I have reworked this a few times, but an older stapshot: https://github.com/n3wscott/chatbot/commit/67ac3c226fbfdc09e171ffb28a95c31f2a3040c4#diff-34cd834803c97715e915dd79c582e49cR54

and a workaround example: https://github.com/n3wscott/chatbot/blob/master/runs/release-0.13/run-update-toml-deps-sample-controller.yaml#L36

[afrittoli]

Thanks @n3wscott.

The first one is definitely something we do not support at that point of time.
The second seems to be a bug as it should work.

Few questions:

• which version of tekton are you using ? of the api (I guess v1alpha1) ?
• is this only on Pipeline / PipelineRun objects or also on Task/TaskRun ?

/kind bug
/kind feature

Having the ability of doing in place modifications of params reduces the need for extra steps / tasks and increases the re-usability of tasks. I didn't know we supported even string interpolation (not-nested), that's good to know.

[afrittoli]

@skaegi Is this something you're working on as part of the work on parameters that you're doing?

[dlorenc]

So there are two issues here. We process strings one at a time here: https://github.com/tektoncd/pipeline/blob/master/pkg/substitution/substitution.go#L120

So if one parameter contains a nested reference, it may get expanded twice, depending on the order.

The other issue is that the order is non-deterministic. We iterate over an unsorted map[string]string.

I can think of a couple options here:

1. Sort the map to guarantee stability. It's still confusing because sometimes things will be expanded and other times they won't depending on order.
2. Disallow nested references all together through validation. This will be tricky - think of something like:

  input: '$(foo)$(bar)'
  replacements:
  foo => $(baz
  bar => )
  baz => bat

1. Fully embrace nested expansion. Repeatedly apply substitutions in all fields until the result is stable. This is powerful but still a little confusing since order could still matter, but we can at least stabilize the order of expansion
2. Change the algorithm to only expand once in a single pass, preventing nested expansions completely.

I'm leaning toward the final option here.

[dlorenc]

Sent a PR here for the final option: https://github.com/tektoncd/pipeline/pull/3024

[dlorenc]

Another challenge to supporting nested expansion: cycles.

foo => bar
bar => foo

How many times would we expand?

[dlorenc]

This was fixed for now in #3024. If we decide to revisit and do sequential expansion we should file a new feature request.