Skip to content

Commit 58cd6e2

Browse files
twoGiantstekton-robot
twoGiants
authored and
tekton-robot
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 #7442. Signed-off-by: Stanislav Jakuschevskij <[email protected]>
1 parent 8700459 commit 58cd6e2

File tree

3 files changed

+319
-320
lines changed

3 files changed

+319
-320
lines changed

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

+303-9
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,28 @@ 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+
"k8s.io/apimachinery/pkg/util/sets"
2733
"k8s.io/apimachinery/pkg/util/validation"
2834
"knative.dev/pkg/apis"
2935
)
3036

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

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

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