WIP: Application load balancer controller by fischerman · Pull Request #3 · stackitcloud/application-load-balancer-controller

fischerman · 2026-06-24T20:24:10Z

No description provided.

nschad · 2026-06-25T09:58:47Z

@fischerman either mark the PR as "Draft" or use "WIP" as the title. The PR will then automatically marked as WIP/Draft

ske-prow · 2026-06-25T14:33:32Z

[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 dergeberl 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

Co-authored-by: Kamil Przybyl <Kamil.Przybyl@digits.schwarz>

dergeberl · 2026-06-29T13:41:35Z

+				WithRule("my-host.local", WithPath("/", new(networkingv1.PathTypePrefix), service.Name, networkingv1.ServiceBackendPort{Number: 80})),
+			)
+			testutil.CreateKubernetesResourceAndDeferDeletion(ctx, k8sClient, &ingress)
+


We also shpuld check that the IP is set in the status of the ingress.

dergeberl · 2026-06-29T13:43:59Z

+			expected: true,
+		},
+		{
+			name: "https certificates changed",


we should add the same but with also 2 Ids.

dergeberl · 2026-06-29T13:44:56Z

+			expected: true,
+		},
+		{
+			name: "target pool tls validation changed",


We should also add targets itself and test.

dergeberl · 2026-06-29T13:48:03Z

@@ -0,0 +1,302 @@
+package ingress


In the update.go are multiple functions which also should be tested, like getServicesForIngresses, getTLSSecretsFromIngresses, getCertificatesForIngressClass. But also the logic for duplicateCerts should get tested.

dergeberl · 2026-06-29T13:48:48Z

+	"k8s.io/utils/ptr"
+)
+
+func Test_updateNeeded(t *testing.T) {


let's stick to ginkgo tests

dergeberl · 2026-06-29T14:33:07Z

+		)
+
+		Expect(errs).To(HaveLen(1))
+		Expect(errs[0].Description).To(Equal("invalid certificate: tls: failed to find any PEM data in certificate input"))


Same as in test should return an error when the TLS secret isn't of type TLS

dergeberl · 2026-06-29T14:37:41Z

+		Expect(tree.GetMissingCertificates(nil)).To(ConsistOf(
+			WorkTreeCertificate{
+				PublicKey:  testdata.FixtureTLS1PublicKey,
+				PrivateKey: testdata.FixtureTLS1PrivateKey,


This test is failing as the reutrn also contains Ports: {443: nil},

Suggested change

PrivateKey: testdata.FixtureTLS1PrivateKey,

PrivateKey: testdata.FixtureTLS1PrivateKey,

Ports: map[int16]any{443: nil},

dergeberl · 2026-06-29T14:54:20Z

+
+	tree := &WorkTreeALB{
+		ingressClass: ingressClass,
+		planID:       GetAnnotation(AnnotationPlanID, "", ingressClass),


Should we validate that we only get working PlanIDs. For NLBs we check that in cloud-provider-stackit

dergeberl · 2026-06-29T14:58:46Z

@@ -0,0 +1,658 @@
+package spec


We should also add tests for:

the AnnotationHTTPSOnly and that it not serves HTTP.

the sorting based on AnnotationPriority and creation timestamp.

AnnotationPlanID

the handling when setting not allowed values in annotation (like no valid int/port, not valid cidr in source range)

GetUnusedCertificates

dergeberl · 2026-06-30T05:48:57Z

@@ -0,0 +1,85 @@
+apiVersion: apps/v1


We should align this with the deploy of CSI and CCM in cloud-provider-stackit (which are also not perfect, see stackitcloud/cloud-provider-stackit#1162).
For example:

we can use also the kube-system namespace as this is also a system component like CSI and CCM

the cloud config don't need to be a secret

the mount path of the service account and cloud config

the apiEndpoints don't need to be in the example (this is only internally needed)

lets also create a /deploy folder instead if the /samples/deploy/

we need to find a image tag which is working and don't need to be updated like latest (not sure if this will be tagged on release)

With #6 we get a latest tag for tagged versions. Lets use that in the deployment.

dergeberl · 2026-06-30T05:50:48Z

+// +kubebuilder:rbac:groups=core,resources=services,verbs=get;list;watch
+// +kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch
+// +kubebuilder:rbac:groups=core,resources=nodes,verbs=get;list;watch
+// +kubebuilder:rbac:groups=core,resources=events,verbs=create;patch
+// +kubebuilder:rbac:groups=networking.k8s.io,resources=ingressclasses,verbs=get;list;watch;update;patch
+// +kubebuilder:rbac:groups=networking.k8s.io,resources=ingresses,verbs=get;list;watch
+// +kubebuilder:rbac:groups=networking.k8s.io,resources=ingresses/status,verbs=get;update;patch
+


We don't use anything from kubebuilder in that repo, lets not add this here.

dergeberl · 2026-06-30T05:59:50Z

+	ConditionNodeTermination corev1.NodeConditionType = "Terminating"
+)
+
+func isNodeTerminating(node *corev1.Node) bool {


That changes also need to be added to the nodePredicate().

dergeberl · 2026-06-30T06:20:30Z

+		if !httpsOnly && (httpPort <= 0 || httpPort > math.MaxUint16) {
+			errors = append(errors, ErrorEvent{
+				Ingress:     ingress,
+				Description: "HTTP port is out of range.",
+			})
+			continue
+		}
+		if len(ingress.Spec.TLS) > 0 && (httpsPort <= 0 || httpsPort > math.MaxUint16) {
+			errors = append(errors, ErrorEvent{
+				Ingress:     ingress,
+				Description: "HTTPS port is out of range.",
+			})
+			continue
+		}
+


We should add a test for that.

Avoid the use of underscores at the beginning of variable names

ske-prow · 2026-06-30T09:41:39Z

@fischerman: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-application-load-balancer-controller-verify	`3fec1b5`	link	true	`/test pull-application-load-balancer-controller-verify`

Full PR test history. Your PR dashboard. Command help for this repository.
Please help us cut down on flakes by linking this test failure to an open flake report or filing a new flake report if you can't find an existing one. Also see the gardener testing guideline for how to avoid and hunt flakes.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

ftl · 2026-06-30T10:34:19Z

+	albOpts := []sdkconfig.ConfigurationOption{
+		sdkconfig.WithUserAgent("application-load-balancer-controller"),
+	}


The SDK client should use the instrumented HTTP client that collects metrics about API requests, especially failed requests.

Suggested change

albOpts := []sdkconfig.ConfigurationOption{

sdkconfig.WithUserAgent("application-load-balancer-controller"),

}

albOpts := []sdkconfig.ConfigurationOption{

sdkconfig.WithUserAgent("application-load-balancer-controller"),

sdkconfig.WithHTTPClient(metrics.NewInstrumentedHTTPClient()),

}

(stackitcloud/cloud-provider-stackit#1228 will extend the coverage of the metrics soon)

ftl · 2026-06-30T10:35:10Z

+	certOpts := []sdkconfig.ConfigurationOption{
+		sdkconfig.WithUserAgent("application-load-balancer-controller"),
+	}


(see my comment above for albOpts)

Suggested change

certOpts := []sdkconfig.ConfigurationOption{

sdkconfig.WithUserAgent("application-load-balancer-controller"),

}

certOpts := []sdkconfig.ConfigurationOption{

sdkconfig.WithHTTPClient(metrics.NewInstrumentedHTTPClient()),

sdkconfig.WithUserAgent("application-load-balancer-controller"),

}

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 Jun 24, 2026

nschad changed the title ~~Draft: Application load balancer controller~~ WIP: Application load balancer controller Jun 25, 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 Jun 25, 2026

nschad force-pushed the main branch from e5c3fee to fba4e1b Compare June 25, 2026 14:11

dergeberl requested review from breuerfelix, dergeberl and hown3d June 25, 2026 14:29

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

kamilprzybyl and others added 19 commits June 25, 2026 16:34

docs: add user documentation for ALB Ingress controller (#2)

2f824df

Co-authored-by: Kamil Przybyl <Kamil.Przybyl@digits.schwarz>

Move ALBC from CCM repository

56ba53d

Fix some linter issues

62d9cfe

Fixed controller tests

9cdcdb3

Add event handler for services

40658b0

Verbosity levels

7ea52ba

Delete certificates on ingress class deletion

dc97548

Move labels package into spec

ef8a935

Clear some TODOs

8ff5db0

Give certificates a proper name

a74c86d

Remove unused certificates even after creation

6438207

Ingress class annotation

1c9d8e1

Support source ranges

e0a040e

Do a couple of documentation TODOs

6f7137e

Implement target pool limit

0a0ae5c

Document target pool limits

5675ad5

Fix test

c23a556

Support internal ALBs

d539b21

Support static LBs

209f335

fischerman added 7 commits June 29, 2026 16:01

Enable lll linter

3dfdfc4

Merge two if's into one

0814a54

Use constants for protocol

07d1bb1

Remove webhook section

836aaa9

Add a topic sentence to README

e2b8aab

Implement feedback for STACKIT package

d722435

Validate that HTTP and HTTPS port are in range

6ade87a

dergeberl reviewed Jun 29, 2026

View reviewed changes

fischerman added 4 commits June 29, 2026 17:01

Change port to uint16

0906f8c

Remove custom scheme

b7601e3

Consistently use LoadBalancerName

12f902a

Use newer DEV image

71328f0

dergeberl reviewed Jun 30, 2026

View reviewed changes

fischerman added 11 commits June 30, 2026 08:59

Make tree receiver of addPath

32a99ac

Fix test assertion

364cdc3

Initialize target pool as late as possible

4477655

Add comment to clarify API behavior for health checks

930450d

Check that health check port is set

c67eabd

Don't use the same name for variables and types

0965e2b

Avoid the use of underscores at the beginning of variable names

Remove early return after adding finalizer

29fb10f

Set user agent for STACKIT APIs

ebb7137

Wrap original error for not found

9b3f364

Don't start variables with underscore

0789a1f

Switch to the new event API

3fec1b5

ftl reviewed Jun 30, 2026

View reviewed changes

	PrivateKey: testdata.FixtureTLS1PrivateKey,
	PrivateKey: testdata.FixtureTLS1PrivateKey,
	Ports: map[int16]any{443: nil},

Uh oh!

Conversation

fischerman commented Jun 24, 2026

Uh oh!

nschad commented Jun 25, 2026

Uh oh!

ske-prow Bot commented Jun 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!

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!

ske-prow Bot commented Jun 30, 2026

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!

8 participants