Validate excludeTopology field

Multiple SriovNetworkNodePolicies may target the same resource, but the new field must have the same value. Add a resource validation step to ensure this condition is met. Signed-off-by: Andrea Panattoni <[email protected]>
k8snetworkplumbingwg · May 9, 2023 · 0318327 · 0318327
1 parent 6ce730b
commit 0318327
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 1 deletion.
diff --git a/pkg/webhook/validate.go b/pkg/webhook/validate.go
@@ -269,7 +269,24 @@ func validatePolicyForNodePolicy(
 			}
 		}
 	}
-	return true, nil
+
+	return validateExludeTopologyField(current, previous)
+}
+
+func validateExludeTopologyField(
+	current *sriovnetworkv1.SriovNetworkNodePolicy,
+	previous *sriovnetworkv1.SriovNetworkNodePolicy,
+) (bool, error) {
+	if current.Spec.ResourceName != previous.Spec.ResourceName {
+		return true, nil
+	}
+
+	if current.Spec.ExcludeTopology == previous.Spec.ExcludeTopology {
+		return true, nil
+	}
+
+	return false, fmt.Errorf("excludeTopology[%t] field conflicts with policy [%s].ExcludeTopology[%t] as they target the same resource[%s]",
+		current.Spec.ExcludeTopology, previous.GetName(), previous.Spec.ExcludeTopology, current.Spec.ResourceName)
 }
 
 func validateNicModel(selector *sriovnetworkv1.SriovNetworkNicSelector, iface *sriovnetworkv1.InterfaceExt, node *corev1.Node) bool {

diff --git a/pkg/webhook/validate_test.go b/pkg/webhook/validate_test.go
@@ -298,6 +298,54 @@ func TestValidatePolicyForNodeStateWithUpdatedExistingVfRange(t *testing.T) {
 	g.Expect(ok).To(Equal(true))
 }
 
+func TestValidatePoliciesWithDifferentExcludeTopologyForTheSameResource(t *testing.T) {
+	current := &SriovNetworkNodePolicy{
+		ObjectMeta: metav1.ObjectMeta{Name: "currentPolicy"},
+		Spec: SriovNetworkNodePolicySpec{
+			ResourceName:    "resourceX",
+			ExcludeTopology: true,
+		},
+	}
+
+	previous := &SriovNetworkNodePolicy{
+		ObjectMeta: metav1.ObjectMeta{Name: "previousPolicy"},
+		Spec: SriovNetworkNodePolicySpec{
+			ResourceName:    "resourceX",
+			ExcludeTopology: false,
+		},
+	}
+
+	ok, err := validatePolicyForNodePolicy(current, previous)
+
+	g := NewGomegaWithT(t)
+	g.Expect(ok).To(Equal(false))
+	g.Expect(err).To(MatchError("excludeTopology[true] field conflicts with policy [previousPolicy].ExcludeTopology[false] as they target the same resource[resourceX]"))
+}
+
+func TestValidatePoliciesWithSameExcludeTopologyForTheSameResource(t *testing.T) {
+	current := &SriovNetworkNodePolicy{
+		ObjectMeta: metav1.ObjectMeta{Name: "currentPolicy"},
+		Spec: SriovNetworkNodePolicySpec{
+			ResourceName:    "resourceX",
+			ExcludeTopology: true,
+		},
+	}
+
+	previous := &SriovNetworkNodePolicy{
+		ObjectMeta: metav1.ObjectMeta{Name: "previousPolicy"},
+		Spec: SriovNetworkNodePolicySpec{
+			ResourceName:    "resourceX",
+			ExcludeTopology: true,
+		},
+	}
+
+	ok, err := validatePolicyForNodePolicy(current, previous)
+
+	g := NewGomegaWithT(t)
+	g.Expect(ok).To(Equal(true))
+	g.Expect(err).NotTo(HaveOccurred())
+}
+
 func TestStaticValidateSriovNetworkNodePolicyWithValidVendorDevice(t *testing.T) {
 	policy := &SriovNetworkNodePolicy{
 		Spec: SriovNetworkNodePolicySpec{