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

Pipeline resource validation does not account for Condition resources #1263

Closed
afrittoli opened this issue Sep 2, 2019 · 4 comments · Fixed by #1270
Closed

Pipeline resource validation does not account for Condition resources #1263

afrittoli opened this issue Sep 2, 2019 · 4 comments · Fixed by #1270
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@afrittoli
Copy link
Member

Expected Behavior

It should be possible to have resources only used by a Condition. For instance the following pipeline should be valid:

apiVersion: tekton.dev/v1alpha1
kind: Pipeline
spec:
  resources:
    - name: src
      type: git
  tasks:
  - name: source-to-image-task
    taskRef:
      name: taskWithNoResources
    conditions:
      - conditionRef: conditionWithResource
        resources:
          - name: workspace
            resource: src

Actual Behavior

Validation of the pipeline fails:

Error from server (InternalError): error when creating "example.yaml": Internal error occurred: admission webhook "webhook.tekton.dev" denied the request: mutation failed: invalid value: Pipeline declared resources didn't match usage in Tasks: Provided extra values: [src]: spec.resources

Steps to Reproduce the Problem

  1. Define a simple condition with a resource
  2. Define a simple task that does not use the resource used by the condition
  3. Define a pipeline like above

Additional Info

The validateDeclaredResource method in

func validateDeclaredResources(ps *PipelineSpec) error {
required := []string{}
for _, t := range ps.Tasks {
if t.Resources != nil {
for _, input := range t.Resources.Inputs {
required = append(required, input.Resource)
}
for _, output := range t.Resources.Outputs {
required = append(required, output.Resource)
}
}
}
provided := make([]string, 0, len(ps.Resources))
for _, resource := range ps.Resources {
provided = append(provided, resource.Name)
}
err := list.IsSame(required, provided)
if err != nil {
return xerrors.Errorf("Pipeline declared resources didn't match usage in Tasks: %w", err)
}
return nil
}
only matches resources of the pipeline with resources of the tasks, it does not account for resources used by conditions.

@vdemeester
Copy link
Member

/kind bug
/cc @dibyom

@tekton-robot tekton-robot added the kind/bug Categorizes issue or PR as related to a bug. label Sep 3, 2019
@afrittoli
Copy link
Member Author

I can propose a fix for this myself, but I want to make sure folks agree that this is a bug :)

@dibyom
Copy link
Member

dibyom commented Sep 3, 2019

Yeah, I think this is a bug. I don't see a reason why we should limit the resources available to the condition to only the ones available to the task.

@dibyom
Copy link
Member

dibyom commented Sep 3, 2019

/assign

dibyom added a commit to dibyom/pipeline that referenced this issue Sep 4, 2019
This commit fixes the Pipeline validation logic to account
for resources that are declared in a `PipelineTaskCondition` but
not in a `PipelineTask`. Previously, each resource used in a
`PipelineTaskCondition` also had to be declared in the `PipelineTask`.

Fixes tektoncd#1263

Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
dibyom added a commit to dibyom/pipeline that referenced this issue Sep 4, 2019
This commit fixes the Pipeline validation logic to account
for resources that are declared in a `PipelineTaskCondition` but
not in a `PipelineTask`. Previously, each resource used in a
`PipelineTaskCondition` also had to be declared in the `PipelineTask`.

Fixes tektoncd#1263

Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
tekton-robot pushed a commit that referenced this issue Sep 4, 2019
This commit fixes the Pipeline validation logic to account
for resources that are declared in a `PipelineTaskCondition` but
not in a `PipelineTask`. Previously, each resource used in a
`PipelineTaskCondition` also had to be declared in the `PipelineTask`.

Fixes #1263

Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants