fix: custom marshalling for Any

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
kyverno · Oct 3, 2023 · 4e62288 · 4e62288
1 parent 133fbaf
commit 4e62288
Show file tree

Hide file tree

Showing 12 changed files with 83 additions and 104 deletions.
diff --git a/config/crds/json.kyverno.io_policies.yaml b/config/crds/json.kyverno.io_policies.yaml
@@ -94,11 +94,7 @@ spec:
                           variable:
                             description: Variable defines an arbitrary JMESPath context
                               variable that can be defined inline.
-                            properties:
-                              value:
-                                description: Value is any arbitrary object.
-                                x-kubernetes-preserve-unknown-fields: true
-                            type: object
+                            x-kubernetes-preserve-unknown-fields: true
                         required:
                         - name
                         type: object

diff --git a/pkg/apis/v1alpha1/any.go b/pkg/apis/v1alpha1/any.go
@@ -1,22 +1,34 @@
 package v1alpha1
 
 import (
+	"encoding/json"
+
 	"github.com/jinzhu/copier"
 )
 
-type Value = interface{}
-
 // +k8s:deepcopy-gen=false
 type Any struct {
-	// TODO: this is needed to workaround a bug in api machinery code
-	// https://kubernetes.slack.com/archives/C0EG7JC6T/p1696331287543159
 	// +kubebuilder:pruning:PreserveUnknownFields
 	// +kubebuilder:validation:Schemaless
-	Value `json:",inline"`
+	Value interface{} `json:",inline"`
 }
 
 func (in *Any) DeepCopyInto(out *Any) {
 	if err := copier.Copy(out, in); err != nil {
 		panic("deep copy failed")
 	}
 }
+
+func (a *Any) MarshalJSON() ([]byte, error) {
+	return json.Marshal(a.Value)
+}
+
+func (a *Any) UnmarshalJSON(data []byte) error {
+	var v interface{}
+	err := json.Unmarshal(data, &v)
+	if err != nil {
+		return err
+	}
+	a.Value = v
+	return nil
+}
diff --git a/pkg/apis/v1alpha1/context_entry.go b/pkg/apis/v1alpha1/context_entry.go
@@ -6,7 +6,9 @@ type ContextEntry struct {
 	Name string `json:"name"`
 
 	// Variable defines an arbitrary JMESPath context variable that can be defined inline.
-	Variable *Variable `json:"variable,omitempty"`
+	// +kubebuilder:pruning:PreserveUnknownFields
+	// +kubebuilder:validation:Schemaless
+	Variable Any `json:"variable,omitempty"`
 
 	// ImageRegistry defines requests to an OCI/Docker V2 registry to fetch image details.
 	ImageRegistry *ImageRegistry `json:"imageRegistry,omitempty"`

diff --git a/pkg/apis/v1alpha1/variable.go b/pkg/apis/v1alpha1/variable.go
@@ -1,25 +1,21 @@
 package v1alpha1
 
-import (
-	"github.com/jinzhu/copier"
-)
+// // Variable defines an arbitrary JMESPath context variable that can be defined inline.
+// // +k8s:deepcopy-gen=false
+// type Variable struct {
+// 	// Value is any arbitrary object.
+// 	// +kubebuilder:pruning:PreserveUnknownFields
+// 	// +kubebuilder:validation:Schemaless
+// 	Value interface{} `json:"value,omitempty"`
+// }
 
-// Variable defines an arbitrary JMESPath context variable that can be defined inline.
-// +k8s:deepcopy-gen=false
-type Variable struct {
-	// Value is any arbitrary object.
-	// +kubebuilder:pruning:PreserveUnknownFields
-	// +kubebuilder:validation:Schemaless
-	Value interface{} `json:"value,omitempty"`
-}
-
-func (in *Variable) DeepCopy() *Variable {
-	if in == nil {
-		return nil
-	}
-	out := &Variable{}
-	if err := copier.Copy(out, in); err != nil {
-		panic("deep copy failed")
-	}
-	return out
-}
+// func (in *Variable) DeepCopy() *Variable {
+// 	if in == nil {
+// 		return nil
+// 	}
+// 	out := &Variable{}
+// 	if err := copier.Copy(out, in); err != nil {
+// 		panic("deep copy failed")
+// 	}
+// 	return out
+// }
diff --git a/pkg/apis/v1alpha1/variable_test.go b/pkg/apis/v1alpha1/variable_test.go
@@ -1,39 +1,39 @@
 package v1alpha1
 
-import (
-	"reflect"
-	"testing"
+// import (
+// 	"reflect"
+// 	"testing"
 
-	"github.com/stretchr/testify/assert"
-)
+// 	"github.com/stretchr/testify/assert"
+// )
 
-func TestVariable_DeepCopy(t *testing.T) {
-	tests := []struct {
-		name string
-		in   *Variable
-	}{{
-		name: "nil",
-		in:   &Variable{nil},
-	}, {
-		name: "nil",
-		in:   nil,
-	}, {
-		name: "int",
-		in:   &Variable{42},
-	}, {
-		name: "string",
-		in:   &Variable{"foo"},
-	}, {
-		name: "slice",
-		in:   &Variable{[]interface{}{42, "string"}},
-	}}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := tt.in.DeepCopy(); !reflect.DeepEqual(got, tt.in) {
-				t.Errorf("Variable.DeepCopy() = %v, want %v", got, tt.in)
-			} else if tt.in != nil {
-				assert.False(t, got == tt.in)
-			}
-		})
-	}
-}
+// func TestVariable_DeepCopy(t *testing.T) {
+// 	tests := []struct {
+// 		name string
+// 		in   *Variable
+// 	}{{
+// 		name: "nil",
+// 		in:   &Variable{nil},
+// 	}, {
+// 		name: "nil",
+// 		in:   nil,
+// 	}, {
+// 		name: "int",
+// 		in:   &Variable{42},
+// 	}, {
+// 		name: "string",
+// 		in:   &Variable{"foo"},
+// 	}, {
+// 		name: "slice",
+// 		in:   &Variable{[]interface{}{42, "string"}},
+// 	}}
+// 	for _, tt := range tests {
+// 		t.Run(tt.name, func(t *testing.T) {
+// 			if got := tt.in.DeepCopy(); !reflect.DeepEqual(got, tt.in) {
+// 				t.Errorf("Variable.DeepCopy() = %v, want %v", got, tt.in)
+// 			} else if tt.in != nil {
+// 				assert.False(t, got == tt.in)
+// 			}
+// 		})
+// 	}
+// }
diff --git a/pkg/apis/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/v1alpha1/zz_generated.deepcopy.go
diff --git a/pkg/data/crds/json.kyverno.io_policies.yaml b/pkg/data/crds/json.kyverno.io_policies.yaml
@@ -94,11 +94,7 @@ spec:
                           variable:
                             description: Variable defines an arbitrary JMESPath context
                               variable that can be defined inline.
