diff --git a/pkg/ccm/loadbalancer.go b/pkg/ccm/loadbalancer.go index f3abe8d6..2f8759f7 100644 --- a/pkg/ccm/loadbalancer.go +++ b/pkg/ccm/loadbalancer.go @@ -148,16 +148,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, l.projectID, name, updatePayload) if err != nil { @@ -300,10 +301,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, l.projectID, name, payload) 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 diff --git a/pkg/ccm/loadbalancer_test.go b/pkg/ccm/loadbalancer_test.go index f0c51f2f..ce3d5bc8 100644 --- a/pkg/ccm/loadbalancer_test.go +++ b/pkg/ccm/loadbalancer_test.go @@ -221,6 +221,67 @@ 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) + 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{ @@ -705,6 +766,24 @@ 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 != 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 + }) +} + // 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 {