KEP-35 Impl Part1: New parameter types and attributes (#1705)

* Added additional types (integer, number, boolean)
* Added enum type
* Added verification of parameter types in admission webhook
* Added descriptive parameters on package level

Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>

config/crds/kudo.dev_operatorversions.yaml

@@ -73,6 +73,11 @@ spec:
                   displayName:
                     description: DisplayName can be used by UIs.
                     type: string
+                  enum:
+                    description: Defines a list of allowed values. If Default is set and Enum is not nil, the value must be in this list as well
+                    items:
+                      type: string
+                    type: array
                   immutable:
                     description: Specifies if the parameter can be changed after the initial installation of the operator
                     type: boolean

pkg/apis/kudo/v1beta1/operatorversion_types.go

@@ -75,7 +75,16 @@ 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.
+	// IntegerValueType is used for parameter values that describe an integral number without any fractional part
+	IntegerValueType ParameterType = "integer"
+
+	// NumberValueType is used for parameter values that describe any numeric value, with or without a fractional part
+	NumberValueType ParameterType = "number"
+
+	// BooleanValueType is used for parameter values that are "true" or "false"
+	BooleanValueType ParameterType = "boolean"
+
+	// ArrayValueType is used for parameter values that describe an array of values.
 	ArrayValueType ParameterType = "array"
 
 	// MapValueType is used for parameter values that describe a mapping type.

pkg/apis/kudo/v1beta1/parameter_types.go

@@ -30,4 +30,7 @@ type Parameter struct {
 
 	// Specifies if the parameter can be changed after the initial installation of the operator
 	Immutable *bool `json:"immutable,omitempty"`
+
+	// Defines a list of allowed values. If Default is set and Enum is not nil, the value must be in this list as well
+	Enum *[]string `json:"enum,omitempty"`
 }

pkg/apis/kudo/v1beta1/parameter_types_helpers.go

@@ -1,5 +1,13 @@
 package v1beta1
 
+import (
+	"fmt"
+	"reflect"
+	"strconv"
+
+	"sigs.k8s.io/yaml"
+)
+
 func (p *Parameter) IsImmutable() bool {
 	return p.Immutable != nil && *p.Immutable
 }
@@ -8,10 +16,173 @@ func (p *Parameter) IsRequired() bool {
 	return p.Required != nil && *p.Required
 }
 
+func (p *Parameter) IsEnum() bool {
+	return p.Enum != nil
+}
+
 func (p *Parameter) HasDefault() bool {
 	return p.Default != nil
 }
 
+func (p *Parameter) EnumValues() []string {
+	if p.IsEnum() {
+		return *p.Enum
+	}
+	return []string{}
+}
+
+func (p *Parameter) ValidateDefault() error {
+	if err := ValidateParameterValueForType(p.Type, p.Default); err != nil {
+		return fmt.Errorf("parameter %q has an invalid default value: %v", p.Name, err)
+	}
+	if p.IsEnum() {
+		if err := ValidateParameterValueForEnum(p.EnumValues(), p.Default); err != nil {
+			return fmt.Errorf("parameter %q has an invalid default value: %v", p.Name, err)
+		}
+	}
+	return nil
+}
+
+// ValidateValue ensures that a the given value is valid for this parameter
+func (p *Parameter) ValidateValue(pValue string) error {
+	if p.IsRequired() && !p.HasDefault() && pValue == "" {
+		return fmt.Errorf("parameter %q is required but has no value set", p.Name)
+	}
+
+	if pValue == "" {
+		return nil
+	}
+
+	if err := ValidateParameterValueForType(p.Type, pValue); err != nil {
+		return fmt.Errorf("parameter %q has an invalid value %q: %v", p.Name, pValue, err)
+	}
+	if p.IsEnum() {
+		if err := ValidateParameterValueForEnum(p.EnumValues(), pValue); err != nil {
+			return fmt.Errorf("parameter %q has an invalid value %q: %v", p.Name, pValue, err)
+		}
+	}
+	return nil
+}
+
+func ValidateParameterValueForType(pType ParameterType, pValue interface{}) error {
+	switch pType {
+	case StringValueType:
+		_, ok := pValue.(string)
+		if !ok {
+			return fmt.Errorf("type is %q but format is invalid: %s", pType, pValue)
+		}
+	case IntegerValueType:
+		return validateIntegerType(pValue)
+	case NumberValueType:
+		return validateNumberType(pValue)
+	case BooleanValueType:
+		return validateBooleanType(pValue)
+	case ArrayValueType:
+		return validateArrayType(pValue)
+	case MapValueType:
+		return validateMapType(pValue)
+	}
+	return nil
+}
+
+func validateIntegerType(pValue interface{}) error {
+	switch v := pValue.(type) {
+	case int, int8, int16, int32, int64:
+		return nil
+	case uint, uint8, uint16, uint32, uint64:
+		return nil
+	case string:
+		_, err := strconv.ParseInt(v, 10, 64)
+		if err != nil {
+			return fmt.Errorf("type is %q but format of %q is invalid: %v", IntegerValueType, pValue, err)
+		}
+	default:
+		return fmt.Errorf("type is %q but format of %s is invalid", IntegerValueType, pValue)
+	}
+	return nil
+}
+
+func validateNumberType(pValue interface{}) error {
+	switch v := pValue.(type) {
+	case int, int8, int16, int32, int64:
+		return nil
+	case uint, uint8, uint16, uint32, uint64:
+		return nil
+	case float32, float64:
+		return nil
+	case string:
+		_, err := strconv.ParseFloat(v, 64)
+		if err != nil {
+			return fmt.Errorf("type is %q but format of %q is invalid: %v", NumberValueType, pValue, err)
+		}
+	default:
+		return fmt.Errorf("type is %q but format of %s is invalid", NumberValueType, v)
+	}
+	return nil
+}
+
+func validateBooleanType(pValue interface{}) error {
+	switch v := pValue.(type) {
+	case bool:
+		return nil
+	case string:
+		_, err := strconv.ParseBool(v)
+		if err != nil {
+			return fmt.Errorf("type is %q but format of %q is invalid: %v", BooleanValueType, pValue, err)
+		}
+	default:
+		return fmt.Errorf("type is %q but format of %s is invalid", BooleanValueType, pValue)
+	}
+	return nil
+}
+
+func validateArrayType(pValue interface{}) error {
+	t := reflect.TypeOf(pValue)
+	switch t.Kind() {
+	case reflect.Slice, reflect.Array:
+		return nil
+	case reflect.String:
+		str, _ := pValue.(string) // We know here this is a string
+
+		var result []interface{}
+		if err := yaml.Unmarshal([]byte(str), &result); err != nil {
+			return fmt.Errorf("type is %q, but format of %s is invalid", ArrayValueType, pValue)
+		}
+
+		return nil
+	default:
+		return fmt.Errorf("type is %q but value %s is not an array", ArrayValueType, pValue)
+	}
+}
+
+func validateMapType(pValue interface{}) error {
+	t := reflect.TypeOf(pValue)
+	switch t.Kind() {
+	case reflect.Map, reflect.Struct:
+		return nil
+	case reflect.String:
+		str, _ := pValue.(string) // We know here this is a string
+
+		var result map[string]interface{}
+		if err := yaml.Unmarshal([]byte(str), &result); err != nil {
+			return fmt.Errorf("type is %q, but format of %s is invalid", MapValueType, pValue)
+		}
+
+		return nil
+	default:
+		return fmt.Errorf("type is %q but value %s is not a map", MapValueType, pValue)
+	}
+}
+
+func ValidateParameterValueForEnum(enumValues []string, pValue interface{}) error {
+	for _, eValue := range enumValues {
+		if pValue == eValue {
+			return nil
+		}
+	}
+	return fmt.Errorf("value is %q, but only allowed values are %v", pValue, enumValues)
+}
+
 // GetChangedParamDefs returns a list of parameters from ov2 that changed based on the given compare function between ov1 and ov2
 func GetChangedParamDefs(p1, p2 []Parameter, isEqual func(p1, p2 Parameter) bool) []Parameter {
 	changedParams := []Parameter{}

pkg/apis/kudo/v1beta1/parameter_types_helpers_test.go

@@ -137,3 +137,92 @@ func TestGetAddedRemovedParamDefs(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateType(t *testing.T) {
+
+	tests := []struct {
+		name        string
+		pValue      interface{}
+		pType       ParameterType
+		expectedErr bool
+	}{
+		{
+			name:   "simple int",
+			pValue: 23,
+			pType:  IntegerValueType,
+		},
+		{
+			name:   "int8",
+			pValue: int8(23),
+			pType:  IntegerValueType,
+		},
+		{
+			name:   "int64",
+			pValue: int64(23),
+			pType:  IntegerValueType,
+		},
+		{
+			name:   "intAsString",
+			pValue: "42",
+			pType:  IntegerValueType,
+		},
+		{
+			name:   "float32",
+			pValue: float32(3.14),
+			pType:  NumberValueType,
+		},
+		{
+			name:   "float64",
+			pValue: float64(3.1415),
+			pType:  NumberValueType,
+		},
+		{
+			name:   "floatAsString",
+			pValue: "3.1415",
+			pType:  NumberValueType,
+		},
+		{
+			name:   "bool",
+			pValue: true,
+			pType:  BooleanValueType,
+		},
+		{
+			name:   "boolAsString",
+			pValue: "true",
+			pType:  BooleanValueType,
+		},
+		{
+			name:   "array",
+			pValue: []string{"oneString", "twoString"},
+			pType:  ArrayValueType,
+		},
+		{
+			name:   "arrayAsString",
+			pValue: `[ "oneString", "twoString" ]`,
+			pType:  ArrayValueType,
+		},
+		{
+			name:   "map",
+			pValue: map[string]string{"oneString": "oneValue", "twoString": "twoValue"},
+			pType:  MapValueType,
+		},
+		{
+			name:   "mapAsString",
+			pValue: `{ "oneString": "oneValue", "twoString": "twoValue" }`,
+			pType:  MapValueType,
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			err := ValidateParameterValueForType(tt.pType, tt.pValue)
+
+			if tt.expectedErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}

pkg/kudoctl/cmd/testdata/deploy-kudo-ns.yaml.golden

@@ -142,6 +142,11 @@ spec:
                   displayName:
                     description: DisplayName can be used by UIs.
                     type: string
+                  enum:
+                    description: Defines a list of allowed values. If Default is set and Enum is not nil, the value must be in this list as well
+                    items:
+                      type: string
+                    type: array
                   immutable:
                     description: Specifies if the parameter can be changed after the initial installation of the operator
                     type: boolean

pkg/kudoctl/cmd/testdata/deploy-kudo-sa.yaml.golden

@@ -142,6 +142,11 @@ spec:
                   displayName:
                     description: DisplayName can be used by UIs.
                     type: string
+                  enum:
+                    description: Defines a list of allowed values. If Default is set and Enum is not nil, the value must be in this list as well
+                    items:
+                      type: string
+                    type: array
                   immutable:
                     description: Specifies if the parameter can be changed after the initial installation of the operator
                     type: boolean

pkg/kudoctl/cmd/testdata/deploy-kudo.json.golden

@@ -196,6 +196,13 @@
                         "description": "DisplayName can be used by UIs.",
                         "type": "string"
                       },
+                      "enum": {
+                        "description": "Defines a list of allowed values. If Default is set and Enum is not nil, the value must be in this list as well",
+                        "type": "array",
+                        "items": {
+                          "type": "string"
+                        }
+                      },
                       "immutable": {
                         "description": "Specifies if the parameter can be changed after the initial installation of the operator",
                         "type": "boolean"

pkg/kudoctl/cmd/testdata/deploy-kudo.yaml.golden

@@ -142,6 +142,11 @@ spec:
                   displayName:
                     description: DisplayName can be used by UIs.
                     type: string
+                  enum:
+                    description: Defines a list of allowed values. If Default is set and Enum is not nil, the value must be in this list as well
+                    items:
+                      type: string
+                    type: array
                   immutable:
                     description: Specifies if the parameter can be changed after the initial installation of the operator
                     type: boolean