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

Parameter variable substitution support for conditionals #1143

Merged
merged 2 commits into from
Aug 2, 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
30 changes: 27 additions & 3 deletions docs/conditions.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ This document defines `Conditions` and their capabilities.

- [Syntax](#syntax)
- [Check](#check)
- [Parameters](#parameters)
- [Examples](#examples)

## Syntax
Expand All @@ -32,11 +33,34 @@ following fields:
### Check

The `check` field is required. You define a single check to define the body of a `Condition`. The
check must specify a container image that adheres to the [container contract](./container-contract.md). The container image
runs till completion. The container must exit successfully i.e. with an exit code 0 for the
condition evaluation to be successful. All other exit codes are considered to be a condition check
check must specify a container image that adheres to the [container contract](./container-contract.md).
The container image runs till completion. The container must exit successfully i.e. with an exit code 0
for the condition evaluation to be successful. All other exit codes are considered to be a condition check
failure.

### Parameters

A Condition can declare parameters that must be supplied to it during a PipelineRun. Sub-fields
within the check field can access the parameter values using the templating syntax:

```yaml
spec:
parameters:
- name: image
default: ubuntu
check:
image: ${params.image}
```

Parameters name are limited to alpha-numeric characters, `-` and `_` and can
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
Copy link
Member

Choose a reason for hiding this comment

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

NIT: space after the comma

Copy link
Member

Choose a reason for hiding this comment

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

NIT: space after the comma

multiple protected branch names. When the actual parameter value is supplied, its parsed type
is validated against the type field.

## Examples

For complete examples, see
Expand Down
21 changes: 18 additions & 3 deletions examples/pipelineruns/conditional-pipelinerun.yaml
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
apiVersion: tekton.dev/v1alpha1
kind: Condition
metadata:
name: always-true
name: strings-equal
spec:
params:
- name: "x"
type: string
- name: "y"
type: string
check:
image: alpine
command: ["/bin/sh"]
args: ['-c', 'exit 0']
args: ['-c', 'echo "Comparing ${params.x} and ${params.y}" && [ "${params.x}" == "${params.y}" ]']
---
apiVersion: tekton.dev/v1alpha1
kind: PipelineResource
Expand Down Expand Up @@ -43,12 +48,22 @@ spec:
resources:
- name: source-repo
type: git
params:
- name: "x"
default: "abc"
- name: "y"
default: "abc"
tasks:
- name: list-files-1
taskRef:
name: list-files
conditions:
- conditionRef: "always-true"
- conditionRef: "strings-equal"
params:
- name: "x"
value: "${params.x}"
- name: "y"
value: "${params.y}"
resources:
inputs:
- name: workspace
Expand Down
11 changes: 7 additions & 4 deletions pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -633,10 +633,13 @@ func (c *Reconciler) makeConditionCheckContainer(rprt *resources.ResolvedPipelin
Spec: v1alpha1.TaskRunSpec{
TaskSpec: rcc.ConditionToTaskSpec(),
ServiceAccount: getServiceAccount(pr, rprt.PipelineTask.Name),
Timeout: getTaskRunTimeout(pr),
NodeSelector: pr.Spec.NodeSelector,
Tolerations: pr.Spec.Tolerations,
Affinity: pr.Spec.Affinity,
Inputs: v1alpha1.TaskRunInputs{
Params: rcc.PipelineTaskCondition.Params,
},
Timeout: getTaskRunTimeout(pr),
NodeSelector: pr.Spec.NodeSelector,
Tolerations: pr.Spec.Tolerations,
Affinity: pr.Spec.Affinity,
}}

cctr, err := c.PipelineClientSet.TektonV1alpha1().TaskRuns(pr.Namespace).Create(tr)
Expand Down
17 changes: 11 additions & 6 deletions pkg/reconciler/v1alpha1/pipelinerun/resources/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,19 @@ func ApplyReplacements(p *v1alpha1.Pipeline, replacements map[string]string, arr
tasks := p.Spec.Tasks

for i := range tasks {
params := tasks[i].Params

for j := range params {
params[j].Value.ApplyReplacements(replacements, arrayReplacements)
tasks[i].Params = replaceParamValues(tasks[i].Params, replacements, arrayReplacements)
for j := range tasks[i].Conditions {
c := tasks[i].Conditions[j]
c.Params = replaceParamValues(c.Params, replacements, arrayReplacements)
}

tasks[i].Params = params
}

return p
}

func replaceParamValues(params []v1alpha1.Param, stringReplacements map[string]string, arrayReplacements map[string][]string) []v1alpha1.Param {
Copy link
Member

Choose a reason for hiding this comment

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

Nice :)

for i := range params {
params[i].Value.ApplyReplacements(stringReplacements, arrayReplacements)
}
return params
}
44 changes: 44 additions & 0 deletions pkg/reconciler/v1alpha1/pipelinerun/resources/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"testing"

"github.com/google/go-cmp/cmp"

"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
tb "github.com/tektoncd/pipeline/test/builder"
)
Expand Down Expand Up @@ -69,6 +70,49 @@ func TestApplyParameters(t *testing.T) {
tb.PipelineTask("first-task-1", "first-task",
tb.PipelineTaskParam("first-task-first-param", "${input.workspace.default-value}"),
))),
}, {
name: "single parameter in task condition",
Copy link
Member

Choose a reason for hiding this comment

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

NIT: I guess the test name refers to the amount of parameters passed in the run? One is left to default and one not. Perhaps the name could be changed?

original: tb.Pipeline("test-pipeline", "foo",
tb.PipelineSpec(
tb.PipelineParamSpec("first-param", v1alpha1.ParamTypeString, tb.ParamSpecDefault("default-value")),
tb.PipelineParamSpec("second-param", v1alpha1.ParamTypeString),
tb.PipelineTask("first-task-1", "first-task",
tb.PipelineTaskCondition("task-condition",
tb.PipelineTaskConditionParam("cond-first-param", "${params.first-param}"),
tb.PipelineTaskConditionParam("cond-second-param", "${params.second-param}"),
),
))),
run: tb.PipelineRun("test-pipeline-run", "foo",
tb.PipelineRunSpec("test-pipeline",
tb.PipelineRunParam("second-param", "second-value"))),
expected: tb.Pipeline("test-pipeline", "foo",
tb.PipelineSpec(
tb.PipelineParamSpec("first-param", v1alpha1.ParamTypeString, tb.ParamSpecDefault("default-value")),
tb.PipelineParamSpec("second-param", v1alpha1.ParamTypeString),
tb.PipelineTask("first-task-1", "first-task",
tb.PipelineTaskCondition("task-condition",
tb.PipelineTaskConditionParam("cond-first-param", "default-value"),
tb.PipelineTaskConditionParam("cond-second-param", "second-value"),
),
))),
}, {
name: "pipeline parameter nested inside condition parameter",
Copy link
Member

Choose a reason for hiding this comment

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

NIT: again, the name of the test is a bit confusing for me, in this case there is only one parameter and it's left to default.

original: tb.Pipeline("test-pipeline", "foo",
tb.PipelineSpec(
tb.PipelineParamSpec("first-param", v1alpha1.ParamTypeString, tb.ParamSpecDefault("default-value")),
tb.PipelineTask("first-task-1", "first-task",
tb.PipelineTaskCondition("cond",
tb.PipelineTaskConditionParam("cond-first-param", "${params.first-param}")),
))),
run: tb.PipelineRun("test-pipeline-run", "foo",
tb.PipelineRunSpec("test-pipeline")),
expected: tb.Pipeline("test-pipeline", "foo",
tb.PipelineSpec(
tb.PipelineParamSpec("first-param", v1alpha1.ParamTypeString, tb.ParamSpecDefault("default-value")),
tb.PipelineTask("first-task-1", "first-task",
tb.PipelineTaskCondition("cond",
tb.PipelineTaskConditionParam("cond-first-param", "default-value")),
))),
}, {
name: "single array parameter",
original: tb.Pipeline("test-pipeline", "foo",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@
package resources

import (
"fmt"

corev1 "k8s.io/api/core/v1"

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

const (
Expand All @@ -36,9 +39,10 @@ type GetCondition func(string) (*v1alpha1.Condition, error)
// exists. ConditionCheck can be nil to represent there being no ConditionCheck (i.e the condition
// has not been evaluated).
type ResolvedConditionCheck struct {
ConditionCheckName string
Condition *v1alpha1.Condition
ConditionCheck *v1alpha1.ConditionCheck
ConditionCheckName string
Condition *v1alpha1.Condition
ConditionCheck *v1alpha1.ConditionCheck
PipelineTaskCondition *v1alpha1.PipelineTaskCondition
}

// TaskConditionCheckState is a slice of ResolvedConditionCheck the represents the current execution
Expand Down Expand Up @@ -95,11 +99,22 @@ func (rcc *ResolvedConditionCheck) ConditionToTaskSpec() *v1alpha1.TaskSpec {
t.Inputs = &v1alpha1.Inputs{
Params: rcc.Condition.Spec.Params,
}
// convert param strings of type ${params.x} to ${inputs.params.x}
// in order to apply taskrun substitution
convertParamTemplates(&t.Steps[0], rcc.Condition.Spec.Params)
dibyom marked this conversation as resolved.
Show resolved Hide resolved
}

return t
}

// Replaces all instances of ${params.x} in the container to ${inputs.params.x} for each param name
func convertParamTemplates(container *corev1.Container, params []v1alpha1.ParamSpec) {
replacements := make(map[string]string)
for _, p := range params {
replacements[fmt.Sprintf("params.%s", p.Name)] = fmt.Sprintf("${inputs.params.%s}", p.Name)
templating.ApplyContainerReplacements(container, replacements, map[string][]string{})
}
}

// NewConditionCheck status creates a ConditionCheckStatus from a ConditionCheck
func (rcc *ResolvedConditionCheck) NewConditionCheckStatus() *v1alpha1.ConditionCheckStatus {
var checkStep corev1.ContainerState
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
package resources

import (
"testing"

"github.com/google/go-cmp/cmp"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -26,8 +28,6 @@ import (

"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
tb "github.com/tektoncd/pipeline/test/builder"

"testing"
)

var c = tb.Condition("conditionname", "foo")
Expand Down Expand Up @@ -214,19 +214,25 @@ func TestResolvedConditionCheck_ConditionToTaskSpec(t *testing.T) {
}, {
name: "with-input-params",
cond: tb.Condition("bar", "foo", tb.ConditionSpec(
tb.ConditionSpecCheck("", "ubuntu"),
tb.ConditionParamSpec("abc", v1alpha1.ParamTypeString),
tb.ConditionSpecCheck("${params.name}", "${params.img}",
tb.WorkingDir("${params.not.replaced}")),
tb.ConditionParamSpec("name", v1alpha1.ParamTypeString),
tb.ConditionParamSpec("img", v1alpha1.ParamTypeString),
)),
want: v1alpha1.TaskSpec{
Inputs: &v1alpha1.Inputs{
Params: []v1alpha1.ParamSpec{{
Name: "abc",
Name: "name",
Type: "string",
}, {
Name: "img",
Type: "string",
}},
},
Steps: []corev1.Container{{
Name: "condition-check-bar",
Image: "ubuntu",
Name: "${inputs.params.name}",
Image: "${inputs.params.img}",
WorkingDir: "${params.not.replaced}",
}},
},
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -450,8 +450,8 @@ func resolveConditionChecks(pt *v1alpha1.PipelineTask,
taskRunStatus map[string]*v1alpha1.PipelineRunTaskRunStatus,
taskRunName string, getTaskRun resources.GetTaskRun, getCondition GetCondition) ([]*ResolvedConditionCheck, error) {
rcc := []*ResolvedConditionCheck{}
for j := range pt.Conditions {
cName := pt.Conditions[j].ConditionRef
for _, ptc := range pt.Conditions {
cName := ptc.ConditionRef
c, err := getCondition(cName)
if err != nil {
return nil, &ConditionNotFoundError{
Expand All @@ -468,9 +468,10 @@ func resolveConditionChecks(pt *v1alpha1.PipelineTask,
}

rcc = append(rcc, &ResolvedConditionCheck{
Condition: c,
ConditionCheckName: conditionCheckName,
ConditionCheck: v1alpha1.NewConditionCheck(cctr),
Condition: c,
ConditionCheckName: conditionCheckName,
ConditionCheck: v1alpha1.NewConditionCheck(cctr),
PipelineTaskCondition: &ptc,
})
}
return rcc, nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1492,12 +1492,15 @@ func TestResolveConditionChecks(t *testing.T) {
},
Spec: v1alpha1.TaskRunSpec{},
}

ptc := v1alpha1.PipelineTaskCondition{
ConditionRef: "always-true",
}

pts := []v1alpha1.PipelineTask{{
Name: "mytask1",
TaskRef: v1alpha1.TaskRef{Name: "task"},
Conditions: []v1alpha1.PipelineTaskCondition{{
ConditionRef: "always-true",
}},
Name: "mytask1",
TaskRef: v1alpha1.TaskRef{Name: "task"},
Conditions: []v1alpha1.PipelineTaskCondition{ptc},
}}
providedResources := map[string]v1alpha1.PipelineResourceRef{}

Expand Down Expand Up @@ -1529,9 +1532,10 @@ func TestResolveConditionChecks(t *testing.T) {
return nil, xerrors.Errorf("getTaskRun called with unexpected name %s", name)
},
expectedConditionCheck: TaskConditionCheckState{{
ConditionCheckName: "pipelinerun-mytask1-9l9zj-always-true-mz4c7",
Condition: &condition,
ConditionCheck: v1alpha1.NewConditionCheck(cc),
ConditionCheckName: "pipelinerun-mytask1-9l9zj-always-true-mz4c7",
Condition: &condition,
ConditionCheck: v1alpha1.NewConditionCheck(cc),
PipelineTaskCondition: &ptc,
}},
},
{
Expand All @@ -1545,8 +1549,9 @@ func TestResolveConditionChecks(t *testing.T) {
return nil, xerrors.Errorf("getTaskRun called with unexpected name %s", name)
},
expectedConditionCheck: TaskConditionCheckState{{
ConditionCheckName: "pipelinerun-mytask1-mssqb-always-true-78c5n",
Condition: &condition,
ConditionCheckName: "pipelinerun-mytask1-mssqb-always-true-78c5n",
Condition: &condition,
PipelineTaskCondition: &ptc,
}},
},
}
Expand Down Expand Up @@ -1626,12 +1631,14 @@ func TestResolveConditionCheck_UseExistingConditionCheckName(t *testing.T) {
Spec: v1alpha1.TaskRunSpec{},
}

ptc := v1alpha1.PipelineTaskCondition{
ConditionRef: "always-true",
}

pts := []v1alpha1.PipelineTask{{
Name: "mytask1",
TaskRef: v1alpha1.TaskRef{Name: "task"},
Conditions: []v1alpha1.PipelineTaskCondition{{
ConditionRef: "always-true",
}},
Name: "mytask1",
TaskRef: v1alpha1.TaskRef{Name: "task"},
Conditions: []v1alpha1.PipelineTaskCondition{ptc},
}}
providedResources := map[string]v1alpha1.PipelineResourceRef{}

Expand Down Expand Up @@ -1674,9 +1681,10 @@ func TestResolveConditionCheck_UseExistingConditionCheckName(t *testing.T) {
t.Fatalf("Did not expect error when resolving PipelineRun without Conditions: %v", err)
}
expectedConditionChecks := TaskConditionCheckState{{
ConditionCheckName: ccName,
Condition: &condition,
ConditionCheck: v1alpha1.NewConditionCheck(cc),
ConditionCheckName: ccName,
Condition: &condition,
ConditionCheck: v1alpha1.NewConditionCheck(cc),
PipelineTaskCondition: &ptc,
}}

if d := cmp.Diff(pipelineState[0].ResolvedConditionChecks, expectedConditionChecks, cmpopts.IgnoreUnexported(v1alpha1.TaskRunSpec{})); d != "" {
Expand Down
Loading