feat: remove CompartmentId mandatory from ocicluster_types (#67)

CompartmentId is still mandatory, but the admission webhook is now handling the validation.
oracle · Apr 23, 2022 · 4ae7195 · 4ae7195
1 parent 46ad859
commit 4ae7195
Show file tree

Hide file tree

Showing 7 changed files with 24 additions and 14 deletions.
diff --git a/api/v1beta1/ocicluster_types.go b/api/v1beta1/ocicluster_types.go
@@ -46,7 +46,8 @@ type OCIClusterSpec struct {
 	DefinedTags map[string]map[string]string `json:"definedTags,omitempty"`
 
 	// Compartment to create the cluster network.
-	CompartmentId string `mandatory:"true" json:"compartmentId"`
+	// +optional
+	CompartmentId string `json:"compartmentId"`
 
 	// Region the cluster operates in. It must be one of available regions in Region Identifier format.
 	// See https://docs.oracle.com/en-us/iaas/Content/General/Concepts/regions.htm

diff --git a/api/v1beta1/ocicluster_webhook.go b/api/v1beta1/ocicluster_webhook.go
@@ -93,8 +93,15 @@ func (c *OCICluster) ValidateUpdate(old runtime.Object) error {
 func (c *OCICluster) validate() field.ErrorList {
 	var allErrs field.ErrorList
 
-	// simple validity test for compartment
-	if !validOcid(c.Spec.CompartmentId) {
+	if len(c.Spec.CompartmentId) <= 0 {
+		allErrs = append(
+			allErrs,
+			field.Invalid(field.NewPath("spec", "compartmentId"), c.Spec.CompartmentId, "field is required"))
+	}
+
+	// Handle case where CompartmentId exists, but isn't valid
+	// the separate "blank" check above is a more clear error for the user
+	if len(c.Spec.CompartmentId) > 0 && !validOcid(c.Spec.CompartmentId) {
 		allErrs = append(
 			allErrs,
 			field.Invalid(field.NewPath("spec", "compartmentId"), c.Spec.CompartmentId, "field is invalid"))

diff --git a/api/v1beta1/ocicluster_webhook_test.go b/api/v1beta1/ocicluster_webhook_test.go
@@ -34,7 +34,7 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
 		expectErr bool
 	}{
 		{
-			name: "shouldn't allow bad ImageId",
+			name: "shouldn't allow bad CompartmentId",
 			c: &OCICluster{
 				ObjectMeta: metav1.ObjectMeta{},
 				Spec: OCIClusterSpec{
@@ -43,6 +43,14 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
 			},
 			expectErr: true,
 		},
+		{
+			name: "shouldn't allow blank CompartmentId",
+			c: &OCICluster{
+				ObjectMeta: metav1.ObjectMeta{},
+				Spec:       OCIClusterSpec{},
+			},
+			expectErr: true,
+		},
 		{
 			name: "should succeed",
 			c: &OCICluster{

diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ociclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ociclusters.yaml
@@ -936,8 +936,6 @@ spec:
                 description: Region the cluster operates in. It must be one of available
                   regions in Region Identifier format. See https://docs.oracle.com/en-us/iaas/Content/General/Concepts/regions.htm
                 type: string
-            required:
-            - compartmentId
             type: object
           status:
             description: OCIClusterStatus defines the observed state of OCICluster

diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ociclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ociclustertemplates.yaml
@@ -1063,8 +1063,6 @@ spec:
                         description: Region the cluster operates in. It must be one
                           of available regions in Region Identifier format. See https://docs.oracle.com/en-us/iaas/Content/General/Concepts/regions.htm
                         type: string
-                    required:
-                    - compartmentId
                     type: object
                 required:
                 - spec

diff --git a/templates/clusterclass-example.yaml b/templates/clusterclass-example.yaml
@@ -76,7 +76,7 @@ spec:
           matchResources:
             infrastructureCluster: true
         jsonPatches:
-          - op: replace
+          - op: add
             path: "/spec/template/spec/compartmentId"
             valueFrom:
               variable: compartmentId
@@ -162,8 +162,7 @@ metadata:
   name: ocicluster
 spec:
   template:
-    spec:
-      compartmentId: REPLACE
+    spec: {}
 ---
 kind: KubeadmControlPlaneTemplate
 apiVersion: controlplane.cluster.x-k8s.io/v1beta1

diff --git a/...structure-oci/v1beta1/cluster-template-cluster-class/clusterclass-test-cluster-class.yaml b/...structure-oci/v1beta1/cluster-template-cluster-class/clusterclass-test-cluster-class.yaml
@@ -76,7 +76,7 @@ spec:
           matchResources:
             infrastructureCluster: true
         jsonPatches:
-          - op: replace
+          - op: add
             path: "/spec/template/spec/compartmentId"
             valueFrom:
               variable: compartmentId
@@ -162,8 +162,7 @@ metadata:
   name: ocicluster
 spec:
   template:
-    spec:
-      compartmentId: REPLACE
+    spec: {}
 ---
 kind: KubeadmControlPlaneTemplate
 apiVersion: controlplane.cluster.x-k8s.io/v1beta1