feat: apply overrides before creating the work (#748)

Azure · Apr 8, 2024 · 649696e · 649696e
1 parent ac9a94e
commit 649696e
Show file tree

Hide file tree

Showing 18 changed files with 2,089 additions and 87 deletions.
diff --git a/apis/placement/v1alpha1/override_types.go b/apis/placement/v1alpha1/override_types.go
@@ -6,6 +6,7 @@ Licensed under the MIT license.
 package v1alpha1
 
 import (
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1"
@@ -67,7 +68,9 @@ type OverridePolicy struct {
 type OverrideRule struct {
 	// ClusterSelectors selects the target clusters.
 	// The resources will be overridden before applying to the matching clusters.
-	// If ClusterSelector is not set, it means selecting ALL the member clusters.
+	// An empty clusterSelector selects ALL the member clusters.
+	// A nil clusterSelector selects NO member clusters.
+	// For now, only labelSelector is supported.
 	// +optional
 	ClusterSelector *placementv1beta1.ClusterSelector `json:"clusterSelector,omitempty"`
 
@@ -139,17 +142,17 @@ type ResourceSelector struct {
 // JSONPatchOverride applies a JSON patch on the selected resources following [RFC 6902](https://datatracker.ietf.org/doc/html/rfc6902).
 type JSONPatchOverride struct {
 	// Operator defines the operation on the target field.
-	// +kubebuilder:validation:Enum=Add;Remove;Replace
+	// +kubebuilder:validation:Enum=add;remove;replace
 	// +required
-	Operator JSONPatchOverrideOperator `json:"operator"`
+	Operator JSONPatchOverrideOperator `json:"op"`
 	// Path defines the target location.
 	// Note: override will fail if the resource path does not exist.
 	// +required
 	Path string `json:"path"`
 	// Value defines the content to be applied on the target location.
-	// Value should be empty when operator is Remove.
+	// Value should be empty when operator is `remove`.
 	// +optional
-	Value string `json:"value,omitempty"`
+	Value apiextensionsv1.JSON `json:"value,omitempty"`
 }
 
 // JSONPatchOverrideOperator defines the supported JSON patch operator.
@@ -164,13 +167,13 @@ const (
 	//   A JSON Patch override:
 	//
 	//   [
-	//     { "operator": "add", "path": "/foo/1", "value": "qux" }
+	//     { "op": "add", "path": "/foo/1", "value": "qux" }
 	//   ]
 	//
 	//   The resulting JSON document:
 	//
 	//   { "foo": [ "bar", "qux", "baz" ] }
-	JSONPatchOverrideOpAdd JSONPatchOverrideOperator = "Add"
+	JSONPatchOverrideOpAdd JSONPatchOverrideOperator = "add"
 	// JSONPatchOverrideOpRemove removes the value from the target location.
 	// An example target JSON document:
 	//
@@ -181,13 +184,13 @@ const (
 	//   A JSON Patch override:
 	//
 	//   [
-	//     { "operator": "remove", "path": "/baz" }
+	//     { "op": "remove", "path": "/baz" }
 	//   ]
 	//
 	//   The resulting JSON document:
 	//
 	//   { "foo": "bar" }
-	JSONPatchOverrideOpRemove JSONPatchOverrideOperator = "Remove"
+	JSONPatchOverrideOpRemove JSONPatchOverrideOperator = "remove"
 	// JSONPatchOverrideOpReplace replaces the value at the target location with a new value.
 	// An example target JSON document:
 	//
@@ -199,7 +202,7 @@ const (
 	//   A JSON Patch override:
 	//
 	//   [
-	//     { "operator": "replace", "path": "/baz", "value": "boo" }
+	//     { "op": "replace", "path": "/baz", "value": "boo" }
 	//   ]
 	//
 	//   The resulting JSON document:
@@ -208,7 +211,7 @@ const (
 	//     "baz": "boo",
 	//     "foo": "bar"
 	//   }
-	JSONPatchOverrideOpReplace JSONPatchOverrideOperator = "Replace"
+	JSONPatchOverrideOpReplace JSONPatchOverrideOperator = "replace"
 )
 
 // ClusterResourceOverrideList contains a list of ClusterResourceOverride.

diff --git a/apis/placement/v1alpha1/zz_generated.deepcopy.go b/apis/placement/v1alpha1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverrides.yaml b/config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverrides.yaml
@@ -139,8 +139,9 @@ spec:
                         clusterSelector:
                           description: ClusterSelectors selects the target clusters.
                             The resources will be overridden before applying to the
-                            matching clusters. If ClusterSelector is not set, it means
-                            selecting ALL the member clusters.
+                            matching clusters. An empty clusterSelector selects ALL
+                            the member clusters. A nil clusterSelector selects NO
+                            member clusters. For now, only labelSelector is supported.
                           properties:
                             clusterSelectorTerms:
                               description: ClusterSelectorTerms is a list of cluster
@@ -294,13 +295,13 @@ spec:
                             description: JSONPatchOverride applies a JSON patch on
                               the selected resources following [RFC 6902](https://datatracker.ietf.org/doc/html/rfc6902).
                             properties:
-                              operator:
+                              op:
                                 description: Operator defines the operation on the
                                   target field.
                                 enum:
-                                - Add
-                                - Remove
-                                - Replace
+                                - add
+                                - remove
+                                - replace
                                 type: string
                               path:
                                 description: 'Path defines the target location. Note:
@@ -310,10 +311,10 @@ spec:
                               value:
                                 description: Value defines the content to be applied
                                   on the target location. Value should be empty when
-                                  operator is Remove.
-                                type: string
+                                  operator is `remove`.
+                                x-kubernetes-preserve-unknown-fields: true
                             required:
-                            - operator
+                            - op
                             - path
                             type: object
                           maxItems: 20

diff --git a/config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverridesnapshots.yaml b/config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverridesnapshots.yaml
@@ -154,8 +154,10 @@ spec:
                             clusterSelector:
                               description: ClusterSelectors selects the target clusters.
                                 The resources will be overridden before applying to
-                                the matching clusters. If ClusterSelector is not set,
-                                it means selecting ALL the member clusters.
+                                the matching clusters. An empty clusterSelector selects
+                                ALL the member clusters. A nil clusterSelector selects
+                                NO member clusters. For now, only labelSelector is
+                                supported.
                               properties:
                                 clusterSelectorTerms:
                                   description: ClusterSelectorTerms is a list of cluster
@@ -315,13 +317,13 @@ spec:
                                 description: JSONPatchOverride applies a JSON patch
                                   on the selected resources following [RFC 6902](https://datatracker.ietf.org/doc/html/rfc6902).
                                 properties:
-                                  operator:
+                                  op:
                                     description: Operator defines the operation on
                                       the target field.
                                     enum:
-                                    - Add
-                                    - Remove
-                                    - Replace
+                                    - add
+                                    - remove
+                                    - replace
                                     type: string
                                   path:
                                     description: 'Path defines the target location.
@@ -331,10 +333,10 @@ spec:
                                   value:
                                     description: Value defines the content to be applied
                                       on the target location. Value should be empty
-                                      when operator is Remove.
-                                    type: string
+                                      when operator is `remove`.
+                                    x-kubernetes-preserve-unknown-fields: true
                                 required:
-                                - operator
+                                - op
                                 - path
                                 type: object
                               maxItems: 20

diff --git a/config/crd/bases/placement.kubernetes-fleet.io_resourceoverrides.yaml b/config/crd/bases/placement.kubernetes-fleet.io_resourceoverrides.yaml
@@ -55,8 +55,9 @@ spec:
                         clusterSelector:
                           description: ClusterSelectors selects the target clusters.
                             The resources will be overridden before applying to the
-                            matching clusters. If ClusterSelector is not set, it means
-                            selecting ALL the member clusters.
+                            matching clusters. An empty clusterSelector selects ALL
+                            the member clusters. A nil clusterSelector selects NO
+                            member clusters. For now, only labelSelector is supported.
                           properties:
                             clusterSelectorTerms:
                               description: ClusterSelectorTerms is a list of cluster
@@ -210,13 +211,13 @@ spec:
                             description: JSONPatchOverride applies a JSON patch on
                               the selected resources following [RFC 6902](https://datatracker.ietf.org/doc/html/rfc6902).
                             properties:
-                              operator:
+                              op:
                                 description: Operator defines the operation on the
                                   target field.
                                 enum:
-                                - Add
-                                - Remove
-                                - Replace
+                                - add
+                                - remove
+                                - replace
                                 type: string
                               path:
                                 description: 'Path defines the target location. Note:
@@ -226,10 +227,10 @@ spec:
                               value:
                                 description: Value defines the content to be applied
                                   on the target location. Value should be empty when
-                                  operator is Remove.
-                                type: string
+                                  operator is `remove`.
+                                x-kubernetes-preserve-unknown-fields: true
                             required:
-                            - operator
+                            - op
                             - path
                             type: object
                           maxItems: 20

diff --git a/config/crd/bases/placement.kubernetes-fleet.io_resourceoverridesnapshots.yaml b/config/crd/bases/placement.kubernetes-fleet.io_resourceoverridesnapshots.yaml
@@ -68,8 +68,10 @@ spec:
                             clusterSelector:
                               description: ClusterSelectors selects the target clusters.
                                 The resources will be overridden before applying to
-                                the matching clusters. If ClusterSelector is not set,
-                                it means selecting ALL the member clusters.
+                                the matching clusters. An empty clusterSelector selects
+                                ALL the member clusters. A nil clusterSelector selects
+                                NO member clusters. For now, only labelSelector is
+                                supported.
                               properties:
                                 clusterSelectorTerms:
                                   description: ClusterSelectorTerms is a list of cluster
@@ -229,13 +231,13 @@ spec:
                                 description: JSONPatchOverride applies a JSON patch
                                   on the selected resources following [RFC 6902](https://datatracker.ietf.org/doc/html/rfc6902).
                                 properties:
-                                  operator:
+                                  op:
                                     description: Operator defines the operation on
                                       the target field.
                                     enum:
-                                    - Add
-                                    - Remove
-                                    - Replace
+                                    - add
+                                    - remove
+                                    - replace
                                     type: string
                                   path:
                                     description: 'Path defines the target location.
@@ -245,10 +247,10 @@ spec:
                                   value:
                                     description: Value defines the content to be applied
                                       on the target location. Value should be empty
-                                      when operator is Remove.
-                                    type: string
+                                      when operator is `remove`.
+                                    x-kubernetes-preserve-unknown-fields: true
                                 required:
-                                - operator
+                                - op
                                 - path
                                 type: object
                               maxItems: 20

diff --git a/go.mod b/go.mod
@@ -7,6 +7,7 @@ require (
 	github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.3.1
 	github.com/Azure/karpenter v0.2.0
 	github.com/crossplane/crossplane-runtime v0.20.1
+	github.com/evanphx/json-patch/v5 v5.9.0
 	github.com/go-logr/logr v1.3.0
 	github.com/google/go-cmp v0.6.0
 	github.com/onsi/ginkgo/v2 v2.13.0
@@ -45,7 +46,6 @@ require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/emicklei/go-restful/v3 v3.11.0 // indirect
 	github.com/evanphx/json-patch v5.6.0+incompatible // indirect
-	github.com/evanphx/json-patch/v5 v5.7.0 // indirect
 	github.com/fsnotify/fsnotify v1.6.0 // indirect
 	github.com/go-logr/zapr v1.3.0 // indirect
 	github.com/go-openapi/jsonpointer v0.20.0 // indirect

diff --git a/go.sum b/go.sum
@@ -52,8 +52,8 @@ github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxER
 github.com/emicklei/go-restful/v3 v3.11.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc=
 github.com/evanphx/json-patch v5.6.0+incompatible h1:jBYDEEiFBPxA0v50tFdvOzQQTCvpL6mnFh5mB2/l16U=
 github.com/evanphx/json-patch v5.6.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
-github.com/evanphx/json-patch/v5 v5.7.0 h1:nJqP7uwL84RJInrohHfW0Fx3awjbm8qZeFv0nW9SYGc=
-github.com/evanphx/json-patch/v5 v5.7.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq14uClGH4abBuQ=
+github.com/evanphx/json-patch/v5 v5.9.0 h1:kcBlZQbplgElYIlo/n1hJbls2z/1awpXxpRi0/FOJfg=
+github.com/evanphx/json-patch/v5 v5.9.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq14uClGH4abBuQ=
 github.com/felixge/httpsnoop v1.0.3 h1:s/nj+GCswXYzN5v2DpNMuMQYe+0DDwt5WVCU6CWBdXk=
 github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY=
 github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbSClcnxKAGw=

diff --git a/pkg/controllers/overrider/common_test.go b/pkg/controllers/overrider/common_test.go
@@ -15,6 +15,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -50,7 +51,7 @@ func getClusterResourceOverrideSpec() fleetv1alpha1.ClusterResourceOverrideSpec
 						{
 							Operator: fleetv1alpha1.JSONPatchOverrideOpReplace,
 							Path:     "spec.replica",
-							Value:    "3",
+							Value:    apiextensionsv1.JSON{Raw: []byte("3")},
 						},
 					},
 				},

diff --git a/pkg/controllers/overrider/resource_controller_integration_test.go b/pkg/controllers/overrider/resource_controller_integration_test.go
@@ -12,6 +12,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -39,7 +40,7 @@ func getResourceOverrideSpec() fleetv1alpha1.ResourceOverrideSpec {
 						{
 							Operator: fleetv1alpha1.JSONPatchOverrideOpReplace,
 							Path:     "spec.replica",
-							Value:    "3",
+							Value:    apiextensionsv1.JSON{Raw: []byte("3")},
 						},
 					},
 				},