From 022f456edc8d6f00c6e2576e6d45f5124335fd18 Mon Sep 17 00:00:00 2001
From: Bartosz Majsak <bartosz.majsak@gmail.com>
Date: Wed, 10 Jul 2024 12:38:09 +0200
Subject: [PATCH] fix(envoy-filter): properly removes headers (#1108)

Current implementation incorrectly calls removal of a header which is
manifested in ingress gateway proxy logs:

```
envoy lua external/envoy/source/extensions/filters/http/lua/lua_filter.cc:930  script log: [string "function envoy_on_request(request_handle)..."]:10: bad argument #1 to 'remove' (N5Envoy10Extensions11HttpFilters3Lua16HeaderMapWrapperE expected, got string`
```

This fixes the Lua syntax and marks Service Mesh Features related to
KServe setup as `Managed()`. This ensures that updating the existing
filters will happen on operator upgrade or next reconcile.
---
 .../servicemesh/envoy-oauth-temp-fix.tmpl.yaml        |  2 +-
 components/kserve/servicemesh_setup.go                |  2 ++
 pkg/feature/builder.go                                | 11 +++++++++--
 .../fixtures/templates/local-gateway-svc.tmpl.yaml    |  1 -
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/components/kserve/resources/servicemesh/envoy-oauth-temp-fix.tmpl.yaml b/components/kserve/resources/servicemesh/envoy-oauth-temp-fix.tmpl.yaml
index b852d2f1e2e..9adb5b74ec9 100644
--- a/components/kserve/resources/servicemesh/envoy-oauth-temp-fix.tmpl.yaml
+++ b/components/kserve/resources/servicemesh/envoy-oauth-temp-fix.tmpl.yaml
@@ -75,6 +75,6 @@ spec:
               local xauth = headers:get("x-authorization")
               if xauth then
                 headers:replace("authorization", xauth)
-                headers.remove("x-authorization")
+                headers:remove("x-authorization")
               end
             end
diff --git a/components/kserve/servicemesh_setup.go b/components/kserve/servicemesh_setup.go
index 52f439bea29..7a98c7a9009 100644
--- a/components/kserve/servicemesh_setup.go
+++ b/components/kserve/servicemesh_setup.go
@@ -43,6 +43,7 @@ func (k *Kserve) defineServiceMeshFeatures(ctx context.Context, cli client.Clien
 		if authorinoInstalled {
 			kserveExtAuthzErr := feature.CreateFeature("kserve-external-authz").
 				For(handler).
+				Managed().
 				ManifestsLocation(Resources.Location).
 				Manifests(
 					path.Join(Resources.ServiceMeshDir, "activator-envoyfilter.tmpl.yaml"),
@@ -62,6 +63,7 @@ func (k *Kserve) defineServiceMeshFeatures(ctx context.Context, cli client.Clien
 
 		temporaryFixesErr := feature.CreateFeature("kserve-temporary-fixes").
 			For(handler).
+			Managed().
 			ManifestsLocation(Resources.Location).
 			Manifests(
 				path.Join(Resources.ServiceMeshDir, "grpc-envoyfilter-temp-fix.tmpl.yaml"),
diff --git a/pkg/feature/builder.go b/pkg/feature/builder.go
index 4119822cb44..eb628796c4d 100644
--- a/pkg/feature/builder.go
+++ b/pkg/feature/builder.go
@@ -105,8 +105,15 @@ func (fb *featureBuilder) TargetNamespace(targetNs string) *featureBuilder {
 	return fb
 }
 
-// Managed marks the feature as managed by the operator.  This effectively marks all resources which are part of this feature
-// as those that should be updated on operator reconcile.
+// Managed marks the feature as managed by the operator.
+//
+// This effectively makes all resources which are part of this feature as reconciled to the desired state
+// defined by provided manifests.
+//
+// NOTE: Although the actual instance of the resource in the cluster might have this configuration altered,
+// we intentionally do not read the management configuration from there due to the lack of clear requirements.
+// This means that management state is defined by the Feature resources provided by the operator
+// and not by the actual state of the resource.
 func (fb *featureBuilder) Managed() *featureBuilder {
 	fb.managed = true
 
diff --git a/tests/integration/features/fixtures/templates/local-gateway-svc.tmpl.yaml b/tests/integration/features/fixtures/templates/local-gateway-svc.tmpl.yaml
index 005db64383c..d0113f4875d 100644
--- a/tests/integration/features/fixtures/templates/local-gateway-svc.tmpl.yaml
+++ b/tests/integration/features/fixtures/templates/local-gateway-svc.tmpl.yaml
@@ -3,7 +3,6 @@ kind: Service
 metadata:
   annotations:
     test: "original-value"
-  labels:
     experimental.istio.io/disable-gateway-port-translation: "true"
   name: knative-local-gateway
   namespace: {{ .ControlPlane.Namespace }}