From 13d833e6351c4270b2a6f1e9ed6c294b42bea18f Mon Sep 17 00:00:00 2001
From: Bartosz Majsak <bartosz.majsak@gmail.com>
Date: Thu, 4 Jul 2024 07:24:25 +0200
Subject: [PATCH] chore: reworks authorino istio injection (#1097)

Instead of performing patching of Authorino deployment as part of
`PostConditions` hook, it is now a `Feature` on its own.

As a result, we no longer need the `ApplyManifest` mehtod for the `Feature` struct.

This function was created solely to apply a single manifest as an `Action` and
was used only for this specific use case. With the dedicated feature, a deployment
patch can now be defined as a regular manifest source and included as part of the Apply phase.
---
 .../dscinitialization/servicemesh_setup.go    | 41 ++++++++++++-------
 pkg/feature/feature.go                        | 26 ------------
 2 files changed, 27 insertions(+), 40 deletions(-)

diff --git a/controllers/dscinitialization/servicemesh_setup.go b/controllers/dscinitialization/servicemesh_setup.go
index 49b24478be4..21a8df55ab5 100644
--- a/controllers/dscinitialization/servicemesh_setup.go
+++ b/controllers/dscinitialization/servicemesh_setup.go
@@ -177,7 +177,7 @@ func (r *DSCInitializationReconciler) authorizationFeatures(instance *dsciv1.DSC
 	return func(handler *feature.FeaturesHandler) error {
 		serviceMeshSpec := instance.Spec.ServiceMesh
 
-		extAuthzErr := feature.CreateFeature("mesh-control-plane-external-authz").
+		errExtAuthz := feature.CreateFeature("mesh-control-plane-external-authz").
 			For(handler).
 			ManifestsLocation(Templates.Location).
 			Manifests(
@@ -193,24 +193,37 @@ func (r *DSCInitializationReconciler) authorizationFeatures(instance *dsciv1.DSC
 			).
 			PostConditions(
 				feature.WaitForPodsToBeReady(serviceMeshSpec.ControlPlane.Namespace),
-				func(ctx context.Context, f *feature.Feature) error {
-					return feature.WaitForPodsToBeReady(handler.DSCInitializationSpec.ServiceMesh.Auth.Namespace)(ctx, f)
-				},
-				func(ctx context.Context, f *feature.Feature) error {
-					// We do not have the control over deployment resource creation.
-					// It is created by Authorino operator using Authorino CR
-					//
-					// To make it part of Service Mesh we have to patch it with injection
-					// enabled instead, otherwise it will not have proxy pod injected.
-					return f.ApplyManifest(ctx, path.Join(Templates.AuthorinoDir, "deployment.injection.patch.tmpl.yaml"))
-				},
 			).
 			OnDelete(
 				servicemesh.RemoveExtensionProvider,
 			).
 			Load()
-		if extAuthzErr != nil {
-			return extAuthzErr
+		if errExtAuthz != nil {
+			return errExtAuthz
+		}
+
+		// We do not have the control over deployment resource creation.
+		// It is created by Authorino operator using Authorino CR and labels are not propagated from Authorino CR to spec.template
+		// See https://issues.redhat.com/browse/RHOAIENG-5494
+		//
+		// To make it part of Service Mesh we have to patch it with injection
+		// enabled instead, otherwise it will not have proxy pod injected.
+		errAuthorinoInjectionPatch := feature.CreateFeature("enable-proxy-injection-in-authorino-deployment").
+			For(handler).
+			ManifestsLocation(Templates.Location).
+			Manifests(
+				path.Join(Templates.AuthorinoDir, "deployment.injection.patch.tmpl.yaml"),
+			).
+			PreConditions(
+				servicemesh.EnsureAuthNamespaceExists,
+				func(ctx context.Context, f *feature.Feature) error {
+					return feature.WaitForPodsToBeReady(handler.DSCInitializationSpec.ServiceMesh.Auth.Namespace)(ctx, f)
+				},
+			).
+			Load()
+
+		if errAuthorinoInjectionPatch != nil {
+			return errAuthorinoInjectionPatch
 		}
 
 		return nil
diff --git a/pkg/feature/feature.go b/pkg/feature/feature.go
index 835cbfc9dc9..353a7599191 100644
--- a/pkg/feature/feature.go
+++ b/pkg/feature/feature.go
@@ -7,7 +7,6 @@ import (
 
 	"github.com/go-logr/logr"
 	"github.com/hashicorp/go-multierror"
-	"github.com/pkg/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -175,31 +174,6 @@ func (f *Feature) addCleanup(cleanupFuncs ...Action) {
 	f.cleanups = append(f.cleanups, cleanupFuncs...)
 }
 
-func (f *Feature) ApplyManifest(ctx context.Context, path string) error {
-	m, err := loadManifestsFrom(f.fsys, path)
-	if err != nil {
-		return err
-	}
-	for i := range m {
-		var objs []*unstructured.Unstructured
-		manifest := m[i]
-		apply := f.createApplier(manifest)
-
-		if objs, err = manifest.Process(f.Spec); err != nil {
-			return errors.WithStack(err)
-		}
-
-		if f.Managed {
-			manifest.MarkAsManaged(objs)
-		}
-
-		if err = apply(ctx, objs); err != nil {
-			return errors.WithStack(err)
-		}
-	}
-	return nil
-}
-
 func (f *Feature) AsOwnerReference() metav1.OwnerReference {
 	return f.Tracker.ToOwnerReference()
 }