NK-31: Types validation methods moved to separate source file.

assertEq and assertNe utils replaced by gotest.tools/assert, deleted these utils.
Implemented proper serialization of JSON patches: all simple types  in values are converted to sting, all maps of interfaces are converted to maps of strings. I.e. implemented applying of JSON patches directly from values in policies.

pkg/apis/policy/v1alpha1/types.go

@@ -1,9 +1,6 @@
 package v1alpha1
 
 import (
-	"errors"
-	"fmt"
-
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -34,43 +31,6 @@ type PolicyRule struct {
 	SecretGenerator    *PolicyConfigGenerator `json:"secretGenerator,omitempty"`
 }
 
-// Checks if rule is not empty and all substructures are valid
-func (pr *PolicyRule) Validate() error {
-	err := pr.Resource.Validate()
-	if err != nil {
-		return err
-	}
-
-	if len(pr.Patches) == 0 && pr.ConfigMapGenerator == nil && pr.SecretGenerator == nil {
-		return errors.New("The rule is empty")
-	}
-
-	if len(pr.Patches) > 0 {
-		for _, patch := range pr.Patches {
-			err = patch.Validate()
-			if err != nil {
-				return err
-			}
-		}
-	}
-
-	if pr.ConfigMapGenerator != nil {
-		err = pr.ConfigMapGenerator.Validate()
-		if err != nil {
-			return err
-		}
-	}
-
-	if pr.SecretGenerator != nil {
-		err = pr.SecretGenerator.Validate()
-		if err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
 // Describes the resource to which the PolicyRule will apply.
 // Either the name or selector must be specified.
 // IMPORTANT: If neither is specified, the policy rule will not apply (TBD).
@@ -80,53 +40,20 @@ type PolicyResource struct {
 	Selector *metav1.LabelSelector `json:"selector,omitempty"`
 }
 
-// Checks if all necesarry fields are present and have values. Also checks a Selector.
-// Returns error if resource definition is invalid.
-func (pr *PolicyResource) Validate() error {
-	// TBD: selector or name MUST be specified
-	if pr.Kind == "" {
-		return errors.New("The Kind is not specified")
-	} else if pr.Name == nil && pr.Selector == nil {
-		return errors.New("Neither Name nor Selector is specified")
-	}
-
-	if pr.Selector != nil {
-		selector, err := metav1.LabelSelectorAsSelector(pr.Selector)
-		if err != nil {
-			return err
-		}
-		requirements, _ := selector.Requirements()
-		if len(requirements) == 0 {
-			return errors.New("The requirements are not specified in selector")
-		}
-	}
-
-	return nil
-}
+// +k8s:deepcopy-gen=false
 
 // PolicyPatch declares patch operation for created object according to the JSONPatch spec:
 // http://jsonpatch.com/
 type PolicyPatch struct {
-	Path      string `json:"path"`
-	Operation string `json:"op"`
-	Value     string `json:"value"`
+	Path      string      `json:"path"`
+	Operation string      `json:"op"`
+	Value     interface{} `json:"value"`
 }
 
-func (pp *PolicyPatch) Validate() error {
-	if pp.Path == "" {
-		return errors.New("JSONPatch field 'path' is mandatory")
+func (in *PolicyPatch) DeepCopyInto(out *PolicyPatch) {
+	if out != nil {
+		*out = *in
 	}
-
-	if pp.Operation == "add" || pp.Operation == "replace" {
-		if pp.Value == "" {
-			return errors.New(fmt.Sprintf("JSONPatch field 'value' is mandatory for operation '%s'", pp.Operation))
-		}
-		return nil
-	} else if pp.Operation == "remove" {
-		return nil
-	}
-
-	return errors.New(fmt.Sprintf("Unsupported JSONPatch operation '%s'", pp.Operation))
 }
 
 // The declaration for a Secret or a ConfigMap, which will be created in the new namespace.
@@ -137,33 +64,12 @@ type PolicyConfigGenerator struct {
 	Data     map[string]string `json:"data"`
 }
 
-// Returns error if generator is configured incompletely
-func (pcg *PolicyConfigGenerator) Validate() error {
-	if pcg.Name == "" {
-		return errors.New("The generator is unnamed")
-	} else if len(pcg.Data) == 0 && pcg.CopyFrom == nil {
-		return errors.New("Neither Data nor CopyFrom (source) is specified")
-	}
-	if pcg.CopyFrom != nil {
-		return pcg.CopyFrom.Validate()
-	}
-	return nil
-}
-
 // Location of a Secret or a ConfigMap which will be used as source when applying PolicyConfigGenerator
 type PolicyCopyFrom struct {
 	Namespace string `json:"namespace"`
 	Name      string `json:"name"`
 }
 
-// Returns error if Name or namespace is not cpecified
-func (pcf *PolicyCopyFrom) Validate() error {
-	if pcf.Name == "" || pcf.Namespace == "" {
-		return errors.New("Name or/and Namespace is not specified")
-	}
-	return nil
-}
-
 // Contains logs about policy application
 type PolicyStatus struct {
 	Logs []string `json:"log"`

pkg/apis/policy/v1alpha1/types_validation.go

@@ -0,0 +1,109 @@
+package v1alpha1
+
+import (
+	"errors"
+	"fmt"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// Checks if rule is not empty and all substructures are valid
+func (pr *PolicyRule) Validate() error {
+	err := pr.Resource.Validate()
+	if err != nil {
+		return err
+	}
+
+	if len(pr.Patches) == 0 && pr.ConfigMapGenerator == nil && pr.SecretGenerator == nil {
+		return errors.New("The rule is empty")
+	}
+
+	if len(pr.Patches) > 0 {
+		for _, patch := range pr.Patches {
+			err = patch.Validate()
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	if pr.ConfigMapGenerator != nil {
+		err = pr.ConfigMapGenerator.Validate()
+		if err != nil {
+			return err
+		}
+	}
+
+	if pr.SecretGenerator != nil {
+		err = pr.SecretGenerator.Validate()
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// Checks if all necesarry fields are present and have values. Also checks a Selector.
+// Returns error if resource definition is invalid.
+func (pr *PolicyResource) Validate() error {
+	// TBD: selector or name MUST be specified
+	if pr.Kind == "" {
+		return errors.New("The Kind is not specified")
+	} else if pr.Name == nil && pr.Selector == nil {
+		return errors.New("Neither Name nor Selector is specified")
+	}
+
+	if pr.Selector != nil {
+		selector, err := metav1.LabelSelectorAsSelector(pr.Selector)
+		if err != nil {
+			return err
+		}
+		requirements, _ := selector.Requirements()
+		if len(requirements) == 0 {
+			return errors.New("The requirements are not specified in selector")
+		}
+	}
+
+	return nil
+}
+
+// Checks if all mandatory PolicyPatch fields are set
+func (pp *PolicyPatch) Validate() error {
+	if pp.Path == "" {
+		return errors.New("JSONPatch field 'path' is mandatory")
+	}
+
+	if pp.Operation == "add" || pp.Operation == "replace" {
+		if pp.Value == nil {
+			return errors.New(fmt.Sprintf("JSONPatch field 'value' is mandatory for operation '%s'", pp.Operation))
+		}
+
+		return nil
+	} else if pp.Operation == "remove" {
+		return nil
+	}
+
+	return errors.New(fmt.Sprintf("Unsupported JSONPatch operation '%s'", pp.Operation))
+}
+
+// Returns error if Name or namespace is not cpecified
+func (pcf *PolicyCopyFrom) Validate() error {
+	if pcf.Name == "" || pcf.Namespace == "" {
+		return errors.New("Name or/and Namespace is not specified")
+	}
+	return nil
+}
+
+// Returns error if generator is configured incompletely
+func (pcg *PolicyConfigGenerator) Validate() error {
+	if pcg.Name == "" {
+		return errors.New("The generator is unnamed")
+	} else if len(pcg.Data) == 0 && pcg.CopyFrom == nil {
+		return errors.New("Neither Data nor CopyFrom (source) is specified")
+	}
+	if pcg.CopyFrom != nil {
+		return pcg.CopyFrom.Validate()
+	}
+	return nil
+}

webhooks/mutation.go

@@ -167,11 +167,7 @@ func SerializePatches(patches []types.PolicyPatch) ([]byte, error) {
 
 	result = append(result, []byte("[\n")...)
 	for index, patch := range patches {
-		if patch.Operation == "" || patch.Path == "" {
-			return nil, errors.New("JSONPatch doesn't contain mandatory fields 'path' or 'op'")
-		}
-
-		patchBytes, err := json.Marshal(patch)
+		patchBytes, err := serializePatch(patch)
 		if err != nil {
 			return nil, err
 		}
@@ -185,6 +181,37 @@ func SerializePatches(patches []types.PolicyPatch) ([]byte, error) {
 	return result, nil
 }
 
+func serializePatch(patch types.PolicyPatch) ([]byte, error) {
+	err := patch.Validate()
+	if err != nil {
+		return nil, err
+	}
+	processedPatchValue, err := processPatchValue(patch.Value)
+	if err != nil {
+		return nil, err
+	} else {
+		patch = types.PolicyPatch{
+			Path:      patch.Path,
+			Operation: patch.Operation,
+			Value:     processedPatchValue,
+		}
+		return json.Marshal(patch)
+	}
+}
+
+// Recursively converts all numbers to strings in JSONPatch value.
+func processPatchValue(value interface{}) interface{} {
+	if interfaceMap, ok := value.(map[string]interface{}); ok {
+		newMap := make(map[string]interface{})
+		for k, v := range interfaceMap {
+			newMap[k] = processPatchValue(v)
+		}
+		return newMap, nil
+	} else {
+		return fmt.Sprintf("%v", value), nil
+	}
+}
+
 func errorToAdmissionResponse(err error, allowed bool) *v1beta1.AdmissionResponse {
 	return &v1beta1.AdmissionResponse{
 		Result: &metav1.Status{

webhooks/mutation_test.go

@@ -1,23 +1,22 @@
 package webhooks_test
 
 import (
+	"gotest.tools/assert"
 	"testing"
 
 	"github.com/nirmata/kube-policy/webhooks"
 
-	//v1beta1 "k8s.io/api/admission/v1beta1"
-	//metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	types "github.com/nirmata/kube-policy/pkg/apis/policy/v1alpha1"
 )
 
 func TestSerializePatches_Empty(t *testing.T) {
 	var patches []types.PolicyPatch
 	bytes, err := webhooks.SerializePatches(patches)
-	assertEq(t, nil, err)
-	assertEq(t, 0, len(bytes))
+	assert.Assert(t, nil == err)
+	assert.Assert(t, 0 == len(bytes))
 }
 
-func TestSerializePatches_SingleValid(t *testing.T) {
+func TestSerializePatches_SingleStringValid(t *testing.T) {
 	patch := types.PolicyPatch{
 		Path:      "/metadata/labels/is-mutated",
 		Operation: "add",
@@ -25,22 +24,144 @@ func TestSerializePatches_SingleValid(t *testing.T) {
 	}
 	patches := []types.PolicyPatch{patch}
 	bytes, err := webhooks.SerializePatches(patches)
-	assertEq(t, nil, err)
+	assert.Assert(t, nil == err)
 	assertEqStringAndData(t, `[
 {"path":"/metadata/labels/is-mutated","op":"add","value":"true"}
 ]`, bytes)
 }
 
-func TestSerializePatches_SingleInvalid(t *testing.T) {
+func TestSerializePatches_SingleStringInvalid(t *testing.T) {
 	patch := types.PolicyPatch{
 		Path:  "/metadata/labels/is-mutated",
 		Value: "true",
 	}
 	patches := []types.PolicyPatch{patch}
 	_, err := webhooks.SerializePatches(patches)
-	assertNe(t, nil, err)
+	assert.Assert(t, nil != err)
 	patches[0].Path = ""
 	patches[0].Operation = "delete"
 	_, err = webhooks.SerializePatches(patches)
-	assertNe(t, nil, err)
+	assert.Assert(t, nil != err)
+}
+
+func TestSerializePatches_MultipleStringsValid(t *testing.T) {
+	patch1 := types.PolicyPatch{
+		Path:      "/metadata/labels/is-mutated",
+		Operation: "add",
+		Value:     "true",
+	}
+	patch2 := types.PolicyPatch{
+		Path:      "/metadata/labels/newLabel",
+		Operation: "add",
+		Value:     "newValue",
+	}
+	patches := []types.PolicyPatch{patch1, patch2}
+	bytes, err := webhooks.SerializePatches(patches)
+	assert.Assert(t, nil == err)
+	assertEqStringAndData(t, `[
+{"path":"/metadata/labels/is-mutated","op":"add","value":"true"},
+{"path":"/metadata/labels/newLabel","op":"add","value":"newValue"}
+]`, bytes)
+}
+
+func TestSerializePatches_SingleIntegerValid(t *testing.T) {
+	const ordinaryInt int = 42
+	patch := types.PolicyPatch{
+		Path:      "/metadata/labels/int",
+		Operation: "add",
+		Value:     ordinaryInt,
+	}
+	patches := []types.PolicyPatch{patch}
+	bytes, err := webhooks.SerializePatches(patches)
+	assert.NilError(t, err)
+	assertEqStringAndData(t, `[
+{"path":"/metadata/labels/int","op":"add","value":"42"}
+]`, bytes)
+}
+
+func TestSerializePatches_SingleIntegerBigValid(t *testing.T) {
+	const bigInt uint64 = 100500100500
+	patch := types.PolicyPatch{
+		Path:      "/metadata/labels/big-int",
+		Operation: "add",
+		Value:     bigInt,
+	}
+	patches := []types.PolicyPatch{patch}
+	bytes, err := webhooks.SerializePatches(patches)
+	assert.NilError(t, err)
+	assertEqStringAndData(t, `[
+{"path":"/metadata/labels/big-int","op":"add","value":"100500100500"}
+]`, bytes)
+}
+
+func TestSerializePatches_SingleFloatValid(t *testing.T) {
+	const ordinaryFloat float32 = 2.71828
+	patch := types.PolicyPatch{
+		Path:      "/metadata/labels/float",
+		Operation: "add",
+		Value:     ordinaryFloat,
+	}
+	patches := []types.PolicyPatch{patch}
+	bytes, err := webhooks.SerializePatches(patches)
+	assert.NilError(t, err)
+	assertEqStringAndData(t, `[
+{"path":"/metadata/labels/float","op":"add","value":"2.71828"}
+]`, bytes)
+}
+
+func TestSerializePatches_SingleFloatBigValid(t *testing.T) {
+	const bigFloat float64 = 3.1415926535
+	patch := types.PolicyPatch{
+		Path:      "/metadata/labels/big-float",
+		Operation: "add",
+		Value:     bigFloat,
+	}
+	patches := []types.PolicyPatch{patch}
+	bytes, err := webhooks.SerializePatches(patches)
+	assert.NilError(t, err)
+	assertEqStringAndData(t, `[
+{"path":"/metadata/labels/big-float","op":"add","value":"3.1415926535"}
+]`, bytes)
+}
+
+func TestSerializePatches_MultipleBoolValid(t *testing.T) {
+	patch1 := types.PolicyPatch{
+		Path:      "/metadata/labels/is-mutated",
+		Operation: "add",
+		Value:     true,
+	}
+	patch2 := types.PolicyPatch{
+		Path:      "/metadata/labels/is-unreal",
+		Operation: "add",
+		Value:     false,
+	}
+	patches := []types.PolicyPatch{patch1, patch2}
+	bytes, err := webhooks.SerializePatches(patches)
+	assert.NilError(t, err)
+	assertEqStringAndData(t, `[
+{"path":"/metadata/labels/is-mutated","op":"add","value":"true"},
+{"path":"/metadata/labels/is-unreal","op":"add","value":"false"}
+]`, bytes)
+}
+
+func TestSerializePatches_MultitypeMap(t *testing.T) {
+	labelsMap := make(map[string]interface{})
+	labelsMap["label1"] = "value1"
+	labelsMap["label42"] = 42
+	nestedMap := make(map[string]interface{})
+	nestedMap["other label"] = "other value"
+	nestedMap["nested"] = true
+	labelsMap["additional"] = nestedMap
+
+	patch := types.PolicyPatch{
+		Path:      "/metadata/labels",
+		Operation: "add",
+		Value:     labelsMap,
+	}
+	patches := []types.PolicyPatch{patch}
+	bytes, err := webhooks.SerializePatches(patches)
+	assert.NilError(t, err)
+	assertEqStringAndData(t, `[
+{"path":"/metadata/labels","op":"add","value":{"additional":{"nested":"true","other label":"other value"},"label1":"value1","label42":"42"}}
+]`, bytes)
 }

webhooks/utils_test.go

@@ -4,18 +4,6 @@ import (
 	"testing"
 )
 
-func assertEq(t *testing.T, expected interface{}, actual interface{}) {
-	if expected != actual {
-		t.Errorf("%s != %s", expected, actual)
-	}
-}
-
-func assertNe(t *testing.T, expected interface{}, actual interface{}) {
-	if expected == actual {
-		t.Errorf("%s != %s", expected, actual)
-	}
-}
-
 func assertEqDataImpl(t *testing.T, expected, actual []byte, formatModifier string) {
 	if len(expected) != len(actual) {
 		t.Errorf("len(expected) != len(actual): %d != %d\n1:"+formatModifier+"\n2:"+formatModifier, len(expected), len(actual), expected, actual)