Check nad has valid clusternetwork referrence

Signed-off-by: Jian Wang <[email protected]>
harvester · Feb 24, 2025 · 30b1a0c · 30b1a0c
1 parent 7dfe2d6
commit 30b1a0c
Show file tree

Hide file tree

Showing 3 changed files with 106 additions and 4 deletions.
diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go
@@ -118,7 +118,7 @@ func run(ctx context.Context, cfg *rest.Config, options *config.Options) error {
 
 	if err := webhookServer.RegisterValidators(
 		clusternetwork.NewCnValidator(c.vcCache),
-		nad.NewNadValidator(c.vmiCache),
+		nad.NewNadValidator(c.vmiCache, c.cnCache),
 		vlanconfig.NewVlanConfigValidator(c.nadCache, c.vcCache, c.vsCache, c.vmiCache, c.cnCache),
 	); err != nil {
 		return fmt.Errorf("failed to register validators: %v", err)

diff --git a/pkg/webhook/nad/validator.go b/pkg/webhook/nad/validator.go
@@ -13,6 +13,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	kubevirtv1 "kubevirt.io/api/core/v1"
 
+	ctlnetworkv1 "github.com/harvester/harvester-network-controller/pkg/generated/controllers/network.harvesterhci.io/v1beta1"
 	"github.com/harvester/harvester-network-controller/pkg/network/iface"
 	"github.com/harvester/harvester-network-controller/pkg/utils"
 )
@@ -28,19 +29,26 @@ const (
 type Validator struct {
 	admission.DefaultValidator
 	vmiCache ctlkubevirtv1.VirtualMachineInstanceCache
+	cnCache  ctlnetworkv1.ClusterNetworkCache
 }
 
 var _ admission.Validator = &Validator{}
 
-func NewNadValidator(vmiCache ctlkubevirtv1.VirtualMachineInstanceCache) *Validator {
+func NewNadValidator(vmiCache ctlkubevirtv1.VirtualMachineInstanceCache, cnCache ctlnetworkv1.ClusterNetworkCache) *Validator {
 	return &Validator{
 		vmiCache: vmiCache,
+		cnCache:  cnCache,
 	}
 }
 
 func (v *Validator) Create(_ *admission.Request, newObj runtime.Object) error {
 	nad := newObj.(*cniv1.NetworkAttachmentDefinition)
 
+	// check clusternetwrork
+	if err := v.checkClusterNetwork(nad); err != nil {
+		return fmt.Errorf(createErr, nad.Namespace, nad.Name, err)
+	}
+
 	if err := v.checkRoute(nad.Annotations[utils.KeyNetworkRoute]); err != nil {
 		return fmt.Errorf(createErr, nad.Namespace, nad.Name, err)
 	}
@@ -65,6 +73,16 @@ func (v *Validator) Update(_ *admission.Request, oldObj, newObj runtime.Object)
 		return nil
 	}
 
+	// check clusternetwrork
+	if err := v.checkClusterNetworkUnchanged(oldNad, newNad); err != nil {
+		return fmt.Errorf(updateErr, newNad.Namespace, newNad.Name, err)
+	}
+
+	// check clusternetwrork
+	if err := v.checkClusterNetwork(newNad); err != nil {
+		return fmt.Errorf(updateErr, newNad.Namespace, newNad.Name, err)
+	}
+
 	if err := v.checkRoute(newNad.Annotations[utils.KeyNetworkRoute]); err != nil {
 		return fmt.Errorf(updateErr, newNad.Namespace, newNad.Name, err)
 	}
@@ -149,6 +167,43 @@ func (v *Validator) checkVmi(nad *cniv1.NetworkAttachmentDefinition) error {
 	return v.generateVmiNoneStopError(nad, vmis)
 }
 
+func (v *Validator) checkClusterNetwork(nad *cniv1.NetworkAttachmentDefinition) error {
+	if nad.Labels == nil {
+		return fmt.Errorf("nad does not have label")
+	}
+
+	cnName := nad.Labels[utils.KeyClusterNetworkLabel]
+	if cnName == "" {
+		return fmt.Errorf("nad has empty cluster network name label")
+	}
+
+	if _, err := v.cnCache.Get(cnName); err != nil {
+		return fmt.Errorf("nad refers to none-existing cluster network %s", cnName)
+	}
+
+	return nil
+}
+
+func (v *Validator) checkClusterNetworkUnchanged(oldNad, newNad *cniv1.NetworkAttachmentDefinition) error {
+	if oldNad.Labels == nil {
+		return fmt.Errorf("old nad does not have label")
+	}
+
+	if newNad.Labels == nil {
+		return fmt.Errorf("new nad does not have label")
+	}
+
+	if oldNad.Labels[utils.KeyClusterNetworkLabel] != newNad.Labels[utils.KeyClusterNetworkLabel] {
+		return fmt.Errorf("nad network can't be changed from %s to %s", oldNad.Labels[utils.KeyClusterNetworkLabel], newNad.Labels[utils.KeyClusterNetworkLabel])
+	}
+
+	if oldNad.Labels[utils.KeyNetworkType] != newNad.Labels[utils.KeyNetworkType] {
+		return fmt.Errorf("nad network type can't be changed from %s to %s", oldNad.Labels[utils.KeyNetworkType], newNad.Labels[utils.KeyNetworkType])
+	}
+
+	return nil
+}
+
 func (v *Validator) checkStorageNetwork(nad *cniv1.NetworkAttachmentDefinition) error {
 	if utils.IsStorageNetworkNad(nad) {
 		fmt.Errorf(storageNetworkErr)

diff --git a/pkg/webhook/nad/validator_test.go b/pkg/webhook/nad/validator_test.go
@@ -60,6 +60,49 @@ func TestCreateNAD(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:      "NAD can't be created as it does not have label",
+			returnErr: true,
+			errKey:    "nad does not have label",
+			currentCN: &networkv1.ClusterNetwork{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        testCnName,
+					Annotations: map[string]string{"test": "test"},
+				},
+			},
+			newNAD: &cniv1.NetworkAttachmentDefinition{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        testNADName,
+					Namespace:   testNamespace,
+					Annotations: map[string]string{"test": "test"},
+				},
+				Spec: cniv1.NetworkAttachmentDefinitionSpec{
+					Config: "{\"cniVersion\",\"name\":\"net1-vlan\",\"type\":\"bridge\",\"bridge\":\"harvester-br\",\"promiscMode\":true,\"vlan\":300,\"ipam\":{}}",
+				},
+			},
+		},
+		{
+			name:      "NAD can't be created as it refers to none-existing cluster network",
+			returnErr: true,
+			errKey:    "nad refers to none-existing cluster network",
+			currentCN: &networkv1.ClusterNetwork{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        testCnName,
+					Annotations: map[string]string{"test": "test"},
+				},
+			},
+			newNAD: &cniv1.NetworkAttachmentDefinition{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        testNADName,
+					Namespace:   testNamespace,
+					Annotations: map[string]string{"test": "test"},
+					Labels:      map[string]string{utils.KeyClusterNetworkLabel: "invalid"},
+				},
+				Spec: cniv1.NetworkAttachmentDefinitionSpec{
+					Config: "{\"cniVersion\",\"name\":\"net1-vlan\",\"type\":\"bridge\",\"bridge\":\"harvester-br\",\"promiscMode\":true,\"vlan\":300,\"ipam\":{}}",
+				},
+			},
+		},
 		{
 			name:      "NAD can't be created as it has invalid config string",
 			returnErr: true,
@@ -210,6 +253,7 @@ func TestCreateNAD(t *testing.T) {
 			// client to inject test data
 			vcClient := fakeclients.VlanConfigClient(nchclientset.NetworkV1beta1().VlanConfigs)
 			cnClient := fakeclients.ClusterNetworkClient(nchclientset.NetworkV1beta1().ClusterNetworks)
+			cnCache := fakeclients.ClusterNetworkCache(nchclientset.NetworkV1beta1().ClusterNetworks)
 
 			if tc.currentVC != nil {
 				vcClient.Create(tc.currentVC)
@@ -218,7 +262,7 @@ func TestCreateNAD(t *testing.T) {
 				cnClient.Create(tc.currentCN)
 			}
 
-			validator := NewNadValidator(vmiCache)
+			validator := NewNadValidator(vmiCache, cnCache)
 
 			err := validator.Create(nil, tc.newNAD)
 			assert.True(t, tc.returnErr == (err != nil))
@@ -304,9 +348,12 @@ func TestDeleteNAD(t *testing.T) {
 				return
 			}
 
+			nchclientset := fake.NewSimpleClientset()
+
 			harvesterclientset := harvesterfake.NewSimpleClientset()
 			vmiCache := harvesterfakeclients.VirtualMachineInstanceCache(harvesterclientset.KubevirtV1().VirtualMachineInstances)
-			validator := NewNadValidator(vmiCache)
+			cnCache := fakeclients.ClusterNetworkCache(nchclientset.NetworkV1beta1().ClusterNetworks)
+			validator := NewNadValidator(vmiCache, cnCache)
 
 			// due to fake vmiCache limitation, just test generateVmiNoneStopError() instead of Delete()
 			err := validator.generateVmiNoneStopError(tc.currentNAD, tc.usedVMs)