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

Remove support for ${} syntax 🗑️ #1311

Merged
merged 1 commit into from
Sep 16, 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
4 changes: 1 addition & 3 deletions docs/pipelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions docs/resources.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,6 @@ The `path` key is pre-defined and refers to the local path to a resource on the
$(inputs.resouces.<name>.path)
```

_The deprecated syntax `${}`, e.g. `${inputs.params.<name>}` 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
Expand Down
3 changes: 0 additions & 3 deletions docs/tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -400,9 +400,6 @@ $(inputs.params.<name>)

Param values from resources can also be accessed using [variable substitution](./resources.md#variable-substitution)

_The deprecated syntax `${}`, e.g. `${inputs.params.<name>}` 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.
Expand Down
36 changes: 0 additions & 36 deletions pkg/apis/pipeline/v1alpha1/param_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
90 changes: 0 additions & 90 deletions pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
34 changes: 17 additions & 17 deletions pkg/apis/pipeline/v1alpha1/step_replacements_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
}},
}}

Expand Down Expand Up @@ -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!",
Expand All @@ -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 != "" {
Expand Down
27 changes: 5 additions & 22 deletions pkg/apis/pipeline/v1alpha1/substitution.go
Original file line number Diff line number Diff line change
Expand Up @@ -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<deprecated>%s)}))|(\\$(\\(%s.(?P<var>%s)\\)))"
const braceMatchingRegex = "(\\$(\\(%s.(?P<var>%s)\\)))"

func ValidateVariable(name, value, prefix, contextPrefix, locationName, path string, vars map[string]struct{}) *apis.FieldError {
if vs, present := extractVariablesFromString(value, contextPrefix+prefix); present {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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.
Expand Down
Loading