Skip to content

Commit

Permalink
refactor param array indexing validation
Browse files Browse the repository at this point in the history 
This commit refactors param array indexing validation to extract all
references first, then validate if references are out of bound.
Previously we traverse all references and collect invalid references
there. The benefit of this refactoring is that it would be convenient
for other validation to reuse these references. This commit also moves
some shared functions into a separate pkg.

Signed-off-by: Yongxuan Zhang [email protected]
  • Loading branch information
@Yongxuanzhang
Yongxuanzhang committed Mar 2, 2023
1 parent b858d7b commit 8ee5a41
Show file tree
Hide file tree
Showing 13 changed files with 893 additions and 914 deletions.
214 changes: 214 additions & 0 deletions pkg/apis/pipeline/v1beta1/param_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

resource "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1"
"github.com/tektoncd/pipeline/pkg/substitution"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets"
"knative.dev/pkg/apis"
)
Expand Down Expand Up @@ -55,6 +56,9 @@ type ParamSpec struct {
Default *ParamValue `json:"default,omitempty"`
}

// ParamSpecs is a list of ParamSpec
type ParamSpecs []ParamSpec

// PropertySpec defines the struct for object keys
type PropertySpec struct {
Type ParamType `json:"type,omitempty"`
Expand Down Expand Up @@ -110,6 +114,216 @@ type Param struct {
Value ParamValue `json:"value"`
}

// Params is a list of Param
type Params []Param

// extractParamValuesFromParams get all param values from params
func (ps Params) extractParamValuesFromParams() []string {
pvs := []string{}
for i := range ps {
pvs = append(pvs, ps[i].Value.StringVal)
pvs = append(pvs, ps[i].Value.ArrayVal...)
for _, v := range ps[i].Value.ObjectVal {
pvs = append(pvs, v)
}
}
return pvs
}

// extractParamArrayLengths extract and return the lengths of all array params
// Example of returned value: {"a-array-params": 2,"b-array-params": 2 }
func (ps Params) extractParamArrayLengths() map[string]int {
// Collect all array params
arrayParamsLengths := make(map[string]int)

// Collect array params lengths from params
for _, p := range ps {
if p.Value.Type == ParamTypeArray {
arrayParamsLengths[p.Name] = len(p.Value.ArrayVal)
}
}
return arrayParamsLengths
}

// extractParamArrayLengths extract and return the lengths of all array params
// Example of returned value: {"a-array-params": 2,"b-array-params": 2 }
func (ps ParamSpecs) extractParamArrayLengths() map[string]int {
// Collect all array params
arrayParamsLengths := make(map[string]int)

// Collect array params lengths from defaults
for _, p := range ps {
if p.Default != nil {
if p.Default.Type == ParamTypeArray {
arrayParamsLengths[p.Name] = len(p.Default.ArrayVal)
}
}
}
return arrayParamsLengths
}

// validateOutofBoundArrayParams validates if the array indexing params are out of bound
// example of arrayIndexingParams: ["$(params.a-array-param[1])", "$(params.b-array-param[2])"]
// example of arrayParamsLengths: {"a-array-params": 2,"b-array-params": 2 }
func validateOutofBoundArrayParams(arrayIndexingParams []string, arrayParamsLengths map[string]int) error {
outofBoundParams := sets.String{}
for _, val := range arrayIndexingParams {
indexString := substitution.ExtractIndexString(val)
idx, _ := substitution.ExtractIndex(indexString)
// this will extract the param name from reference
// e.g. $(params.a-array-param[1]) -> a-array-param
paramName, _, _ := substitution.ExtractVariablesFromString(substitution.TrimArrayIndex(val), "params")

if paramLength, ok := arrayParamsLengths[paramName[0]]; ok {
if idx >= paramLength {
outofBoundParams.Insert(val)
}
}
}
if outofBoundParams.Len() > 0 {
return fmt.Errorf("non-existent param references:%v", outofBoundParams.List())
}
return nil
}

// extractArrayIndexingParamRefs takes a string of the form `foo-$(params.array-param[1])-bar` and extracts the portions of the string that reference an element in an array param.
// For example, for the string “foo-$(params.array-param[1])-bar-$(params.other-array-param[2])-$(params.string-param)`,
// it would return ["$(params.array-param[1])", "$(params.other-array-param[2])"].
func extractArrayIndexingParamRefs(paramReference string) []string {
l := []string{}
list := substitution.ExtractParamsExpressions(paramReference)
for _, val := range list {
indexString := substitution.ExtractIndexString(val)
if indexString != "" {
l = append(l, val)
}
}
return l
}

// extractParamRefsFromSteps get all array indexing references from steps
func extractParamRefsFromSteps(steps []Step) []string {
paramsRefs := []string{}
for _, step := range steps {
paramsRefs = append(paramsRefs, step.Script)
container := step.ToK8sContainer()
paramsRefs = append(paramsRefs, extractParamRefsFromContainer(container)...)
}
return paramsRefs
}

// extractParamRefsFromStepTemplate get all array indexing references from StepsTemplate
func extractParamRefsFromStepTemplate(stepTemplate *StepTemplate) []string {
if stepTemplate == nil {
return nil
}
container := stepTemplate.ToK8sContainer()
return extractParamRefsFromContainer(container)
}

// extractParamRefsFromSidecars get all array indexing references from sidecars
func extractParamRefsFromSidecars(sidecars []Sidecar) []string {
paramsRefs := []string{}
for _, s := range sidecars {
paramsRefs = append(paramsRefs, s.Script)
container := s.ToK8sContainer()
paramsRefs = append(paramsRefs, extractParamRefsFromContainer(container)...)
}
return paramsRefs
}

// extractParamRefsFromVolumes get all array indexing references from volumes
func extractParamRefsFromVolumes(volumes []corev1.Volume) []string {
paramsRefs := []string{}
for i, v := range volumes {
paramsRefs = append(paramsRefs, v.Name)
if v.VolumeSource.ConfigMap != nil {
paramsRefs = append(paramsRefs, v.ConfigMap.Name)
for _, item := range v.ConfigMap.Items {
paramsRefs = append(paramsRefs, item.Key)
paramsRefs = append(paramsRefs, item.Path)
}
}
if v.VolumeSource.Secret != nil {
paramsRefs = append(paramsRefs, v.Secret.SecretName)
for _, item := range v.Secret.Items {
paramsRefs = append(paramsRefs, item.Key)
paramsRefs = append(paramsRefs, item.Path)
}
}
if v.PersistentVolumeClaim != nil {
paramsRefs = append(paramsRefs, v.PersistentVolumeClaim.ClaimName)
}
if v.Projected != nil {
for _, s := range volumes[i].Projected.Sources {
if s.ConfigMap != nil {
paramsRefs = append(paramsRefs, s.ConfigMap.Name)
}
if s.Secret != nil {
paramsRefs = append(paramsRefs, s.Secret.Name)
}
if s.ServiceAccountToken != nil {
paramsRefs = append(paramsRefs, s.ServiceAccountToken.Audience)
}
}
}
if v.CSI != nil {
if v.CSI.NodePublishSecretRef != nil {
paramsRefs = append(paramsRefs, v.CSI.NodePublishSecretRef.Name)
}
if v.CSI.VolumeAttributes != nil {
for _, value := range v.CSI.VolumeAttributes {
paramsRefs = append(paramsRefs, value)
}
}
}
}
return paramsRefs
}

// extractParamRefsFromContainer get all array indexing references from container
func extractParamRefsFromContainer(c *corev1.Container) []string {
paramsRefs := []string{}
paramsRefs = append(paramsRefs, c.Name)
paramsRefs = append(paramsRefs, c.Image)
paramsRefs = append(paramsRefs, string(c.ImagePullPolicy))
paramsRefs = append(paramsRefs, c.Args...)

for ie, e := range c.Env {
paramsRefs = append(paramsRefs, e.Value)
if c.Env[ie].ValueFrom != nil {
if e.ValueFrom.SecretKeyRef != nil {
paramsRefs = append(paramsRefs, e.ValueFrom.SecretKeyRef.LocalObjectReference.Name)
paramsRefs = append(paramsRefs, e.ValueFrom.SecretKeyRef.Key)
}
if e.ValueFrom.ConfigMapKeyRef != nil {
paramsRefs = append(paramsRefs, e.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name)
paramsRefs = append(paramsRefs, e.ValueFrom.ConfigMapKeyRef.Key)
}
}
}

for _, e := range c.EnvFrom {
paramsRefs = append(paramsRefs, e.Prefix)
if e.ConfigMapRef != nil {
paramsRefs = append(paramsRefs, e.ConfigMapRef.LocalObjectReference.Name)
}
if e.SecretRef != nil {
paramsRefs = append(paramsRefs, e.SecretRef.LocalObjectReference.Name)
}
}

paramsRefs = append(paramsRefs, c.WorkingDir)
paramsRefs = append(paramsRefs, c.Command...)

for _, v := range c.VolumeMounts {
paramsRefs = append(paramsRefs, v.Name)
paramsRefs = append(paramsRefs, v.MountPath)
paramsRefs = append(paramsRefs, v.SubPath)
}
return paramsRefs
}

// ParamType indicates the type of an input parameter;
// Used to distinguish between a single string and an array of strings.
type ParamType string
Expand Down
8 changes: 4 additions & 4 deletions pkg/apis/pipeline/v1beta1/pipeline_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ type PipelineSpec struct {
// Params declares a list of input parameters that must be supplied when
// this Pipeline is run.
// +listType=atomic
Params []ParamSpec `json:"params,omitempty"`
Params ParamSpecs `json:"params,omitempty"`
// Workspaces declares a set of named workspaces that are expected to be
// provided by a PipelineRun.
// +optional
Expand Down Expand Up @@ -168,7 +168,7 @@ type Matrix struct {
// Each array element is supplied to the `PipelineTask` by substituting `params` of type `"string"` in the underlying `Task`.
// The names of the `params` in the `Matrix` must match the names of the `params` in the underlying `Task` that they will be substituting.
// +listType=atomic
Params []Param `json:"params,omitempty"`
Params Params `json:"params,omitempty"`

// Include is a list of MatrixInclude which allows passing in specific combinations of Parameters into the Matrix.
// Note that Include is in preview mode and not yet supported.
Expand All @@ -186,7 +186,7 @@ type MatrixInclude struct {
// Params takes only `Parameters` of type `"string"`
// The names of the `params` must match the names of the `params` in the underlying `Task`
// +listType=atomic
Params []Param `json:"params,omitempty"`
Params Params `json:"params,omitempty"`
}

// PipelineTask defines a task in a Pipeline, passing inputs from both
Expand Down Expand Up @@ -227,7 +227,7 @@ type PipelineTask struct {
// Parameters declares parameters passed to this task.
// +optional
// +listType=atomic
Params []Param `json:"params,omitempty"`
Params Params `json:"params,omitempty"`

// Matrix declares parameters used to fan out this task.
// +optional
Expand Down
48 changes: 48 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -535,3 +535,51 @@ func validateResultsFromMatrixedPipelineTasksNotConsumed(tasks []PipelineTask, f
}
return errs
}

// ValidateParamArrayIndex validates if the param reference to an array param is out of bound.
// error is returned when the array indexing reference is out of bound of the array param
// e.g. if a param reference of $(params.array-param[2]) and the array param is of length 2.
func (ps *PipelineSpec) ValidateParamArrayIndex(ctx context.Context, params Params) error {
if !config.CheckAlphaOrBetaAPIFields(ctx) {
return nil
}

// Collect all array params lengths
arrayParamsLengths := ps.Params.extractParamArrayLengths()
for k, v := range params.extractParamArrayLengths() {
arrayParamsLengths[k] = v
}

paramsRefs := []string{}
for i := range ps.Tasks {
paramsRefs = append(paramsRefs, ps.Tasks[i].Params.extractParamValuesFromParams()...)
if ps.Tasks[i].IsMatrixed() {
paramsRefs = append(paramsRefs, ps.Tasks[i].Matrix.Params.extractParamValuesFromParams()...)
}
for j := range ps.Tasks[i].Workspaces {
paramsRefs = append(paramsRefs, ps.Tasks[i].Workspaces[j].SubPath)
}
for _, wes := range ps.Tasks[i].WhenExpressions {
paramsRefs = append(paramsRefs, wes.Input)
paramsRefs = append(paramsRefs, wes.Values...)
}
}

for i := range ps.Finally {
paramsRefs = append(paramsRefs, ps.Finally[i].Params.extractParamValuesFromParams()...)
if ps.Finally[i].IsMatrixed() {
paramsRefs = append(paramsRefs, ps.Finally[i].Matrix.Params.extractParamValuesFromParams()...)
}
for _, wes := range ps.Finally[i].WhenExpressions {
paramsRefs = append(paramsRefs, wes.Values...)
}
}

// extract all array indexing references, for example []{"$(params.array-params[1])"}
arrayIndexParamRefs := []string{}
for _, p := range paramsRefs {
arrayIndexParamRefs = append(arrayIndexParamRefs, extractArrayIndexingParamRefs(p)...)
}

return validateOutofBoundArrayParams(arrayIndexParamRefs, arrayParamsLengths)
}
Loading

0 comments on commit 8ee5a41

Please sign in to comment.