Preserve external controller state during reconciliation

internal/controller/provisioner/setter.go

@@ -2,6 +2,8 @@ package provisioner
 
 import (
 	"maps"
+	"slices"
+	"strings"
 
 	appsv1 "k8s.io/api/apps/v1"
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
@@ -83,8 +85,56 @@ func serviceSpecSetter(
 	objectMeta metav1.ObjectMeta,
 ) controllerutil.MutateFn {
 	return func() error {
+		const managedKeysAnnotation = "gateway.nginx.org/internal-managed-annotation-keys"
+
+		// Track which annotation keys NGF currently manages
+		currentManagedKeys := make(map[string]bool)
+		for k := range objectMeta.Annotations {
+			currentManagedKeys[k] = true
+		}
+
+		// Get previously managed keys from existing service
+		var previousManagedKeys map[string]bool
+		if prevKeysStr, ok := service.Annotations[managedKeysAnnotation]; ok {
+			previousManagedKeys = make(map[string]bool)
+			for _, k := range strings.Split(prevKeysStr, ",") {
+				if k != "" {
+					previousManagedKeys[k] = true
+				}
+			}
+		}
+
+		// Start with existing annotations (preserves external controller annotations)
+		mergedAnnotations := make(map[string]string)
+		for k, v := range service.Annotations {
+			// Skip the internal tracking annotation
+			if k == managedKeysAnnotation {
+				continue
+			}
+			// Remove annotations that NGF previously managed but no longer wants
+			if previousManagedKeys != nil && previousManagedKeys[k] && !currentManagedKeys[k] {
+				continue // Remove this annotation
+			}
+			mergedAnnotations[k] = v
+		}
+
+		// Apply NGF-managed annotations (take precedence)
+		for k, v := range objectMeta.Annotations {
+			mergedAnnotations[k] = v
+		}
+
+		// Store current managed keys for next reconciliation
+		if len(currentManagedKeys) > 0 {
+			var managedKeysList []string
+			for k := range currentManagedKeys {
+				managedKeysList = append(managedKeysList, k)
+			}
+			slices.Sort(managedKeysList) // Sort for deterministic output
+			mergedAnnotations[managedKeysAnnotation] = strings.Join(managedKeysList, ",")
+		}
+
 		service.Labels = objectMeta.Labels
-		service.Annotations = objectMeta.Annotations
+		service.Annotations = mergedAnnotations
 		service.Spec = spec
 		return nil
 	}

internal/controller/provisioner/setter_test.go

@@ -0,0 +1,153 @@
+package provisioner
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestServiceSpecSetter_PreservesExternalAnnotations(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		existingAnnotations map[string]string
+		desiredAnnotations  map[string]string
+		expectedAnnotations map[string]string
+		name                string
+	}{
+		{
+			name: "preserves MetalLB annotations while adding NGF annotations",
+			existingAnnotations: map[string]string{
+				"metallb.universe.tf/ip-allocated-from-pool": "production-public-ips",
+				"metallb.universe.tf/loadBalancerIPs":        "192.168.1.100",
+			},
+			desiredAnnotations: map[string]string{
+				"custom.annotation": "from-gateway-infrastructure",
+			},
+			expectedAnnotations: map[string]string{
+				"metallb.universe.tf/ip-allocated-from-pool":         "production-public-ips",
+				"metallb.universe.tf/loadBalancerIPs":                "192.168.1.100",
+				"custom.annotation":                                  "from-gateway-infrastructure",
+				"gateway.nginx.org/internal-managed-annotation-keys": "custom.annotation",
+			},
+		},
+		{
+			name: "NGF annotations take precedence on conflicts",
+			existingAnnotations: map[string]string{
+				"custom.annotation":                "old-value",
+				"metallb.universe.tf/address-pool": "staging",
+			},
+			desiredAnnotations: map[string]string{
+				"custom.annotation": "new-value",
+			},
+			expectedAnnotations: map[string]string{
+				"custom.annotation":                                  "new-value",
+				"metallb.universe.tf/address-pool":                   "staging",
+				"gateway.nginx.org/internal-managed-annotation-keys": "custom.annotation",
+			},
+		},
+		{
+			name:                "creates new service with annotations",
+			existingAnnotations: nil,
+			desiredAnnotations: map[string]string{
+				"custom.annotation": "value",
+			},
+			expectedAnnotations: map[string]string{
+				"custom.annotation": "value",
+				"gateway.nginx.org/internal-managed-annotation-keys": "custom.annotation",
+			},
+		},
+		{
+			name: "removes NGF-managed annotations when no longer desired",
+			existingAnnotations: map[string]string{
+				"custom.annotation":                                  "should-be-removed",
+				"metallb.universe.tf/ip-allocated-from-pool":         "production",
+				"gateway.nginx.org/internal-managed-annotation-keys": "custom.annotation",
+			},
+			desiredAnnotations: map[string]string{},
+			expectedAnnotations: map[string]string{
+				"metallb.universe.tf/ip-allocated-from-pool": "production",
+			},
+		},
+		{
+			name: "preserves cloud provider annotations",
+			existingAnnotations: map[string]string{
+				"service.beta.kubernetes.io/aws-load-balancer-type":   "nlb",
+				"service.beta.kubernetes.io/aws-load-balancer-scheme": "internet-facing",
+			},
+			desiredAnnotations: map[string]string{
+				"custom.annotation": "from-nginxproxy-patch",
+			},
+			expectedAnnotations: map[string]string{
+				"service.beta.kubernetes.io/aws-load-balancer-type":   "nlb",
+				"service.beta.kubernetes.io/aws-load-balancer-scheme": "internet-facing",
+				"custom.annotation": "from-nginxproxy-patch",
+				"gateway.nginx.org/internal-managed-annotation-keys": "custom.annotation",
+			},
+		},
+		{
+			name: "updates tracking annotation when managed keys change",
+			existingAnnotations: map[string]string{
+				"annotation-to-keep":                                 "value1",
+				"annotation-to-remove":                               "value2",
+				"metallb.universe.tf/address-pool":                   "production",
+				"gateway.nginx.org/internal-managed-annotation-keys": "annotation-to-keep,annotation-to-remove",
+			},
+			desiredAnnotations: map[string]string{
+				"annotation-to-keep": "value1",
+			},
+			expectedAnnotations: map[string]string{
+				"annotation-to-keep":                                 "value1",
+				"metallb.universe.tf/address-pool":                   "production",
+				"gateway.nginx.org/internal-managed-annotation-keys": "annotation-to-keep",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			g := NewWithT(t)
+
+			// Create existing service with annotations
+			existingService := &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "test-service",
+					Namespace:   "default",
+					Annotations: tt.existingAnnotations,
+				},
+			}
+
+			// Create desired object metadata with NGF-managed annotations
+			desiredMeta := metav1.ObjectMeta{
+				Labels: map[string]string{
+					"app": "nginx-gateway",
+				},
+				Annotations: tt.desiredAnnotations,
+			}
+
+			// Create desired spec
+			desiredSpec := corev1.ServiceSpec{
+				Type: corev1.ServiceTypeLoadBalancer,
+				Ports: []corev1.ServicePort{
+					{
+						Name:     "http",
+						Port:     80,
+						Protocol: corev1.ProtocolTCP,
+					},
+				},
+			}
+
+			// Execute the setter
+			setter := serviceSpecSetter(existingService, desiredSpec, desiredMeta)
+			err := setter()
+
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(existingService.Annotations).To(Equal(tt.expectedAnnotations))
+			g.Expect(existingService.Labels).To(Equal(desiredMeta.Labels))
+			g.Expect(existingService.Spec).To(Equal(desiredSpec))
+		})
+	}
+}