Make spec.namespace fields immutable (#418)

* Make spec.namespace fields immutable Moving a control plane around has consequences beyond just the templates installed by helm, as the control plane namespace has its own semantics within Istio: it is often the place where you can put Istio configs that serve as defaults. By making the field immutable, we avoid strange side- effects that might occur when moving a control plane to another namespace. Signed-off-by: Daniel Grimm <[email protected]> * Add docs Signed-off-by: Daniel Grimm <[email protected]> --------- Signed-off-by: Daniel Grimm <[email protected]>
istio-ecosystem · Oct 15, 2024 · d459300 · d459300
1 parent e6155da
commit d459300
Show file tree

Hide file tree

Showing 10 changed files with 29 additions and 4 deletions.
diff --git a/api/v1alpha1/istio_types.go b/api/v1alpha1/istio_types.go
@@ -57,9 +57,10 @@ type IstioSpec struct {
 	// +kubebuilder:validation:Enum=ambient;default;demo;empty;external;openshift-ambient;openshift;preview;stable
 	Profile string `json:"profile,omitempty"`
 
-	// Namespace to which the Istio components should be installed.
+	// Namespace to which the Istio components should be installed. Note that this field is immutable.
 	// +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:io.kubernetes:Namespace"}
 	// +kubebuilder:default=istio-system
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	Namespace string `json:"namespace"`
 
 	// Defines the values to be passed to the Helm charts when installing Istio.

diff --git a/api/v1alpha1/istiorevision_types.go b/api/v1alpha1/istiorevision_types.go
@@ -42,6 +42,7 @@ type IstioRevisionSpec struct {
 
 	// Namespace to which the Istio components should be installed.
 	// +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:io.kubernetes:Namespace"}
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	Namespace string `json:"namespace"`
 
 	// Defines the values to be passed to the Helm charts when installing Istio.

diff --git a/bundle/manifests/sailoperator.clusterserviceversion.yaml b/bundle/manifests/sailoperator.clusterserviceversion.yaml
@@ -34,7 +34,7 @@ metadata:
     capabilities: Seamless Upgrades
     categories: OpenShift Optional, Integration & Delivery, Networking, Security
     containerImage: quay.io/maistra-dev/sail-operator:0.2-latest
-    createdAt: "2024-10-15T05:04:53Z"
+    createdAt: "2024-10-15T06:09:50Z"
     description: Experimental operator for installing Istio service mesh
     features.operators.openshift.io/cnf: "false"
     features.operators.openshift.io/cni: "true"
@@ -273,6 +273,7 @@ spec:
         x-descriptors:
         - urn:alm:descriptor:com.tectonic.ui:booleanSwitch
       - description: Namespace to which the Istio components should be installed.
+          Note that this field is immutable.
         displayName: Namespace
         path: namespace
         x-descriptors:

diff --git a/bundle/manifests/sailoperator.io_istiorevisions.yaml b/bundle/manifests/sailoperator.io_istiorevisions.yaml
@@ -75,6 +75,9 @@ spec:
               namespace:
                 description: Namespace to which the Istio components should be installed.
                 type: string
+                x-kubernetes-validations:
+                - message: Value is immutable
+                  rule: self == oldSelf
               type:
                 default: Local
                 description: Type indicates whether this revision represents a local

diff --git a/bundle/manifests/sailoperator.io_istios.yaml b/bundle/manifests/sailoperator.io_istios.yaml
@@ -86,7 +86,11 @@ spec:
               namespace:
                 default: istio-system
                 description: Namespace to which the Istio components should be installed.
+                  Note that this field is immutable.
                 type: string
+                x-kubernetes-validations:
+                - message: Value is immutable
+                  rule: self == oldSelf
               profile:
                 description: |-
                   The built-in installation configuration profile to use.

diff --git a/chart/crds/sailoperator.io_istiorevisions.yaml b/chart/crds/sailoperator.io_istiorevisions.yaml
@@ -75,6 +75,9 @@ spec:
               namespace:
                 description: Namespace to which the Istio components should be installed.
                 type: string
+                x-kubernetes-validations:
+                - message: Value is immutable
+                  rule: self == oldSelf
               type:
                 default: Local
                 description: Type indicates whether this revision represents a local

diff --git a/chart/crds/sailoperator.io_istios.yaml b/chart/crds/sailoperator.io_istios.yaml
@@ -86,7 +86,11 @@ spec:
               namespace:
                 default: istio-system
                 description: Namespace to which the Istio components should be installed.
+                  Note that this field is immutable.
                 type: string
+                x-kubernetes-validations:
+                - message: Value is immutable
+                  rule: self == oldSelf
               profile:
                 description: |-
                   The built-in installation configuration profile to use.

diff --git a/docs/README.md b/docs/README.md
@@ -56,7 +56,7 @@ Sail Operator manages the lifecycle of your Istio control planes. Instead of cre
 ## Concepts
 
 ### Istio resource
-The `Istio` resource is used to manage your Istio control planes. It is a cluster-wide resource, as the Istio control plane operates in and requires access to the entire cluster. To select a namespace to run the control plane pods in, you can use the `spec.namespace` field. You can access all helm chart options through the `values` field in the `spec`:
+The `Istio` resource is used to manage your Istio control planes. It is a cluster-wide resource, as the Istio control plane operates in and requires access to the entire cluster. To select a namespace to run the control plane pods in, you can use the `spec.namespace` field. Note that this field is immutable, though: in order to move a control plane to another namespace, you have to remove the Istio resource and recreate it with a different `spec.namespace`. You can access all helm chart options through the `values` field in the `spec`:
 
 ```yaml
 apiVersion: sailoperator.io/v1alpha1

diff --git a/docs/api-reference/sailoperator.io.md b/docs/api-reference/sailoperator.io.md
@@ -909,7 +909,7 @@ _Appears in:_
 | `version` _string_ | Defines the version of Istio to install. Must be one of: v1.23.2, v1.22.5, v1.21.6, latest. | v1.23.2 | Enum: [v1.23.2 v1.22.5 v1.21.6 latest]   |
 | `updateStrategy` _[IstioUpdateStrategy](#istioupdatestrategy)_ | Defines the update strategy to use when the version in the Istio CR is updated. | \{ type:InPlace \} |  |
 | `profile` _string_ | The built-in installation configuration profile to use. The 'default' profile is always applied. On OpenShift, the 'openshift' profile is also applied on top of 'default'. Must be one of: ambient, default, demo, empty, external, openshift-ambient, openshift, preview, stable. |  | Enum: [ambient default demo empty external openshift-ambient openshift preview stable]   |
-| `namespace` _string_ | Namespace to which the Istio components should be installed. | istio-system |  |
+| `namespace` _string_ | Namespace to which the Istio components should be installed. Note that this field is immutable. | istio-system |  |
 | `values` _[Values](#values)_ | Defines the values to be passed to the Helm charts when installing Istio. |  |  |
 
 

diff --git a/tests/integration/api/istio_test.go b/tests/integration/api/istio_test.go
@@ -272,6 +272,14 @@ var _ = Describe("Istio resource", Ordered, func() {
 						deleteAllIstiosAndRevisions(ctx)
 					})
 
+					When("namespace is updated", func() {
+						It("throws a validation error as the field is immutable", func() {
+							Expect(k8sClient.Get(ctx, istioKey, istio)).To(Succeed())
+							istio.Spec.Namespace = workloadNamespace
+							Expect(k8sClient.Update(ctx, istio)).To(MatchError(ContainSubstring("immutable")))
+						})
+					})
+
 					When("version is updated", func() {
 						BeforeAll(func() {
 							Expect(k8sClient.Get(ctx, istioKey, istio)).To(Succeed())