tailscale/tailscale

Early return in serviceHandlerForIngress skips matching Tailscale Ingresses, causing missed reconciliations

Summary

  • Context: The serviceHandlerForIngress function in cmd/k8s-operator/operator.go is a Kubernetes controller handler that determines which Ingresses need to be reconciled when a backend Service changes.

  • Bug: The function uses return nil instead of continue when encountering an Ingress with a non-matching ingress class name.

  • Actual vs. expected: Instead of skipping non-matching Ingresses and continuing to check the rest, the function exits early, preventing any subsequent Ingresses from being processed.

  • Impact: Tailscale Ingresses may fail to update when their backend Services change, leading to stale routing configuration that directs traffic to wrong or unavailable backends.

Code with bug

Location: cmd/k8s-operator/operator.go

func serviceHandlerForIngress(
    cl client.Client,
    logger *zap.SugaredLogger,
    ingressClassName string,
) handler.MapFunc {
    return func(ctx context.Context, o client.Object) []reconcile.Request {
        if isManagedByType(o, "ingress") {
            ingName := parentFromObjectLabels(o)
            return []reconcile.Request{{NamespacedName: ingName}}
        }

        ingList := networkingv1.IngressList{}
        if err := cl.List(ctx, &ingList, client.InNamespace(o.GetNamespace())); err != nil {
            logger.Debugf("error listing Ingresses: %v", err)
            return nil
        }

        reqs := make([]reconcile.Request, 0)

        for _, ing := range ingList.Items {
            if ing.Spec.IngressClassName == nil ||
                *ing.Spec.IngressClassName != ingressClassName {
                return nil // <-- BUG 🔴 exits function early, should skip to next iteration
            }

            if hasProxyGroupAnnotation(&ing) {
                continue
            }

            if ing.Spec.DefaultBackend != nil &&
                ing.Spec.DefaultBackend.Service != nil &&
                ing.Spec.DefaultBackend.Service.Name == o.GetName() {
                reqs = append(reqs, reconcile.Request{
                    NamespacedName: client.ObjectKeyFromObject(&ing),
                })
            }

            for _, rule := range ing.Spec.Rules {
                if rule.HTTP == nil {
                    continue
                }

                for _, path := range rule.HTTP.Paths {
                    if path.Backend.Service != nil &&
                        path.Backend.Service.Name == o.GetName() {
                        reqs = append(reqs, reconcile.Request{
                            NamespacedName: client.ObjectKeyFromObject(&ing),
                        })
                    }
                }
            }
        }

        return reqs

Example

Consider a cluster with three Ingresses all using the same backend Service:

  • ingress-nginx (class: “nginx”)

  • ingress-tailscale-1 (class: “tailscale”)

  • ingress-tailscale-2 (class: “tailscale”)

When the backend Service changes, the handler should return [ingress-tailscale-1, ingress-tailscale-2] to reconcile both Tailscale Ingresses.

Buggy behavior:


Expected behavior:

Input:
  List returns:
    - ingress-nginx
    - ingress-tailscale-1
    - ingress-tailscale-2

Output:
  [ingress-tailscale-1, ingress-tailscale-2]

The bug is non-deterministic because Kubernetes List() ordering is not guaranteed. It only manifests when:

  1. Multiple ingress controllers exist in the cluster

  2. A Service is shared across Ingresses with different classes

  3. A non-Tailscale Ingress appears before a Tailscale Ingress in the list

This explains why the bug may work correctly in simple test environments but fail unpredictably in production clusters with multiple ingress controllers.

Recommended fix

Change the early return to a continue statement:

for _, ing := range ingList.Items {
    if ing.Spec.IngressClassName == nil ||
        *ing.Spec.IngressClassName != ingressClassName {
        continue // <-- FIX 🟢 skip to next iteration instead of exiting
    }

    // ... rest of loop logic