fix(vm): make virtualMachineCIDRs optional

images/hooks/pkg/hooks/validate-module-config/hook.go

@@ -121,6 +121,9 @@ func validateModuleConfigSettings(mc *mcapi.ModuleConfig, nodes []corev1.Node, p
 	if err != nil {
 		return err
 	}
+	if len(CIDRs) == 0 {
+		return nil
+	}
 
 	err = moduleconfig.CheckCIDRsOverlap(CIDRs)
 	if err != nil {
@@ -161,10 +164,6 @@ func validateRequiredModuleConfigSettings(mc *mcapi.ModuleConfig) error {
 		return fmt.Errorf("spec.settings is empty")
 	}
 
-	if _, ok := mc.Spec.Settings["virtualMachineCIDRs"]; !ok {
-		return fmt.Errorf("spec.settings.virtualMachineCIDRs is required")
-	}
-
 	if _, ok := mc.Spec.Settings["dvcr"]; !ok {
 		return fmt.Errorf("spec.settings.dvcr is required")
 	}

images/hooks/pkg/hooks/validate-module-config/hook_test.go

@@ -222,12 +222,41 @@ var _ = Describe("ModuleConfig validation hook", func() {
 		Expect(validationErrorSet).To(BeTrue(), "should set validation error")
 	},
 		Entry("settings are nil", nil, "spec.settings is empty"),
-		Entry("settings are empty", map[string]any{}, "spec.settings.virtualMachineCIDRs is required"),
+		Entry("settings are empty", map[string]any{}, "spec.settings.dvcr is required"),
 		Entry("dvcr is missing", map[string]any{
 			"virtualMachineCIDRs": validVirtualMachineCIDRs,
 		}, "spec.settings.dvcr is required"),
 	)
 
+	It("Should allow module config without virtualMachineCIDRs", func() {
+		settingsWithoutCIDRs := map[string]any{
+			"dvcr": validSettings["dvcr"],
+		}
+		prepareValues(defaultPodSubnet, defaultServiceSubnet)
+		prepareConfigValues(settingsWithoutCIDRs)
+		prepareSnapshots(
+			newModuleConfigSnapshot(settingsWithoutCIDRs),
+			newNodesSnapshot(defaultNodeAddresses),
+		)
+
+		values.SetMock.Set(func(path string, v any) {
+			switch path {
+			case settings.InternalValuesConfigCopyPath:
+				Expect(v).ToNot(HaveKey("virtualMachineCIDRs"))
+				Expect(v).To(HaveKey("dvcr"))
+			case settings.InternalValuesConfigValidationPath:
+				Fail("Should not set validation error")
+			default:
+				Fail("unexpected path")
+			}
+		})
+		values.RemoveMock.Set(func(path string) {
+			Expect(path).To(Equal(settings.InternalValuesConfigValidationPath))
+		})
+
+		Expect(Reconcile(context.Background(), newInput())).To(Succeed())
+	})
+
 	DescribeTable("Should set validation error if virtualMachineCIDRs overlap with other addresses in cluster", func(cidrs []any) {
 		prepareValues(defaultPodSubnet, defaultServiceSubnet)
 		invalidSettings := map[string]any{

images/hooks/pkg/settings/module_config.go

@@ -55,10 +55,6 @@ func CanRunWithModuleConfig(ctx context.Context, input *pkg.HookInput) (bool, er
 		return false, nil
 	}
 
-	if _, ok := moduleConfig.Spec.Settings["virtualMachineCIDRs"]; !ok {
-		return false, nil
-	}
-
 	if _, ok := moduleConfig.Spec.Settings["dvcr"]; !ok {
 		return false, nil
 	}

images/hooks/pkg/settings/module_config_test.go

@@ -116,6 +116,24 @@ func TestCanRunWithModuleConfig(t *testing.T) {
 	})
 
 	t.Run("returns true when required settings exist", func(t *testing.T) {
+		input := newInput(&fakeKubernetesClient{get: func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error {
+			mc := obj.(*mcapi.ModuleConfig)
+			*mc = *NewModuleConfigForTest(map[string]any{
+				"dvcr": map[string]any{},
+			})
+			return nil
+		}}, nil)
+
+		ok, err := CanRunWithModuleConfig(context.Background(), input)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if !ok {
+			t.Fatalf("expected CanRunWithModuleConfig to return true")
+		}
+	})
+
+	t.Run("returns true when required settings and cidrs exist", func(t *testing.T) {
 		input := newInput(&fakeKubernetesClient{get: func(ctx context.Context, key ctrlclient.ObjectKey, obj ctrlclient.Object) error {
 			mc := obj.(*mcapi.ModuleConfig)
 			*mc = *NewModuleConfigForTest(map[string]any{

images/virtualization-artifact/pkg/controller/moduleconfig/checks.go

@@ -97,8 +97,21 @@ func CheckCIDRsOverlapWithServiceSubnet(cidrs []netip.Prefix, subnet netip.Prefi
 }
 
 func ParseCIDRs(settings mcapi.SettingsValues) ([]netip.Prefix, error) {
-	raw := settings[virtualMachineCIDRs].([]interface{})
-	CIDRs, err := convertToStringSlice(raw)
+	if settings == nil {
+		return nil, nil
+	}
+
+	raw, ok := settings[virtualMachineCIDRs]
+	if !ok || raw == nil {
+		return nil, nil
+	}
+
+	rawCIDRs, ok := raw.([]interface{})
+	if !ok {
+		return nil, fmt.Errorf("failed to convert %q to []interface{}", virtualMachineCIDRs)
+	}
+
+	CIDRs, err := convertToStringSlice(rawCIDRs)
 	if err != nil {
 		return nil, err
 	}

images/virtualization-artifact/pkg/controller/moduleconfig/checks_test.go

@@ -0,0 +1,41 @@
+package moduleconfig
+
+import (
+	"testing"
+
+	mcapi "github.com/deckhouse/virtualization-controller/pkg/controller/moduleconfig/api"
+)
+
+func TestParseCIDRs(t *testing.T) {
+	tests := []struct {
+		name     string
+		settings mcapi.SettingsValues
+		wantLen  int
+		wantErr  bool
+	}{
+		{name: "nil settings", settings: nil, wantLen: 0},
+		{name: "missing key", settings: mcapi.SettingsValues{"dvcr": map[string]any{}}, wantLen: 0},
+		{name: "nil value", settings: mcapi.SettingsValues{"virtualMachineCIDRs": nil}, wantLen: 0},
+		{name: "invalid type", settings: mcapi.SettingsValues{"virtualMachineCIDRs": []string{"10.0.0.0/24"}}, wantErr: true},
+		{name: "invalid cidr", settings: mcapi.SettingsValues{"virtualMachineCIDRs": []any{"bad"}}, wantErr: true},
+		{name: "valid cidrs", settings: mcapi.SettingsValues{"virtualMachineCIDRs": []any{"10.0.0.0/24", "10.0.1.0/24"}}, wantLen: 2},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cidrs, err := ParseCIDRs(tt.settings)
+			if tt.wantErr {
+				if err == nil {
+					t.Fatalf("expected error")
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if len(cidrs) != tt.wantLen {
+				t.Fatalf("expected %d cidrs, got %d", tt.wantLen, len(cidrs))
+			}
+		})
+	}
+}

images/virtualization-artifact/pkg/controller/moduleconfig/cidrs_validator.go

@@ -46,6 +46,9 @@ func (v cidrsValidator) ValidateUpdate(ctx context.Context, _, newMC *mcapi.Modu
 	if err != nil {
 		return admission.Warnings{}, err
 	}
+	if len(cidrs) == 0 {
+		return admission.Warnings{}, nil
+	}
 
 	err = CheckCIDRsOverlap(cidrs)
 	if err != nil {

images/virtualization-artifact/pkg/controller/moduleconfig/config_state.go

@@ -0,0 +1,31 @@
+package moduleconfig
+
+import (
+	"context"
+	"fmt"
+
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	mcapi "github.com/deckhouse/virtualization-controller/pkg/controller/moduleconfig/api"
+)
+
+func HasVirtualMachineCIDRs(settings mcapi.SettingsValues) (bool, error) {
+	cidrs, err := ParseCIDRs(settings)
+	if err != nil {
+		return false, err
+	}
+	return len(cidrs) > 0, nil
+}
+
+func ModuleConfigHasVirtualMachineCIDRs(ctx context.Context, c client.Client) (bool, error) {
+	if c == nil {
+		return false, fmt.Errorf("kubernetes client is nil")
+	}
+
+	var moduleConfig mcapi.ModuleConfig
+	if err := c.Get(ctx, client.ObjectKey{Name: moduleConfigName}, &moduleConfig); err != nil {
+		return false, fmt.Errorf("get ModuleConfig/%s: %w", moduleConfigName, err)
+	}
+
+	return HasVirtualMachineCIDRs(moduleConfig.Spec.Settings)
+}

images/virtualization-artifact/pkg/controller/moduleconfig/validators_test.go

@@ -0,0 +1,73 @@
+package moduleconfig
+
+import (
+	"context"
+	"net/netip"
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	appconfig "github.com/deckhouse/virtualization-controller/pkg/config"
+	mcapi "github.com/deckhouse/virtualization-controller/pkg/controller/moduleconfig/api"
+	"github.com/deckhouse/virtualization/api/core/v1alpha2"
+)
+
+func TestCIDRsValidatorValidateUpdate(t *testing.T) {
+	scheme := runtime.NewScheme()
+	if err := corev1.AddToScheme(scheme); err != nil {
+		t.Fatalf("AddToScheme: %v", err)
+	}
+
+	podSubnet := netip.MustParsePrefix("10.111.0.0/16")
+	serviceSubnet := netip.MustParsePrefix("10.222.0.0/16")
+
+	validator := newCIDRsValidator(fake.NewClientBuilder().WithScheme(scheme).Build(), &appconfig.ClusterSubnets{
+		PodSubnet:     podSubnet,
+		ServiceSubnet: serviceSubnet,
+	})
+
+	newMC := &mcapi.ModuleConfig{Spec: mcapi.ModuleConfigSpec{Settings: mcapi.SettingsValues{"dvcr": map[string]any{}}}}
+	if _, err := validator.ValidateUpdate(context.Background(), nil, newMC); err != nil {
+		t.Fatalf("expected no error without CIDRs, got: %v", err)
+	}
+
+	newMC.Spec.Settings["virtualMachineCIDRs"] = []any{"10.111.0.0/24", "10.111.0.0/25"}
+	if _, err := validator.ValidateUpdate(context.Background(), nil, newMC); err == nil {
+		t.Fatalf("expected overlap validation error")
+	}
+}
+
+func TestRemoveCIDRsValidatorValidateUpdate(t *testing.T) {
+	scheme := runtime.NewScheme()
+	if err := v1alpha2.AddToScheme(scheme); err != nil {
+		t.Fatalf("AddToScheme: %v", err)
+	}
+
+	oldNone := &mcapi.ModuleConfig{Spec: mcapi.ModuleConfigSpec{Settings: mcapi.SettingsValues{"dvcr": map[string]any{}}}}
+	newNone := &mcapi.ModuleConfig{Spec: mcapi.ModuleConfigSpec{Settings: mcapi.SettingsValues{"dvcr": map[string]any{}}}}
+	newSome := &mcapi.ModuleConfig{Spec: mcapi.ModuleConfigSpec{Settings: mcapi.SettingsValues{"dvcr": map[string]any{}, "virtualMachineCIDRs": []any{"10.0.0.0/24"}}}}
+	oldSome := &mcapi.ModuleConfig{Spec: mcapi.ModuleConfigSpec{Settings: mcapi.SettingsValues{"dvcr": map[string]any{}, "virtualMachineCIDRs": []any{"10.0.0.0/24"}}}}
+
+	validator := newRemoveCIDRsValidator(fake.NewClientBuilder().WithScheme(scheme).Build())
+	for name, pair := range map[string]struct{ oldMC, newMC *mcapi.ModuleConfig }{
+		"old none new none":                {oldNone, newNone},
+		"old none new some":                {oldNone, newSome},
+		"old some new none without leases": {oldSome, newNone},
+		"old some new some same":           {oldSome, newSome},
+	} {
+		t.Run(name, func(t *testing.T) {
+			if _, err := validator.ValidateUpdate(context.Background(), pair.oldMC, pair.newMC); err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+		})
+	}
+
+	lease := &v1alpha2.VirtualMachineIPAddressLease{ObjectMeta: metav1.ObjectMeta{Name: "ip-10-0-0-10"}}
+	validator = newRemoveCIDRsValidator(fake.NewClientBuilder().WithScheme(scheme).WithObjects(lease).Build())
+	if _, err := validator.ValidateUpdate(context.Background(), oldSome, newNone); err == nil {
+		t.Fatalf("expected lease protection error")
+	}
+}

images/virtualization-artifact/pkg/controller/vm/internal/validators/ipam_validator.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/deckhouse/virtualization-controller/pkg/common/network"
 	"github.com/deckhouse/virtualization-controller/pkg/common/object"
+	"github.com/deckhouse/virtualization-controller/pkg/controller/moduleconfig"
 	"github.com/deckhouse/virtualization/api/core/v1alpha2"
 )
 
@@ -38,8 +39,8 @@ func NewIPAMValidator(client client.Client) *IPAMValidator {
 }
 
 func (v *IPAMValidator) ValidateCreate(ctx context.Context, vm *v1alpha2.VirtualMachine) (admission.Warnings, error) {
-	if vm.Spec.VirtualMachineIPAddress != "" && len(vm.Spec.Networks) > 0 && !network.HasMainNetworkSpec(vm.Spec.Networks) {
-		return nil, fmt.Errorf("spec.virtualMachineIPAddressName cannot be set without Main network type in spec.networks")
+	if err := v.validateVMIPUsage(ctx, vm); err != nil {
+		return nil, err
 	}
 
 	vmipName := vm.Spec.VirtualMachineIPAddress
@@ -67,8 +68,8 @@ func (v *IPAMValidator) ValidateCreate(ctx context.Context, vm *v1alpha2.Virtual
 }
 
 func (v *IPAMValidator) ValidateUpdate(ctx context.Context, oldVM, newVM *v1alpha2.VirtualMachine) (admission.Warnings, error) {
-	if newVM.Spec.VirtualMachineIPAddress != "" && len(newVM.Spec.Networks) > 0 && !network.HasMainNetworkSpec(newVM.Spec.Networks) {
-		return nil, fmt.Errorf("spec.virtualMachineIPAddressName cannot be set without Main network type in spec.networks")
+	if err := v.validateVMIPUsage(ctx, newVM); err != nil {
+		return nil, err
 	}
 
 	if oldVM.Spec.VirtualMachineIPAddress == newVM.Spec.VirtualMachineIPAddress {
@@ -91,3 +92,23 @@ func (v *IPAMValidator) ValidateUpdate(ctx context.Context, oldVM, newVM *v1alph
 
 	return nil, nil
 }
+
+func (v *IPAMValidator) validateVMIPUsage(ctx context.Context, vm *v1alpha2.VirtualMachine) error {
+	if vm.Spec.VirtualMachineIPAddress == "" {
+		return nil
+	}
+
+	hasCIDRs, err := moduleconfig.ModuleConfigHasVirtualMachineCIDRs(ctx, v.client)
+	if err != nil {
+		return fmt.Errorf("unable to check ModuleConfig virtualization virtualMachineCIDRs: %w", err)
+	}
+	if !hasCIDRs {
+		return fmt.Errorf("spec.virtualMachineIPAddressName cannot be set when ModuleConfig/virtualization spec.settings.virtualMachineCIDRs is not configured")
+	}
+
+	if len(vm.Spec.Networks) > 0 && !network.HasMainNetworkSpec(vm.Spec.Networks) {
+		return fmt.Errorf("spec.virtualMachineIPAddressName cannot be set without Main network type in spec.networks")
+	}
+
+	return nil
+}