Confirm release belongs to HR before upgrading (#2123)

Confirm release belongs to HR before upgrading

integrations/helm/chartsync/chartsync.go

@@ -377,11 +377,18 @@ func (chs *ChartChangeSync) reconcileReleaseDef(fhr fluxv1beta1.HelmRelease) {
 		}
 		chs.setCondition(fhr, fluxv1beta1.HelmReleaseReleased, v1.ConditionTrue, ReasonSuccess, "helm install succeeded")
 		if err = status.UpdateReleaseRevision(chs.ifClient.FluxV1beta1().HelmReleases(fhr.Namespace), fhr, chartRevision); err != nil {
-			chs.logger.Log("warning", "could not update the release revision", "namespace", fhr.Namespace, "resource", fhr.Name, "err", err)
+			chs.logger.Log("warning", "could not update the release revision", "resource", fhr.ResourceID().String(), "err", err)
 		}
 		return
 	}
 
+	if !chs.release.OwnedByHelmRelease(rel, fhr) {
+		msg := fmt.Sprintf("release '%s' does not belong to HelmRelease", releaseName)
+		chs.setCondition(fhr, fluxv1beta1.HelmReleaseReleased, v1.ConditionFalse, ReasonUpgradeFailed, msg)
+		chs.logger.Log("warning", msg + ", this may be an indication that multiple HelmReleases with the same release name exist", "resource", fhr.ResourceID().String())
+		return
+	}
+
 	changed, err := chs.shouldUpgrade(chartPath, rel, fhr)
 	if err != nil {
 		chs.logger.Log("warning", "unable to determine if release has changed", "resource", fhr.ResourceID().String(), "err", err)

integrations/helm/release/release.go

@@ -267,6 +267,46 @@ func (r *Release) Delete(name string) error {
 	return nil
 }
 
+// OwnedByHelmRelease validates the release is managed by the given
+// HelmRelease, by looking for the resource ID in the antecedent
+// annotation. This validation is necessary because we can not
+// validate the uniqueness of a release name on the creation of a
+// HelmRelease, which would result in the operator attempting to
+// upgrade a release indefinitely when multiple HelmReleases with the
+// same release name exist.
+//
+// To be able to migrate existing releases to a HelmRelease, empty
+// (missing) annotations are handled as true / owned by.
+func (r *Release) OwnedByHelmRelease(release *hapi_release.Release, fhr flux_v1beta1.HelmRelease) bool {
+	objs := releaseManifestToUnstructured(release.Manifest, log.NewNopLogger())
+
+	escapedAnnotation := strings.ReplaceAll(fluxk8s.AntecedentAnnotation, ".", `\.`)
+	args := []string{"-o", "jsonpath={.metadata.annotations."+escapedAnnotation+"}", "get"}
+
+	for ns, res := range namespacedResourceMap(objs, release.Namespace) {
+		for _, r := range res {
+			a := append(args, "--namespace", ns, r)
+
+			ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+			defer cancel()
+
+			cmd := exec.CommandContext(ctx, "kubectl", a...)
+			out, err := cmd.Output()
+			if err != nil {
+				continue
+			}
+
+			v := strings.TrimSpace(string(out))
+			if v == "" {
+				return true
+			}
+			return v == fhr.ResourceID().String()
+		}
+	}
+
+	return false
+}
+
 // annotateResources annotates each of the resources created (or updated)
 // by the release so that we can spot them.
 func (r *Release) annotateResources(release *hapi_release.Release, fhr flux_v1beta1.HelmRelease) {
@@ -277,7 +317,9 @@ func (r *Release) annotateResources(release *hapi_release.Release, fhr flux_v1be
 		args = append(args, res...)
 		args = append(args, fluxk8s.AntecedentAnnotation+"="+fhrResourceID(fhr).String())
 
-		ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+		// The timeout is set to a high value as it may take some time
+		// to annotate large umbrella charts.
+		ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
 		defer cancel()
 
 		cmd := exec.CommandContext(ctx, "kubectl", args...)