-                            properties:
-                              value:
-                                description: Value is any arbitrary object.
-                                x-kubernetes-preserve-unknown-fields: true
-                            type: object
+                            x-kubernetes-preserve-unknown-fields: true
                         required:
                         - name
                         type: object

diff --git a/pkg/policy/load.go b/pkg/policy/load.go
@@ -9,10 +9,9 @@ import (
 	"github.com/kyverno/kyverno-json/pkg/apis/v1alpha1"
 	"github.com/kyverno/kyverno-json/pkg/data"
 	fileinfo "github.com/kyverno/kyverno-json/pkg/utils/file-info"
+	"github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/resource/convert"
 	"github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/resource/loader"
 	yamlutils "github.com/kyverno/kyverno/pkg/utils/yaml"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"sigs.k8s.io/kubectl-validate/pkg/openapiclient"
 )
@@ -81,9 +80,7 @@ func Parse(content []byte) ([]*v1alpha1.Policy, error) {
 		}
 		switch gvk {
 		case policy_v1alpha1:
-			// TODO: don't use kyverno's convert for now to workaround the bug in api machinery code
-			// https://kubernetes.slack.com/archives/C0EG7JC6T/p1696331287543159
-			policy, err := To[v1alpha1.Policy](untyped)
+			policy, err := convert.To[v1alpha1.Policy](untyped)
 			if err != nil {
 				return nil, err
 			}
@@ -94,15 +91,3 @@ func Parse(content []byte) ([]*v1alpha1.Policy, error) {
 	}
 	return policies, nil
 }
-
-func Into[T any](untyped unstructured.Unstructured, result *T) error {
-	return runtime.DefaultUnstructuredConverter.FromUnstructured(untyped.UnstructuredContent(), result)
-}
-
-func To[T any](untyped unstructured.Unstructured) (*T, error) {
-	var result T
-	if err := Into(untyped, &result); err != nil {
-		return nil, err
-	}
-	return &result, nil
-}
diff --git a/testdata/payload-yaml/policy.yaml b/testdata/payload-yaml/policy.yaml
@@ -12,8 +12,7 @@ spec:
       context:
       - name: tags
         variable:
-          value:
-            Team: Kyverno
+          Team: Kyverno
       validate:
         message: Bucket `{{ resource.name }}` ({{ resource.address }}) does not have the required tags {{ to_string($tags) }}
         assert:

diff --git a/testdata/pod-all-latest/policy.yaml b/testdata/pod-all-latest/policy.yaml
@@ -7,11 +7,9 @@ spec:
     - name: pod-no-latest
       context:
         - name: tag
-          variable:
-            value: latest
+          variable: latest
         - name: tag
-          variable:
-            value: (concat(':', $tag))
+          variable: (concat(':', $tag))
       match:
         any:
         - resource:

diff --git a/testdata/pod-no-latest/policy.yaml b/testdata/pod-no-latest/policy.yaml
@@ -7,8 +7,7 @@ spec:
     - name: pod-no-latest
       context:
         - name: tag
-          variable:
-            value: :latest
+          variable: :latest
       match:
         any:
         - resource:

diff --git a/testdata/tf-plan/policy.yaml b/testdata/tf-plan/policy.yaml
@@ -12,8 +12,7 @@ spec:
       context:
       - name: tags
         variable:
-          value:
-            Team: Kyverno
+          Team: Kyverno
       validate:
         message: Bucket `{{ resource.name }}` ({{ resource.address }}) does not have the required tags {{ to_string($tags) }}
         assert: