Remove the deprecated domain annotation on Load balancer services in … #722

alex-bezek · 2025-12-05T18:55:14Z

…favor of the url annotation

What

This has been deprecated for a while in favor of the k8s.ngrok.com/url annotation. We should remove this

How

Simply removes the old code paths and updates some comments and documentation.

Breaking Changes

Yes, this deletes the k8s.ngrok.com/domain annotation support from the LoadBalancer Service. Its been deprecated for some time now in favor of k8s.ngrok.com/url

Validation

Created a couple of load balancer services

# TCP LoadBalancer (no URL annotation = auto-assigned)
apiVersion: v1
kind: Service
metadata:
  name: test-tcp
  namespace: default
  annotations:
    k8s.ngrok.com/mapping-strategy: endpoints-verbose
  labels:
    app.kubernetes.io/name: test-server
spec:
  allocateLoadBalancerNodePorts: false
  loadBalancerClass: ngrok
  ports:
    - name: tcp
      port: 8080
      protocol: TCP
      targetPort: 8080
  selector:
    app.kubernetes.io/name: test-server
  type: LoadBalancer

---
# TLS LoadBalancer (with custom domain)
apiVersion: v1
kind: Service
metadata:
  name: test-tls
  namespace: default
  annotations:
    k8s.ngrok.com/url: tls://test-tcp.alexbezek.me:443
    k8s.ngrok.com/mapping-strategy: endpoints-verbose
  labels:
    app.kubernetes.io/name: test-server
spec:
  allocateLoadBalancerNodePorts: false
  loadBalancerClass: ngrok
  ports:
    - name: tcp
      port: 8080
      protocol: TCP
      targetPort: 8080
  selector:
    app.kubernetes.io/name: test-server
  type: LoadBalancer

and confirmed the status is set correctly

k get svc -o yaml
apiVersion: v1
items:
- apiVersion: v1
  kind: Service
  metadata:
    creationTimestamp: "2025-12-05T16:19:59Z"
    labels:
      component: apiserver
      provider: kubernetes
    name: kubernetes
    namespace: default
    resourceVersion: "195"
    uid: 5470f946-dbec-4c47-9a70-aa6b21d09ee5
  spec:
    clusterIP: 10.96.0.1
    clusterIPs:
    - 10.96.0.1
    internalTrafficPolicy: Cluster
    ipFamilies:
    - IPv4
    ipFamilyPolicy: SingleStack
    ports:
    - name: https
      port: 443
      protocol: TCP
      targetPort: 6443
    sessionAffinity: None
    type: ClusterIP
  status:
    loadBalancer: {}
- apiVersion: v1
  kind: Service
  metadata:
    annotations:
      annotation1: val1
      annotation2: val2
    creationTimestamp: "2025-12-11T19:57:07Z"
    labels:
      app.kubernetes.io/managed-by: ngrok-operator
      bindings.k8s.ngrok.com/endpoint-binding-name: ngrok-d30e730c-d44b-5746-a308-a58507613e80
      bindings.k8s.ngrok.com/endpoint-binding-namespace: ngrok-operator
      label1: val1
    name: ngrok-operator-bindings-test-clep
    namespace: default
    resourceVersion: "28387"
    uid: 0a41d823-5845-4d7c-b286-29df81d389dc
  spec:
    externalName: ngrok-d30e730c-d44b-5746-a308-a58507613e80.ngrok-operator.svc.cluster.local
    ports:
    - name: http
      port: 80
      protocol: TCP
      targetPort: 80
    sessionAffinity: ClientIP
    sessionAffinityConfig:
      clientIP:
        timeoutSeconds: 10800
    type: ExternalName
  status:
    loadBalancer: {}
- apiVersion: v1
  kind: Service
  metadata:
    annotations:
      k8s.ngrok.com/computed-url: tcp://5.tcp.ngrok.io:24266
      k8s.ngrok.com/mapping-strategy: endpoints-verbose
      kubectl.kubernetes.io/last-applied-configuration: |
        {"apiVersion":"v1","kind":"Service","metadata":{"annotations":{"k8s.ngrok.com/mapping-strategy":"endpoints-verbose"},"labels":{"app.kubernetes.io/name":"test-server"},"name":"test-tcp","namespace":"default"},"spec":{"allocateLoadBalancerNodePorts":false,"loadBalancerClass":"ngrok","ports":[{"name":"tcp","port":8080,"protocol":"TCP","targetPort":8080}],"selector":{"app.kubernetes.io/name":"test-server"},"type":"LoadBalancer"}}
    creationTimestamp: "2025-12-11T21:17:41Z"
    finalizers:
    - k8s.ngrok.com/finalizer
    labels:
      app.kubernetes.io/name: test-server
    name: test-tcp
    namespace: default
    resourceVersion: "37896"
    uid: 421b56b7-5bb3-434c-9cf1-7af7ac2ba999
  spec:
    allocateLoadBalancerNodePorts: false
    clusterIP: 10.96.224.124
    clusterIPs:
    - 10.96.224.124
    externalTrafficPolicy: Cluster
    internalTrafficPolicy: Cluster
    ipFamilies:
    - IPv4
    ipFamilyPolicy: SingleStack
    loadBalancerClass: ngrok
    ports:
    - name: tcp
      port: 8080
      protocol: TCP
      targetPort: 8080
    selector:
      app.kubernetes.io/name: test-server
    sessionAffinity: None
    type: LoadBalancer
  status:
    loadBalancer:
      ingress:
      - hostname: 5.tcp.ngrok.io
        ports:
        - port: 24266
          protocol: TCP
- apiVersion: v1
  kind: Service
  metadata:
    annotations:
      k8s.ngrok.com/computed-url: tls://test-tcp.alexbezek.me:443
      k8s.ngrok.com/mapping-strategy: endpoints-verbose
      k8s.ngrok.com/url: tls://test-tcp.alexbezek.me:443
      kubectl.kubernetes.io/last-applied-configuration: |
        {"apiVersion":"v1","kind":"Service","metadata":{"annotations":{"k8s.ngrok.com/mapping-strategy":"endpoints-verbose","k8s.ngrok.com/url":"tls://test-tcp.alexbezek.me:443"},"labels":{"app.kubernetes.io/name":"test-server"},"name":"test-tls","namespace":"default"},"spec":{"allocateLoadBalancerNodePorts":false,"loadBalancerClass":"ngrok","ports":[{"name":"tcp","port":8080,"protocol":"TCP","targetPort":8080}],"selector":{"app.kubernetes.io/name":"test-server"},"type":"LoadBalancer"}}
    creationTimestamp: "2025-12-11T21:17:41Z"
    finalizers:
    - k8s.ngrok.com/finalizer
    labels:
      app.kubernetes.io/name: test-server
    name: test-tls
    namespace: default
    resourceVersion: "59839"
    uid: 87674c17-b861-4187-a1b0-cc86eb1416d0
  spec:
    allocateLoadBalancerNodePorts: false
    clusterIP: 10.96.172.116
    clusterIPs:
    - 10.96.172.116
    externalTrafficPolicy: Cluster
    internalTrafficPolicy: Cluster
    ipFamilies:
    - IPv4
    ipFamilyPolicy: SingleStack
    loadBalancerClass: ngrok
    ports:
    - name: tcp
      port: 8080
      protocol: TCP
      targetPort: 8080
    selector:
      app.kubernetes.io/name: test-server
    sessionAffinity: None
    type: LoadBalancer
  status:
    loadBalancer:
      ingress:
      - hostname: 2kapoyyjhr5reymrz.3kg7imcniwfgy1ozt.ngrok-cname.com
        ports:
        - port: 443
          protocol: TCP
kind: List
metadata:
  resourceVersion: ""

internal/controller/service/controller_test.go

codecov · 2025-12-05T20:33:13Z

Codecov Report

❌ Patch coverage is 76.92308% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.06%. Comparing base (0c3f398) to head (6584e00).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/controller/service/controller.go	76.92%	3 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #722      +/-   ##
==========================================
- Coverage   49.14%   49.06%   -0.09%     
==========================================
  Files          95       95              
  Lines       10570    10557      -13     
==========================================
- Hits         5195     5180      -15     
+ Misses       5016     5014       -2     
- Partials      359      363       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

internal/controller/service/controller.go

jonstacks

Found an small issue while testing.

jonstacks · 2025-12-12T03:45:50Z

internal/controller/service/controller.go

-	default: // computedURL not present, fallback to the domain annotation
-		domain, err := parser.GetStringAnnotation("domain", svc)
-		if err != nil {
-			if errors.IsMissingAnnotations(err) {
-				return clearIngressStatus(svc)
-			}
-			return err
-		}
-
-		// Use this domain temporarily, but also check if there is a
-		// more specific CNAME value on the domain to use
-		hostname = domain
-
-		dr := endpoint.GetDomainRef()
-		if dr != nil {
-			// Lookup the domain
-			domain := &ingressv1alpha1.Domain{}
-			if err := c.Get(ctx, client.ObjectKey{Namespace: *dr.Namespace, Name: dr.Name}, domain); err != nil {
-				return err
-			}
-			if domain.Status.CNAMETarget != nil {
-				hostname = *domain.Status.CNAMETarget
-			}
-		}


I broke this for the url annotation prior to this PR, but it was working for the domain annotation. Leaving the comment here, because I think its related to this.

I've deployed this branch and saw that my custom domains no longer get the ngrok-cname in the load balancer status. It just happens to copy the custom domain to the load balancer status and then external-dns tells me that the record is invalid.

…favor of the url annotation

…nd isntead derive it from the computed url satus

…alue for custom domains

alex-bezek · 2025-12-15T19:33:55Z

tests/chainsaw/loadbalancer-services/test-tls-custom-domain-service.yaml

+metadata:
+  name: test-tls-custom-domain-lb
+  annotations:
+    k8s.ngrok.com/url: tls://service-lb-tls-test-ngrok-operator.example.com:443


This test fails in the e2e merge queue. I picked a domain i thought would be fairly custom and not used to avoid conflicts with future clients. However, the issue is that i run it locally so my account reserved the domain and now the CI one will fail. I can't just unreserve it as we'll then face the same issue in our local systems.

Instead i'm going to just comment this test out for now and will investigate how we can ensure these domains are custom either by having chainsaw create a unique namespace for each run and use that in its templating, or by injecting a unique value from the PR number or codespace

#722) * Remove the deprecated domain annotation on Load balancer services in favor of the url annotation * Fix status calculation and update tests * remove default behavior of trying to parse annotation to set status and isntead derive it from the computed url satus * Set computed URL even for tls connections * lint * handle setting service status field based on domainRef to get CNAME value for custom domains * remove chainsaw test for now

alex-bezek requested a review from a team as a code owner December 5, 2025 18:55

github-actions bot added area/controller Issues dealing with the controller size/M Denotes a PR that changes 30-99 lines labels Dec 5, 2025

jonstacks reviewed Dec 5, 2025

View reviewed changes

internal/controller/service/controller_test.go Show resolved Hide resolved

github-actions bot added size/L Denotes a PR that changes 100-499 lines and removed size/M Denotes a PR that changes 30-99 lines labels Dec 5, 2025

jonstacks added this to the controller-0.20.0 milestone Dec 5, 2025

alex-bezek marked this pull request as draft December 5, 2025 20:49

jonstacks reviewed Dec 5, 2025

View reviewed changes

internal/controller/service/controller.go Outdated Show resolved Hide resolved

alex-bezek marked this pull request as ready for review December 11, 2025 03:23

alex-bezek requested a review from jonstacks December 11, 2025 16:30

jonstacks approved these changes Dec 11, 2025

View reviewed changes

internal/controller/service/controller.go Show resolved Hide resolved

jonstacks added this pull request to the merge queue Dec 11, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 11, 2025

jonstacks added this pull request to the merge queue Dec 11, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 11, 2025

alex-bezek requested a review from jonstacks December 11, 2025 22:37

jonstacks reviewed Dec 12, 2025

View reviewed changes

alex-bezek added 6 commits December 12, 2025 16:37

Remove the deprecated domain annotation on Load balancer services in …

b6043e4

…favor of the url annotation

Fix status calculation and update tests

038d206

remove default behavior of trying to parse annotation to set status a…

f505853

…nd isntead derive it from the computed url satus

Set computed URL even for tls connections

6c17781

lint

9298d83

handle setting service status field based on domainRef to get CNAME v…

81d65ee

…alue for custom domains

alex-bezek force-pushed the alex/remove-domain-annotation-from-lb-svc branch from 06dfc94 to 81d65ee Compare December 12, 2025 22:19

jonstacks approved these changes Dec 15, 2025

View reviewed changes

alex-bezek added this pull request to the merge queue Dec 15, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 15, 2025

alex-bezek commented Dec 15, 2025

View reviewed changes

remove chainsaw test for now

8cc7718

jonstacks approved these changes Dec 15, 2025

View reviewed changes

alex-bezek added this pull request to the merge queue Dec 15, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 15, 2025

alex-bezek added this pull request to the merge queue Dec 15, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 15, 2025

remove chainsaw asserts as well...

6584e00

alex-bezek enabled auto-merge December 15, 2025 21:06

alex-bezek added this pull request to the merge queue Dec 15, 2025

Merged via the queue into main with commit a956d80 Dec 15, 2025
11 checks passed

alex-bezek deleted the alex/remove-domain-annotation-from-lb-svc branch December 15, 2025 21:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove the deprecated domain annotation on Load balancer services in … #722

Remove the deprecated domain annotation on Load balancer services in … #722

Uh oh!

alex-bezek commented Dec 5, 2025 •

edited

Loading

Uh oh!

Uh oh!

codecov bot commented Dec 5, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jonstacks left a comment

Uh oh!

jonstacks Dec 12, 2025

Uh oh!

Uh oh!

alex-bezek Dec 15, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Remove the deprecated domain annotation on Load balancer services in … #722

Remove the deprecated domain annotation on Load balancer services in … #722

Uh oh!

Conversation

alex-bezek commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

How

Breaking Changes

Validation

Uh oh!

Uh oh!

codecov bot commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jonstacks left a comment

Choose a reason for hiding this comment

Uh oh!

jonstacks Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alex-bezek Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alex-bezek commented Dec 5, 2025 •

edited

Loading

codecov bot commented Dec 5, 2025 •

edited

Loading