Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Task may have uninterpolated variables in BuildSpec #212

Closed
tanner-bruce opened this issue Nov 1, 2018 · 6 comments
Closed

Task may have uninterpolated variables in BuildSpec #212

tanner-bruce opened this issue Nov 1, 2018 · 6 comments
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.

Comments

@tanner-bruce
Copy link

Expected Behavior

A Task would fail validation if it tries to define a BuildSpec that has variables which would not be interpolated due to missing inputs or outputs. Having this error on validation would shorten the loop when trying to write a Pipeline as errors with Tasks would be caught much sooner.

Actual Behavior

The Task will create the Build with the uninterpolated variable

Steps to Reproduce the Problem

  1. Add a variable ${inputs.params.thisdoesnotexist} to part of a BuildSpec
@bobcatfish
Copy link
Collaborator

Good call, thanks for opening this @tanner-bruce !

@bobcatfish bobcatfish added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels Nov 2, 2018
bobcatfish added a commit to bobcatfish/pipeline that referenced this issue Nov 2, 2018
While talking with @jonjohnsonjr about tektoncd#216 I wanted to show him an
example of a Pipeline wherein there are Tasks that output Images
Resources, which are then used as inputs for subsequent Tasks. Our
examples Pipelines were meant to do that, but unfortunately when I
looked at them, I realized these were not actually hooked up properly.
The Tasks were not actually using the Images that were built by previous
Tasks, so I have updated them to do that.

I also discovered that the templating was incorrect (#108 - adding tests
for these examples - and tektoncd#212 - making sure templated variables are
actually used - can't come quickly enough!) so I've updated that in the
examples as well.
@bobcatfish bobcatfish added this to the 0.0.1 Alpha release milestone Nov 3, 2018
@abayer
Copy link
Contributor

abayer commented Nov 5, 2018

So, if I'm understanding correctly (which is always an open question!), this would entail traversing the BuildSpec to find any fields in it that have ${input.params.whatever} and erroring for any cases where whatever isn't a defined parameter?

@tanner-bruce
Copy link
Author

@abayer Yup. I wrote a quick PoC in pkg/apis/pipeline/v1alpha1/task_validation.go last night doing exactly that. I'm going to clean it up later tonight and make a PR.

${inputs.resources.whatever} also needs to be considered

/assign tanner-bruce

@abayer
Copy link
Contributor

abayer commented Nov 5, 2018

@tanner-bruce - gotcha! Will watch for your PR and won't bother trying to do it myself. =)

bobcatfish added a commit to bobcatfish/pipeline that referenced this issue Nov 7, 2018
While talking with @jonjohnsonjr about tektoncd#216 I wanted to show him an
example of a Pipeline wherein there are Tasks that output Images
Resources, which are then used as inputs for subsequent Tasks. Our
examples Pipelines were meant to do that, but unfortunately when I
looked at them, I realized these were not actually hooked up properly.
The Tasks were not actually using the Images that were built by previous
Tasks, so I have updated them to do that.

I also discovered that the templating was incorrect (#108 - adding tests
for these examples - and tektoncd#212 - making sure templated variables are
actually used - can't come quickly enough!) so I've updated that in the
examples as well.
bobcatfish added a commit to bobcatfish/pipeline that referenced this issue Nov 7, 2018
While talking with @jonjohnsonjr about tektoncd#216 I wanted to show him an
example of a Pipeline wherein there are Tasks that output Images
Resources, which are then used as inputs for subsequent Tasks. Our
examples Pipelines were meant to do that, but unfortunately when I
looked at them, I realized these were not actually hooked up properly.
The Tasks were not actually using the Images that were built by previous
Tasks, so I have updated them to do that.

I also discovered that the templating was incorrect (#108 - adding tests
for these examples - and tektoncd#212 - making sure templated variables are
actually used - can't come quickly enough!) so I've updated that in the
examples as well.
bobcatfish added a commit to bobcatfish/pipeline that referenced this issue Nov 7, 2018
While talking with @jonjohnsonjr about tektoncd#216 I wanted to show him an
example of a Pipeline wherein there are Tasks that output Images
Resources, which are then used as inputs for subsequent Tasks. Our
examples Pipelines were meant to do that, but unfortunately when I
looked at them, I realized these were not actually hooked up properly.
The Tasks were not actually using the Images that were built by previous
Tasks, so I have updated them to do that.

I also discovered that the templating was incorrect (#108 - adding tests
for these examples - and tektoncd#212 - making sure templated variables are
actually used - can't come quickly enough!) so I've updated that in the
examples as well.
knative-prow-robot pushed a commit that referenced this issue Nov 7, 2018
While talking with @jonjohnsonjr about #216 I wanted to show him an
example of a Pipeline wherein there are Tasks that output Images
Resources, which are then used as inputs for subsequent Tasks. Our
examples Pipelines were meant to do that, but unfortunately when I
looked at them, I realized these were not actually hooked up properly.
The Tasks were not actually using the Images that were built by previous
Tasks, so I have updated them to do that.

I also discovered that the templating was incorrect (#108 - adding tests
for these examples - and #212 - making sure templated variables are
actually used - can't come quickly enough!) so I've updated that in the
examples as well.
@abayer
Copy link
Contributor

abayer commented Nov 27, 2018

@tanner-bruce - just checking if you're still going to open that PR - I'm trying to find a small chunk of work to use as my first code contribution and wanna make sure this isn't a good option. =)

@vdemeester
Copy link
Member

/assign

@tanner-bruce as you didn't open a PR and it staled I took a bit at it 👼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.
Projects
None yet
Development

No branches or pull requests

4 participants