From 583c75a44d3d24ddead357b7e6276f239aba5a11 Mon Sep 17 00:00:00 2001 From: Andrei Alexandru Date: Thu, 1 Jun 2023 12:13:53 +0300 Subject: [PATCH] Add spec wrapper for cluster patch (#60) Co-authored-by: aalexandru --- pkg/apiserver/web/handler/v2/handler.go | 34 ++++++++++-------- pkg/apiserver/web/handler/v2/handler_test.go | 37 ++++++++++---------- 2 files changed, 38 insertions(+), 33 deletions(-) diff --git a/pkg/apiserver/web/handler/v2/handler.go b/pkg/apiserver/web/handler/v2/handler.go index 3ee5db5a..8a1083b4 100644 --- a/pkg/apiserver/web/handler/v2/handler.go +++ b/pkg/apiserver/web/handler/v2/handler.go @@ -41,11 +41,15 @@ type Handler interface { Register(*echo.Group) } -// ClusterPatch is the struct for updating a cluster's dynamic fields +// ClusterSpec is the struct for updating a cluster's dynamic fields +type ClusterSpec struct { + Status *string `json:"status,omitempty" validate:"omitempty,oneof=Inactive Active Deprecated Deleted"` + Phase *string `json:"phase,omitempty" validate:"omitempty,oneof=Building Testing Running Upgrading"` + Tags *map[string]string `json:"tags,omitempty" validate:"omitempty"` +} + type ClusterPatch struct { - Status string `json:"status" validate:"omitempty,oneof=Inactive Active Deprecated Deleted"` - Phase string `json:"phase" validate:"omitempty,oneof=Building Testing Running Upgrading"` - Tags map[string]string `json:"tags" validate:"omitempty"` + Spec ClusterSpec `json:"spec" validate:"required"` } // handler struct @@ -162,7 +166,7 @@ func (h *handler) ListClusters(c echo.Context) error { // @Accept json // @Produce json // @Param name path string true "Name of the cluster to patch" -// @Param clusterPatch body ClusterPatch true "Request body" +// @Param clusterSpec body ClusterSpec true "Request body" // @Success 200 {object} registryv1.ClusterSpec // @Failure 400 {object} errors.Error // @Failure 500 {object} errors.Error @@ -181,17 +185,17 @@ func (h *handler) PatchCluster(c echo.Context) error { return c.JSON(http.StatusNotFound, errors.NotFound()) } - var clusterPatch ClusterPatch + var clusterSpec ClusterSpec - if err = c.Bind(&clusterPatch); err != nil { + if err = c.Bind(&clusterSpec); err != nil { return c.JSON(http.StatusBadRequest, errors.NewError(err)) } - if err = clusterPatch.Validate(c); err != nil { + if err = clusterSpec.Validate(c); err != nil { return c.JSON(http.StatusBadRequest, errors.NewError(err)) } - err = h.patchCluster(cluster, clusterPatch) + err = h.patchCluster(cluster, clusterSpec) if err != nil { return c.JSON(http.StatusInternalServerError, errors.NewError(err)) } @@ -225,13 +229,13 @@ func (h *handler) getCluster(db database.Db, name string) (*registryv1.Cluster, } // patchCluster -func (h *handler) patchCluster(cluster *registryv1.Cluster, patch ClusterPatch) error { +func (h *handler) patchCluster(cluster *registryv1.Cluster, spec ClusterSpec) error { client, err := h.kcp.GetClient(h.appConfig, cluster) if err != nil { return fmt.Errorf("failed to get client for cluster %s: %v", cluster.Spec.Name, err) } - jsonPatch, err := json.Marshal(patch) + patch, err := json.Marshal(&ClusterPatch{Spec: spec}) if err != nil { return err } @@ -242,7 +246,7 @@ func (h *handler) patchCluster(cluster *registryv1.Cluster, patch ClusterPatch) Namespace("cluster-registry"). Resource("clusters"). Name(cluster.Spec.Name). - Body(jsonPatch). + Body(patch). DoRaw(context.TODO()) log.Debugf("Patch response: %s", string(res)) @@ -263,13 +267,13 @@ func getQueryConditions(c echo.Context) []string { return []string{} } -func (patch *ClusterPatch) Validate(c echo.Context) error { +func (patch *ClusterSpec) Validate(c echo.Context) error { if err := c.Validate(patch); err != nil { return err } - if len(patch.Tags) > 0 { - for key, value := range patch.Tags { + if patch.Tags != nil && len(*patch.Tags) > 0 { + for key, value := range *patch.Tags { if err := validateTag(key, value); err != nil { return err } diff --git a/pkg/apiserver/web/handler/v2/handler_test.go b/pkg/apiserver/web/handler/v2/handler_test.go index a207b913..0b32f0ee 100644 --- a/pkg/apiserver/web/handler/v2/handler_test.go +++ b/pkg/apiserver/web/handler/v2/handler_test.go @@ -28,6 +28,7 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/client-go/kubernetes" testclient "k8s.io/client-go/kubernetes/fake" + "k8s.io/utils/pointer" "net/http" "net/http/httptest" "strings" @@ -269,7 +270,7 @@ func TestPatchCluster(t *testing.T) { tcs := []struct { name string cluster *registryv1.Cluster - clusterPatch ClusterPatch + clusterSpec ClusterSpec expectedStatus int expectedBody string }{ @@ -285,11 +286,11 @@ func TestPatchCluster(t *testing.T) { Tags: map[string]string{"onboarding": "on", "scaling": "off"}, }, }, - clusterPatch: ClusterPatch{ - Status: "inactive", + clusterSpec: ClusterSpec{ + Status: pointer.String("inactive"), }, expectedStatus: http.StatusBadRequest, - expectedBody: `{"errors":{"body":"Key: 'ClusterPatch.Status' Error:Field validation for 'Status' failed on the 'oneof' tag"}}`, + expectedBody: `{"errors":{"body":"Key: 'ClusterSpec.Status' Error:Field validation for 'Status' failed on the 'oneof' tag"}}`, }, { name: "invalid phase (case sensitive)", @@ -303,12 +304,12 @@ func TestPatchCluster(t *testing.T) { Tags: map[string]string{"onboarding": "on", "scaling": "off"}, }, }, - clusterPatch: ClusterPatch{ - Status: "Inactive", - Phase: "upgrading", + clusterSpec: ClusterSpec{ + Status: pointer.String("Inactive"), + Phase: pointer.String("upgrading"), }, expectedStatus: http.StatusBadRequest, - expectedBody: `{"errors":{"body":"Key: 'ClusterPatch.Phase' Error:Field validation for 'Phase' failed on the 'oneof' tag"}}`, + expectedBody: `{"errors":{"body":"Key: 'ClusterSpec.Phase' Error:Field validation for 'Phase' failed on the 'oneof' tag"}}`, }, { name: "invalid value for `scaling` tag", @@ -322,9 +323,9 @@ func TestPatchCluster(t *testing.T) { Tags: map[string]string{"onboarding": "on", "scaling": "off"}, }, }, - clusterPatch: ClusterPatch{ - Status: "Inactive", - Tags: map[string]string{ + clusterSpec: ClusterSpec{ + Status: pointer.String("Inactive"), + Tags: &map[string]string{ "onboarding": "off", "scaling": "enabled", }, @@ -344,9 +345,9 @@ func TestPatchCluster(t *testing.T) { Tags: map[string]string{"onboarding": "on", "scaling": "off"}, }, }, - clusterPatch: ClusterPatch{ - Status: "Inactive", - Tags: map[string]string{ + clusterSpec: ClusterSpec{ + Status: pointer.String("Inactive"), + Tags: &map[string]string{ "onboarding": "false", }, }, @@ -365,9 +366,9 @@ func TestPatchCluster(t *testing.T) { Tags: map[string]string{"onboarding": "on", "scaling": "off"}, }, }, - clusterPatch: ClusterPatch{ - Status: "Inactive", - Tags: map[string]string{ + clusterSpec: ClusterSpec{ + Status: pointer.String("Inactive"), + Tags: &map[string]string{ "some-made-up-tag": "on", }, }, @@ -381,7 +382,7 @@ func TestPatchCluster(t *testing.T) { r := web.NewRouter() h := NewHandler(appConfig, db, m, &TestClientProvider{}) - patch, _ := json.Marshal(tc.clusterPatch) + patch, _ := json.Marshal(tc.clusterSpec) body := strings.NewReader(string(patch)) req := httptest.NewRequest(echo.PATCH, "/api/v2/clusters/:name", body) req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON)