Skip to content

Commit 2f83d73

Browse files
twoGiantstwoGiants
committed
cleanup: move step and sidecar validation
Move step and sidecar validation functions with their call chain to container_validation.go and fix small typos. Issue tektoncd#7442. Signed-off-by: Stanislav Jakuschevskij <[email protected]>
1 parent 6c1020d commit 2f83d73

File tree

3 files changed

+343
-343
lines changed

3 files changed

+343
-343
lines changed

‎pkg/apis/pipeline/v1/container_validation.go

+342-9
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,29 @@ import (
2121
"errors"
2222
"fmt"
2323
"regexp"
24+
"slices"
2425
"strings"
26+
"time"
2527

28+
"github.com/tektoncd/pipeline/internal/artifactref"
2629
"github.com/tektoncd/pipeline/pkg/apis/config"
30+
"github.com/tektoncd/pipeline/pkg/apis/pipeline"
31+
"github.com/tektoncd/pipeline/pkg/internal/resultref"
32+
"github.com/tektoncd/pipeline/pkg/substitution"
33+
"k8s.io/apimachinery/pkg/util/sets"
2734
"k8s.io/apimachinery/pkg/util/validation"
2835
"knative.dev/pkg/apis"
2936
)
3037

38+
// Validate ensures that a supplied Ref field is populated
39+
// correctly. No errors are returned for a nil Ref.
40+
func (ref *Ref) Validate(ctx context.Context) (errs *apis.FieldError) {
41+
if ref == nil {
42+
return errs
43+
}
44+
return validateRef(ctx, ref.Name, ref.Resolver, ref.Params)
45+
}
46+
3147
func validateRef(ctx context.Context, refName string, refResolver ResolverName, refParams Params) (errs *apis.FieldError) {
3248
switch {
3349
case refResolver != "" || refParams != nil:
@@ -80,15 +96,6 @@ func validateRef(ctx context.Context, refName string, refResolver ResolverName,
8096
return errs
8197
}
8298

83-
// Validate ensures that a supplied Ref field is populated
84-
// correctly. No errors are returned for a nil Ref.
85-
func (ref *Ref) Validate(ctx context.Context) (errs *apis.FieldError) {
86-
if ref == nil {
87-
return errs
88-
}
89-
return validateRef(ctx, ref.Name, ref.Resolver, ref.Params)
90-
}
91-
9299
// RefNameLikeUrl checks if the name is url parsable and returns an error if it isn't.
93100
func RefNameLikeUrl(name string) error {
94101
schemeRegex := regexp.MustCompile(`[\w-]+:\/\/*`)
@@ -97,3 +104,329 @@ func RefNameLikeUrl(name string) error {
97104
}
98105
return nil
99106
}
107+
108+
func validateSteps(ctx context.Context, steps []Step) (errs *apis.FieldError) {
109+
// Task must not have duplicate step names.
110+
names := sets.NewString()
111+
for idx, s := range steps {
112+
errs = errs.Also(validateStep(ctx, s, names).ViaIndex(idx))
113+
if s.Results != nil {
114+
errs = errs.Also(ValidateStepResultsVariables(ctx, s.Results, s.Script).ViaIndex(idx))
115+
errs = errs.Also(ValidateStepResults(ctx, s.Results).ViaIndex(idx).ViaField("results"))
116+
}
117+
if len(s.When) > 0 {
118+
errs = errs.Also(s.When.validate(ctx).ViaIndex(idx))
119+
}
120+
}
121+
return errs
122+
}
123+
124+
func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.FieldError) {
125+
if err := validateArtifactsReferencesInStep(ctx, s); err != nil {
126+
return err
127+
}
128+
129+
if s.Ref != nil {
130+
if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions && isCreateOrUpdateAndDiverged(ctx, s) {
131+
return apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true to reference StepActions in Steps.", config.EnableStepActions), "")
132+
}
133+
errs = errs.Also(s.Ref.Validate(ctx))
134+
if s.Image != "" {
135+
errs = errs.Also(&apis.FieldError{
136+
Message: "image cannot be used with Ref",
137+
Paths: []string{"image"},
138+
})
139+
}
140+
if len(s.Command) > 0 {
141+
errs = errs.Also(&apis.FieldError{
142+
Message: "command cannot be used with Ref",
143+
Paths: []string{"command"},
144+
})
145+
}
146+
if len(s.Args) > 0 {
147+
errs = errs.Also(&apis.FieldError{
148+
Message: "args cannot be used with Ref",
149+
Paths: []string{"args"},
150+
})
151+
}
152+
if s.Script != "" {
153+
errs = errs.Also(&apis.FieldError{
154+
Message: "script cannot be used with Ref",
155+
Paths: []string{"script"},
156+
})
157+
}
158+
if s.WorkingDir != "" {
159+
errs = errs.Also(&apis.FieldError{
160+
Message: "working dir cannot be used with Ref",
161+
Paths: []string{"workingDir"},
162+
})
163+
}
164+
if s.Env != nil {
165+
errs = errs.Also(&apis.FieldError{
166+
Message: "env cannot be used with Ref",
167+
Paths: []string{"env"},
168+
})
169+
}
170+
if len(s.VolumeMounts) > 0 {
171+
errs = errs.Also(&apis.FieldError{
172+
Message: "volumeMounts cannot be used with Ref",
173+
Paths: []string{"volumeMounts"},
174+
})
175+
}
176+
if len(s.Results) > 0 {
177+
errs = errs.Also(&apis.FieldError{
178+
Message: "results cannot be used with Ref",
179+
Paths: []string{"results"},
180+
})
181+
}
182+
} else {
183+
if len(s.Params) > 0 {
184+
errs = errs.Also(&apis.FieldError{
185+
Message: "params cannot be used without Ref",
186+
Paths: []string{"params"},
187+
})
188+
}
189+
if len(s.Results) > 0 {
190+
if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions && isCreateOrUpdateAndDiverged(ctx, s) {
191+
return apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true in order to use Results in Steps.", config.EnableStepActions), "")
192+
}
193+
}
194+
if len(s.When) > 0 {
195+
if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions && isCreateOrUpdateAndDiverged(ctx, s) {
196+
return apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true in order to use When in Steps.", config.EnableStepActions), "")
197+
}
198+
}
199+
if s.Image == "" {
200+
errs = errs.Also(apis.ErrMissingField("Image"))
201+
}
202+
203+
if s.Script != "" {
204+
if len(s.Command) > 0 {
205+
errs = errs.Also(&apis.FieldError{
206+
Message: "script cannot be used with command",
207+
Paths: []string{"script"},
208+
})
209+
}
210+
}
211+
}
212+
213+
if s.Name != "" {
214+
if names.Has(s.Name) {
215+
errs = errs.Also(apis.ErrInvalidValue(s.Name, "name"))
216+
}
217+
if e := validation.IsDNS1123Label(s.Name); len(e) > 0 {
218+
errs = errs.Also(&apis.FieldError{
219+
Message: fmt.Sprintf("invalid value %q", s.Name),
220+
Paths: []string{"name"},
221+
Details: "Task step name must be a valid DNS Label, For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names",
222+
})
223+
}
224+
names.Insert(s.Name)
225+
}
226+
227+
if s.Timeout != nil {
228+
if s.Timeout.Duration < time.Duration(0) {
229+
return apis.ErrInvalidValue(s.Timeout.Duration, "negative timeout")
230+
}
231+
}
232+
233+
for j, vm := range s.VolumeMounts {
234+
if strings.HasPrefix(vm.MountPath, "/tekton/") &&
235+
!strings.HasPrefix(vm.MountPath, "/tekton/home") {
236+
errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("volumeMount cannot be mounted under /tekton/ (volumeMount %q mounted at %q)", vm.Name, vm.MountPath), "mountPath").ViaFieldIndex("volumeMounts", j))
237+
}
238+
if strings.HasPrefix(vm.Name, "tekton-internal-") {
239+
errs = errs.Also(apis.ErrGeneric(fmt.Sprintf(`volumeMount name %q cannot start with "tekton-internal-"`, vm.Name), "name").ViaFieldIndex("volumeMounts", j))
240+
}
241+
}
242+
243+
if s.OnError != "" {
244+
if !isParamRefs(string(s.OnError)) && s.OnError != Continue && s.OnError != StopAndFail {
245+
errs = errs.Also(&apis.FieldError{
246+
Message: fmt.Sprintf("invalid value: \"%v\"", s.OnError),
247+
Paths: []string{"onError"},
248+
Details: "Task step onError must be either \"continue\" or \"stopAndFail\"",
249+
})
250+
}
251+
}
252+
253+
if s.Script != "" {
254+
cleaned := strings.TrimSpace(s.Script)
255+
if strings.HasPrefix(cleaned, "#!win") {
256+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "windows script support", config.AlphaAPIFields).ViaField("script"))
257+
}
258+
}
259+
260+
// StdoutConfig is an alpha feature and will fail validation if it's used in a task spec
261+
// when the enable-api-fields feature gate is not "alpha".
262+
if s.StdoutConfig != nil {
263+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "step stdout stream support", config.AlphaAPIFields).ViaField("stdoutconfig"))
264+
}
265+
// StderrConfig is an alpha feature and will fail validation if it's used in a task spec
266+
// when the enable-api-fields feature gate is not "alpha".
267+
if s.StderrConfig != nil {
268+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "step stderr stream support", config.AlphaAPIFields).ViaField("stderrconfig"))
269+
}
270+
271+
// Validate usage of step result reference.
272+
// Referencing previous step's results are only allowed in `env`, `command` and `args`.
273+
errs = errs.Also(validateStepResultReference(s))
274+
275+
// Validate usage of step artifacts output reference
276+
// Referencing previous step's results are only allowed in `env`, `command` and `args`, `script`.
277+
errs = errs.Also(validateStepArtifactsReference(s))
278+
return errs
279+
}
280+
281+
// isParamRefs attempts to check if a specified string looks like it contains any parameter reference
282+
// This is useful to make sure the specified value looks like a Parameter Reference before performing any strict validation
283+
func isParamRefs(s string) bool {
284+
return strings.HasPrefix(s, "$("+ParamsPrefix)
285+
}
286+
287+
func validateArtifactsReferencesInStep(ctx context.Context, s Step) *apis.FieldError {
288+
if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableArtifacts {
289+
var t []string
290+
t = append(t, s.Script)
291+
t = append(t, s.Command...)
292+
t = append(t, s.Args...)
293+
for _, e := range s.Env {
294+
t = append(t, e.Value)
295+
}
296+
if slices.ContainsFunc(t, stepArtifactReferenceExists) || slices.ContainsFunc(t, taskArtifactReferenceExists) {
297+
return apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true to use artifacts feature.", config.EnableArtifacts), "")
298+
}
299+
}
300+
return nil
301+
}
302+
303+
func stepArtifactReferenceExists(src string) bool {
304+
return len(artifactref.StepArtifactRegex.FindAllStringSubmatch(src, -1)) > 0 || strings.Contains(src, "$("+artifactref.StepArtifactPathPattern+")")
305+
}
306+
307+
func taskArtifactReferenceExists(src string) bool {
308+
return len(artifactref.TaskArtifactRegex.FindAllStringSubmatch(src, -1)) > 0 || strings.Contains(src, "$("+artifactref.TaskArtifactPathPattern+")")
309+
}
310+
311+
func validateStepResultReference(s Step) (errs *apis.FieldError) {
312+
errs = errs.Also(errorIfStepResultReferencedInField(s.Name, "name"))
313+
errs = errs.Also(errorIfStepResultReferencedInField(s.Image, "image"))
314+
errs = errs.Also(errorIfStepResultReferencedInField(s.Script, "script"))
315+
errs = errs.Also(errorIfStepResultReferencedInField(string(s.ImagePullPolicy), "imagePullPolicy"))
316+
errs = errs.Also(errorIfStepResultReferencedInField(s.WorkingDir, "workingDir"))
317+
for _, e := range s.EnvFrom {
318+
errs = errs.Also(errorIfStepResultReferencedInField(e.Prefix, "envFrom.prefix"))
319+
if e.ConfigMapRef != nil {
320+
errs = errs.Also(errorIfStepResultReferencedInField(e.ConfigMapRef.LocalObjectReference.Name, "envFrom.configMapRef"))
321+
}
322+
if e.SecretRef != nil {
323+
errs = errs.Also(errorIfStepResultReferencedInField(e.SecretRef.LocalObjectReference.Name, "envFrom.secretRef"))
324+
}
325+
}
326+
for _, v := range s.VolumeMounts {
327+
errs = errs.Also(errorIfStepResultReferencedInField(v.Name, "volumeMounts.name"))
328+
errs = errs.Also(errorIfStepResultReferencedInField(v.MountPath, "volumeMounts.mountPath"))
329+
errs = errs.Also(errorIfStepResultReferencedInField(v.SubPath, "volumeMounts.subPath"))
330+
}
331+
for _, v := range s.VolumeDevices {
332+
errs = errs.Also(errorIfStepResultReferencedInField(v.Name, "volumeDevices.name"))
333+
errs = errs.Also(errorIfStepResultReferencedInField(v.DevicePath, "volumeDevices.devicePath"))
334+
}
335+
return errs
336+
}
337+
338+
func errorIfStepResultReferencedInField(value, fieldName string) (errs *apis.FieldError) {
339+
matches := resultref.StepResultRegex.FindAllStringSubmatch(value, -1)
340+
if len(matches) > 0 {
341+
errs = errs.Also(&apis.FieldError{
342+
Message: "stepResult substitutions are only allowed in env, command and args. Found usage in",
343+
Paths: []string{fieldName},
344+
})
345+
}
346+
return errs
347+
}
348+
349+
func validateStepArtifactsReference(s Step) (errs *apis.FieldError) {
350+
errs = errs.Also(errorIfStepArtifactReferencedInField(s.Name, "name"))
351+
errs = errs.Also(errorIfStepArtifactReferencedInField(s.Image, "image"))
352+
errs = errs.Also(errorIfStepArtifactReferencedInField(string(s.ImagePullPolicy), "imagePullPolicy"))
353+
errs = errs.Also(errorIfStepArtifactReferencedInField(s.WorkingDir, "workingDir"))
354+
for _, e := range s.EnvFrom {
355+
errs = errs.Also(errorIfStepArtifactReferencedInField(e.Prefix, "envFrom.prefix"))
356+
if e.ConfigMapRef != nil {
357+
errs = errs.Also(errorIfStepArtifactReferencedInField(e.ConfigMapRef.LocalObjectReference.Name, "envFrom.configMapRef"))
358+
}
359+
if e.SecretRef != nil {
360+
errs = errs.Also(errorIfStepArtifactReferencedInField(e.SecretRef.LocalObjectReference.Name, "envFrom.secretRef"))
361+
}
362+
}
363+
for _, v := range s.VolumeMounts {
364+
errs = errs.Also(errorIfStepArtifactReferencedInField(v.Name, "volumeMounts.name"))
365+
errs = errs.Also(errorIfStepArtifactReferencedInField(v.MountPath, "volumeMounts.mountPath"))
366+
errs = errs.Also(errorIfStepArtifactReferencedInField(v.SubPath, "volumeMounts.subPath"))
367+
}
368+
for _, v := range s.VolumeDevices {
369+
errs = errs.Also(errorIfStepArtifactReferencedInField(v.Name, "volumeDevices.name"))
370+
errs = errs.Also(errorIfStepArtifactReferencedInField(v.DevicePath, "volumeDevices.devicePath"))
371+
}
372+
return errs
373+
}
374+
375+
func errorIfStepArtifactReferencedInField(value, fieldName string) (errs *apis.FieldError) {
376+
if stepArtifactReferenceExists(value) {
377+
errs = errs.Also(&apis.FieldError{
378+
Message: "stepArtifact substitutions are only allowed in env, command, args and script. Found usage in",
379+
Paths: []string{fieldName},
380+
})
381+
}
382+
return errs
383+
}
384+
385+
// ValidateStepResultsVariables validates if the StepResults referenced in step script are defined in step's results.
386+
func ValidateStepResultsVariables(ctx context.Context, results []StepResult, script string) (errs *apis.FieldError) {
387+
resultsNames := sets.NewString()
388+
for _, r := range results {
389+
resultsNames.Insert(r.Name)
390+
}
391+
errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(script, "step.results", resultsNames).ViaField("script"))
392+
errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(script, "results", resultsNames).ViaField("script"))
393+
return errs
394+
}
395+
396+
// ValidateStepResults validates that all of the declared StepResults are valid.
397+
func ValidateStepResults(ctx context.Context, results []StepResult) (errs *apis.FieldError) {
398+
for index, result := range results {
399+
errs = errs.Also(result.Validate(ctx).ViaIndex(index))
400+
}
401+
return errs
402+
}
403+
404+
func validateSidecars(sidecars []Sidecar) (errs *apis.FieldError) {
405+
for _, sc := range sidecars {
406+
errs = errs.Also(validateSidecarName(sc))
407+
408+
if sc.Image == "" {
409+
errs = errs.Also(apis.ErrMissingField("image"))
410+
}
411+
412+
if sc.Script != "" {
413+
if len(sc.Command) > 0 {
414+
errs = errs.Also(&apis.FieldError{
415+
Message: "script cannot be used with command",
416+
Paths: []string{"script"},
417+
})
418+
}
419+
}
420+
}
421+
return errs
422+
}
423+
424+
func validateSidecarName(sc Sidecar) (errs *apis.FieldError) {
425+
if sc.Name == pipeline.ReservedResultsSidecarName {
426+
errs = errs.Also(&apis.FieldError{
427+
Message: fmt.Sprintf("Invalid: cannot use reserved sidecar name %v ", sc.Name),
428+
Paths: []string{"name"},
429+
})
430+
}
431+
return errs
432+
}

‎pkg/apis/pipeline/v1/container_validation_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func TestRef_Valid(t *testing.T) {
4848
name: "simple ref",
4949
ref: &v1.Ref{Name: "refname"},
5050
}, {
51-
name: "ref name - consice syntax",
51+
name: "ref name - concise syntax",
5252
ref: &v1.Ref{Name: "foo://baz:ver", ResolverRef: v1.ResolverRef{Resolver: "git"}},
5353
wc: enableConciseResolverSyntax,
5454
}, {

0 commit comments

Comments
 (0)