From 1eada22d5d86cd05b8b8b664b903910c6d05ff04 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniele=20Trifir=C3=B2?= <dtrifiro@redhat.com>
Date: Thu, 21 Mar 2024 14:49:03 +0100
Subject: [PATCH] partial revert of temporary grpc-only EnvoyFilter

This partially reverts commit 48e0b8bad9bbae951731a36c6e17bf26981fed7b
as well as including a mechanism to deleted the then-introduced feature
by removing the corresponding FeatureTracker.

Related: RHOAIENG-165, #888
---
 components/kserve/servicemesh_setup.go        | 12 ------
 main.go                                       | 11 +++++
 .../kserve/grpc-envoyfilter-temp-fix.tmpl     | 37 -----------------
 pkg/upgrade/upgrade.go                        | 40 +++++++++++++++++++
 4 files changed, 51 insertions(+), 49 deletions(-)
 delete mode 100644 pkg/feature/templates/servicemesh/kserve/grpc-envoyfilter-temp-fix.tmpl

diff --git a/components/kserve/servicemesh_setup.go b/components/kserve/servicemesh_setup.go
index c8217cb8504..0d523323fed 100644
--- a/components/kserve/servicemesh_setup.go
+++ b/components/kserve/servicemesh_setup.go
@@ -44,18 +44,6 @@ func (k *Kserve) defineServiceMeshFeatures() feature.FeaturesProvider {
 			return kserveExtAuthzErr
 		}
 
-		temporaryFixesErr := feature.CreateFeature("kserve-temporary-fixes").
-			For(handler).
-			Manifests(
-				path.Join(feature.KServeDir, "grpc-envoyfilter-temp-fix.tmpl"),
-			).
-			WithData(servicemesh.ClusterDetails).
-			Load()
-
-		if temporaryFixesErr != nil {
-			return temporaryFixesErr
-		}
-
 		return nil
 	}
 }
diff --git a/main.go b/main.go
index 4c1b9e32746..e56ff0cf2ff 100644
--- a/main.go
+++ b/main.go
@@ -237,6 +237,17 @@ func main() { //nolint:funlen
 		setupLog.Error(err, "error remove deprecated resources from previous version")
 	}
 
+	var cleanDeprecatedFeatures manager.RunnableFunc = func(ctx context.Context) error {
+		if err = upgrade.CleanupDeprecatedFeatures(ctx, setupClient, platform); err != nil {
+			setupLog.Error(err, "unable to clean deprecated features")
+		}
+		return err
+	}
+	err = mgr.Add(cleanDeprecatedFeatures)
+	if err != nil {
+		setupLog.Error(err, "unable to cleanup deprecated features")
+	}
+
 	if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
 		setupLog.Error(err, "unable to set up health check")
 		os.Exit(1)
diff --git a/pkg/feature/templates/servicemesh/kserve/grpc-envoyfilter-temp-fix.tmpl b/pkg/feature/templates/servicemesh/kserve/grpc-envoyfilter-temp-fix.tmpl
deleted file mode 100644
index d16a0220482..00000000000
--- a/pkg/feature/templates/servicemesh/kserve/grpc-envoyfilter-temp-fix.tmpl
+++ /dev/null
@@ -1,37 +0,0 @@
-# Temporary workaround for https://issues.redhat.com/browse/RHOAIENG-165
-apiVersion: networking.istio.io/v1alpha3
-kind: EnvoyFilter
-metadata:
-  name: grpc-only
-  namespace: {{ .ControlPlane.Namespace }}
-  labels:
-    app.opendatahub.io/kserve: "true"
-    app.kubernetes.io/part-of: kserve
-    opendatahub.io/related-to: RHOAIENG-165
-spec:
-  priority: 20
-  workloadSelector:
-    labels:
-      protocol: "grpconly"
-  configPatches:
-    - applyTo: HTTP_FILTER
-      match:
-        context: SIDECAR_INBOUND
-        listener:
-          filterChain:
-            filter:
-              name: envoy.filters.network.http_connection_manager
-      patch:
-        operation: INSERT_BEFORE
-        value:
-          name: envoy.filters.http.lua
-          typed_config:
-            "@type": type.googleapis.com/envoy.extensions.filters.http.lua.v3.Lua
-            inlineCode: |
-              function envoy_on_request(request_handle)
-                local header_value = request_handle:headers():get("Content-Type")
-                local pattern = "^application/grpc"
-                if header_value:find(pattern) == nil then
-                  request_handle:respond({[":status"] = "400"}, "Invalid request, this endpoint only serves grpc.")
-                end
-              end
diff --git a/pkg/upgrade/upgrade.go b/pkg/upgrade/upgrade.go
index feb9664512d..29ccf12941a 100644
--- a/pkg/upgrade/upgrade.go
+++ b/pkg/upgrade/upgrade.go
@@ -351,6 +351,46 @@ func UpdateFromLegacyVersion(cli client.Client, platform deploy.Platform, appNS
 	return nil
 }
 
+// CleanupDeprecatedFeatures deletes deprecated features by removing the corresponding FeatureTrackers
+func CleanupDeprecatedFeatures(ctx context.Context, cli client.Client, platform deploy.Platform) error {
+	if platform != deploy.ManagedRhods {
+		return nil
+	}
+
+	crd := &apiextv1.CustomResourceDefinition{}
+	if err := cli.Get(ctx, client.ObjectKey{Name: "featuretrackers.features.opendatahub.io"}, crd); err != nil {
+		return err
+	}
+
+	type ResourceToDelete struct {
+		InstanceName     string
+		GroupVersionKind schema.GroupVersionKind
+	}
+
+	toDelete := []string{
+		"opendatahub-kserve-temporary-fixes", // Workaround for RHOAIENG-165
+	}
+
+	var multiErr *multierror.Error
+	for _, name := range toDelete {
+		obj := &unstructured.Unstructured{}
+		obj.SetGroupVersionKind(schema.GroupVersionKind{Kind: "FeatureTracker", Group: "features.opendatahub.io", Version: "v1"})
+
+		if err := cli.Get(ctx, client.ObjectKey{Name: name}, obj); err != nil {
+			multiErr = multierror.Append(multiErr, client.IgnoreNotFound(err))
+			continue
+		}
+
+		if err := cli.Delete(ctx, obj); err != nil {
+			msg := fmt.Sprintf("error deleting FeatureTracker instance %s: %s", name, err)
+			fmt.Printf(msg)
+			multiErr = multierror.Append(multiErr, fmt.Errorf(msg))
+		}
+	}
+
+	return multiErr.ErrorOrNil()
+}
+
 // TODO: remove function once we have a generic solution across all components.
 func CleanupExistingResource(ctx context.Context, cli client.Client, platform deploy.Platform, dscApplicationsNamespace, dscMonitoringNamespace string) error {
 	var multiErr *multierror.Error