Fix: Add owner reference to existing resources when initially missing

Fixes #8718

ReconcileResource now checks and updates owner references independently
of the NeedsUpdate function. Previously, if a resource existed without
an owner reference and NeedsUpdate returned false (because data, labels,
and annotations were unchanged), the owner reference would never be added.

Changes:
Added owner reference check before the NeedsUpdate block in ReconcileResource
Owner references are now added to existing resources even when other fields don't require updates
Added test case to verify owner reference is added when NeedsUpdate returns false

The fix ensures proper resource ownership management and enables garbage collection for resources that were created before owner references were implemented.

Author: ArsalanAnwer0

pkg/controller/common/reconciler/reconciler.go

@@ -144,7 +144,49 @@ func ReconcileResource(params Params) error {
 		log.Info("Deleted resource successfully")
 		return create()
 	}
+	// Check if owner reference needs to be added/updated independently of NeedsUpdate
+	if params.Owner != nil {
+		reconciledMeta, err := meta.Accessor(params.Reconciled)
+		if err != nil {
+			return err
+		}
+		expectedMeta, err := meta.Accessor(params.Expected)
+		if err != nil {
+			return err
+		}
+		expectedOwners := expectedMeta.GetOwnerReferences()
 
+		// Check if we need to update the owner reference
+		needsOwnerUpdate := false
+		if len(expectedOwners) > 0 {
+			reconciledOwners := reconciledMeta.GetOwnerReferences()
+			// Check if controller reference is missing or different
+			controllerRefIndex := -1
+			for i, owner := range reconciledOwners {
+				if owner.Controller != nil && *owner.Controller {
+					controllerRefIndex = i
+					break
+				}
+			}
+
+			if controllerRefIndex == -1 {
+				// No controller reference exists
+				needsOwnerUpdate = true
+			} else if reconciledOwners[controllerRefIndex].UID != expectedOwners[0].UID {
+				// Controller reference exists but points to a different owner
+				needsOwnerUpdate = true
+			}
+		}
+
+		if needsOwnerUpdate {
+			log.Info("Updating owner reference")
+			k8s.OverrideControllerReference(reconciledMeta, expectedOwners[0])
+			if err := params.Client.Update(params.Context, params.Reconciled); err != nil {
+				return err
+			}
+			log.Info("Updated owner reference successfully", resourceVersionKey, params.Reconciled.GetResourceVersion())
+		}
+	}
 	//nolint:nestif
 	// Update if needed
 	if params.NeedsUpdate() {

pkg/controller/common/reconciler/reconciler_test.go

@@ -258,6 +258,44 @@ func TestReconcileResource(t *testing.T) {
 				require.Equal(t, "newOwner", serverState.OwnerReferences[0].Name)
 			},
 		},
+		{
+			name: "Add owner reference to existing resource without owner when NeedsUpdate returns false",
+			args: func() args {
+				return args{
+					Expected:   obj.DeepCopy(),
+					Reconciled: &corev1.Secret{},
+					Owner: &appsv1.Deployment{
+						ObjectMeta: metav1.ObjectMeta{
+							Namespace: objectKey.Namespace,
+							Name:      "test-owner",
+							UID:       "test-uid",
+						},
+					},
+					NeedsUpdate: func() bool {
+						return false
+					},
+					UpdateReconciled: noopModifier,
+				}
+			},
+			initialObjects: []client.Object{
+				&corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: objectKey.Namespace,
+						Name:      objectKey.Name,
+					},
+				},
+			},
+			argAssertion: func(args args) {
+				accessor, err := meta.Accessor(args.Reconciled)
+				require.NoError(t, err)
+				require.Len(t, accessor.GetOwnerReferences(), 1, "owner reference should be added")
+				require.Equal(t, "test-owner", accessor.GetOwnerReferences()[0].Name)
+			},
+			serverStateAssertion: func(serverState corev1.Secret) {
+				require.Len(t, serverState.OwnerReferences, 1, "owner reference should be added on server")
+				require.Equal(t, "test-owner", serverState.OwnerReferences[0].Name)
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {