ir: Fix Service updates not propagating through translation

internal/kgateway/extensions2/plugins/backendtlspolicy/plugin.go

@@ -70,7 +70,7 @@ func (d *backendTlsPolicy) Equals(in any) bool {
 }
 
 func registerTypes() {
-	kubeclient.Register[*gwv1.BackendTLSPolicy](
+	kubeclient.Register(
 		backendTlsPolicyGvr,
 		backendTlsPolicyGroupKind,
 		func(c kubeclient.ClientGetter, namespace string, o metav1.ListOptions) (runtime.Object, error) {

internal/kgateway/setup/setup_test.go

@@ -322,6 +322,116 @@ spec:
 	})
 }
 
+func TestServiceAppProtocolUpdate(t *testing.T) {
+	st, err := envtestutil.BuildSettings()
+	if err != nil {
+		t.Fatalf("can't get settings %v", err)
+	}
+	setupEnvTestAndRun(t, st, func(t *testing.T, ctx context.Context, kdbg *krt.DebugHandler, client istiokube.CLIClient, xdsPort, _ int) {
+		client.Kube().CoreV1().Namespaces().Create(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "gwtest"}}, metav1.CreateOptions{})
+
+		err = client.ApplyYAMLContents("gwtest", `kind: Gateway
+apiVersion: gateway.networking.k8s.io/v1
+metadata:
+  name: http-gw
+  namespace: gwtest
+spec:
+  gatewayClassName: kgateway
+  listeners:
+  - protocol: HTTP
+    port: 8080
+    name: http
+    allowedRoutes:
+      namespaces:
+        from: All`, `apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+  name: ws-route
+  namespace: gwtest
+spec:
+  parentRefs:
+    - name: http-gw
+  hostnames:
+    - "ws.example.com"
+  rules:
+    - backendRefs:
+        - name: test-service
+          port: 8080
+      matches:
+        - path:
+            type: PathPrefix
+            value: /`, `apiVersion: v1
+kind: Service
+metadata:
+  name: test-service
+  namespace: gwtest
+spec:
+  selector:
+    app: reviews
+  ports:
+    - protocol: TCP
+      port: 8080
+      targetPort: 8080
+`)
+		if err != nil {
+			t.Fatalf("failed to apply initial resources: %v", err)
+		}
+
+		time.Sleep(time.Second * 2)
+		t.Cleanup(func() {
+			if t.Failed() {
+				logKrtState(t, fmt.Sprintf("krt state for failed test: %s", t.Name()), kdbg)
+			} else if os.Getenv("KGW_DUMP_KRT_ON_SUCCESS") == "true" {
+				logKrtState(t, fmt.Sprintf("krt state for successful test: %s", t.Name()), kdbg)
+			}
+		})
+		initialDump, err := dumpXdsConfig(t, ctx, xdsPort, "http-gw")
+		if err != nil {
+			t.Fatalf("failed to get initial xDS dump: %v", err)
+		}
+		hasWebsocketUpgrade := hasWebsocketUpgradeConfig(initialDump)
+		if hasWebsocketUpgrade {
+			t.Fatalf("expected no websocket upgrade in initial state, but found one")
+		}
+
+		err = client.ApplyYAMLContents("gwtest", `apiVersion: v1
+kind: Service
+metadata:
+  name: test-service
+  namespace: gwtest
+spec:
+  selector:
+    app: reviews
+  ports:
+    - protocol: TCP
+      appProtocol: kubernetes.io/ws
+      port: 8080
+      targetPort: 8080
+`)
+		if err != nil {
+			t.Fatalf("failed to update service with appProtocol: %v", err)
+		}
+
+		time.Sleep(time.Second * 2)
+
+		updatedDump, err := dumpXdsConfig(t, ctx, xdsPort, "http-gw")
+		if err != nil {
+			t.Fatalf("failed to get updated xDS dump: %v", err)
+		}
+		if len(updatedDump.Routes) > 0 {
+			hasWebsocketUpgrade := hasWebsocketUpgradeConfig(updatedDump)
+			if !hasWebsocketUpgrade {
+				t.Fatalf("expected websocket upgrade configuration after updating appProtocol, but found none")
+			}
+			t.Logf("SUCCESS: Found websocket upgrade configuration after updating appProtocol")
+		} else {
+			t.Fatalf("No routes found in updated dump")
+		}
+
+		t.Logf("%s finished", t.Name())
+	})
+}
+
 func runScenario(t *testing.T, scenarioDir string, globalSettings *apisettings.Settings) {
 	setupEnvTestAndRun(t, globalSettings, func(t *testing.T, ctx context.Context, kdbg *krt.DebugHandler, client istiokube.CLIClient, xdsPort, _ int) {
 		// list all yamls in test data
@@ -1023,3 +1133,29 @@ func getroutesnames(l *envoylistenerv3.Listener) []string {
 	}
 	return routes
 }
+
+// dumpXdsConfig dumps the xDS config for the given gateway name. helper function to
+// ensure the dumper is closed after the dump is complete.
+func dumpXdsConfig(t *testing.T, ctx context.Context, xdsPort int, gwName string) (xdsDump, error) {
+	dumper := newXdsDumper(t, ctx, xdsPort, gwName)
+	defer dumper.Close()
+	return dumper.Dump(t, ctx)
+}
+
+// hasWebsocketUpgradeConfig checks if any route in the xDS dump has websocket upgrade configuration
+func hasWebsocketUpgradeConfig(dump xdsDump) bool {
+	for _, route := range dump.Routes {
+		for _, vhost := range route.GetVirtualHosts() {
+			for _, r := range vhost.GetRoutes() {
+				if routeAction := r.GetRoute(); routeAction != nil {
+					for _, upgradeConfig := range routeAction.GetUpgradeConfigs() {
+						if upgradeConfig.GetUpgradeType() == "websocket" {
+							return true
+						}
+					}
+				}
+			}
+		}
+	}
+	return false
+}

pkg/pluginsdk/ir/backend.go

@@ -142,10 +142,10 @@ type BackendObjectIR struct {
 
 	// Errors is a list of errors, if any, encountered while constructing this BackendObject
 	// Not added to Equals() as it is derived from the inner ObjIr, which is already evaluated
-	// +krtEqualsTodo decide whether construction errors should impact equality
+	// +noKrtEquals
 	Errors []error
 
-	// Name is the pre-calculated resource name. used as the krt resource name.
+	// resourceName is the pre-calculated resource name. used as the krt resource name.
 	resourceName string
 
 	// TrafficDistribution is the desired traffic distribution for the backend.
@@ -167,10 +167,6 @@ func NewBackendObjectIR(objSource ObjectSource, port int32, extraKey string) Bac
 	}
 }
 
-func (c BackendObjectIR) ResourceName() string {
-	return c.resourceName
-}
-
 func BackendResourceName(objSource ObjectSource, port int32, extraKey string) string {
 	var sb strings.Builder
 	sb.WriteString(objSource.ResourceName())
@@ -183,22 +179,32 @@ func BackendResourceName(objSource ObjectSource, port int32, extraKey string) st
 	return sb.String()
 }
 
+// ResourceName returns the pre-calculated resource name for this backend.
+// This method is required to implement the krt.Named interface.
+func (c BackendObjectIR) ResourceName() string {
+	return c.resourceName
+}
+
 func (c BackendObjectIR) Equals(in BackendObjectIR) bool {
-	objEq := c.ObjectSource.Equals(in.ObjectSource)
-	objVersionEq := versionEquals(c.Obj, in.Obj)
-	polEq := c.AttachedPolicies.Equals(in.AttachedPolicies)
-	nameEq := c.resourceName == in.resourceName
-	disableIstioAutoMTLSEq := c.DisableIstioAutoMTLS == in.DisableIstioAutoMTLS
-
-	// objIr may currently be nil in the case of k8s Services
-	// TODO: add an IR for Services to avoid the need for this
-	// see: internal/kgateway/extensions2/plugins/kubernetes/k8s.go
-	objIrEq := true
-	if c.ObjIr != nil {
-		objIrEq = c.ObjIr.Equals(in.ObjIr)
+	if !c.ObjectSource.Equals(in.ObjectSource) {
+		return false
 	}
-
-	return objEq && objVersionEq && objIrEq && polEq && nameEq && disableIstioAutoMTLSEq
+	if !versionEquals(c.Obj, in.Obj) {
+		return false
+	}
+	if c.ObjIr != nil && !c.ObjIr.Equals(in.ObjIr) {
+		return false
+	}
+	if !c.AttachedPolicies.Equals(in.AttachedPolicies) {
+		return false
+	}
+	if c.resourceName != in.resourceName {
+		return false
+	}
+	if c.DisableIstioAutoMTLS != in.DisableIstioAutoMTLS {
+		return false
+	}
+	return true
 }
 
 func (c BackendObjectIR) ClusterName() string {
@@ -349,20 +355,28 @@ func (c Gateway) Equals(in Gateway) bool {
 		c.DeniedListenerSets.Equals(in.DeniedListenerSets)
 }
 
+type BackendRefIR struct {
+	// TODO: remove cluster name from here, it's redundant.
+	ClusterName string
+	Weight      uint32
+
+	// backend could be nil if not found or no ref grant
+	BackendObject *BackendObjectIR
+	// if nil, error might say why
+	Err error
+}
+
 // Equals returns true if the two BackendRefIR instances are equal in cluster name, weight, backend object equality, and error.
 func (a BackendRefIR) Equals(b BackendRefIR) bool {
 	if a.ClusterName != b.ClusterName || a.Weight != b.Weight {
 		return false
 	}
-
 	if !backendObjectEqual(a.BackendObject, b.BackendObject) {
 		return false
 	}
-
 	if !errorsEqual(a.Err, b.Err) {
 		return false
 	}
-
 	return true
 }
 

pkg/pluginsdk/ir/gw.go

@@ -254,23 +254,6 @@ func (l AttachedPolicies) MarshalJSON() ([]byte, error) {
 	return json.Marshal(m)
 }
 
-type BackendRefIR struct {
-	// TODO: remove cluster name from here, it's redundant.
-	ClusterName string
-	Weight      uint32
-
-	// backend could be nil if not found or no ref grant
-	BackendObject *BackendObjectIR
-	// if nil, error might say why
-	Err error
-}
-
-type HttpBackendOrDelegate struct {
-	Backend          *BackendRefIR
-	Delegate         *ObjectSource
-	AttachedPolicies AttachedPolicies
-}
-
 type HttpRouteRuleIR struct {
 	ExtensionRefs    AttachedPolicies
 	AttachedPolicies AttachedPolicies
@@ -282,3 +265,28 @@ type HttpRouteRuleIR struct {
 	// that should be propagated through to translation to any derived ir.HttpRouteRuleMatchIRs
 	Err error
 }
+
+type HttpBackendOrDelegate struct {
+	AttachedPolicies AttachedPolicies
+	Backend          *BackendRefIR
+	Delegate         *ObjectSource
+}
+
+func (h HttpBackendOrDelegate) Equals(other HttpBackendOrDelegate) bool {
+	if !h.AttachedPolicies.Equals(other.AttachedPolicies) {
+		return false
+	}
+	if h.Backend != nil && other.Backend != nil {
+		return h.Backend.Equals(*other.Backend)
+	}
+	if h.Backend == nil && other.Backend == nil {
+		if h.Delegate == nil && other.Delegate == nil {
+			return true
+		}
+		if h.Delegate != nil && other.Delegate != nil {
+			return h.Delegate.Equals(*other.Delegate)
+		}
+		return false
+	}
+	return false
+}

pkg/pluginsdk/ir/routes.go

@@ -19,6 +19,8 @@ type Route interface {
 	GetSourceObject() metav1.Object
 }
 
+var _ Route = &HttpRouteIR{}
+
 // this is 1:1 with httproute, and is a krt type
 // maybe move this to krtcollections package?
 type HttpRouteIR struct {
@@ -54,7 +56,6 @@ func (c HttpRouteIR) ResourceName() string {
 	return c.ObjectSource.ResourceName()
 }
 
-// get hostnames
 func (c *HttpRouteIR) GetHostnames() []string {
 	if c == nil {
 		return nil
@@ -99,21 +100,7 @@ func (c HttpRouteIR) rulesEqual(in HttpRouteIR) bool {
 			return false
 		}
 		for j, backend := range backendsa {
-			otherbackend := backendsb[j]
-			if backend.Backend == nil && otherbackend.Backend == nil {
-				continue
-			}
-			if backend.Backend != nil && otherbackend.Backend != nil {
-				if backend.Backend.ClusterName != otherbackend.Backend.ClusterName {
-					return false
-				}
-				if backend.Backend.Weight != otherbackend.Backend.Weight {
-					return false
-				}
-				if !backend.AttachedPolicies.Equals(otherbackend.AttachedPolicies) {
-					return false
-				}
-			} else {
+			if !backend.Equals(backendsb[j]) {
 				return false
 			}
 		}