ALB Ingress Controller - Refactor unit and gingko tests

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
 	}
@@ -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)
 	}
 
@@ -300,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 {

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),
+					},
+				},
+			},
+		},
+	}
+}

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.