pkg/controllers/externalsecret/informer_manager.go (1)

250-257: ⚠️ Potential issue | 🟠 Major

Match RemoveInformer object type to GetInformer: use Unstructured consistently.

The informer is created via GetInformer(ctx, &unstructured.Unstructured{}) at line 119, but removed via RemoveInformer(ctx, &metav1.PartialObjectMetadata{}) at line 254. Controller-runtime's cache implementation routes removals based on the object type, so RemoveInformer will look for the informer in the PartialObjectMetadata map rather than the Unstructured map where it was registered. This causes the removal to fail silently, leaking the informer resource.

Proposed fix

 	if len(entry.externalSecrets) == 0 {
-		partial := &metav1.PartialObjectMetadata{}
-		partial.SetGroupVersionKind(gvk)
+		obj := &unstructured.Unstructured{}
+		obj.SetGroupVersionKind(gvk)
 
-		if err := m.cache.RemoveInformer(ctx, partial); err != nil {
+		if err := m.cache.RemoveInformer(ctx, obj); err != nil {
 			m.log.Error(err, "failed to remove informer, will clean up tracking anyway",
 				"gvk", key)
 		}

pkg/controllers/externalsecret/informer_manager.go (1)

250-257: ⚠️ Potential issue | 🔴 Critical

Use Unstructured instead of PartialObjectMetadata when removing informers — the current mismatch causes silent cleanup failure and resource leaks.

The informer is registered in EnsureInformer with &unstructured.Unstructured{} (line 117–119), but ReleaseInformer attempts removal with &metav1.PartialObjectMetadata{} (line 251). controller-runtime's cache maintains separate internal maps for each object "flavor" (structured, unstructured, metadata). Since RemoveInformer only removes from the map corresponding to the object type passed, removal with PartialObjectMetadata silently succeeds without actually stopping the unstructured informer—the tracking entry is deleted, but the underlying informer keeps running and consuming resources.

Fix

 	if len(entry.externalSecrets) == 0 {
-		partial := &metav1.PartialObjectMetadata{}
-		partial.SetGroupVersionKind(gvk)
-
-		if err := m.cache.RemoveInformer(ctx, partial); err != nil {
+		obj := &unstructured.Unstructured{}
+		obj.SetGroupVersionKind(gvk)
+
+		if err := m.cache.RemoveInformer(ctx, obj); err != nil {
 			m.log.Error(err, "failed to remove informer, will clean up tracking anyway",
 				"gvk", key)
 		}

pkg/controllers/externalsecret/externalsecret_controller.go (1)

658-673: ⚠️ Potential issue | 🔴 Critical

Nil pointer dereference in isGenericTargetValid when target resource is deleted externally.

When getGenericResource returns (nil, notFoundErr) for a missing target (line 663), the isGenericTargetValid function is called with existing=nil at line 670. The function immediately dereferences existingTarget at GetUID() without a null check, causing a panic.

This is reachable: after a controller restart where the ExternalSecret has a recent sync (shouldRefresh → false) but the target resource has been deleted externally.

🔧 Fix — add nil guard before calling `isGenericTargetValid`

-	if !shouldRefresh(externalSecret) && isGenericTargetValid(existing, externalSecret) {
+	if !shouldRefresh(externalSecret) && existing != nil && isGenericTargetValid(existing, externalSecret) {
 		log.V(1).Info("skipping refresh of generic target")
 		return r.getRequeueResult(externalSecret), nil
 	}

Alternatively, add the nil guard inside isGenericTargetValid before calling GetUID().

pkg/controllers/externalsecret/externalsecret_controller_manifest_test.go (1)

284-337: ⚠️ Potential issue | 🟡 Minor

Add test coverage for isGenericTargetValid and genericTargetContentHash error paths.

The functions isGenericTargetValid (line 1188) and genericTargetContentHash (line 1215) in externalsecret_controller.go lack test coverage. Specifically:

• genericTargetContentHash returns an error when an object has neither spec nor data fields, but this error path is not tested.
• isGenericTargetValid does not validate null pointers and calls genericTargetContentHash, but neither function is tested in this file.

Add tests for:

• genericTargetContentHash with objects containing spec, data, and neither (expecting an error).
• isGenericTargetValid with nil/empty targets, valid targets, and mismatched hashes.

pkg/controllers/externalsecret/externalsecret_controller_manifest.go (1)

261-265: ⚠️ Potential issue | 🟡 Minor

Unchecked type assertion can panic.

If obj.Object["spec"] exists but is not map[string]any (e.g., set to a different type by a prior code path or an unusual existing object), line 264 will panic.

Proposed fix — use a checked assertion

-	if obj.Object["spec"] == nil {
-		obj.Object["spec"] = make(map[string]any)
-	}
-	spec := obj.Object["spec"].(map[string]any)
-	spec["data"] = data
+	if obj.Object["spec"] == nil {
+		obj.Object["spec"] = make(map[string]any)
+	}
+	spec, ok := obj.Object["spec"].(map[string]any)
+	if !ok {
+		obj.Object["spec"] = map[string]any{"data": data}
+		return obj
+	}
+	spec["data"] = data