From 99e479ba0f0883bf7337b3eba8a67200dcc7509c Mon Sep 17 00:00:00 2001 From: Felipe Gehrke Date: Fri, 13 Sep 2024 15:19:58 -0300 Subject: [PATCH] Adding Group, Version, Kind in Update proxy_store.go (#270) * adding gvk completion to update method in proxy_store --- pkg/stores/partition/store.go | 3 +- pkg/stores/proxy/proxy_store.go | 10 +- pkg/stores/proxy/proxy_store_test.go | 358 +++++++++++++++++++++++- pkg/stores/sqlproxy/proxy_store.go | 10 +- pkg/stores/sqlproxy/proxy_store_test.go | 354 +++++++++++++++++++++++ 5 files changed, 726 insertions(+), 9 deletions(-) diff --git a/pkg/stores/partition/store.go b/pkg/stores/partition/store.go index f90aabcd..c018fc4b 100644 --- a/pkg/stores/partition/store.go +++ b/pkg/stores/partition/store.go @@ -4,6 +4,7 @@ package partition import ( "context" + "errors" "fmt" "os" "reflect" @@ -389,7 +390,7 @@ func ToAPIEvent(apiOp *types.APIRequest, schema *types.APISchema, event watch.Ev if event.Type == watch.Error { status, _ := event.Object.(*metav1.Status) - apiEvent.Error = fmt.Errorf(status.Message) + apiEvent.Error = errors.New(status.Message) return apiEvent } diff --git a/pkg/stores/proxy/proxy_store.go b/pkg/stores/proxy/proxy_store.go index 7d0f7117..7faf6aee 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 ( @@ -515,9 +516,12 @@ 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, errors.New(errResourceVersionRequired) } + gvk := attributes.GVK(schema) + input["apiVersion"], input["kind"] = gvk.ToAPIVersionAndKind() + opts := metav1.UpdateOptions{} if err := decodeParams(apiOp, &opts); err != nil { return nil, nil, err diff --git a/pkg/stores/proxy/proxy_store_test.go b/pkg/stores/proxy/proxy_store_test.go index af90ef00..b1b488f9 100644 --- a/pkg/stores/proxy/proxy_store_test.go +++ b/pkg/stores/proxy/proxy_store_test.go @@ -343,7 +343,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 +466,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 +488,357 @@ 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.MethodPost, + }, + Schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + }, + }, + Method: http.MethodPost, + }, + schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + Attributes: map[string]interface{}{ + "kind": "Secret", + "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{}{ + "version": "v2", + "kind": "Secret", + "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 - different apiVersion and kind (params and schema) - 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", + "kind": "Secret", + "namespaced": true, + }, + }, + }, + params: types.APIObject{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "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 apiVersion and kind in params - 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.MethodPost, + }, + Schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + }, + }, + Method: http.MethodPost, + }, + + schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + Attributes: map[string]interface{}{ + "version": "v2", + "kind": "Secret", + "namespaced": true, + }, + }, + }, + params: types.APIObject{ + Object: map[string]interface{}{ + "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: errors.New(errResourceVersionRequired), + }, + }, + { + 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) + + if tt.expected.err != nil { + assert.Equal(t, tt.expected.err.Error(), err.Error()) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/pkg/stores/sqlproxy/proxy_store.go b/pkg/stores/sqlproxy/proxy_store.go index c975973e..d2ab50f6 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 ( @@ -598,9 +599,12 @@ 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, errors.New(errResourceVersionRequired) } + gvk := attributes.GVK(schema) + input["apiVersion"], input["kind"] = gvk.ToAPIVersionAndKind() + opts := metav1.UpdateOptions{} if err := decodeParams(apiOp, &opts); err != nil { return nil, nil, err diff --git a/pkg/stores/sqlproxy/proxy_store_test.go b/pkg/stores/sqlproxy/proxy_store_test.go index ec1cb501..54f31f0d 100644 --- a/pkg/stores/sqlproxy/proxy_store_test.go +++ b/pkg/stores/sqlproxy/proxy_store_test.go @@ -1104,3 +1104,357 @@ 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.MethodPost, + }, + Schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + }, + }, + Method: http.MethodPost, + }, + schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + Attributes: map[string]interface{}{ + "kind": "Secret", + "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{}{ + "version": "v2", + "kind": "Secret", + "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 - different apiVersion and kind (params and schema) - 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", + "kind": "Secret", + "namespaced": true, + }, + }, + }, + params: types.APIObject{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "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 apiVersion and kind in params - 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.MethodPost, + }, + Schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + }, + }, + Method: http.MethodPost, + }, + + schema: &types.APISchema{ + Schema: &schemas.Schema{ + ID: "testing", + Attributes: map[string]interface{}{ + "version": "v2", + "kind": "Secret", + "namespaced": true, + }, + }, + }, + params: types.APIObject{ + Object: map[string]interface{}{ + "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: errors.New(errResourceVersionRequired), + }, + }, + { + 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) + + if tt.expected.err != nil { + assert.Equal(t, tt.expected.err.Error(), err.Error()) + } else { + assert.NoError(t, err) + } + }) + } +}