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.

Sign up for GitHub

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

Jump to bottom

fix:Refactored Steps and Sidecars to container_validation #7454 #7538

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix:Refactored Steps and Sidecars to container_validation #7454 #7538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
163 changes: 163 additions & 0 deletions pkg/apis/pipeline/v1/container_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@ package v1

import (
"context"
"fmt"
"strings"
"time"

"github.com/tektoncd/pipeline/pkg/apis/config"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation"
"knative.dev/pkg/apis"
)
Expand Down Expand Up @@ -60,3 +63,163 @@ func (ref *Ref) Validate(ctx context.Context) (errs *apis.FieldError) {
}
return errs
}

func validateSidecars(sidecars []Sidecar) (errs *apis.FieldError) {
for _, sc := range sidecars {
errs = errs.Also(validateSidecarName(sc))

if sc.Image == "" {
errs = errs.Also(apis.ErrMissingField("image"))
}

if sc.Script != "" {
if len(sc.Command) > 0 {
errs = errs.Also(&apis.FieldError{
Message: "script cannot be used with command",
Paths: []string{"script"},
})
}
}
}
return errs
}

func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.FieldError) {
if s.Ref != nil {
if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions && isCreateOrUpdateAndDiverged(ctx, s) {
return apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true to reference StepActions in Steps.", config.EnableStepActions), "")
}
errs = errs.Also(s.Ref.Validate(ctx))
if s.Image != "" {
errs = errs.Also(&apis.FieldError{
Message: "image cannot be used with Ref",
Paths: []string{"image"},
})
}
if len(s.Command) > 0 {
errs = errs.Also(&apis.FieldError{
Message: "command cannot be used with Ref",
Paths: []string{"command"},
})
}
if len(s.Args) > 0 {
errs = errs.Also(&apis.FieldError{
Message: "args cannot be used with Ref",
Paths: []string{"args"},
})
}
if s.Script != "" {
errs = errs.Also(&apis.FieldError{
Message: "script cannot be used with Ref",
Paths: []string{"script"},
})
}
if s.WorkingDir != "" {
errs = errs.Also(&apis.FieldError{
Message: "working dir cannot be used with Ref",
Paths: []string{"workingDir"},
})
}
if s.Env != nil {
errs = errs.Also(&apis.FieldError{
Message: "env cannot be used with Ref",
Paths: []string{"env"},
})
}
if len(s.VolumeMounts) > 0 {
errs = errs.Also(&apis.FieldError{
Message: "volumeMounts cannot be used with Ref",
Paths: []string{"volumeMounts"},
})
}
if len(s.Results) > 0 {
errs = errs.Also(&apis.FieldError{
Message: "results cannot be used with Ref",
Paths: []string{"results"},
})
}
} else {
if len(s.Params) > 0 {
errs = errs.Also(&apis.FieldError{
Message: "params cannot be used without Ref",
Paths: []string{"params"},
})
}
if len(s.Results) > 0 {
if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions && isCreateOrUpdateAndDiverged(ctx, s) {
return apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true in order to use Results in Steps.", config.EnableStepActions), "")
}
}
if s.Image == "" {
errs = errs.Also(apis.ErrMissingField("Image"))
}

if s.Script != "" {
if len(s.Command) > 0 {
errs = errs.Also(&apis.FieldError{
Message: "script cannot be used with command",
Paths: []string{"script"},
})
}
}
}

if s.Name != "" {
if names.Has(s.Name) {
errs = errs.Also(apis.ErrInvalidValue(s.Name, "name"))
}
if e := validation.IsDNS1123Label(s.Name); len(e) > 0 {
errs = errs.Also(&apis.FieldError{
Message: fmt.Sprintf("invalid value %q", s.Name),
Paths: []string{"name"},
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",
})
}
names.Insert(s.Name)
}

if s.Timeout != nil {
if s.Timeout.Duration < time.Duration(0) {
return apis.ErrInvalidValue(s.Timeout.Duration, "negative timeout")
}
}

for j, vm := range s.VolumeMounts {
if strings.HasPrefix(vm.MountPath, "/tekton/") &&
!strings.HasPrefix(vm.MountPath, "/tekton/home") {
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))
}
if strings.HasPrefix(vm.Name, "tekton-internal-") {
errs = errs.Also(apis.ErrGeneric(fmt.Sprintf(`volumeMount name %q cannot start with "tekton-internal-"`, vm.Name), "name").ViaFieldIndex("volumeMounts", j))
}
}

