[platform] add hr reconciler - #870

packages/system/keycloak/templates/ingress.yaml (1)

8-15: DRY up tenant-root lookup across templates.
This tenant-root lookup block is repeated in multiple charts. Consider extracting it into a helper function in _helpers.tpl (e.g., {{ include "cozystack.tenantRootHost" . }}) to reduce duplication and improve maintainability.

packages/extra/info/templates/kubeconfig.yaml (1)

15-22: Extract tenant-root lookup into a helper.
This snippet recurs across multiple templates. Move it into a function in _helpers.tpl (e.g., tenantRootHost) and call it in each chart to keep templates DRY.

packages/system/keycloak/templates/sts.yaml (1)

10-17: Consolidate repeated tenant-root lookup.
The same tenant-root lookup logic appears in multiple templates. Extract it into a shared helper in _helpers.tpl to improve maintainability and reduce duplication.

cmd/cozystack-controller/main.go (1)

193-199: Incorrect controller name in log message

setupLog.Error(err, "unable to create controller", "controller", "Workload") will mis-lead operators when the TenantHelmReconciler fails because the structured field still says "Workload".
Use the actual controller name to avoid confusion during troubleshooting.

-		setupLog.Error(err, "unable to create controller", "controller", "Workload")
+		setupLog.Error(err, "unable to create controller", "controller", "TenantHelm")

internal/controller/tenant_helm_reconciler.go (2)

67-70: Incorrect field in log: namespace is set to HelmRelease name

logger.Error(err, ... "namespace", hr.Name) passes the name instead of the namespace, making logs misleading.

-	logger.Error(err, "unable to list HelmReleases in namespace", "namespace", hr.Name)
+	logger.Error(err, "unable to list HelmReleases in namespace", "namespace", childNamespace)

---

127-152: Namespace update should use patch & incoming context

annotateTenantRootNs fetches and Updates the Namespace with a full object write.
Using Patch avoids clobbering concurrent updates and honours server-side apply semantics; also context.TODO() loses cancellation.

Consider:

-	if err := c.Get(context.TODO(), client.ObjectKey{Name: "tenant-root"}, &ns); err != nil {
+	if err := c.Get(ctx, client.ObjectKey{Name: "tenant-root"}, &ns); err != nil {
...
-	if err := c.Update(context.TODO(), &ns); err != nil {
+	if err := c.Patch(ctx, &ns, client.MergeFrom(original)); err != nil {

(Where ctx is passed down and original is a deep copy made right after Get.)

This reduces race risk with other controllers or kubectl edits.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

internal/controller/tenant_helm_reconciler.go (3)

45-50: ⚠️ Potential issue

hr.Status.History[0] still indexed before the length check (duplicate of earlier review)

The first access at line 45 precedes the len(hr.Status.History) guard on line 49, so the panic risk reported in the previous review remains.

Suggested quick fix (move the guard before the access and drop the redundant second check):

-	if hr.Status.History[0].Status != "deployed" {
-		return ctrl.Result{}, nil
-	}
-
-	if len(hr.Status.History) == 0 {
+	if len(hr.Status.History) == 0 {
 		logger.Info("no history in HelmRelease status", "name", hr.Name)
 		return ctrl.Result{}, nil
 	}
 
-	if hr.Status.History[0].Status != "deployed" {
+	if hr.Status.History[0].Status != "deployed" {
 		return ctrl.Result{}, nil
 	}

---

61-66: ⚠️ Potential issue

Possible nil-pointer dereference of hr.Spec.Values (duplicate of earlier review)

hr.Spec.Values is a pointer; if it’s nil, the dereference *hr.Spec.Values will panic.

-		err := annotateTenantRootNs(*hr.Spec.Values, r.Client)
+		if hr.Spec.Values == nil {
+			logger.Info("tenant-root HelmRelease has no .spec.values; skipping namespace annotation")
+		} else if err := annotateTenantRootNs(*hr.Spec.Values, r.Client); err != nil {
+			logger.Error(err, "cant annotate tenant-root ns")
+			return ctrl.Result{}, err
+		}

---

117-128: 🛠️ Refactor suggestion

getChildNamespace still duplicates tenant name for dedicated namespaces (duplicate of earlier review)

For a HelmRelease tenant-foo living in namespace tenant-foo, the function returns tenant-foo-foo, breaking look-ups.

-	default:
-		// 3) tenant in a dedicated namespace
-		return fmt.Sprintf("%s-%s", currentNamespace, tenantName)
+	default:
+		// 3) tenant in its own namespace – return namespace unchanged
+		return currentNamespace

internal/controller/tenant_helm_reconciler.go (2)

71-73: Incorrect namespace in error log

hr.Name is logged where childNamespace (or hr.Namespace) is intended, which can mislead troubleshooting.

-	logger.Error(err, "unable to list HelmReleases in namespace", "namespace", hr.Name)
+	logger.Error(err, "unable to list HelmReleases in namespace", "namespace", childNamespace)

---

131-156: Use caller context instead of context.TODO() and prefer JSON unmarshal

Minor but useful clean-ups:

1. context.TODO() discards cancellation/deadline from the Reconcile context.
2. Since values.Raw is already JSON, json.Unmarshal avoids YAML quirks.

-func annotateTenantRootNs(values apiextensionsv1.JSON, c client.Client) error {
+func annotateTenantRootNs(ctx context.Context, values apiextensionsv1.JSON, c client.Client) error {
@@
-	if err := yaml.Unmarshal(values.Raw, &data); err != nil {
+	if err := json.Unmarshal(values.Raw, &data); err != nil {
@@
-	if err := c.Get(context.TODO(), client.ObjectKey{Name: "tenant-root"}, &ns); err != nil {
+	if err := c.Get(ctx, client.ObjectKey{Name: "tenant-root"}, &ns); err != nil {
@@
-	if err := c.Update(context.TODO(), &ns); err != nil {
+	if err := c.Update(ctx, &ns); err != nil {

Caller adjustment:

-		err := annotateTenantRootNs(*hr.Spec.Values, r.Client)
+		err := annotateTenantRootNs(ctx, *hr.Spec.Values, r.Client)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro