From e668e5fdb3b6da0f246f6453a12a122196e8d849 Mon Sep 17 00:00:00 2001 From: Menekse Ceylan Date: Wed, 27 May 2026 14:06:20 +0200 Subject: [PATCH 1/6] fix tls bridging missing tlspool config and add tests and fix typo in tls annotations --- pkg/alb/ingress/alb_spec.go | 20 +++ pkg/alb/ingress/alb_spec_test.go | 268 ++++++++++++++++++------------- pkg/alb/ingress/annotations.go | 6 +- pkg/alb/ingress/resources.go | 6 +- 4 files changed, 182 insertions(+), 118 deletions(-) diff --git a/pkg/alb/ingress/alb_spec.go b/pkg/alb/ingress/alb_spec.go index 6a5dfa86..7f5f9118 100644 --- a/pkg/alb/ingress/alb_spec.go +++ b/pkg/alb/ingress/alb_spec.go @@ -192,6 +192,26 @@ func (r *IngressClassReconciler) getAlbSpecForResources( Targets: targets, ActiveHealthCheck: nil, // TODO } + + if targetPool.tlsEnabled { + albsdkTargetPool.TlsConfig = &albsdk.TlsConfig{ + Enabled: new(bool), + SkipCertificateValidation: nil, + CustomCa: nil, + } + *albsdkTargetPool.TlsConfig.Enabled = true + + if targetPool.skipCertificateValidation { + albsdkTargetPool.TlsConfig.SkipCertificateValidation = new(bool) + *albsdkTargetPool.TlsConfig.SkipCertificateValidation = true + } + + if targetPool.customCA != "" { + albsdkTargetPool.TlsConfig.CustomCa = new(string) + *albsdkTargetPool.TlsConfig.CustomCa = targetPool.customCA + } + } + alb.TargetPools = append(alb.TargetPools, albsdkTargetPool) } diff --git a/pkg/alb/ingress/alb_spec_test.go b/pkg/alb/ingress/alb_spec_test.go index cfc47bce..cc74d853 100644 --- a/pkg/alb/ingress/alb_spec_test.go +++ b/pkg/alb/ingress/alb_spec_test.go @@ -32,8 +32,7 @@ var _ = Describe("Node Controller", func() { node corev1.Node reconciler IngressClassReconciler - - albSpec albsdk.CreateLoadBalancerPayload + albSpec albsdk.CreateLoadBalancerPayload ) BeforeEach(func() { @@ -86,57 +85,7 @@ var _ = Describe("Node Controller", func() { ApplicationLoadBalancer: stackitconfig.ApplicationLoadBalancerOpts{NetworkID: networkID}}, } - albSpec = albsdk.CreateLoadBalancerPayload{ - DisableTargetSecurityGroupAssignment: new(true), - Labels: new(map[string]string{labels.LabelIngressClassUID: "test-ingress-class-uid"}), - Listeners: []albsdk.Listener{ - { - Http: new(albsdk.ProtocolOptionsHTTP{ - Hosts: []albsdk.HostConfig{ - { - Host: new("example.com"), - Rules: []albsdk.Rule{ - { - Path: new(albsdk.Path{ - Prefix: new("/"), - }), - TargetPool: new(fmt.Sprintf("port-%d", service.Spec.Ports[0].NodePort)), - WebSocket: new(false), - }, - }, - }, - }, - }), - Name: new("80-http"), - Port: new(int32(80)), - Protocol: new("PROTOCOL_HTTP"), - }, - }, - Name: new(string(ingressClass.UID)), //todo - Networks: []albsdk.Network{ - { - NetworkId: new(reconciler.ALBConfig.ApplicationLoadBalancer.NetworkID), - Role: new("ROLE_LISTENERS_AND_TARGETS"), - }, - }, - Options: new(albsdk.LoadBalancerOptions{ - EphemeralAddress: new(true), - }), - // Region: new(reconciler.ALBConfig.Global.Region), why is there a region in spec? TODO - TargetPools: []albsdk.TargetPool{ - { - Name: new(fmt.Sprintf("port-%d", service.Spec.Ports[0].NodePort)), - TargetPort: new(service.Spec.Ports[0].NodePort), - Targets: []albsdk.Target{ - { - DisplayName: new(node.Name), - Ip: new(node.Status.Addresses[0].Address), - }, - }, - }, - }, - } - + albSpec = getInitialAlbSpec(&ingressClass, &service, &node, reconciler.ALBConfig.ApplicationLoadBalancer.NetworkID) }) Describe("Generate ALB spec", func() { @@ -151,91 +100,133 @@ var _ = Describe("Node Controller", func() { Context("when handling labels", func() { It("should work with ownership labels", func() { - spec, errorEventList, err := reconciler.getAlbSpecForIngressClass(context.Background(), &ingressClass) Expect(err).To(Succeed()) Expect(errorEventList).To(BeEmpty()) Expect(spec).ToNot(BeNil()) expectedLabels := map[string]string{ - labels.LabelIngressClassUID: string(ingressClass.UID), // Ownership label must be present + labels.LabelIngressClassUID: string(ingressClass.UID), } Expect(*spec).To(BeEquivalentTo(albSpec)) Expect(*spec.Labels).To(BeEquivalentTo(expectedLabels)) }) - }) - It("should work with certificates", func() { + Context("when certificates are configured", func() { + var ( + targetCertID string + ) + + BeforeEach(func() { + // 1. Properly initialize the mock controller + mockCtrl = gomock.NewController(GinkgoT()) + certClient = stackitmocks.NewMockCertificatesClient(mockCtrl) + reconciler.CertificateClient = certClient + + // 2. Clear state pollution by providing a fresh in-memory cluster API server instance + k8sClient = fake.NewClientBuilder(). + WithScheme(scheme.Scheme). + Build() + reconciler.Client = k8sClient + + // reset necessary shared basic entities into the fresh cluster space + ingressClass.ResourceVersion = "" + service.ResourceVersion = "" + node.ResourceVersion = "" + ingressClass.UID = "test-ingress-class-uid" // Preserve initial constant UID value + + // Re-seed necessary shared basic entities into the fresh cluster space + Expect(k8sClient.Create(context.Background(), &ingressClass)).To(Succeed()) + Expect(k8sClient.Create(context.Background(), &service)).To(Succeed()) + Expect(k8sClient.Create(context.Background(), &node)).To(Succeed()) + + // 3. Seed the k8s secret + certSecret := corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-secret-cert", + UID: "dummy-secret-uid-value-1234567", + }, + Type: corev1.SecretTypeTLS, + Data: map[string][]byte{ + "tls.crt": []byte("mock-public-key"), + "tls.key": []byte("mock-private-key"), + }, + } + Expect(k8sClient.Create(context.Background(), &certSecret)).To(Succeed()) - mockCtrl = gomock.NewController(GinkgoT()) - certClient = stackitmocks.NewMockCertificatesClient(mockCtrl) + actualStoredSecret := &corev1.Secret{} + err := k8sClient.Get(context.Background(), client.ObjectKey{Name: "my-secret-cert"}, actualStoredSecret) + Expect(err).NotTo(HaveOccurred()) - // Bind this mock instance to live reconciler reference context - reconciler.CertificateClient = certClient + expectedGeneratedCertName := getCertName(&ingressClass, actualStoredSecret) + targetCertID = "real-certificate-uuid-abc-123" - certSecret := corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-secret-cert", - UID: "dummy-secret-uid-value-1234567", - }, - Type: corev1.SecretTypeTLS, - Data: map[string][]byte{ - "tls.crt": []byte("mock-public-key"), - "tls.key": []byte("mock-private-key"), - }, - } - Expect(k8sClient.Create(context.Background(), &certSecret)).To(Succeed()) + mockResponse := &certsdk.GetCertificateResponse{ + Id: new(targetCertID), + Name: new(expectedGeneratedCertName), + } - actualStoredSecret := &corev1.Secret{} - err := k8sClient.Get(context.Background(), client.ObjectKey{Name: "my-secret-cert"}, actualStoredSecret) - Expect(err).NotTo(HaveOccurred()) + // 4. Setup mock client expectation + certClient.EXPECT(). + CreateCertificate(gomock.Any(), "test-project", "test-region", gomock.Any()). + Return(mockResponse, nil). + AnyTimes() - expectedGeneratedCertName := getCertName(&ingressClass, actualStoredSecret) - targetCertID := "real-certificate-uuid-abc-123" + albSpec = getInitialAlbSpec(&ingressClass, &service, &node, reconciler.ALBConfig.ApplicationLoadBalancer.NetworkID) - mockResponse := &certsdk.GetCertificateResponse{ - Id: new(targetCertID), - Name: new(expectedGeneratedCertName), - } + // 5. Reset expected listeners on albSpec template + httpsListener := testHttpsListener(service.Spec.Ports[0].NodePort, targetCertID) + albSpec.Listeners = []albsdk.Listener{ + httpsListener, + } + }) - certClient.EXPECT(). - CreateCertificate( - gomock.Any(), - "test-project", - "test-region", - gomock.Any(), // Intercepts any incoming *certsdk.CreateCertificatePayload matching - ). - Return(mockResponse, nil). - Times(1) - - httpsIngress := testHttpsIngress(&ingressClass, &service) - if httpsIngress.Annotations == nil { - httpsIngress.Annotations = make(map[string]string) - } - httpsIngress.Annotations = map[string]string{"alb.stackit.cloud/https-only": "true"} + AfterEach(func() { + mockCtrl.Finish() + }) - Expect(k8sClient.Create(context.Background(), &httpsIngress)).To(Succeed()) + It("should work with certificates", func() { + httpsIngress := testHttpsIngress(&ingressClass, &service) + if httpsIngress.Annotations == nil { + httpsIngress.Annotations = make(map[string]string) + } + httpsIngress.Annotations = map[string]string{"alb.stackit.cloud/https-only": "true"} - // expected albSpec should include new https listener - httpListener := testHttpListener(service.Spec.Ports[0].NodePort) - httpsListener := testHttpsListener(service.Spec.Ports[0].NodePort, targetCertID) - albSpec.Listeners = []albsdk.Listener{ - httpsListener, - httpListener, - } + Expect(k8sClient.Create(context.Background(), &httpsIngress)).To(Succeed()) - // get the specs and compare - spec, errorEventList, err := reconciler.getAlbSpecForIngressClass(context.Background(), &ingressClass) - Expect(err).To(Succeed()) - Expect(errorEventList).To(BeEmpty()) + spec, errorEventList, err := reconciler.getAlbSpecForIngressClass(context.Background(), &ingressClass) + Expect(err).To(Succeed()) + Expect(errorEventList).To(BeEmpty()) + Expect(spec).ToNot(BeNil()) + Expect(*spec).To(BeEquivalentTo(albSpec)) + }) - Expect(spec).ToNot(BeNil()) + It("should work with tls bridging", func() { + httpsIngress := testHttpsIngress(&ingressClass, &service) + if httpsIngress.Annotations == nil { + httpsIngress.Annotations = make(map[string]string) + } + httpsIngress.Annotations = map[string]string{ + "alb.stackit.cloud/https-only": "true", + "alb.stackit.cloud/target-pool-tls-enabled": "true", + } - // compare - Expect(*spec).To(BeEquivalentTo(albSpec)) + Expect(k8sClient.Create(context.Background(), &httpsIngress)).To(Succeed()) + + // Corrected Pointer initialization logic to prevent nil dereferences + albSpec.TargetPools[0].TlsConfig = &albsdk.TlsConfig{ + Enabled: new(bool), + } + *albSpec.TargetPools[0].TlsConfig.Enabled = true + spec, errorEventList, err := reconciler.getAlbSpecForIngressClass(context.Background(), &ingressClass) + Expect(err).To(Succeed()) + Expect(errorEventList).To(BeEmpty()) + Expect(spec).ToNot(BeNil()) + Expect(*spec).To(BeEquivalentTo(albSpec)) + }) }) It("should work with 2 ingresses different path", func() { @@ -385,3 +376,56 @@ func testHttpsListener(nodePort int32, certID string) albsdk.Listener { }, } } + +// Add this to the bottom of your test file alongside your other helpers +func getInitialAlbSpec(ingressClass *networkingv1.IngressClass, service *corev1.Service, node *corev1.Node, networkID string) albsdk.CreateLoadBalancerPayload { + return albsdk.CreateLoadBalancerPayload{ + DisableTargetSecurityGroupAssignment: new(true), + Labels: new(map[string]string{labels.LabelIngressClassUID: "test-ingress-class-uid"}), + Listeners: []albsdk.Listener{ + { + Http: new(albsdk.ProtocolOptionsHTTP{ + Hosts: []albsdk.HostConfig{ + { + Host: new("example.com"), + Rules: []albsdk.Rule{ + { + Path: new(albsdk.Path{ + Prefix: new("/"), + }), + TargetPool: new(fmt.Sprintf("port-%d", service.Spec.Ports[0].NodePort)), + WebSocket: new(false), + }, + }, + }, + }, + }), + Name: new("80-http"), + Port: new(int32(80)), + Protocol: new("PROTOCOL_HTTP"), + }, + }, + Name: new(string(ingressClass.UID)), + Networks: []albsdk.Network{ + { + NetworkId: new(networkID), + Role: new("ROLE_LISTENERS_AND_TARGETS"), + }, + }, + Options: new(albsdk.LoadBalancerOptions{ + EphemeralAddress: new(true), + }), + TargetPools: []albsdk.TargetPool{ + { + Name: new(fmt.Sprintf("port-%d", service.Spec.Ports[0].NodePort)), + TargetPort: new(service.Spec.Ports[0].NodePort), + Targets: []albsdk.Target{ + { + DisplayName: new(node.Name), + Ip: new(node.Status.Addresses[0].Address), + }, + }, + }, + }, + } +} diff --git a/pkg/alb/ingress/annotations.go b/pkg/alb/ingress/annotations.go index f2f890fe..a3335fec 100644 --- a/pkg/alb/ingress/annotations.go +++ b/pkg/alb/ingress/annotations.go @@ -24,13 +24,13 @@ const ( // AnnotationTargetPoolTLSEnabled If true, the application load balancer enables TLS bridging. // It uses the trusted CAs from the operating system for validation. // Can be set on IngressClass, Ingress and Service. - AnnotationTargetPoolTLSEnabled = "alb.stackit.cloud/traget-pool-tls-enabled" + AnnotationTargetPoolTLSEnabled = "alb.stackit.cloud/target-pool-tls-enabled" // AnnotationTargetPoolTLSCustomCa If set, the application load balancer enables TLS bridging with a custom CA provided as value. // Can be set on IngressClass, Ingress and Service - AnnotationTargetPoolTLSCustomCa = "alb.stackit.cloud/traget-pool-tls-custom-ca" + AnnotationTargetPoolTLSCustomCa = "alb.stackit.cloud/target-pool-tls-custom-ca" // AnnotationTargetPoolTLSSkipCertificateValidation If true, the application load balancer enables TLS bridging but skips validation. // Can be set on IngressClass, Ingress and Service. - AnnotationTargetPoolTLSSkipCertificateValidation = "alb.stackit.cloud/traget-pool-tls-skip-certificate-validation" + AnnotationTargetPoolTLSSkipCertificateValidation = "alb.stackit.cloud/target-pool-tls-skip-certificate-validation" // AnnotationHTTPPort Specifies the HTTP port. // Can be set on IngressClass and Ingress. diff --git a/pkg/alb/ingress/resources.go b/pkg/alb/ingress/resources.go index 5984d205..16b37ccb 100644 --- a/pkg/alb/ingress/resources.go +++ b/pkg/alb/ingress/resources.go @@ -78,21 +78,21 @@ func mergeTargetPools(dst, src albTargetPools) (albTargetPools, []error) { if dstTargetPool.skipCertificateValidation != srcTargetPool.skipCertificateValidation { mergeErrors = append(mergeErrors, &errorEvent{ ingressRef: srcTargetPool.ingressRef, - description: fmt.Sprintf("%s annotation ignored as it already is configred differently in ingress %v", AnnotationTargetPoolTLSSkipCertificateValidation, dstTargetPool.ingressRef), + description: fmt.Sprintf("%s annotation ignored as it already is configured differently in ingress %v", AnnotationTargetPoolTLSSkipCertificateValidation, dstTargetPool.ingressRef), typ: "Warning", }) } if dstTargetPool.tlsEnabled != srcTargetPool.tlsEnabled { mergeErrors = append(mergeErrors, &errorEvent{ ingressRef: srcTargetPool.ingressRef, - description: fmt.Sprintf("%s annotation ignored as it already is configred differently in ingress %v", AnnotationTargetPoolTLSEnabled, dstTargetPool.ingressRef), + description: fmt.Sprintf("%s annotation ignored as it already is configured differently in ingress %v", AnnotationTargetPoolTLSEnabled, dstTargetPool.ingressRef), typ: "Warning", }) } if dstTargetPool.customCA != srcTargetPool.customCA { mergeErrors = append(mergeErrors, &errorEvent{ ingressRef: srcTargetPool.ingressRef, - description: fmt.Sprintf("%s annotation ignored as it already is configred differently in ingress %v", AnnotationTargetPoolTLSCustomCa, dstTargetPool.ingressRef), + description: fmt.Sprintf("%s annotation ignored as it already is configured differently in ingress %v", AnnotationTargetPoolTLSCustomCa, dstTargetPool.ingressRef), typ: "Warning", }) } From 5570e63d6b5f0e6a760907cf7ebfc1c1f13c5754 Mon Sep 17 00:00:00 2001 From: Menekse Ceylan Date: Thu, 28 May 2026 10:27:09 +0200 Subject: [PATCH 2/6] updated annotation lookup when sorting ingresses for ingressClass Utilized getSortedIngressesForIngressClass for getAlbSpecForIngressClass # Conflicts: # pkg/alb/ingress/alb_spec.go --- pkg/alb/ingress/alb_spec.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/alb/ingress/alb_spec.go b/pkg/alb/ingress/alb_spec.go index 7f5f9118..9d48118f 100644 --- a/pkg/alb/ingress/alb_spec.go +++ b/pkg/alb/ingress/alb_spec.go @@ -22,7 +22,7 @@ func (r *IngressClassReconciler) getAlbSpecForIngressClass( ctx context.Context, class *networkingv1.IngressClass, ) (payload *albsdk.CreateLoadBalancerPayload, validationErrors []error, err error) { - ingresses, err := r.getIngressesForIngressClass(ctx, class) + ingresses, err := r.getSortedIngressesForIngressClass(ctx, class) if err != nil { return nil, nil, err } From db98c01ee6eccc446b2d3aa5d832dc99932ced37 Mon Sep 17 00:00:00 2001 From: Menekse Ceylan Date: Mon, 1 Jun 2026 16:38:14 +0200 Subject: [PATCH 3/6] updated tests --- pkg/alb/ingress/alb_spec_test.go | 105 ++++++++++++++++++++++++------- 1 file changed, 82 insertions(+), 23 deletions(-) diff --git a/pkg/alb/ingress/alb_spec_test.go b/pkg/alb/ingress/alb_spec_test.go index cc74d853..f9b584dd 100644 --- a/pkg/alb/ingress/alb_spec_test.go +++ b/pkg/alb/ingress/alb_spec_test.go @@ -3,6 +3,7 @@ package ingress import ( "context" "fmt" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -229,34 +230,92 @@ var _ = Describe("Node Controller", func() { }) }) - It("should work with 2 ingresses different path", func() { - ingress2 := testIngress(&ingressClass, &service) - ingress2.Name = "ingress2" - ingress2.Spec.Rules[0].HTTP.Paths[0].Path = "/foobar" + Context("when sorting multiple ingresses", func() { + var ( + ingress2 networkingv1.Ingress + ) - Expect(k8sClient.Create(context.Background(), &ingress2)).To(Succeed()) + It("should prioritize priority annotations if defined", func() { - secTargetPool := *albSpec.Listeners[0].Http.Hosts[0].Rules[0].TargetPool - albSpec.Listeners[0].Http.Hosts[0].Rules = []albsdk.Rule{ - { - Path: &albsdk.Path{Prefix: new("/foobar")}, - TargetPool: new(secTargetPool), - WebSocket: new(false), - }, - { - Path: &albsdk.Path{Prefix: new("/")}, - TargetPool: new(secTargetPool), - WebSocket: new(false), - }, - } + err := k8sClient.Get(context.Background(), client.ObjectKey{Namespace: ingress.Namespace, Name: ingress.Name}, &ingress) + Expect(err).NotTo(HaveOccurred()) - spec, errorEventList, err := reconciler.getAlbSpecForIngressClass(context.Background(), &ingressClass) - Expect(err).To(Succeed()) - Expect(errorEventList).To(BeEmpty()) + ingress.Annotations = nil // ensuring no priority annotations are lingering + Expect(k8sClient.Update(context.Background(), &ingress)).To(Succeed()) + + ingress2 = testIngress(&ingressClass, &service) + ingress2.Name = "ingress2" + ingress2.Spec.Rules[0].HTTP.Paths[0].Path = "/foobar" + if ingress2.Annotations == nil { + ingress2.Annotations = make(map[string]string) + } + ingress2.Annotations = map[string]string{"alb.stackit.cloud/priority": "100"} + + Expect(k8sClient.Create(context.Background(), &ingress2)).To(Succeed()) + + secTargetPool := *albSpec.Listeners[0].Http.Hosts[0].Rules[0].TargetPool + albSpec.Listeners[0].Http.Hosts[0].Rules = []albsdk.Rule{ + { + Path: &albsdk.Path{Prefix: new("/foobar")}, + TargetPool: new(secTargetPool), + WebSocket: new(false), + }, + { + Path: &albsdk.Path{Prefix: new("/")}, + TargetPool: new(secTargetPool), + WebSocket: new(false), + }, + } + + spec, errorEventList, err := reconciler.getAlbSpecForIngressClass(context.Background(), &ingressClass) + Expect(err).To(Succeed()) + Expect(errorEventList).To(BeEmpty()) + + Expect(spec).ToNot(BeNil()) + Expect(*spec).To(BeEquivalentTo(albSpec)) + }) + + It("should sort by creationTimeStamp when priority annotations are missing ", func() { + + // manipulate the base ingress' (created in the top-level BeforeEach) creation timestamp to be the oldest + err := k8sClient.Get(context.Background(), client.ObjectKey{Namespace: ingress.Namespace, Name: ingress.Name}, &ingress) + Expect(err).NotTo(HaveOccurred()) + ingress.Annotations = nil // ensuring no priority annotations are lingering + Expect(k8sClient.Update(context.Background(), &ingress)).To(Succeed()) + + time.Sleep(1 * time.Second) + + ingress2 = testIngress(&ingressClass, &service) + ingress2.Name = "ingress2" + ingress2.Spec.Rules[0].HTTP.Paths[0].Path = "/foobar" + ingress2.Annotations = nil // ensuring no priority annotations + + Expect(k8sClient.Create(context.Background(), &ingress2)).To(Succeed()) + + secTargetPool := *albSpec.Listeners[0].Http.Hosts[0].Rules[0].TargetPool + albSpec.Listeners[0].Http.Hosts[0].Rules = []albsdk.Rule{ + { + Path: &albsdk.Path{Prefix: new("/")}, + TargetPool: new(secTargetPool), + WebSocket: new(false), + }, + { + Path: &albsdk.Path{Prefix: new("/foobar")}, + TargetPool: new(secTargetPool), + WebSocket: new(false), + }, + } + + spec, errorEventList, err := reconciler.getAlbSpecForIngressClass(context.Background(), &ingressClass) + Expect(err).To(Succeed()) + Expect(errorEventList).To(BeEmpty()) + + Expect(spec).ToNot(BeNil()) + Expect(*spec).To(BeEquivalentTo(albSpec)) + }) - Expect(spec).ToNot(BeNil()) - Expect(*spec).To(BeEquivalentTo(albSpec)) }) + }) }) From 4a9bac79a21f25bbcac99d2c7aaab08e4a8b4fb7 Mon Sep 17 00:00:00 2001 From: Menekse Ceylan Date: Tue, 2 Jun 2026 09:37:21 +0200 Subject: [PATCH 4/6] introduced sequenceIndex to not lose the order of rules when creating alb specs via getAlbSpecForResources at last step --- pkg/alb/ingress/alb_spec.go | 34 +++++++++++++++++++++++++++++--- pkg/alb/ingress/alb_spec_test.go | 9 +++++---- pkg/alb/ingress/resources.go | 1 + 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/pkg/alb/ingress/alb_spec.go b/pkg/alb/ingress/alb_spec.go index 9d48118f..7b982cef 100644 --- a/pkg/alb/ingress/alb_spec.go +++ b/pkg/alb/ingress/alb_spec.go @@ -41,9 +41,9 @@ func (r *IngressClassReconciler) getAlbSpecForIngresses( certificates := albCertificates{} targetPools := albTargetPools{} - for _, ingress := range ingresses { + for i, ingress := range ingresses { var listenerMergeError, targetPoolMergeError []error - ingressListeners, ingressCertificates, ingressTargetPools, ingressErrorList := r.getALBResourcesForIngress(ctx, class, &ingress) + ingressListeners, ingressCertificates, ingressTargetPools, ingressErrorList := r.getALBResourcesForIngress(ctx, class, &ingress, i) errorList = append(errorList, ingressErrorList...) certificates = mergeCertificates(certificates, ingressCertificates) @@ -147,6 +147,33 @@ func (r *IngressClassReconciler) getAlbSpecForResources( albsdkHost.Rules = append(albsdkHost.Rules, albsdkRule) } + + /// menekse + + sort.SliceStable(albsdkHost.Rules, func(i, j int) bool { + pathI := "" + if albsdkHost.Rules[i].Path.Prefix != nil { + pathI = *albsdkHost.Rules[i].Path.Prefix + } else if albsdkHost.Rules[i].Path.ExactMatch != nil { + pathI = *albsdkHost.Rules[i].Path.ExactMatch + } + + pathJ := "" + if albsdkHost.Rules[j].Path.Prefix != nil { + pathJ = *albsdkHost.Rules[j].Path.Prefix + } else if albsdkHost.Rules[j].Path.ExactMatch != nil { + pathJ = *albsdkHost.Rules[j].Path.ExactMatch + } + + ruleI := hostPaths.path[pathI] + ruleJ := hostPaths.path[pathJ] + + // Smaller sequence index means it was processed earlier (higher priority) + return ruleI.sequenceIndex < ruleJ.sequenceIndex + }) + + //// + albsdkHosts = append(albsdkHosts, albsdkHost) albsdkListener.Http = new(albsdk.ProtocolOptionsHTTP{ @@ -218,7 +245,7 @@ func (r *IngressClassReconciler) getAlbSpecForResources( return alb, errorList, nil } -func (r *IngressClassReconciler) getALBResourcesForIngress(ctx context.Context, class *networkingv1.IngressClass, ingress *networkingv1.Ingress) (albListeners, albCertificates, albTargetPools, []error) { +func (r *IngressClassReconciler) getALBResourcesForIngress(ctx context.Context, class *networkingv1.IngressClass, ingress *networkingv1.Ingress, sequenceIndex int) (albListeners, albCertificates, albTargetPools, []error) { ref := getIngressRefForIngress(r.Scheme, ingress) certificates := albCertificates{} @@ -283,6 +310,7 @@ func (r *IngressClassReconciler) getALBResourcesForIngress(ctx context.Context, cookiePersistenceTtlSeconds: getAnnotation(AnnotationCookiePersistenceTTLSeconds, 0, class, ingress), targetPoolName: poolName, websocket: getAnnotation(AnnotationWebSocket, false, class, ingress), + sequenceIndex: sequenceIndex, } } } diff --git a/pkg/alb/ingress/alb_spec_test.go b/pkg/alb/ingress/alb_spec_test.go index f9b584dd..31d48351 100644 --- a/pkg/alb/ingress/alb_spec_test.go +++ b/pkg/alb/ingress/alb_spec_test.go @@ -280,15 +280,16 @@ var _ = Describe("Node Controller", func() { // manipulate the base ingress' (created in the top-level BeforeEach) creation timestamp to be the oldest err := k8sClient.Get(context.Background(), client.ObjectKey{Namespace: ingress.Namespace, Name: ingress.Name}, &ingress) Expect(err).NotTo(HaveOccurred()) + ingress.Annotations = nil // ensuring no priority annotations are lingering Expect(k8sClient.Update(context.Background(), &ingress)).To(Succeed()) - - time.Sleep(1 * time.Second) + Expect(k8sClient.Get(context.Background(), client.ObjectKey{Namespace: ingress.Namespace, Name: ingress.Name}, &ingress)).To(Succeed()) ingress2 = testIngress(&ingressClass, &service) ingress2.Name = "ingress2" - ingress2.Spec.Rules[0].HTTP.Paths[0].Path = "/foobar" + ingress2.Spec.Rules[0].HTTP.Paths[0].Path = "/menekse" ingress2.Annotations = nil // ensuring no priority annotations + ingress2.ObjectMeta.CreationTimestamp = metav1.NewTime(ingress.CreationTimestamp.Add(1 * time.Hour)) Expect(k8sClient.Create(context.Background(), &ingress2)).To(Succeed()) @@ -300,7 +301,7 @@ var _ = Describe("Node Controller", func() { WebSocket: new(false), }, { - Path: &albsdk.Path{Prefix: new("/foobar")}, + Path: &albsdk.Path{Prefix: new("/menekse")}, TargetPool: new(secTargetPool), WebSocket: new(false), }, diff --git a/pkg/alb/ingress/resources.go b/pkg/alb/ingress/resources.go index 16b37ccb..b219e2d3 100644 --- a/pkg/alb/ingress/resources.go +++ b/pkg/alb/ingress/resources.go @@ -30,6 +30,7 @@ type albListenerRule struct { pathTyp networkingv1.PathType targetPoolName string websocket bool + sequenceIndex int } type albTargetPools map[string]albTargetPool From 658962c279b619e2a74b6003e62322ec70a1d279 Mon Sep 17 00:00:00 2001 From: Menekse Ceylan Date: Wed, 3 Jun 2026 21:35:07 +0200 Subject: [PATCH 5/6] deleted unused comment fixed unit tests and added more for ingress controller --- .../ingressclass_controller_unit_test.go | 384 ++++++++++++++---- 1 file changed, 308 insertions(+), 76 deletions(-) diff --git a/pkg/alb/ingress/ingressclass_controller_unit_test.go b/pkg/alb/ingress/ingressclass_controller_unit_test.go index 5556f032..18d496ba 100644 --- a/pkg/alb/ingress/ingressclass_controller_unit_test.go +++ b/pkg/alb/ingress/ingressclass_controller_unit_test.go @@ -1,8 +1,5 @@ package ingress -/* -TODO - import ( "context" "testing" @@ -11,6 +8,7 @@ import ( "github.com/google/go-cmp/cmp" stackitconfig "github.com/stackitcloud/cloud-provider-stackit/pkg/stackit/config" gomock "go.uber.org/mock/gomock" + corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" @@ -23,12 +21,15 @@ import ( ) const ( - testProjectID = "test-project" - testRegion = "test-region" - testALBName = "k8s-ingress-test-ingressclass" - testNamespace = "test-namespace" - testPublicIP = "1.2.3.4" - testPrivateIP = "10.0.0.1" + testProjectID = "test-project" + testRegion = "test-region" + testNamespace = "test-namespace" + testPublicIP = "1.2.3.4" + testPrivateIP = "10.0.0.1" + testIngressName = "test-ingress" + testIngressClassName = "k8s-ingress-test-ingress-class" + testIngressClassUID = "11111111-2222-3333-4444-555555555555" + testALBName = testIngressClassUID ) //nolint:funlen // Just many test cases. @@ -36,6 +37,19 @@ func TestIngressClassReconciler_updateStatus(t *testing.T) { testIngressClass := &networkingv1.IngressClass{ ObjectMeta: metav1.ObjectMeta{ Name: testIngressClassName, + UID: testIngressClassUID, + }, + } + + testService := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-service", + Namespace: testNamespace, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + {Port: 8080}, + }, }, } @@ -50,9 +64,8 @@ func TestIngressClassReconciler_updateStatus(t *testing.T) { { name: "ALB not ready (Terminating), should requeue", mockK8sClient: func(c client.Client) error { - return c.Create(context.Background(), &networkingv1.Ingress{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testIngressName}, - }) + return c.Create(context.Background(), new(testIngress(testIngressClass, testService))) + }, mockALBClient: func(m *stackit.MockApplicationLoadBalancerClient) { m.EXPECT(). @@ -64,17 +77,13 @@ func TestIngressClassReconciler_updateStatus(t *testing.T) { wantResult: reconcile.Result{RequeueAfter: 10 * time.Second}, wantErr: false, }, - // This case only checks the reconcile result, not whether the ingress status was actually updated. - // The actual update logic will be verified in integration tests. { name: "ALB ready, public IP available, ingress status needs update", ingresses: []*networkingv1.Ingress{ - {ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testIngressName}}, + new(testIngress(testIngressClass, testService)), }, mockK8sClient: func(c client.Client) error { - return c.Create(context.Background(), &networkingv1.Ingress{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testIngressName}, - }) + return c.Create(context.Background(), new(testIngress(testIngressClass, testService))) }, mockALBClient: func(m *stackit.MockApplicationLoadBalancerClient) { m.EXPECT(). @@ -87,17 +96,13 @@ func TestIngressClassReconciler_updateStatus(t *testing.T) { wantResult: reconcile.Result{}, wantErr: false, }, - // This case only checks the reconcile result, not whether the ingress status was actually updated. - // The actual update logic will be verified in integration tests. { name: "ALB ready, private IP available, ingress status needs update", ingresses: []*networkingv1.Ingress{ - {ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testIngressName}}, + new(testIngress(testIngressClass, testService)), }, mockK8sClient: func(c client.Client) error { - return c.Create(context.Background(), &networkingv1.Ingress{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testIngressName}, - }) + return c.Create(context.Background(), new(testIngress(testIngressClass, testService))) }, mockALBClient: func(m *stackit.MockApplicationLoadBalancerClient) { m.EXPECT(). @@ -110,36 +115,40 @@ func TestIngressClassReconciler_updateStatus(t *testing.T) { wantResult: reconcile.Result{}, wantErr: false, }, - // This case only checks the reconcile result, not whether the ingress status was actually updated. - // The actual update logic will be verified in integration tests. { - name: "ALB ready, IP already correct, no update", + name: "ALB ready, IP already correct, no status update", ingresses: []*networkingv1.Ingress{ - { - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testIngressName}, - Status: networkingv1.IngressStatus{ + new(func() networkingv1.Ingress { + ing := testIngress(testIngressClass, testService) + ing.Spec.IngressClassName = new(testIngressClassName) + ing.Status = networkingv1.IngressStatus{ LoadBalancer: networkingv1.IngressLoadBalancerStatus{ Ingress: []networkingv1.IngressLoadBalancerIngress{{IP: testPublicIP}}, }, - }, - }, + } + return ing + }()), }, mockK8sClient: func(c client.Client) error { - return c.Create(context.Background(), &networkingv1.Ingress{ - ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testIngressName}, - Status: networkingv1.IngressStatus{ - LoadBalancer: networkingv1.IngressLoadBalancerStatus{ - Ingress: []networkingv1.IngressLoadBalancerIngress{{IP: testPublicIP}}, - }, + ing := testIngress(testIngressClass, testService) + ing.Spec.IngressClassName = new(testIngressClassName) + if err := c.Create(context.Background(), &ing); err != nil { + return err + } + + ing.Status = networkingv1.IngressStatus{ + LoadBalancer: networkingv1.IngressLoadBalancerStatus{ + Ingress: []networkingv1.IngressLoadBalancerIngress{{IP: testPublicIP}}, }, - }) + } + return c.Status().Update(context.Background(), &ing) }, mockALBClient: func(m *stackit.MockApplicationLoadBalancerClient) { m.EXPECT(). GetLoadBalancer(gomock.Any(), testProjectID, testRegion, testALBName). Return(&albsdk.LoadBalancer{ - Status: new("STATUS_READY"), - PrivateAddress: new(testPublicIP), + Status: new("STATUS_READY"), + ExternalAddress: new(testPublicIP), }, nil) }, wantResult: reconcile.Result{}, @@ -148,7 +157,7 @@ func TestIngressClassReconciler_updateStatus(t *testing.T) { { name: "failed to get load balancer", ingresses: []*networkingv1.Ingress{ - {ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testIngressName}}, + new(testIngress(testIngressClass, testService)), }, mockALBClient: func(m *stackit.MockApplicationLoadBalancerClient) { m.EXPECT().GetLoadBalancer(gomock.Any(), testProjectID, testRegion, testALBName).Return(nil, stackit.ErrorNotFound) @@ -157,65 +166,269 @@ func TestIngressClassReconciler_updateStatus(t *testing.T) { wantErr: true, }, { - name: "failed to get latest ingress", + name: "ALB ready, no public or private IP, return error", ingresses: []*networkingv1.Ingress{ - {ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testIngressName}}, + new(testIngress(testIngressClass, testService)), }, mockALBClient: func(m *stackit.MockApplicationLoadBalancerClient) { m.EXPECT(). GetLoadBalancer(gomock.Any(), testProjectID, testRegion, testALBName). Return(&albsdk.LoadBalancer{ - Status: new("STATUS_READY"), - PrivateAddress: new(testPublicIP), + Status: new("STATUS_READY"), }, nil) }, wantResult: reconcile.Result{}, wantErr: true, }, - // This case only checks the reconcile result, not whether the ingress status was actually updated. - // The actual update logic will be verified in integration tests. + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + + mockAlbClient := stackit.NewMockApplicationLoadBalancerClient(ctrl) + fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() + + r := &IngressClassReconciler{ + Client: fakeClient, + ALBClient: mockAlbClient, + ALBConfig: stackitconfig.ALBConfig{ + Global: stackitconfig.GlobalOpts{ + ProjectID: testProjectID, + Region: testRegion, + }, + }, + } + + if tt.mockK8sClient != nil { + if err := tt.mockK8sClient(fakeClient); err != nil { + t.Fatalf("mockK8sClient failed: %v", err) + } + } + + if tt.mockALBClient != nil { + tt.mockALBClient(mockAlbClient) + } + + expectedIngress := testIngress(testIngressClass, testService) + + got, err := r.updateStatus(context.Background(), testIngressClass) + if (err != nil) != tt.wantErr { + t.Fatalf("expected error %v, got %v", tt.wantErr, err) + } + if diff := cmp.Diff(tt.wantResult, got); diff != "" { + t.Fatalf("unexpected result (-want +got):\n%s", diff) + } + + if tt.name == "ALB ready, public IP available, ingress status needs update" { + latestIngress := &networkingv1.Ingress{} + if err := fakeClient.Get(context.Background(), client.ObjectKey{Namespace: expectedIngress.Namespace, Name: expectedIngress.Name}, latestIngress); err != nil { + t.Fatalf("failed to fetch ingress: %v", err) + } + if len(latestIngress.Status.LoadBalancer.Ingress) == 0 || latestIngress.Status.LoadBalancer.Ingress[0].IP != testPublicIP { + t.Errorf("Ingress status was not updated with the expected IP!") + } + } + + }) + } +} + +func TestIngressClassReconciler_Reconcile(t *testing.T) { + + testIngressClass := &networkingv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: testIngressClassName, + UID: testIngressClassUID, + Finalizers: []string{finalizerName}, + }, + Spec: networkingv1.IngressClassSpec{ + Controller: controllerName, + }, + } + + tests := []struct { + name string + ingressClass *networkingv1.IngressClass + mockALB func(*stackit.MockApplicationLoadBalancerClient) + mockCerts func(*stackit.MockCertificatesClient) + wantResult reconcile.Result + wantErr bool + checkFinalizer bool + }{ { - name: "failed to update ingress status", - ingresses: []*networkingv1.Ingress{ - {ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testIngressName}}, + name: "existing ingress class, happy", + ingressClass: testIngressClass, + mockALB: func(m *stackit.MockApplicationLoadBalancerClient) { + m.EXPECT(). + GetLoadBalancer(gomock.Any(), testProjectID, testRegion, testALBName). + Return(&albsdk.LoadBalancer{ + Status: new("STATUS_READY"), + ExternalAddress: new(testPublicIP), + }, nil).Times(2) }, - mockALBClient: func(m *stackit.MockApplicationLoadBalancerClient) { + wantResult: reconcile.Result{}, + wantErr: false, + }, + { + name: "ingress class doesn't match the controller, should ignore and exit cleanly", + ingressClass: &networkingv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: testIngressClassName, + UID: testIngressClassUID, + Finalizers: []string{finalizerName}, + }, + Spec: networkingv1.IngressClassSpec{ + Controller: "unknown-controller", + }, + }, + mockALB: nil, + wantResult: reconcile.Result{}, + wantErr: false, + }, + { + name: "ingress class has emtpy/mismatched controller specs, should ignore and exit cleanly", + ingressClass: &networkingv1.IngressClass{}, + mockALB: nil, + wantResult: reconcile.Result{}, + wantErr: false, + }, + { + name: "ingress class not found, should ignore and exit cleanly", + ingressClass: nil, + mockALB: nil, + wantResult: reconcile.Result{}, + wantErr: false, + }, + { + name: "missing finalizer, should add finalizer", + ingressClass: &networkingv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: testIngressClassName, + UID: testIngressClassUID, + Finalizers: []string{}, + }, + Spec: networkingv1.IngressClassSpec{ + Controller: controllerName, + }, + }, + mockALB: func(m *stackit.MockApplicationLoadBalancerClient) {}, + wantResult: reconcile.Result{}, + wantErr: false, + checkFinalizer: true, + }, + { + name: "ALB status not ready, should requeue", + ingressClass: &networkingv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: testIngressClassName, + UID: testIngressClassUID, + Finalizers: []string{finalizerName}, + }, + Spec: networkingv1.IngressClassSpec{ + Controller: controllerName, + }, + }, + + mockALB: func(m *stackit.MockApplicationLoadBalancerClient) { m.EXPECT(). GetLoadBalancer(gomock.Any(), testProjectID, testRegion, testALBName). Return(&albsdk.LoadBalancer{ - Status: new("STATUS_READY"), - PrivateAddress: new(testPublicIP), + Status: new("STATUS_NOT_READY"), + ExternalAddress: new(testPublicIP), + }, nil).Times(2) + }, + wantResult: reconcile.Result{RequeueAfter: 10 * time.Second}, + wantErr: false, + checkFinalizer: true, + }, + { + name: "ALB does not exist yet, should create a new one successfully", + ingressClass: testIngressClass, + mockALB: func(m *stackit.MockApplicationLoadBalancerClient) { + m.EXPECT(). + GetLoadBalancer(gomock.Any(), testProjectID, testRegion, testALBName). + Return(nil, stackit.ErrorNotFound) + m.EXPECT(). + CreateLoadBalancer(gomock.Any(), testProjectID, testRegion, gomock.Any()). + Return(&albsdk.LoadBalancer{}, nil) + m.EXPECT(). + GetLoadBalancer(gomock.Any(), testProjectID, testRegion, testALBName). + Return(&albsdk.LoadBalancer{ + Status: new("STATUS_READY"), + ExternalAddress: new(testPublicIP), }, nil) }, wantResult: reconcile.Result{}, - wantErr: true, + wantErr: false, }, { - name: "ALB ready, no public or private IP, should requeue", - ingresses: []*networkingv1.Ingress{ - {ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testIngressName}}, + name: "ingress class has deletion timestamp, should run handleIngressClassDeletion", + ingressClass: &networkingv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: testIngressClassName, + UID: testIngressClassUID, + DeletionTimestamp: &metav1.Time{Time: time.Now()}, + Finalizers: []string{finalizerName}, + }, + Spec: networkingv1.IngressClassSpec{Controller: controllerName}, }, - mockALBClient: func(m *stackit.MockApplicationLoadBalancerClient) { + mockALB: func(m *stackit.MockApplicationLoadBalancerClient) { + m.EXPECT(). + DeleteLoadBalancer(gomock.Any(), testProjectID, testRegion, testALBName). + Return(nil) + }, + mockCerts: func(m *stackit.MockCertificatesClient) { + m.EXPECT(). + ListCertificate(gomock.Any(), testProjectID, testRegion). + Return(nil, nil) + }, + wantResult: reconcile.Result{}, + wantErr: false, + }, + { + name: "ALB configuration has changed, should issue an update call", + ingressClass: testIngressClass, + mockALB: func(m *stackit.MockApplicationLoadBalancerClient) { m.EXPECT(). GetLoadBalancer(gomock.Any(), testProjectID, testRegion, testALBName). Return(&albsdk.LoadBalancer{ - Status: new("STATUS_READY"), + Status: new("STATUS_READY"), + Version: new("lb-v1"), + Listeners: []albsdk.Listener{{Port: new(int32(80))}}, // add a listener to empty config + }, nil) + + m.EXPECT(). + UpdateLoadBalancer(gomock.Any(), testProjectID, testRegion, testALBName, gomock.Any()). + Return(&albsdk.LoadBalancer{}, nil) + + m.EXPECT(). + GetLoadBalancer(gomock.Any(), testProjectID, testRegion, testALBName). + Return(&albsdk.LoadBalancer{ + Status: new("STATUS_READY"), + ExternalAddress: new(testPublicIP), }, nil) }, - wantResult: reconcile.Result{RequeueAfter: 10 * time.Second}, + wantResult: reconcile.Result{}, wantErr: false, }, } - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctrl := gomock.NewController(t) - mockAlbClient := stackit.NewMockApplicationLoadBalancerClient(ctrl) - fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() + mockCertsClient := stackit.NewMockCertificatesClient(ctrl) + + clientBuilder := fake.NewClientBuilder().WithScheme(scheme.Scheme) + if tt.ingressClass != nil { + clientBuilder.WithRuntimeObjects(tt.ingressClass) + } + fakeClient := clientBuilder.Build() + r := &IngressClassReconciler{ - Client: fakeClient, - ALBClient: mockAlbClient, + Client: fakeClient, + ALBClient: mockAlbClient, + CertificateClient: mockCertsClient, ALBConfig: stackitconfig.ALBConfig{ Global: stackitconfig.GlobalOpts{ ProjectID: testProjectID, @@ -224,24 +437,43 @@ func TestIngressClassReconciler_updateStatus(t *testing.T) { }, } - if tt.mockK8sClient != nil { - if err := tt.mockK8sClient(fakeClient); err != nil { - t.Fatalf("mockK8sClient failed: %v", err) - } + if tt.mockALB != nil { + tt.mockALB(mockAlbClient) + } + if tt.mockCerts != nil { + tt.mockCerts(mockCertsClient) } - if tt.mockALBClient != nil { - tt.mockALBClient(mockAlbClient) + req := reconcile.Request{ + NamespacedName: client.ObjectKey{ + Name: testIngressClassName, + }, } - got, err := r.updateStatus(context.Background(), testIngressClass) + got, err := r.Reconcile(context.Background(), req) if (err != nil) != tt.wantErr { - t.Fatalf("expected error %v, got %v", tt.wantErr, err) + t.Fatalf("Reconcile() - expected error %v, got %v", tt.wantErr, err) } if diff := cmp.Diff(tt.wantResult, got); diff != "" { - t.Fatalf("unexpected result (-want +got):\n%s", diff) + t.Fatalf("Reconcile() - unexpected result (-want +got):\n%s", diff) + } + + if tt.checkFinalizer { + // fetching the absolute latest state of the object directly from the fake K8s API server + latestClass := &networkingv1.IngressClass{} + key := client.ObjectKey{Name: testIngressClassName} + + if fetchErr := fakeClient.Get(context.Background(), key, latestClass); fetchErr != nil { + t.Fatalf("Failed to fetch latest IngressClass state from fake client: %v", fetchErr) + } + + // assertion: the finalizer string list is no longer empty and contains finalizerName + if len(latestClass.Finalizers) == 0 { + t.Errorf("Verification failed: expected IngressClass to have finalizers, but the list is empty") + } else if latestClass.Finalizers[0] != finalizerName { + t.Errorf("Verification failed: expected finalizer %q, but found %q", finalizerName, latestClass.Finalizers[0]) + } } }) } } -*/ From 3237fe874d2bad7b62e5b7e9979142d3cfcdac9f Mon Sep 17 00:00:00 2001 From: Menekse Ceylan Date: Mon, 8 Jun 2026 16:06:41 +0200 Subject: [PATCH 6/6] internal alb annotation is fixed unit tests for alb spec are added ensured that ephemeral address option defaults to false when internal-alb annotation is defined. --- pkg/alb/ingress/alb_spec.go | 5 +- pkg/alb/ingress/alb_spec_old.test.go | 447 -------------------------- pkg/alb/ingress/alb_spec_unit_test.go | 355 ++++++++++++++++++++ pkg/alb/ingress/annotations.go | 2 +- 4 files changed, 360 insertions(+), 449 deletions(-) delete mode 100644 pkg/alb/ingress/alb_spec_old.test.go create mode 100644 pkg/alb/ingress/alb_spec_unit_test.go diff --git a/pkg/alb/ingress/alb_spec.go b/pkg/alb/ingress/alb_spec.go index 7b982cef..93a5a418 100644 --- a/pkg/alb/ingress/alb_spec.go +++ b/pkg/alb/ingress/alb_spec.go @@ -41,7 +41,7 @@ func (r *IngressClassReconciler) getAlbSpecForIngresses( certificates := albCertificates{} targetPools := albTargetPools{} - for i, ingress := range ingresses { + for i, ingress := range ingresses { var listenerMergeError, targetPoolMergeError []error ingressListeners, ingressCertificates, ingressTargetPools, ingressErrorList := r.getALBResourcesForIngress(ctx, class, &ingress, i) errorList = append(errorList, ingressErrorList...) @@ -91,6 +91,7 @@ func (r *IngressClassReconciler) getAlbSpecForResources( if getAnnotation(AnnotationInternal, false, class) { alb.Options.PrivateNetworkOnly = new(true) + alb.Options.EphemeralAddress = new(false) } if plan := getAnnotation(AnnotationPlanID, "", class); plan != "" { @@ -280,6 +281,7 @@ func (r *IngressClassReconciler) getALBResourcesForIngress(ctx context.Context, path: map[string]albListenerRule{}, } } + for _, path := range rule.HTTP.Paths { if _, ok := hosts[rule.Host].path[path.Path]; ok { errorList = append(errorList, &errorEvent{ @@ -313,6 +315,7 @@ func (r *IngressClassReconciler) getALBResourcesForIngress(ctx context.Context, sequenceIndex: sequenceIndex, } } + } httpPort := getAnnotation(AnnotationHTTPPort, 80, ingress, class) diff --git a/pkg/alb/ingress/alb_spec_old.test.go b/pkg/alb/ingress/alb_spec_old.test.go deleted file mode 100644 index c89eb19f..00000000 --- a/pkg/alb/ingress/alb_spec_old.test.go +++ /dev/null @@ -1,447 +0,0 @@ -package ingress - -/* -TODO - -import ( - "context" - "testing" - - "github.com/google/go-cmp/cmp" - "github.com/stackitcloud/cloud-provider-stackit/pkg/stackit/config" - "google.golang.org/protobuf/testing/protocmp" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - - corev1 "k8s.io/api/core/v1" - networkingv1 "k8s.io/api/networking/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - albsdk "github.com/stackitcloud/stackit-sdk-go/services/alb/v2api" -) - -const ( - testController = "test-controller" - testIngressClassName = "test-ingressclass" - testIngressName = "test-ingress" - testNetworkID = "test-network" - testHost = "example.com" - testPath = "/" - testNodeName = "node-0" - testNodeIP = "1.1.1.1" - testServiceName = "test-service" - testServicePort = 80 - testNodePort = 30080 - testTLSName = "test-tls-secret" -) - -func ingressPrefixPath(path, serviceName string) networkingv1.HTTPIngressPath { - return networkingv1.HTTPIngressPath{ - Path: path, - PathType: new(networkingv1.PathTypePrefix), - Backend: networkingv1.IngressBackend{ - Service: &networkingv1.IngressServiceBackend{ - Name: serviceName, - Port: networkingv1.ServiceBackendPort{Number: testServicePort}, - }, - }, - } -} - -func ingressExactPath(path, serviceName string) networkingv1.HTTPIngressPath { - return networkingv1.HTTPIngressPath{ - Path: path, - PathType: new(networkingv1.PathTypeExact), - Backend: networkingv1.IngressBackend{ - Service: &networkingv1.IngressServiceBackend{ - Name: serviceName, - Port: networkingv1.ServiceBackendPort{Number: testServicePort}, - }, - }, - } -} - -func ingressRule(host string, paths ...networkingv1.HTTPIngressPath) networkingv1.IngressRule { - return networkingv1.IngressRule{ - Host: host, - IngressRuleValue: networkingv1.IngressRuleValue{ - HTTP: &networkingv1.HTTPIngressRuleValue{Paths: paths}, - }, - } -} - -func fixtureIngressWithParams(name, namespace string, annotations map[string]string, rules ...networkingv1.IngressRule) networkingv1.Ingress { - return networkingv1.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - Annotations: annotations, - }, - Spec: networkingv1.IngressSpec{ - IngressClassName: new(testIngressClassName), - Rules: rules, - }, - } -} - -func fixtureServiceWithParams(name string, port, nodePort int32) *corev1.Service { //nolint:unparam // We might need it later. - return &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{Name: name}, - Spec: corev1.ServiceSpec{ - Type: corev1.ServiceTypeNodePort, - Ports: []corev1.ServicePort{ - { - Port: port, - NodePort: nodePort, - }, - }, - }, - } -} - -func fixtureNode(mods ...func(*corev1.Node)) *corev1.Node { - node := &corev1.Node{ - ObjectMeta: metav1.ObjectMeta{Name: testNodeName}, - Status: corev1.NodeStatus{ - Addresses: []corev1.NodeAddress{{Type: corev1.NodeInternalIP, Address: testNodeIP}}, - }, - } - for _, mod := range mods { - mod(node) - } - return node -} - -func fixtureIngress(mods ...func(networkingv1.Ingress)) networkingv1.Ingress { - ingress := networkingv1.Ingress{ - ObjectMeta: metav1.ObjectMeta{Name: testIngressName}, - Spec: networkingv1.IngressSpec{ - IngressClassName: new(testIngressClassName), - Rules: []networkingv1.IngressRule{ - { - Host: testHost, - IngressRuleValue: networkingv1.IngressRuleValue{ - HTTP: &networkingv1.HTTPIngressRuleValue{ - Paths: []networkingv1.HTTPIngressPath{ - { - Path: testPath, - PathType: new(networkingv1.PathTypePrefix), - Backend: networkingv1.IngressBackend{ - Service: &networkingv1.IngressServiceBackend{ - Name: testServiceName, - Port: networkingv1.ServiceBackendPort{Number: testServicePort}, - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - for _, mod := range mods { - mod(ingress) - } - return ingress -} - -func fixtureIngressClass(mods ...func(networkingv1.IngressClass)) networkingv1.IngressClass { - ingressClass := networkingv1.IngressClass{ - ObjectMeta: metav1.ObjectMeta{Name: testIngressClassName}, - Spec: networkingv1.IngressClassSpec{Controller: testController}, - } - for _, mod := range mods { - mod(ingressClass) - } - return ingressClass -} - -func fixtureAlbPayload(mods ...func(*albsdk.CreateLoadBalancerPayload)) *albsdk.CreateLoadBalancerPayload { - payload := &albsdk.CreateLoadBalancerPayload{ - Name: new("k8s-ingress-" + testIngressClassName), - Listeners: []albsdk.Listener{ - { - Name: new("http"), - Port: new(int32(80)), - Protocol: new("PROTOCOL_HTTP"), - Http: &albsdk.ProtocolOptionsHTTP{ - Hosts: []albsdk.HostConfig{ - { - Host: new(testHost), - Rules: []albsdk.Rule{ - { - Path: &albsdk.Path{ - Prefix: new(testPath), - }, - TargetPool: new("pool-30080"), - }, - }, - }, - }, - }, - }, - }, - Networks: []albsdk.Network{{NetworkId: new(testNetworkID), Role: new("ROLE_LISTENERS_AND_TARGETS")}}, - Options: &albsdk.LoadBalancerOptions{EphemeralAddress: new(true)}, - TargetPools: []albsdk.TargetPool{ - {Name: new("pool-30080"), TargetPort: new(int32(30080)), Targets: []albsdk.Target{{DisplayName: new(testNodeName), Ip: new(testNodeIP)}}}, - }, - } - for _, mod := range mods { - mod(payload) - } - return payload -} - -//nolint:funlen // Just many test cases. -func Test_albSpecFromIngress(t *testing.T) { - r := &IngressClassReconciler{ - ALBConfig: config.ALBConfig{ - ApplicationLoadBalancer: config.ApplicationLoadBalancerOpts{ - NetworkID: testNetworkID, - }, - }, - } - nodes := []*corev1.Node{fixtureNode()} - - tests := []struct { - name string - ingresses []networkingv1.Ingress - ingressClass networkingv1.IngressClass - services []*corev1.Service - want *albsdk.CreateLoadBalancerPayload - wantErr bool - }{ - { - name: "valid ingress with HTTP listener", - ingresses: []networkingv1.Ingress{fixtureIngress()}, - ingressClass: fixtureIngressClass(), - services: []*corev1.Service{fixtureServiceWithParams(testServiceName, testServicePort, testNodePort)}, - want: fixtureAlbPayload(), - }, - { - name: "valid ingress with HTTP listener with external ip address", - ingresses: []networkingv1.Ingress{fixtureIngress()}, - ingressClass: fixtureIngressClass( - func(ing networkingv1.IngressClass) { - ing.Annotations = map[string]string{AnnotationExternalIP: "2.2.2.2"} - }, - ), - services: []*corev1.Service{fixtureServiceWithParams(testServiceName, testServicePort, testNodePort)}, - want: fixtureAlbPayload(func(payload *albsdk.CreateLoadBalancerPayload) { - payload.ExternalAddress = new("2.2.2.2") - payload.Options = &albsdk.LoadBalancerOptions{EphemeralAddress: nil} - }), - }, - { - name: "valid ingress with HTTP listener with internal ip address", - ingresses: []networkingv1.Ingress{fixtureIngress()}, - ingressClass: fixtureIngressClass( - func(ing networkingv1.IngressClass) { - ing.Annotations = map[string]string{AnnotationInternal: "true"} - }, - ), - services: []*corev1.Service{fixtureServiceWithParams(testServiceName, testServicePort, testNodePort)}, - want: fixtureAlbPayload(func(payload *albsdk.CreateLoadBalancerPayload) { - payload.Options = &albsdk.LoadBalancerOptions{PrivateNetworkOnly: new(true)} - }), - }, - { - name: "host ordering", - ingressClass: fixtureIngressClass(), - ingresses: []networkingv1.Ingress{ - fixtureIngressWithParams("ingress", "ns", nil, - ingressRule("z-host.com", ingressPrefixPath("/a", "svc1")), - ingressRule("a-host.com", ingressPrefixPath("/a", "svc2")), - ), - }, - services: []*corev1.Service{ - fixtureServiceWithParams("svc1", testServicePort, 30001), - fixtureServiceWithParams("svc2", testServicePort, 30002), - }, - want: fixtureAlbPayload(func(p *albsdk.CreateLoadBalancerPayload) { - p.Listeners[0].Http.Hosts = []albsdk.HostConfig{ - { - Host: new("a-host.com"), - Rules: []albsdk.Rule{ - {Path: &albsdk.Path{Prefix: new("/a")}, TargetPool: new("pool-30002")}, - }, - }, - { - Host: new("z-host.com"), - Rules: []albsdk.Rule{ - {Path: &albsdk.Path{Prefix: new("/a")}, TargetPool: new("pool-30001")}, - }, - }, - } - p.TargetPools = []albsdk.TargetPool{ - {Name: new("pool-30001"), TargetPort: new(int32(30001)), Targets: []albsdk.Target{{DisplayName: new(testNodeName), Ip: new(testNodeIP)}}}, - {Name: new("pool-30002"), TargetPort: new(int32(30002)), Targets: []albsdk.Target{{DisplayName: new(testNodeName), Ip: new(testNodeIP)}}}, - } - }), - }, - { - name: "priority annotation ordering", - ingressClass: fixtureIngressClass(), - ingresses: []networkingv1.Ingress{ - fixtureIngressWithParams("low", "ns", nil, - ingressRule("host.com", ingressPrefixPath("/x", "svc1")), - ), - fixtureIngressWithParams("high", "ns", map[string]string{AnnotationPriority: "5"}, - ingressRule("host.com", ingressPrefixPath("/x", "svc2")), - ), - }, - services: []*corev1.Service{ - fixtureServiceWithParams("svc1", testServicePort, 30003), - fixtureServiceWithParams("svc2", testServicePort, 30004), - }, - want: fixtureAlbPayload(func(p *albsdk.CreateLoadBalancerPayload) { - p.Listeners[0].Http.Hosts[0].Host = new("host.com") - p.Listeners[0].Http.Hosts[0].Rules = []albsdk.Rule{ - {Path: &albsdk.Path{Prefix: new("/x")}, TargetPool: new("pool-30004")}, - {Path: &albsdk.Path{Prefix: new("/x")}, TargetPool: new("pool-30003")}, - } - p.TargetPools = []albsdk.TargetPool{ - {Name: new("pool-30003"), TargetPort: new(int32(30003)), Targets: []albsdk.Target{{DisplayName: new(testNodeName), Ip: new(testNodeIP)}}}, - {Name: new("pool-30004"), TargetPort: new(int32(30004)), Targets: []albsdk.Target{{DisplayName: new(testNodeName), Ip: new(testNodeIP)}}}, - } - }), - }, - { - name: "path specificity ordering", - ingressClass: fixtureIngressClass(), - ingresses: []networkingv1.Ingress{ - fixtureIngressWithParams("ingress", "ns", nil, - ingressRule("host.com", - ingressPrefixPath("/short", "svc1"), - ingressPrefixPath("/very/very/long/specific", "svc2"), - ), - ), - }, - services: []*corev1.Service{ - fixtureServiceWithParams("svc1", testServicePort, 30005), - fixtureServiceWithParams("svc2", testServicePort, 30006), - }, - want: fixtureAlbPayload(func(p *albsdk.CreateLoadBalancerPayload) { - p.Listeners[0].Http.Hosts[0].Host = new("host.com") - p.Listeners[0].Http.Hosts[0].Rules = []albsdk.Rule{ - {Path: &albsdk.Path{Prefix: new("/very/very/long/specific")}, TargetPool: new("pool-30006")}, - {Path: &albsdk.Path{Prefix: new("/short")}, TargetPool: new("pool-30005")}, - } - p.TargetPools = []albsdk.TargetPool{ - {Name: new("pool-30005"), TargetPort: new(int32(30005)), Targets: []albsdk.Target{{DisplayName: new(testNodeName), Ip: new(testNodeIP)}}}, - {Name: new("pool-30006"), TargetPort: new(int32(30006)), Targets: []albsdk.Target{{DisplayName: new(testNodeName), Ip: new(testNodeIP)}}}, - } - }), - }, - { - name: "path type ordering (Exact before Prefix)", - ingressClass: fixtureIngressClass(), - ingresses: []networkingv1.Ingress{ - fixtureIngressWithParams("ingress", "ns", nil, - ingressRule("host.com", - ingressExactPath("/same", "svc-exact"), - ingressPrefixPath("/same", "svc-prefix"), - ), - ), - }, - services: []*corev1.Service{ - fixtureServiceWithParams("svc-exact", testServicePort, 30100), - fixtureServiceWithParams("svc-prefix", testServicePort, 30101), - }, - want: fixtureAlbPayload(func(p *albsdk.CreateLoadBalancerPayload) { - p.Listeners[0].Http.Hosts[0].Host = new("host.com") - p.Listeners[0].Http.Hosts[0].Rules = []albsdk.Rule{ - {Path: &albsdk.Path{ExactMatch: new("/same")}, TargetPool: new("pool-30100")}, - {Path: &albsdk.Path{Prefix: new("/same")}, TargetPool: new("pool-30101")}, - } - p.TargetPools = []albsdk.TargetPool{ - {Name: new("pool-30100"), TargetPort: new(int32(30100)), Targets: []albsdk.Target{{DisplayName: new(testNodeName), Ip: new(testNodeIP)}}}, - {Name: new("pool-30101"), TargetPort: new(int32(30101)), Targets: []albsdk.Target{{DisplayName: new(testNodeName), Ip: new(testNodeIP)}}}, - } - }), - }, - { - name: "ingress name ordering", - ingressClass: fixtureIngressClass(), - ingresses: []networkingv1.Ingress{ - fixtureIngressWithParams("b-ingress", "ns", nil, - ingressRule("host.com", ingressPrefixPath("/x", "svc1")), - ), - fixtureIngressWithParams("a-ingress", "ns", nil, - ingressRule("host.com", ingressPrefixPath("/x", "svc2")), - ), - }, - services: []*corev1.Service{ - fixtureServiceWithParams("svc1", testServicePort, 30007), - fixtureServiceWithParams("svc2", testServicePort, 30008), - }, - want: fixtureAlbPayload(func(p *albsdk.CreateLoadBalancerPayload) { - p.Listeners[0].Http.Hosts[0].Host = new("host.com") - p.Listeners[0].Http.Hosts[0].Rules = []albsdk.Rule{ - {Path: &albsdk.Path{Prefix: new("/x")}, TargetPool: new("pool-30008")}, - {Path: &albsdk.Path{Prefix: new("/x")}, TargetPool: new("pool-30007")}, - } - p.TargetPools = []albsdk.TargetPool{ - {Name: new("pool-30007"), TargetPort: new(int32(30007)), Targets: []albsdk.Target{{DisplayName: new(testNodeName), Ip: new(testNodeIP)}}}, - {Name: new("pool-30008"), TargetPort: new(int32(30008)), Targets: []albsdk.Target{{DisplayName: new(testNodeName), Ip: new(testNodeIP)}}}, - } - }), - }, - { - name: "namespace ordering", - ingressClass: fixtureIngressClass(), - ingresses: []networkingv1.Ingress{ - fixtureIngressWithParams("ingress", "ns-b", nil, - ingressRule("host.com", ingressPrefixPath("/x", "svc1")), - ), - fixtureIngressWithParams("ingress", "ns-a", nil, - ingressRule("host.com", ingressPrefixPath("/x", "svc2")), - ), - }, - services: []*corev1.Service{ - fixtureServiceWithParams("svc1", testServicePort, 30009), - fixtureServiceWithParams("svc2", testServicePort, 30010), - }, - want: fixtureAlbPayload(func(p *albsdk.CreateLoadBalancerPayload) { - p.Listeners[0].Http.Hosts[0].Host = new("host.com") - p.Listeners[0].Http.Hosts[0].Rules = []albsdk.Rule{ - {Path: &albsdk.Path{Prefix: new("/x")}, TargetPool: new("pool-30010")}, - {Path: &albsdk.Path{Prefix: new("/x")}, TargetPool: new("pool-30009")}, - } - p.TargetPools = []albsdk.TargetPool{ - {Name: new("pool-30009"), TargetPort: new(int32(30009)), Targets: []albsdk.Target{{DisplayName: new(testNodeName), Ip: new(testNodeIP)}}}, - {Name: new("pool-30010"), TargetPort: new(int32(30010)), Targets: []albsdk.Target{{DisplayName: new(testNodeName), Ip: new(testNodeIP)}}}, - } - }), - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - obj := []client.Object{} - for _, svc := range tt.services { - obj = append(obj, svc) - } - - for _, node := range nodes { - obj = append(obj, node) - } - - r.Client = fake.NewClientBuilder(). - WithObjects(obj...). - Build() - - _, got, err := r.getAlbSpecForIngresses(context.TODO(), &tt.ingressClass, tt.ingresses) - if (err != nil) != tt.wantErr { - t.Errorf("got error = %v, wantErr %v", err, tt.wantErr) - return - } - if diff := cmp.Diff(tt.want, got, protocmp.Transform()); diff != "" { - t.Errorf("got %v, want %v, diff=%s", got, tt.want, diff) - } - }) - } -} -*/ diff --git a/pkg/alb/ingress/alb_spec_unit_test.go b/pkg/alb/ingress/alb_spec_unit_test.go new file mode 100644 index 00000000..d1ad67a9 --- /dev/null +++ b/pkg/alb/ingress/alb_spec_unit_test.go @@ -0,0 +1,355 @@ +package ingress + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/stackitcloud/cloud-provider-stackit/pkg/stackit/config" + "google.golang.org/protobuf/testing/protocmp" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + albsdk "github.com/stackitcloud/stackit-sdk-go/services/alb/v2api" +) + +const ( + testNetworkID = "test-network" + testHost = "example.com" + testPath = "/" + testNodeName = "node-0" + testNodeIP = "1.1.1.1" + testServiceName = "test-service" + testServicePort = 80 + testNodePort = 30080 +) + +func ingressPrefixPath(path, serviceName string) networkingv1.HTTPIngressPath { + return networkingv1.HTTPIngressPath{ + Path: path, + PathType: new(networkingv1.PathTypePrefix), + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: serviceName, + Port: networkingv1.ServiceBackendPort{Number: testServicePort}, + }, + }, + } +} + +func ingressExactPath(path, serviceName string) networkingv1.HTTPIngressPath { + return networkingv1.HTTPIngressPath{ + Path: path, + PathType: new(networkingv1.PathTypeExact), + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: serviceName, + Port: networkingv1.ServiceBackendPort{Number: testServicePort}, + }, + }, + } +} + +func ingressRule(host string, paths ...networkingv1.HTTPIngressPath) networkingv1.IngressRule { + return networkingv1.IngressRule{ + Host: host, + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{Paths: paths}, + }, + } +} + +func fixtureIngressWithParams(name, namespace string, annotations map[string]string, rules ...networkingv1.IngressRule) networkingv1.Ingress { + return networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: annotations, + }, + Spec: networkingv1.IngressSpec{ + IngressClassName: new(testIngressClassName), + Rules: rules, + }, + } +} + +func fixtureServiceWithParams(name string, port, nodePort int32) *corev1.Service { //nolint:unparam // We might need it later. + return &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeNodePort, + Ports: []corev1.ServicePort{ + { + Port: port, + NodePort: nodePort, + }, + }, + }, + } +} + +func fixtureNode(mods ...func(*corev1.Node)) *corev1.Node { + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: testNodeName}, + Status: corev1.NodeStatus{ + Addresses: []corev1.NodeAddress{{Type: corev1.NodeInternalIP, Address: testNodeIP}}, + }, + } + for _, mod := range mods { + mod(node) + } + return node +} + +func fixtureIngress(mods ...func(networkingv1.Ingress)) networkingv1.Ingress { + ingress := networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{Name: testIngressName, Annotations: nil}, + Spec: networkingv1.IngressSpec{ + IngressClassName: new(testIngressClassName), + Rules: []networkingv1.IngressRule{ + { + Host: testHost, + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{ + { + Path: testPath, + PathType: new(networkingv1.PathTypePrefix), + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: testServiceName, + Port: networkingv1.ServiceBackendPort{Number: testServicePort}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + for _, mod := range mods { + mod(ingress) + } + return ingress +} + +func fixtureIngressClass(mods ...func(*networkingv1.IngressClass)) networkingv1.IngressClass { + ingressClass := networkingv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{Name: testIngressClassName, UID: testIngressClassUID, Finalizers: []string{finalizerName}}, + Spec: networkingv1.IngressClassSpec{Controller: controllerName}, + } + for _, mod := range mods { + mod(&ingressClass) + } + return ingressClass +} + +func fixtureAlbPayload(mods ...func(*albsdk.CreateLoadBalancerPayload)) *albsdk.CreateLoadBalancerPayload { + payload := &albsdk.CreateLoadBalancerPayload{ + Name: new(testIngressClassUID), + DisableTargetSecurityGroupAssignment: new(true), + Labels: new(map[string]string{ + "alb-ingress-controller-ingress-class-uid": "11111111-2222-3333-4444-555555555555", + }), + Listeners: []albsdk.Listener{ + { + Name: new("80-http"), + Port: new(int32(80)), + Protocol: new("PROTOCOL_HTTP"), + Http: &albsdk.ProtocolOptionsHTTP{ + Hosts: []albsdk.HostConfig{ + { + Host: new(testHost), + Rules: []albsdk.Rule{ + { + Path: &albsdk.Path{ + Prefix: new(testPath), + }, + TargetPool: new("port-30080"), + WebSocket: new(false), + }, + }, + }, + }, + }, + }, + }, + Networks: []albsdk.Network{{NetworkId: new(testNetworkID), Role: new("ROLE_LISTENERS_AND_TARGETS")}}, + Options: &albsdk.LoadBalancerOptions{EphemeralAddress: new(true)}, + TargetPools: []albsdk.TargetPool{ + {Name: new("port-30080"), TargetPort: new(int32(30080)), Targets: []albsdk.Target{{DisplayName: new(testNodeName), Ip: new(testNodeIP)}}}, + }, + } + for _, mod := range mods { + mod(payload) + } + return payload +} + +//nolint:funlen // Just many test cases. +func Test_albSpecFromIngress(t *testing.T) { + r := &IngressClassReconciler{ + ALBConfig: config.ALBConfig{ + ApplicationLoadBalancer: config.ApplicationLoadBalancerOpts{ + NetworkID: testNetworkID, + }, + }, + } + nodes := []*corev1.Node{fixtureNode()} + + tests := []struct { + name string + ingresses []networkingv1.Ingress + ingressClass networkingv1.IngressClass + services []*corev1.Service + want *albsdk.CreateLoadBalancerPayload + wantErr bool + wantErrorList []error + }{ + { + name: "valid ingress with HTTP listener", + ingresses: []networkingv1.Ingress{fixtureIngress()}, + ingressClass: fixtureIngressClass(), + services: []*corev1.Service{fixtureServiceWithParams(testServiceName, testServicePort, testNodePort)}, + want: fixtureAlbPayload(), + wantErr: false, + wantErrorList: []error{}, + }, + { + name: "valid ingress with HTTP listener with external ip address", + ingresses: []networkingv1.Ingress{fixtureIngress()}, + ingressClass: fixtureIngressClass( + func(ic *networkingv1.IngressClass) { + ic.Annotations = map[string]string{AnnotationExternalIP: "2.2.2.2"} + }, + ), + services: []*corev1.Service{fixtureServiceWithParams(testServiceName, testServicePort, testNodePort)}, + want: fixtureAlbPayload(func(payload *albsdk.CreateLoadBalancerPayload) { + payload.ExternalAddress = new("2.2.2.2") + payload.Options = &albsdk.LoadBalancerOptions{EphemeralAddress: nil} + }), + wantErrorList: []error{}, + }, + { + name: "valid ingress with HTTP listener with internal ip address", + ingresses: []networkingv1.Ingress{fixtureIngress()}, + ingressClass: fixtureIngressClass( + func(ic *networkingv1.IngressClass) { + ic.Annotations = map[string]string{AnnotationInternal: "true"} + }, + ), + services: []*corev1.Service{fixtureServiceWithParams(testServiceName, testServicePort, testNodePort)}, + want: fixtureAlbPayload(func(payload *albsdk.CreateLoadBalancerPayload) { + payload.Options = &albsdk.LoadBalancerOptions{PrivateNetworkOnly: new(true), EphemeralAddress: new(false)} + }), + wantErrorList: []error{}, + }, + { + name: "same path prefix in same ingress", + ingressClass: fixtureIngressClass(), + ingresses: []networkingv1.Ingress{fixtureIngress( + func(ing networkingv1.Ingress) { + ing.Spec.Rules[0].HTTP.Paths[0].Path = "/x" + + // second rule with the same path prefix as the first rule + ing.Spec.Rules[0].HTTP.Paths = append(ing.Spec.Rules[0].HTTP.Paths, networkingv1.HTTPIngressPath{ + Path: "/x", + PathType: new(networkingv1.PathTypePrefix), + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "svc1", + Port: networkingv1.ServiceBackendPort{Number: testServicePort}, + }, + }, + }) + }, + )}, + + services: []*corev1.Service{ + fixtureServiceWithParams(testServiceName, testServicePort, testNodePort), + fixtureServiceWithParams("svc1", testServicePort, 30003), + }, + want: fixtureAlbPayload(func(p *albsdk.CreateLoadBalancerPayload) { + p.Listeners[0].Http.Hosts[0].Rules = []albsdk.Rule{ + {Path: &albsdk.Path{Prefix: new("/x")}, TargetPool: new("port-30080"), WebSocket: new(false)}, + } + p.TargetPools = []albsdk.TargetPool{ + {Name: new("port-30080"), TargetPort: new(int32(30080)), Targets: []albsdk.Target{{DisplayName: new(testNodeName), Ip: new(testNodeIP)}}}, + } + }), + wantErrorList: []error{ + &errorEvent{description: `path "/x" already exists within same ingress`, typ: "error"}, + }, + }, + /* + // TODO: Fix this test case once ordering and sorting rules are adjusted. + { + name: "path type ordering (Exact before Prefix)", + ingressClass: fixtureIngressClass(), + ingresses: []networkingv1.Ingress{ + fixtureIngressWithParams("ingress", "ns", nil, + ingressRule("host.com", + ingressExactPath("/same", "svc-exact"), + ingressPrefixPath("/same", "svc-prefix"), + ), + ), + }, + services: []*corev1.Service{ + fixtureServiceWithParams("svc-exact", testServicePort, 30100), + fixtureServiceWithParams("svc-prefix", testServicePort, 30101), + }, + want: fixtureAlbPayload(func(p *albsdk.CreateLoadBalancerPayload) { + p.Listeners[0].Http.Hosts[0].Host = new("host.com") + p.Listeners[0].Http.Hosts[0].Rules = []albsdk.Rule{ + {Path: &albsdk.Path{ExactMatch: new("/same")}, TargetPool: new("pool-30100")}, + {Path: &albsdk.Path{Prefix: new("/same")}, TargetPool: new("pool-30101")}, + } + p.TargetPools = []albsdk.TargetPool{ + {Name: new("pool-30100"), TargetPort: new(int32(30100)), Targets: []albsdk.Target{{DisplayName: new(testNodeName), Ip: new(testNodeIP)}}}, + {Name: new("pool-30101"), TargetPort: new(int32(30101)), Targets: []albsdk.Target{{DisplayName: new(testNodeName), Ip: new(testNodeIP)}}}, + } + }), + }, + + */ + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + obj := []client.Object{} + for _, svc := range tt.services { + obj = append(obj, svc) + } + + for _, node := range nodes { + obj = append(obj, node) + } + + r.Client = fake.NewClientBuilder(). + WithObjects(obj...). + Build() + + got, errList, err := r.getAlbSpecForIngresses(context.TODO(), &tt.ingressClass, tt.ingresses) + if (err != nil) != tt.wantErr { + t.Errorf("got error = %v, wantErr %v", err, tt.wantErr) + return + } + if diff := cmp.Diff(tt.want, got, protocmp.Transform()); diff != "" { + t.Errorf("got %v, want %v, diff=%s", got, tt.want, diff) + } + + if len(errList) > 0 { + if diff := cmp.Diff(tt.wantErrorList, errList, cmp.AllowUnexported(errorEvent{})); diff != "" { + t.Errorf("errList mismatch (-want +got):\n%s", diff) + } + } + + }) + } +} diff --git a/pkg/alb/ingress/annotations.go b/pkg/alb/ingress/annotations.go index a3335fec..7134b27b 100644 --- a/pkg/alb/ingress/annotations.go +++ b/pkg/alb/ingress/annotations.go @@ -16,7 +16,7 @@ const ( AnnotationExternalIP = "alb.stackit.cloud/external-address" // AnnotationInternal If true, the application load balancer is not exposed via a floating IP. // Can be set on IngressClass. - AnnotationInternal = "alb.stackit.cloud/internal" + AnnotationInternal = "alb.stackit.cloud/internal-alb" // AnnotationPlanID sets the plan for the ALB. // Can be set on IngressClass. AnnotationPlanID = "alb.stackit.cloud/plan-id"