if s.OnError != "" {
if !isParamRefs(string(s.OnError)) && s.OnError != Continue && s.OnError != StopAndFail {
errs = errs.Also(&apis.FieldError{
Message: fmt.Sprintf("invalid value: \"%v\"", s.OnError),
Paths: []string{"onError"},
Details: "Task step onError must be either \"continue\" or \"stopAndFail\"",
})
}
}

if s.Script != "" {
cleaned := strings.TrimSpace(s.Script)
if strings.HasPrefix(cleaned, "#!win") {
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "windows script support", config.AlphaAPIFields).ViaField("script"))
}
}

// StdoutConfig is an alpha feature and will fail validation if it's used in a task spec
// when the enable-api-fields feature gate is not "alpha".
if s.StdoutConfig != nil {
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "step stdout stream support", config.AlphaAPIFields).ViaField("stdoutconfig"))
}
// StderrConfig is an alpha feature and will fail validation if it's used in a task spec
// when the enable-api-fields feature gate is not "alpha".
if s.StderrConfig != nil {
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "step stderr stream support", config.AlphaAPIFields).ViaField("stderrconfig"))
}
return errs
}
162 changes: 0 additions & 162 deletions pkg/apis/pipeline/v1/task_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"reflect"
"regexp"
"strings"
"time"

"github.com/tektoncd/pipeline/pkg/apis/config"
"github.com/tektoncd/pipeline/pkg/apis/pipeline"
Expand All @@ -32,7 +31,6 @@ import (
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation"
"knative.dev/pkg/apis"
"knative.dev/pkg/webhook/resourcesemantics"
)
Expand Down Expand Up @@ -149,26 +147,6 @@ func ValidateObjectParamsHaveProperties(ctx context.Context, params ParamSpecs)
return errs
}

func validateSidecars(sidecars []Sidecar) (errs *apis.FieldError) {
for _, sc := range sidecars {
errs = errs.Also(validateSidecarName(sc))

if sc.Image == "" {
errs = errs.Also(apis.ErrMissingField("image"))
}

if sc.Script != "" {
if len(sc.Command) > 0 {
errs = errs.Also(&apis.FieldError{
Message: "script cannot be used with command",
Paths: []string{"script"},
})
}
}
}
return errs
}

