From f1613f7156b032ba15e91e7a490d1e14406714dc Mon Sep 17 00:00:00 2001 From: Sascha Schwarze Date: Wed, 19 Jan 2022 16:56:37 +0100 Subject: [PATCH 1/5] API updates --- deploy/crds/shipwright.io_buildruns.yaml | 176 +++++++++++++++++- deploy/crds/shipwright.io_builds.yaml | 87 ++++++++- .../crds/shipwright.io_buildstrategies.yaml | 11 +- .../shipwright.io_clusterbuildstrategies.yaml | 11 +- pkg/apis/build/v1alpha1/build_types.go | 10 + pkg/apis/build/v1alpha1/buildstrategy.go | 20 +- pkg/apis/build/v1alpha1/parameter.go | 46 ++++- .../build/v1alpha1/zz_generated.deepcopy.go | 81 +++++++- .../buildrun/resources/conditions.go | 31 +-- 9 files changed, 452 insertions(+), 21 deletions(-) diff --git a/deploy/crds/shipwright.io_buildruns.yaml b/deploy/crds/shipwright.io_buildruns.yaml index 61b42918c0..25ad7c8187 100644 --- a/deploy/crds/shipwright.io_buildruns.yaml +++ b/deploy/crds/shipwright.io_buildruns.yaml @@ -211,13 +211,98 @@ spec: description: ParamValue is a key/value that populates a strategy parameter used in the execution of the strategy steps properties: + configMapValue: + description: The ConfigMap value of the parameter + properties: + format: + description: An optional format to add pre- or suffix to + the object value. For example 'KEY=${SECRET_VALUE}' or + 'KEY=${CONFIGMAP_VALUE}' depending on the context. + type: string + key: + description: Key inside the object + type: string + name: + description: Name of the object + type: string + required: + - key + - name + type: object name: + description: Name of the parameter type: string + secretValue: + description: The secret value of the parameter + properties: + format: + description: An optional format to add pre- or suffix to + the object value. For example 'KEY=${SECRET_VALUE}' or + 'KEY=${CONFIGMAP_VALUE}' depending on the context. + type: string + key: + description: Key inside the object + type: string + name: + description: Name of the object + type: string + required: + - key + - name + type: object value: + description: The value of the parameter type: string + values: + description: Values of an array parameter + items: + description: The value type contains the properties for a + value, this allows for an easy extension in the future to + support more kinds + properties: + configMapValue: + description: The ConfigMap value of the parameter + properties: + format: + description: An optional format to add pre- or suffix + to the object value. For example 'KEY=${SECRET_VALUE}' + or 'KEY=${CONFIGMAP_VALUE}' depending on the context. + type: string + key: + description: Key inside the object + type: string + name: + description: Name of the object + type: string + required: + - key + - name + type: object + secretValue: + description: The secret value of the parameter + properties: + format: + description: An optional format to add pre- or suffix + to the object value. For example 'KEY=${SECRET_VALUE}' + or 'KEY=${CONFIGMAP_VALUE}' depending on the context. + type: string + key: + description: Key inside the object + type: string + name: + description: Name of the object + type: string + required: + - key + - name + type: object + value: + description: The value of the parameter + type: string + type: object + type: array required: - name - - value type: object type: array serviceAccount: @@ -459,13 +544,100 @@ spec: description: ParamValue is a key/value that populates a strategy parameter used in the execution of the strategy steps properties: + configMapValue: + description: The ConfigMap value of the parameter + properties: + format: + description: An optional format to add pre- or suffix + to the object value. For example 'KEY=${SECRET_VALUE}' + or 'KEY=${CONFIGMAP_VALUE}' depending on the context. + type: string + key: + description: Key inside the object + type: string + name: + description: Name of the object + type: string + required: + - key + - name + type: object name: + description: Name of the parameter type: string + secretValue: + description: The secret value of the parameter + properties: + format: + description: An optional format to add pre- or suffix + to the object value. For example 'KEY=${SECRET_VALUE}' + or 'KEY=${CONFIGMAP_VALUE}' depending on the context. + type: string + key: + description: Key inside the object + type: string + name: + description: Name of the object + type: string + required: + - key + - name + type: object value: + description: The value of the parameter type: string + values: + description: Values of an array parameter + items: + description: The value type contains the properties for + a value, this allows for an easy extension in the future + to support more kinds + properties: + configMapValue: + description: The ConfigMap value of the parameter + properties: + format: + description: An optional format to add pre- or + suffix to the object value. For example 'KEY=${SECRET_VALUE}' + or 'KEY=${CONFIGMAP_VALUE}' depending on the + context. + type: string + key: + description: Key inside the object + type: string + name: + description: Name of the object + type: string + required: + - key + - name + type: object + secretValue: + description: The secret value of the parameter + properties: + format: + description: An optional format to add pre- or + suffix to the object value. For example 'KEY=${SECRET_VALUE}' + or 'KEY=${CONFIGMAP_VALUE}' depending on the + context. + type: string + key: + description: Key inside the object + type: string + name: + description: Name of the object + type: string + required: + - key + - name + type: object + value: + description: The value of the parameter + type: string + type: object + type: array required: - name - - value type: object type: array source: diff --git a/deploy/crds/shipwright.io_builds.yaml b/deploy/crds/shipwright.io_builds.yaml index e663b205a9..7c14ec98fc 100644 --- a/deploy/crds/shipwright.io_builds.yaml +++ b/deploy/crds/shipwright.io_builds.yaml @@ -235,13 +235,98 @@ spec: description: ParamValue is a key/value that populates a strategy parameter used in the execution of the strategy steps properties: + configMapValue: + description: The ConfigMap value of the parameter + properties: + format: + description: An optional format to add pre- or suffix to + the object value. For example 'KEY=${SECRET_VALUE}' or + 'KEY=${CONFIGMAP_VALUE}' depending on the context. + type: string + key: + description: Key inside the object + type: string + name: + description: Name of the object + type: string + required: + - key + - name + type: object name: + description: Name of the parameter type: string + secretValue: + description: The secret value of the parameter + properties: + format: + description: An optional format to add pre- or suffix to + the object value. For example 'KEY=${SECRET_VALUE}' or + 'KEY=${CONFIGMAP_VALUE}' depending on the context. + type: string + key: + description: Key inside the object + type: string + name: + description: Name of the object + type: string + required: + - key + - name + type: object value: + description: The value of the parameter type: string + values: + description: Values of an array parameter + items: + description: The value type contains the properties for a + value, this allows for an easy extension in the future to + support more kinds + properties: + configMapValue: + description: The ConfigMap value of the parameter + properties: + format: + description: An optional format to add pre- or suffix + to the object value. For example 'KEY=${SECRET_VALUE}' + or 'KEY=${CONFIGMAP_VALUE}' depending on the context. + type: string + key: + description: Key inside the object + type: string + name: + description: Name of the object + type: string + required: + - key + - name + type: object + secretValue: + description: The secret value of the parameter + properties: + format: + description: An optional format to add pre- or suffix + to the object value. For example 'KEY=${SECRET_VALUE}' + or 'KEY=${CONFIGMAP_VALUE}' depending on the context. + type: string + key: + description: Key inside the object + type: string + name: + description: Name of the object + type: string + required: + - key + - name + type: object + value: + description: The value of the parameter + type: string + type: object + type: array required: - name - - value type: object type: array source: diff --git a/deploy/crds/shipwright.io_buildstrategies.yaml b/deploy/crds/shipwright.io_buildstrategies.yaml index 833aa4d3fe..519ae677fe 100644 --- a/deploy/crds/shipwright.io_buildstrategies.yaml +++ b/deploy/crds/shipwright.io_buildstrategies.yaml @@ -1185,14 +1185,23 @@ spec: object. properties: default: - description: Reasonable default value for the parameter + description: Default value for a string parameter type: string + defaults: + description: Default values for an array parameter + items: + type: string + type: array description: description: Description on the parameter purpose type: string name: description: Name of the parameter type: string + type: + description: Type of the parameter. The possible types are "string" + and "array", and "string" is the default. + type: string required: - description - name diff --git a/deploy/crds/shipwright.io_clusterbuildstrategies.yaml b/deploy/crds/shipwright.io_clusterbuildstrategies.yaml index b4970b435b..34654ecdaf 100644 --- a/deploy/crds/shipwright.io_clusterbuildstrategies.yaml +++ b/deploy/crds/shipwright.io_clusterbuildstrategies.yaml @@ -1185,14 +1185,23 @@ spec: object. properties: default: - description: Reasonable default value for the parameter + description: Default value for a string parameter type: string + defaults: + description: Default values for an array parameter + items: + type: string + type: array description: description: Description on the parameter purpose type: string name: description: Name of the parameter type: string + type: + description: Type of the parameter. The possible types are "string" + and "array", and "string" is the default. + type: string required: - description - name diff --git a/pkg/apis/build/v1alpha1/build_types.go b/pkg/apis/build/v1alpha1/build_types.go index 63a0b710c2..cad38fb497 100644 --- a/pkg/apis/build/v1alpha1/build_types.go +++ b/pkg/apis/build/v1alpha1/build_types.go @@ -38,8 +38,18 @@ const ( RuntimePathsCanNotBeEmpty BuildReason = "RuntimePathsCanNotBeEmpty" // RestrictedParametersInUse indicates the definition of reserved shipwright parameters RestrictedParametersInUse BuildReason = "RestrictedParametersInUse" + // WrongParameterValueType indicates that a single value was provided for an array parameter, or vice-versa + WrongParameterValueType BuildReason = "WrongParameterValueType" // UndefinedParameter indicates the definition of param that was not defined in the strategy parameters UndefinedParameter BuildReason = "UndefinedParameter" + // InconsistentParameterValues indicates that parameter values have more than one of configMapValue, secretValue, or value set + InconsistentParameterValues BuildReason = "InconsistentParameterValues" + // EmptyArrayItemParameterValues indicates that array parameters contain an item where none of configMapValue, secretValue, or value is set + EmptyArrayItemParameterValues BuildReason = "EmptyArrayItemParameterValues" + // IncompleteConfigMapValueParameterValues indicates that a configMapValue is specified where the name or the key is empty + IncompleteConfigMapValueParameterValues BuildReason = "IncompleteConfigMapValueParameterValues" + // IncompleteSecretValueParameterValues indicates that a secretValue is specified where the name or the key is empty + IncompleteSecretValueParameterValues BuildReason = "IncompleteSecretValueParameterValues" // RemoteRepositoryUnreachable indicates the referenced repository is unreachable RemoteRepositoryUnreachable BuildReason = "RemoteRepositoryUnreachable" // BuildNameInvalid indicates the build name is invalid diff --git a/pkg/apis/build/v1alpha1/buildstrategy.go b/pkg/apis/build/v1alpha1/buildstrategy.go index b824a1fbd3..de56316def 100644 --- a/pkg/apis/build/v1alpha1/buildstrategy.go +++ b/pkg/apis/build/v1alpha1/buildstrategy.go @@ -22,6 +22,15 @@ type BuildStrategySpec struct { Parameters []Parameter `json:"parameters,omitempty"` } +// ParameterType indicates the type of a parameter +type ParameterType string + +// Valid ParamTypes: +const ( + ParameterTypeString ParameterType = "string" + ParameterTypeArray ParameterType = "array" +) + // Parameter holds a name-description with a default value // that allows strategy steps to be parameterize. // Build users can set a value for parameter via the Build @@ -36,9 +45,18 @@ type Parameter struct { // +required Description string `json:"description"` - // Reasonable default value for the parameter + // Type of the parameter. The possible types are "string" and "array", + // and "string" is the default. + // +optional + Type ParameterType `json:"type,omitempty"` + + // Default value for a string parameter // +optional Default *string `json:"default"` + + // Default values for an array parameter + // +optional + Defaults *[]string `json:"defaults"` } // BuildStep defines a partial step that needs to run in container for building the image. diff --git a/pkg/apis/build/v1alpha1/parameter.go b/pkg/apis/build/v1alpha1/parameter.go index 84fe3fc87d..0f6bfff3db 100644 --- a/pkg/apis/build/v1alpha1/parameter.go +++ b/pkg/apis/build/v1alpha1/parameter.go @@ -4,9 +4,51 @@ package v1alpha1 +type ObjectKeyRef struct { + + // Name of the object + // +required + Name string `json:"name"` + + // Key inside the object + // +required + Key string `json:"key"` + + // An optional format to add pre- or suffix to the object value. For example 'KEY=${SECRET_VALUE}' or 'KEY=${CONFIGMAP_VALUE}' depending on the context. + // +optional + Format *string `json:"format"` +} + +// The value type contains the properties for a value, this allows for an +// easy extension in the future to support more kinds +type SingleValue struct { + + // The value of the parameter + // +optional + Value *string `json:"value"` + + // The ConfigMap value of the parameter + // +optional + ConfigMapValue *ObjectKeyRef `json:"configMapValue"` + + // The secret value of the parameter + // +optional + SecretValue *ObjectKeyRef `json:"secretValue"` +} + // ParamValue is a key/value that populates a strategy parameter // used in the execution of the strategy steps type ParamValue struct { - Name string `json:"name"` - Value string `json:"value"` + + // Inline the properties of a value + // +optional + *SingleValue `json:",inline"` + + // Name of the parameter + // +required + Name string `json:"name"` + + // Values of an array parameter + // +optional + Values []SingleValue `json:"values,omitempty"` } diff --git a/pkg/apis/build/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/build/v1alpha1/zz_generated.deepcopy.go index 55a278e00c..170b354431 100644 --- a/pkg/apis/build/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/build/v1alpha1/zz_generated.deepcopy.go @@ -185,7 +185,9 @@ func (in *BuildRunSpec) DeepCopyInto(out *BuildRunSpec) { if in.ParamValues != nil { in, out := &in.ParamValues, &out.ParamValues *out = make([]ParamValue, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.Output != nil { in, out := &in.Output, &out.Output @@ -329,7 +331,9 @@ func (in *BuildSpec) DeepCopyInto(out *BuildSpec) { if in.ParamValues != nil { in, out := &in.ParamValues, &out.ParamValues *out = make([]ParamValue, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } in.Output.DeepCopyInto(&out.Output) if in.Timeout != nil { @@ -717,6 +721,27 @@ func (in *Image) DeepCopy() *Image { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ObjectKeyRef) DeepCopyInto(out *ObjectKeyRef) { + *out = *in + if in.Format != nil { + in, out := &in.Format, &out.Format + *out = new(string) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ObjectKeyRef. +func (in *ObjectKeyRef) DeepCopy() *ObjectKeyRef { + if in == nil { + return nil + } + out := new(ObjectKeyRef) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Output) DeepCopyInto(out *Output) { *out = *in @@ -736,6 +761,18 @@ func (in *Output) DeepCopy() *Output { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ParamValue) DeepCopyInto(out *ParamValue) { *out = *in + if in.SingleValue != nil { + in, out := &in.SingleValue, &out.SingleValue + *out = new(SingleValue) + (*in).DeepCopyInto(*out) + } + if in.Values != nil { + in, out := &in.Values, &out.Values + *out = make([]SingleValue, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } return } @@ -757,6 +794,15 @@ func (in *Parameter) DeepCopyInto(out *Parameter) { *out = new(string) **out = **in } + if in.Defaults != nil { + in, out := &in.Defaults, &out.Defaults + *out = new([]string) + if **in != nil { + in, out := *in, *out + *out = make([]string, len(*in)) + copy(*out, *in) + } + } return } @@ -791,6 +837,37 @@ func (in *ServiceAccount) DeepCopy() *ServiceAccount { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SingleValue) DeepCopyInto(out *SingleValue) { + *out = *in + if in.Value != nil { + in, out := &in.Value, &out.Value + *out = new(string) + **out = **in + } + if in.ConfigMapValue != nil { + in, out := &in.ConfigMapValue, &out.ConfigMapValue + *out = new(ObjectKeyRef) + (*in).DeepCopyInto(*out) + } + if in.SecretValue != nil { + in, out := &in.SecretValue, &out.SecretValue + *out = new(ObjectKeyRef) + (*in).DeepCopyInto(*out) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SingleValue. +func (in *SingleValue) DeepCopy() *SingleValue { + if in == nil { + return nil + } + out := new(SingleValue) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Source) DeepCopyInto(out *Source) { *out = *in diff --git a/pkg/reconciler/buildrun/resources/conditions.go b/pkg/reconciler/buildrun/resources/conditions.go index 897198c911..a8ca482c70 100644 --- a/pkg/reconciler/buildrun/resources/conditions.go +++ b/pkg/reconciler/buildrun/resources/conditions.go @@ -7,6 +7,7 @@ package resources import ( "context" "fmt" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "knative.dev/pkg/apis" "sigs.k8s.io/controller-runtime/pkg/client" @@ -21,17 +22,25 @@ import ( // Common condition strings for reason, kind, etc. const ( - ConditionUnknownStrategyKind string = "UnknownStrategyKind" - ClusterBuildStrategyNotFound string = "ClusterBuildStrategyNotFound" - BuildStrategyNotFound string = "BuildStrategyNotFound" - ConditionSetOwnerReferenceFailed string = "SetOwnerReferenceFailed" - ConditionFailed string = "Failed" - ConditionTaskRunIsMissing string = "TaskRunIsMissing" - ConditionTaskRunGenerationFailed string = "TaskRunGenerationFailed" - ConditionServiceAccountNotFound string = "ServiceAccountNotFound" - ConditionBuildRegistrationFailed string = "BuildRegistrationFailed" - ConditionBuildNotFound string = "BuildNotFound" - BuildRunNameInvalid string = "BuildRunNameInvalid" + ConditionUnknownStrategyKind string = "UnknownStrategyKind" + ClusterBuildStrategyNotFound string = "ClusterBuildStrategyNotFound" + BuildStrategyNotFound string = "BuildStrategyNotFound" + ConditionSetOwnerReferenceFailed string = "SetOwnerReferenceFailed" + ConditionFailed string = "Failed" + ConditionTaskRunIsMissing string = "TaskRunIsMissing" + ConditionTaskRunGenerationFailed string = "TaskRunGenerationFailed" + ConditionServiceAccountNotFound string = "ServiceAccountNotFound" + ConditionBuildRegistrationFailed string = "BuildRegistrationFailed" + ConditionBuildNotFound string = "BuildNotFound" + ConditionMissingParameterValues string = "MissingParameterValues" + ConditionRestrictedParametersInUse string = "RestrictedParametersInUse" + ConditionUndefinedParameter string = "UndefinedParameter" + ConditionWrongParameterValueType string = "WrongParameterValueType" + ConditionInconsistentParameterValues string = "InconsistentParameterValues" + ConditionEmptyArrayItemParameterValues string = "EmptyArrayItemParameterValues" + ConditionIncompleteConfigMapValueParameterValues string = "IncompleteConfigMapValueParameterValues" + ConditionIncompleteSecretValueParameterValues string = "IncompleteSecretValueParameterValues" + BuildRunNameInvalid string = "BuildRunNameInvalid" ) // UpdateBuildRunUsingTaskRunCondition updates the BuildRun Succeeded Condition From e3c4606304c6d9a22790ca90754a2d4be1a126f2 Mon Sep 17 00:00:00 2001 From: Sascha Schwarze Date: Wed, 19 Jan 2022 16:57:46 +0100 Subject: [PATCH 2/5] Implementation --- pkg/reconciler/buildrun/buildrun.go | 9 + pkg/reconciler/buildrun/resources/params.go | 546 ++++++++++++++++++- pkg/reconciler/buildrun/resources/taskrun.go | 91 ++-- pkg/validate/strategies.go | 109 ++-- 4 files changed, 618 insertions(+), 137 deletions(-) diff --git a/pkg/reconciler/buildrun/buildrun.go b/pkg/reconciler/buildrun/buildrun.go index 79c1d72954..0d2ba67813 100644 --- a/pkg/reconciler/buildrun/buildrun.go +++ b/pkg/reconciler/buildrun/buildrun.go @@ -215,6 +215,15 @@ func (r *ReconcileBuildRun) Reconcile(ctx context.Context, request reconcile.Req return reconcile.Result{}, err } + // Validate the parameters + valid, reason, message := resources.ValidateBuildRunParameters(strategy.GetParameters(), build.Spec.ParamValues, buildRun.Spec.ParamValues) + if !valid { + if err := resources.UpdateConditionWithFalseStatus(ctx, r.client, buildRun, message, reason); err != nil { + return reconcile.Result{}, err + } + return reconcile.Result{}, nil + } + // Create the TaskRun, this needs to be the last step in this block to be idempotent generatedTaskRun, err := r.createTaskRun(ctx, svcAccount, strategy, build, buildRun) if err != nil { diff --git a/pkg/reconciler/buildrun/resources/params.go b/pkg/reconciler/buildrun/resources/params.go index cf408aa899..7a6ad7f444 100644 --- a/pkg/reconciler/buildrun/resources/params.go +++ b/pkg/reconciler/buildrun/resources/params.go @@ -5,16 +5,28 @@ package resources import ( + "crypto/rand" + "fmt" + "math/big" "strings" buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" + pipeline "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + corev1 "k8s.io/api/core/v1" ) -var systemReservedParamKeys = map[string]bool{ - "BUILDER_IMAGE": true, - "DOCKERFILE": true, - "CONTEXT_DIR": true, -} +const ( + envVarNameSuffixChars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" +) + +var ( + envVarNameSuffixCharsCount = big.NewInt(int64(len(envVarNameSuffixChars))) + systemReservedParamKeys = map[string]bool{ + "BUILDER_IMAGE": true, + "DOCKERFILE": true, + "CONTEXT_DIR": true, + } +) // overrideParams allows to override an existing list of parameters with a second list, // as long as their entry names matches @@ -53,3 +65,527 @@ func overrideParams(originalParams []buildv1alpha1.ParamValue, overrideParams [] func IsSystemReservedParameter(param string) bool { return systemReservedParamKeys[param] || strings.HasPrefix(param, "shp-") } + +// FindParameterByName returns the first entry in a Parameter array with a specified name, or nil +func FindParameterByName(parameters []buildv1alpha1.Parameter, name string) *buildv1alpha1.Parameter { + for _, candidate := range parameters { + if candidate.Name == name { + return &candidate + } + } + + return nil +} + +// FindParamValueByName returns the first entry in a ParamValue array with a specified name, or nil +func FindParamValueByName(paramValues []buildv1alpha1.ParamValue, name string) *buildv1alpha1.ParamValue { + for _, candidate := range paramValues { + if candidate.Name == name { + return &candidate + } + } + + return nil +} + +// HandleTaskRunParam makes the necessary changes to a TaskRun for a parameter +func HandleTaskRunParam(taskRun *pipeline.TaskRun, parameterDefinition *buildv1alpha1.Parameter, paramValue buildv1alpha1.ParamValue) error { + taskRunParam := pipeline.Param{ + Name: paramValue.Name, + Value: pipeline.ArrayOrString{}, + } + + switch parameterDefinition.Type { + case "": // string is default + fallthrough + case buildv1alpha1.ParameterTypeString: + taskRunParam.Value.Type = pipeline.ParamTypeString + + switch { + case paramValue.SingleValue == nil && parameterDefinition.Default == nil: + // this error should never happen because we validate this upfront in ValidateBuildRunParameters + return fmt.Errorf("unexpected parameter without any value: %s", parameterDefinition.Name) + + case paramValue.SingleValue == nil: + // we tolerate this for optional parameters, we enter this code path if a user provides a paramValue without any value. The theoretic but + // not documented use case is that a Build specifies a value for an optional parameter (which has a default in the strategy). Then one can + // set a paramValue without a value in the BuildRun to get back to the default value. + return nil + + case paramValue.SingleValue.ConfigMapValue != nil: + envVarName, err := addConfigMapEnvVar(taskRun, paramValue.Name, paramValue.SingleValue.ConfigMapValue.Name, paramValue.SingleValue.ConfigMapValue.Key) + + if err != nil { + return err + } + + envVarExpression := fmt.Sprintf("$(%s)", envVarName) + if paramValue.SingleValue.ConfigMapValue.Format != nil { + taskRunParam.Value.StringVal = strings.ReplaceAll(*paramValue.SingleValue.ConfigMapValue.Format, "${CONFIGMAP_VALUE}", envVarExpression) + } else { + taskRunParam.Value.StringVal = envVarExpression + } + + case paramValue.SingleValue.SecretValue != nil: + envVarName, err := addSecretEnvVar(taskRun, paramValue.Name, paramValue.SingleValue.SecretValue.Name, paramValue.SingleValue.SecretValue.Key) + if err != nil { + return err + } + + envVarExpression := fmt.Sprintf("$(%s)", envVarName) + if paramValue.SingleValue.SecretValue.Format != nil { + taskRunParam.Value.StringVal = strings.ReplaceAll(*paramValue.SingleValue.SecretValue.Format, "${SECRET_VALUE}", envVarExpression) + } else { + taskRunParam.Value.StringVal = envVarExpression + } + + case paramValue.SingleValue.Value != nil: + taskRunParam.Value.StringVal = *paramValue.SingleValue.Value + + } + + case buildv1alpha1.ParameterTypeArray: + taskRunParam.Value.Type = pipeline.ParamTypeArray + + switch { + case paramValue.Values == nil && parameterDefinition.Defaults == nil: + // this error should never happen because we validate this upfront in ValidateBuildRunParameters + return fmt.Errorf("unexpected parameter without any value: %s", parameterDefinition.Name) + + case paramValue.Values == nil: + // we tolerate this for optional parameters, we enter this code path if a user provides a paramValue without any values. The theoretic but + // not documented use case is that a Build specifies values for an optional parameter (which has defaults in the strategy). Then one can + // set a paramValue without values in the BuildRun to get back to the default values. + return nil + + default: + for index, value := range paramValue.Values { + switch { + case value.ConfigMapValue != nil: + envVarName, err := addConfigMapEnvVar(taskRun, paramValue.Name, value.ConfigMapValue.Name, value.ConfigMapValue.Key) + if err != nil { + return err + } + + envVarExpression := fmt.Sprintf("$(%s)", envVarName) + if value.ConfigMapValue.Format != nil { + taskRunParam.Value.ArrayVal = append(taskRunParam.Value.ArrayVal, strings.ReplaceAll(*value.ConfigMapValue.Format, "${CONFIGMAP_VALUE}", envVarExpression)) + } else { + taskRunParam.Value.ArrayVal = append(taskRunParam.Value.ArrayVal, envVarExpression) + } + + case value.SecretValue != nil: + envVarName, err := addSecretEnvVar(taskRun, paramValue.Name, value.SecretValue.Name, value.SecretValue.Key) + if err != nil { + return err + } + + envVarExpression := fmt.Sprintf("$(%s)", envVarName) + if value.SecretValue.Format != nil { + taskRunParam.Value.ArrayVal = append(taskRunParam.Value.ArrayVal, strings.ReplaceAll(*value.SecretValue.Format, "${SECRET_VALUE}", envVarExpression)) + } else { + taskRunParam.Value.ArrayVal = append(taskRunParam.Value.ArrayVal, envVarExpression) + } + case value.Value != nil: + taskRunParam.Value.ArrayVal = append(taskRunParam.Value.ArrayVal, *value.Value) + + default: + // this error should never happen because we validate this upfront in ValidateBuildRunParameters + return fmt.Errorf("unexpected parameter without any value: %s[%d]", parameterDefinition.Name, index) + + } + } + } + } + + taskRun.Spec.Params = append(taskRun.Spec.Params, taskRunParam) + + return nil +} + +// generateEnvVarName adds a random suffix of five characters or digits to a given prefix +func generateEnvVarName(prefix string) (string, error) { + result := prefix + + // We add five random characters or digits out of the envVarNameSuffixChars pool which contains + // capital latin characters and digits as they are allowed in env vars. + // For this we run a loop which creates a random index from 0 to the number of possible characters, + // and then appends this character to the result. + for i := 0; i < 5; i++ { + num, err := rand.Int(rand.Reader, envVarNameSuffixCharsCount) + if err != nil { + return "", err + } + result += string(envVarNameSuffixChars[num.Int64()]) + } + + return result, nil +} + +// addConfigMapEnvVar modifies all steps which are referencing a parameter name in their command, args, or environment variable values, +// to contain a mapped environment variable for the ConfigMap key. It returns the name of the environment variable name. +func addConfigMapEnvVar(taskRun *pipeline.TaskRun, paramName string, configMapName string, configMapKey string) (string, error) { + envVarName := "" + + // In this first loop, we check whether any step already references the same ConfigMap key. This can + // happen when multiple paramValues reference the same ConfigMap key. We then reuse the environment + // variable name. This ensures that we do not have multiple environment variables that reference the + // same key. This is done for efficiency. +stepLookupLoop: + for _, step := range taskRun.Spec.TaskSpec.Steps { + for _, env := range step.Env { + if strings.HasPrefix(env.Name, "SHP_CONFIGMAP_PARAM_") && env.ValueFrom != nil && env.ValueFrom.ConfigMapKeyRef != nil && env.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name == configMapName && env.ValueFrom.ConfigMapKeyRef.Key == configMapKey { + envVarName = env.Name + break stepLookupLoop + } + } + } + + // generate an environment variable name if there is not yet one + if envVarName == "" { + generatedEnvVarName, err := generateEnvVarName("SHP_CONFIGMAP_PARAM_") + if err != nil { + return "", err + } + envVarName = generatedEnvVarName + } + + // In this second loop, we iterate all the steps and add the environment variable to all steps + // where the parameter is referenced. +stepModifyLoop: + for i, step := range taskRun.Spec.TaskSpec.Steps { + if isStepReferencingParameter(&step, paramName) { + // make sure we don't add a duplicate environment variable to a step + for _, env := range step.Env { + if env.Name == envVarName { + continue stepModifyLoop + } + } + + // we need to prepend our environment variables, this allows environment variables defined in the step of a build strategy to reference our environment variables + // From: https://kubernetes.io/docs/tasks/inject-data-application/define-environment-variable-container/: + // > Environment variables may reference each other, however ordering is important. Variables making use of others defined in the same context must come later in the list. + taskRun.Spec.TaskSpec.Steps[i].Env = append([]corev1.EnvVar{{ + Name: envVarName, + ValueFrom: &corev1.EnvVarSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: configMapName, + }, + Key: configMapKey, + }, + }, + }}, taskRun.Spec.TaskSpec.Steps[i].Env...) + } + } + + return envVarName, nil +} + +// addSecretEnvVar modifies all steps which are referencing a parameter name in their command, args, or environment variable values, +// to contain a mapped environment variable for the Secret key. It returns the name of the environment variable name. +func addSecretEnvVar(taskRun *pipeline.TaskRun, paramName string, secretName string, secretKey string) (string, error) { + envVarName := "" + + // In this first loop, we check whether any step already references the same Secret key. This can + // happen when multiple paramValues reference the same Secret key. We then reuse the environment + // variable name. This ensures that we do not have multiple environment variables that reference the + // same key. This is done for efficiency. +stepLookupLoop: + for _, step := range taskRun.Spec.TaskSpec.Steps { + for _, env := range step.Env { + if strings.HasPrefix(env.Name, "SHP_SECRET_PARAM_") && env.ValueFrom != nil && env.ValueFrom.SecretKeyRef != nil && env.ValueFrom.SecretKeyRef.LocalObjectReference.Name == secretName && env.ValueFrom.SecretKeyRef.Key == secretKey { + envVarName = env.Name + break stepLookupLoop + } + } + } + + // generate an environment variable name if there is not yet one + if envVarName == "" { + generatedEnvVarName, err := generateEnvVarName("SHP_SECRET_PARAM_") + if err != nil { + return "", err + } + envVarName = generatedEnvVarName + } + + // In this second loop, we iterate all the steps and add the environment variable to all steps + // where the parameter is referenced. +stepModifyLoop: + for i, step := range taskRun.Spec.TaskSpec.Steps { + if isStepReferencingParameter(&step, paramName) { + // make sure we don't add a duplicate environment variable to a step + for _, env := range step.Env { + if env.Name == envVarName { + continue stepModifyLoop + } + } + + // we need to prepend our environment variables, this allows environment variables defined in the step of a build strategy to reference our environment variables + // From: https://kubernetes.io/docs/tasks/inject-data-application/define-environment-variable-container/: + // > Environment variables may reference each other, however ordering is important. Variables making use of others defined in the same context must come later in the list. + taskRun.Spec.TaskSpec.Steps[i].Env = append([]corev1.EnvVar{{ + Name: envVarName, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secretName, + }, + Key: secretKey, + }, + }, + }}, taskRun.Spec.TaskSpec.Steps[i].Env...) + } + } + + return envVarName, nil +} + +// isStepReferencingParameter checks if a step is referencing a parameter in its command, args, or environment variable values +func isStepReferencingParameter(step *pipeline.Step, paramName string) bool { + searchStrings := []string{ + // the trailing ) is intentionally omitted because of arrays + // Tekton reference: https://github.com/tektoncd/pipeline/blob/main/docs/tasks.md#using-variable-substitution + fmt.Sprintf("$(params.%s", paramName), + fmt.Sprintf("$(params['%s']", paramName), + fmt.Sprintf("$(params[\"%s\"]", paramName), + } + + for _, command := range step.Command { + if isStringContainingAnySearchString(command, searchStrings) { + return true + } + } + for _, arg := range step.Args { + if isStringContainingAnySearchString(arg, searchStrings) { + return true + } + } + for _, env := range step.Env { + if isStringContainingAnySearchString(env.Value, searchStrings) { + return true + } + } + + return false +} + +func isStringContainingAnySearchString(aString string, searchStrings []string) bool { + for _, searchString := range searchStrings { + if strings.Contains(aString, searchString) { + return true + } + } + return false +} + +// hasMoreThanOneValue checks if a SingleValue has more than one value set (plain text, secret, and config map key reference) +func hasMoreThanOneValue(singleValue buildv1alpha1.SingleValue) bool { + if singleValue.Value != nil && (singleValue.ConfigMapValue != nil || singleValue.SecretValue != nil) { + return true + } + if singleValue.ConfigMapValue != nil && (singleValue.SecretValue != nil || singleValue.Value != nil) { + return true + } + if singleValue.SecretValue != nil && (singleValue.ConfigMapValue != nil || singleValue.Value != nil) { + return true + } + + return false +} + +// hasNoValue checks if a SingleValue has no value set (plain text, secret, and config map key reference) +func hasNoValue(singleValue buildv1alpha1.SingleValue) bool { + if singleValue.ConfigMapValue == nil && singleValue.SecretValue == nil && singleValue.Value == nil { + return true + } + + return false +} + +// hasIncompleteConfigMapValue checks if a SingleValue has a ConfigMap value with an empty name or key +func hasIncompleteConfigMapValue(singleValue buildv1alpha1.SingleValue) bool { + if singleValue.ConfigMapValue != nil && (singleValue.ConfigMapValue.Name == "" || singleValue.ConfigMapValue.Key == "") { + return true + } + + return false +} + +// hasIncompleteSecretValue checks if a SingleValue has a Secret value with an empty name or key +func hasIncompleteSecretValue(singleValue buildv1alpha1.SingleValue) bool { + if singleValue.SecretValue != nil && (singleValue.SecretValue.Name == "" || singleValue.SecretValue.Key == "") { + return true + } + + return false +} + +// ValidateBuildParameters validates that the parameter values specified in Build are suitable for what is defined in the BuildStrategy +func ValidateBuildParameters(parameterDefinitions []buildv1alpha1.Parameter, buildParamValues []buildv1alpha1.ParamValue) (bool, buildv1alpha1.BuildReason, string) { + valid, reason, message := validateParameters(parameterDefinitions, buildParamValues, true) + return valid, buildv1alpha1.BuildReason(reason), message +} + +// ValidateBuildRunParameters validates that the parameter values specified in Build and BuildRun +// are suitable for what is defined in the BuildStrategy +func ValidateBuildRunParameters(parameterDefinitions []buildv1alpha1.Parameter, buildParamValues []buildv1alpha1.ParamValue, buildRunParamValues []buildv1alpha1.ParamValue) (bool, string, string) { + paramValues := overrideParams(buildParamValues, buildRunParamValues) + return validateParameters(parameterDefinitions, paramValues, false) +} + +func validateParameters(parameterDefinitions []buildv1alpha1.Parameter, paramValues []buildv1alpha1.ParamValue, ignoreMissingParameters bool) (bool, string, string) { + // list of params that collide with reserved system strategy parameters + undesiredParams := []string{} + + // list of params that are not defined in the strategy + undefinedParams := []string{} + + // first loop is through the values to catch those that should not be there + for _, paramValue := range paramValues { + if isReserved := IsSystemReservedParameter(paramValue.Name); isReserved { + undesiredParams = append(undesiredParams, paramValue.Name) + } else { + parameterDefinition := FindParameterByName(parameterDefinitions, paramValue.Name) + if parameterDefinition == nil { + undefinedParams = append(undefinedParams, paramValue.Name) + } + } + } + + if len(undesiredParams) > 0 { + return false, ConditionRestrictedParametersInUse, fmt.Sprintf("The following parameters are restricted and cannot be set: %s", strings.Join(undesiredParams, ", ")) + } + + if len(undefinedParams) > 0 { + return false, ConditionUndefinedParameter, fmt.Sprintf("The following parameters are not defined in the build strategy: %s", strings.Join(undefinedParams, ", ")) + } + + // list of parameters where the value is of the wrong type + wrongValueTypeParameters := []string{} + + // list of parameters where there is no value + missingParameters := []string{} + + // list of parameters where at least one array item is empty + arrayItemEmptyParameters := []string{} + + // list of params that have multiple values set + multiValueParams := []string{} + + // list of params that have incomplete ConfigMap values + incompleteConfigMapValueParameters := []string{} + + // list of params that have incomplete Secret values + incompleteSecretValueParameters := []string{} + + // second loop is through the strategy parameters to determine those with missing or incorrect values + for _, parameterDefinition := range parameterDefinitions { + paramValue := FindParamValueByName(paramValues, parameterDefinition.Name) + + switch parameterDefinition.Type { + case "": // string is default + fallthrough + case buildv1alpha1.ParameterTypeString: + if paramValue != nil { + // check if a string value contains array values + if paramValue.Values != nil { + wrongValueTypeParameters = append(wrongValueTypeParameters, parameterDefinition.Name) + } + + if paramValue.SingleValue != nil { + // check if a single value contains multiple values + if hasMoreThanOneValue(*paramValue.SingleValue) { + multiValueParams = append(multiValueParams, parameterDefinition.Name) + } + + // check if a configmap value is incomplete + if hasIncompleteConfigMapValue(*paramValue.SingleValue) { + incompleteConfigMapValueParameters = append(incompleteConfigMapValueParameters, parameterDefinition.Name) + } + + // check if a secret value is incomplete + if hasIncompleteSecretValue(*paramValue.SingleValue) { + incompleteSecretValueParameters = append(incompleteSecretValueParameters, parameterDefinition.Name) + } + } + } + + // check if a string parameter without default has no value + if parameterDefinition.Default == nil && (paramValue == nil || paramValue.SingleValue == nil || hasNoValue(*paramValue.SingleValue)) { + missingParameters = append(missingParameters, parameterDefinition.Name) + continue + } + + case buildv1alpha1.ParameterTypeArray: + if paramValue != nil { + // check if an array value contains a single value + if paramValue.SingleValue != nil { + wrongValueTypeParameters = append(wrongValueTypeParameters, parameterDefinition.Name) + } + + // check whether any array item contains no value + for _, arrayItemParamValue := range paramValue.Values { + if hasNoValue(arrayItemParamValue) { + arrayItemEmptyParameters = append(arrayItemEmptyParameters, parameterDefinition.Name) + break + } + } + + // check whether any array item has more than one value + for _, arrayItemParamValue := range paramValue.Values { + if hasMoreThanOneValue(arrayItemParamValue) { + multiValueParams = append(multiValueParams, parameterDefinition.Name) + break + } + } + + // check whether any array item has an incomplete configMapValue + for _, arrayItemParamValue := range paramValue.Values { + if hasIncompleteConfigMapValue(arrayItemParamValue) { + incompleteConfigMapValueParameters = append(incompleteConfigMapValueParameters, parameterDefinition.Name) + } + } + + // check whether any array item has an incomplete secretValue + for _, arrayItemParamValue := range paramValue.Values { + if hasIncompleteSecretValue(arrayItemParamValue) { + incompleteSecretValueParameters = append(incompleteSecretValueParameters, parameterDefinition.Name) + } + } + } + + // check if an array parameter without defaults has no values + if parameterDefinition.Defaults == nil && (paramValue == nil || paramValue.Values == nil) { + missingParameters = append(missingParameters, parameterDefinition.Name) + } + } + } + + if len(wrongValueTypeParameters) > 0 { + return false, ConditionWrongParameterValueType, fmt.Sprintf("The values for the following parameters are using the wrong type: %s", strings.Join(wrongValueTypeParameters, ", ")) + } + + if !ignoreMissingParameters && len(missingParameters) > 0 { + return false, ConditionMissingParameterValues, fmt.Sprintf("The following parameters are required but no value has been provided: %s", strings.Join(missingParameters, ", ")) + } + + if len(multiValueParams) > 0 { + return false, ConditionInconsistentParameterValues, fmt.Sprintf("The following parameters have more than one of 'configMapValue', 'secretValue', and 'value' set: %s", strings.Join(multiValueParams, ", ")) + } + + if len(arrayItemEmptyParameters) > 0 { + return false, ConditionEmptyArrayItemParameterValues, fmt.Sprintf("The values for the following array parameters are containing at least one item where none of 'configMapValue', 'secretValue', and 'value' are set: %s", strings.Join(arrayItemEmptyParameters, ", ")) + } + + if len(incompleteConfigMapValueParameters) > 0 { + return false, ConditionIncompleteConfigMapValueParameterValues, fmt.Sprintf("The values for the following parameters are containing a 'configMapValue' with an empty 'name' or 'key': %s", strings.Join(incompleteConfigMapValueParameters, ", ")) + } + + if len(incompleteSecretValueParameters) > 0 { + return false, ConditionIncompleteSecretValueParameterValues, fmt.Sprintf("The values for the following parameters are containing a 'secretValue' with an empty 'name' or 'key': %s", strings.Join(incompleteSecretValueParameters, ", ")) + } + + return true, "", "" +} diff --git a/pkg/reconciler/buildrun/resources/taskrun.go b/pkg/reconciler/buildrun/resources/taskrun.go index 6ecf015098..8400d573da 100644 --- a/pkg/reconciler/buildrun/resources/taskrun.go +++ b/pkg/reconciler/buildrun/resources/taskrun.go @@ -10,8 +10,7 @@ import ( "strconv" "strings" - taskv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + v1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -66,7 +65,7 @@ func GenerateTaskSpec( build *buildv1alpha1.Build, buildRun *buildv1alpha1.BuildRun, buildSteps []buildv1alpha1.BuildStep, - strategyParams []buildv1alpha1.Parameter, + parameterDefinitions []buildv1alpha1.Parameter, ) (*v1beta1.TaskSpec, error) { generatedTaskSpec := v1beta1.TaskSpec{ @@ -92,17 +91,17 @@ func GenerateTaskSpec( { Name: fmt.Sprintf("%s-%s", prefixParamsResultsVolumes, paramOutputImage), Description: "The URL of the image that the build produces", - Type: taskv1.ParamTypeString, + Type: v1beta1.ParamTypeString, }, { Name: fmt.Sprintf("%s-%s", prefixParamsResultsVolumes, paramSourceContext), Description: "The context directory inside the source directory", - Type: taskv1.ParamTypeString, + Type: v1beta1.ParamTypeString, }, { Name: fmt.Sprintf("%s-%s", prefixParamsResultsVolumes, paramSourceRoot), Description: "The source directory", - Type: taskv1.ParamTypeString, + Type: v1beta1.ParamTypeString, }, }, Workspaces: []v1beta1.WorkspaceDeclaration{ @@ -131,19 +130,32 @@ func GenerateTaskSpec( AmendTaskSpecWithSources(cfg, &generatedTaskSpec, build, buildRun) // Add the strategy defined parameters into the Task spec - for _, p := range strategyParams { + for _, parameterDefinition := range parameterDefinitions { param := v1beta1.ParamSpec{ - Name: p.Name, - Description: p.Description, + Name: parameterDefinition.Name, + Description: parameterDefinition.Description, } - // verify if the paramSpec Default requires a default - // value or not - if p.Default != nil { - param.Default = &v1beta1.ArrayOrString{ - Type: v1beta1.ParamTypeString, - StringVal: *p.Default, + switch parameterDefinition.Type { + case "": // string is default + fallthrough + case buildv1alpha1.ParameterTypeString: + param.Type = v1beta1.ParamTypeString + if parameterDefinition.Default != nil { + param.Default = &v1beta1.ArrayOrString{ + Type: v1beta1.ParamTypeString, + StringVal: *parameterDefinition.Default, + } + } + + case buildv1alpha1.ParameterTypeArray: + param.Type = v1beta1.ParamTypeArray + if parameterDefinition.Defaults != nil { + param.Default = &v1beta1.ArrayOrString{ + Type: v1beta1.ParamTypeArray, + ArrayVal: *parameterDefinition.Defaults, + } } } @@ -360,52 +372,19 @@ func GenerateTaskRun( // Ensure a proper override of params between Build and BuildRun // A BuildRun can override a param as long as it was defined in the Build - buildUserParams := overrideParams(build.Spec.ParamValues, buildRun.Spec.ParamValues) - - // list of params that collide with reserved system strategy parameters - undesiredParams := []string{} + paramValues := overrideParams(build.Spec.ParamValues, buildRun.Spec.ParamValues) // Append params to the TaskRun spec definition - for _, p := range buildUserParams { - - if isReserved := IsSystemReservedParameter(p.Name); isReserved { - undesiredParams = append(undesiredParams, p.Name) + for _, paramValue := range paramValues { + parameterDefinition := FindParameterByName(strategy.GetParameters(), paramValue.Name) + if parameterDefinition == nil { + // this error should never happen because we validate this upfront in ValidateBuildRunParameters + return nil, fmt.Errorf("the parameter %q is not defined in the build strategy %q", paramValue.Name, strategy.GetName()) } - buildParam := v1beta1.Param{ - Name: p.Name, - Value: v1beta1.ArrayOrString{ - Type: v1beta1.ParamTypeString, - StringVal: p.Value, - }, + if err := HandleTaskRunParam(expectedTaskRun, parameterDefinition, paramValue); err != nil { + return nil, err } - expectedTaskRun.Spec.Params = append(expectedTaskRun.Spec.Params, buildParam) - } - // if system parameters names are being use, fail the taskRun creation and update the condition message - // with a custom error - if len(undesiredParams) > 0 { - return nil, fmt.Errorf("restricted parameters in use: %s", strings.Join(undesiredParams, ",")) - } - - // check if there are parameters from strategies where a value was never set, if this is the case, - // then throw a custom error message. - paramsWithoutValues := []string{} - -StrategyParametersLoop: - for _, strategyParam := range strategy.GetParameters() { - if strategyParam.Default == nil { - for _, p := range buildUserParams { - if strategyParam.Name == p.Name { - // go back to the outer loop - continue StrategyParametersLoop - } - } - paramsWithoutValues = append(paramsWithoutValues, strategyParam.Name) - } - } - - if len(paramsWithoutValues) > 0 { - return nil, fmt.Errorf("parameters without a value definition: %s", strings.Join(paramsWithoutValues, ",")) } return expectedTaskRun, nil diff --git a/pkg/validate/strategies.go b/pkg/validate/strategies.go index e5cf2171a4..e5ab33ab02 100644 --- a/pkg/validate/strategies.go +++ b/pkg/validate/strategies.go @@ -7,7 +7,6 @@ package validate import ( "context" "fmt" - "strings" build "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" "github.com/shipwright-io/build/pkg/ctxlog" @@ -29,110 +28,68 @@ type Strategy struct { // namespaced or cluster scoped strategies func (s Strategy) ValidatePath(ctx context.Context) error { if s.Build.Spec.Strategy != nil { - buildStrategy := &build.BuildStrategy{} + var builderStrategy build.BuilderStrategy + strategyExists := false + var err error + if s.Build.Spec.Strategy.Kind != nil { switch *s.Build.Spec.Strategy.Kind { case build.NamespacedBuildStrategyKind: - if err := s.validateBuildStrategy(ctx, s.Build.Spec.Strategy.Name, buildStrategy); err != nil { - return err - } - if err := s.validateBuildParams(buildStrategy.Spec.Parameters); err != nil { - return err - } + buildStrategy := &build.BuildStrategy{} + strategyExists, err = s.validateBuildStrategy(ctx, s.Build.Spec.Strategy.Name, buildStrategy) + builderStrategy = buildStrategy case build.ClusterBuildStrategyKind: clusterBuildStrategy := &build.ClusterBuildStrategy{} - if err := s.validateClusterBuildStrategy(ctx, s.Build.Spec.Strategy.Name, clusterBuildStrategy); err != nil { - return err - } - if err := s.validateBuildParams(clusterBuildStrategy.Spec.Parameters); err != nil { - return err - } + strategyExists, err = s.validateClusterBuildStrategy(ctx, s.Build.Spec.Strategy.Name, clusterBuildStrategy) + builderStrategy = clusterBuildStrategy default: return fmt.Errorf("unknown strategy kind: %v", *s.Build.Spec.Strategy.Kind) } } else { ctxlog.Info(ctx, "buildStrategy kind is nil, use default NamespacedBuildStrategyKind", namespace, s.Build.Namespace, name, s.Build.Name) - if err := s.validateBuildStrategy(ctx, s.Build.Spec.Strategy.Name, buildStrategy); err != nil { - return err - } - if err := s.validateBuildParams(buildStrategy.Spec.Parameters); err != nil { - return err - } + buildStrategy := &build.BuildStrategy{} + strategyExists, err = s.validateBuildStrategy(ctx, s.Build.Spec.Strategy.Name, buildStrategy) + builderStrategy = buildStrategy + } + + if err != nil { + return err + } + + if strategyExists { + s.validateBuildParams(builderStrategy.GetParameters()) } } return nil } -func (s Strategy) validateBuildStrategy(ctx context.Context, strategyName string, buildStrategy *build.BuildStrategy) error { +func (s Strategy) validateBuildStrategy(ctx context.Context, strategyName string, buildStrategy *build.BuildStrategy) (bool, error) { if err := s.Client.Get(ctx, types.NamespacedName{Name: strategyName, Namespace: s.Build.Namespace}, buildStrategy); err != nil && !apierrors.IsNotFound(err) { - return err + return false, err } else if apierrors.IsNotFound(err) { s.Build.Status.Reason = build.BuildStrategyNotFound s.Build.Status.Message = fmt.Sprintf("buildStrategy %s does not exist in namespace %s", s.Build.Spec.Strategy.Name, s.Build.Namespace) + return false, nil } - - return nil + return true, nil } -func (s Strategy) validateClusterBuildStrategy(ctx context.Context, strategyName string, clusterBuildStrategy *build.ClusterBuildStrategy) error { +func (s Strategy) validateClusterBuildStrategy(ctx context.Context, strategyName string, clusterBuildStrategy *build.ClusterBuildStrategy) (bool, error) { if err := s.Client.Get(ctx, types.NamespacedName{Name: strategyName}, clusterBuildStrategy); err != nil && !apierrors.IsNotFound(err) { - return err + return false, err } else if apierrors.IsNotFound(err) { s.Build.Status.Reason = build.ClusterBuildStrategyNotFound s.Build.Status.Message = fmt.Sprintf("clusterBuildStrategy %s does not exist", s.Build.Spec.Strategy.Name) + return false, nil } - return nil + return true, nil } -func (s Strategy) validateBuildParams(parameters []build.Parameter) error { - - if len(s.Build.Spec.ParamValues) == 0 { - return nil - } - - // Check that the Build param is defined in the strategies parameters - s.validateParamsInStrategies(s.Build.Spec.ParamValues, parameters) - - // Check that the Build param is not a restricted shipwright one - s.validateParamsNamesDefinition() - - return nil -} - -func (s Strategy) validateParamsNamesDefinition() { - - undesiredParams := []string{} - - for _, p := range s.Build.Spec.ParamValues { - if isReserved := resources.IsSystemReservedParameter(p.Name); isReserved { - undesiredParams = append(undesiredParams, p.Name) - } - } - - if len(undesiredParams) > 0 { - s.Build.Status.Reason = build.RestrictedParametersInUse - s.Build.Status.Message = fmt.Sprintf("restricted parameters in use: %s", strings.Join(undesiredParams, ",")) - } -} - -func (s Strategy) validateParamsInStrategies(params []build.ParamValue, parameters []build.Parameter) { - - undefinedParams := []string{} - definedParameter := false - for _, bp := range params { - for _, sp := range parameters { - if bp.Name == sp.Name { - definedParameter = true - } - } - if !definedParameter { - undefinedParams = append(undefinedParams, bp.Name) - } - definedParameter = false - } +func (s Strategy) validateBuildParams(parameterDefinitions []build.Parameter) { + valid, reason, message := resources.ValidateBuildParameters(parameterDefinitions, s.Build.Spec.ParamValues) - if len(undefinedParams) > 0 { - s.Build.Status.Reason = build.UndefinedParameter - s.Build.Status.Message = fmt.Sprintf("parameter not defined in the strategies: %s", strings.Join(undefinedParams, ",")) + if !valid { + s.Build.Status.Reason = reason + s.Build.Status.Message = message } } From 0c71add8d969d051bddec3be86be4cf351fc903c Mon Sep 17 00:00:00 2001 From: Sascha Schwarze Date: Wed, 19 Jan 2022 16:58:36 +0100 Subject: [PATCH 3/5] Tests --- .../buildrun/resources/params_test.go | 1064 ++++++++++++++++- test/build_samples.go | 55 +- test/buildrun_samples.go | 4 +- test/buildstrategy_samples.go | 86 +- test/catalog.go | 22 + test/e2e/common_suite_test.go | 211 ++++ test/e2e/e2e_params_test.go | 139 +++ .../buildstrategy_to_taskruns_test.go | 129 +- test/utils/configmaps.go | 33 + 9 files changed, 1676 insertions(+), 67 deletions(-) create mode 100644 test/e2e/e2e_params_test.go create mode 100644 test/utils/configmaps.go diff --git a/pkg/reconciler/buildrun/resources/params_test.go b/pkg/reconciler/buildrun/resources/params_test.go index 87f1e7ed84..54029c6364 100644 --- a/pkg/reconciler/buildrun/resources/params_test.go +++ b/pkg/reconciler/buildrun/resources/params_test.go @@ -5,96 +5,1080 @@ package resources import ( + "fmt" + . "github.com/onsi/ginkgo" . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" "github.com/onsi/gomega/types" buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" + pipeline "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + corev1 "k8s.io/api/core/v1" + "k8s.io/utils/pointer" ) var _ = Describe("Params overrides", func() { DescribeTable("original params can be overridden", - func(b []buildv1alpha1.ParamValue, br []buildv1alpha1.ParamValue, expected types.GomegaMatcher) { - Expect(overrideParams(b, br)).To(expected) + func(buildParams []buildv1alpha1.ParamValue, buildRunParams []buildv1alpha1.ParamValue, expected types.GomegaMatcher) { + Expect(overrideParams(buildParams, buildRunParams)).To(expected) }, Entry("override a single parameter", []buildv1alpha1.ParamValue{ - {Name: "a", Value: "2"}, + {Name: "a", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("2"), + }}, }, []buildv1alpha1.ParamValue{ - {Name: "a", Value: "3"}, + {Name: "a", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("3"), + }}, }, ContainElements([]buildv1alpha1.ParamValue{ - {Name: "a", Value: "3"}, + {Name: "a", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("3"), + }}, })), Entry("override two parameters", []buildv1alpha1.ParamValue{ - {Name: "a", Value: "2"}, - {Name: "b", Value: "2"}, + {Name: "a", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("2"), + }}, + {Name: "b", SingleValue: &buildv1alpha1.SingleValue{ + SecretValue: &buildv1alpha1.ObjectKeyRef{ + Name: "a-secret", + Key: "a-key", + }, + }}, }, []buildv1alpha1.ParamValue{ - {Name: "a", Value: "3"}, - {Name: "b", Value: "3"}, + {Name: "a", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("3"), + }}, + {Name: "b", SingleValue: &buildv1alpha1.SingleValue{ + ConfigMapValue: &buildv1alpha1.ObjectKeyRef{ + Name: "a-config-map", + Key: "a-cm-key", + }, + }}, }, ContainElements([]buildv1alpha1.ParamValue{ - {Name: "a", Value: "3"}, - {Name: "b", Value: "3"}, + {Name: "a", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("3"), + }}, + {Name: "b", SingleValue: &buildv1alpha1.SingleValue{ + ConfigMapValue: &buildv1alpha1.ObjectKeyRef{ + Name: "a-config-map", + Key: "a-cm-key", + }, + }}, })), Entry("override multiple parameters", []buildv1alpha1.ParamValue{ - {Name: "a", Value: "2"}, - {Name: "b", Value: "2"}, - {Name: "c", Value: "2"}, + {Name: "a", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("2"), + }}, + {Name: "b", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("2"), + }}, + {Name: "c", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("2"), + }}, }, []buildv1alpha1.ParamValue{ - {Name: "a", Value: "6"}, - {Name: "c", Value: "6"}, + {Name: "a", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("6"), + }}, + {Name: "c", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("6"), + }}, }, ContainElements([]buildv1alpha1.ParamValue{ - {Name: "a", Value: "6"}, - {Name: "b", Value: "2"}, - {Name: "c", Value: "6"}, + {Name: "a", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("6"), + }}, + {Name: "b", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("2"), + }}, + {Name: "c", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("6"), + }}, })), Entry("dont override when second list is empty", []buildv1alpha1.ParamValue{ - {Name: "t", Value: "2"}, - {Name: "z", Value: "2"}, - {Name: "g", Value: "2"}, + {Name: "t", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("2"), + }}, + {Name: "z", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("2"), + }}, + {Name: "g", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("2"), + }}, }, []buildv1alpha1.ParamValue{ // no overrides }, ContainElements([]buildv1alpha1.ParamValue{ - {Name: "t", Value: "2"}, - {Name: "z", Value: "2"}, - {Name: "g", Value: "2"}, + {Name: "t", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("2"), + }}, + {Name: "z", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("2"), + }}, + {Name: "g", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("2"), + }}, })), Entry("override when first list is empty but not the second list", []buildv1alpha1.ParamValue{ // no original values }, []buildv1alpha1.ParamValue{ - {Name: "a", Value: "6"}, - {Name: "c", Value: "6"}, + {Name: "a", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("6"), + }}, + {Name: "c", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("6"), + }}, }, ContainElements([]buildv1alpha1.ParamValue{ - {Name: "a", Value: "6"}, - {Name: "c", Value: "6"}, + {Name: "a", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("6"), + }}, + {Name: "c", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("6"), + }}, })), Entry("override multiple parameters if the match and add them if not present in first list", []buildv1alpha1.ParamValue{ - {Name: "a", Value: "2"}, + {Name: "a", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("2"), + }}, }, []buildv1alpha1.ParamValue{ - {Name: "a", Value: "22"}, - {Name: "b", Value: "20"}, - {Name: "c", Value: "10"}, - {Name: "d", Value: "8"}, - {Name: "e", Value: "4"}, + {Name: "a", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("22"), + }}, + {Name: "b", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("20"), + }}, + {Name: "c", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("10"), + }}, + {Name: "d", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("8"), + }}, + {Name: "e", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("4"), + }}, }, ContainElements([]buildv1alpha1.ParamValue{ - {Name: "a", Value: "22"}, - {Name: "b", Value: "20"}, - {Name: "c", Value: "10"}, - {Name: "d", Value: "8"}, - {Name: "e", Value: "4"}, + {Name: "a", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("22"), + }}, + {Name: "b", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("20"), + }}, + {Name: "c", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("10"), + }}, + {Name: "d", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("8"), + }}, + {Name: "e", SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("4"), + }}, })), ) }) + +var _ = Describe("IsSystemReservedParameter", func() { + + Context("for a shp-prefixed parameter", func() { + + It("returns true", func() { + Expect(IsSystemReservedParameter("shp-source-root")).To(BeTrue()) + }) + }) + + Context("for a non shp-prefixed paramerer", func() { + + It("returns false", func() { + Expect(IsSystemReservedParameter("custom-param")).To(BeFalse()) + }) + }) +}) + +var _ = Describe("FindParameterByName", func() { + + Context("For a list of three parameters", func() { + + parameters := []buildv1alpha1.Parameter{{ + Name: "some-parameter", + Type: "string", + }, { + Name: "another-parameter", + Type: "array", + }, { + Name: "last-parameter", + Type: "string", + }} + + It("returns nil if no parameter with a matching name exists", func() { + Expect(FindParameterByName(parameters, "non-existing-parameter")).To(BeNil()) + }) + + It("returns the correct parameter with a matching name", func() { + parameter := FindParameterByName(parameters, "another-parameter") + Expect(parameter).ToNot(BeNil()) + Expect(parameter).To(BeEquivalentTo(&buildv1alpha1.Parameter{ + Name: "another-parameter", + Type: "array", + })) + }) + }) +}) + +var _ = Describe("FindParamValueByName", func() { + + Context("For a list of three parameter values", func() { + + paramValues := []buildv1alpha1.ParamValue{{ + Name: "some-parameter", + SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("some-value"), + }, + }, { + Name: "another-parameter", + Values: []buildv1alpha1.SingleValue{ + { + Value: pointer.String("item"), + }, + { + ConfigMapValue: &buildv1alpha1.ObjectKeyRef{ + Name: "a-configmap", + Key: "a-key", + }, + }, + }, + }, { + Name: "last-parameter", + SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("last-value"), + }, + }} + + It("returns nil if no parameter with a matching name exists", func() { + Expect(FindParamValueByName(paramValues, "non-existing-parameter")).To(BeNil()) + }) + + It("returns the correct parameter with a matching name", func() { + parameter := FindParamValueByName(paramValues, "another-parameter") + Expect(parameter).ToNot(BeNil()) + Expect(parameter).To(BeEquivalentTo(&buildv1alpha1.ParamValue{ + Name: "another-parameter", + Values: []buildv1alpha1.SingleValue{ + { + Value: pointer.String("item"), + }, + { + ConfigMapValue: &buildv1alpha1.ObjectKeyRef{ + Name: "a-configmap", + Key: "a-key", + }, + }, + }, + })) + }) + }) +}) + +var _ = Describe("generateEnvVarName", func() { + + Context("For a provided prefix", func() { + + It("returns a variable name with a random suffix", func() { + name, err := generateEnvVarName(("MY_PREFIX_")) + Expect(err).ToNot(HaveOccurred()) + Expect(name).To(HavePrefix("MY_PREFIX_")) + Expect(len(name)).To(Equal(15)) + }) + }) +}) + +var _ = Describe("isStepReferencingParameter", func() { + + Context("for a Step referencing parameters in different ways", func() { + + step := &pipeline.Step{ + Container: corev1.Container{ + Command: []string{ + "some-command", + "$(params.first-param)", + }, + Args: []string{ + "--flag=$(params['dot.param'])", + "$(params.array-param[*])", + }, + Env: []corev1.EnvVar{{ + Name: "MY_ENV_VAR", + Value: "hohe $(params[\"another.dot.param\"])", + }}, + }, + } + + It("returns true for a classical referenced parameter in the command", func() { + Expect(isStepReferencingParameter(step, "first-param")).To(BeTrue()) + }) + + It("returns true for a parameter referenced using brackets in an argument", func() { + Expect(isStepReferencingParameter(step, "dot.param")).To(BeTrue()) + }) + + It("returns true for a parameter referenced using brackets in an environment variable", func() { + Expect(isStepReferencingParameter(step, "another.dot.param")).To(BeTrue()) + }) + + It("returns true for an array referenced parameter in an argument", func() { + Expect(isStepReferencingParameter(step, "array-param")).To(BeTrue()) + }) + + It("returns false for a non-referenced parameter", func() { + Expect(isStepReferencingParameter(step, "second-param")).To(BeFalse()) + }) + }) +}) + +var _ = Describe("HandleTaskRunParam", func() { + + var taskRun *pipeline.TaskRun + + BeforeEach(func() { + taskRun = &pipeline.TaskRun{ + Spec: pipeline.TaskRunSpec{ + TaskSpec: &pipeline.TaskSpec{ + Steps: []pipeline.Step{ + { + Container: corev1.Container{ + Name: "first-container", + Args: []string{ + "--an-argument=$(params.string-parameter)", + }, + }, + }, + { + Container: corev1.Container{ + Name: "second-container", + Args: []string{ + "$(params.array-parameter[*])", + }, + }, + }, + }, + }, + }, + } + }) + + Context("for a string parameter", func() { + + parameterDefinition := &buildv1alpha1.Parameter{ + Name: "string-parameter", + Type: buildv1alpha1.ParameterTypeString, + } + + It("adds a simple value", func() { + err := HandleTaskRunParam(taskRun, parameterDefinition, buildv1alpha1.ParamValue{ + Name: "string-parameter", + SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("My value"), + }, + }) + Expect(err).ToNot(HaveOccurred()) + + Expect(taskRun.Spec.Params).To(BeEquivalentTo([]pipeline.Param{ + { + Name: "string-parameter", + Value: pipeline.ArrayOrString{ + Type: pipeline.ParamTypeString, + StringVal: "My value", + }, + }, + })) + }) + + It("adds a configmap value without a format", func() { + err := HandleTaskRunParam(taskRun, parameterDefinition, buildv1alpha1.ParamValue{ + Name: "string-parameter", + SingleValue: &buildv1alpha1.SingleValue{ + ConfigMapValue: &buildv1alpha1.ObjectKeyRef{ + Name: "config-map-name", + Key: "my-key", + }, + }, + }) + Expect(err).ToNot(HaveOccurred()) + + // Verify the environment variable that is only added to the first step + Expect(len(taskRun.Spec.TaskSpec.Steps[0].Env)).To(Equal(1)) + envVarName := taskRun.Spec.TaskSpec.Steps[0].Env[0].Name + + Expect(envVarName).To(HavePrefix("SHP_CONFIGMAP_PARAM_")) + Expect(taskRun.Spec.TaskSpec.Steps[0].Env[0]).To(BeEquivalentTo(corev1.EnvVar{ + Name: envVarName, + ValueFrom: &corev1.EnvVarSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "config-map-name", + }, + Key: "my-key", + }, + }, + })) + + Expect(len(taskRun.Spec.TaskSpec.Steps[1].Env)).To(Equal(0)) + + // Verify the parameters + Expect(taskRun.Spec.Params).To(BeEquivalentTo([]pipeline.Param{ + { + Name: "string-parameter", + Value: pipeline.ArrayOrString{ + Type: pipeline.ParamTypeString, + StringVal: fmt.Sprintf("$(%s)", envVarName), + }, + }, + })) + }) + + It("adds a configmap value with a format", func() { + err := HandleTaskRunParam(taskRun, parameterDefinition, buildv1alpha1.ParamValue{ + Name: "string-parameter", + SingleValue: &buildv1alpha1.SingleValue{ + ConfigMapValue: &buildv1alpha1.ObjectKeyRef{ + Name: "config-map-name", + Key: "my-key", + Format: pointer.String("The value from the config map is '${CONFIGMAP_VALUE}'."), + }, + }, + }) + Expect(err).ToNot(HaveOccurred()) + + // Verify the environment variable that is only added to the first step + Expect(len(taskRun.Spec.TaskSpec.Steps[0].Env)).To(Equal(1)) + envVarName := taskRun.Spec.TaskSpec.Steps[0].Env[0].Name + + Expect(envVarName).To(HavePrefix("SHP_CONFIGMAP_PARAM_")) + Expect(taskRun.Spec.TaskSpec.Steps[0].Env[0]).To(BeEquivalentTo(corev1.EnvVar{ + Name: envVarName, + ValueFrom: &corev1.EnvVarSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "config-map-name", + }, + Key: "my-key", + }, + }, + })) + + Expect(len(taskRun.Spec.TaskSpec.Steps[1].Env)).To(Equal(0)) + + // Verify the parameters + Expect(taskRun.Spec.Params).To(BeEquivalentTo([]pipeline.Param{ + { + Name: "string-parameter", + Value: pipeline.ArrayOrString{ + Type: pipeline.ParamTypeString, + StringVal: fmt.Sprintf("The value from the config map is '$(%s)'.", envVarName), + }, + }, + })) + }) + + It("adds a secret value without a format", func() { + err := HandleTaskRunParam(taskRun, parameterDefinition, buildv1alpha1.ParamValue{ + Name: "string-parameter", + SingleValue: &buildv1alpha1.SingleValue{ + SecretValue: &buildv1alpha1.ObjectKeyRef{ + Name: "secret-name", + Key: "secret-key", + }, + }, + }) + Expect(err).ToNot(HaveOccurred()) + + // Verify the environment variable that is only added to the first step + Expect(len(taskRun.Spec.TaskSpec.Steps[0].Env)).To(Equal(1)) + envVarName := taskRun.Spec.TaskSpec.Steps[0].Env[0].Name + + Expect(envVarName).To(HavePrefix("SHP_SECRET_PARAM_")) + Expect(taskRun.Spec.TaskSpec.Steps[0].Env[0]).To(BeEquivalentTo(corev1.EnvVar{ + Name: envVarName, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "secret-name", + }, + Key: "secret-key", + }, + }, + })) + + Expect(len(taskRun.Spec.TaskSpec.Steps[1].Env)).To(Equal(0)) + + // Verify the parameters + Expect(taskRun.Spec.Params).To(BeEquivalentTo([]pipeline.Param{ + { + Name: "string-parameter", + Value: pipeline.ArrayOrString{ + Type: pipeline.ParamTypeString, + StringVal: fmt.Sprintf("$(%s)", envVarName), + }, + }, + })) + }) + + It("adds a secret value with a format", func() { + err := HandleTaskRunParam(taskRun, parameterDefinition, buildv1alpha1.ParamValue{ + Name: "string-parameter", + SingleValue: &buildv1alpha1.SingleValue{ + SecretValue: &buildv1alpha1.ObjectKeyRef{ + Name: "secret-name", + Key: "secret-key", + Format: pointer.String("secret-value: ${SECRET_VALUE}"), + }, + }, + }) + Expect(err).ToNot(HaveOccurred()) + + // Verify the environment variable that is only added to the first step + Expect(len(taskRun.Spec.TaskSpec.Steps[0].Env)).To(Equal(1)) + envVarName := taskRun.Spec.TaskSpec.Steps[0].Env[0].Name + + Expect(envVarName).To(HavePrefix("SHP_SECRET_PARAM_")) + Expect(taskRun.Spec.TaskSpec.Steps[0].Env[0]).To(BeEquivalentTo(corev1.EnvVar{ + Name: envVarName, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "secret-name", + }, + Key: "secret-key", + }, + }, + })) + + Expect(len(taskRun.Spec.TaskSpec.Steps[1].Env)).To(Equal(0)) + + // Verify the parameters + Expect(taskRun.Spec.Params).To(BeEquivalentTo([]pipeline.Param{ + { + Name: "string-parameter", + Value: pipeline.ArrayOrString{ + Type: pipeline.ParamTypeString, + StringVal: fmt.Sprintf("secret-value: $(%s)", envVarName), + }, + }, + })) + }) + }) + + Context("for an array parameter", func() { + + parameterDefinition := &buildv1alpha1.Parameter{ + Name: "array-parameter", + Type: buildv1alpha1.ParameterTypeArray, + } + + It("adds simple values correctly", func() { + err := HandleTaskRunParam(taskRun, parameterDefinition, buildv1alpha1.ParamValue{ + Name: "array-parameter", + Values: []buildv1alpha1.SingleValue{ + { + Value: pointer.String("first entry"), + }, + { + Value: pointer.String(""), + }, + { + Value: pointer.String("third entry"), + }, + }, + }) + Expect(err).ToNot(HaveOccurred()) + + Expect(len(taskRun.Spec.TaskSpec.Steps[1].Env)).To(Equal(0)) + + // Verify the parameters + Expect(taskRun.Spec.Params).To(BeEquivalentTo([]pipeline.Param{ + { + Name: "array-parameter", + Value: pipeline.ArrayOrString{ + Type: pipeline.ParamTypeArray, + ArrayVal: []string{ + "first entry", + "", + "third entry", + }, + }, + }, + })) + }) + + It("adds values from different sources correctly", func() { + err := HandleTaskRunParam(taskRun, parameterDefinition, buildv1alpha1.ParamValue{ + Name: "array-parameter", + Values: []buildv1alpha1.SingleValue{ + { + Value: pointer.String("first entry"), + }, + { + SecretValue: &buildv1alpha1.ObjectKeyRef{ + Name: "secret-name", + Key: "secret-key", + }, + }, + { + SecretValue: &buildv1alpha1.ObjectKeyRef{ + Name: "secret-name", + Key: "secret-key", + Format: pointer.String("The secret value is ${SECRET_VALUE}"), + }, + }, + }, + }) + Expect(err).ToNot(HaveOccurred()) + + // Verify the environment variable that is only added to the second step + Expect(len(taskRun.Spec.TaskSpec.Steps[0].Env)).To(Equal(0)) + + Expect(len(taskRun.Spec.TaskSpec.Steps[1].Env)).To(Equal(1)) + envVarName := taskRun.Spec.TaskSpec.Steps[1].Env[0].Name + + Expect(envVarName).To(HavePrefix("SHP_SECRET_PARAM_")) + Expect(taskRun.Spec.TaskSpec.Steps[1].Env[0]).To(BeEquivalentTo(corev1.EnvVar{ + Name: envVarName, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "secret-name", + }, + Key: "secret-key", + }, + }, + })) + + // Verify the parameters + Expect(taskRun.Spec.Params).To(BeEquivalentTo([]pipeline.Param{ + { + Name: "array-parameter", + Value: pipeline.ArrayOrString{ + Type: pipeline.ParamTypeArray, + ArrayVal: []string{ + "first entry", + fmt.Sprintf("$(%s)", envVarName), + fmt.Sprintf("The secret value is $(%s)", envVarName), + }, + }, + }, + })) + }) + }) +}) + +var _ = Describe("ValidateBuildRunParameters", func() { + + Context("for a set of parameter definitions", func() { + parameterDefinitions := []buildv1alpha1.Parameter{ + { + Name: "string-param-no-default", + }, + { + Name: "string-param-with-default", + Type: buildv1alpha1.ParameterTypeString, + Default: pointer.String("default value"), + }, + { + Name: "array-param-no-defaults", + Type: buildv1alpha1.ParameterTypeArray, + }, + { + Name: "array-param-with-defaults", + Type: buildv1alpha1.ParameterTypeArray, + Defaults: &[]string{}, + }, + } + + Context("for parameters just for the required fields", func() { + buildParamValues := []buildv1alpha1.ParamValue{ + { + Name: "string-param-no-default", + SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("a value"), + }, + }, + } + + buildRunParamValues := []buildv1alpha1.ParamValue{ + { + Name: "array-param-no-defaults", + Values: []buildv1alpha1.SingleValue{}, + }, + } + + It("validates without an error", func() { + valid, _, _ := ValidateBuildRunParameters(parameterDefinitions, buildParamValues, buildRunParamValues) + Expect(valid).To(BeTrue()) + }) + }) + + Context("for parameter values from different sources", func() { + buildParamValues := []buildv1alpha1.ParamValue{ + { + Name: "string-param-no-default", + SingleValue: &buildv1alpha1.SingleValue{ + ConfigMapValue: &buildv1alpha1.ObjectKeyRef{ + Name: "a-config-map", + Key: "some-key", + }, + }, + }, + { + Name: "string-param-with-default", + // This is invalid but will be corrected in the BuildRun + Values: []buildv1alpha1.SingleValue{}, + }, + } + + buildRunParamValues := []buildv1alpha1.ParamValue{ + { + Name: "array-param-no-defaults", + Values: []buildv1alpha1.SingleValue{ + { + SecretValue: &buildv1alpha1.ObjectKeyRef{ + Name: "a-secret", + Key: "my-credential-key", + }, + }, + }, + }, + { + Name: "string-param-with-default", + SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String(""), + }, + }, + } + + It("validates without an error", func() { + valid, _, _ := ValidateBuildRunParameters(parameterDefinitions, buildParamValues, buildRunParamValues) + Expect(valid).To(BeTrue()) + }) + }) + + Context("for parameter values that are missing", func() { + + It("validates with the correct validation error", func() { + valid, reason, message := ValidateBuildRunParameters(parameterDefinitions, []buildv1alpha1.ParamValue{}, []buildv1alpha1.ParamValue{}) + Expect(valid).To(BeFalse()) + Expect(reason).To(Equal("MissingParameterValues")) + Expect(message).To(HavePrefix("The following parameters are required but no value has been provided:")) + Expect(message).To(ContainSubstring("array-param-no-defaults")) + Expect(message).To(ContainSubstring("string-param-no-default")) + }) + }) + + Context("for a parameter value that is defined but contains no value", func() { + buildParamValues := []buildv1alpha1.ParamValue{ + { + Name: "string-param-no-default", + SingleValue: &buildv1alpha1.SingleValue{}, + }, + } + + buildRunParamValues := []buildv1alpha1.ParamValue{ + { + Name: "array-param-no-defaults", + Values: []buildv1alpha1.SingleValue{}, + }, + } + + It("validates with the correct validation error", func() { + valid, reason, message := ValidateBuildRunParameters(parameterDefinitions, buildParamValues, buildRunParamValues) + Expect(valid).To(BeFalse()) + Expect(reason).To(Equal("MissingParameterValues")) + Expect(message).To(Equal("The following parameters are required but no value has been provided: string-param-no-default")) + }) + }) + + Context("for parameter values that contain a value for a system parameter", func() { + buildParamValues := []buildv1alpha1.ParamValue{ + { + Name: "string-param-no-default", + SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("a value"), + }, + }, + { + Name: "array-param-no-defaults", + Values: []buildv1alpha1.SingleValue{}, + }, + } + + buildRunParamValues := []buildv1alpha1.ParamValue{ + { + Name: "shp-source-context", + SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("/my-source"), + }, + }, + } + + It("validates with the correct validation error", func() { + valid, reason, message := ValidateBuildRunParameters(parameterDefinitions, buildParamValues, buildRunParamValues) + Expect(valid).To(BeFalse()) + Expect(reason).To(Equal("RestrictedParametersInUse")) + Expect(message).To(Equal("The following parameters are restricted and cannot be set: shp-source-context")) + }) + }) + + Context("for parameter values that are not defined in the build strategy", func() { + buildParamValues := []buildv1alpha1.ParamValue{ + { + Name: "string-param-no-default", + SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("a value"), + }, + }, + { + Name: "array-param-no-defaults", + Values: []buildv1alpha1.SingleValue{}, + }, + { + Name: "non-existing-parameter-on-build", + Values: []buildv1alpha1.SingleValue{}, + }, + } + + buildRunParamValues := []buildv1alpha1.ParamValue{ + { + Name: "non-existing-parameter", + SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("my value"), + }, + }, + } + + It("validates with the correct validation error", func() { + valid, reason, message := ValidateBuildRunParameters(parameterDefinitions, buildParamValues, buildRunParamValues) + Expect(valid).To(BeFalse()) + Expect(reason).To(Equal("UndefinedParameter")) + Expect(message).To(HavePrefix("The following parameters are not defined in the build strategy:")) + Expect(message).To(ContainSubstring("non-existing-parameter-on-build")) + Expect(message).To(ContainSubstring("non-existing-parameter")) + }) + }) + + Context("for parameter values that contain more than one value", func() { + buildParamValues := []buildv1alpha1.ParamValue{ + { + Name: "string-param-no-default", + SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("a value"), + ConfigMapValue: &buildv1alpha1.ObjectKeyRef{ + Name: "a-config-map", + Key: "a-key", + }, + }, + }, + } + + buildRunParamValues := []buildv1alpha1.ParamValue{ + { + Name: "array-param-no-defaults", + Values: []buildv1alpha1.SingleValue{ + { + Value: pointer.String("a good item"), + }, + { + ConfigMapValue: &buildv1alpha1.ObjectKeyRef{ + Name: "a-config-map", + Key: "a-key", + }, + SecretValue: &buildv1alpha1.ObjectKeyRef{ + Name: "a-secret", + Key: "a-key", + }, + }, + }, + }, + } + + It("validates with the correct validation error", func() { + valid, reason, message := ValidateBuildRunParameters(parameterDefinitions, buildParamValues, buildRunParamValues) + Expect(valid).To(BeFalse()) + Expect(reason).To(Equal("InconsistentParameterValues")) + Expect(message).To(HavePrefix("The following parameters have more than one of 'configMapValue', 'secretValue', and 'value' set:")) + Expect(message).To(ContainSubstring("string-param-no-default")) + Expect(message).To(ContainSubstring("array-param-no-defaults")) + }) + }) + + Context("for parameter values that use the wrong type", func() { + buildParamValues := []buildv1alpha1.ParamValue{ + { + Name: "string-param-no-default", + Values: []buildv1alpha1.SingleValue{ + { + Value: pointer.String("an item"), + }, + { + ConfigMapValue: &buildv1alpha1.ObjectKeyRef{ + Name: "a-config-map", + Key: "a-key", + }, + }, + }, + }, + } + + buildRunParamValues := []buildv1alpha1.ParamValue{ + { + Name: "array-param-no-defaults", + SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String("a value"), + }, + }, + } + + It("validates with the correct validation error", func() { + valid, reason, message := ValidateBuildRunParameters(parameterDefinitions, buildParamValues, buildRunParamValues) + Expect(valid).To(BeFalse()) + Expect(reason).To(Equal("WrongParameterValueType")) + Expect(message).To(HavePrefix("The values for the following parameters are using the wrong type:")) + Expect(message).To(ContainSubstring("string-param-no-default")) + Expect(message).To(ContainSubstring("array-param-no-defaults")) + }) + }) + + Context("for array parameter values that contain empty items", func() { + buildParamValues := []buildv1alpha1.ParamValue{ + { + Name: "string-param-no-default", + SingleValue: &buildv1alpha1.SingleValue{ + Value: pointer.String(" some value"), + }, + }, + { + Name: "array-param-with-defaults", + Values: []buildv1alpha1.SingleValue{ + { + // the bad item without any value + }, + }, + }, + } + + buildRunParamValues := []buildv1alpha1.ParamValue{ + { + Name: "array-param-no-defaults", + Values: []buildv1alpha1.SingleValue{ + { + Value: pointer.String("a good item"), + }, + { + // the bad item without any value + }, + { + ConfigMapValue: &buildv1alpha1.ObjectKeyRef{ + Name: "a-configmap", + Key: "a-key", + }, + }, + }, + }, + } + + It("validates with the correct validation error", func() { + valid, reason, message := ValidateBuildRunParameters(parameterDefinitions, buildParamValues, buildRunParamValues) + Expect(valid).To(BeFalse()) + Expect(reason).To(Equal("EmptyArrayItemParameterValues")) + Expect(message).To(HavePrefix("The values for the following array parameters are containing at least one item where none of 'configMapValue', 'secretValue', and 'value' are set:")) + Expect(message).To(ContainSubstring("array-param-with-defaults")) + Expect(message).To(ContainSubstring("array-param-no-defaults")) + }) + }) + + Context("for parameter values that contain incomplete configMapValues", func() { + buildParamValues := []buildv1alpha1.ParamValue{ + { + Name: "string-param-no-default", + SingleValue: &buildv1alpha1.SingleValue{ + ConfigMapValue: &buildv1alpha1.ObjectKeyRef{ + Name: "a-config-map", + }, + }, + }, + } + + buildRunParamValues := []buildv1alpha1.ParamValue{ + { + Name: "array-param-no-defaults", + Values: []buildv1alpha1.SingleValue{ + { + Value: pointer.String("an item"), + }, + { + ConfigMapValue: &buildv1alpha1.ObjectKeyRef{ + Key: "a-key", + }, + }, + }, + }, + } + + It("validates with the correct validation error", func() { + valid, reason, message := ValidateBuildRunParameters(parameterDefinitions, buildParamValues, buildRunParamValues) + Expect(valid).To(BeFalse()) + Expect(reason).To(Equal("IncompleteConfigMapValueParameterValues")) + Expect(message).To(HavePrefix("The values for the following parameters are containing a 'configMapValue' with an empty 'name' or 'key':")) + Expect(message).To(ContainSubstring("string-param-no-default")) + Expect(message).To(ContainSubstring("array-param-no-defaults")) + }) + }) + + Context("for parameter values that contain incomplete secretValues", func() { + buildParamValues := []buildv1alpha1.ParamValue{ + { + Name: "string-param-no-default", + SingleValue: &buildv1alpha1.SingleValue{ + SecretValue: &buildv1alpha1.ObjectKeyRef{ + Name: "a-secret", + }, + }, + }, + } + + buildRunParamValues := []buildv1alpha1.ParamValue{ + { + Name: "array-param-no-defaults", + Values: []buildv1alpha1.SingleValue{ + { + Value: pointer.String("an item"), + }, + { + SecretValue: &buildv1alpha1.ObjectKeyRef{ + Key: "a-key", + }, + }, + }, + }, + } + + It("validates with the correct validation error", func() { + valid, reason, message := ValidateBuildRunParameters(parameterDefinitions, buildParamValues, buildRunParamValues) + Expect(valid).To(BeFalse()) + Expect(reason).To(Equal("IncompleteSecretValueParameterValues")) + Expect(message).To(HavePrefix("The values for the following parameters are containing a 'secretValue' with an empty 'name' or 'key':")) + Expect(message).To(ContainSubstring("string-param-no-default")) + Expect(message).To(ContainSubstring("array-param-no-defaults")) + }) + }) + }) +}) diff --git a/test/build_samples.go b/test/build_samples.go index a8d9abfcdd..cce41843dd 100644 --- a/test/build_samples.go +++ b/test/build_samples.go @@ -372,7 +372,50 @@ spec: url: "https://github.com/shipwright-io/sample-go" paramValues: - name: sleep-time - value: 30 + value: "30" + strategy: + kind: BuildStrategy + output: + image: image-registry.openshift-image-registry.svc:5000/example/buildpacks-app +` + +// BuildWithArrayParam defines a Build with an array parameter +const BuildWithArrayParam = ` +apiVersion: shipwright.io/v1alpha1 +kind: Build +spec: + source: + url: "https://github.com/shipwright-io/sample-go" + paramValues: + - name: array-param + values: + - value: "3" + - value: "-1" + strategy: + kind: BuildStrategy + output: + image: image-registry.openshift-image-registry.svc:5000/example/buildpacks-app +` + +// BuildWithConfigMapSecretParams defines a Build with parameter values referencing a ConfigMap and Secret +const BuildWithConfigMapSecretParams = ` +apiVersion: shipwright.io/v1alpha1 +kind: Build +spec: + source: + url: "https://github.com/shipwright-io/sample-go" + paramValues: + - name: array-param + values: + - value: "3" + - configMapValue: + name: a-configmap + key: a-cm-key + - value: "-1" + - name: sleep-time + secretValue: + name: a-secret + key: a-secret-key strategy: kind: BuildStrategy output: @@ -389,9 +432,9 @@ spec: url: "https://github.com/shipwright-io/sample-go" paramValues: - name: shp-something - value: 30 + value: "30" - name: DOCKERFILE - value: 30 + value: "30" strategy: kind: BuildStrategy output: @@ -408,7 +451,7 @@ spec: url: "https://github.com/shipwright-io/sample-go" paramValues: - name: sleep-not - value: 30 + value: "30" strategy: kind: BuildStrategy output: @@ -441,7 +484,7 @@ spec: url: "https://github.com/shipwright-io/sample-go" paramValues: - name: sleep-not - value: 30 + value: "30" strategy: kind: ClusterBuildStrategy output: @@ -457,7 +500,7 @@ spec: url: "https://github.com/shipwright-io/sample-go" paramValues: - name: sleep-time - value: 30 + value: "30" strategy: kind: ClusterBuildStrategy output: diff --git a/test/buildrun_samples.go b/test/buildrun_samples.go index 25eb1b15f6..8b9945527a 100644 --- a/test/buildrun_samples.go +++ b/test/buildrun_samples.go @@ -138,7 +138,7 @@ kind: BuildRun spec: paramValues: - name: sleep-time - value: 15 + value: "15" buildRef: name: foobar ` @@ -149,7 +149,7 @@ kind: BuildRun spec: paramValues: - name: shp-sleep-time - value: 15 + value: "15" buildRef: name: foobar ` diff --git a/test/buildstrategy_samples.go b/test/buildstrategy_samples.go index 39731cf729..5babdf3a33 100644 --- a/test/buildstrategy_samples.go +++ b/test/buildstrategy_samples.go @@ -198,7 +198,7 @@ spec: // BuildStrategyWithParameters is a strategy that uses a // sleep command with a value for its spec.parameters const BuildStrategyWithParameters = ` -apiVersion: build.dev/v1alpha1 +apiVersion: shipwright.io/v1alpha1 kind: BuildStrategy metadata: name: strategy-with-param @@ -207,6 +207,10 @@ spec: - name: sleep-time description: "time in seconds for sleeping" default: "1" + - name: array-param + descripion: "An arbitrary array" + type: array + defaults: [] buildSteps: - name: sleep30 image: alpine:latest @@ -214,13 +218,32 @@ spec: - sleep args: - $(params.sleep-time) + - name: echo-array-sum + image: alpine:latest + command: + - /bin/bash + args: + - -c + - | + set -euo pipefail + + sum=0 + + for var in "$@" + do + sum=$((sum+var)) + done + + echo "Sum: ${sum}" + - -- + - $(params.array-param[*]) ` // BuildStrategyWithoutDefaultInParameter is a strategy that uses a // sleep command with a value from its spec.parameters, where the parameter // have no default const BuildStrategyWithoutDefaultInParameter = ` -apiVersion: build.dev/v1alpha1 +apiVersion: shipwright.io/v1alpha1 kind: BuildStrategy metadata: name: strategy-with-param-and-no-default @@ -240,7 +263,7 @@ spec: // BuildStrategyWithErrorResult is a strategy that always fails // and surfaces and error reason and message to the user const BuildStrategyWithErrorResult = ` -apiVersion: build.dev/v1alpha1 +apiVersion: shipwright.io/v1alpha1 kind: BuildStrategy metadata: name: strategy-with-error-results @@ -257,3 +280,60 @@ spec: printf "integration test error message" > $(results.shp-error-message.path); return 1 ` + +// BuildStrategyWithParameterVerification is a strategy that verifies that parameters can be used at all expected places +const BuildStrategyWithParameterVerification = ` +apiVersion: shipwright.io/v1alpha1 +kind: BuildStrategy +metadata: + name: strategy-with-parameter-verification +spec: + parameters: + - name: env1 + description: "This parameter will be used in the env of the build step" + type: string + - name: env2 + description: "This parameter will be used in the env of the build step" + type: string + - name: env3 + description: "This parameter will be used in the env of the build step" + type: string + - name: image + description: "This parameter will be used as the image of the build step" + type: string + - name: commands + description: "This parameter will be used as the command of the build step" + type: array + - name: args + description: "This parameter will be used as the args of the build step" + type: array + buildSteps: + - name: calculate-sum + image: $(params.image) + env: + - name: ENV_FROM_PARAMETER1 + value: $(params.env1) + - name: ENV_FROM_PARAMETER2 + value: $(params.env2) + - name: ENV_FROM_PARAMETER3 + value: $(params.env3) + command: + - $(params.commands[*]) + args: + - | + set -euo pipefail + + sum=$((ENV_FROM_PARAMETER1 + ENV_FROM_PARAMETER2 + ENV_FROM_PARAMETER3)) + + for var in "$@" + do + sum=$((sum+var)) + done + + echo "Sum: ${sum}" + # Once we have strategy-defined results, then those would be better suitable + # Until then, just store it as image size :-) + echo -n "${sum}" > '$(results.shp-image-size.path)' + - -- + - $(params.args[*]) +` diff --git a/test/catalog.go b/test/catalog.go index 17a8f9bf83..f321e35f2e 100644 --- a/test/catalog.go +++ b/test/catalog.go @@ -51,6 +51,28 @@ func (c *Catalog) SecretWithoutAnnotation(name string, ns string) *corev1.Secret } } +// SecretWithStringData creates a Secret with stringData (not base64 encoded) +func (c *Catalog) SecretWithStringData(name string, ns string, stringData map[string]string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: ns, + }, + StringData: stringData, + } +} + +// ConfigMapWithData creates a ConfigMap with data +func (c *Catalog) ConfigMapWithData(name string, ns string, data map[string]string) *corev1.ConfigMap { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: ns, + }, + Data: data, + } +} + // BuildWithClusterBuildStrategyAndFalseSourceAnnotation gives you an specific Build CRD func (c *Catalog) BuildWithClusterBuildStrategyAndFalseSourceAnnotation(name string, ns string, strategyName string) *build.Build { buildStrategy := build.ClusterBuildStrategyKind diff --git a/test/e2e/common_suite_test.go b/test/e2e/common_suite_test.go index 4aae3f0125..b82652fb36 100644 --- a/test/e2e/common_suite_test.go +++ b/test/e2e/common_suite_test.go @@ -47,6 +47,15 @@ func (b *buildPrototype) Namespace(namespace string) *buildPrototype { return b } +func (b *buildPrototype) BuildStrategy(name string) *buildPrototype { + var bs = buildv1alpha1.NamespacedBuildStrategyKind + b.build.Spec.Strategy = &buildv1alpha1.Strategy{ + Kind: &bs, + Name: name, + } + return b +} + func (b *buildPrototype) ClusterBuildStrategy(name string) *buildPrototype { var cbs = buildv1alpha1.ClusterBuildStrategyKind b.build.Spec.Strategy = &buildv1alpha1.Strategy{ @@ -56,6 +65,12 @@ func (b *buildPrototype) ClusterBuildStrategy(name string) *buildPrototype { return b } +func (b *buildPrototype) SourceGit(repository string) *buildPrototype { + b.build.Spec.Source.URL = repository + b.build.Spec.Source.BundleContainer = nil + return b +} + func (b *buildPrototype) SourceBundle(image string) *buildPrototype { if b.build.Spec.Source.BundleContainer == nil { b.build.Spec.Source.BundleContainer = &buildv1alpha1.BundleContainer{} @@ -79,6 +94,104 @@ func (b *buildPrototype) OutputImage(image string) *buildPrototype { return b } +func (b *buildPrototype) determineParameterIndex(name string) int { + index := -1 + for i, paramValue := range b.build.Spec.ParamValues { + if paramValue.Name == name { + index = i + break + } + } + + if index == -1 { + index = len(b.build.Spec.ParamValues) + b.build.Spec.ParamValues = append(b.build.Spec.ParamValues, buildv1alpha1.ParamValue{ + Name: name, + }) + } + + return index +} + +// ArrayParamValue adds an item to an array parameter, if the parameter is not yet present, it is being added +func (b *buildPrototype) ArrayParamValue(name string, value string) *buildPrototype { + index := b.determineParameterIndex(name) + b.build.Spec.ParamValues[index].Values = append(b.build.Spec.ParamValues[index].Values, buildv1alpha1.SingleValue{ + Value: &value, + }) + + return b +} + +// ArrayParamValueFromConfigMap adds an item to an array parameter, if the parameter is not yet present, it is being added +func (b *buildPrototype) ArrayParamValueFromConfigMap(name string, configMapName string, configMapKey string, format *string) *buildPrototype { + index := b.determineParameterIndex(name) + b.build.Spec.ParamValues[index].Values = append(b.build.Spec.ParamValues[index].Values, buildv1alpha1.SingleValue{ + ConfigMapValue: &buildv1alpha1.ObjectKeyRef{ + Name: configMapName, + Key: configMapKey, + Format: format, + }, + }) + + return b +} + +// ArrayParamValueFromSecret adds an item to an array parameter, if the parameter is not yet present, it is being added +func (b *buildPrototype) ArrayParamValueFromSecret(name string, secretName string, secretKey string, format *string) *buildPrototype { + index := b.determineParameterIndex(name) + b.build.Spec.ParamValues[index].Values = append(b.build.Spec.ParamValues[index].Values, buildv1alpha1.SingleValue{ + SecretValue: &buildv1alpha1.ObjectKeyRef{ + Name: secretName, + Key: secretKey, + Format: format, + }, + }) + + return b +} + +func (b *buildPrototype) StringParamValue(name string, value string) *buildPrototype { + b.build.Spec.ParamValues = append(b.build.Spec.ParamValues, buildv1alpha1.ParamValue{ + Name: name, + SingleValue: &buildv1alpha1.SingleValue{ + Value: &value, + }, + }) + + return b +} + +func (b *buildPrototype) StringParamValueFromConfigMap(name string, configMapName string, configMapKey string, format *string) *buildPrototype { + b.build.Spec.ParamValues = append(b.build.Spec.ParamValues, buildv1alpha1.ParamValue{ + Name: name, + SingleValue: &buildv1alpha1.SingleValue{ + ConfigMapValue: &buildv1alpha1.ObjectKeyRef{ + Name: configMapName, + Key: configMapKey, + Format: format, + }, + }, + }) + + return b +} + +func (b *buildPrototype) StringParamValueFromSecret(name string, secretName string, secretKey string, format *string) *buildPrototype { + b.build.Spec.ParamValues = append(b.build.Spec.ParamValues, buildv1alpha1.ParamValue{ + Name: name, + SingleValue: &buildv1alpha1.SingleValue{ + SecretValue: &buildv1alpha1.ObjectKeyRef{ + Name: secretName, + Key: secretKey, + Format: format, + }, + }, + }) + + return b +} + func (b *buildPrototype) OutputImageCredentials(name string) *buildPrototype { if name != "" { b.build.Spec.Output.Credentials = &core.LocalObjectReference{Name: name} @@ -135,6 +248,104 @@ func (b *buildRunPrototype) GenerateServiceAccount() *buildRunPrototype { return b } +func (b *buildRunPrototype) determineParameterIndex(name string) int { + index := -1 + for i, paramValue := range b.buildRun.Spec.ParamValues { + if paramValue.Name == name { + index = i + break + } + } + + if index == -1 { + index = len(b.buildRun.Spec.ParamValues) + b.buildRun.Spec.ParamValues = append(b.buildRun.Spec.ParamValues, buildv1alpha1.ParamValue{ + Name: name, + }) + } + + return index +} + +// ArrayParamValue adds an item to an array parameter, if the parameter is not yet present, it is being added +func (b *buildRunPrototype) ArrayParamValue(name string, value string) *buildRunPrototype { + index := b.determineParameterIndex(name) + b.buildRun.Spec.ParamValues[index].Values = append(b.buildRun.Spec.ParamValues[index].Values, buildv1alpha1.SingleValue{ + Value: &value, + }) + + return b +} + +// ArrayParamValueFromConfigMap adds an item to an array parameter, if the parameter is not yet present, it is being added +func (b *buildRunPrototype) ArrayParamValueFromConfigMap(name string, configMapName string, configMapKey string, format *string) *buildRunPrototype { + index := b.determineParameterIndex(name) + b.buildRun.Spec.ParamValues[index].Values = append(b.buildRun.Spec.ParamValues[index].Values, buildv1alpha1.SingleValue{ + ConfigMapValue: &buildv1alpha1.ObjectKeyRef{ + Name: configMapName, + Key: configMapKey, + Format: format, + }, + }) + + return b +} + +// ArrayParamValueFromSecret adds an item to an array parameter, if the parameter is not yet present, it is being added +func (b *buildRunPrototype) ArrayParamValueFromSecret(name string, secretName string, secretKey string, format *string) *buildRunPrototype { + index := b.determineParameterIndex(name) + b.buildRun.Spec.ParamValues[index].Values = append(b.buildRun.Spec.ParamValues[index].Values, buildv1alpha1.SingleValue{ + SecretValue: &buildv1alpha1.ObjectKeyRef{ + Name: secretName, + Key: secretKey, + Format: format, + }, + }) + + return b +} + +func (b *buildRunPrototype) StringParamValue(name string, value string) *buildRunPrototype { + b.buildRun.Spec.ParamValues = append(b.buildRun.Spec.ParamValues, buildv1alpha1.ParamValue{ + Name: name, + SingleValue: &buildv1alpha1.SingleValue{ + Value: &value, + }, + }) + + return b +} + +func (b *buildRunPrototype) StringParamValueFromConfigMap(name string, configMapName string, configMapKey string, format *string) *buildRunPrototype { + b.buildRun.Spec.ParamValues = append(b.buildRun.Spec.ParamValues, buildv1alpha1.ParamValue{ + Name: name, + SingleValue: &buildv1alpha1.SingleValue{ + ConfigMapValue: &buildv1alpha1.ObjectKeyRef{ + Name: configMapName, + Key: configMapKey, + Format: format, + }, + }, + }) + + return b +} + +func (b *buildRunPrototype) StringParamValueFromSecret(name string, secretName string, secretKey string, format *string) *buildRunPrototype { + b.buildRun.Spec.ParamValues = append(b.buildRun.Spec.ParamValues, buildv1alpha1.ParamValue{ + Name: name, + SingleValue: &buildv1alpha1.SingleValue{ + SecretValue: &buildv1alpha1.ObjectKeyRef{ + Name: secretName, + Key: secretKey, + Format: format, + }, + }, + }) + + return b +} + func (b *buildRunPrototype) Create() (*buildv1alpha1.BuildRun, error) { return testBuild. BuildClientSet. diff --git a/test/e2e/e2e_params_test.go b/test/e2e/e2e_params_test.go new file mode 100644 index 0000000000..1fc58a5a41 --- /dev/null +++ b/test/e2e/e2e_params_test.go @@ -0,0 +1,139 @@ +// Copyright The Shipwright Contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package e2e_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" + "github.com/shipwright-io/build/test" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/pointer" +) + +var _ = Describe("For a Kubernetes cluster with Tekton and build installed", func() { + var ( + testID string + err error + + build *buildv1alpha1.Build + buildRun *buildv1alpha1.BuildRun + buildStrategy *buildv1alpha1.BuildStrategy + configMap *corev1.ConfigMap + secret *corev1.Secret + ) + + AfterEach(func() { + if CurrentGinkgoTestDescription().Failed { + printTestFailureDebugInfo(testBuild, testBuild.Namespace, testID) + } else if buildRun != nil { + validateServiceAccountDeletion(buildRun, testBuild.Namespace) + } + + if buildRun != nil { + testBuild.DeleteBR(buildRun.Name) + buildRun = nil + } + + if build != nil { + testBuild.DeleteBuild(build.Name) + build = nil + } + + if buildStrategy != nil { + testBuild.DeleteBuildStrategy(buildStrategy.Name) + buildStrategy = nil + } + + if configMap != nil { + testBuild.DeleteConfigMap(configMap.Name) + configMap = nil + } + + if secret != nil { + testBuild.DeleteSecret(secret.Name) + secret = nil + } + }) + + Context("when using a cluster build strategy is used that uses a lot parameters", func() { + + BeforeEach(func() { + buildStrategy, err = testBuild.Catalog.LoadBuildStrategyFromBytes([]byte(test.BuildStrategyWithParameterVerification)) + Expect(err).ToNot(HaveOccurred()) + err = testBuild.CreateBuildStrategy(buildStrategy) + Expect(err).ToNot(HaveOccurred()) + }) + + Context("when a secret and a configmap are in place with suitable values", func() { + BeforeEach(func() { + // prepare a ConfigMap + configMap = testBuild.Catalog.ConfigMapWithData("a-configmap", testBuild.Namespace, map[string]string{ + "number1": "1", + "shell": "/bin/bash", + }) + err = testBuild.CreateConfigMap(configMap) + Expect(err).ToNot(HaveOccurred()) + + // prepare a secret + secret = testBuild.Catalog.SecretWithStringData("a-secret", testBuild.Namespace, map[string]string{ + "number2": "2", + "number3": "3", + }) + err = testBuild.CreateSecret(secret) + Expect(err).ToNot(HaveOccurred()) + }) + + Context("when a Build is in place that sets some of the parameters", func() { + BeforeEach(func() { + testID = generateTestID("params") + + build, err = NewBuildPrototype(). + BuildStrategy(buildStrategy.Name). + Name(testID). + Namespace(testBuild.Namespace). + // The source is not actually used by the build, so just take a small one + SourceGit("https://github.com/shipwright-io/sample-go.git"). + // There is not actually an image pushed + OutputImage("dummy"). + // The parameters + StringParamValue("env1", "13"). + StringParamValueFromConfigMap("env2", "a-configmap", "number1", pointer.String("2${CONFIGMAP_VALUE}")). + ArrayParamValueFromConfigMap("commands", "a-configmap", "shell", nil). + ArrayParamValue("commands", "-c"). + Create() + Expect(err).ToNot(HaveOccurred()) + }) + + It("correctly runs a BuildRun that passes the remaining parameters", func() { + buildRun, err = NewBuildRunPrototype(). + ForBuild(build). + Name(testID). + GenerateServiceAccount(). + StringParamValue("image", "registry.access.redhat.com/ubi8/ubi-minimal"). + StringParamValueFromSecret("env3", "a-secret", "number2", nil). + ArrayParamValueFromSecret("args", "a-secret", "number3", pointer.String("${SECRET_VALUE}9")). + ArrayParamValue("args", "47"). + Create() + Expect(err).ToNot(HaveOccurred()) + + validateBuildRunToSucceed(testBuild, buildRun) + + // we verify the image digest here which is mis-used by the strategy to store a calculated sum + // 13 (env1) + 21 (env2 = 2${a-configmap:number1}) + 2 (env3 = ${a-secret:number2}) + 39 (args[0] = ${a-secret:number3}9) + 47 (args[1]) = 122 + buildRun, err = testBuild.LookupBuildRun(types.NamespacedName{ + Namespace: buildRun.Namespace, + Name: buildRun.Name, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(buildRun.Status.Output).NotTo(BeNil()) + Expect(buildRun.Status.Output.Size).To(BeEquivalentTo(122)) + }) + }) + }) + }) +}) diff --git a/test/integration/buildstrategy_to_taskruns_test.go b/test/integration/buildstrategy_to_taskruns_test.go index 3c2a8eeb2a..8476effd7a 100644 --- a/test/integration/buildstrategy_to_taskruns_test.go +++ b/test/integration/buildstrategy_to_taskruns_test.go @@ -5,12 +5,15 @@ package integration_test import ( + "fmt" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" "github.com/shipwright-io/build/test" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + corev1 "k8s.io/api/core/v1" ) var _ = Describe("Integration tests BuildStrategies and TaskRuns", func() { @@ -18,6 +21,8 @@ var _ = Describe("Integration tests BuildStrategies and TaskRuns", func() { bsObject *v1alpha1.BuildStrategy buildObject *v1alpha1.Build buildRunObject *v1alpha1.BuildRun + secret *corev1.Secret + configMap *corev1.ConfigMap buildSample []byte buildRunSample []byte ) @@ -31,9 +36,8 @@ var _ = Describe("Integration tests BuildStrategies and TaskRuns", func() { Expect(err).To(BeNil()) }) - // Delete the BuildStrategies after each test case + // Delete the Build and BuildStrategy after each test case AfterEach(func() { - _, err = tb.GetBuild(buildObject.Name) if err == nil { Expect(tb.DeleteBuild(buildObject.Name)).To(BeNil()) @@ -41,6 +45,16 @@ var _ = Describe("Integration tests BuildStrategies and TaskRuns", func() { err := tb.DeleteBuildStrategy(bsObject.Name) Expect(err).To(BeNil()) + + if configMap != nil { + Expect(tb.DeleteConfigMap(configMap.Name)).NotTo(HaveOccurred()) + configMap = nil + } + + if secret != nil { + Expect(tb.DeleteSecret(secret.Name)).NotTo(HaveOccurred()) + secret = nil + } }) // Override the Build and BuildRun CRD instances to use @@ -109,7 +123,7 @@ var _ = Describe("Integration tests BuildStrategies and TaskRuns", func() { Expect(err).To(BeNil()) }) - var constructedParam = func(paramName string, val string) v1beta1.Param { + var constructStringParam = func(paramName string, val string) v1beta1.Param { return v1beta1.Param{ Name: paramName, Value: v1beta1.ArrayOrString{ @@ -119,6 +133,16 @@ var _ = Describe("Integration tests BuildStrategies and TaskRuns", func() { } } + var constructArrayParam = func(paramName string, values ...string) v1beta1.Param { + return v1beta1.Param{ + Name: paramName, + Value: v1beta1.ArrayOrString{ + Type: v1beta1.ParamTypeArray, + ArrayVal: values, + }, + } + } + var constructBuildObjectAndWait = func(b *v1alpha1.Build) { // Create the Build object in-cluster Expect(tb.CreateBuild(b)).To(BeNil()) @@ -135,7 +159,6 @@ var _ = Describe("Integration tests BuildStrategies and TaskRuns", func() { // Wait until the BuildRun is registered _, err = tb.GetBRTillStartTime(br.Name) Expect(err).To(BeNil()) - } It("uses sleep-time param if specified in the Build with buildstrategy", func() { @@ -154,8 +177,7 @@ var _ = Describe("Integration tests BuildStrategies and TaskRuns", func() { taskRun, err := tb.GetTaskRunFromBuildRun(buildRunObject.Name) Expect(err).To(BeNil()) - Expect(taskRun.Spec.Params).To(ContainElement(constructedParam("sleep-time", "30"))) - + Expect(taskRun.Spec.Params).To(ContainElement(constructStringParam("sleep-time", "30"))) }) It("overrides sleep-time param if specified in the BuildRun", func() { @@ -183,9 +205,82 @@ var _ = Describe("Integration tests BuildStrategies and TaskRuns", func() { taskRun, err := tb.GetTaskRunFromBuildRun(buildRunObject.Name) Expect(err).To(BeNil()) - Expect(taskRun.Spec.Params).To(ContainElement(constructedParam("sleep-time", "15"))) + Expect(taskRun.Spec.Params).To(ContainElement(constructStringParam("sleep-time", "15"))) + }) + + It("uses array-param if specified in the Build with buildstrategy", func() { + // Set BuildWithArrayParam with an array value of "3" and "-1" + buildObject, err = tb.Catalog.LoadBuildWithNameAndStrategy( + BUILD+tb.Namespace, + bsObject.Name, + []byte(test.BuildWithArrayParam), + ) + Expect(err).To(BeNil()) + + constructBuildObjectAndWait(buildObject) + constructBuildRunObjectAndWait(buildRunObject) + + taskRun, err := tb.GetTaskRunFromBuildRun(buildRunObject.Name) + Expect(err).To(BeNil()) + + Expect(taskRun.Spec.Params).To(ContainElement(constructArrayParam("array-param", "3", "-1"))) }) + + It("uses params with references to ConfigMaps and Secrets correctly", func() { + // prepare a ConfigMap + configMap = tb.Catalog.ConfigMapWithData("a-configmap", tb.Namespace, map[string]string{ + "a-cm-key": "configmap-data", + }) + err = tb.CreateConfigMap(configMap) + Expect(err).ToNot(HaveOccurred()) + + // prepare a secret + secret = tb.Catalog.SecretWithStringData("a-secret", tb.Namespace, map[string]string{ + "a-secret-key": "a-value", + }) + err = tb.CreateSecret(secret) + Expect(err).ToNot(HaveOccurred()) + + // create the build + buildObject, err = tb.Catalog.LoadBuildWithNameAndStrategy( + BUILD+tb.Namespace, + bsObject.Name, + []byte(test.BuildWithConfigMapSecretParams), + ) + Expect(err).ToNot(HaveOccurred()) + + constructBuildObjectAndWait(buildObject) + + constructBuildRunObjectAndWait(buildRunObject) + + taskRun, err := tb.GetTaskRunFromBuildRun(buildRunObject.Name) + Expect(err).ToNot(HaveOccurred()) + + // the sleep30 step should have an env var for the secret + Expect(len(taskRun.Spec.TaskSpec.Steps)).To(Equal(3)) + Expect(len(taskRun.Spec.TaskSpec.Steps[1].Env)).To(Equal(1)) + Expect(taskRun.Spec.TaskSpec.Steps[1].Env[0].ValueFrom).NotTo(BeNil()) + Expect(taskRun.Spec.TaskSpec.Steps[1].Env[0].ValueFrom.SecretKeyRef).NotTo(BeNil()) + Expect(taskRun.Spec.TaskSpec.Steps[1].Env[0].ValueFrom.SecretKeyRef.Name).To(Equal("a-secret")) + Expect(taskRun.Spec.TaskSpec.Steps[1].Env[0].ValueFrom.SecretKeyRef.Key).To(Equal("a-secret-key")) + envVarNameSecret := taskRun.Spec.TaskSpec.Steps[1].Env[0].Name + Expect(envVarNameSecret).To(HavePrefix("SHP_SECRET_PARAM_")) + + // the echo-array-sum step should have an env var for the ConfigMap + Expect(len(taskRun.Spec.TaskSpec.Steps[2].Env)).To(Equal(1)) + Expect(taskRun.Spec.TaskSpec.Steps[2].Env[0].ValueFrom).NotTo(BeNil()) + Expect(taskRun.Spec.TaskSpec.Steps[2].Env[0].ValueFrom.ConfigMapKeyRef).NotTo(BeNil()) + Expect(taskRun.Spec.TaskSpec.Steps[2].Env[0].ValueFrom.ConfigMapKeyRef.Name).To(Equal("a-configmap")) + Expect(taskRun.Spec.TaskSpec.Steps[2].Env[0].ValueFrom.ConfigMapKeyRef.Key).To(Equal("a-cm-key")) + envVarNameConfigMap := taskRun.Spec.TaskSpec.Steps[2].Env[0].Name + Expect(envVarNameConfigMap).To(HavePrefix("SHP_CONFIGMAP_PARAM_")) + + // verify the parameters + Expect(taskRun.Spec.Params).To(ContainElement(constructStringParam("sleep-time", fmt.Sprintf("$(%s)", envVarNameSecret)))) + Expect(taskRun.Spec.Params).To(ContainElement(constructArrayParam("array-param", "3", fmt.Sprintf("$(%s)", envVarNameConfigMap), "-1"))) + }) + It("fails the TaskRun generation if the buildRun specifies a reserved system parameter", func() { // Build without params buildObject, err = tb.Catalog.LoadBuildWithNameAndStrategy( @@ -213,8 +308,9 @@ var _ = Describe("Integration tests BuildStrategies and TaskRuns", func() { br, err := tb.GetBRTillCompletion(buildRunObjectWithReservedParams.Name) Expect(err).To(BeNil()) - Expect(br.Status.GetCondition(v1alpha1.Succeeded).GetReason()).To(Equal("TaskRunGenerationFailed")) - Expect(br.Status.GetCondition(v1alpha1.Succeeded).GetMessage()).To(ContainSubstring("restricted parameters in use")) + Expect(br.Status.GetCondition(v1alpha1.Succeeded).GetReason()).To(Equal("RestrictedParametersInUse")) + Expect(br.Status.GetCondition(v1alpha1.Succeeded).GetMessage()).To(HavePrefix("The following parameters are restricted and cannot be set")) + Expect(br.Status.GetCondition(v1alpha1.Succeeded).GetMessage()).To(ContainSubstring("shp-sleep-time")) }) It("add params from buildRun if they are not defined in the Build", func() { @@ -241,7 +337,6 @@ var _ = Describe("Integration tests BuildStrategies and TaskRuns", func() { _, err = tb.GetTaskRunFromBuildRun(buildRunObject.Name) Expect(err).To(BeNil()) - }) It("fails the Build due to the usage of a restricted parameter name", func() { @@ -261,7 +356,8 @@ var _ = Describe("Integration tests BuildStrategies and TaskRuns", func() { Expect(err).To(BeNil()) Expect(buildObject.Status.Reason).To(Equal(v1alpha1.RestrictedParametersInUse)) - Expect(buildObject.Status.Message).To(ContainSubstring("restricted parameters in use")) + Expect(buildObject.Status.Message).To(HavePrefix("The following parameters are restricted and cannot be set:")) + Expect(buildObject.Status.Message).To(ContainSubstring("shp-something")) }) It("fails the Build due to the definition of an undefined param in the strategy", func() { @@ -281,7 +377,7 @@ var _ = Describe("Integration tests BuildStrategies and TaskRuns", func() { Expect(err).To(BeNil()) Expect(buildObject.Status.Reason).To(Equal(v1alpha1.UndefinedParameter)) - Expect(buildObject.Status.Message).To(ContainSubstring("parameter not defined in the strategies")) + Expect(buildObject.Status.Message).To(Equal("The following parameters are not defined in the build strategy: sleep-not")) }) It("allows a user to set an empty string on parameter without default", func() { @@ -386,10 +482,11 @@ var _ = Describe("Integration tests BuildStrategies and TaskRuns", func() { br, err := tb.GetBRTillCompletion(buildRunObject.Name) Expect(err).To(BeNil()) - Expect(br.Status.GetCondition(v1alpha1.Succeeded).GetReason()).To(Equal("TaskRunGenerationFailed")) - Expect(br.Status.GetCondition(v1alpha1.Succeeded).GetMessage()).To(Equal("parameters without a value definition: sleep-time")) - + Expect(br.Status.GetCondition(v1alpha1.Succeeded).GetReason()).To(Equal("MissingParameterValues")) + Expect(br.Status.GetCondition(v1alpha1.Succeeded).GetMessage()).To(HavePrefix("The following parameters are required but no value has been provided:")) + Expect(br.Status.GetCondition(v1alpha1.Succeeded).GetMessage()).To(ContainSubstring("sleep-time")) }) + Context("when a taskrun fails with an error result", func() { It("surfaces the result to the buildrun", func() { // Create a BuildStrategy that guarantees a failure diff --git a/test/utils/configmaps.go b/test/utils/configmaps.go new file mode 100644 index 0000000000..748bbd803b --- /dev/null +++ b/test/utils/configmaps.go @@ -0,0 +1,33 @@ +// Copyright The Shipwright Contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package utils + +import ( + "context" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// This class is intended to host all CRUD calls for testing configmap primitive resources + +// CreateConfigMap generates a ConfigMap on the current test namespace +func (t *TestBuild) CreateConfigMap(configMap *corev1.ConfigMap) error { + client := t.Clientset.CoreV1().ConfigMaps(t.Namespace) + _, err := client.Create(context.TODO(), configMap, metav1.CreateOptions{}) + if err != nil { + return err + } + return nil +} + +// DeleteConfigMap removes the desired configMap +func (t *TestBuild) DeleteConfigMap(name string) error { + client := t.Clientset.CoreV1().ConfigMaps(t.Namespace) + if err := client.Delete(context.TODO(), name, metav1.DeleteOptions{}); err != nil { + return err + } + return nil +} From 60ecd3aedeed0779122c79b987402025f9ecc0a2 Mon Sep 17 00:00:00 2001 From: Sascha Schwarze Date: Wed, 19 Jan 2022 16:59:23 +0100 Subject: [PATCH 4/5] Update BuildKit strategy to use array parameters --- samples/build/build_buildkit_cr.yaml | 4 +- .../buildkit/buildstrategy_buildkit_cr.yaml | 99 +++++++++++++++++-- .../build_buildkit_cr_insecure_registry.yaml | 4 +- 3 files changed, 91 insertions(+), 16 deletions(-) diff --git a/samples/build/build_buildkit_cr.yaml b/samples/build/build_buildkit_cr.yaml index 7972a10c2a..c77acef782 100644 --- a/samples/build/build_buildkit_cr.yaml +++ b/samples/build/build_buildkit_cr.yaml @@ -8,9 +8,7 @@ metadata: spec: source: url: https://github.com/shipwright-io/sample-go - contextDir: docker-build/ - # Note: This needs to be a path to where the Dockerfile is located, it cannot contain the file name - dockerfile: docker-build/ + contextDir: docker-build strategy: name: buildkit kind: ClusterBuildStrategy diff --git a/samples/buildstrategy/buildkit/buildstrategy_buildkit_cr.yaml b/samples/buildstrategy/buildkit/buildstrategy_buildkit_cr.yaml index 9a666a5b7a..27b48700a4 100644 --- a/samples/buildstrategy/buildkit/buildstrategy_buildkit_cr.yaml +++ b/samples/buildstrategy/buildkit/buildstrategy_buildkit_cr.yaml @@ -11,9 +11,22 @@ metadata: container.seccomp.security.alpha.kubernetes.io/step-build-and-push: unconfined spec: parameters: + - name: build-args + description: "The values for the ARGs in the Dockerfile. Values must be in the format KEY=VALUE." + type: array + defaults: [] + - name: cache + description: "Configure BuildKit's cache usage. Allowed values are 'disabled' and 'registry'. The default is 'registry'." + type: string + default: registry - name: insecure-registry + type: string description: "enables the push to an insecure registry" default: "false" + - name: secrets + description: "The secrets to pass to the build. Values must be in the format ID=FILE_CONTENT." + type: array + defaults: [] buildSteps: - name: prepare image: alpine:latest @@ -22,6 +35,7 @@ spec: capabilities: add: - CHOWN + - DAC_OVERRIDE command: - /bin/chown args: @@ -33,6 +47,10 @@ spec: imagePullPolicy: Always securityContext: allowPrivilegeEscalation: true + capabilities: + add: + - SETGID + - SETUID runAsUser: 1000 runAsGroup: 1000 workingDir: $(params.shp-source-root) @@ -51,15 +69,76 @@ spec: - | set -euo pipefail - buildctl-daemonless.sh build \ - --progress=plain \ - --frontend=dockerfile.v0 \ - --local=context='$(params.shp-source-context)' \ - --local=dockerfile='$(params.shp-source-root)/$(build.dockerfile)' \ - --output=type=image,name='$(params.shp-output-image)',push=true,registry.insecure=$(params.insecure-registry) \ - --export-cache=type=inline \ - --import-cache=type=registry,ref='$(params.shp-output-image)' \ - --metadata-file /tmp/image-metadata.json - + # Prepare the file arguments + DOCKERFILE_PATH='$(params.shp-source-context)/$(build.dockerfile)' + DOCKERFILE_DIR="$(dirname "${DOCKERFILE_PATH}")" + DOCKERFILE_NAME="$(basename "${DOCKERFILE_PATH}")" + + # We only have ash here and therefore no bash arrays to help add dynamic arguments (the build-args) to the build command. + + echo "#!/bin/ash" > /tmp/run.sh + echo "set -euo pipefail" >> /tmp/run.sh + echo "buildctl-daemonless.sh \\" >> /tmp/run.sh + echo "build \\" >> /tmp/run.sh + echo "--progress=plain \\" >> /tmp/run.sh + echo "--frontend=dockerfile.v0 \\" >> /tmp/run.sh + echo "--opt=filename=\"${DOCKERFILE_NAME}\" \\" >> /tmp/run.sh + echo "--local=context='$(params.shp-source-context)' \\" >> /tmp/run.sh + echo "--local=dockerfile=\"${DOCKERFILE_DIR}\" \\" >> /tmp/run.sh + echo "--output=type=image,name='$(params.shp-output-image)',push=true,registry.insecure=$(params.insecure-registry) \\" >> /tmp/run.sh + if [ "$(params.cache)" == "registry" ]; then + echo "--export-cache=type=inline \\" >> /tmp/run.sh + echo "--import-cache=type=registry,ref='$(params.shp-output-image)' \\" >> /tmp/run.sh + elif [ "$(params.cache)" == "disabled" ]; then + echo "--no-cache \\" >> /tmp/run.sh + else + echo -e "An invalid value for the parameter 'cache' has been provided: '$(params.cache)'. Allowed values are 'disabled' and 'registry'." + echo -n "InvalidParameterValue" > '$(results.shp-error-reason.path)' + echo -n "An invalid value for the parameter 'cache' has been provided: '$(params.cache)'. Allowed values are 'disabled' and 'registry'." > '$(results.shp-error-message.path)' + exit 1 + fi + + stage="" + for a in "$@" + do + if [ "${a}" == "--build-args" ]; then + stage=build-args + elif [ "${a}" == "--secrets" ]; then + stage=secrets + elif [ "${stage}" == "build-args" ]; then + echo "--opt=\"build-arg:${a}\" \\" >> /tmp/run.sh + elif [ "${stage}" == "secrets" ]; then + # Split ID=FILE_CONTENT into variables id and data + + # using head because the data could be multiline + id="$(echo "${a}" | head -1 | sed 's/=.*//')" + + # This is hacky, we remove the suffix ${id}= from all lines of the data. + # If the data would be multiple lines and a line would start with ${id}= + # then we would remove it. We could force users to give us the secret + # base64 encoded. But ultimately, the best solution might be if the user + # mounts the secret and just gives us the path here. + data="$(echo "${a}" | sed "s/^${id}=//")" + + # Write the secret data into a temporary file, once we have volume support + # in the build strategy, we should use a memory based emptyDir for this. + echo -n "${data}" > "/tmp/secret_${id}" + + # Add the secret argument + echo "--secret id=${id},src="/tmp/secret_${id}" \\" >> /tmp/run.sh + fi + done + + echo "--metadata-file /tmp/image-metadata.json" >> /tmp/run.sh + + chmod +x /tmp/run.sh + /tmp/run.sh + # Store the image digest sed -E 's/.*containerimage.digest":"([^"]*).*/\1/' < /tmp/image-metadata.json > '$(results.shp-image-digest.path)' + # That's the separator between the shell script and its args + - -- + - --build-args + - $(params.build-args[*]) + - --secrets + - $(params.secrets[*]) diff --git a/test/data/build_buildkit_cr_insecure_registry.yaml b/test/data/build_buildkit_cr_insecure_registry.yaml index 9e427d856c..aaa8c59324 100644 --- a/test/data/build_buildkit_cr_insecure_registry.yaml +++ b/test/data/build_buildkit_cr_insecure_registry.yaml @@ -8,9 +8,7 @@ metadata: spec: source: url: https://github.com/shipwright-io/sample-go - contextDir: docker-build/ - # Note: This needs to be a path to where the Dockerfile is located, it cannot contain the file name - dockerfile: docker-build/ + contextDir: docker-build paramValues: - name: insecure-registry value: "true" From 35eb99394f4e57e7e33fb01136bdba13380389be Mon Sep 17 00:00:00 2001 From: Sascha Schwarze Date: Wed, 19 Jan 2022 16:59:29 +0100 Subject: [PATCH 5/5] Documentation --- docs/build.md | 161 +++++++++++++++++---- docs/buildrun.md | 97 +++++++------ docs/buildstrategies.md | 165 +++++++++++++++++++--- docs/proposals/parameterize-strategies.md | 4 +- docs/tutorials/building_with_buildkit.md | 5 +- 5 files changed, 339 insertions(+), 93 deletions(-) diff --git a/docs/build.md b/docs/build.md index 110748371e..fe1016455d 100644 --- a/docs/build.md +++ b/docs/build.md @@ -80,7 +80,7 @@ The `Build` definition supports the following fields: - `spec.output.credentials.name`- Reference an existing secret to get access to the container registry. - Optional: - - `spec.paramValues` - Refers to a list of `key/value` that could be used to loosely type `parameters` in the `BuildStrategy`. + - `spec.paramValues` - Refers to a name-value(s) list to specify values for `parameters` defined in the `BuildStrategy`. - `spec.dockerfile` - Path to a Dockerfile to be used for building an image. (_Use this path for strategies that require a Dockerfile_) - `spec.sources` - [Sources](#Sources) describes a slice of artifacts that will be imported into project context, before the actual build process starts. - `spec.timeout` - Defines a custom timeout. The value needs to be parsable by [ParseDuration](https://golang.org/pkg/time/#ParseDuration), for example `5m`. The default is ten minutes. The value can be overwritten in the `BuildRun`. @@ -240,60 +240,167 @@ spec: ### Defining ParamValues -A `Build` resource can specify _params_, these allow users to modify the behaviour of the referenced `BuildStrategy` steps. +A `Build` resource can specify _paramValues_ for parameters that are defined in the referenced `BuildStrategy`. This allows one to control how the steps of the build strategy behave. Values can be overwritten in the `BuildRun` resource. See the related [documentation](./buildrun.md#defining-params) for more information. -When using _params_, users should avoid: +The build strategy author can define a parameter to be either a simple string, or an array. Depending on that, you must specify the value accordingly. The build strategy parameter can be specified with a default value. For parameters without a default, a value must be specified in the `Build` or `BuildRun`. + +You can either specify values directly, or reference keys from [ConfigMaps](https://kubernetes.io/docs/concepts/configuration/configmap/) and [Secrets](https://kubernetes.io/docs/concepts/configuration/secret/). **Note**: the usage of ConfigMaps and Secrets is limited by the usage of the parameter in the build strategy steps. You can only use them if the parameter is used in the command, arguments, or as environment variable values. + +When using _paramValues_, users should avoid: - Defining a `spec.paramValues` name that doesn't match one of the `spec.parameters` defined in the `BuildStrategy`. -- Defining a `spec.paramValues` name that collides with the Shipwright reserved parameters. These are _BUILDER_IMAGE_,_DOCKERFILE_,_CONTEXT_DIR_ and any name starting with _shp-_. +- Defining a `spec.paramValues` name that collides with the Shipwright reserved parameters. These are _BUILDER\_IMAGE_, _DOCKERFILE_, _CONTEXT\_DIR_ and any name starting with _shp-_. -In general, _params_ are tighly bound to Strategy _parameters_, please make sure you understand the contents of your strategy of choice, before defining _params_ in the _Build_. `BuildRun` resources allow users to override `Build` _params_, see the related [docs](./buildrun.md#defining-params) for more information. +In general, _paramValues_ are tightly bound to Strategy _parameters_, please make sure you understand the contents of your strategy of choice, before defining _paramValues_ in the _Build_. #### Example -The following `BuildStrategy` contains a single step ( _a-strategy-step_ ) with a command and arguments. The strategy defines a parameter( _sleep-time_ ) with a reasonable default, that is used in the step arguments, see _$(params.sleep-time)_. +The [BuildKit sample `BuildStrategy`](../samples/buildstrategy/buildkit/buildstrategy_buildkit_cr.yaml) contains various parameters. Two of them are outlined here: ```yaml ---- apiVersion: shipwright.io/v1alpha1 -kind: BuildStrategy +kind: ClusterBuildStrategy metadata: - name: sleepy-strategy + name: buildkit + ... spec: parameters: - - name: sleep-time - description: "time in seconds for sleeping" - default: "1" + - name: build-args + description: "The values for the ARGs in the Dockerfile. Values must be in the format KEY=VALUE." + type: array + defaults: [] + - name: cache + description: "Configure BuildKit's cache usage. Allowed values are 'disabled' and 'registry'. The default is 'registry'." + type: string + default: registry + ... buildSteps: - - name: a-strategy-step - image: alpine:latest - command: - - sleep - args: - - $(params.sleep-time) + ... ``` -If users would like the above strategy to change its behaviour, e.g. _allow the step to trigger a sleep cmd longer than 1 second_, then users can modify the default behaviour, via their `Build` `spec.paramValues` definition. For example: +The `cache` parameter is a simple string. You can provide it like this in your Build: ```yaml ---- apiVersion: shipwright.io/v1alpha1 kind: Build metadata: name: a-build + namespace: a-namespace spec: + paramValues: + - name: cache + value: disabled + strategy: + name: buildkit + kind: ClusterBuildStrategy source: - url: https://github.com/shipwright-io/sample-go - contextDir: docker-build/ + ... + output: + ... +``` + +If you have multiple Builds and want to centrally control this parameter, then you can create a ConfigMap: + +```yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: buildkit-configuration + namespace: a-namespace +data: + cache: disabled +``` + +You reference the ConfigMap as a parameter value like this: + +```yaml +apiVersion: shipwright.io/v1alpha1 +kind: Build +metadata: + name: a-build + namespace: a-namespace +spec: + paramValues: + - name: cache + configMapValue: + name: buildkit-configuration + key: cache + strategy: + name: buildkit + kind: ClusterBuildStrategy + source: + ... + output: + ... +``` + +The `build-args` parameter is defined as an array. In the BuildKit strategy, it is used to set the values of [`ARG`s in the Dockerfile](https://docs.docker.com/engine/reference/builder/#arg), specified as key-value pairs separated by an equals sign, for example `NODE_VERSION=16`. Your Build then looks like this (the value for `cache` is retained to outline how multiple _paramValue_ can be set): + +```yaml +apiVersion: shipwright.io/v1alpha1 +kind: Build +metadata: + name: a-build + namespace: a-namespace +spec: paramValues: - - name: sleep-time - value: "60" + - name: cache + configMapValue: + name: buildkit-configuration + key: cache + - name: build-args + values: + - value: NODE_VERSION=16 strategy: - name: sleepy-strategy - kind: BuildStrategy + name: buildkit + kind: ClusterBuildStrategy + source: + ... + output: + ... ``` -The above `Build` definition uses _sleep-time_ param, a well-defined _parameter_ under its referenced `BuildStrategy`. By doing this, the user signalizes to the referenced sleepy-strategy, the usage of a different value for its _sleep-time_ parameter. +Similar to simple values, you can also reference ConfigMaps and Secrets for every item in the array. Example: + +```yaml +apiVersion: shipwright.io/v1alpha1 +kind: Build +metadata: + name: a-build + namespace: a-namespace +spec: + paramValues: + - name: cache + configMapValue: + name: buildkit-configuration + key: cache + - name: build-args + values: + - configMapValue: + name: project-configuration + key: node-version + format: NODE_VERSION=${CONFIGMAP_VALUE} + - value: DEBUG_MODE=true + - secretValue: + name: npm-registry-access + key: npm-auth-token + format: NPM_AUTH_TOKEN=${SECRET_VALUE} + strategy: + name: buildkit + kind: ClusterBuildStrategy + source: + ... + output: + ... +``` + +Here, we pass three items in the `build-args` array: + +1. The first item references a ConfigMap. As the ConfigMap just contains the value (for example `"16"`) as the data of the `node-version` key, the `format` setting is used to prepend `NODE_VERSION=` to make it a complete key-value pair. +2. The second item is just a hard-coded value. +3. The third item references a Secret. This works in the same way as with ConfigMaps. + +**NOTE**: the logging output of BuildKit contains expanded `ARG`s in `RUN` commands. Also, such information ends up in the final container image if you use such args in the [final stage of your Dockerfile](https://docs.docker.com/develop/develop-images/multistage-build/). An alternative approach to pass secrets is using [secret mounts](https://docs.docker.com/develop/develop-images/build_enhancements/#new-docker-build-secret-information). The BuildKit sample strategy supports them using the `secrets` parameter. ### Defining the Builder or Dockerfile diff --git a/docs/buildrun.md b/docs/buildrun.md index 41be66ce9e..7eacf5bb24 100644 --- a/docs/buildrun.md +++ b/docs/buildrun.md @@ -13,7 +13,7 @@ SPDX-License-Identifier: Apache-2.0 - [Defining ParamValues](#defining-paramvalues) - [Defining the ServiceAccount](#defining-the-serviceaccount) - [Canceling a `BuildRun`](#canceling-a-buildrun) - - [Specifying Environment Variables](#specifying-environment-variables) +- [Specifying Environment Variables](#specifying-environment-variables) - [BuildRun Status](#buildrun-status) - [Understanding the state of a BuildRun](#understanding-the-state-of-a-buildrun) - [Understanding failed BuildRuns](#understanding-failed-buildruns) @@ -61,7 +61,7 @@ The `BuildRun` definition supports the following fields: - Optional: - `spec.serviceAccount` - Refers to the SA to use when building the image. (_defaults to the `default` SA_) - `spec.timeout` - Defines a custom timeout. The value needs to be parsable by [ParseDuration](https://golang.org/pkg/time/#ParseDuration), for example `5m`. The value overwrites the value that is defined in the `Build`. - - `spec.paramValues` - Override any _params_ defined in the referenced `Build`, as long as their name matches. + - `spec.paramValues` - Refers to a name-value(s) list to specify values for `parameters` defined in the `BuildStrategy`. This overwrites values defined with the same name in the Build. - `spec.output.image` - Refers to a custom location where the generated image would be pushed. The value will overwrite the `output.image` value which is defined in `Build`. ( Note: other properties of the output, for example, the credentials cannot be specified in the buildRun spec. ) - `spec.output.credentials.name` - Reference an existing secret to get access to the container registry. This secret will be added to the service account along with the ones requested by the `Build`. - `spec.env` - Specifies additional environment variables that should be passed to the build container. Overrides any environment variables that are specified in the `Build` resource. The available variables depend on the tool that is being used by the chosen build strategy. @@ -82,41 +82,44 @@ spec: ### Defining ParamValues -A `BuildRun` resource can override _paramValues_ defined in its referenced `Build`, as long as the `Build` defines the same _params_ name. +A `BuildRun` resource can define _paramValues_ for parameters specified in the build strategy. If a value has been provided for a parameter with the same name in the `Build` already, then the value from the `BuildRun` will have precedence. For example, the following `BuildRun` overrides the value for _sleep-time_ param, that is defined in the _a-build_ `Build` resource. ```yaml --- apiVersion: shipwright.io/v1alpha1 -kind: BuildRun +kind: Build metadata: - name: a-buildrun + name: a-build + namespace: a-namespace spec: - buildRef: - name: a-build paramValues: - - name: sleep-time - value: "30" + - name: cache + value: disabled + strategy: + name: buildkit + kind: ClusterBuildStrategy + source: + ... + output: + ... --- apiVersion: shipwright.io/v1alpha1 -kind: Build +kind: BuildRun metadata: - name: a-build + name: a-buildrun + namespace: a-namespace spec: - source: - url: https://github.com/shipwright-io/sample-go - contextDir: docker-build/ + buildRef: + name: a-build paramValues: - - name: sleep-time - value: "60" - strategy: - name: sleepy-strategy - kind: BuildStrategy + - name: cache + value: registry ``` -See more about `paramValues` usage in the related [Build](./build.md#defining-paramvalues) resource docs. +See more about _paramValues_ usage in the related [Build](./build.md#defining-paramvalues) resource docs. ### Defining the ServiceAccount @@ -156,7 +159,7 @@ spec: state: "BuildRunCanceled" ``` -### Specifying Environment Variables +## Specifying Environment Variables An example of a `BuildRun` that specifies environment variables: @@ -246,25 +249,33 @@ The following table illustrates the different states a BuildRun can have under i | Status | Reason | CompletionTime is set | Description | | --- | --- | --- | --- | -| Unknown | Pending | No | The BuildRun is waiting on a Pod in status Pending. | -| Unknown | Running | No | The BuildRun has been validate and started to perform its work. |l -| Unknown | Running | No | The BuildRun has been validate and started to perform its work. | -| Unknown | BuildRunCanceled | No | The user requested the BuildRun to be canceled. This results in the BuildRun controller requesting the TaskRun be canceled. Cancellation has not been done yet. | -| True | Succeeded | Yes | The BuildRun Pod is done. | -| False | Failed | Yes | The BuildRun failed in one of the steps. | -| False | BuildRunTimeout | Yes | The BuildRun timed out. | -| False | UnknownStrategyKind | Yes | The Build specified strategy Kind is unknown. (_options: ClusterBuildStrategy or BuildStrategy_) | -| False | ClusterBuildStrategyNotFound | Yes | The referenced cluster strategy was not found in the cluster. | -| False | BuildStrategyNotFound | Yes | The referenced namespaced strategy was not found in the cluster. | -| False | SetOwnerReferenceFailed | Yes | Setting ownerreferences from the BuildRun to the related TaskRun failed. | -| False | TaskRunIsMissing | Yes | The BuildRun related TaskRun was not found. | -| False | TaskRunGenerationFailed | Yes | The generation of a TaskRun spec failed. | -| False | ServiceAccountNotFound | Yes | The referenced service account was not found in the cluster. | -| False | BuildRegistrationFailed | Yes | The related Build in the BuildRun is on a Failed state. | -| False | BuildNotFound | Yes | The related Build in the BuildRun was not found. | -| False | BuildRunCanceled | Yes | The BuildRun and underlying TaskRun were canceled successfully. | -| False | BuildRunNameInvalid | Yes | The defined `BuildRun` name (`metadata.name`) is invalid. The `BuildRun` name should be a [valid label value](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set). | -| False | PodEvicted | Yes | The BuildRun Pod was evicted from the node it was running on. See [API-initiated Eviction](https://kubernetes.io/docs/concepts/scheduling-eviction/api-eviction/) and [Node-pressure Eviction](https://kubernetes.io/docs/concepts/scheduling-eviction/node-pressure-eviction/) for more information. | +| Unknown | Pending | No | The BuildRun is waiting on a Pod in status Pending. | +| Unknown | Running | No | The BuildRun has been validate and started to perform its work. |l +| Unknown | Running | No | The BuildRun has been validate and started to perform its work. | +| Unknown | BuildRunCanceled | No | The user requested the BuildRun to be canceled. This results in the BuildRun controller requesting the TaskRun be canceled. Cancellation has not been done yet. | +| True | Succeeded | Yes | The BuildRun Pod is done. | +| False | Failed | Yes | The BuildRun failed in one of the steps. | +| False | BuildRunTimeout | Yes | The BuildRun timed out. | +| False | UnknownStrategyKind | Yes | The Build specified strategy Kind is unknown. (_options: ClusterBuildStrategy or BuildStrategy_) | +| False | ClusterBuildStrategyNotFound | Yes | The referenced cluster strategy was not found in the cluster. | +| False | BuildStrategyNotFound | Yes | The referenced namespaced strategy was not found in the cluster. | +| False | SetOwnerReferenceFailed | Yes | Setting ownerreferences from the BuildRun to the related TaskRun failed. | +| False | TaskRunIsMissing | Yes | The BuildRun related TaskRun was not found. | +| False | TaskRunGenerationFailed | Yes | The generation of a TaskRun spec failed. | +| False | MissingParameterValues | Yes | No value has been provided for some parameters that are defined in the build strategy without any default. Values for those parameters must be provided through the Build or the BuildRun. | +| False | RestrictedParametersInUse | Yes | A value for a system parameter was provided. This is not allowed. | +| False | UndefinedParameter | Yes | A value for a parameter was provided that is not defined in the build strategy. | +| False | WrongParameterValueType | Yes | A value was provided for a build strategy parameter using the wrong type. The parameter is defined as `array` or `string` in the build strategy. Depending on that you must provide `values` or a direct value. | +| False | InconsistentParameterValues | Yes | A value for a parameter contained more than one of `value`, `configMapValue`, and `secretValue`. Any values including array items must only provide one of them. | +| False | EmptyArrayItemParameterValues | Yes | An item inside the `values` of an array parameter contained none of `value`, `configMapValue`, and `secretValue`. Exactly one of them must be provided. Null array items are not allowed. | +| False | IncompleteConfigMapValueParameterValues | Yes | A value for a parameter contained a `configMapValue` where the `name` or the `value` were empty. You must specify them to point to an existing ConfigMap key in your namespace. | +| False | IncompleteSecretValueParameterValues | Yes | A value for a parameter contained a `secretValue` where the `name` or the `value` were empty. You must specify them to point to an existing Secret key in your namespace. | +| False | ServiceAccountNotFound | Yes | The referenced service account was not found in the cluster. | +| False | BuildRegistrationFailed | Yes | The related Build in the BuildRun is on a Failed state. | +| False | BuildNotFound | Yes | The related Build in the BuildRun was not found. | +| False | BuildRunCanceled | Yes | The BuildRun and underlying TaskRun were canceled successfully. | +| False | BuildRunNameInvalid | Yes | The defined `BuildRun` name (`metadata.name`) is invalid. The `BuildRun` name should be a [valid label value](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set). | +| False | PodEvicted | Yes | The BuildRun Pod was evicted from the node it was running on. See [API-initiated Eviction](https://kubernetes.io/docs/concepts/scheduling-eviction/api-eviction/) and [Node-pressure Eviction](https://kubernetes.io/docs/concepts/scheduling-eviction/node-pressure-eviction/) for more information. | _Note_: We heavily rely on the Tekton TaskRun [Conditions](https://github.com/tektoncd/pipeline/blob/main/docs/taskruns.md#monitoring-execution-status) for populating the BuildRun ones, with some exceptions. @@ -274,13 +285,13 @@ _Note_: We heavily rely on the Tekton TaskRun [Conditions](https://github.com/te In addition, the `Status.Conditions` will host under the `Message` field a compacted message containing the `kubectl` command to trigger, in order to retrieve the logs. -Lastly, users can check `Status.FailureDetails` field, which includes the same information available in the `Status.FailedAt` field, +Lastly, users can check `Status.FailureDetails` field, which includes the same information available in the `Status.FailedAt` field, as well as a humanly-readable error message and reason. The message and reason are only included if the build strategy provides them. Example of failed BuildRun: -```yaml +```yaml # [...] status: # [...] @@ -313,7 +324,7 @@ error reasons: ### Step Results in BuildRun Status After the successful completion of a `BuildRun`, the `.status` field contains the results (`.status.taskResults`) emitted from the `TaskRun` steps generate by the `BuildRun` controller as part of processing the `BuildRun`. These results contain valuable metadata for users, like the _image digest_ or the _commit sha_ of the source code used for building. -The results from the source step will be surfaced to the `.status.sources` and the results from +The results from the source step will be surfaced to the `.status.sources` and the results from the [output step](buildstrategies.md#system-results) will be surfaced to the `.status.output` field of a `BuildRun`. Example of a `BuildRun` with surfaced results for `git` source (note that the `branchName` is only included if the Build does not specify any `revision`): diff --git a/docs/buildstrategies.md b/docs/buildstrategies.md index 3e86120ecc..d89736fbfb 100644 --- a/docs/buildstrategies.md +++ b/docs/buildstrategies.md @@ -140,13 +140,16 @@ kubectl apply -f samples/buildstrategy/kaniko/buildstrategy_kaniko-trivy_cr.yaml ### Cache Exporters -By default, the `buildkit` ClusterBuildStrategy will use caching to optimize the build times. When pushing an image to a registry, it will use the `inline` export cache, which pushes the image and cache together. Please refer to [export-cache docs](https://github.com/moby/buildkit#export-cache) for more information. +By default, the `buildkit` ClusterBuildStrategy will use caching to optimize the build times. When pushing an image to a registry, it will use the inline export cache, which appends cache information to the image that is built. Please refer to [export-cache docs](https://github.com/moby/buildkit#export-cache) for more information. Caching can be disabled by setting the `cache` parameter to disabled. See [Defining ParamValues](build.md#defining-paramvalues) for more information. + +### Build-args and secrets + +The sample build strategy contains array parameters to set values for [`ARG`s in your Dockerfile](https://docs.docker.com/engine/reference/builder/#arg), and for [mounts with type=secret](https://docs.docker.com/develop/develop-images/build_enhancements/#new-docker-build-secret-information). The parameter names are `build-args` and `secrets`. [Defining ParamValues](build.md#defining-paramvalues) contains example usage. ### Known Limitations The `buildkit` ClusterBuildStrategy currently locks the following parameters: -- A `Dockerfile` name needs to be `Dockerfile`, this is currently not configurable. - Exporter caches are enabled by default, this is currently not configurable. - To allow running rootless, it requires both [AppArmor](https://kubernetes.io/docs/tutorials/clusters/apparmor/) as well as [SecComp](https://kubernetes.io/docs/tutorials/clusters/seccomp/) to be disabled using the `unconfined` profile. @@ -230,33 +233,161 @@ Strategy parameters allow users to parameterize their strategy definition, by al Users defining _parameters_ under their strategies require to understand the following: -- **Definition**: A list of parameters should be defined under `spec.parameters`. Each list item should consist of a _name_, a _description_ and a reasonable _default_ value (_type string_). Note that a default value is not mandatory. -- **Usage**: In order to use a parameter in the strategy steps, users should follow the following syntax: `$(params.your-parameter-name)` +- **Definition**: A list of parameters should be defined under `spec.parameters`. Each list item should consist of a _name_, a _description_, a _type_ (either `"array"` or `"string"`) and optionally a _default_ value (for type=string), or _defaults_ values (for type=array). If no default(s) are provided, then the user must define a value in the Build or BuildRun. +- **Usage**: In order to use a parameter in the strategy steps, use the following syntax for type=string: `$(params.your-parameter-name)`. String parameters can be used in all places in the `buildSteps`. Some example scenarios are: + - `image`: to use a custom tag, for example `golang:$(params.go-version)` as it is done in the [ko sample build strategy](../samples/buildstrategy/ko/buildstrategy_ko_cr.yaml)) + - `args`: to pass data into your builder command + - `env`: to force a user to provide a value for an environment variable. + + Arrays are referenced using `$(params.your-array-parameter-name[*])`, and can only be used in as the value for `args` or `command` because the defined as arrays by Kubernetes. For every item in the array, an arg will be set. For example, if you specify this in your build strategy step: + + ```yaml + spec: + parameteres: + - name: tool-args + description: Parameters for the tool + type: array + buildSteps: + - name: a-step + command: + - some-tool + args: + - $(params.tool-args[*]) + ``` + + If the build user sets the value of tool-args to ["--some-arg", "some-value"], then the Pod will contain these args: + + ```yaml + spec: + containers: + - name: a-step + args: + ... + - --some-arg + - some-value + ``` + - **Parameterize**: Any `Build` or `BuildRun` referencing your strategy, can set a value for _your-parameter-name_ parameter if needed. -The following is an example of a strategy that defines and uses the `sleep-time` parameter: +**Note**: Users can provide parameter values as simple strings or as references to keys in [ConfigMaps](https://kubernetes.io/docs/concepts/configuration/configmap/) and [Secrets](https://kubernetes.io/docs/concepts/configuration/secret/). If they use a ConfigMap or Secret, then the value can only be used if the parameter is used in the `command`, `args`, or `env` section of the `buildSteps`. For example, the above mentioned scenario to set a step's `image` to `golang:$(params.go-version)` does not allow the usage of ConfigMaps or Secrets. + +The following example is from the [BuildKit sample build strategy](../samples/buildstrategy/buildkit/buildstrategy_buildkit_cr.yaml). It defines and uses several parameters: ```yaml --- apiVersion: shipwright.io/v1alpha1 -kind: BuildStrategy +kind: ClusterBuildStrategy metadata: - name: sleepy-strategy + name: buildkit + ... spec: parameters: - - name: sleep-time - description: "time in seconds for sleeping" - default: "1" + - name: build-args + description: "The values for the ARGs in the Dockerfile. Values must be in the format KEY=VALUE." + type: array + defaults: [] + - name: cache + description: "Configure BuildKit's cache usage. Allowed values are 'disabled' and 'registry'. The default is 'registry'." + type: string + default: registry + - name: insecure-registry + type: string + description: "enables the push to an insecure registry" + default: "false" + - name: secrets + description: "The secrets to pass to the build. Values must be in the format ID=FILE_CONTENT." + type: array + defaults: [] buildSteps: - - name: a-strategy-step - image: alpine:latest - command: - - sleep - args: - - $(params.sleep-time) + ... + - name: build-and-push + image: moby/buildkit:nightly-rootless + imagePullPolicy: Always + workingDir: $(params.shp-source-root) + ... + command: + - /bin/ash + args: + - -c + - | + set -euo pipefail + + # Prepare the file arguments + DOCKERFILE_PATH='$(params.shp-source-context)/$(build.dockerfile)' + DOCKERFILE_DIR="$(dirname "${DOCKERFILE_PATH}")" + DOCKERFILE_NAME="$(basename "${DOCKERFILE_PATH}")" + + # We only have ash here and therefore no bash arrays to help add dynamic arguments (the build-args) to the build command. + + echo "#!/bin/ash" > /tmp/run.sh + echo "set -euo pipefail" >> /tmp/run.sh + echo "buildctl-daemonless.sh \\" >> /tmp/run.sh + echo "build \\" >> /tmp/run.sh + echo "--progress=plain \\" >> /tmp/run.sh + echo "--frontend=dockerfile.v0 \\" >> /tmp/run.sh + echo "--opt=filename=\"${DOCKERFILE_NAME}\" \\" >> /tmp/run.sh + echo "--local=context='$(params.shp-source-context)' \\" >> /tmp/run.sh + echo "--local=dockerfile=\"${DOCKERFILE_DIR}\" \\" >> /tmp/run.sh + echo "--output=type=image,name='$(params.shp-output-image)',push=true,registry.insecure=$(params.insecure-registry) \\" >> /tmp/run.sh + if [ "$(params.cache)" == "registry" ]; then + echo "--export-cache=type=inline \\" >> /tmp/run.sh + echo "--import-cache=type=registry,ref='$(params.shp-output-image)' \\" >> /tmp/run.sh + elif [ "$(params.cache)" == "disabled" ]; then + echo "--no-cache \\" >> /tmp/run.sh + else + echo -e "An invalid value for the parameter 'cache' has been provided: '$(params.cache)'. Allowed values are 'disabled' and 'registry'." + echo -n "InvalidParameterValue" > '$(results.shp-error-reason.path)' + echo -n "An invalid value for the parameter 'cache' has been provided: '$(params.cache)'. Allowed values are 'disabled' and 'registry'." > '$(results.shp-error-message.path)' + exit 1 + fi + + stage="" + for a in "$@" + do + if [ "${a}" == "--build-args" ]; then + stage=build-args + elif [ "${a}" == "--secrets" ]; then + stage=secrets + elif [ "${stage}" == "build-args" ]; then + echo "--opt=\"build-arg:${a}\" \\" >> /tmp/run.sh + elif [ "${stage}" == "secrets" ]; then + # Split ID=FILE_CONTENT into variables id and data + + # using head because the data could be multiline + id="$(echo "${a}" | head -1 | sed 's/=.*//')" + + # This is hacky, we remove the suffix ${id}= from all lines of the data. + # If the data would be multiple lines and a line would start with ${id}= + # then we would remove it. We could force users to give us the secret + # base64 encoded. But ultimately, the best solution might be if the user + # mounts the secret and just gives us the path here. + data="$(echo "${a}" | sed "s/^${id}=//")" + + # Write the secret data into a temporary file, once we have volume support + # in the build strategy, we should use a memory based emptyDir for this. + echo -n "${data}" > "/tmp/secret_${id}" + + # Add the secret argument + echo "--secret id=${id},src="/tmp/secret_${id}" \\" >> /tmp/run.sh + fi + done + + echo "--metadata-file /tmp/image-metadata.json" >> /tmp/run.sh + + chmod +x /tmp/run.sh + /tmp/run.sh + + # Store the image digest + sed -E 's/.*containerimage.digest":"([^"]*).*/\1/' < /tmp/image-metadata.json > '$(results.shp-image-digest.path)' + # That's the separator between the shell script and its args + - -- + - --build-args + - $(params.build-args[*]) + - --secrets + - $(params.secrets[*]) ``` -See more information on how to use this parameter in a `Build` or `BuildRun` in the related [docs](./build.md#defining-paramvalues). +See more information on how to use these parameters in a `Build` or `BuildRun` in the related [documentation](./build.md#defining-paramvalues). ## System parameters diff --git a/docs/proposals/parameterize-strategies.md b/docs/proposals/parameterize-strategies.md index 57fb4258e0..e29da3c87f 100644 --- a/docs/proposals/parameterize-strategies.md +++ b/docs/proposals/parameterize-strategies.md @@ -315,7 +315,7 @@ spec: output: #Content omitted for this example paramValues: - name: INSECURE_REGISTRY - value: false + value: "false" ``` ```yaml @@ -328,7 +328,7 @@ spec: name: a-build paramValues: - name: INSECURE_REGISTRY - value: true + value: "true" ``` The above allows a user to opt-in for pushing to an insecure registry, although the referenced Build disables this behaviour. diff --git a/docs/tutorials/building_with_buildkit.md b/docs/tutorials/building_with_buildkit.md index 8cfa8448f0..69a5d5d491 100644 --- a/docs/tutorials/building_with_buildkit.md +++ b/docs/tutorials/building_with_buildkit.md @@ -61,8 +61,7 @@ metadata: spec: source: url: https://github.com/shipwright-io/sample-go - contextDir: docker-build/ - dockerfile: docker-build/ + contextDir: docker-build strategy: name: buildkit kind: ClusterBuildStrategy @@ -73,8 +72,6 @@ spec: EOF ``` -_Note_: Pay attention to the definition under `spec.source.dockerfile`. The `buildkit` expects a path to where your `Dockerfile` is located. - Verify that the `go-tutorial` Build was created successfully: ```sh