Validate vip_subnet for legacy vip address at runtime (#1500)

* Validate vip_subnet for legacy vip address at runtime

Signed-off-by: orkun incili <orkunincili0@gmail.com>

* Fix lifecycle: only cancel on init/config failure paths

Signed-off-by: orkun incili <orkunincili0@gmail.com>

* Update changelog for failure-path cancel behavior

Signed-off-by: orkun incili <orkunincili0@gmail.com>

---------

Signed-off-by: orkun incili <orkunincili0@gmail.com>

CHANGELOG.md

@@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 - Retry on 403 Forbidden and 401 Unauthorized in `ServicesWatcher` at startup with exponential backoff. Fixes #1464.
 - Reintroduce BGP config via node annotations. Fixes #1488.
+- Fail fast in runtime `manager` and `service` paths when legacy `vip_address` is used without `vip_subnet` in control-plane ARP, BGP, or Routing Table mode.
+- Cancel the mode context on init or configuration failure before waiting on goroutines during shutdown.
+
 
 ### Added
 - SIGUSR1 signal handler for runtime configuration dumps (#1301)

cmd/kube-vip.go

@@ -235,6 +235,15 @@ var kubeVipService = &cobra.Command{
 			configMap = envConfigMap
 		}
 
+		// Legacy vip_address requires vip_subnet for control-plane ARP, BGP, and Routing Table modes.
+		if initConfig.EnableControlPlane &&
+			(initConfig.EnableARP || initConfig.EnableBGP || initConfig.EnableRoutingTable) {
+			if err := initConfig.CheckSubnetExists(); err != nil {
+				log.Error("checking subnet exists if vip_address defined", "err", err)
+				return
+			}
+		}
+
 		// Ensure there is an address to generate the CIDR from
 		if initConfig.VIPSubnet == "" && initConfig.Address != "" {
 			initConfig.VIPSubnet, err = GenerateCidrRange(initConfig.Address, initConfig.DNSMode)
@@ -291,6 +300,15 @@ var kubeVipManager = &cobra.Command{
 		// Set the logging level for all subsequent functions
 		log.SetLogLoggerLevel(log.Level(initConfig.Logging))
 
+		// Legacy vip_address requires vip_subnet for control-plane ARP, BGP, and Routing Table modes.
+		if initConfig.EnableControlPlane &&
+			(initConfig.EnableARP || initConfig.EnableBGP || initConfig.EnableRoutingTable) {
+			if err := initConfig.CheckSubnetExists(); err != nil {
+				log.Error("checking subnet exists if vip_address defined", "err", err)
+				return
+			}
+		}
+
 		// Ensure there is an address to generate the CIDR from
 		if initConfig.VIPSubnet == "" && initConfig.Address != "" {
 			initConfig.VIPSubnet, err = GenerateCidrRange(initConfig.Address, initConfig.DNSMode)

pkg/kubevip/config_manager.go

@@ -12,6 +12,14 @@ const (
 	Auto = "auto"
 )
 
+func (c *Config) CheckSubnetExists() error {
+	if c.VIPSubnet == "" && c.VIP != "" && c.Address == "" {
+		return fmt.Errorf("vip_subnet must be set if using vip_address instead of address environment variable")
+	}
+
+	return nil
+}
+
 func (c *Config) CheckInterface() error {
 	if c.Interface != "" {
 		if err := isValidInterface(c.Interface); err != nil {

pkg/kubevip/config_manager_test.go

@@ -0,0 +1,56 @@
+package kubevip
+
+import "testing"
+
+func TestCheckSubnetExists(t *testing.T) {
+	tests := []struct {
+		name    string
+		config  Config
+		wantErr bool
+	}{
+		{
+			name: "vip only without subnet returns error",
+			config: Config{
+				VIP: "172.18.0.20",
+			},
+			wantErr: true,
+		},
+		{
+			name: "vip with subnet does not return error",
+			config: Config{
+				VIP:       "172.18.0.20",
+				VIPSubnet: "32",
+			},
+			wantErr: false,
+		},
+		{
+			name: "address only without subnet does not return error",
+			config: Config{
+				Address: "172.18.0.20",
+			},
+			wantErr: false,
+		},
+		{
+			name: "address overrides vip without subnet",
+			config: Config{
+				VIP:     "172.18.0.20",
+				Address: "172.18.0.30",
+			},
+			wantErr: false,
+		},
+		{
+			name:    "empty config does not return error",
+			config:  Config{},
+			wantErr: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := tt.config.CheckSubnetExists()
+			if (err != nil) != tt.wantErr {
+				t.Errorf("CheckSubnetExists() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}

pkg/manager/manager.go

@@ -344,7 +344,7 @@ func (sm *Manager) startMode(ctx context.Context) error {
 	wg := sync.WaitGroup{}
 	modeCtx, cancel := context.WithCancel(ctx)
 	defer func() {
-		// wait for gorutines then cancel context just in case
+
 		wg.Wait()
 		w.Cleanup()
 		cancel()
@@ -353,12 +353,14 @@ func (sm *Manager) startMode(ctx context.Context) error {
 
 	log.Info("starting Kube-vip Manager", "mode", w.Name())
 	if err := w.Configure(modeCtx, &wg); err != nil {
+		defer cancel()
 		return fmt.Errorf("failed to configure %s mode: %w", w.Name(), err)
 	}
 
 	if sm.config.EnableControlPlane {
 		err = w.InitControlPlane()
 		if err != nil {
+			defer cancel()
 			return err
 		}
 	}