From 19f7841813e6b1faf7aafc83e08a8353fae76bdc Mon Sep 17 00:00:00 2001 From: Niclas Schad Date: Thu, 18 Jun 2026 15:53:50 +0200 Subject: [PATCH 1/8] Set DisableTargetSecurityGroupAssignment to "true" for new loadbalancers Signed-off-by: Niclas Schad --- pkg/ccm/loadbalancer.go | 6 ++++-- pkg/ccm/loadbalancer_spec.go | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/ccm/loadbalancer.go b/pkg/ccm/loadbalancer.go index 35df7e90..2d1fe931 100644 --- a/pkg/ccm/loadbalancer.go +++ b/pkg/ccm/loadbalancer.go @@ -154,8 +154,10 @@ func (l *LoadBalancer) EnsureLoadBalancer( //nolint:gocyclo // not really comple PlanId: spec.PlanId, Region: spec.Region, Labels: spec.Labels, - TargetPools: spec.TargetPools, - Version: spec.Version, + // we keep the current setting for DisableTargetSecurityGroupAssignment + DisableTargetSecurityGroupAssignment: lb.DisableTargetSecurityGroupAssignment, + TargetPools: spec.TargetPools, + Version: spec.Version, } lb, err = l.client.UpdateLoadBalancer(ctx, name, updatePayload) if err != nil { diff --git a/pkg/ccm/loadbalancer_spec.go b/pkg/ccm/loadbalancer_spec.go index e88ff129..938dc113 100644 --- a/pkg/ccm/loadbalancer_spec.go +++ b/pkg/ccm/loadbalancer_spec.go @@ -294,6 +294,9 @@ func lbSpecFromService( lb.Labels = new(opts.ExtraLabels) } + // For new lb's always set DisableTargetSecurityGroupAssignment to true + lb.DisableTargetSecurityGroupAssignment = new(true) + // Add metric metricsRemoteWrite settings lb.Options.Observability = observability From 2c92dc524aa87903747fd26be8ebc4027c30b0aa Mon Sep 17 00:00:00 2001 From: Niclas Schad Date: Thu, 18 Jun 2026 16:11:44 +0200 Subject: [PATCH 2/8] do not unnecessarily set DisableTargetSecurityGroupAssignment in Update call Signed-off-by: Niclas Schad --- pkg/ccm/loadbalancer.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/ccm/loadbalancer.go b/pkg/ccm/loadbalancer.go index 2d1fe931..35df7e90 100644 --- a/pkg/ccm/loadbalancer.go +++ b/pkg/ccm/loadbalancer.go @@ -154,10 +154,8 @@ func (l *LoadBalancer) EnsureLoadBalancer( //nolint:gocyclo // not really comple PlanId: spec.PlanId, Region: spec.Region, Labels: spec.Labels, - // we keep the current setting for DisableTargetSecurityGroupAssignment - DisableTargetSecurityGroupAssignment: lb.DisableTargetSecurityGroupAssignment, - TargetPools: spec.TargetPools, - Version: spec.Version, + TargetPools: spec.TargetPools, + Version: spec.Version, } lb, err = l.client.UpdateLoadBalancer(ctx, name, updatePayload) if err != nil { From f6a3ecd3c60324d57d7a40cf61ae42e7ae16f32e Mon Sep 17 00:00:00 2001 From: Niclas Schad Date: Thu, 18 Jun 2026 16:54:09 +0200 Subject: [PATCH 3/8] add unit test Signed-off-by: Niclas Schad --- pkg/ccm/loadbalancer_test.go | 50 ++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/pkg/ccm/loadbalancer_test.go b/pkg/ccm/loadbalancer_test.go index e9cb66ad..9cb08e7f 100644 --- a/pkg/ccm/loadbalancer_test.go +++ b/pkg/ccm/loadbalancer_test.go @@ -220,6 +220,47 @@ var _ = Describe("LoadBalancer", func() { // Expected CreateLoadBalancer to have been called. }) + It("should not set DisableTargetSecurityGroupAssignment to true when lb is updated", func() { + svc := minimalLoadBalancerService() + spec, _, err := lbSpecFromService(svc, []*corev1.Node{}, lbOpts, nil) + Expect(err).NotTo(HaveOccurred()) + myLb := &loadbalancer.LoadBalancer{ + Errors: []loadbalancer.LoadBalancerError{}, + ExternalAddress: spec.ExternalAddress, + Listeners: spec.Listeners, + Name: spec.Name, + Networks: spec.Networks, + Options: spec.Options, + PrivateAddress: spec.PrivateAddress, + Status: new(loadbalancer.LOADBALANCERSTATUS_STATUS_READY), + TargetPools: spec.TargetPools, + DisableTargetSecurityGroupAssignment: new(false), + Version: new("current-version"), + PlanId: new(p10), + } + + mockClient.EXPECT().GetLoadBalancer(gomock.Any(), projectID, gomock.Any()).Return(myLb, nil) + // For simplicity, we return the original load balancer. In reality, the updated load balancer should be returned. + mockClient.EXPECT().UpdateLoadBalancer( + gomock.Any(), + projectID, + loadBalancer.GetLoadBalancerName(context.Background(), clusterName, svc), + hasNotEnabledDisableTargetSecurityGroupAssignment(), + ).MinTimes(1).Return(myLb, nil) + + // trigger an update by changing the service + svc = svc.DeepCopy() + svc.Spec.Ports = append(svc.Spec.Ports, corev1.ServicePort{ + Name: "a-port", + Protocol: corev1.ProtocolTCP, + Port: 80, + NodePort: 1234, + }) + + _, err = loadBalancer.EnsureLoadBalancer(context.Background(), clusterName, svc, []*corev1.Node{}) + Expect(err).NotTo(HaveOccurred()) + }) + It("should update observability credential if credentials are specified in load balancer", func() { svc := minimalLoadBalancerService() spec, _, err := lbSpecFromService(svc, []*corev1.Node{}, lbOpts, &loadbalancer.LoadbalancerOptionObservability{ @@ -715,6 +756,15 @@ func hasNoObservabilityConfigured() gomock.Matcher { }) } +// hasNotEnabledDisableTargetSecurityGroupAssignment ensures that the given UpdateLoadBalancerPayload +// has not set DisableTargetSecurityGroupAssignment to true. +func hasNotEnabledDisableTargetSecurityGroupAssignment() gomock.Matcher { + return gomock.Cond(func(x any) bool { + lb := x.(*loadbalancer.UpdateLoadBalancerPayload) + return lb.DisableTargetSecurityGroupAssignment != new(true) + }) +} + // externalAddressSet ensures that the given UpdateLoadBalancerPayload has external address set to address. func externalAddressSet(address string) gomock.Matcher { return gomock.Cond(func(x any) bool { From 55e9762326aea41a935b37ad38c21688e280d271 Mon Sep 17 00:00:00 2001 From: Niclas Schad Date: Fri, 19 Jun 2026 12:42:41 +0200 Subject: [PATCH 4/8] set DisableTargetSecurityGroupAssignment for UPDATE calls Signed-off-by: Niclas Schad --- pkg/ccm/loadbalancer.go | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/pkg/ccm/loadbalancer.go b/pkg/ccm/loadbalancer.go index 35df7e90..8605c88b 100644 --- a/pkg/ccm/loadbalancer.go +++ b/pkg/ccm/loadbalancer.go @@ -146,16 +146,17 @@ func (l *LoadBalancer) EnsureLoadBalancer( //nolint:gocyclo // not really comple spec.Version = lb.Version spec.Name = &name updatePayload := &loadbalancer.UpdateLoadBalancerPayload{ - ExternalAddress: spec.ExternalAddress, - Listeners: spec.Listeners, - Name: spec.Name, - Networks: spec.Networks, - Options: spec.Options, - PlanId: spec.PlanId, - Region: spec.Region, - Labels: spec.Labels, - TargetPools: spec.TargetPools, - Version: spec.Version, + ExternalAddress: spec.ExternalAddress, + Listeners: spec.Listeners, + Name: spec.Name, + Networks: spec.Networks, + Options: spec.Options, + PlanId: spec.PlanId, + Region: spec.Region, + Labels: spec.Labels, + TargetPools: spec.TargetPools, + DisableTargetSecurityGroupAssignment: lb.DisableTargetSecurityGroupAssignment, + Version: spec.Version, } lb, err = l.client.UpdateLoadBalancer(ctx, name, updatePayload) if err != nil { @@ -298,10 +299,11 @@ func (l *LoadBalancer) EnsureLoadBalancerDeleted( Observability: nil, PrivateNetworkOnly: lb.Options.PrivateNetworkOnly, }, - TargetPools: lb.TargetPools, - Version: lb.Version, - PlanId: lb.PlanId, - Labels: lb.Labels, + TargetPools: lb.TargetPools, + DisableTargetSecurityGroupAssignment: lb.DisableTargetSecurityGroupAssignment, + Version: lb.Version, + PlanId: lb.PlanId, + Labels: lb.Labels, } _, err = l.client.UpdateLoadBalancer(ctx, name, payload) if err != nil { From 13af114290ed6f12001994aa4bdd1b094c48d48d Mon Sep 17 00:00:00 2001 From: Niclas Schad Date: Fri, 19 Jun 2026 12:43:11 +0200 Subject: [PATCH 5/8] add unit test for LB creation with DisableTargetSecurityGroupAssignment set to true Signed-off-by: Niclas Schad --- pkg/ccm/loadbalancer_test.go | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/pkg/ccm/loadbalancer_test.go b/pkg/ccm/loadbalancer_test.go index 9cb08e7f..d909385b 100644 --- a/pkg/ccm/loadbalancer_test.go +++ b/pkg/ccm/loadbalancer_test.go @@ -220,6 +220,26 @@ var _ = Describe("LoadBalancer", func() { // Expected CreateLoadBalancer to have been called. }) + It("should create a load balancer with DisableTargetSecurityGroupAssignment set to true", func() { + mockClient.EXPECT().GetLoadBalancer(gomock.Any(), projectID, gomock.Any()).Return(nil, stackit.ErrorNotFound) + mockClient.EXPECT().ListCredentials(gomock.Any(), projectID).Return(&loadbalancer.ListCredentialsResponse{ + Credentials: []loadbalancer.CredentialsResponse{}, + }, nil) + mockClient.EXPECT().CreateCredentials(gomock.Any(), projectID, gomock.Any()).MinTimes(1). + DoAndReturn(func(_ context.Context, _ string, payload loadbalancer.CreateCredentialsPayload) (*loadbalancer.CreateCredentialsResponse, error) { + return &loadbalancer.CreateCredentialsResponse{ + Credential: &loadbalancer.CredentialsResponse{ + CredentialsRef: new("my-credential-ref"), + DisplayName: new(*payload.DisplayName), + Username: new(*payload.Username), + }, + }, nil + }) + mockClient.EXPECT().CreateLoadBalancer(gomock.Any(), projectID, hasEnabledDisableTargetSecurityGroupAssignment()).MinTimes(1).Return(&loadbalancer.LoadBalancer{}, nil) + _, err := lbInModeIgnoreAndObs.EnsureLoadBalancer(context.Background(), clusterName, minimalLoadBalancerService(), []*corev1.Node{}) + Expect(err).To(MatchError(notYetReadyError)) + }) + It("should not set DisableTargetSecurityGroupAssignment to true when lb is updated", func() { svc := minimalLoadBalancerService() spec, _, err := lbSpecFromService(svc, []*corev1.Node{}, lbOpts, nil) @@ -761,7 +781,16 @@ func hasNoObservabilityConfigured() gomock.Matcher { func hasNotEnabledDisableTargetSecurityGroupAssignment() gomock.Matcher { return gomock.Cond(func(x any) bool { lb := x.(*loadbalancer.UpdateLoadBalancerPayload) - return lb.DisableTargetSecurityGroupAssignment != new(true) + return lb.DisableTargetSecurityGroupAssignment != nil && *lb.DisableTargetSecurityGroupAssignment != true + }) +} + +// hasEnabledDisableTargetSecurityGroupAssignment ensures that new lbs are always created +// with DisableTargetSecurityGroupAssignment set to true. +func hasEnabledDisableTargetSecurityGroupAssignment() gomock.Matcher { + return gomock.Cond(func(x any) bool { + lb := x.(*loadbalancer.CreateLoadBalancerPayload) + return lb.DisableTargetSecurityGroupAssignment != nil && *lb.DisableTargetSecurityGroupAssignment == true }) } From 599aa40a1f16dcf799510ce358e4f148ae5421fd Mon Sep 17 00:00:00 2001 From: Niclas Schad Date: Tue, 23 Jun 2026 14:59:11 +0200 Subject: [PATCH 6/8] expand comment Signed-off-by: Niclas Schad --- pkg/ccm/loadbalancer_spec.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/ccm/loadbalancer_spec.go b/pkg/ccm/loadbalancer_spec.go index 938dc113..d18ca712 100644 --- a/pkg/ccm/loadbalancer_spec.go +++ b/pkg/ccm/loadbalancer_spec.go @@ -295,6 +295,8 @@ func lbSpecFromService( } // For new lb's always set DisableTargetSecurityGroupAssignment to true + // This function is also used by the LB UPDATE call, but for UPDATE we just + // use the value from the already created lb instead of this one. lb.DisableTargetSecurityGroupAssignment = new(true) // Add metric metricsRemoteWrite settings From 27a7653156043026e6875994cccbffaeffd116a0 Mon Sep 17 00:00:00 2001 From: Niclas Schad Date: Tue, 23 Jun 2026 15:10:51 +0200 Subject: [PATCH 7/8] rewrite UPDATE lb test; use matrix for either DisableTargetSecurityGroupAssignment set to true or false Signed-off-by: Niclas Schad --- pkg/ccm/loadbalancer_test.go | 105 ++++++++++++++++------------------- 1 file changed, 48 insertions(+), 57 deletions(-) diff --git a/pkg/ccm/loadbalancer_test.go b/pkg/ccm/loadbalancer_test.go index d909385b..dd999307 100644 --- a/pkg/ccm/loadbalancer_test.go +++ b/pkg/ccm/loadbalancer_test.go @@ -220,66 +220,57 @@ var _ = Describe("LoadBalancer", func() { // Expected CreateLoadBalancer to have been called. }) - It("should create a load balancer with DisableTargetSecurityGroupAssignment set to true", func() { - mockClient.EXPECT().GetLoadBalancer(gomock.Any(), projectID, gomock.Any()).Return(nil, stackit.ErrorNotFound) - mockClient.EXPECT().ListCredentials(gomock.Any(), projectID).Return(&loadbalancer.ListCredentialsResponse{ - Credentials: []loadbalancer.CredentialsResponse{}, - }, nil) - mockClient.EXPECT().CreateCredentials(gomock.Any(), projectID, gomock.Any()).MinTimes(1). - DoAndReturn(func(_ context.Context, _ string, payload loadbalancer.CreateCredentialsPayload) (*loadbalancer.CreateCredentialsResponse, error) { - return &loadbalancer.CreateCredentialsResponse{ - Credential: &loadbalancer.CredentialsResponse{ - CredentialsRef: new("my-credential-ref"), - DisplayName: new(*payload.DisplayName), - Username: new(*payload.Username), - }, - }, nil - }) - mockClient.EXPECT().CreateLoadBalancer(gomock.Any(), projectID, hasEnabledDisableTargetSecurityGroupAssignment()).MinTimes(1).Return(&loadbalancer.LoadBalancer{}, nil) - _, err := lbInModeIgnoreAndObs.EnsureLoadBalancer(context.Background(), clusterName, minimalLoadBalancerService(), []*corev1.Node{}) - Expect(err).To(MatchError(notYetReadyError)) - }) + DescribeTable("LoadBalancer UPDATE behavior for DisableTargetSecurityGroupAssignment", + func(disableTargetSG bool, matcher gomock.Matcher) { + svc := minimalLoadBalancerService() + spec, _, err := lbSpecFromService(svc, []*corev1.Node{}, lbOpts, nil) + Expect(err).NotTo(HaveOccurred()) - It("should not set DisableTargetSecurityGroupAssignment to true when lb is updated", func() { - svc := minimalLoadBalancerService() - spec, _, err := lbSpecFromService(svc, []*corev1.Node{}, lbOpts, nil) - Expect(err).NotTo(HaveOccurred()) - myLb := &loadbalancer.LoadBalancer{ - Errors: []loadbalancer.LoadBalancerError{}, - ExternalAddress: spec.ExternalAddress, - Listeners: spec.Listeners, - Name: spec.Name, - Networks: spec.Networks, - Options: spec.Options, - PrivateAddress: spec.PrivateAddress, - Status: new(loadbalancer.LOADBALANCERSTATUS_STATUS_READY), - TargetPools: spec.TargetPools, - DisableTargetSecurityGroupAssignment: new(false), - Version: new("current-version"), - PlanId: new(p10), - } + myLb := &loadbalancer.LoadBalancer{ + Errors: []loadbalancer.LoadBalancerError{}, + ExternalAddress: spec.ExternalAddress, + Listeners: spec.Listeners, + Name: spec.Name, + Networks: spec.Networks, + Options: spec.Options, + PrivateAddress: spec.PrivateAddress, + Status: new(loadbalancer.LOADBALANCERSTATUS_STATUS_READY), + TargetPools: spec.TargetPools, + DisableTargetSecurityGroupAssignment: &disableTargetSG, + Version: new("current-version"), + PlanId: new(p10), + } - mockClient.EXPECT().GetLoadBalancer(gomock.Any(), projectID, gomock.Any()).Return(myLb, nil) - // For simplicity, we return the original load balancer. In reality, the updated load balancer should be returned. - mockClient.EXPECT().UpdateLoadBalancer( - gomock.Any(), - projectID, - loadBalancer.GetLoadBalancerName(context.Background(), clusterName, svc), - hasNotEnabledDisableTargetSecurityGroupAssignment(), - ).MinTimes(1).Return(myLb, nil) + mockClient.EXPECT().GetLoadBalancer(gomock.Any(), projectID, gomock.Any()).Return(myLb, nil) - // trigger an update by changing the service - svc = svc.DeepCopy() - svc.Spec.Ports = append(svc.Spec.Ports, corev1.ServicePort{ - Name: "a-port", - Protocol: corev1.ProtocolTCP, - Port: 80, - NodePort: 1234, - }) + mockClient.EXPECT().UpdateLoadBalancer( + gomock.Any(), + projectID, + loadBalancer.GetLoadBalancerName(context.Background(), clusterName, svc), + matcher, + ).MinTimes(1).Return(myLb, nil) + + // Trigger an update by changing the service + svc = svc.DeepCopy() + svc.Spec.Ports = append(svc.Spec.Ports, corev1.ServicePort{ + Name: "a-port", + Protocol: corev1.ProtocolTCP, + Port: 80, + NodePort: 1234, + }) - _, err = loadBalancer.EnsureLoadBalancer(context.Background(), clusterName, svc, []*corev1.Node{}) - Expect(err).NotTo(HaveOccurred()) - }) + _, err = loadBalancer.EnsureLoadBalancer(context.Background(), clusterName, svc, []*corev1.Node{}) + Expect(err).NotTo(HaveOccurred()) + }, + Entry("should not set DisableTargetSecurityGroupAssignment to true when it is initially false", + false, + hasNotEnabledDisableTargetSecurityGroupAssignment(), + ), + Entry("should keep DisableTargetSecurityGroupAssignment as true when it is initially true", + true, + hasEnabledDisableTargetSecurityGroupAssignment(), + ), + ) It("should update observability credential if credentials are specified in load balancer", func() { svc := minimalLoadBalancerService() @@ -789,7 +780,7 @@ func hasNotEnabledDisableTargetSecurityGroupAssignment() gomock.Matcher { // with DisableTargetSecurityGroupAssignment set to true. func hasEnabledDisableTargetSecurityGroupAssignment() gomock.Matcher { return gomock.Cond(func(x any) bool { - lb := x.(*loadbalancer.CreateLoadBalancerPayload) + lb := x.(*loadbalancer.UpdateLoadBalancerPayload) return lb.DisableTargetSecurityGroupAssignment != nil && *lb.DisableTargetSecurityGroupAssignment == true }) } From 7965b8c449e1287d6bc69a8d9250b5b91538dd02 Mon Sep 17 00:00:00 2001 From: Niclas Schad Date: Tue, 23 Jun 2026 15:18:19 +0200 Subject: [PATCH 8/8] add isCreatedWithEnabledDisableTargetSecurityGroupAssignment() gomock.Match to LB create call to test if DisableTargetSecurityGroupAssignment is set to true Signed-off-by: Niclas Schad --- pkg/ccm/loadbalancer_test.go | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/pkg/ccm/loadbalancer_test.go b/pkg/ccm/loadbalancer_test.go index dd999307..b741ddf5 100644 --- a/pkg/ccm/loadbalancer_test.go +++ b/pkg/ccm/loadbalancer_test.go @@ -212,7 +212,8 @@ var _ = Describe("LoadBalancer", func() { }, }, nil }) - mockClient.EXPECT().CreateLoadBalancer(gomock.Any(), gomock.Any()).MinTimes(1).Return(&loadbalancer.LoadBalancer{}, nil) + // isCreatedWithEnabledDisableTargetSecurityGroupAssignment ensures that DisableTargetSecurityGroupAssignment is set to true + mockClient.EXPECT().CreateLoadBalancer(gomock.Any(), isCreatedWithEnabledDisableTargetSecurityGroupAssignment()).MinTimes(1).Return(&loadbalancer.LoadBalancer{}, nil) _, err := lbInModeIgnoreAndObs.EnsureLoadBalancer(context.Background(), clusterName, minimalLoadBalancerService(), []*corev1.Node{}) Expect(err).To(MatchError(notYetReadyError)) @@ -241,11 +242,10 @@ var _ = Describe("LoadBalancer", func() { PlanId: new(p10), } - mockClient.EXPECT().GetLoadBalancer(gomock.Any(), projectID, gomock.Any()).Return(myLb, nil) + mockClient.EXPECT().GetLoadBalancer(gomock.Any(), gomock.Any()).Return(myLb, nil) mockClient.EXPECT().UpdateLoadBalancer( gomock.Any(), - projectID, loadBalancer.GetLoadBalancerName(context.Background(), clusterName, svc), matcher, ).MinTimes(1).Return(myLb, nil) @@ -776,8 +776,8 @@ func hasNotEnabledDisableTargetSecurityGroupAssignment() gomock.Matcher { }) } -// hasEnabledDisableTargetSecurityGroupAssignment ensures that new lbs are always created -// with DisableTargetSecurityGroupAssignment set to true. +// hasEnabledDisableTargetSecurityGroupAssignment ensures that the given UpdateLoadBalancerPayload +// has set DisableTargetSecurityGroupAssignment to true. func hasEnabledDisableTargetSecurityGroupAssignment() gomock.Matcher { return gomock.Cond(func(x any) bool { lb := x.(*loadbalancer.UpdateLoadBalancerPayload) @@ -785,6 +785,15 @@ func hasEnabledDisableTargetSecurityGroupAssignment() gomock.Matcher { }) } +// isCreatedWithEnabledDisableTargetSecurityGroupAssignment ensures that new lbs are always created +// with DisableTargetSecurityGroupAssignment set to true. +func isCreatedWithEnabledDisableTargetSecurityGroupAssignment() gomock.Matcher { + return gomock.Cond(func(x any) bool { + lb := x.(*loadbalancer.CreateLoadBalancerPayload) + return lb.DisableTargetSecurityGroupAssignment != nil && *lb.DisableTargetSecurityGroupAssignment == true + }) +} + // externalAddressSet ensures that the given UpdateLoadBalancerPayload has external address set to address. func externalAddressSet(address string) gomock.Matcher { return gomock.Cond(func(x any) bool {