Add defaulting for Istio and IstioCNI resources (istio-ecosystem#43)

Signed-off-by: Marko Lukša <mluksa@redhat.com>
twhite0 · Apr 15, 2024 · ed8576d · ed8576d
1 parent be689eb
commit ed8576d
Show file tree

Hide file tree

Showing 9 changed files with 108 additions and 16 deletions.
diff --git a/api/v1alpha1/istio_types.go b/api/v1alpha1/istio_types.go
@@ -40,10 +40,12 @@ type IstioSpec struct {
 	// Must be one of: v1.21.0, latest.
 	// +operator-sdk:csv:customresourcedefinitions:type=spec,order=1,displayName="Istio Version",xDescriptors={"urn:alm:descriptor:com.tectonic.ui:fieldGroup:General", "urn:alm:descriptor:com.tectonic.ui:select:v1.21.0", "urn:alm:descriptor:com.tectonic.ui:select:latest"}
 	// +kubebuilder:validation:Enum=v1.21.0;latest
+	// +kubebuilder:default=v1.21.0
 	Version string `json:"version"`
 
 	// Defines the update strategy to use when the version in the Istio CR is updated.
 	// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Update Strategy"
+	// +kubebuilder:default={type: "InPlace"}
 	UpdateStrategy *IstioUpdateStrategy `json:"updateStrategy,omitempty"`
 
 	// +sail:profile
@@ -57,6 +59,7 @@ type IstioSpec struct {
 
 	// Namespace to which the Istio components should be installed.
 	// +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:io.kubernetes:Namespace"}
+	// +kubebuilder:default=istio-system
 	Namespace string `json:"namespace"`
 
 	// Defines the values to be passed to the Helm charts when installing Istio.
@@ -77,6 +80,7 @@ type IstioUpdateStrategy struct {
 	// The "InPlace" strategy is the default.	TODO: change default to "RevisionBased"
 	// +operator-sdk:csv:customresourcedefinitions:type=spec,order=1,displayName="Type",xDescriptors={"urn:alm:descriptor:com.tectonic.ui:select:InPlace", "urn:alm:descriptor:com.tectonic.ui:select:RevisionBased"}
 	// +kubebuilder:validation:Enum=InPlace;RevisionBased
+	// +kubebuilder:default=InPlace
 	Type UpdateStrategyType `json:"type,omitempty"`
 
 	// Defines how many seconds the operator should wait before removing a non-active revision after all
@@ -249,7 +253,9 @@ type Istio struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 
-	Spec   IstioSpec   `json:"spec,omitempty"`
+	// +kubebuilder:default={version: "v1.21.0", namespace: "istio-system", updateStrategy: {type:"InPlace"}}
+	Spec IstioSpec `json:"spec,omitempty"`
+
 	Status IstioStatus `json:"status,omitempty"`
 }
 

diff --git a/api/v1alpha1/istiocni_types.go b/api/v1alpha1/istiocni_types.go
@@ -31,6 +31,7 @@ type IstioCNISpec struct {
 	// Must be one of: v1.21.0, latest.
 	// +operator-sdk:csv:customresourcedefinitions:type=spec,order=1,displayName="Istio Version",xDescriptors={"urn:alm:descriptor:com.tectonic.ui:fieldGroup:General", "urn:alm:descriptor:com.tectonic.ui:select:v1.21.0", "urn:alm:descriptor:com.tectonic.ui:select:latest"}
 	// +kubebuilder:validation:Enum=v1.21.0;latest
+	// +kubebuilder:default=v1.21.0
 	Version string `json:"version"`
 
 	// +sail:profile
@@ -44,6 +45,7 @@ type IstioCNISpec struct {
 
 	// Namespace to which the Istio CNI component should be installed.
 	// +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:io.kubernetes:Namespace"}
+	// +kubebuilder:default=istio-cni
 	Namespace string `json:"namespace"`
 
 	// Defines the values to be passed to the Helm charts when installing Istio CNI.
@@ -174,7 +176,9 @@ type IstioCNI struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 
-	Spec   IstioCNISpec   `json:"spec,omitempty"`
+	// +kubebuilder:default={version: "v1.21.0", namespace: "istio-cni"}
+	Spec IstioCNISpec `json:"spec,omitempty"`
+
 	Status IstioCNIStatus `json:"status,omitempty"`
 }
 

diff --git a/bundle/manifests/operator.istio.io_istiocnis.yaml b/bundle/manifests/operator.istio.io_istiocnis.yaml
@@ -56,9 +56,13 @@ spec:
           metadata:
             type: object
           spec:
+            default:
+              namespace: istio-cni
+              version: v1.21.0
             description: IstioCNISpec defines the desired state of IstioCNI
             properties:
               namespace:
+                default: istio-cni
                 description: Namespace to which the Istio CNI component should be
                   installed.
                 type: string
@@ -1367,6 +1371,7 @@ spec:
                     type: object
                 type: object
               version:
+                default: v1.21.0
                 description: |-
                   Defines the version of Istio to install.
                   Must be one of: v1.21.0, latest.

diff --git a/bundle/manifests/operator.istio.io_istios.yaml b/bundle/manifests/operator.istio.io_istios.yaml
@@ -72,9 +72,15 @@ spec:
           metadata:
             type: object
           spec:
+            default:
+              namespace: istio-system
+              updateStrategy:
+                type: InPlace
+              version: v1.21.0
             description: IstioSpec defines the desired state of Istio
             properties:
               namespace:
+                default: istio-system
                 description: Namespace to which the Istio components should be installed.
                 type: string
               profile:
@@ -95,6 +101,8 @@ spec:
                 - remote
                 type: string
               updateStrategy:
+                default:
+                  type: InPlace
                 description: Defines the update strategy to use when the version in
                   the Istio CR is updated.
                 properties:
@@ -107,6 +115,7 @@ spec:
                     minimum: 30
                     type: integer
                   type:
+                    default: InPlace
                     description: "Type of strategy to use. Can be \"InPlace\" or \"RevisionBased\".
                       When the \"InPlace\" strategy\nis used, the existing Istio control
                       plane is updated in-place. The workloads therefore\ndon't need
@@ -7981,6 +7990,7 @@ spec:
                     type: object
                 type: object
               version:
+                default: v1.21.0
                 description: |-
                   Defines the version of Istio to install.
                   Must be one of: v1.21.0, latest.

diff --git a/chart/crds/operator.istio.io_istiocnis.yaml b/chart/crds/operator.istio.io_istiocnis.yaml
@@ -56,9 +56,13 @@ spec:
           metadata:
             type: object
           spec:
+            default:
+              namespace: istio-cni
+              version: v1.21.0
             description: IstioCNISpec defines the desired state of IstioCNI
             properties:
               namespace:
+                default: istio-cni
                 description: Namespace to which the Istio CNI component should be
                   installed.
                 type: string
@@ -1367,6 +1371,7 @@ spec:
                     type: object
                 type: object
               version:
+                default: v1.21.0
                 description: |-
                   Defines the version of Istio to install.
                   Must be one of: v1.21.0, latest.

diff --git a/chart/crds/operator.istio.io_istios.yaml b/chart/crds/operator.istio.io_istios.yaml
@@ -72,9 +72,15 @@ spec:
           metadata:
             type: object
           spec:
+            default:
+              namespace: istio-system
+              updateStrategy:
+                type: InPlace
+              version: v1.21.0
             description: IstioSpec defines the desired state of Istio
             properties:
               namespace:
+                default: istio-system
                 description: Namespace to which the Istio components should be installed.
                 type: string
               profile:
@@ -95,6 +101,8 @@ spec:
                 - remote
                 type: string
               updateStrategy:
+                default:
+                  type: InPlace
                 description: Defines the update strategy to use when the version in
                   the Istio CR is updated.
                 properties:
@@ -107,6 +115,7 @@ spec:
                     minimum: 30
                     type: integer
                   type:
+                    default: InPlace
                     description: "Type of strategy to use. Can be \"InPlace\" or \"RevisionBased\".
                       When the \"InPlace\" strategy\nis used, the existing Istio control
                       plane is updated in-place. The workloads therefore\ndon't need
@@ -7981,6 +7990,7 @@ spec:
                     type: object
                 type: object
               version:
+                default: v1.21.0
                 description: |-
                   Defines the version of Istio to install.
                   Must be one of: v1.21.0, latest.

diff --git a/hack/update-version-list.sh b/hack/update-version-list.sh
@@ -23,11 +23,14 @@ function updateVersionsInIstioTypeComment() {
     selectValues=$(yq '.versions[].name | ", \"urn:alm:descriptor:com.tectonic.ui:select:" + . + "\""' "${VERSIONS_YAML_FILE}" | tr -d '\n')
     versionsEnum=$(yq '.versions[].name' "${VERSIONS_YAML_FILE}" | tr '\n' ';' | sed 's/;$//g')
     versions=$(yq '.versions[].name' "${VERSIONS_YAML_FILE}" | tr '\n' ',' | sed -e 's/,/, /g' -e 's/, $//g')
+    defaultVersion=$(yq '.versions[0].name' "${VERSIONS_YAML_FILE}")
 
     sed -i -E \
       -e "/\+sail:version/,/Version string/ s/(\/\/ \+operator-sdk:csv:customresourcedefinitions:type=spec,order=1,displayName=\"Istio Version\",xDescriptors=\{.*fieldGroup:General\")[^}]*(})/\1$selectValues}/g" \
       -e "/\+sail:version/,/Version string/ s/(\/\/ \+kubebuilder:validation:Enum=)(.*)/\1$versionsEnum/g" \
+      -e "/\+sail:version/,/Version string/ s/(\/\/ \+kubebuilder:default=)(.*)/\1$defaultVersion/g" \
       -e "/\+sail:version/,/Version string/ s/(\/\/ \Must be one of:)(.*)/\1 $versions./g" \
+      -e "s/(\+kubebuilder:default=.*version: \")[^\"]*\"/\1$defaultVersion\"/g" \
       api/v1alpha1/istio_types.go api/v1alpha1/istiorevision_types.go api/v1alpha1/istiocni_types.go
 }
 

diff --git a/pkg/test/util/supportedversion/supportedversion.go b/pkg/test/util/supportedversion/supportedversion.go
@@ -47,7 +47,7 @@ func init() {
 	}
 
 	List = versions.Versions
-	Default = "latest"
+	Default = List[0].Name
 	Old = List[1].Name
 	New = List[0].Name
 }

diff --git a/tests/e2e/controlplane/control_plane_test.go b/tests/e2e/controlplane/control_plane_test.go
@@ -45,23 +45,72 @@ var _ = Describe("Control Plane Installation", Ordered, func() {
 	SetDefaultEventuallyTimeout(120 * time.Second)
 	SetDefaultEventuallyPollingInterval(time.Second)
 
-	Describe("given Istio version", func() {
-		BeforeAll(func(ctx SpecContext) {
-			Expect(kubectl.CreateNamespace(namespace)).To(Succeed(), "Namespace failed to be created")
+	BeforeAll(func(ctx SpecContext) {
+		Expect(kubectl.CreateNamespace(namespace)).To(Succeed(), "Namespace failed to be created")
 
-			extraArg := ""
-			if ocp {
-				extraArg = "--set=platform=openshift"
-			}
+		extraArg := ""
+		if ocp {
+			extraArg = "--set=platform=openshift"
+		}
 
-			Expect(helm.Install("sail-operator", filepath.Join(baseDir, "chart"), "--namespace "+namespace, "--set=image="+image, extraArg)).
-				To(Succeed(), "Operator failed to be deployed")
+		Expect(helm.Install("sail-operator", filepath.Join(baseDir, "chart"), "--namespace "+namespace, "--set=image="+image, extraArg)).
+			To(Succeed(), "Operator failed to be deployed")
 
-			Eventually(common.GetObject).WithArguments(ctx, cl, common.Key(deploymentName, namespace), &appsv1.Deployment{}).
-				Should(HaveCondition(appsv1.DeploymentAvailable, metav1.ConditionTrue), "Error getting Istio CRD")
-			Success("Operator is deployed in the namespace and Running")
-		})
+		Eventually(common.GetObject).WithArguments(ctx, cl, common.Key(deploymentName, namespace), &appsv1.Deployment{}).
+			Should(HaveCondition(appsv1.DeploymentAvailable, metav1.ConditionTrue), "Error getting Istio CRD")
+		Success("Operator is deployed in the namespace and Running")
+	})
+
+	Describe("defaulting", func() {
+		DescribeTable("IstioCNI",
+			Entry("no spec", ""),
+			Entry("empty spec", "spec: {}"),
+			func(ctx SpecContext, spec string) {
+				yaml := `
+apiVersion: operator.istio.io/v1alpha1
+kind: IstioCNI
+metadata:
+  name: default
+` + spec
+				Expect(kubectl.ApplyString(yaml)).To(Succeed(), "IstioCNI creation failed")
+				Success("IstioCNI created")
+
+				cni := &v1alpha1.IstioCNI{}
+				Expect(cl.Get(ctx, common.Key("default"), cni)).To(Succeed())
+				Expect(cni.Spec.Version).To(Equal(supportedversion.Default))
+				Expect(cni.Spec.Namespace).To(Equal("istio-cni"))
+
+				Expect(cl.Delete(ctx, cni)).To(Succeed())
+			},
+		)
+
+		DescribeTable("Istio",
+			Entry("no spec", ""),
+			Entry("empty spec", "spec: {}"),
+			Entry("empty updateStrategy", "spec: {updateStrategy: {}}"),
+			func(ctx SpecContext, spec string) {
+				yaml := `
+apiVersion: operator.istio.io/v1alpha1
+kind: Istio
+metadata:
+  name: default
+` + spec
+				Expect(kubectl.ApplyString(yaml)).To(Succeed(), "Istio creation failed")
+				Success("Istio created")
+
+				istio := &v1alpha1.Istio{}
+				Expect(cl.Get(ctx, common.Key("default"), istio)).To(Succeed())
+				Expect(istio.Spec.Version).To(Equal(supportedversion.Default))
+				Expect(istio.Spec.Namespace).To(Equal("istio-system"))
+				Expect(istio.Spec.UpdateStrategy).ToNot(BeNil())
+				Expect(istio.Spec.UpdateStrategy.Type).To(Equal(v1alpha1.UpdateStrategyTypeInPlace))
+
+				Expect(cl.Delete(ctx, istio)).To(Succeed())
+			},
+		)
+	})
 
+	Describe("given Istio version", func() {
 		for _, version := range supportedversion.List {
 			// Note: This var version is needed to avoid the closure of the loop
 			version := version