Several problems when reconciling a deletion

[matheuscscp]

xref: fluxcd/pkg#959

When spec.serviceAccountName is specified, the controller confirms the existence of the ServiceAccount before attempting to impersonate it for reconciling the deletion:

if err = r.Client.Get(ctx, types.NamespacedName{
	Namespace: obj.GetNamespace(),
	Name:      serviceAccount,
}, &corev1.ServiceAccount{}); err != nil {
	if client.IgnoreNotFound(err) == nil {
		// Without a ServiceAccount reference, we cannot confirm
		// the ServiceAccount exists.
		ctrl.LoggerFrom(ctx).Error(err, "skipping Helm release uninstallation")
		return nil
	}

	conditions.MarkFalse(obj, meta.ReadyCondition, v2.UninstallFailedReason,
		"failed to confirm ServiceAccount '%s' can be used to uninstall release: %s", serviceAccount, err)
	return err
}

There are several problems in this code:

• When the ServiceAccount does not exist, the error message is vague, there's no explanation of why the uninstallation was skipped: skipping Helm release uninstallation.
• The observability behavior is inconsistent with kustomize-controller, where we both print an error log and send an error event (which seems like the right behavior).
• The error behavior is inconsistent with kustomize-controller. If kustomize-controller fails to confirm the existence of the ServiceAccount due to an error that is not not found, it reports the error log and event, but still allows the Kustomization object to be finalized and deleted from etcd. If helm-controller fails to confirm the existence of the ServiceAccount due to an error that is not not found, it blocks the HelmRelease object finalization and deletion from etcd (by triggering controller-runtime's retry with backoff). In this case I'm not sure which controller is doing the best option.
• Similar to Bug: runtime/client.(*Impersonator).CanImpersonate does not take kubeconfig into account pkg#959, a configured spec.kubeConfig is not taken into account when confirming the existence of the ServiceAccount, with the difference that kustomize-controller will still try to confirm the existence, but in the wrong cluster (the current cluster, and not the remote one). The helm-controller will not try to confirm, and will simply go ahead and try to reconcile the deletion.