Fix getLoadBalancerByID accidentally deleting non CCM managed rules + default to TCP proto in lb rules if proto empty

cloudstack/cloudstack_loadbalancer.go

@@ -667,7 +667,8 @@ func (cs *CSCloud) getLoadBalancerByID(name, ipAddrID, networkID string) (*loadB
 		return nil, fmt.Errorf("error retrieving load balancer rules by IP ID %v: %w", ipAddrID, err)
 	}
 
-	for _, lbRule := range l.LoadBalancerRules {
+	filtered := filterRulesByPrefix(l.LoadBalancerRules, lb.name+"-")
+	for _, lbRule := range filtered {
 		lb.rules[lbRule.Name] = lbRule
 
 		if lb.ipAddr != "" && lb.ipAddr != lbRule.Publicip {

cloudstack/cloudstack_loadbalancer_test.go

@@ -4172,8 +4172,8 @@ func TestGetLoadBalancerByID(t *testing.T) {
 		listResp := &cloudstack.ListLoadBalancerRulesResponse{
 			Count: 2,
 			LoadBalancerRules: []*cloudstack.LoadBalancerRule{
-				{Name: "lb-tcp-80", Publicip: "1.2.3.4", Publicipid: "ip-1", Networkid: "net-1"},
-				{Name: "lb-tcp-443", Publicip: "1.2.3.4", Publicipid: "ip-1", Networkid: "net-1"},
+				{Name: "my-lb-tcp-80", Publicip: "1.2.3.4", Publicipid: "ip-1", Networkid: "net-1"},
+				{Name: "my-lb-tcp-443", Publicip: "1.2.3.4", Publicipid: "ip-1", Networkid: "net-1"},
 			},
 		}
 
@@ -4204,6 +4204,51 @@ func TestGetLoadBalancerByID(t *testing.T) {
 		}
 	})
 
+	t.Run("filters out rules not matching LB name prefix", func(t *testing.T) {
+		ctrl := gomock.NewController(t)
+		t.Cleanup(ctrl.Finish)
+
+		mockLB := cloudstack.NewMockLoadBalancerServiceIface(ctrl)
+		listParams := &cloudstack.ListLoadBalancerRulesParams{}
+
+		// API returns rules from multiple services sharing the same IP
+		listResp := &cloudstack.ListLoadBalancerRulesResponse{
+			Count: 4,
+			LoadBalancerRules: []*cloudstack.LoadBalancerRule{
+				{Name: "my-lb-tcp-80", Publicip: "1.2.3.4", Publicipid: "ip-1", Networkid: "net-1"},
+				{Name: "my-lb-tcp-443", Publicip: "1.2.3.4", Publicipid: "ip-1", Networkid: "net-1"},
+				{Name: "other-svc-tcp-8080", Publicip: "1.2.3.4", Publicipid: "ip-1", Networkid: "net-1"},
+				{Name: "another-svc-tcp-9090", Publicip: "1.2.3.4", Publicipid: "ip-1", Networkid: "net-1"},
+			},
+		}
+
+		mockLB.EXPECT().NewListLoadBalancerRulesParams().Return(listParams)
+		mockLB.EXPECT().ListLoadBalancerRules(gomock.Any()).Return(listResp, nil)
+
+		cs := &CSCloud{
+			client: &cloudstack.CloudStackClient{
+				LoadBalancer: mockLB,
+			},
+		}
+
+		lb, err := cs.getLoadBalancerByID("my-lb", "ip-1", "net-1")
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if len(lb.rules) != 2 {
+			t.Fatalf("expected 2 rules (only my-lb- prefix), got %d", len(lb.rules))
+		}
+		if _, ok := lb.rules["my-lb-tcp-80"]; !ok {
+			t.Error("expected rule my-lb-tcp-80 to be present")
+		}
+		if _, ok := lb.rules["my-lb-tcp-443"]; !ok {
+			t.Error("expected rule my-lb-tcp-443 to be present")
+		}
+		if _, ok := lb.rules["other-svc-tcp-8080"]; ok {
+			t.Error("rule other-svc-tcp-8080 should have been filtered out")
+		}
+	})
+
 	t.Run("no rules found", func(t *testing.T) {
 		ctrl := gomock.NewController(t)
 		t.Cleanup(ctrl.Finish)
@@ -4341,7 +4386,7 @@ func TestGetLoadBalancerOrchestrator(t *testing.T) {
 		idResp := &cloudstack.ListLoadBalancerRulesResponse{
 			Count: 1,
 			LoadBalancerRules: []*cloudstack.LoadBalancerRule{
-				{Name: "lb-tcp-80", Publicip: "1.2.3.4", Publicipid: "ip-1", Networkid: "net-1"},
+				{Name: "my-lb-tcp-80", Publicip: "1.2.3.4", Publicipid: "ip-1", Networkid: "net-1"},
 			},
 		}
 

cloudstack/protocol.go

@@ -101,6 +101,8 @@ func ProtocolFromServicePort(port corev1.ServicePort, service *corev1.Service) L
 // CloudStack load balancer protocol name.
 func ProtocolFromLoadBalancer(protocol string) LoadBalancerProtocol {
 	switch protocol {
+	case "":
+		fallthrough
 	case ProtoTCP:
 		return LoadBalancerProtocolTCP
 	case ProtoUDP:

cloudstack/protocol_test.go

@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package cloudstack
+
+import (
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestLoadBalancerProtocol_String(t *testing.T) {
+	tests := []struct {
+		name     string
+		protocol LoadBalancerProtocol
+		want     string
+	}{
+		{"TCP", LoadBalancerProtocolTCP, "tcp"},
+		{"UDP", LoadBalancerProtocolUDP, "udp"},
+		{"TCPProxy", LoadBalancerProtocolTCPProxy, "tcp-proxy"},
+		{"Invalid", LoadBalancerProtocolInvalid, ""},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.protocol.String(); got != tt.want {
+				t.Errorf("String() = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestLoadBalancerProtocol_CSProtocol(t *testing.T) {
+	tests := []struct {
+		name     string
+		protocol LoadBalancerProtocol
+		want     string
+	}{
+		{"TCP", LoadBalancerProtocolTCP, "tcp"},
+		{"UDP", LoadBalancerProtocolUDP, "udp"},
+		{"TCPProxy", LoadBalancerProtocolTCPProxy, "tcp-proxy"},
+		{"Invalid", LoadBalancerProtocolInvalid, ""},
+		{"Unknown value", LoadBalancerProtocol(99), ""},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.protocol.CSProtocol(); got != tt.want {
+				t.Errorf("CSProtocol() = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestLoadBalancerProtocol_IPProtocol(t *testing.T) {
+	tests := []struct {
+		name     string
+		protocol LoadBalancerProtocol
+		want     string
+	}{
+		{"TCP", LoadBalancerProtocolTCP, "tcp"},
+		{"UDP", LoadBalancerProtocolUDP, "udp"},
+		{"TCPProxy maps to tcp", LoadBalancerProtocolTCPProxy, "tcp"},
+		{"Invalid", LoadBalancerProtocolInvalid, ""},
+		{"Unknown value", LoadBalancerProtocol(99), ""},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.protocol.IPProtocol(); got != tt.want {
+				t.Errorf("IPProtocol() = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestProtocolFromServicePort(t *testing.T) {
+	tests := []struct {
+		name        string
+		port        corev1.ServicePort
+		annotations map[string]string
+		want        LoadBalancerProtocol
+	}{
+		{
+			name: "TCP without proxy",
+			port: corev1.ServicePort{Protocol: corev1.ProtocolTCP},
+			want: LoadBalancerProtocolTCP,
+		},
+		{
+			name: "TCP with proxy annotation",
+			port: corev1.ServicePort{Protocol: corev1.ProtocolTCP},
+			annotations: map[string]string{
+				ServiceAnnotationLoadBalancerProxyProtocol: "true",
+			},
+			want: LoadBalancerProtocolTCPProxy,
+		},
+		{
+			name: "TCP with proxy annotation false",
+			port: corev1.ServicePort{Protocol: corev1.ProtocolTCP},
+			annotations: map[string]string{
+				ServiceAnnotationLoadBalancerProxyProtocol: "false",
+			},
+			want: LoadBalancerProtocolTCP,
+		},
+		{
+			name: "UDP",
+			port: corev1.ServicePort{Protocol: corev1.ProtocolUDP},
+			want: LoadBalancerProtocolUDP,
+		},
+		{
+			name: "SCTP is invalid",
+			port: corev1.ServicePort{Protocol: corev1.ProtocolSCTP},
+			want: LoadBalancerProtocolInvalid,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			svc := &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "test-svc",
+					Annotations: tt.annotations,
+				},
+			}
+			if got := ProtocolFromServicePort(tt.port, svc); got != tt.want {
+				t.Errorf("ProtocolFromServicePort() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestProtocolFromLoadBalancer(t *testing.T) {
+	tests := []struct {
+		name     string
+		protocol string
+		want     LoadBalancerProtocol
+	}{
+		{"empty string defaults to TCP", "", LoadBalancerProtocolTCP},
+		{"tcp", "tcp", LoadBalancerProtocolTCP},
+		{"udp", "udp", LoadBalancerProtocolUDP},
+		{"tcp-proxy", "tcp-proxy", LoadBalancerProtocolTCPProxy},
+		{"unknown protocol", "sctp", LoadBalancerProtocolInvalid},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := ProtocolFromLoadBalancer(tt.protocol); got != tt.want {
+				t.Errorf("ProtocolFromLoadBalancer(%q) = %v, want %v", tt.protocol, got, tt.want)
+			}
+		})
+	}
+}