From 1a15c2fc7a8d0daa1203245e28f9890cbcd45035 Mon Sep 17 00:00:00 2001 From: Menekse Ceylan Date: Wed, 27 May 2026 14:06:20 +0200 Subject: [PATCH 1/2] 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 128bfe71..8df9e4cd 100644 --- a/pkg/alb/ingress/alb_spec.go +++ b/pkg/alb/ingress/alb_spec.go @@ -179,6 +179,26 @@ func (r *IngressClassReconciler) getAlbSpecForResources(ctx context.Context, cla 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 d53e895d..a9ad6574 100644 --- a/pkg/alb/ingress/resources.go +++ b/pkg/alb/ingress/resources.go @@ -78,21 +78,21 @@ func mergeTargetPools(dst, src albTargetPools) (albTargetPools, []errorEvents) { if dstTargetPool.skipCertificateValidation != srcTargetPool.skipCertificateValidation { mergeErrors = append(mergeErrors, errorEvents{ 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, errorEvents{ 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, errorEvents{ 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 f28b4d8e56257015f32626c2911113419c652fe8 Mon Sep 17 00:00:00 2001 From: Menekse Ceylan Date: Thu, 28 May 2026 10:27:09 +0200 Subject: [PATCH 2/2] updated annotation lookup when sorting ingresses for ingressClass Utilized getSortedIngressesForIngressClass for getAlbSpecForIngressClass --- pkg/alb/ingress/alb_spec.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/alb/ingress/alb_spec.go b/pkg/alb/ingress/alb_spec.go index 8df9e4cd..df6865ff 100644 --- a/pkg/alb/ingress/alb_spec.go +++ b/pkg/alb/ingress/alb_spec.go @@ -19,7 +19,7 @@ import ( ) func (r *IngressClassReconciler) getAlbSpecForIngressClass(ctx context.Context, class *networkingv1.IngressClass) (*albsdk.CreateLoadBalancerPayload, []errorEvents, error) { - ingresses, err := r.getIngressesForIngressClass(ctx, class) + ingresses, err := r.getSortedIngressesForIngressClass(ctx, class) if err != nil { return nil, nil, err } @@ -320,8 +320,10 @@ func (r *IngressClassReconciler) getSortedIngressesForIngressClass(ctx context.C } sort.SliceStable(ingresses, func(i, j int) bool { - prioI := getAnnotation(AnnotationPriority, 0, class, &ingresses[i]) - prioJ := getAnnotation(AnnotationPriority, 0, class, &ingresses[j]) + //prioI := getAnnotation(AnnotationPriority, 0, class, &ingresses[i]) + //prioJ := getAnnotation(AnnotationPriority, 0, class, &ingresses[j]) + prioI := getAnnotation(AnnotationPriority, 0, &ingresses[i], class) + prioJ := getAnnotation(AnnotationPriority, 0, &ingresses[j], class) // Sort by Priority (Highest at the beginning) if prioI != prioJ {