Incorrect templating should result in error

[bobcatfish]

Expected Behavior

Whenever a user uses ${} templating syntax in their Task, if the value within the {} can't be resolved, an error should be returned to the user and the execution of any related TaskRuns or PipelineRuns should stop.

Actual Behavior

With these output resources:

  outputs:
    resources:
    - name: bucket
      type: storage
    - name: builtBaseImage
      type: image
    - name: builtEntrypointImage
      type: image
    - name: builtKubeconfigWriterImage
      type: image
    - name: builtCredsInitImage
      type: image
    - name: builtGitInitImage
      type: image
    - name: builtNopImage
      type: image
    - name: builtBashImage
      type: image
    - name: builtGsutilImage
      type: image
    - name: builtControllerImage
      type: image
    - name: builtWebhookImage
      type: image

I tried to use this within one of my steps:

        ${inputs.params.pathToProject}/${outputs.resources.builtCredsInitImage}: ${inputs.params.imageRegistry}/${inputs.params.pathToProject}/build-base:latest
        ${inputs.params.pathToProject}/${outputs.resources.builtGitInitImage}: ${inputs.params.imageRegistry}/${inputs.params.pathToProject}/build-base:latest

Values like ${outputs.resources.builtCredsInitImage} are incomplete b/c I actually should have been using attributes on the resource, e.g. ${outputs.resources.builtCredsInitImage.url}.

Instead of getting an error, the pod was created and tried to execute.

Steps to Reproduce the Problem

Additional Info

side note: how does a user escape the templating, if they need to literally use ${} in their Task?

[mikeykhalil]

I was going to try to fix this issue, if nobody has jumped on this yet. Just getting my dev environment setup, but will report back once I make some progress.

I don't plan on tackling the problem mentioned in the Additional Info portion, though. That does seem like something that needs to be addressed. It almost feels like a separate issue though, if I understand it correctly.

[mikeykhalil]

Haven't had a ton of time to look at this. I added a few tests to pipeline/pkg/templating/templating_test.go based off this issue. I would have expected these new tests to fail, but they passed without any source code changes. So I'm guessing that if the issue is still present, it's further up in the call stack and is just a matter of handling the error from the templating package properly.

Anyways, I will try to find some time tomorrow to take a deeper look.

[mikeykhalil]

This issue ended up being a bit more complex than we originally had thought.

Seems like there are some more involved design decisions that would have to be made in order to correctly validate variables at task creation time.

Feel free to follow the discussion on this PR for more details:

#765

[vdemeester]

@bobcatfish is it still valid ? I feel with the move to $() we now do more validation ?

[bobcatfish]

I'm not sure we do more validation @vdemeester but we are certainly relying on pipelineresources less, and i think pipelineresources are at the heart of the problem here, b/c you don't know what values are available for substitution until runtime. Feels safe to me to close this given #1673