From a1ba9075d66f365cdc67a8befbc41cc617999f05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Thu, 21 Sep 2023 22:37:32 +0200 Subject: [PATCH] refactor: reuse code moved in kyverno repo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché --- backend/go.mod | 2 +- backend/go.sum | 4 +- backend/pkg/cluster/cm_resolver.go | 3 +- backend/pkg/resource/convert/convert.go | 18 -- backend/pkg/resource/convert/convert_test.go | 42 ----- backend/pkg/resource/{loader => }/helpers.go | 10 +- .../pkg/resource/{loader => }/helpers_test.go | 7 +- backend/pkg/resource/loader/loader.go | 39 ---- backend/pkg/resource/loader/loader_test.go | 177 ------------------ backend/pkg/server/api/engine/handler.go | 2 +- backend/pkg/server/api/engine/request.go | 11 +- backend/pkg/utils/exception.go | 7 +- backend/pkg/utils/exception_test.go | 2 +- backend/pkg/utils/policy.go | 7 +- backend/pkg/utils/policy_test.go | 2 +- 15 files changed, 30 insertions(+), 303 deletions(-) delete mode 100644 backend/pkg/resource/convert/convert.go delete mode 100644 backend/pkg/resource/convert/convert_test.go rename backend/pkg/resource/{loader => }/helpers.go (66%) rename backend/pkg/resource/{loader => }/helpers_test.go (88%) delete mode 100644 backend/pkg/resource/loader/loader.go delete mode 100644 backend/pkg/resource/loader/loader_test.go diff --git a/backend/go.mod b/backend/go.mod index 4524f4eb..6b30d1f3 100644 --- a/backend/go.mod +++ b/backend/go.mod @@ -8,7 +8,7 @@ require ( github.com/gin-contrib/cors v1.4.0 github.com/gin-gonic/gin v1.9.1 github.com/go-logr/logr v1.2.4 - github.com/kyverno/kyverno v1.11.0-beta.1 + github.com/kyverno/kyverno v1.11.0-beta.1.0.20230921175422-81f2646963bf github.com/loopfz/gadgeto v0.11.3 github.com/spf13/cobra v1.7.0 github.com/stretchr/testify v1.8.4 diff --git a/backend/go.sum b/backend/go.sum index 471b5667..f4e212f9 100644 --- a/backend/go.sum +++ b/backend/go.sum @@ -1039,8 +1039,8 @@ github.com/kyverno/go-jmespath v0.4.1-0.20230705123211-d067dc3d6613 h1:M0uOLuCAZ github.com/kyverno/go-jmespath v0.4.1-0.20230705123211-d067dc3d6613/go.mod h1:yzDHaKovQy16rjN4kFnjF+IdNoN4p1ndw+va6+B8zUU= github.com/kyverno/go-jmespath/internal/testify v1.5.2-0.20230630133209-945021c749d9 h1:lL311dF3a2aeNibJj8v+uhFU3XkvRHZmCtAdSPOrQYY= github.com/kyverno/go-jmespath/internal/testify v1.5.2-0.20230630133209-945021c749d9/go.mod h1:XRxUGHIiCy1WYma1CdfdO1WOhIe8dLPTENaZr5D1ex4= -github.com/kyverno/kyverno v1.11.0-beta.1 h1:pICon1vz0CwIm3YVQL185q9kW0QM09Xu2JY/vh9wKyY= -github.com/kyverno/kyverno v1.11.0-beta.1/go.mod h1:xQfQwdPstbC//8ZSLz5ttZOLFMdsuGVgI7Y45DlYzBg= +github.com/kyverno/kyverno v1.11.0-beta.1.0.20230921175422-81f2646963bf h1:T9aTNGsYBA0xom5suRgiuN1y7DiOiCdjBteBv6C5dhc= +github.com/kyverno/kyverno v1.11.0-beta.1.0.20230921175422-81f2646963bf/go.mod h1:xQfQwdPstbC//8ZSLz5ttZOLFMdsuGVgI7Y45DlYzBg= github.com/ldez/gomoddirectives v0.2.1/go.mod h1:sGicqkRgBOg//JfpXwkB9Hj0X5RyJ7mlACM5B9f6Me4= github.com/ldez/tagliatelle v0.2.0/go.mod h1:8s6WJQwEYHbKZDsp/LjArytKOG8qaMrKQQ3mFukHs88= github.com/leodido/go-urn v1.2.0/go.mod h1:+8+nEpDfqqsY+g338gtMEUOtuK+4dEMhiQEgxpxOKII= diff --git a/backend/pkg/cluster/cm_resolver.go b/backend/pkg/cluster/cm_resolver.go index 7a1b593a..3a7c55a1 100644 --- a/backend/pkg/cluster/cm_resolver.go +++ b/backend/pkg/cluster/cm_resolver.go @@ -4,11 +4,10 @@ import ( "context" "errors" + "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/resource/convert" "github.com/kyverno/kyverno/pkg/clients/dclient" engineapi "github.com/kyverno/kyverno/pkg/engine/api" corev1 "k8s.io/api/core/v1" - - "github.com/kyverno/playground/backend/pkg/resource/convert" ) type clientBasedResolver struct { diff --git a/backend/pkg/resource/convert/convert.go b/backend/pkg/resource/convert/convert.go deleted file mode 100644 index 876c2c76..00000000 --- a/backend/pkg/resource/convert/convert.go +++ /dev/null @@ -1,18 +0,0 @@ -package convert - -import ( - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" -) - -func Into[T any](untyped unstructured.Unstructured, result *T) error { - return runtime.DefaultUnstructuredConverter.FromUnstructuredWithValidation(untyped.UnstructuredContent(), result, true) -} - -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/backend/pkg/resource/convert/convert_test.go b/backend/pkg/resource/convert/convert_test.go deleted file mode 100644 index 7b79a2b0..00000000 --- a/backend/pkg/resource/convert/convert_test.go +++ /dev/null @@ -1,42 +0,0 @@ -package convert - -import ( - "os" - "testing" - - "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "sigs.k8s.io/yaml" -) - -func TestTo(t *testing.T) { - { - data, err := os.ReadFile("../../../testdata/single-policy.yaml") - require.NoError(t, err) - - json, err := yaml.YAMLToJSON(data) - require.NoError(t, err) - - var untyped unstructured.Unstructured - require.NoError(t, untyped.UnmarshalJSON(json)) - - typed, err := To[corev1.ConfigMap](untyped) - require.Nil(t, typed) - require.Error(t, err) - } - { - data, err := os.ReadFile("../../../testdata/namespace.yaml") - require.NoError(t, err) - - json, err := yaml.YAMLToJSON(data) - require.NoError(t, err) - - var untyped unstructured.Unstructured - require.NoError(t, untyped.UnmarshalJSON(json)) - - typed, err := To[corev1.Namespace](untyped) - require.NotNil(t, typed) - require.NoError(t, err) - } -} diff --git a/backend/pkg/resource/loader/helpers.go b/backend/pkg/resource/helpers.go similarity index 66% rename from backend/pkg/resource/loader/helpers.go rename to backend/pkg/resource/helpers.go index 5dc6400d..37fa92a7 100644 --- a/backend/pkg/resource/loader/helpers.go +++ b/backend/pkg/resource/helpers.go @@ -1,13 +1,13 @@ -package loader +package resource import ( + "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" - - "github.com/kyverno/playground/backend/pkg/resource/convert" ) -func Load[T any](l Loader, content []byte) (*T, error) { +func Load[T any](l loader.Loader, content []byte) (*T, error) { untyped, err := l.Load(content) if err != nil { return nil, err @@ -19,7 +19,7 @@ func Load[T any](l Loader, content []byte) (*T, error) { return result, nil } -func LoadResources(l Loader, content []byte) ([]unstructured.Unstructured, error) { +func LoadResources(l loader.Loader, content []byte) ([]unstructured.Unstructured, error) { documents, err := yamlutils.SplitDocuments(content) if err != nil { return nil, err diff --git a/backend/pkg/resource/loader/helpers_test.go b/backend/pkg/resource/helpers_test.go similarity index 88% rename from backend/pkg/resource/loader/helpers_test.go rename to backend/pkg/resource/helpers_test.go index 64aa062d..72d52161 100644 --- a/backend/pkg/resource/loader/helpers_test.go +++ b/backend/pkg/resource/helpers_test.go @@ -1,11 +1,12 @@ -package loader_test +package resource_test import ( "testing" + "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/resource/loader" "sigs.k8s.io/kubectl-validate/pkg/openapiclient" - "github.com/kyverno/playground/backend/pkg/resource/loader" + "github.com/kyverno/playground/backend/pkg/resource" ) const ( @@ -98,7 +99,7 @@ func Test_LoadResources(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if res, err := loader.LoadResources(l, []byte(tt.resources)); (err != nil) != tt.wantErr { + if res, err := resource.LoadResources(l, []byte(tt.resources)); (err != nil) != tt.wantErr { t.Errorf("loader.Resources() error = %v, wantErr %v", err, tt.wantErr) } else if len(res) != tt.wantLoaded { t.Errorf("loader.Resources() loaded amount = %v, wantLoaded %v", len(res), tt.wantLoaded) diff --git a/backend/pkg/resource/loader/loader.go b/backend/pkg/resource/loader/loader.go deleted file mode 100644 index a016036e..00000000 --- a/backend/pkg/resource/loader/loader.go +++ /dev/null @@ -1,39 +0,0 @@ -package loader - -import ( - "fmt" - - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/client-go/openapi" - "sigs.k8s.io/kubectl-validate/pkg/validator" -) - -type Loader interface { - Load([]byte) (unstructured.Unstructured, error) -} - -type loader struct { - validator *validator.Validator -} - -func New(client openapi.Client) (Loader, error) { - factory, err := validator.New(client) - if err != nil { - return nil, err - } - return &loader{ - validator: factory, - }, nil -} - -func (l *loader) Load(document []byte) (unstructured.Unstructured, error) { - _, result, err := l.validator.Parse(document) - if err != nil { - return unstructured.Unstructured{}, fmt.Errorf("failed to parse document (%w)", err) - } - // TODO: remove DeepCopy when fixed upstream - if err := l.validator.Validate(result.DeepCopy()); err != nil { - return unstructured.Unstructured{}, fmt.Errorf("failed to validate resource (%w)", err) - } - return *result, nil -} diff --git a/backend/pkg/resource/loader/loader_test.go b/backend/pkg/resource/loader/loader_test.go deleted file mode 100644 index 1d08aba8..00000000 --- a/backend/pkg/resource/loader/loader_test.go +++ /dev/null @@ -1,177 +0,0 @@ -package loader - -import ( - "errors" - "os" - "reflect" - "testing" - - "github.com/stretchr/testify/require" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/client-go/openapi" - "sigs.k8s.io/kubectl-validate/pkg/openapiclient" - "sigs.k8s.io/kubectl-validate/pkg/validator" - "sigs.k8s.io/yaml" - - "github.com/kyverno/playground/backend/data" -) - -type errClient struct{} - -func (errClient) Paths() (map[string]openapi.GroupVersion, error) { - return nil, errors.New("error") -} - -func TestNew(t *testing.T) { - tests := []struct { - name string - client openapi.Client - want Loader - wantErr bool - }{{ - name: "err client", - client: errClient{}, - wantErr: true, - }, { - name: "builtin", - client: openapiclient.NewHardcodedBuiltins("1.27"), - want: func() Loader { - validator, err := validator.New(openapiclient.NewHardcodedBuiltins("1.27")) - require.NoError(t, err) - return &loader{ - validator: validator, - } - }(), - }, { - name: "invalid local", - client: openapiclient.NewLocalSchemaFiles(data.Schemas(), "blam"), - want: func() Loader { - validator, err := validator.New(openapiclient.NewLocalSchemaFiles(data.Schemas(), "blam")) - require.NoError(t, err) - return &loader{ - validator: validator, - } - }(), - }, { - name: "composite - no clients", - client: openapiclient.NewComposite(), - want: func() Loader { - validator, err := validator.New(openapiclient.NewComposite()) - require.NoError(t, err) - return &loader{ - validator: validator, - } - }(), - }, { - name: "composite - err client", - client: openapiclient.NewComposite(errClient{}), - wantErr: true, - }, { - name: "composite - with err client", - client: openapiclient.NewComposite(openapiclient.NewHardcodedBuiltins("1.27"), errClient{}), - wantErr: true, - }, { - name: "composite - invalid local", - client: openapiclient.NewComposite(openapiclient.NewLocalSchemaFiles(data.Schemas(), "blam")), - want: func() Loader { - validator, err := validator.New(openapiclient.NewComposite(openapiclient.NewLocalSchemaFiles(data.Schemas(), "blam"))) - require.NoError(t, err) - return &loader{ - validator: validator, - } - }(), - }} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := New(tt.client) - if (err != nil) != tt.wantErr { - t.Errorf("New() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("New() = %v, want %v", got, tt.want) - } - }) - } -} - -func Test_loader_Load(t *testing.T) { - loadFile := func(path string) []byte { - bytes, err := os.ReadFile(path) - require.NoError(t, err) - return bytes - } - newLoader := func(client openapi.Client) Loader { - loader, err := New(client) - require.NoError(t, err) - return loader - } - toUnstructured := func(data []byte) unstructured.Unstructured { - json, err := yaml.YAMLToJSON(data) - require.NoError(t, err) - var result unstructured.Unstructured - require.NoError(t, result.UnmarshalJSON(json)) - if result.GetCreationTimestamp().Time.IsZero() { - require.NoError(t, unstructured.SetNestedField(result.UnstructuredContent(), nil, "metadata", "creationTimestamp")) - } - return result - } - tests := []struct { - name string - loader Loader - document []byte - want unstructured.Unstructured - wantErr bool - }{{ - name: "nil", - loader: newLoader(openapiclient.NewLocalSchemaFiles(data.Schemas(), "schemas")), - wantErr: true, - }, { - name: "empty GVK", - loader: newLoader(openapiclient.NewLocalSchemaFiles(data.Schemas(), "schemas")), - document: []byte(`foo: bar`), - wantErr: true, - }, { - name: "not yaml", - loader: newLoader(openapiclient.NewLocalSchemaFiles(data.Schemas(), "schemas")), - document: []byte(` - foo - bar - - baz`), - wantErr: true, - }, { - name: "unknown GVK", - loader: newLoader(openapiclient.NewLocalSchemaFiles(data.Schemas(), "schemas")), - document: loadFile("../../../testdata/namespace.yaml"), - wantErr: true, - }, { - name: "bad schema", - loader: newLoader(openapiclient.NewHardcodedBuiltins("1.27")), - document: []byte(` - apiVersion: v1 - kind: Namespace - bad: field - metadata: - name: prod-bus-app1 - labels: - purpose: production`), - wantErr: true, - }, { - name: "ok", - loader: newLoader(openapiclient.NewHardcodedBuiltins("1.27")), - document: loadFile("../../../testdata/namespace.yaml"), - want: toUnstructured(loadFile("../../../testdata/namespace.yaml")), - }} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := tt.loader.Load(tt.document) - if (err != nil) != tt.wantErr { - t.Errorf("loader.Load() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("loader.Load() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/backend/pkg/server/api/engine/handler.go b/backend/pkg/server/api/engine/handler.go index c2c4aa7e..37b48fbd 100644 --- a/backend/pkg/server/api/engine/handler.go +++ b/backend/pkg/server/api/engine/handler.go @@ -6,6 +6,7 @@ import ( "github.com/Masterminds/semver/v3" "github.com/gin-gonic/gin" + "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/resource/loader" engineapi "github.com/kyverno/kyverno/pkg/engine/api" "github.com/loopfz/gadgeto/tonic" "sigs.k8s.io/kubectl-validate/pkg/openapiclient" @@ -13,7 +14,6 @@ import ( "github.com/kyverno/playground/backend/data" "github.com/kyverno/playground/backend/pkg/cluster" "github.com/kyverno/playground/backend/pkg/engine" - "github.com/kyverno/playground/backend/pkg/resource/loader" ) func newEngineHandler(cl cluster.Cluster, config APIConfiguration) (gin.HandlerFunc, error) { diff --git a/backend/pkg/server/api/engine/request.go b/backend/pkg/server/api/engine/request.go index 941fd60c..6d879685 100644 --- a/backend/pkg/server/api/engine/request.go +++ b/backend/pkg/server/api/engine/request.go @@ -5,6 +5,7 @@ import ( kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" kyvernov2alpha1 "github.com/kyverno/kyverno/api/kyverno/v2alpha1" + "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/resource/loader" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/openapi" @@ -14,7 +15,7 @@ import ( "github.com/kyverno/playground/backend/data" "github.com/kyverno/playground/backend/pkg/cluster" "github.com/kyverno/playground/backend/pkg/engine/models" - "github.com/kyverno/playground/backend/pkg/resource/loader" + "github.com/kyverno/playground/backend/pkg/resource" "github.com/kyverno/playground/backend/pkg/utils" ) @@ -43,15 +44,15 @@ func (r *EngineRequest) LoadPolicies(policyLoader loader.Loader) ([]kyvernov1.Po } func (r *EngineRequest) LoadResources(resourceLoader loader.Loader) ([]unstructured.Unstructured, error) { - return loader.LoadResources(resourceLoader, []byte(r.Resources)) + return resource.LoadResources(resourceLoader, []byte(r.Resources)) } func (r *EngineRequest) LoadClusterResources(resourceLoader loader.Loader) ([]unstructured.Unstructured, error) { - return loader.LoadResources(resourceLoader, []byte(r.ClusterResources)) + return resource.LoadResources(resourceLoader, []byte(r.ClusterResources)) } func (r *EngineRequest) LoadOldResources(resourceLoader loader.Loader) ([]unstructured.Unstructured, error) { - return loader.LoadResources(resourceLoader, []byte(r.OldResources)) + return resource.LoadResources(resourceLoader, []byte(r.OldResources)) } func (r *EngineRequest) LoadPolicyExceptions(resourceLoader loader.Loader) ([]*kyvernov2alpha1.PolicyException, error) { @@ -62,7 +63,7 @@ func (r *EngineRequest) LoadConfig(resourceLoader loader.Loader) (*corev1.Config if len(r.Config) == 0 { return nil, nil } - return loader.Load[corev1.ConfigMap](resourceLoader, []byte(r.Config)) + return resource.Load[corev1.ConfigMap](resourceLoader, []byte(r.Config)) } func (r *EngineRequest) ResourceLoader(cluster cluster.Cluster, kubeVersion string, config APIConfiguration) (loader.Loader, error) { diff --git a/backend/pkg/utils/exception.go b/backend/pkg/utils/exception.go index 6d122b72..281ea45e 100644 --- a/backend/pkg/utils/exception.go +++ b/backend/pkg/utils/exception.go @@ -2,13 +2,14 @@ package utils import ( kyvernov2alpha1 "github.com/kyverno/kyverno/api/kyverno/v2alpha1" + "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/resource/convert" + "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/resource/loader" - "github.com/kyverno/playground/backend/pkg/resource/convert" - "github.com/kyverno/playground/backend/pkg/resource/loader" + "github.com/kyverno/playground/backend/pkg/resource" ) func LoadPolicyExceptions(l loader.Loader, content []byte) ([]*kyvernov2alpha1.PolicyException, error) { - untyped, err := loader.LoadResources(l, content) + untyped, err := resource.LoadResources(l, content) if err != nil { return nil, err } diff --git a/backend/pkg/utils/exception_test.go b/backend/pkg/utils/exception_test.go index cd489494..2b1423e8 100644 --- a/backend/pkg/utils/exception_test.go +++ b/backend/pkg/utils/exception_test.go @@ -4,11 +4,11 @@ import ( "os" "testing" + "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/resource/loader" "github.com/stretchr/testify/require" "sigs.k8s.io/kubectl-validate/pkg/openapiclient" "github.com/kyverno/playground/backend/data" - "github.com/kyverno/playground/backend/pkg/resource/loader" "github.com/kyverno/playground/backend/pkg/utils" ) diff --git a/backend/pkg/utils/policy.go b/backend/pkg/utils/policy.go index 0e92e55e..daa43a42 100644 --- a/backend/pkg/utils/policy.go +++ b/backend/pkg/utils/policy.go @@ -4,10 +4,11 @@ import ( "fmt" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" + "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/resource/convert" + "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/resource/loader" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "github.com/kyverno/playground/backend/pkg/resource/convert" - "github.com/kyverno/playground/backend/pkg/resource/loader" + "github.com/kyverno/playground/backend/pkg/resource" ) func ToPolicyInterface(untyped unstructured.Unstructured) (kyvernov1.PolicyInterface, error) { @@ -29,7 +30,7 @@ func ToPolicyInterface(untyped unstructured.Unstructured) (kyvernov1.PolicyInter } func LoadPolicies(l loader.Loader, content []byte) ([]kyvernov1.PolicyInterface, error) { - untyped, err := loader.LoadResources(l, content) + untyped, err := resource.LoadResources(l, content) if err != nil { return nil, err } diff --git a/backend/pkg/utils/policy_test.go b/backend/pkg/utils/policy_test.go index 6b01725c..613cb567 100644 --- a/backend/pkg/utils/policy_test.go +++ b/backend/pkg/utils/policy_test.go @@ -4,12 +4,12 @@ import ( "os" "testing" + "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/resource/loader" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/kubectl-validate/pkg/openapiclient" "github.com/kyverno/playground/backend/data" - "github.com/kyverno/playground/backend/pkg/resource/loader" "github.com/kyverno/playground/backend/pkg/utils" )