diff --git a/docs/pipelines.md b/docs/pipelines.md index fd09d3c228b..9ea591611cb 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -91,9 +91,7 @@ parameters can be passed to the `Pipeline` from a `PipelineRun`. Input parameters in the form of `$(params.foo)` are replaced inside of the [`PipelineTask` parameters' values](#pipeline-tasks) (see also -[variable substitution](tasks.md#variable-substitution)). _As with -[variable substitution](tasks.md#variable-substitution), the deprecated syntax -`${params.foo}` will be supported until [#1170](https://github.com/tektoncd/pipeline/issues/1170)._ +[variable substitution](tasks.md#variable-substitution)). The following `Pipeline` declares an input parameter called 'context', and uses it in the `PipelineTask`'s parameter. The `description` and `default` fields for diff --git a/docs/resources.md b/docs/resources.md index d000e4911f9..2489f566a80 100644 --- a/docs/resources.md +++ b/docs/resources.md @@ -86,9 +86,6 @@ The `path` key is pre-defined and refers to the local path to a resource on the $(inputs.resouces..path) ``` -_The deprecated syntax `${}`, e.g. `${inputs.params.}` will be supported -until [#1170](https://github.com/tektoncd/pipeline/issues/1170)._ - ### Controlling where resources are mounted The optional field `targetPath` can be used to initialize a resource in specific diff --git a/docs/tasks.md b/docs/tasks.md index 86ce8a2ae3e..deaaf122799 100644 --- a/docs/tasks.md +++ b/docs/tasks.md @@ -400,9 +400,6 @@ $(inputs.params.) Param values from resources can also be accessed using [variable substitution](./resources.md#variable-substitution) -_The deprecated syntax `${}`, e.g. `${inputs.params.}` will be supported -until [#1170](https://github.com/tektoncd/pipeline/issues/1170)._ - #### Variable Substitution with Parameters of Type `Array` Referenced parameters of type `array` will expand to insert the array elements in the reference string's spot. diff --git a/pkg/apis/pipeline/v1alpha1/param_types_test.go b/pkg/apis/pipeline/v1alpha1/param_types_test.go index e83cb9a8ceb..4d875fe8820 100644 --- a/pkg/apis/pipeline/v1alpha1/param_types_test.go +++ b/pkg/apis/pipeline/v1alpha1/param_types_test.go @@ -128,42 +128,6 @@ func TestArrayOrString_ApplyReplacements(t *testing.T) { arrayReplacements: map[string][]string{"arraykey": {}}, }, expectedOutput: builder.ArrayOrString("firstvalue", "lastvalue"), - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated string replacements on string", - args: args{ - input: builder.ArrayOrString("astring${some} asdf ${anotherkey}"), - stringReplacements: map[string]string{"some": "value", "anotherkey": "value"}, - arrayReplacements: map[string][]string{"arraykey": {"array", "value"}, "sdfdf": {"asdf", "sdfsd"}}, - }, - expectedOutput: builder.ArrayOrString("astringvalue asdf value"), - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated single array replacement", - args: args{ - input: builder.ArrayOrString("firstvalue", "${arraykey}", "lastvalue"), - stringReplacements: map[string]string{"some": "value", "anotherkey": "value"}, - arrayReplacements: map[string][]string{"arraykey": {"array", "value"}, "sdfdf": {"asdf", "sdfsd"}}, - }, - expectedOutput: builder.ArrayOrString("firstvalue", "array", "value", "lastvalue"), - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated multiple array replacement", - args: args{ - input: builder.ArrayOrString("firstvalue", "${arraykey}", "lastvalue", "${sdfdf}"), - stringReplacements: map[string]string{"some": "value", "anotherkey": "value"}, - arrayReplacements: map[string][]string{"arraykey": {"array", "value"}, "sdfdf": {"asdf", "sdfsd"}}, - }, - expectedOutput: builder.ArrayOrString("firstvalue", "array", "value", "lastvalue", "asdf", "sdfsd"), - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated empty array replacement", - args: args{ - input: builder.ArrayOrString("firstvalue", "${arraykey}", "lastvalue"), - stringReplacements: map[string]string{"some": "value", "anotherkey": "value"}, - arrayReplacements: map[string][]string{"arraykey": {}}, - }, - expectedOutput: builder.ArrayOrString("firstvalue", "lastvalue"), }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go b/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go index 0b8421fd94e..29f1a9ddbf9 100644 --- a/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go @@ -152,35 +152,6 @@ func TestPipelineSpec_Validate(t *testing.T) { tb.PipelineTaskParam("a-param", "$(input.workspace.$(baz))")), )), failureExpected: false, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated valid parameter variables", - p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( - tb.PipelineParamSpec("baz", v1alpha1.ParamTypeString), - tb.PipelineParamSpec("foo-is-baz", v1alpha1.ParamTypeString), - tb.PipelineTask("bar", "bar-task", - tb.PipelineTaskParam("a-param", "${baz} and ${foo-is-baz}")), - )), - failureExpected: false, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated valid array parameter variables", - p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( - tb.PipelineParamSpec("baz", v1alpha1.ParamTypeArray, tb.ParamSpecDefault("some", "default")), - tb.PipelineParamSpec("foo-is-baz", v1alpha1.ParamTypeArray), - tb.PipelineTask("bar", "bar-task", - tb.PipelineTaskParam("a-param", "${baz}", "and", "${foo-is-baz}")), - )), - failureExpected: false, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated pipeline parameter nested in task parameter", - p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( - tb.PipelineParamSpec("baz", v1alpha1.ParamTypeString), - tb.PipelineTask("bar", "bar-task", - tb.PipelineTaskParam("a-param", "${input.workspace.${baz}}")), - )), - failureExpected: false, }, { name: "duplicate tasks", p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( @@ -305,67 +276,6 @@ func TestPipelineSpec_Validate(t *testing.T) { tb.PipelineTaskParam("a-param", "first", "value: $(params.baz)", "last")), )), failureExpected: true, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated not defined parameter variable", - p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( - tb.PipelineTask("foo", "foo-task", - tb.PipelineTaskParam("a-param", "${params.does-not-exist}")))), - failureExpected: true, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "not defined parameter variable with defined", - p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( - tb.PipelineParamSpec("foo", v1alpha1.ParamTypeString, tb.ParamSpecDefault("something")), - tb.PipelineTask("foo", "foo-task", - tb.PipelineTaskParam("a-param", "${params.foo} and ${params.does-not-exist}")))), - failureExpected: true, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated invalid parameter type", - p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( - tb.PipelineParamSpec("baz", "invalidtype", tb.ParamSpecDefault("some", "default")), - tb.PipelineParamSpec("foo-is-baz", v1alpha1.ParamTypeArray), - tb.PipelineTask("bar", "bar-task", - tb.PipelineTaskParam("a-param", "${baz}", "and", "${foo-is-baz}")), - )), - failureExpected: true, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated array parameter mismatching default type", - p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( - tb.PipelineParamSpec("baz", v1alpha1.ParamTypeArray, tb.ParamSpecDefault("astring")), - tb.PipelineTask("bar", "bar-task", - tb.PipelineTaskParam("a-param", "arrayelement", "${baz}")), - )), - failureExpected: true, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated string parameter mismatching default type", - p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( - tb.PipelineParamSpec("baz", v1alpha1.ParamTypeString, tb.ParamSpecDefault("anarray", "elements")), - tb.PipelineTask("bar", "bar-task", - tb.PipelineTaskParam("a-param", "arrayelement", "${baz}")), - )), - failureExpected: true, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated array parameter used as string", - p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( - tb.PipelineParamSpec("baz", v1alpha1.ParamTypeArray, tb.ParamSpecDefault("anarray", "elements")), - tb.PipelineTask("bar", "bar-task", - tb.PipelineTaskParam("a-param", "${params.baz}")), - )), - failureExpected: true, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated array parameter string template not isolated", - p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( - tb.PipelineParamSpec("baz", v1alpha1.ParamTypeArray, tb.ParamSpecDefault("anarray", "elements")), - tb.PipelineTask("bar", "bar-task", - tb.PipelineTaskParam("a-param", "first", "value: ${params.baz}", "last")), - )), - failureExpected: true, }, { name: "invalid dependency graph between the tasks", p: tb.Pipeline("foo", "namespace", tb.PipelineSpec( diff --git a/pkg/apis/pipeline/v1alpha1/step_replacements_test.go b/pkg/apis/pipeline/v1alpha1/step_replacements_test.go index b889fd7b09b..af4ae8dcc15 100644 --- a/pkg/apis/pipeline/v1alpha1/step_replacements_test.go +++ b/pkg/apis/pipeline/v1alpha1/step_replacements_test.go @@ -34,45 +34,45 @@ func TestApplyStepReplacements(t *testing.T) { } s := v1alpha1.Step{Container: corev1.Container{ - Name: "${replace.me}", - Image: "${replace.me}", - Command: []string{"${array.replace.me}"}, - Args: []string{"${array.replace.me}"}, - WorkingDir: "${replace.me}", + Name: "$(replace.me)", + Image: "$(replace.me)", + Command: []string{"$(array.replace.me)"}, + Args: []string{"$(array.replace.me)"}, + WorkingDir: "$(replace.me)", EnvFrom: []corev1.EnvFromSource{{ ConfigMapRef: &corev1.ConfigMapEnvSource{ LocalObjectReference: corev1.LocalObjectReference{ - Name: "${replace.me}", + Name: "$(replace.me)", }, }, SecretRef: &corev1.SecretEnvSource{ LocalObjectReference: corev1.LocalObjectReference{ - Name: "${replace.me}", + Name: "$(replace.me)", }, }, }}, Env: []corev1.EnvVar{{ Name: "not_me", - Value: "${replace.me}", + Value: "$(replace.me)", ValueFrom: &corev1.EnvVarSource{ ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ - Name: "${replace.me}", + Name: "$(replace.me)", }, - Key: "${replace.me}", + Key: "$(replace.me)", }, SecretKeyRef: &corev1.SecretKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ - Name: "${replace.me}", + Name: "$(replace.me)", }, - Key: "${replace.me}", + Key: "$(replace.me)", }, }, }}, VolumeMounts: []corev1.VolumeMount{{ - Name: "${replace.me}", - MountPath: "${replace.me}", - SubPath: "${replace.me}", + Name: "$(replace.me)", + MountPath: "$(replace.me)", + SubPath: "$(replace.me)", }}, }} @@ -126,7 +126,7 @@ func TestApplyStepReplacements(t *testing.T) { func TestApplyStepReplacements_NotDefined(t *testing.T) { s := v1alpha1.Step{Container: corev1.Container{ - Name: "${params.not.defined}", + Name: "$(params.not.defined)", }} replacements := map[string]string{ "replace.me": "replaced!", @@ -137,7 +137,7 @@ func TestApplyStepReplacements_NotDefined(t *testing.T) { } expected := v1alpha1.Step{Container: corev1.Container{ - Name: "${params.not.defined}", + Name: "$(params.not.defined)", }} v1alpha1.ApplyStepReplacements(&s, replacements, arrayReplacements) if d := cmp.Diff(s, expected); d != "" { diff --git a/pkg/apis/pipeline/v1alpha1/substitution.go b/pkg/apis/pipeline/v1alpha1/substitution.go index c79c224c681..5bab384883d 100644 --- a/pkg/apis/pipeline/v1alpha1/substitution.go +++ b/pkg/apis/pipeline/v1alpha1/substitution.go @@ -26,8 +26,7 @@ import ( const parameterSubstitution = "[_a-zA-Z][_a-zA-Z0-9.-]*" -//TODO(#1170): Regex matches both ${} and $(), we will need to remove ${} compatibility. -const braceMatchingRegex = "(\\$({%s.(?P%s)}))|(\\$(\\(%s.(?P%s)\\)))" +const braceMatchingRegex = "(\\$(\\(%s.(?P%s)\\)))" func ValidateVariable(name, value, prefix, contextPrefix, locationName, path string, vars map[string]struct{}) *apis.FieldError { if vs, present := extractVariablesFromString(value, contextPrefix+prefix); present { @@ -76,10 +75,10 @@ func ValidateVariableIsolated(name, value, prefix, contextPrefix, locationName, return nil } -// Extract a the first full string expressions found (e.g "${input.params.foo}"). Return +// Extract a the first full string expressions found (e.g "$(input.params.foo)"). Return // "" and false if nothing is found. func extractExpressionFromString(s, prefix string) (string, bool) { - pattern := fmt.Sprintf(braceMatchingRegex, prefix, parameterSubstitution, prefix, parameterSubstitution) + pattern := fmt.Sprintf(braceMatchingRegex, prefix, parameterSubstitution) re := regexp.MustCompile(pattern) match := re.FindStringSubmatch(s) if match == nil { @@ -89,7 +88,7 @@ func extractExpressionFromString(s, prefix string) (string, bool) { } func extractVariablesFromString(s, prefix string) ([]string, bool) { - pattern := fmt.Sprintf(braceMatchingRegex, prefix, parameterSubstitution, prefix, parameterSubstitution) + pattern := fmt.Sprintf(braceMatchingRegex, prefix, parameterSubstitution) re := regexp.MustCompile(pattern) matches := re.FindAllStringSubmatch(s, -1) if len(matches) == 0 { @@ -101,14 +100,7 @@ func extractVariablesFromString(s, prefix string) ([]string, bool) { // foo -> foo // foo.bar -> foo // foo.bar.baz -> foo - var v string - if groups["var"] != "" { - v = groups["var"] - } else { - //TODO(#1170): Regex matches both ${} and $(), we will need to remove ${} compatibility. - v = groups["deprecated"] - } - vars[i] = strings.SplitN(v, ".", 2)[0] + vars[i] = strings.SplitN(groups["var"], ".", 2)[0] } return vars, true } @@ -124,9 +116,6 @@ func matchGroups(matches []string, pattern *regexp.Regexp) map[string]string { func ApplyReplacements(in string, replacements map[string]string) string { for k, v := range replacements { in = strings.Replace(in, fmt.Sprintf("$(%s)", k), v, -1) - - //TODO(#1170): Delete the line below when we want to remove support for ${} variable interpolation: - in = strings.Replace(in, fmt.Sprintf("${%s}", k), v, -1) } return in } @@ -143,12 +132,6 @@ func ApplyArrayReplacements(in string, stringReplacements map[string]string, arr if (strings.Count(in, stringToReplace) == 1) && len(in) == len(stringToReplace) { return v } - - //TODO(#1170): Delete the block below when we want to remove support for ${} variable interpolation: - deprecatedStringToReplace := fmt.Sprintf("${%s}", k) - if (strings.Count(in, deprecatedStringToReplace) == 1) && len(in) == len(deprecatedStringToReplace) { - return v - } } // Otherwise return a size-1 array containing the input string with standard stringReplacements applied. diff --git a/pkg/apis/pipeline/v1alpha1/substitution_test.go b/pkg/apis/pipeline/v1alpha1/substitution_test.go index aee4588d6fe..4059d78e92e 100644 --- a/pkg/apis/pipeline/v1alpha1/substitution_test.go +++ b/pkg/apis/pipeline/v1alpha1/substitution_test.go @@ -109,82 +109,6 @@ func TestValidateVariables(t *testing.T) { Message: `non-existent variable in "--flag=$(inputs.params.baz) $(input.params.foo)" for step somefield`, Paths: []string{"taskspec.steps.somefield"}, }, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated valid variable", - args: args{ - input: "--flag=${inputs.params.baz}", - prefix: "params", - contextPrefix: "inputs.", - locationName: "step", - path: "taskspec.steps", - vars: map[string]struct{}{ - "baz": {}, - }, - }, - expectedError: nil, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated multiple variables", - args: args{ - input: "--flag=${inputs.params.baz} ${input.params.foo}", - prefix: "params", - contextPrefix: "inputs.", - locationName: "step", - path: "taskspec.steps", - vars: map[string]struct{}{ - "baz": {}, - "foo": {}, - }, - }, - expectedError: nil, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated different context and prefix", - args: args{ - input: "--flag=${something.baz}", - prefix: "something", - locationName: "step", - path: "taskspec.steps", - vars: map[string]struct{}{ - "baz": {}, - }, - }, - expectedError: nil, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated undefined variable", - args: args{ - input: "--flag=${inputs.params.baz}", - prefix: "params", - contextPrefix: "inputs.", - locationName: "step", - path: "taskspec.steps", - vars: map[string]struct{}{ - "foo": {}, - }, - }, - expectedError: &apis.FieldError{ - Message: `non-existent variable in "--flag=${inputs.params.baz}" for step somefield`, - Paths: []string{"taskspec.steps.somefield"}, - }, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated undefined variable and defined variable", - args: args{ - input: "--flag=${inputs.params.baz} ${input.params.foo}", - prefix: "params", - contextPrefix: "inputs.", - locationName: "step", - path: "taskspec.steps", - vars: map[string]struct{}{ - "foo": {}, - }, - }, - expectedError: &apis.FieldError{ - Message: `non-existent variable in "--flag=${inputs.params.baz} ${input.params.foo}" for step somefield`, - Paths: []string{"taskspec.steps.somefield"}, - }, }} { t.Run(tc.name, func(t *testing.T) { got := v1alpha1.ValidateVariable("somefield", tc.args.input, tc.args.prefix, tc.args.contextPrefix, tc.args.locationName, tc.args.path, tc.args.vars) @@ -217,7 +141,7 @@ func TestApplyReplacements(t *testing.T) { { name: "single replacement requested", args: args{ - input: "this is ${a} string", + input: "this is $(a) string", replacements: map[string]string{"a": "not a"}, }, expectedOutput: "this is not a string", @@ -225,7 +149,7 @@ func TestApplyReplacements(t *testing.T) { { name: "single replacement requested multiple matches", args: args{ - input: "this ${is} a string ${is} a string", + input: "this $(is) a string $(is) a string", replacements: map[string]string{"is": "a"}, }, expectedOutput: "this a a string a a string", @@ -233,7 +157,7 @@ func TestApplyReplacements(t *testing.T) { { name: "multiple replacements requested", args: args{ - input: "this ${is} a ${string} ${is} a ${string}", + input: "this $(is) a $(string) $(is) a $(string)", replacements: map[string]string{"is": "a", "string": "sstring"}, }, expectedOutput: "this a a sstring a a sstring", @@ -286,7 +210,7 @@ func TestApplyArrayReplacements(t *testing.T) { }, { name: "multiple replacements only string replacement possible", args: args{ - input: "${string}rep${lacement}${string}", + input: "$(string)rep$(lacement)$(string)", stringReplacements: map[string]string{"string": "word", "lacement": "lacements"}, arrayReplacements: map[string][]string{"ace": {"replacement", "a"}, "string": {"1", "2"}}, }, @@ -294,7 +218,7 @@ func TestApplyArrayReplacements(t *testing.T) { }, { name: "array replacement", args: args{ - input: "${match}", + input: "$(match)", stringReplacements: map[string]string{"string": "word", "lacement": "lacements"}, arrayReplacements: map[string][]string{"ace": {"replacement", "a"}, "match": {"1", "2"}}, }, diff --git a/pkg/apis/pipeline/v1alpha1/task_validation_test.go b/pkg/apis/pipeline/v1alpha1/task_validation_test.go index b0d79a6ae26..33e6d1fca81 100644 --- a/pkg/apis/pipeline/v1alpha1/task_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/task_validation_test.go @@ -180,53 +180,6 @@ func TestTaskSpecValidate(t *testing.T) { WorkingDir: "/foo/bar/$(outputs.resources.source)", }}}, }, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated valid template variable", - fields: fields{ - Inputs: &v1alpha1.Inputs{ - Resources: []v1alpha1.TaskResource{validImageResource}, - Params: []v1alpha1.ParamSpec{{ - Name: "baz", - }, { - Name: "foo-is-baz", - }}, - }, - Outputs: &v1alpha1.Outputs{ - Resources: []v1alpha1.TaskResource{validResource}, - }, - Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "mystep", - Image: "${inputs.resources.source.url}", - Args: []string{"--flag=${inputs.params.baz} && ${input.params.foo-is-baz}"}, - WorkingDir: "/foo/bar/${outputs.resources.source}", - }}}, - }, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated valid array template variable", - fields: fields{ - Inputs: &v1alpha1.Inputs{ - Resources: []v1alpha1.TaskResource{validImageResource}, - Params: []v1alpha1.ParamSpec{{ - Name: "baz", - Type: v1alpha1.ParamTypeArray, - }, { - Name: "foo-is-baz", - Type: v1alpha1.ParamTypeArray, - }}, - }, - Outputs: &v1alpha1.Outputs{ - Resources: []v1alpha1.TaskResource{validResource}, - }, - Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "mystep", - Image: "${inputs.resources.source.url}", - Command: []string{"${inputs.param.foo-is-baz}"}, - Args: []string{"${inputs.params.baz}", "middle string", "${input.params.foo-is-baz}"}, - WorkingDir: "/foo/bar/${outputs.resources.source}", - }}}, - }, }, { name: "step template included in validation", fields: fields{ @@ -467,20 +420,6 @@ func TestTaskSpecValidateError(t *testing.T) { Message: `non-existent variable in "--flag=$(inputs.params.inexistent)" for step arg[0]`, Paths: []string{"taskspec.steps.arg[0]"}, }, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated inexistent input param variable", - fields: fields{ - Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "mystep", - Image: "myimage", - Args: []string{"--flag=${inputs.params.inexistent}"}, - }}}, - }, - expectedError: apis.FieldError{ - Message: `non-existent variable in "--flag=${inputs.params.inexistent}" for step arg[0]`, - Paths: []string{"taskspec.steps.arg[0]"}, - }, }, { name: "array used in unaccepted field", fields: fields{ @@ -502,7 +441,7 @@ func TestTaskSpecValidateError(t *testing.T) { Image: "$(inputs.params.baz)", Command: []string{"$(inputs.param.foo-is-baz)"}, Args: []string{"$(inputs.params.baz)", "middle string", "$(input.resources.foo.url)"}, - WorkingDir: "/foo/bar/${outputs.resources.source}", + WorkingDir: "/foo/bar/$(outputs.resources.source)", }}}, }, expectedError: apis.FieldError{ @@ -530,7 +469,7 @@ func TestTaskSpecValidateError(t *testing.T) { Image: "someimage", Command: []string{"$(inputs.param.foo-is-baz)"}, Args: []string{"not isolated: $(inputs.params.baz)", "middle string", "$(input.resources.foo.url)"}, - WorkingDir: "/foo/bar/${outputs.resources.source}", + WorkingDir: "/foo/bar/$(outputs.resources.source)", }}}, }, expectedError: apis.FieldError{ @@ -658,191 +597,6 @@ func TestTaskSpecValidateError(t *testing.T) { Message: `non-existent variable in "$(inputs.params.foo) && $(inputs.params.inexistent)" for step arg[0]`, Paths: []string{"taskspec.steps.arg[0]"}, }, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprected array used in unaccepted field", - fields: fields{ - Inputs: &v1alpha1.Inputs{ - Resources: []v1alpha1.TaskResource{validImageResource}, - Params: []v1alpha1.ParamSpec{{ - Name: "baz", - Type: v1alpha1.ParamTypeArray, - }, { - Name: "foo-is-baz", - Type: v1alpha1.ParamTypeArray, - }}, - }, - Outputs: &v1alpha1.Outputs{ - Resources: []v1alpha1.TaskResource{validResource}, - }, - Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "mystep", - Image: "${inputs.params.baz}", - Command: []string{"${inputs.param.foo-is-baz}"}, - Args: []string{"${inputs.params.baz}", "middle string", "${input.resources.foo.url}"}, - WorkingDir: "/foo/bar/${outputs.resources.source}", - }}}, - }, - expectedError: apis.FieldError{ - Message: `variable type invalid in "${inputs.params.baz}" for step image`, - Paths: []string{"taskspec.steps.image"}, - }, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "depreated array not properly isolated", - fields: fields{ - Inputs: &v1alpha1.Inputs{ - Resources: []v1alpha1.TaskResource{validImageResource}, - Params: []v1alpha1.ParamSpec{{ - Name: "baz", - Type: v1alpha1.ParamTypeArray, - }, { - Name: "foo-is-baz", - Type: v1alpha1.ParamTypeArray, - }}, - }, - Outputs: &v1alpha1.Outputs{ - Resources: []v1alpha1.TaskResource{validResource}, - }, - Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "mystep", - Image: "someimage", - Command: []string{"${inputs.param.foo-is-baz}"}, - Args: []string{"not isolated: ${inputs.params.baz}", "middle string", "${input.resources.foo.url}"}, - WorkingDir: "/foo/bar/${outputs.resources.source}", - }}}, - }, - expectedError: apis.FieldError{ - Message: `variable is not properly isolated in "not isolated: ${inputs.params.baz}" for step arg[0]`, - Paths: []string{"taskspec.steps.arg[0]"}, - }, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated array not properly isolated", - fields: fields{ - Inputs: &v1alpha1.Inputs{ - Resources: []v1alpha1.TaskResource{validImageResource}, - Params: []v1alpha1.ParamSpec{{ - Name: "baz", - Type: v1alpha1.ParamTypeArray, - }, { - Name: "foo-is-baz", - Type: v1alpha1.ParamTypeArray, - }}, - }, - Outputs: &v1alpha1.Outputs{ - Resources: []v1alpha1.TaskResource{validResource}, - }, - Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "mystep", - Image: "someimage", - Command: []string{"${inputs.param.foo-is-baz}"}, - Args: []string{"not isolated: ${inputs.params.baz}", "middle string", "${input.resources.foo.url}"}, - WorkingDir: "/foo/bar/${outputs.resources.source}", - }}}, - }, - expectedError: apis.FieldError{ - Message: `variable is not properly isolated in "not isolated: ${inputs.params.baz}" for step arg[0]`, - Paths: []string{"taskspec.steps.arg[0]"}, - }, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated inexistent input resource variable", - fields: fields{ - Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "mystep", - Image: "myimage:${inputs.resources.inputs}", - }}}, - }, - expectedError: apis.FieldError{ - Message: `non-existent variable in "myimage:${inputs.resources.inputs}" for step image`, - Paths: []string{"taskspec.steps.image"}, - }, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated inferred array not properly isolated", - fields: fields{ - Inputs: &v1alpha1.Inputs{ - Resources: []v1alpha1.TaskResource{validImageResource}, - Params: []v1alpha1.ParamSpec{{ - Name: "baz", - Default: &v1alpha1.ArrayOrString{ - Type: v1alpha1.ParamTypeArray, - ArrayVal: []string{"implied", "array", "type"}, - }, - }, { - Name: "foo-is-baz", - Default: &v1alpha1.ArrayOrString{ - Type: v1alpha1.ParamTypeArray, - ArrayVal: []string{"implied", "array", "type"}, - }, - }}, - }, - Outputs: &v1alpha1.Outputs{ - Resources: []v1alpha1.TaskResource{validResource}, - }, - Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "mystep", - Image: "someimage", - Command: []string{"${inputs.param.foo-is-baz}"}, - Args: []string{"not isolated: ${inputs.params.baz}", "middle string", "${input.resources.foo.url}"}, - WorkingDir: "/foo/bar/${outputs.resources.source}", - }}}, - }, - expectedError: apis.FieldError{ - Message: `variable is not properly isolated in "not isolated: ${inputs.params.baz}" for step arg[0]`, - Paths: []string{"taskspec.steps.arg[0]"}, - }, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated inexistent input resource variable", - fields: fields{ - Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "mystep", - Image: "myimage:${inputs.resources.inputs}", - }}}, - }, - expectedError: apis.FieldError{ - Message: `non-existent variable in "myimage:${inputs.resources.inputs}" for step image`, - Paths: []string{"taskspec.steps.image"}, - }, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated inexistent output param variable", - fields: fields{ - Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "mystep", - Image: "myimage", - WorkingDir: "/foo/bar/${outputs.resources.inexistent}", - }}}, - }, - expectedError: apis.FieldError{ - Message: `non-existent variable in "/foo/bar/${outputs.resources.inexistent}" for step workingDir`, - Paths: []string{"taskspec.steps.workingDir"}, - }, - }, { - // TODO(#1170): Remove support for ${} syntax - name: "deprecated Inexistent param variable with existing", - fields: fields{ - Inputs: &v1alpha1.Inputs{ - Params: []v1alpha1.ParamSpec{ - { - Name: "foo", - Description: "param", - Default: builder.ArrayOrString("default"), - }, - }, - }, - Steps: []v1alpha1.Step{{Container: corev1.Container{ - Name: "mystep", - Image: "myimage", - Args: []string{"${inputs.params.foo} && ${inputs.params.inexistent}"}, - }}}, - }, - expectedError: apis.FieldError{ - Message: `non-existent variable in "${inputs.params.foo} && ${inputs.params.inexistent}" for step arg[0]`, - Paths: []string{"taskspec.steps.arg[0]"}, - }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/reconciler/pipelinerun/resources/apply_test.go b/pkg/reconciler/pipelinerun/resources/apply_test.go index 03c9af2b458..24bc03def68 100644 --- a/pkg/reconciler/pipelinerun/resources/apply_test.go +++ b/pkg/reconciler/pipelinerun/resources/apply_test.go @@ -39,8 +39,7 @@ func TestApplyParameters(t *testing.T) { tb.PipelineParamSpec("second-param", v1alpha1.ParamTypeString), tb.PipelineTask("first-task-1", "first-task", tb.PipelineTaskParam("first-task-first-param", "$(params.first-param)"), - // TODO(#1170): Remove support for ${} syntax - tb.PipelineTaskParam("first-task-second-param", "${params.second-param}"), + tb.PipelineTaskParam("first-task-second-param", "$(params.second-param)"), tb.PipelineTaskParam("first-task-third-param", "static value"), ))), run: tb.PipelineRun("test-pipeline-run", "foo", @@ -63,8 +62,7 @@ func TestApplyParameters(t *testing.T) { tb.PipelineParamSpec("second-param", v1alpha1.ParamTypeString, tb.ParamSpecDefault("default-value")), tb.PipelineTask("first-task-1", "first-task", tb.PipelineTaskParam("first-task-first-param", "$(input.workspace.$(params.first-param))"), - // TODO(#1170): Remove support for ${} syntax - tb.PipelineTaskParam("first-task-second-param", "${input.workspace.${params.second-param}}"), + tb.PipelineTaskParam("first-task-second-param", "$(input.workspace.$(params.second-param))"), ))), run: tb.PipelineRun("test-pipeline-run", "foo", tb.PipelineRunSpec("test-pipeline")), @@ -74,7 +72,7 @@ func TestApplyParameters(t *testing.T) { tb.PipelineParamSpec("second-param", v1alpha1.ParamTypeString, tb.ParamSpecDefault("default-value")), tb.PipelineTask("first-task-1", "first-task", tb.PipelineTaskParam("first-task-first-param", "$(input.workspace.default-value)"), - tb.PipelineTaskParam("first-task-second-param", "${input.workspace.default-value}"), + tb.PipelineTaskParam("first-task-second-param", "$(input.workspace.default-value)"), ))), }, { name: "parameters in task condition", @@ -85,8 +83,7 @@ func TestApplyParameters(t *testing.T) { tb.PipelineTask("first-task-1", "first-task", tb.PipelineTaskCondition("task-condition", tb.PipelineTaskConditionParam("cond-first-param", "$(params.first-param)"), - // TODO(#1170): Remove support for ${} syntax - tb.PipelineTaskConditionParam("cond-second-param", "${params.second-param}"), + tb.PipelineTaskConditionParam("cond-second-param", "$(params.second-param)"), ), ))), run: tb.PipelineRun("test-pipeline-run", "foo", @@ -114,8 +111,7 @@ func TestApplyParameters(t *testing.T) { tb.PipelineTaskParam("first-task-first-param", "firstelement", "$(params.first-param)"), tb.PipelineTaskParam("first-task-second-param", "first", "$(params.second-param)"), tb.PipelineTaskParam("first-task-third-param", "static value"), - // TODO(#1170): Remove support for ${} syntax - tb.PipelineTaskParam("first-task-fourth-param", "first", "${params.fourth-param}"), + tb.PipelineTaskParam("first-task-fourth-param", "first", "$(params.fourth-param)"), ))), run: tb.PipelineRun("test-pipeline-run", "foo", tb.PipelineRunSpec("test-pipeline", diff --git a/pkg/reconciler/pipelinerun/resources/conditionresolution.go b/pkg/reconciler/pipelinerun/resources/conditionresolution.go index eeafee3a8ff..d3f3338c1c3 100644 --- a/pkg/reconciler/pipelinerun/resources/conditionresolution.go +++ b/pkg/reconciler/pipelinerun/resources/conditionresolution.go @@ -107,9 +107,9 @@ func (rcc *ResolvedConditionCheck) ConditionToTaskSpec() (*v1alpha1.TaskSpec, er }) } - // convert param strings of type ${params.x} to ${inputs.params.x} + // convert param strings of type $(params.x) to $(inputs.params.x) convertParamTemplates(&t.Steps[0], rcc.Condition.Spec.Params) - // convert resource strings of type ${resources.name.key} to ${inputs.resources.name.key} + // convert resource strings of type $(resources.name.key) to $(inputs.resources.name.key) err := ApplyResourceSubstitution(&t.Steps[0], rcc.ResolvedResources, rcc.Condition.Spec.Resources) if err != nil { @@ -119,7 +119,7 @@ func (rcc *ResolvedConditionCheck) ConditionToTaskSpec() (*v1alpha1.TaskSpec, er return t, nil } -// Replaces all instances of ${params.x} in the container to ${inputs.params.x} for each param name +// Replaces all instances of $(params.x) in the container to $(inputs.params.x) for each param name func convertParamTemplates(step *v1alpha1.Step, params []v1alpha1.ParamSpec) { replacements := make(map[string]string) for _, p := range params { diff --git a/pkg/reconciler/taskrun/resources/apply_test.go b/pkg/reconciler/taskrun/resources/apply_test.go index 43b1fa1be08..cd7a54a25af 100644 --- a/pkg/reconciler/taskrun/resources/apply_test.go +++ b/pkg/reconciler/taskrun/resources/apply_test.go @@ -55,20 +55,17 @@ var simpleTaskSpec = &v1alpha1.TaskSpec{ Image: "quux", Args: []string{"$(outputs.resources.imageToUse.url)"}, }}, {Container: corev1.Container{ - Name: "foo", - // TODO(#1170): Remove support for ${} syntax - Image: "${inputs.params.myimage}", + Name: "foo", + Image: "$(inputs.params.myimage)", }}, {Container: corev1.Container{ - Name: "baz", - Image: "bat", - // TODO(#1170): Remove support for ${} syntax - WorkingDir: "${inputs.resources.workspace.path}", - Args: []string{"${inputs.resources.workspace.url}"}, + Name: "baz", + Image: "bat", + WorkingDir: "$(inputs.resources.workspace.path)", + Args: []string{"$(inputs.resources.workspace.url)"}, }}, {Container: corev1.Container{ Name: "qux", Image: "quux", - // TODO(#1170): Remove support for ${} syntax - Args: []string{"${outputs.resources.imageToUse.url}"}, + Args: []string{"$(outputs.resources.imageToUse.url)"}, }}}, } @@ -88,12 +85,11 @@ var envTaskSpec = &v1alpha1.TaskSpec{ }, }, }, { - // TODO(#1170): Remove support for ${} syntax Name: "baz", ValueFrom: &corev1.EnvVarSource{ SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "secret-${inputs.params.FOO}"}, - Key: "secret-key-${inputs.params.FOO}", + LocalObjectReference: corev1.LocalObjectReference{Name: "secret-$(inputs.params.FOO)"}, + Key: "secret-key-$(inputs.params.FOO)", }, }, }}, @@ -103,10 +99,9 @@ var envTaskSpec = &v1alpha1.TaskSpec{ LocalObjectReference: corev1.LocalObjectReference{Name: "config-$(inputs.params.FOO)"}, }, }, { - // TODO(#1170): Remove support for ${} syntax - Prefix: "prefix-1-${inputs.params.FOO}", + Prefix: "prefix-1-$(inputs.params.FOO)", SecretRef: &corev1.SecretEnvSource{ - LocalObjectReference: corev1.LocalObjectReference{Name: "secret-${inputs.params.FOO}"}, + LocalObjectReference: corev1.LocalObjectReference{Name: "secret-$(inputs.params.FOO)"}, }, }}, }}}, @@ -737,11 +732,10 @@ func TestVolumeReplacement(t *testing.T) { }}, }, }, { - // TODO(#1170): Remove support for ${} syntax name: "deprecated volume replacement", ts: &v1alpha1.TaskSpec{ Volumes: []corev1.Volume{{ - Name: "${foo}", + Name: "$(foo)", }}, }, repl: map[string]string{"foo": "bar"}, @@ -751,15 +745,14 @@ func TestVolumeReplacement(t *testing.T) { }}, }, }, { - // TODO(#1170): Remove support for ${} syntax name: "deprecated volume configmap", ts: &v1alpha1.TaskSpec{ Volumes: []corev1.Volume{{ - Name: "${name}", + Name: "$(name)", VolumeSource: corev1.VolumeSource{ ConfigMap: &corev1.ConfigMapVolumeSource{ LocalObjectReference: corev1.LocalObjectReference{ - Name: "${configmapname}", + Name: "$(configmapname)", }, }, }}, @@ -782,14 +775,13 @@ func TestVolumeReplacement(t *testing.T) { }, }, }, { - // TODO(#1170): Remove support for ${} syntax name: "deprecated volume secretname", ts: &v1alpha1.TaskSpec{ Volumes: []corev1.Volume{{ - Name: "${name}", + Name: "$(name)", VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ - SecretName: "${secretname}", + SecretName: "$(secretname)", }, }}, }, @@ -809,14 +801,13 @@ func TestVolumeReplacement(t *testing.T) { }, }, }, { - // TODO(#1170): Remove support for ${} syntax name: "deprecated volume PVC name", ts: &v1alpha1.TaskSpec{ Volumes: []corev1.Volume{{ - Name: "${name}", + Name: "$(name)", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: "${FOO}", + ClaimName: "$(FOO)", }, }, }}, diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index 8e504fee003..d93a4b070d9 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -122,8 +122,7 @@ var ( "--my-arg=$(inputs.params.myarg)", "--my-arg-with-default=$(inputs.params.myarghasdefault)", "--my-arg-with-default2=$(inputs.params.myarghasdefault2)", - // TODO(#1170): Remove support for ${} syntax - "--my-additional-arg=${outputs.resources.myimage.url}", + "--my-additional-arg=$(outputs.resources.myimage.url)", )), tb.Step("myothercontainer", "myotherimage", tb.StepCommand("/mycmd"), tb.StepArgs( "--my-other-arg=$(inputs.resources.workspace.url)", diff --git a/test/dag_test.go b/test/dag_test.go index 485cb058e43..cc869989699 100644 --- a/test/dag_test.go +++ b/test/dag_test.go @@ -56,7 +56,7 @@ func TestDAGPipelineRun(t *testing.T) { ), tb.TaskOutputs(tb.OutputsResource("repo", v1alpha1.PipelineResourceTypeGit)), tb.Step("echo-text", "busybox", tb.StepCommand("echo"), tb.StepArgs("$(inputs.params.text)")), - tb.Step("ln", "busybox", tb.StepCommand("ln"), tb.StepArgs("-s", "${inputs.resources.repo.path}", "${outputs.resources.repo.path}")), + tb.Step("ln", "busybox", tb.StepCommand("ln"), tb.StepArgs("-s", "$(inputs.resources.repo.path)", "$(outputs.resources.repo.path)")), )) if _, err := c.TaskClient.Create(echoTask); err != nil { t.Fatalf("Failed to create echo Task: %s", err) diff --git a/test/pipelinerun_test.go b/test/pipelinerun_test.go index 4174a0dab4f..1e0209fdae1 100644 --- a/test/pipelinerun_test.go +++ b/test/pipelinerun_test.go @@ -100,8 +100,7 @@ func TestPipelineRun(t *testing.T) { // Reference build: https://github.com/knative/build/tree/master/test/docker-basic tb.Step("config-docker", "quay.io/rhpipeline/skopeo:alpine", tb.StepCommand("skopeo"), - // TODO(#1170): This test is using a mix of ${} and $() syntax to make sure both work. - tb.StepArgs("copy", "${inputs.params.path}", "$(inputs.params.dest)"), + tb.StepArgs("copy", "$(inputs.params.path)", "$(inputs.params.dest)"), ), )) if _, err := c.TaskClient.Create(task); err != nil { @@ -234,9 +233,7 @@ func getHelloWorldPipelineWithSingularTask(suffix int, namespace string) *v1alph tb.PipelineParamSpec("path", v1alpha1.ParamTypeString), tb.PipelineParamSpec("dest", v1alpha1.ParamTypeString), tb.PipelineTask(task1Name, getName(taskName, suffix), - tb.PipelineTaskParam("path", "${params.path}"), - // TODO(#1170): This test is using a mix of ${} and $() syntax to make sure both work. - // In the next release we will remove support for $() entirely. + tb.PipelineTaskParam("path", "$(params.path)"), tb.PipelineTaskParam("dest", "$(params.dest)")), )) } @@ -254,18 +251,14 @@ func getFanInFanOutTasks(namespace string) []*v1alpha1.Task { tb.StepArgs("-c", "echo stuff > $(outputs.resources.workspace.path)/stuff"), ), tb.Step("write-data-task-0-step-1", "ubuntu", tb.StepCommand("/bin/bash"), - // TODO(#1170): This test is using a mix of ${} and $() syntax to make sure both work. - // In the next release we will remove support for $() entirely. - tb.StepArgs("-c", "echo other > ${outputs.resources.workspace.path}/other"), + tb.StepArgs("-c", "echo other > $(outputs.resources.workspace.path)/other"), ), )), tb.Task("check-create-files-exists", namespace, tb.TaskSpec( tb.TaskInputs(inWorkspaceResource), tb.TaskOutputs(outWorkspaceResource), tb.Step("read-from-task-0", "ubuntu", tb.StepCommand("/bin/bash"), - // TODO(#1170): This test is using a mix of ${} and $() syntax to make sure both work. - // In the next release we will remove support for $() entirely. - tb.StepArgs("-c", "[[ stuff == $(cat ${inputs.resources.workspace.path}/stuff) ]]"), + tb.StepArgs("-c", "[[ stuff == $(cat $(inputs.resources.workspace.path)/stuff) ]]"), ), tb.Step("write-data-task-1", "ubuntu", tb.StepCommand("/bin/bash"), tb.StepArgs("-c", "echo something > $(outputs.resources.workspace.path)/something"), @@ -289,9 +282,7 @@ func getFanInFanOutTasks(namespace string) []*v1alpha1.Task { tb.StepArgs("-c", "[[ something == $(cat $(inputs.resources.workspace.path)/something) ]]"), ), tb.Step("read-from-task-1", "ubuntu", tb.StepCommand("/bin/bash"), - // TODO(#1170): This test is using a mix of ${} and $() syntax to make sure both work. - // In the next release we will remove support for $() entirely. - tb.StepArgs("-c", "[[ else == $(cat ${inputs.resources.workspace.path}/else) ]]"), + tb.StepArgs("-c", "[[ else == $(cat $(inputs.resources.workspace.path)/else) ]]"), ), )), }