Skip to content

Commit

Permalink
fix(envoy-filter): properly removes headers (opendatahub-io#1108)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bartoszmajsak authored Jul 10, 2024
1 parent 011c946 commit 022f456
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions components/kserve/servicemesh_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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"),
Expand Down
11 changes: 9 additions & 2 deletions pkg/feature/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down

0 comments on commit 022f456

Please sign in to comment.