Fix network/image webhook validations

Signed-off-by: Prajyot-Parab <[email protected]>
kubernetes-sigs · Jan 31, 2022 · 95b586f · 95b586f
1 parent b6ebb5f
commit 95b586f
Show file tree

Hide file tree

Showing 10 changed files with 50 additions and 11 deletions.
diff --git a/api/v1beta1/common.go b/api/v1beta1/common.go
@@ -16,6 +16,8 @@ limitations under the License.
 
 package v1beta1
 
+import "k8s.io/apimachinery/pkg/util/validation/field"
+
 func defaultIBMPowerVSMachineSpec(spec *IBMPowerVSMachineSpec) {
 
 	if spec.Memory == "" {
@@ -52,11 +54,11 @@ func validateIBMPowerVSProcType(spec IBMPowerVSMachineSpec) (bool, IBMPowerVSMac
 	return false, spec
 }
 
-func validateIBMPowerVSNetwork(network IBMPowerVSResourceReference) (bool, IBMPowerVSResourceReference) {
-	if network.ID != nil && network.Name != nil {
-		return false, network
+func validateIBMPowerVSResourceReference(res IBMPowerVSResourceReference, resType string) (bool, *field.Error) {
+	if res.ID != nil && res.Name != nil {
+		return false, field.Invalid(field.NewPath("spec", resType), res, "Only one of "+resType+" - ID or Name may be specified")
 	}
-	return true, IBMPowerVSResourceReference{}
+	return true, nil
 }
 
 func defaultIBMVPCMachineSpec(spec *IBMVPCMachineSpec) {

diff --git a/api/v1beta1/ibmpowervscluster_types.go b/api/v1beta1/ibmpowervscluster_types.go
@@ -35,6 +35,7 @@ type IBMPowerVSClusterSpec struct {
 	// Important: Run "make" to regenerate code after modifying this file
 
 	// ServiceInstanceID is the id of the power cloud instance where the vsi instance will get deployed
+	// +kubebuilder:validation:MinLength=1
 	ServiceInstanceID string `json:"serviceInstanceID"`
 
 	// Network is the reference to the Network to use for this cluster.

diff --git a/api/v1beta1/ibmpowervscluster_webhook.go b/api/v1beta1/ibmpowervscluster_webhook.go
@@ -82,8 +82,8 @@ func (r *IBMPowerVSCluster) validateIBMPowerVSCluster() error {
 }
 
 func (r *IBMPowerVSCluster) validateIBMPowerVSClusterNetwork() *field.Error {
-	if res, net := validateIBMPowerVSNetwork(r.Spec.Network); !res {
-		return field.Invalid(field.NewPath("spec", "network"), net, "Only one of Network - ID or Name may be specified")
+	if res, err := validateIBMPowerVSResourceReference(r.Spec.Network, "Network"); !res {
+		return err
 	}
 	return nil
 }
diff --git a/api/v1beta1/ibmpowervsmachine_types.go b/api/v1beta1/ibmpowervsmachine_types.go
@@ -37,6 +37,7 @@ type IBMPowerVSMachineSpec struct {
 	// Important: Run "make" to regenerate code after modifying this file
 
 	// ServiceInstanceID is the id of the power cloud instance where the vsi instance will get deployed
+	// +kubebuilder:validation:MinLength=1
 	ServiceInstanceID string `json:"serviceInstanceID"`
 
 	// SSHKey is the name of the SSH key pair provided to the vsi for authenticating users
@@ -74,10 +75,12 @@ type IBMPowerVSMachineSpec struct {
 // a validation error.
 type IBMPowerVSResourceReference struct {
 	// ID of resource
+	// +kubebuilder:validation:MinLength=1
 	// +optional
 	ID *string `json:"id,omitempty"`
 
 	// Name of resource
+	// +kubebuilder:validation:MinLength=1
 	// +optional
 	Name *string `json:"name,omitempty"`
 }

diff --git a/api/v1beta1/ibmpowervsmachine_webhook.go b/api/v1beta1/ibmpowervsmachine_webhook.go
@@ -78,6 +78,9 @@ func (r *IBMPowerVSMachine) validateIBMPowerVSMachine() error {
 	if err := r.validateIBMPowerVSMachineNetwork(); err != nil {
 		allErrs = append(allErrs, err)
 	}
+	if err := r.validateIBMPowerVSMachineImage(); err != nil {
+		allErrs = append(allErrs, err)
+	}
 	if len(allErrs) == 0 {
 		return nil
 	}
@@ -102,8 +105,15 @@ func (r *IBMPowerVSMachine) validateIBMPowerVSMachineProcType() *field.Error {
 }
 
 func (r *IBMPowerVSMachine) validateIBMPowerVSMachineNetwork() *field.Error {
-	if res, net := validateIBMPowerVSNetwork(r.Spec.Network); !res {
-		return field.Invalid(field.NewPath("spec", "network"), net, "Only one of Network - ID or Name may be specified")
+	if res, err := validateIBMPowerVSResourceReference(r.Spec.Network, "Network"); !res {
+		return err
+	}
+	return nil
+}
+
+func (r *IBMPowerVSMachine) validateIBMPowerVSMachineImage() *field.Error {
+	if res, err := validateIBMPowerVSResourceReference(r.Spec.Image, "Image"); !res {
+		return err
 	}
 	return nil
 }
diff --git a/api/v1beta1/ibmpowervsmachinetemplate_webhook.go b/api/v1beta1/ibmpowervsmachinetemplate_webhook.go
@@ -78,6 +78,9 @@ func (r *IBMPowerVSMachineTemplate) validateIBMPowerVSMachineTemplate() error {
 	if err := r.validateIBMPowerVSMachineTemplateNetwork(); err != nil {
 		allErrs = append(allErrs, err)
 	}
+	if err := r.validateIBMPowerVSMachineTemplateImage(); err != nil {
+		allErrs = append(allErrs, err)
+	}
 	if len(allErrs) == 0 {
 		return nil
 	}
@@ -102,8 +105,15 @@ func (r *IBMPowerVSMachineTemplate) validateIBMPowerVSMachineTemplateProcType()
 }
 
 func (r *IBMPowerVSMachineTemplate) validateIBMPowerVSMachineTemplateNetwork() *field.Error {
-	if res, net := validateIBMPowerVSNetwork(r.Spec.Template.Spec.Network); !res {
-		return field.Invalid(field.NewPath("spec", "template", "spec", "network"), net, "Only one of Network - ID or Name may be specified")
+	if res, err := validateIBMPowerVSResourceReference(r.Spec.Template.Spec.Network, "Network"); !res {
+		return err
+	}
+	return nil
+}
+
+func (r *IBMPowerVSMachineTemplate) validateIBMPowerVSMachineTemplateImage() *field.Error {
+	if res, err := validateIBMPowerVSResourceReference(r.Spec.Template.Spec.Image, "Image"); !res {
+		return err
 	}
 	return nil
 }
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclusters.yaml
@@ -126,14 +126,17 @@ spec:
                 properties:
                   id:
                     description: ID of resource
+                    minLength: 1
                     type: string
                   name:
                     description: Name of resource
+                    minLength: 1
                     type: string
                 type: object
               serviceInstanceID:
                 description: ServiceInstanceID is the id of the power cloud instance
                   where the vsi instance will get deployed
+                minLength: 1
                 type: string
             required:
             - network

diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsmachines.yaml
@@ -193,9 +193,11 @@ spec:
                 properties:
                   id:
                     description: ID of resource
+                    minLength: 1
                     type: string
                   name:
                     description: Name of resource
+                    minLength: 1
                     type: string
                 type: object
               memory:
@@ -207,9 +209,11 @@ spec:
                 properties:
                   id:
                     description: ID of resource
+                    minLength: 1
                     type: string
                   name:
                     description: Name of resource
+                    minLength: 1
                     type: string
                 type: object
               procType:
@@ -226,6 +230,7 @@ spec:
               serviceInstanceID:
                 description: ServiceInstanceID is the id of the power cloud instance
                   where the vsi instance will get deployed
+                minLength: 1
                 type: string
               sshKey:
                 description: SSHKey is the name of the SSH key pair provided to the

diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsmachinetemplates.yaml
@@ -150,9 +150,11 @@ spec:
                         properties:
                           id:
                             description: ID of resource
+                            minLength: 1
                             type: string
                           name:
                             description: Name of resource
+                            minLength: 1
                             type: string
                         type: object
                       memory:
@@ -164,9 +166,11 @@ spec:
                         properties:
                           id:
                             description: ID of resource
+                            minLength: 1
                             type: string
                           name:
                             description: Name of resource
+                            minLength: 1
                             type: string
                         type: object
                       procType:
@@ -183,6 +187,7 @@ spec:
                       serviceInstanceID:
                         description: ServiceInstanceID is the id of the power cloud
                           instance where the vsi instance will get deployed
+                        minLength: 1
                         type: string
                       sshKey:
                         description: SSHKey is the name of the SSH key pair provided

diff --git a/controllers/ibmpowervscluster_controller_test.go b/controllers/ibmpowervscluster_controller_test.go
@@ -41,7 +41,7 @@ var _ = Describe("IBMPowerVSClusterReconciler", func() {
 				Log:    klogr.New(),
 			}
 
-			instance := &v1beta1.IBMPowerVSCluster{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}}
+			instance := &v1beta1.IBMPowerVSCluster{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, Spec: v1beta1.IBMPowerVSClusterSpec{ServiceInstanceID: "foo"}}
 
 			// Create the IBMPowerVSCluster object and expect the Reconcile to be created
 			Expect(k8sClient.Create(ctx, instance)).To(Succeed())