fix(feature): uses server-side apply to reconile (opendatahub-io#1098)

Aligns reconcile strategy for Managed() features to rely on Server-side
apply rather than Update. Former apprach is used by deploy package when
reconciling kustomize resources. Later tries to overwrite the entire resource
rather than using patch which can lead to errors when trying to
overwrite immutable fields such as:

```
failed to update object test-namespace/knative-local-gateway: Service "knative-local-gateway" is invalid: [metadata.resourceVersion: Invalid value: "": must be specified for an update, spec.clusterIP: Invalid value: "": field is immutable]
```

Additional changes
- if "managed" annotation is present in one of the resources being part
  of the feature it will be respected
- reworked tests to be more self-descriptive. extracted funcs were
  adding more hops while reading the code before understanding what is
actually happening

(cherry picked from commit 9b68ea8)

pkg/feature/manifest.go

@@ -106,8 +106,11 @@ func markAsManaged(objs []*unstructured.Unstructured) {
 			objAnnotations = make(map[string]string)
 		}
 
-		objAnnotations[annotations.ManagedByODHOperator] = "true"
-		obj.SetAnnotations(objAnnotations)
+		// If resource already has an annotation, leave it as defined
+		if _, exists := objAnnotations[annotations.ManagedByODHOperator]; !exists {
+			objAnnotations[annotations.ManagedByODHOperator] = "true"
+			obj.SetAnnotations(objAnnotations)
+		}
 	}
 }
 
@@ -149,7 +152,7 @@ func CreateRawManifestFrom(fsys fs.FS, path string) *rawManifest {
 	return &rawManifest{
 		name:  basePath,
 		path:  path,
-		patch: strings.Contains(basePath, ".patch"),
+		patch: strings.Contains(basePath, ".patch."),
 		fsys:  fsys,
 	}
 }
