Skip to content

Commit ce9363f

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. Create `validateSidecar` and move to `container_validation.go` to be consistent with the `validateStep` function location. Inline `validateSidecarName` function into `validateSidecar` to keep consistency because the fields `sc.Image` and `sc.Script` are validated in the caller. Keep `validationSteps` and `validateSidecars` in `task_validation.go`. Issue tektoncd#7442. Signed-off-by: Stanislav Jakuschevskij <[email protected]>
1 parent 0f5882c commit ce9363f

File tree

3 files changed

+352
-353
lines changed

3 files changed

+352
-353
lines changed

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

+336-9
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,30 @@ import (
2020
"context"
2121
"errors"
2222
"fmt"
23+
"reflect"
2324
"regexp"
25+
"slices"
2426
"strings"
27+
"time"
2528

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

‎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)