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

Allow declaring and passing resources to conditions #1151

Merged
merged 3 commits into from
Aug 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion docs/conditions.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ This document defines `Conditions` and their capabilities.
- [Syntax](#syntax)
- [Check](#check)
- [Parameters](#parameters)
- [Resources](#resources)
- [Examples](#examples)

## Syntax
Expand Down Expand Up @@ -57,10 +58,31 @@ only start with alpha characters and `_`. For example, `fooIs-Bar_` is a valid
parameter name, `barIsBa$` or `0banana` are not.

Each declared parameter has a type field, assumed to be string if not provided by the user.
The other possible type is array — useful, checking a pushed branch name doesn't match any of
The other possible type is array — useful, for instance, checking that a pushed branch name doesn't match any of
multiple protected branch names. When the actual parameter value is supplied, its parsed type
is validated against the type field.

### Resources

Conditions can declare input [`PipelineResources`](resources.md) via the `resources` field to
provide the Condition container step with data or context that is needed to perform the check.

Input resources, like source code (git), are dumped at path
`/workspace/resource_name` within a mounted
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm I'm thinking that we should create a separate section in the resources.md docs for how the paths work - the docs are a bit spread out, e.g.

https://github.com/tektoncd/pipeline/blob/master/docs/tasks.md#controlling-where-resources-are-mounted
and
https://github.com/tektoncd/pipeline/blob/master/docs/taskruns.md#overriding-where-resources-are-copied-from
And even the path templating (like you said!)
https://github.com/tektoncd/pipeline/blob/master/docs/tasks.md#templating

But I think/hope that both of those docs would apply to this use of resources as well? In which case it'd be good if we can put all those docs in one place and link to them from here and from the other places

🙏

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the thing is since things like targetPath are part of TaskResource, they are in the tasks doc. Now with conditions, there is some duplication. I'll add a section in the resource doc on how to use resources in conditions and tasks. That section will link to both the Task and Condition resource docs and contain the common bits e.g. targetPaths, the path variable substitution.

I'm thinking of doing this in a follow-up PR.

[volume](https://kubernetes.io/docs/concepts/storage/volumes/). The condition container can use the `path`
[template](./tasks.md#Templating) key to refer to the local path to the mounted resource.

```yaml
spec:
resources:
- name: workspace
type: git
check:
image: alpine
command: ["/bin/sh"]
args: ['-c', 'test -f $(resources.workspace.path)/README.md']
```

## Examples

For complete examples, see
Expand Down
6 changes: 6 additions & 0 deletions docs/pipelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,12 @@ tasks:
name: build-push
conditions:
- conditionRef: my-condition
params:
- name: my-param
value: my-value
resources:
- name: workspace
resource: source-repo
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm something just occurred to me - can conditions use the from clause for resources? If there is more work to do here I totally would expect that in a follow up PR, but basically what I'm thinking is that if a Task is getting a resource from another Task, but has some condition to execute on that resource, the user would probably want the condition to be executed on that same task

e.g. a Task has a git output (assuming a world where a git output creates a new commit), another Task gets the git resource from that task, but the condition says only if the most recent commit changed XYZ files (kind of contrived but maybe??)

```

In this example, `my-condition` refers to a [Condition](#conditions) custom resource. The `build-push`
Expand Down
36 changes: 18 additions & 18 deletions examples/pipelineruns/conditional-pipelinerun.yaml
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
apiVersion: tekton.dev/v1alpha1
kind: Condition
metadata:
name: strings-equal
name: file-exists
spec:
params:
- name: "x"
type: string
- name: "y"
type: string
check:
params:
- name: "path"
type: string
resources:
- name: workspace
type: git
check:
image: alpine
command: ["/bin/sh"]
args: ['-c', 'echo "Comparing ${params.x} and ${params.y}" && [ "${params.x}" == "${params.y}" ]']
args: ['-c', 'test -f $(resources.workspace.path)/$(params.path)']
---
apiVersion: tekton.dev/v1alpha1
kind: PipelineResource
Expand All @@ -38,7 +39,7 @@ spec:
- name: run-ls
image: ubuntu
command: ["/bin/bash"]
args: ['-c', 'ls -al ${inputs.resources.workspace.path}']
args: ['-c', 'ls -al $(inputs.resources.workspace.path)']
---
apiVersion: tekton.dev/v1alpha1
kind: Pipeline
Expand All @@ -49,21 +50,20 @@ spec:
- name: source-repo
type: git
params:
- name: "x"
default: "abc"
- name: "y"
default: "abc"
- name: "path"
default: "README.md"
tasks:
- name: list-files-1
taskRef:
name: list-files
conditions:
- conditionRef: "strings-equal"
- conditionRef: "file-exists"
params:
- name: "x"
value: "${params.x}"
- name: "y"
value: "${params.y}"
- name: "path"
value: "$(params.path)"
resources:
- name: workspace
resource: source-repo
resources:
inputs:
- name: workspace
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1alpha1/condition_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ type ConditionSpec struct {
// is evaluated
// +optional
Params []ParamSpec `json:"params,omitempty"`

// Resources is a list of the ConditionResources required to run the condition.
// +optional
Resources []ResourceDeclaration `json:"resources,omitempty"`
}

// ConditionCheck represents a single evaluation of a Condition step.
Expand Down
12 changes: 12 additions & 0 deletions pkg/apis/pipeline/v1alpha1/pipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ type PipelineTaskCondition struct {
// Params declare parameters passed to this Condition
// +optional
Params []Param `json:"params,omitempty"`

// Resources declare the resources provided to this Condition as input
Resources []PipelineConditionResource `json:"resources,omitempty"`
}

// PipelineDeclaredResource is used by a Pipeline to declare the types of the
Expand All @@ -135,6 +138,15 @@ type PipelineDeclaredResource struct {
Type PipelineResourceType `json:"type"`
}

// PipelineConditionResource allows a Pipeline to declare how its DeclaredPipelineResources
// should be provided to a Condition as its inputs.
type PipelineConditionResource struct {
// Name is the name of the PipelineResource as declared by the Condition.
Name string `json:"name"`
// Resource is the name of the DeclaredPipelineResource to use.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm this is kinda odd but also really minor - these comments refer to DeclaredPipelineResource but I don't think that DeclaredPipelineResource is a type that's actually defined anywhere?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Maybe this is ResourceDeclaration)?

Resource string `json:"resource"`
}

// PipelineTaskResources allows a Pipeline to declare how its DeclaredPipelineResources
// should be provided to a Task as its inputs and outputs.
type PipelineTaskResources struct {
Expand Down
37 changes: 37 additions & 0 deletions pkg/apis/pipeline/v1alpha1/resource_paths.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
Copyright 2019 The Tekton Authors

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha1

import "path/filepath"

// InputResourcePath returns the path where the given input resource
// will get mounted in a Pod
func InputResourcePath(r ResourceDeclaration) string {
return path("/workspace", r)
}

// OutputResourcePath returns the path to the output resouce in a Pod
func OutputResourcePath(r ResourceDeclaration) string {
return path("/workspace/output", r)
}

func path(root string, r ResourceDeclaration) string {
if r.TargetPath != "" {
return filepath.Join("/workspace", r.TargetPath)
}
return filepath.Join(root, r.Name)
}
81 changes: 81 additions & 0 deletions pkg/apis/pipeline/v1alpha1/resource_paths_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
Copyright 2019 The Tekton Authors

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha1_test

import (
"testing"

"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
)

func TestInputResourcePath(t *testing.T) {
tcs := []struct {
name string
resource v1alpha1.ResourceDeclaration
expected string
}{{
name: "default_path",
resource: v1alpha1.ResourceDeclaration{
Name: "foo",
},
expected: "/workspace/foo",
}, {
name: "with target path",
resource: v1alpha1.ResourceDeclaration{
Name: "foo",
TargetPath: "bar",
},
expected: "/workspace/bar",
}}

for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
if actual := v1alpha1.InputResourcePath(tc.resource); actual != tc.expected {
t.Errorf("Unexpected input resource path: %s", actual)
}
})
}
}

func TestOutputResourcePath(t *testing.T) {
tcs := []struct {
name string
resource v1alpha1.ResourceDeclaration
expected string
}{{
name: "default_path",
resource: v1alpha1.ResourceDeclaration{
Name: "foo",
},
expected: "/workspace/output/foo",
}, {
name: "with target path",
resource: v1alpha1.ResourceDeclaration{
Name: "foo",
TargetPath: "bar",
},
expected: "/workspace/bar",
}}

for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
if actual := v1alpha1.OutputResourcePath(tc.resource); actual != tc.expected {
t.Errorf("Unexpected output resource path: %s", actual)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be helpful to included the expected path in the error message as well

}
})
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha yeessssss i love the super focused test coverage XD

18 changes: 18 additions & 0 deletions pkg/apis/pipeline/v1alpha1/resource_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,24 @@ type PipelineResourceList struct {
Items []PipelineResource `json:"items"`
}

// ResourceDeclaration defines an input or output PipelineResource declared as a requirement
// by another type such as a Task or Condition. The Name field will be used to refer to these
// PipelineResources within the type's definition, and when provided as an Input, the Name will be the
// path to the volume mounted containing this PipelineResource as an input (e.g.
// an input Resource named `workspace` will be mounted at `/workspace`).
type ResourceDeclaration struct {
// Name declares the name by which a resource is referenced in the
// definition. Resources may be referenced by name in the definition of a
// Task's steps.
Name string `json:"name"`
// Type is the type of this resource;
Type PipelineResourceType `json:"type"`
// TargetPath is the path in workspace directory where the resource
// will be copied.
// +optional
TargetPath string `json:"targetPath,omitempty"`
}

// ResourceFromType returns a PipelineResourceInterface from a PipelineResource's type.
func ResourceFromType(r *PipelineResource) (PipelineResourceInterface, error) {
switch r.Spec.Type {
Expand Down
12 changes: 1 addition & 11 deletions pkg/apis/pipeline/v1alpha1/task_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,7 @@ type Inputs struct {
// path to the volume mounted containing this Resource as an input (e.g.
// an input Resource named `workspace` will be mounted at `/workspace`).
type TaskResource struct {
// Name declares the name by which a resource is referenced in the Task's
// definition. Resources may be referenced by name in the definition of a
// Task's steps.
Name string `json:"name"`
// Type is the type of this resource;
Type PipelineResourceType `json:"type"`
// TargetPath is the path in workspace directory where the task resource
// will be copied.
// +optional
TargetPath string `json:"targetPath,omitempty"`
// Path to the index.json file for output container images.
ResourceDeclaration
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 😇

// +optional
OutputImageDir string `json:"outputImageDir,omitempty"`
}
Expand Down
Loading