Support typed parameter values (#1376)

This introduces the 'string', 'array', and 'masp' types for parameter values. By default, the 'string' type is used. This type is used for a simple value. The types 'array' and 'map' can be used to describe lists and dictionaries. Their values are input using YAML.

Co-Authored-By: Ken Sipe <kensipe@gmail.com>
Signed-off-by: Jan Schlicht <jan@d2iq.com>
Signed-off-by: Thomas Runyon <runyontr@gmail.com>

config/crds/kudo.dev_operatorversions.yaml

@@ -157,6 +157,9 @@ spec:
                       this parameter changes in the Instance object. Default is `update`
                       if a plan with that name exists, otherwise it's `deploy`.
                     type: string
+                  value-type:
+                    description: Type specifies the value type. Defaults to `string`.
+                    type: string
                 type: object
               type: array
             plans:

pkg/apis/kudo/v1beta1/instance_types_helpers.go

@@ -5,8 +5,6 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/uuid"
-
-	"github.com/kudobuilder/kudo/pkg/util/kudo"
 )
 
 // GetPlanInProgress returns plan status of currently active plan or nil if no plan is running
@@ -162,9 +160,9 @@ func ParameterDiff(old, new map[string]string) map[string]string {
 
 // SelectPlan returns nil if none of the plan exists, otherwise the first one in list that exists
 func SelectPlan(possiblePlans []string, ov *OperatorVersion) *string {
-	for _, n := range possiblePlans {
-		if _, ok := ov.Spec.Plans[n]; ok {
-			return kudo.String(n)
+	for _, plan := range possiblePlans {
+		if _, ok := ov.Spec.Plans[plan]; ok {
+			return &plan
 		}
 	}
 	return nil

pkg/apis/kudo/v1beta1/operatorversion_types.go

@@ -71,6 +71,20 @@ type Plan struct {
 	Phases []Phase `json:"phases"`
 }
 
+// ParameterType specifies the type of a parameter value.
+type ParameterType string
+
+const (
+	// StringValueType is used for parameter values that are provided as a string.
+	StringValueType ParameterType = "string"
+
+	// ArrayValueType is used for parameter values that described an array of values.
+	ArrayValueType ParameterType = "array"
+
+	// MapValueType is used for parameter values that describe a mapping type.
+	MapValueType ParameterType = "map"
+)
+
 // Parameter captures the variability of an OperatorVersion being instantiated in an instance.
 type Parameter struct {
 	// DisplayName can be used by UIs.
@@ -95,6 +109,9 @@ type Parameter struct {
 	// Trigger identifies the plan that gets executed when this parameter changes in the Instance object.
 	// Default is `update` if a plan with that name exists, otherwise it's `deploy`.
 	Trigger string `json:"trigger,omitempty"`
+
+	// Type specifies the value type. Defaults to `string`.
+	Type ParameterType `json:"value-type,omitempty"`
 }
 
 // Phase specifies a list of steps that contain Kubernetes objects.

pkg/controller/instance/instance_controller.go

@@ -51,7 +51,7 @@ import (
 	"github.com/kudobuilder/kudo/pkg/engine/renderer"
 	"github.com/kudobuilder/kudo/pkg/engine/task"
 	"github.com/kudobuilder/kudo/pkg/engine/workflow"
-	"github.com/kudobuilder/kudo/pkg/util/kudo"
+	"github.com/kudobuilder/kudo/pkg/util/convert"
 )
 
 const (
@@ -207,12 +207,12 @@ func (r *Reconciler) Reconcile(request ctrl.Request) (ctrl.Result, error) {
 		return reconcile.Result{}, err
 	}
 	if planToBeExecuted != nil {
-		log.Printf("InstanceController: Going to start execution of plan %s on instance %s/%s", kudo.StringValue(planToBeExecuted), instance.Namespace, instance.Name)
-		err = startPlanExecution(instance, kudo.StringValue(planToBeExecuted), ov)
+		log.Printf("InstanceController: Going to start execution of plan %s on instance %s/%s", convert.StringValue(planToBeExecuted), instance.Namespace, instance.Name)
+		err = startPlanExecution(instance, convert.StringValue(planToBeExecuted), ov)
 		if err != nil {
 			return reconcile.Result{}, r.handleError(err, instance, oldInstance)
 		}
-		r.Recorder.Event(instance, "Normal", "PlanStarted", fmt.Sprintf("Execution of plan %s started", kudo.StringValue(planToBeExecuted)))
+		r.Recorder.Event(instance, "Normal", "PlanStarted", fmt.Sprintf("Execution of plan %s started", convert.StringValue(planToBeExecuted)))
 	}
 
 	// ---------- 4. If there's currently active plan, continue with the execution ----------
@@ -312,7 +312,11 @@ func preparePlanExecution(instance *kudov1beta1.Instance, ov *kudov1beta1.Operat
 		return nil, &engine.ExecutionError{Err: fmt.Errorf("%wcould not find required plan: %v", engine.ErrFatalExecution, activePlanStatus.Name), EventName: "InvalidPlan"}
 	}
 
-	params := paramsMap(instance, ov)
+	params, err := paramsMap(instance, ov)
+	if err != nil {
+		return nil, &engine.ExecutionError{Err: fmt.Errorf("%wcould not parse parameters: %v", engine.ErrFatalExecution, err), EventName: "InvalidParams"}
+	}
+
 	pipes, err := pipesMap(activePlanStatus.Name, &planSpec, ov.Spec.Tasks, meta)
 	if err != nil {
 		return nil, &engine.ExecutionError{Err: fmt.Errorf("%wcould not make task pipes: %v", engine.ErrFatalExecution, err), EventName: "InvalidPlan"}
@@ -356,7 +360,7 @@ func (r *Reconciler) handleError(err error, instance *kudov1beta1.Instance, oldI
 	var iError *kudov1beta1.InstanceError
 	if errors.As(err, &iError) {
 		if iError.EventName != nil {
-			r.Recorder.Event(instance, "Warning", kudo.StringValue(iError.EventName), err.Error())
+			r.Recorder.Event(instance, "Warning", convert.StringValue(iError.EventName), err.Error())
 		}
 	}
 	return err
@@ -392,21 +396,27 @@ func GetOperatorVersion(instance *kudov1beta1.Instance, c client.Client) (ov *ku
 }
 
 // paramsMap generates {{ Params.* }} map of keys and values which is later used during template rendering.
-func paramsMap(instance *kudov1beta1.Instance, operatorVersion *kudov1beta1.OperatorVersion) map[string]string {
-	params := make(map[string]string)
+func paramsMap(instance *kudov1beta1.Instance, operatorVersion *kudov1beta1.OperatorVersion) (map[string]interface{}, error) {
+	params := make(map[string]interface{}, len(operatorVersion.Spec.Parameters))
 
-	for k, v := range instance.Spec.Parameters {
-		params[k] = v
-	}
-
-	// Merge instance parameter overrides with operator version, if no override exist, use the default one
 	for _, param := range operatorVersion.Spec.Parameters {
-		if _, ok := params[param.Name]; !ok {
-			params[param.Name] = kudo.StringValue(param.Default)
+		var value *string
+
+		if v, ok := instance.Spec.Parameters[param.Name]; ok {
+			value = &v
+		} else {
+			value = param.Default
+		}
+
+		var err error
+
+		params[param.Name], err = convert.UnwrapParamValue(value, param.Type)
+		if err != nil {
+			return nil, err
 		}
 	}
 
-	return params
+	return params, nil
 }
 
 // pipesMap generates {{ Pipes.* }} map of keys and values which is later used during template rendering.
@@ -481,7 +491,7 @@ func startPlanExecution(i *v1beta1.Instance, planName string, ov *v1beta1.Operat
 		}
 	}
 	if notFound {
-		return &v1beta1.InstanceError{Err: fmt.Errorf("asked to execute a plan %s but no such plan found in instance %s/%s", planName, i.Namespace, i.Name), EventName: kudo.String("PlanNotFound")}
+		return &v1beta1.InstanceError{Err: fmt.Errorf("asked to execute a plan %s but no such plan found in instance %s/%s", planName, i.Namespace, i.Name), EventName: convert.StringPtr("PlanNotFound")}
 	}
 
 	err := saveSnapshot(i)
@@ -576,7 +586,7 @@ func getPlanToBeExecuted(i *v1beta1.Instance, ov *v1beta1.OperatorVersion) (*str
 
 	// new instance, need to run deploy plan
 	if i.NoPlanEverExecuted() {
-		return kudo.String(v1beta1.DeployPlanName), nil
+		return convert.StringPtr(v1beta1.DeployPlanName), nil
 	}
 
 	// did the instance change so that we need to run deploy/upgrade/update plan?
@@ -586,15 +596,15 @@ func getPlanToBeExecuted(i *v1beta1.Instance, ov *v1beta1.OperatorVersion) (*str
 	}
 	if instanceSnapshot == nil {
 		// we don't have snapshot -> we never run deploy, also we cannot run update/upgrade. This should never happen
-		return nil, &v1beta1.InstanceError{Err: fmt.Errorf("unexpected state: no plan is running, no snapshot present - this should never happen :) for instance %s/%s", i.Namespace, i.Name), EventName: kudo.String("UnexpectedState")}
+		return nil, &v1beta1.InstanceError{Err: fmt.Errorf("unexpected state: no plan is running, no snapshot present - this should never happen :) for instance %s/%s", i.Namespace, i.Name), EventName: convert.StringPtr("UnexpectedState")}
 	}
 	if instanceSnapshot.OperatorVersion.Name != i.Spec.OperatorVersion.Name {
 		// this instance was upgraded to newer version
 		log.Printf("Instance: instance %s/%s was upgraded from %s to %s operatorVersion", i.Namespace, i.Name, instanceSnapshot.OperatorVersion.Name, i.Spec.OperatorVersion.Name)
 		plan := kudov1beta1.SelectPlan([]string{v1beta1.UpgradePlanName, v1beta1.UpdatePlanName, v1beta1.DeployPlanName}, ov)
 		if plan == nil {
 			return nil, &v1beta1.InstanceError{Err: fmt.Errorf("supposed to execute plan because instance %s/%s was upgraded but none of the deploy, upgrade, update plans found in linked operatorVersion", i.Namespace, i.Name),
-				EventName: kudo.String("PlanNotFound")}
+				EventName: convert.StringPtr("PlanNotFound")}
 		}
 		return plan, nil
 	}
@@ -606,7 +616,7 @@ func getPlanToBeExecuted(i *v1beta1.Instance, ov *v1beta1.OperatorVersion) (*str
 		paramDefinitions := kudov1beta1.GetParamDefinitions(paramDiff, ov)
 		plan, err := planNameFromParameters(paramDefinitions, ov)
 		if err != nil {
-			return nil, &v1beta1.InstanceError{Err: fmt.Errorf("supposed to execute plan because instance %s/%s was updated but no valid plan found: %v", i.Namespace, i.Name, err), EventName: kudo.String("PlanNotFound")}
+			return nil, &v1beta1.InstanceError{Err: fmt.Errorf("supposed to execute plan because instance %s/%s was updated but no valid plan found: %v", i.Namespace, i.Name, err), EventName: convert.StringPtr("PlanNotFound")}
 		}
 		return plan, nil
 	}
@@ -619,7 +629,7 @@ func planNameFromParameters(params []v1beta1.Parameter, ov *v1beta1.OperatorVers
 	for _, p := range params {
 		if p.Trigger != "" {
 			if kudov1beta1.SelectPlan([]string{p.Trigger}, ov) != nil {
-				return kudo.String(p.Trigger), nil
+				return convert.StringPtr(p.Trigger), nil
 			}
 			return nil, fmt.Errorf("param %s defined trigger plan %s, but plan not defined in operatorversion", p.Name, p.Trigger)
 		}

pkg/controller/instance/instance_controller_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1"
 	"github.com/kudobuilder/kudo/pkg/engine"
 	"github.com/kudobuilder/kudo/pkg/test/utils"
+	"github.com/kudobuilder/kudo/pkg/util/convert"
 	"github.com/kudobuilder/kudo/pkg/util/kudo"
 )
 
@@ -82,7 +83,7 @@ func TestRestartController(t *testing.T) {
 			Parameters: []v1beta1.Parameter{
 				{
 					Name:    "param",
-					Default: kudo.String("default"),
+					Default: convert.StringPtr("default"),
 				},
 			},
 		},

pkg/engine/task/task.go

@@ -21,9 +21,9 @@ type Context struct {
 	Config     *rest.Config
 	Enhancer   renderer.Enhancer
 	Meta       renderer.Metadata
-	Templates  map[string]string // Raw templates
-	Parameters map[string]string // Instance and OperatorVersion parameters merged
-	Pipes      map[string]string // Pipe artifacts
+	Templates  map[string]string      // Raw templates
+	Parameters map[string]interface{} // Instance and OperatorVersion parameters merged
+	Pipes      map[string]string      // Pipe artifacts
 }
 
 // Tasker is an interface that represents any runnable task for an operator. This method is treated

pkg/engine/task/task_toggle.go

@@ -43,7 +43,13 @@ func (tt ToggleTask) delegateTask(ctx Context) (Tasker, error) {
 	if !exists {
 		return task, fmt.Errorf("no value for parameter %s found", tt.Parameter)
 	}
-	enabled, err := strconv.ParseBool(val)
+
+	stringVal, ok := val.(string)
+	if !ok {
+		return task, fmt.Errorf("value of parameter %s isn't a string", tt.Parameter)
+	}
+
+	enabled, err := strconv.ParseBool(stringVal)
 	if err != nil {
 		return task, fmt.Errorf("could not parse value of parameter %s: %v", tt.Parameter, err)
 	}

pkg/engine/task/task_toggle_test.go

@@ -66,7 +66,7 @@ func TestToggleTask_Run(t *testing.T) {
 			done:    true,
 			wantErr: false,
 			ctx: Context{
-				Parameters: map[string]string{
+				Parameters: map[string]interface{}{
 					"feature-enabled": "true",
 				},
 				Client:    fake.NewFakeClientWithScheme(scheme.Scheme),
@@ -86,7 +86,7 @@ func TestToggleTask_Run(t *testing.T) {
 			done:    false,
 			wantErr: false,
 			ctx: Context{
-				Parameters: map[string]string{
+				Parameters: map[string]interface{}{
 					"feature-enabled": "true",
 				},
 				Client:    fake.NewFakeClientWithScheme(scheme.Scheme),
@@ -142,7 +142,7 @@ func TestToggleTask_intermediateTask(t *testing.T) {
 			},
 			wantErr: false,
 			ctx: Context{
-				Parameters: map[string]string{
+				Parameters: map[string]interface{}{
 					"feature-enabled": "true",
 				},
 				Client:    fake.NewFakeClientWithScheme(scheme.Scheme),
@@ -162,7 +162,7 @@ func TestToggleTask_intermediateTask(t *testing.T) {
 			},
 			wantErr: false,
 			ctx: Context{
-				Parameters: map[string]string{
+				Parameters: map[string]interface{}{
 					"feature-enabled": "false",
 				},
 				Client:    fake.NewFakeClientWithScheme(scheme.Scheme),
@@ -182,7 +182,7 @@ func TestToggleTask_intermediateTask(t *testing.T) {
 			},
 			wantErr: true,
 			ctx: Context{
-				Parameters: map[string]string{
+				Parameters: map[string]interface{}{
 					"feature-enabled": "notABooleanValue",
 				},
 				Client:    fake.NewFakeClientWithScheme(scheme.Scheme),
@@ -202,7 +202,7 @@ func TestToggleTask_intermediateTask(t *testing.T) {
 			},
 			wantErr: true,
 			ctx: Context{
-				Parameters: map[string]string{
+				Parameters: map[string]interface{}{
 					"feature-enabled": "",
 				},
 				Client:    fake.NewFakeClientWithScheme(scheme.Scheme),
@@ -221,7 +221,7 @@ func TestToggleTask_intermediateTask(t *testing.T) {
 			},
 			wantErr: true,
 			ctx: Context{
-				Parameters: map[string]string{
+				Parameters: map[string]interface{}{
 					"feature-enabled": "someValue",
 				},
 				Client:    fake.NewFakeClientWithScheme(scheme.Scheme),

pkg/engine/workflow/engine.go

@@ -30,7 +30,7 @@ type ActivePlan struct {
 	Spec      *v1beta1.Plan
 	Tasks     []v1beta1.Task
 	Templates map[string]string
-	Params    map[string]string
+	Params    map[string]interface{}
 	Pipes     map[string]string
 }
 

pkg/kudoctl/cmd/generate/operator.go

@@ -76,7 +76,7 @@ func Operator(fs afero.Fs, dir string, op *packages.OperatorFile, overwrite bool
 	// if params doesn't exist create it
 	p := packages.ParamsFile{
 		APIVersion: reader.APIVersion,
-		Parameters: []v1beta1.Parameter{},
+		Parameters: []packages.Parameter{},
 	}
 	return writeParameters(fs, dir, p)
 }

pkg/kudoctl/cmd/generate/operator_test.go

@@ -7,7 +7,6 @@ import (
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 
-	"github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1"
 	"github.com/kudobuilder/kudo/pkg/kudoctl/packages"
 	"github.com/kudobuilder/kudo/pkg/kudoctl/packages/reader"
 )
@@ -70,7 +69,7 @@ func TestOperator_Write(t *testing.T) {
 	// updating params file and testing params are not overwritten
 	pf := packages.ParamsFile{
 		APIVersion: "FOO",
-		Parameters: []v1beta1.Parameter{},
+		Parameters: []packages.Parameter{},
 	}
 	// replace param file with a marker "FOO" to test that we do NOT overwrite it
 	err = writeParameters(fs, "operator", pf)

pkg/kudoctl/cmd/generate/parameter.go

@@ -3,12 +3,12 @@ package generate
 import (
 	"github.com/spf13/afero"
 
-	"github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1"
+	"github.com/kudobuilder/kudo/pkg/kudoctl/packages"
 	"github.com/kudobuilder/kudo/pkg/kudoctl/packages/reader"
 )
 
 // AddParameter writes a parameter to the params.yaml file
-func AddParameter(fs afero.Fs, path string, p *v1beta1.Parameter) error {
+func AddParameter(fs afero.Fs, path string, p *packages.Parameter) error {
 
 	pf, err := reader.ReadDir(fs, path)
 	if err != nil {

pkg/kudoctl/cmd/generate/parameter_test.go

@@ -9,8 +9,8 @@ import (
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 
-	"github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1"
 	"github.com/kudobuilder/kudo/pkg/kudoctl/files"
+	"github.com/kudobuilder/kudo/pkg/kudoctl/packages"
 )
 
 func TestAddParameter(t *testing.T) {
@@ -20,7 +20,7 @@ func TestAddParameter(t *testing.T) {
 	files.CopyOperatorToFs(fs, "../../packages/testdata/zk", "/opt")
 
 	bar := "Bar"
-	p := v1beta1.Parameter{
+	p := packages.Parameter{
 		Name:    "Foo",
 		Default: &bar,
 	}
@@ -52,7 +52,7 @@ func TestAddParameter_bad_path(t *testing.T) {
 	fs := afero.OsFs{}
 
 	bar := "Bar"
-	p := v1beta1.Parameter{
+	p := packages.Parameter{
 		Name:    "Foo",
 		Default: &bar,
 	}