⚠️ MachineHealthCheck Spec.Selector field cannot be empty

Signed-off-by: Vince Prignano <[email protected]>
kubernetes-sigs · May 11, 2020 · 721bda3 · 721bda3
1 parent df9fcf9
commit 721bda3
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 1 deletion.
diff --git a/api/v1alpha3/machinehealthcheck_webhook.go b/api/v1alpha3/machinehealthcheck_webhook.go
@@ -90,14 +90,22 @@ func (m *MachineHealthCheck) validate(old *MachineHealthCheck) error {
 	var allErrs field.ErrorList
 
 	// Validate selector parses as Selector
-	_, err := metav1.LabelSelectorAsSelector(&m.Spec.Selector)
+	selector, err := metav1.LabelSelectorAsSelector(&m.Spec.Selector)
 	if err != nil {
 		allErrs = append(
 			allErrs,
 			field.Invalid(field.NewPath("spec", "selector"), m.Spec.Selector, err.Error()),
 		)
 	}
 
+	// Validate that the selector isn't empty.
+	if selector != nil && selector.Empty() {
+		allErrs = append(
+			allErrs,
+			field.Invalid(field.NewPath("spec", "selector"), m.Spec.Selector, "selector must not be empty"),
+		)
+	}
+
 	if old != nil && old.Spec.ClusterName != m.Spec.ClusterName {
 		allErrs = append(
 			allErrs,

diff --git a/api/v1alpha3/machinehealthcheck_webhook_test.go b/api/v1alpha3/machinehealthcheck_webhook_test.go
@@ -105,11 +105,21 @@ func TestMachineHealthCheckClusterNameImmutable(t *testing.T) {
 			newMHC := &MachineHealthCheck{
 				Spec: MachineHealthCheckSpec{
 					ClusterName: tt.newClusterName,
+					Selector: metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"test": "test",
+						},
+					},
 				},
 			}
 			oldMHC := &MachineHealthCheck{
 				Spec: MachineHealthCheckSpec{
 					ClusterName: tt.oldClusterName,
+					Selector: metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"test": "test",
+						},
+					},
 				},
 			}
 
@@ -172,6 +182,11 @@ func TestMachineHealthCheckNodeStartupTimeout(t *testing.T) {
 		mhc := &MachineHealthCheck{
 			Spec: MachineHealthCheckSpec{
 				NodeStartupTimeout: tt.timeout,
+				Selector: metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"test": "test",
+					},
+				},
 			},
 		}
 
@@ -220,6 +235,11 @@ func TestMachineHealthCheckMaxUnhealthy(t *testing.T) {
 		mhc := &MachineHealthCheck{
 			Spec: MachineHealthCheckSpec{
 				MaxUnhealthy: &maxUnhealthy,
+				Selector: metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"test": "test",
+					},
+				},
 			},
 		}
 
@@ -232,3 +252,11 @@ func TestMachineHealthCheckMaxUnhealthy(t *testing.T) {
 		}
 	}
 }
+
+func TestMachineHealthCheckSelectorValidation(t *testing.T) {
+	g := NewWithT(t)
+	mhc := &MachineHealthCheck{}
+	err := mhc.validate(nil)
+	g.Expect(err).ToNot(BeNil())
+	g.Expect(err.Error()).To(ContainSubstring("selector must not be empty"))
+}