@@ -170,7 +173,7 @@ func isTemplateManifest(path string) bool {
 }
 
 func convertToUnstructuredSlice(resources string) ([]*unstructured.Unstructured, error) {
-	splitter := regexp.MustCompile(YamlSeparator)
+	splitter := regexp.MustCompile(yamlResourceSeparator)
 	objectStrings := splitter.Split(resources, -1)
 	objs := make([]*unstructured.Unstructured, 0, len(objectStrings))
 	for _, str := range objectStrings {

pkg/feature/raw_resources.go

@@ -17,6 +17,7 @@ import (
 	"context"
 	"fmt"
 
+	k8serr "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	k8stypes "k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -26,55 +27,80 @@ import (
 )
 
 const (
-	YamlSeparator = "(?m)^---[ \t]*$"
+	yamlResourceSeparator = "(?m)^---[ \t]*$"
 )
 
 func applyResources(ctx context.Context, cli client.Client, objects []*unstructured.Unstructured, metaOptions ...cluster.MetaOptions) error {
-	for _, object := range objects {
+	for _, source := range objects {
+		target := source.DeepCopy()
+
+		name := source.GetName()
+		namespace := source.GetNamespace()
+
+		errGet := cli.Get(ctx, k8stypes.NamespacedName{Name: name, Namespace: namespace}, target)
+		if client.IgnoreNotFound(errGet) != nil {
+			return fmt.Errorf("failed to get resource %s/%s: %w", namespace, name, errGet)
+		}
+
 		for _, opt := range metaOptions {
-			if err := opt(object); err != nil {
+			if err := opt(target); err != nil {
 				return err
 			}
 		}
 
-		name := object.GetName()
-		namespace := object.GetNamespace()
+		if k8serr.IsNotFound(errGet) {
+			if errCreate := cli.Create(ctx, target); client.IgnoreAlreadyExists(errCreate) != nil {
+				return fmt.Errorf("failed to create source %s/%s: %w", namespace, name, errCreate)
+			}
 
-		err := cli.Get(ctx, k8stypes.NamespacedName{Name: name, Namespace: namespace}, object.DeepCopy())
-		if client.IgnoreNotFound(err) != nil {
-			return fmt.Errorf("failed to get object %s/%s: %w", namespace, name, err)
+			return nil
 		}
 
-		if err != nil {
-			// object does not exist and should be created
-			if createErr := cli.Create(ctx, object); client.IgnoreAlreadyExists(createErr) != nil {
-				return fmt.Errorf("failed to create object %s/%s: %w", namespace, name, createErr)
-			}
-		}
-		// object exists, check if it is managed
-		isManaged, isAnnotated := object.GetAnnotations()[annotations.ManagedByODHOperator]
+		isManaged, isAnnotated := target.GetAnnotations()[annotations.ManagedByODHOperator]
 		if isAnnotated && isManaged == "true" {
-			// update the object since we manage it
-			if updateErr := cli.Update(ctx, object); updateErr != nil {
-				return fmt.Errorf("failed to update object %s/%s: %w", namespace, name, updateErr)
+			// reconcile the target since we manage it
+			if errUpdate := patchUsingApplyStrategy(ctx, cli, source, target); errUpdate != nil {
+				return fmt.Errorf("failed to update source %s/%s: %w", namespace, name, errUpdate)
 			}
 		}
-		// object exists and is not manged, skip reconcile allowing users to tweak it
+		// source exists and is not manged, skip reconcile allowing users to tweak it
 	}
+
 	return nil
 }
 
 func patchResources(ctx context.Context, cli client.Client, patches []*unstructured.Unstructured) error {
 	for _, patch := range patches {
-		// Convert the individual resource patch to JSON
-		patchAsJSON, err := patch.MarshalJSON()
-		if err != nil {
-			return fmt.Errorf("error converting yaml to json: %w", err)
+		if errPatch := patchUsingMergeStrategy(ctx, cli, patch); errPatch != nil {
+			return errPatch
 		}
+	}
 
-		if err = cli.Patch(ctx, patch, client.RawPatch(k8stypes.MergePatchType, patchAsJSON)); err != nil {
-			return fmt.Errorf("failed patching resource: %w", err)
-		}
+	return nil
+}
+
+// patchUsingApplyStrategy applies a server-side apply patch to a Kubernetes resource.
+// It treats the provided source as the desired state of the resource and attempts to
+// reconcile the target resource to match this state. The function takes ownership of the
+// fields specified in the target and will ensure they match desired state.
+func patchUsingApplyStrategy(ctx context.Context, cli client.Client, source, target *unstructured.Unstructured) error {
+	data, errJSON := source.MarshalJSON()
+	if errJSON != nil {
+		return fmt.Errorf("error converting yaml to json: %w", errJSON)
+	}
+	return cli.Patch(ctx, target, client.RawPatch(k8stypes.ApplyPatchType, data), client.ForceOwnership, client.FieldOwner("rhods-operator"))
+}
+
+// patchUsingMergeStrategy merges the specified fields into the existing resources.
+// Fields included in the patch will overwrite existing fields, while fields not included will remain unchanged.
+func patchUsingMergeStrategy(ctx context.Context, cli client.Client, patch *unstructured.Unstructured) error {
+	data, errJSON := patch.MarshalJSON()
+	if errJSON != nil {
+		return fmt.Errorf("error converting yaml to json: %w", errJSON)
+	}
+
+	if errPatch := cli.Patch(ctx, patch, client.RawPatch(k8stypes.MergePatchType, data)); errPatch != nil {
+		return fmt.Errorf("failed patching resource: %w", errPatch)
 	}
 
 	return nil

tests/integration/features/fixtures/templates/local-gateway-svc.tmpl.yaml

@@ -1,6 +1,8 @@
 apiVersion: v1
 kind: Service
 metadata:
+  annotations:
+    test-annotation: "original-value"
   labels:
     experimental.istio.io/disable-gateway-port-translation: "true"
   name: knative-local-gateway

tests/integration/features/fixtures/templates/managed-svc.yaml

@@ -5,6 +5,7 @@ metadata:
   namespace: "test-namespace"
   annotations:
     opendatahub.io/managed: "true"
+    test-annotation: "original-value"
 spec:
   ports:
     - name: http2

tests/integration/features/fixtures/templates/unmanaged-svc.yaml

@@ -0,0 +1,17 @@
+apiVersion: v1
+kind: Service
+metadata:
+  name: unmanaged-svc
+  namespace: "test-namespace"
+  annotations:
+    opendatahub.io/managed: "false"
+    test-annotation: "original-value"
+spec:
+  ports:
+    - name: http2
+      port: 80
+      protocol: TCP
+      targetPort: 8081
+  selector:
+    knative: ingressgateway
+  type: ClusterIP