Add spec wrapper for cluster patch (#60)

Co-authored-by: aalexandru <[email protected]>

Author: aalexandru

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
 			}

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)