From d797fd33b96d246cf9839430c96c8dbdafdd6b41 Mon Sep 17 00:00:00 2001
From: Sebastian Sch <sebassch@gmail.com>
Date: Mon, 8 May 2023 17:24:26 +0300
Subject: [PATCH] Create the webhook validations for externallyCreated variable

Signed-off-by: Sebastian Sch <sebassch@gmail.com>
---
 pkg/webhook/validate.go      |  10 ++++
 pkg/webhook/validate_test.go | 111 +++++++++++++++++++++++++++++++++++
 2 files changed, 121 insertions(+)

diff --git a/pkg/webhook/validate.go b/pkg/webhook/validate.go
index 971d147cde..f609b20500 100644
--- a/pkg/webhook/validate.go
+++ b/pkg/webhook/validate.go
@@ -231,6 +231,10 @@ func validatePolicyForNodeState(policy *sriovnetworkv1.SriovNetworkNodePolicy, s
 			if policy.Spec.NumVfs > MlxMaxVFs && iface.Vendor == MellanoxID {
 				return false, fmt.Errorf("numVfs(%d) in CR %s exceed the maximum allowed value(%d)", policy.Spec.NumVfs, policy.GetName(), MlxMaxVFs)
 			}
+
+			if policy.Spec.ExternallyCreated && policy.Spec.NumVfs != iface.NumVfs {
+				return false, fmt.Errorf("numVfs(%d) in CR %s is not equal to the number of virtual functions allocated for the PF externally value(%d)", policy.Spec.NumVfs, policy.GetName(), iface.NumVfs)
+			}
 			// vdpa: only mellanox cards are supported
 			if policy.Spec.VdpaType == constants.VdpaTypeVirtio && iface.Vendor != MellanoxID {
 				return false, fmt.Errorf("vendor(%s) in CR %s not supported for virtio-vdpa", iface.Vendor, policy.GetName())
@@ -261,6 +265,12 @@ func validatePolicyForNodePolicy(
 			// since it should already be evaluated in previous run.
 			preName, preRngSt, preRngEnd, _ := sriovnetworkv1.ParsePFName(prePf)
 			if curName == preName {
+				// reject policy with externallyManage if there is a policy on the same PF without it
+				if current.Spec.ExternallyCreated != previous.Spec.ExternallyCreated {
+					return false, fmt.Errorf("externallyManage is inconsistent with existing policy %s", previous.GetName())
+				}
+
+				// Check for overlapping ranges
 				if curRngEnd < preRngSt || curRngSt > preRngEnd {
 					return true, nil
 				} else {
diff --git a/pkg/webhook/validate_test.go b/pkg/webhook/validate_test.go
index 906d5e85a6..01687421a7 100644
--- a/pkg/webhook/validate_test.go
+++ b/pkg/webhook/validate_test.go
@@ -245,6 +245,117 @@ func TestValidatePolicyForNodeStateWithInvalidNumVfsPolicy(t *testing.T) {
 	g.Expect(ok).To(Equal(false))
 }
 
+func TestValidatePolicyForNodeStateWithInvalidNumVfsExternallyCreated(t *testing.T) {
+	state := newNodeState()
+	policy := &SriovNetworkNodePolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "p1",
+		},
+		Spec: SriovNetworkNodePolicySpec{
+			DeviceType: "netdevice",
+			NicSelector: SriovNetworkNicSelector{
+				PfNames:     []string{"ens803f0"},
+				RootDevices: []string{"0000:86:00.0"},
+				Vendor:      "8086",
+			},
+			NodeSelector: map[string]string{
+				"feature.node.kubernetes.io/network-sriov.capable": "true",
+			},
+			NumVfs:            5,
+			Priority:          99,
+			ResourceName:      "p0",
+			ExternallyCreated: true,
+		},
+	}
+	g := NewGomegaWithT(t)
+	ok, err := validatePolicyForNodeState(policy, state, NewNode())
+	g.Expect(err).To(MatchError(ContainSubstring(fmt.Sprintf("numVfs(%d) in CR %s is not equal to the number of virtual functions allocated for the PF externally value(%d)", policy.Spec.NumVfs, policy.GetName(), state.Status.Interfaces[0].NumVfs))))
+	g.Expect(ok).To(Equal(false))
+}
+
+func TestValidatePolicyForNodeStateWithValidNumVfsExternallyCreated(t *testing.T) {
+	state := newNodeState()
+	policy := &SriovNetworkNodePolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "p1",
+		},
+		Spec: SriovNetworkNodePolicySpec{
+			DeviceType: "netdevice",
+			NicSelector: SriovNetworkNicSelector{
+				PfNames:     []string{"ens803f0"},
+				RootDevices: []string{"0000:86:00.0"},
+				Vendor:      "8086",
+			},
+			NodeSelector: map[string]string{
+				"feature.node.kubernetes.io/network-sriov.capable": "true",
+			},
+			NumVfs:            4,
+			Priority:          99,
+			ResourceName:      "p0",
+			ExternallyCreated: true,
+		},
+	}
+	g := NewGomegaWithT(t)
+	ok, err := validatePolicyForNodeState(policy, state, NewNode())
+	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(ok).To(Equal(true))
+}
+
+func TestValidatePolicyForNodePolicyWithOutExternallyManageConflict(t *testing.T) {
+	appliedPolicy := newNodePolicy()
+	appliedPolicy.Spec.ExternallyCreated = true
+	policy := &SriovNetworkNodePolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "p0",
+		},
+		Spec: SriovNetworkNodePolicySpec{
+			DeviceType: "netdevice",
+			NicSelector: SriovNetworkNicSelector{
+				PfNames: []string{"ens803f1#3-4"},
+				Vendor:  "8086",
+			},
+			NodeSelector: map[string]string{
+				"feature.node.kubernetes.io/network-sriov.capable": "true",
+			},
+			NumVfs:            63,
+			Priority:          99,
+			ResourceName:      "p0",
+			ExternallyCreated: true,
+		},
+	}
+	g := NewGomegaWithT(t)
+	ok, err := validatePolicyForNodePolicy(policy, appliedPolicy)
+	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(ok).To(Equal(true))
+}
+
+func TestValidatePolicyForNodePolicyWithExternallyManageConflict(t *testing.T) {
+	appliedPolicy := newNodePolicy()
+	policy := &SriovNetworkNodePolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "p0",
+		},
+		Spec: SriovNetworkNodePolicySpec{
+			DeviceType: "netdevice",
+			NicSelector: SriovNetworkNicSelector{
+				PfNames: []string{"ens803f1#3-4"},
+				Vendor:  "8086",
+			},
+			NodeSelector: map[string]string{
+				"feature.node.kubernetes.io/network-sriov.capable": "true",
+			},
+			NumVfs:            63,
+			Priority:          99,
+			ResourceName:      "p0",
+			ExternallyCreated: true,
+		},
+	}
+	g := NewGomegaWithT(t)
+	ok, err := validatePolicyForNodePolicy(policy, appliedPolicy)
+	g.Expect(err).To(MatchError(ContainSubstring(fmt.Sprintf("externallyManage is inconsistent with existing policy %s", appliedPolicy.ObjectMeta.Name))))
+	g.Expect(ok).To(Equal(false))
+}
+
 func TestValidatePolicyForNodePolicyWithOverlappedVfRange(t *testing.T) {
 	appliedPolicy := newNodePolicy()
 	policy := &SriovNetworkNodePolicy{