func validateSidecarName(sc Sidecar) (errs *apis.FieldError) {
if sc.Name == pipeline.ReservedResultsSidecarName {
errs = errs.Also(&apis.FieldError{
Expand Down Expand Up @@ -322,146 +300,6 @@ func isCreateOrUpdateAndDiverged(ctx context.Context, s Step) bool {
return false
}

func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.FieldError) {
if s.Ref != nil {
if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions && isCreateOrUpdateAndDiverged(ctx, s) {
return apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true to reference StepActions in Steps.", config.EnableStepActions), "")
}
errs = errs.Also(s.Ref.Validate(ctx))
if s.Image != "" {
errs = errs.Also(&apis.FieldError{
Message: "image cannot be used with Ref",
Paths: []string{"image"},
})
}
if len(s.Command) > 0 {
errs = errs.Also(&apis.FieldError{
Message: "command cannot be used with Ref",
Paths: []string{"command"},
})
}
if len(s.Args) > 0 {
errs = errs.Also(&apis.FieldError{
Message: "args cannot be used with Ref",
Paths: []string{"args"},
})
}
if s.Script != "" {
errs = errs.Also(&apis.FieldError{
Message: "script cannot be used with Ref",
Paths: []string{"script"},
})
}
if s.WorkingDir != "" {
errs = errs.Also(&apis.FieldError{
Message: "working dir cannot be used with Ref",
Paths: []string{"workingDir"},
})
}
if s.Env != nil {
errs = errs.Also(&apis.FieldError{
Message: "env cannot be used with Ref",
Paths: []string{"env"},
})
}
if len(s.VolumeMounts) > 0 {
errs = errs.Also(&apis.FieldError{
Message: "volumeMounts cannot be used with Ref",
Paths: []string{"volumeMounts"},
})
}
if len(s.Results) > 0 {
errs = errs.Also(&apis.FieldError{
Message: "results cannot be used with Ref",
Paths: []string{"results"},
})
}
} else {
if len(s.Params) > 0 {
errs = errs.Also(&apis.FieldError{
Message: "params cannot be used without Ref",
Paths: []string{"params"},
})
}
if len(s.Results) > 0 {
if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions && isCreateOrUpdateAndDiverged(ctx, s) {
return apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true in order to use Results in Steps.", config.EnableStepActions), "")
}
}
if s.Image == "" {
errs = errs.Also(apis.ErrMissingField("Image"))
}

if s.Script != "" {
if len(s.Command) > 0 {
errs = errs.Also(&apis.FieldError{
Message: "script cannot be used with command",
Paths: []string{"script"},
})
}
}
}

if s.Name != "" {
if names.Has(s.Name) {
errs = errs.Also(apis.ErrInvalidValue(s.Name, "name"))
}
if e := validation.IsDNS1123Label(s.Name); len(e) > 0 {
errs = errs.Also(&apis.FieldError{
Message: fmt.Sprintf("invalid value %q", s.Name),
Paths: []string{"name"},
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",
})
}
names.Insert(s.Name)
}

if s.Timeout != nil {
if s.Timeout.Duration < time.Duration(0) {
return apis.ErrInvalidValue(s.Timeout.Duration, "negative timeout")
}
}

for j, vm := range s.VolumeMounts {
if strings.HasPrefix(vm.MountPath, "/tekton/") &&
!strings.HasPrefix(vm.MountPath, "/tekton/home") {
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))
}
if strings.HasPrefix(vm.Name, "tekton-internal-") {
errs = errs.Also(apis.ErrGeneric(fmt.Sprintf(`volumeMount name %q cannot start with "tekton-internal-"`, vm.Name), "name").ViaFieldIndex("volumeMounts", j))
}
}

if s.OnError != "" {
if !isParamRefs(string(s.OnError)) && s.OnError != Continue && s.OnError != StopAndFail {
errs = errs.Also(&apis.FieldError{
Message: fmt.Sprintf("invalid value: \"%v\"", s.OnError),
Paths: []string{"onError"},
Details: "Task step onError must be either \"continue\" or \"stopAndFail\"",
})
}
}

if s.Script != "" {
cleaned := strings.TrimSpace(s.Script)
if strings.HasPrefix(cleaned, "#!win") {
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "windows script support", config.AlphaAPIFields).ViaField("script"))
}
}

// StdoutConfig is an alpha feature and will fail validation if it's used in a task spec
// when the enable-api-fields feature gate is not "alpha".
if s.StdoutConfig != nil {
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "step stdout stream support", config.AlphaAPIFields).ViaField("stdoutconfig"))
}
// StderrConfig is an alpha feature and will fail validation if it's used in a task spec
// when the enable-api-fields feature gate is not "alpha".
if s.StderrConfig != nil {
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "step stderr stream support", config.AlphaAPIFields).ViaField("stderrconfig"))
}
return errs
}

// ValidateParameterTypes validates all the types within a slice of ParamSpecs
func ValidateParameterTypes(ctx context.Context, params []ParamSpec) (errs *apis.FieldError) {
for _, p := range params {
Expand Down