WIP: Add Application Load Balancer Controller Manager by kamilprzybyl · Pull Request #879 · stackitcloud/cloud-provider-stackit

kamilprzybyl · 2026-03-17T09:27:48Z

How to categorize this PR?

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Breaking changes:

ske-prow · 2026-03-17T09:27:55Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign timebertt for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Adi146 · 2026-03-18T08:30:26Z

cmd/application-load-balancer-controller-manager/main.go

+	// generate self-signed certificates for the metrics server. While convenient for development and testing,
+	// this setup is not recommended for production.
+	//
+	// TODO(user): If you enable certManager, uncomment the following lines:


might be good to create separate examples for that

cmd/application-load-balancer-controller-manager/main.go

pkg/alb/ingress/alb_spec.go

Adi146 · 2026-03-18T08:48:39Z

pkg/alb/ingress/alb_spec.go

+	sort.SliceStable(ruleMetadataList, func(i, j int) bool {
+		a, b := ruleMetadataList[i], ruleMetadataList[j]
+		// 1. Host name (lexicographically)
+		if a.host != b.host {


does this work with * host matcher?

Adi146 · 2026-03-18T08:56:24Z

pkg/alb/ingress/alb_spec.go

+}
+
+// cleanupCerts deletes the certificates from the Certificates API that are no longer associated with any Ingress in the IngressClass
+func (r *IngressClassReconciler) cleanupCerts(ctx context.Context, ingressClass *networkingv1.IngressClass, ingresses []*networkingv1.Ingress) error {


do we really want to delete certificates? That could be very dangerous as users might just not associate the certificate anymore and want to keep it.

Each Ingress TLS secret reference is mapped to a unique ALB Certificate resource. Even if multiple Ingresses reference the same k8s Secret, they will each trigger the creation of a separate, independent certificate on the ALB. That means, deleting one Ingress (and its associated ALB Certificate) does not impact the certificates still being used by other Ingresses.

Adi146 · 2026-03-18T08:57:19Z

pkg/alb/ingress/alb_spec.go

+
+// isCertValid checks if the certificate chain is complete. It is used for checking if
+// the cert-manager's ACME challenge is completed, or if it's sill ongoing.
+func isCertValid(secret *corev1.Secret) (bool, error) {


isn't that already handled by the certificate api?

The Cert API only checks if the public and private keys are matching + if the certificate is a valid X.509 format but it does not reject incomplete chains.

Renamed the function from isCertValid to isCertReady and clarified the function in the comment above.

samples/ingress/issuer.yaml

fischerman · 2026-03-19T15:06:54Z

pkg/alb/ingress/ingressclass_controller.go

+	}
+
+	if len(albIngressList) < 1 {
+		err := r.handleIngressClassWithoutIngresses(ctx, albIngressList, ingressClass)


It doesn't make sense to pass albIngressList if it is always empty.

You're right, passing an empty list is useless. And with that, the controller does not clean up certificates properly as the list is always empty.

Removed the redundant ingresses parameter from handleIngressClassWithoutIngresses since it was always empty. Updated cleanupCerts to identify and delete certificates using the IngressClass UID prefix, ensuring orphaned certificates are properly removed even when no Ingress resources exist.

kamilprzybyl · 2026-03-22T10:13:48Z

We currently only delete certificates when the IngressClass is deleted or no Ingress references the IngressClass.
We need to add logic to clean up certificates that are no longer referenced by active Ingresses.

kamilprzybyl · 2026-03-25T11:53:05Z

We currently only delete certificates when the IngressClass is deleted or no Ingress references the IngressClass. We need to add logic to clean up certificates that are no longer referenced by active Ingresses.

Added ExactMatch -Path- to change detection

… references; adjusted requeuing logic for certificates

… this requires the makefile from kubebuilder

hown3d · 2026-03-27T12:30:16Z

pkg/alb/ingress/alb_spec.go

+	// It uses the trusted CAs from the operating system for validation.
+	tlsBridgingTrustedCaAnnotation = "alb.stackit.cloud/tls-bridging-trusted-ca"
+	// If set, the application load balancer enables TLS bridging with a custom CA provided as value.
+	tlsBridgingCustomCaAnnotation = "alb.stackit.cloud/tls-bridging-custom-ca"


This should probably use as value a reference to a TLS secret.

hown3d · 2026-03-27T12:31:00Z

pkg/alb/ingress/alb_spec.go

+// It merges and sorts all routing rules across the ingresses based on host, priority, path specificity, path type, and ingress origin.
+// The resulting ALB payload includes targets derived from cluster nodes, target pools per backend service, HTTP(S) listeners,
+// and optional TLS certificate bindings. This spec is later used to create or update the actual ALB instance.
+func (r *IngressClassReconciler) albSpecFromIngress( //nolint:funlen,gocyclo // We go through a lot of fields. Not much complexity.


this is indeed complex, should be split up into multiple functions.

hown3d · 2026-03-27T12:34:27Z

pkg/alb/ingress/alb_spec.go

+		return 0, fmt.Errorf("service not found: %s", path.Backend.Service.Name)
+	}
+
+	if path.Backend.Service.Port.Name != "" {


should be refactored to have less branching, 4 indents is too much imo.

hown3d · 2026-03-27T12:36:03Z

docs/albcm.md

+export KUBECONFIG=~/.kube/config
+```
+
+### Create your deployment and expose it via Ingress


We should link to the samples yamls instead of duplicating this into this doc.
Also a list of annotations should be added.

hown3d · 2026-03-27T12:37:52Z

docs/albcm.md

+
+> NOTE: The service has to be of type NodePort to enable access to the nodes from the outside of the cluster.
+
+3. Create an IngressClass that specifies the ALB Ingress controller:


I have a bit of information missing:
Is each ingress under the same ingress class being added to the same ALB? If so this should be made clear so that it's clear to end users.

hown3d · 2026-03-27T12:38:32Z

docs/deployment.md

 ## Overview

-The STACKIT Cloud Provider includes both the Cloud Controller Manager (CCM) for managing cloud resources and the CSI driver for persistent storage. This deployment provides a unified solution for cloud integration and storage provisioning.
+The STACKIT Cloud Provider includes the Cloud Controller Manager (CCM) for managing cloud resources, the CSI driver for persistent storage and the Application Load Balancer Controller Manager (ALBCM) for managing STACKIT Application Load Balancer (ALB) via Ingress Resources.


we should have different deployment docs for each binary. CCM and ALBCM should be different md files. It's easier to comprehend.

hown3d · 2026-03-27T12:40:03Z

cmd/application-load-balancer-controller-manager/main.go

+
+// nolint:funlen // TODO: Refactor into smaller functions.
+func main() {
+	var metricsAddr string


move into a options struct

ske-prow bot added do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 17, 2026

kamilprzybyl changed the title ~~Add Application Load Balancer Controller Manager~~ WIP: Add Application Load Balancer Controller Manager Mar 17, 2026

ske-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 17, 2026

kamilprzybyl force-pushed the feat/kp/add-alb-ingress-controller branch from 7ecc416 to 86bdf1d Compare March 17, 2026 09:37

Adi146 reviewed Mar 18, 2026

View reviewed changes

ske-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 19, 2026

fischerman reviewed Mar 19, 2026

View reviewed changes

kamilprzybyl force-pushed the feat/kp/add-alb-ingress-controller branch 2 times, most recently from d8de28a to f903b25 Compare March 20, 2026 19:05

kamilprzybyl closed this Mar 25, 2026

kamilprzybyl reopened this Mar 25, 2026

dergeberl mentioned this pull request Mar 25, 2026

Refactor config #915

Merged

Kamil Przybyl and others added 13 commits March 26, 2026 10:48

add application load balancer controller manager

64b5f64

chore: add alb ingress controller docs run-it-locally how-to

d623425

Fix errors in stackit package

cbf5622

chore: add new Makefile build for alb ingress controller manager

d7971df

Fix errors in ingress package (only non-test files)

05be587

Add mocks for ALB and certificates API

9440cac

Fix syntax errors in test in ingress package

cd270f1

wip: Add alb-controller-manager deploy files

cf3d7ac

Fix main.go

bb40d21

Add mock generation for ALB and certificates API

c18c393

Fix linter issues

6bd4ba5

Added waf config to change detection

bd2bf6c

Added ExactMatch -Path- to change detection

Update docs for ALBCM

4302bb9

fischerman and others added 13 commits March 26, 2026 10:48

Fix ALB unit tests

76f737e

feat: read configuration from cloud config

5744c59

chore: add a short description for setIPAddresses function

a4c3eee

Include envtest for controller tests

76e1f49

Undo temporary changes

1b1f8da

Fix linter issues

931b420

Remove license from code

a19d91f

chore: adjust issuer sample

261c701

chore: clarify isCertValid

8eeb62c

chore: remove debug messages

f7d1a76

fix: certificates not created because loadCerts skips all ingress tls…

1ff73e6

… references; adjusted requeuing logic for certificates

fix: certificate deletion logic

838e4c5

chore: adjsut externalIPAnnotation comment

9d6f767

dergeberl force-pushed the feat/kp/add-alb-ingress-controller branch from 3ad18ef to 9d6f767 Compare March 26, 2026 10:04

ske-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 26, 2026

dergeberl added 10 commits March 26, 2026 11:07

Adopt config to config structure

98c71c0

Move ReadConfig to config package

e823347

Remove unused webhook

c66f776

Remove secure metrics

3ec8b81

Enable LeaderElectionReleaseOnCancel

f9fdf11

Make linter happy

d8cb339

Remove kubebuilder scaffold comments

693873a

Remove crd import in envtest; remove getFirstFoundEnvTestBinaryDir as…

f4ec638

… this requires the makefile from kubebuilder

Remove dummy comment

85e6a10

Refactor SetupWithManager

87dc2c6

hown3d reviewed Mar 27, 2026

View reviewed changes


		> NOTE: The service has to be of type NodePort to enable access to the nodes from the outside of the cluster.

		3. Create an IngressClass that specifies the ALB Ingress controller:

Conversation

kamilprzybyl commented Mar 17, 2026

Uh oh!

ske-prow bot commented Mar 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kamilprzybyl Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kamilprzybyl commented Mar 22, 2026

Uh oh!

kamilprzybyl commented Mar 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

kamilprzybyl Mar 20, 2026 •

edited

Loading