From cc1b580a18afb2547dd6286dd0ecea66a8df3ff1 Mon Sep 17 00:00:00 2001 From: gehrkefc Date: Wed, 11 Sep 2024 21:32:02 -0300 Subject: [PATCH] added TestUpdate --- pkg/stores/proxy/proxy_store.go | 7 +- pkg/stores/proxy/proxy_store_test.go | 349 +++++++++++++++++++++++- pkg/stores/sqlproxy/proxy_store.go | 28 +- pkg/stores/sqlproxy/proxy_store_test.go | 344 +++++++++++++++++++++++ 4 files changed, 719 insertions(+), 9 deletions(-) diff --git a/pkg/stores/proxy/proxy_store.go b/pkg/stores/proxy/proxy_store.go index 487f5dca..b2bb31b7 100644 --- a/pkg/stores/proxy/proxy_store.go +++ b/pkg/stores/proxy/proxy_store.go @@ -40,8 +40,9 @@ import ( ) const ( - watchTimeoutEnv = "CATTLE_WATCH_TIMEOUT_SECONDS" - errNamespaceRequired = "metadata.namespace is required" + watchTimeoutEnv = "CATTLE_WATCH_TIMEOUT_SECONDS" + errNamespaceRequired = "metadata.namespace is required" + errResourceVersionRequired = "metadata.resourceVersion is required for update" ) var ( @@ -523,7 +524,7 @@ func (s *Store) Update(apiOp *types.APIRequest, schema *types.APISchema, params resourceVersion := input.String("metadata", "resourceVersion") if resourceVersion == "" { - return nil, nil, fmt.Errorf("metadata.resourceVersion is required for update") + return nil, nil, fmt.Errorf(errResourceVersionRequired) } gvk := attributes.GVK(schema) diff --git a/pkg/stores/proxy/proxy_store_test.go b/pkg/stores/proxy/proxy_store_test.go index af90ef00..5fe97dfb 100644 --- a/pkg/stores/proxy/proxy_store_test.go +++ b/pkg/stores/proxy/proxy_store_test.go @@ -1,6 +1,7 @@ package proxy import ( + "fmt" "net/http" "net/url" "strings" @@ -343,7 +344,7 @@ func TestCreate(t *testing.T) { }, }, { - name: "missing namespace in the params / should copy from apiOp", + name: "missing namespace in the params (should copy from apiOp)", input: input{ apiOp: &types.APIRequest{ Schema: &types.APISchema{ @@ -466,7 +467,7 @@ func TestCreate(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { testClientFactory, err := client.NewFactory(&rest.Config{}, false) - assert.Nil(t, err) + assert.NoError(t, err) fakeClient := fake.NewSimpleDynamicClient(runtime.NewScheme()) @@ -488,3 +489,347 @@ func TestCreate(t *testing.T) { }) } } + +func TestUpdate(t *testing.T) { + type input struct { + apiOp *types.APIRequest + schema *types.APISchema + params types.APIObject + id string + } + + type expected struct { + value *unstructured.Unstructured + warning []types.Warning + err error + } + + sampleCreateInput := input{ + apiOp: &types.APIRequest{ + Request: &http.Request{ + URL: &url.URL{}, + Method: http.MethodPut, + }, + Schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + }, + }, + Method: http.MethodPut, + }, + schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + Attributes: map[string]interface{}{ + "version": "v1", + "namespaced": true, + }, + }, + }, + params: types.APIObject{ + Object: map[string]interface{}{ + "kind": "Secret", + "apiVersion": "v1", + "metadata": map[string]interface{}{ + "name": "testing-secret", + "namespace": "testing-ns", + "resourceVersion": "1", + }, + }, + }, + } + + testCases := []struct { + name string + updateCallbackFunc clientgotesting.ReactionFunc + createInput *input + updateInput input + expected expected + }{ + { + name: "update - usual request", + updateCallbackFunc: func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { + return false, ret, nil + }, + createInput: &sampleCreateInput, + updateInput: input{ + apiOp: &types.APIRequest{ + Request: &http.Request{ + URL: &url.URL{}, + Method: http.MethodPut, + }, + Schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + }, + }, + Method: http.MethodPut, + }, + + schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + Attributes: map[string]interface{}{ + "namespaced": true, + }, + }, + }, + params: types.APIObject{ + Object: map[string]interface{}{ + "apiVersion": "v2", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "testing-secret", + "namespace": "testing-ns", + "resourceVersion": "1", + }, + }, + }, + }, + expected: expected{ + value: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "v2", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "testing-secret", + "namespace": "testing-ns", + "resourceVersion": "1", + }, + }}, + warning: []types.Warning{}, + err: nil, + }, + }, + { + name: "update - missing resource version", + updateCallbackFunc: func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { + return false, ret, nil + }, + updateInput: input{ + apiOp: &types.APIRequest{ + Request: &http.Request{ + URL: &url.URL{}, + Method: http.MethodPut, + }, + Schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + }, + }, + Method: http.MethodPut, + }, + + schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + Attributes: map[string]interface{}{ + "version": "v1", + "kind": "Secret", + "namespaced": true, + }, + }, + }, + params: types.APIObject{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "testing-secret", + "namespace": "testing-ns", + }, + }, + }, + }, + expected: expected{ + value: nil, + warning: nil, + err: fmt.Errorf(errResourceVersionRequired), + }, + }, + { + name: "update - missing apiVersion (should copy from schema)", + updateCallbackFunc: func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { + return false, ret, nil + }, + createInput: &sampleCreateInput, + updateInput: input{ + apiOp: &types.APIRequest{ + Request: &http.Request{ + URL: &url.URL{}, + Method: http.MethodPut, + }, + Schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + }, + }, + Method: http.MethodPut, + }, + + schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + Attributes: map[string]interface{}{ + "version": "v2", + "namespaced": true, + }, + }, + }, + params: types.APIObject{ + Object: map[string]interface{}{ + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "testing-secret", + "namespace": "testing-ns", + "resourceVersion": "1", + }, + }, + }, + }, + expected: expected{ + value: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "v2", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "testing-secret", + "namespace": "testing-ns", + "resourceVersion": "1", + }, + }}, + warning: []types.Warning{}, + err: nil, + }, + }, + { + name: "update - missing kind (should copy from schema)", + updateCallbackFunc: func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { + return false, ret, nil + }, + createInput: &sampleCreateInput, + updateInput: input{ + apiOp: &types.APIRequest{ + Request: &http.Request{ + URL: &url.URL{}, + Method: http.MethodPut, + }, + Schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + }, + }, + Method: http.MethodPut, + }, + + schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + Attributes: map[string]interface{}{ + "kind": "Secret", + "namespaced": true, + }, + }, + }, + params: types.APIObject{ + Object: map[string]interface{}{ + "apiVersion": "v2", + "metadata": map[string]interface{}{ + "name": "testing-secret", + "namespace": "testing-ns", + "resourceVersion": "1", + }, + }, + }, + }, + expected: expected{ + value: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "v2", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "testing-secret", + "namespace": "testing-ns", + "resourceVersion": "1", + }, + }}, + warning: []types.Warning{}, + err: nil, + }, + }, + { + name: "update - error request", + updateCallbackFunc: func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, apierrors.NewUnauthorized("sample reason") + }, + createInput: &sampleCreateInput, + updateInput: input{ + apiOp: &types.APIRequest{ + Request: &http.Request{ + URL: &url.URL{}, + Method: http.MethodPut, + }, + Schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + }, + }, + Method: http.MethodPut, + }, + + schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + Attributes: map[string]interface{}{ + "kind": "Secret", + "namespaced": true, + }, + }, + }, + params: types.APIObject{ + Object: map[string]interface{}{ + "apiVersion": "v2", + "metadata": map[string]interface{}{ + "name": "testing-secret", + "namespace": "testing-ns", + "resourceVersion": "1", + }, + }, + }, + }, + expected: expected{ + value: nil, + warning: nil, + err: apierrors.NewUnauthorized("sample reason"), + }, + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + testClientFactory, err := client.NewFactory(&rest.Config{}, false) + assert.NoError(t, err) + + fakeClient := fake.NewSimpleDynamicClient(runtime.NewScheme()) + + if tt.updateCallbackFunc != nil { + fakeClient.PrependReactor("update", "*", tt.updateCallbackFunc) + } + + testStore := Store{ + clientGetter: &testFactory{Factory: testClientFactory, + fakeClient: fakeClient, + }, + } + + // Creating the object first, so we can update it later (this function is not the SUT) + if tt.createInput != nil { + _, _, err = testStore.Create(tt.createInput.apiOp, tt.createInput.schema, tt.createInput.params) + assert.NoError(t, err) + } + + value, warning, err := testStore.Update(tt.updateInput.apiOp, tt.updateInput.schema, tt.updateInput.params, tt.updateInput.id) + + assert.Equal(t, tt.expected.value, value) + assert.Equal(t, tt.expected.warning, warning) + assert.Equal(t, tt.expected.err, err) + }) + } +} diff --git a/pkg/stores/sqlproxy/proxy_store.go b/pkg/stores/sqlproxy/proxy_store.go index c975973e..62132e45 100644 --- a/pkg/stores/sqlproxy/proxy_store.go +++ b/pkg/stores/sqlproxy/proxy_store.go @@ -50,8 +50,9 @@ import ( ) const ( - watchTimeoutEnv = "CATTLE_WATCH_TIMEOUT_SECONDS" - errNamespaceRequired = "metadata.namespace or apiOp.namespace are required" + watchTimeoutEnv = "CATTLE_WATCH_TIMEOUT_SECONDS" + errNamespaceRequired = "metadata.namespace or apiOp.namespace are required" + errResourceVersionRequired = "metadata.resourceVersion is required for update" ) var ( @@ -528,7 +529,15 @@ func (s *Store) Create(apiOp *types.APIRequest, schema *types.APISchema, params } gvk := attributes.GVK(schema) - input["apiVersion"], input["kind"] = gvk.ToAPIVersionAndKind() + apiVersion, kind := gvk.ToAPIVersionAndKind() + + if value, found := input["apiVersion"]; !found || value == "" { + input["apiVersion"] = apiVersion + } + + if value, found := input["kind"]; !found || value == "" { + input["kind"] = kind + } buffer := WarningBuffer{} k8sClient, err := metricsStore.Wrap(s.clientGetter.TableClient(apiOp, schema, namespace, &buffer)) @@ -598,7 +607,18 @@ func (s *Store) Update(apiOp *types.APIRequest, schema *types.APISchema, params resourceVersion := input.String("metadata", "resourceVersion") if resourceVersion == "" { - return nil, nil, fmt.Errorf("metadata.resourceVersion is required for update") + return nil, nil, fmt.Errorf(errResourceVersionRequired) + } + + gvk := attributes.GVK(schema) + apiVersion, kind := gvk.ToAPIVersionAndKind() + + if value, found := input["apiVersion"]; !found || value == "" { + input["apiVersion"] = apiVersion + } + + if value, found := input["kind"]; !found || value == "" { + input["kind"] = kind } opts := metav1.UpdateOptions{} diff --git a/pkg/stores/sqlproxy/proxy_store_test.go b/pkg/stores/sqlproxy/proxy_store_test.go index ec1cb501..336d2f8e 100644 --- a/pkg/stores/sqlproxy/proxy_store_test.go +++ b/pkg/stores/sqlproxy/proxy_store_test.go @@ -1104,3 +1104,347 @@ func TestCreate(t *testing.T) { }) } } + +func TestUpdate(t *testing.T) { + type input struct { + apiOp *types.APIRequest + schema *types.APISchema + params types.APIObject + id string + } + + type expected struct { + value *unstructured.Unstructured + warning []types.Warning + err error + } + + sampleCreateInput := input{ + apiOp: &types.APIRequest{ + Request: &http.Request{ + URL: &url.URL{}, + Method: http.MethodPut, + }, + Schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + }, + }, + Method: http.MethodPut, + }, + schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + Attributes: map[string]interface{}{ + "version": "v1", + "namespaced": true, + }, + }, + }, + params: types.APIObject{ + Object: map[string]interface{}{ + "kind": "Secret", + "apiVersion": "v1", + "metadata": map[string]interface{}{ + "name": "testing-secret", + "namespace": "testing-ns", + "resourceVersion": "1", + }, + }, + }, + } + + testCases := []struct { + name string + updateCallbackFunc clientgotesting.ReactionFunc + createInput *input + updateInput input + expected expected + }{ + { + name: "update - usual request", + updateCallbackFunc: func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { + return false, ret, nil + }, + createInput: &sampleCreateInput, + updateInput: input{ + apiOp: &types.APIRequest{ + Request: &http.Request{ + URL: &url.URL{}, + Method: http.MethodPut, + }, + Schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + }, + }, + Method: http.MethodPut, + }, + + schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + Attributes: map[string]interface{}{ + "namespaced": true, + }, + }, + }, + params: types.APIObject{ + Object: map[string]interface{}{ + "apiVersion": "v2", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "testing-secret", + "namespace": "testing-ns", + "resourceVersion": "1", + }, + }, + }, + }, + expected: expected{ + value: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "v2", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "testing-secret", + "namespace": "testing-ns", + "resourceVersion": "1", + }, + }}, + warning: []types.Warning{}, + err: nil, + }, + }, + { + name: "update - missing resource version", + updateCallbackFunc: func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { + return false, ret, nil + }, + updateInput: input{ + apiOp: &types.APIRequest{ + Request: &http.Request{ + URL: &url.URL{}, + Method: http.MethodPut, + }, + Schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + }, + }, + Method: http.MethodPut, + }, + + schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + Attributes: map[string]interface{}{ + "version": "v1", + "kind": "Secret", + "namespaced": true, + }, + }, + }, + params: types.APIObject{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "testing-secret", + "namespace": "testing-ns", + }, + }, + }, + }, + expected: expected{ + value: nil, + warning: nil, + err: fmt.Errorf(errResourceVersionRequired), + }, + }, + { + name: "update - missing apiVersion (should copy from schema)", + updateCallbackFunc: func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { + return false, ret, nil + }, + createInput: &sampleCreateInput, + updateInput: input{ + apiOp: &types.APIRequest{ + Request: &http.Request{ + URL: &url.URL{}, + Method: http.MethodPut, + }, + Schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + }, + }, + Method: http.MethodPut, + }, + + schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + Attributes: map[string]interface{}{ + "version": "v2", + "namespaced": true, + }, + }, + }, + params: types.APIObject{ + Object: map[string]interface{}{ + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "testing-secret", + "namespace": "testing-ns", + "resourceVersion": "1", + }, + }, + }, + }, + expected: expected{ + value: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "v2", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "testing-secret", + "namespace": "testing-ns", + "resourceVersion": "1", + }, + }}, + warning: []types.Warning{}, + err: nil, + }, + }, + { + name: "update - missing kind (should copy from schema)", + updateCallbackFunc: func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { + return false, ret, nil + }, + createInput: &sampleCreateInput, + updateInput: input{ + apiOp: &types.APIRequest{ + Request: &http.Request{ + URL: &url.URL{}, + Method: http.MethodPut, + }, + Schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + }, + }, + Method: http.MethodPut, + }, + + schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + Attributes: map[string]interface{}{ + "kind": "Secret", + "namespaced": true, + }, + }, + }, + params: types.APIObject{ + Object: map[string]interface{}{ + "apiVersion": "v2", + "metadata": map[string]interface{}{ + "name": "testing-secret", + "namespace": "testing-ns", + "resourceVersion": "1", + }, + }, + }, + }, + expected: expected{ + value: &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "v2", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "testing-secret", + "namespace": "testing-ns", + "resourceVersion": "1", + }, + }}, + warning: []types.Warning{}, + err: nil, + }, + }, + { + name: "update - error request", + updateCallbackFunc: func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, apierrors.NewUnauthorized("sample reason") + }, + createInput: &sampleCreateInput, + updateInput: input{ + apiOp: &types.APIRequest{ + Request: &http.Request{ + URL: &url.URL{}, + Method: http.MethodPut, + }, + Schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + }, + }, + Method: http.MethodPut, + }, + + schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + Attributes: map[string]interface{}{ + "kind": "Secret", + "namespaced": true, + }, + }, + }, + params: types.APIObject{ + Object: map[string]interface{}{ + "apiVersion": "v2", + "metadata": map[string]interface{}{ + "name": "testing-secret", + "namespace": "testing-ns", + "resourceVersion": "1", + }, + }, + }, + }, + expected: expected{ + value: nil, + warning: nil, + err: apierrors.NewUnauthorized("sample reason"), + }, + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + testClientFactory, err := client.NewFactory(&rest.Config{}, false) + assert.NoError(t, err) + + fakeClient := fake.NewSimpleDynamicClient(runtime.NewScheme()) + + if tt.updateCallbackFunc != nil { + fakeClient.PrependReactor("update", "*", tt.updateCallbackFunc) + } + + testStore := Store{ + clientGetter: &testFactory{Factory: testClientFactory, + fakeClient: fakeClient, + }, + } + + // Creating the object first, so we can update it later (this function is not the SUT) + if tt.createInput != nil { + _, _, err = testStore.Create(tt.createInput.apiOp, tt.createInput.schema, tt.createInput.params) + assert.NoError(t, err) + } + + value, warning, err := testStore.Update(tt.updateInput.apiOp, tt.updateInput.schema, tt.updateInput.params, tt.updateInput.id) + + assert.Equal(t, tt.expected.value, value) + assert.Equal(t, tt.expected.warning, warning) + assert.Equal(t, tt.expected.err, err) + }) + } +}