Skip to content

Commit a18cbc5

Browse files
mattia-eleuterilexfrei
mattia-eleuteri
authored and
lexfrei
committed
fix(api): preserve foreign HelmRelease metadata on Application update
Update rebuilds the HelmRelease from the Application and issues a full-replace PUT, which stripped metadata owned by other controllers: finalizers, ownerReferences, and any labels or annotations outside the apps.cozystack.io- prefix. Losing helm-controller's finalizers.fluxcd.io finalizer is the harmful case: a delete landing before helm-controller re-adds it removes the HelmRelease without running helm uninstall, orphaning every resource of the release (VMs, databases, PVCs). A label patch immediately followed by a delete - a common client pattern - hits that window almost every time. Fetch the current HelmRelease before the PUT and carry over its finalizers, ownerReferences, and non-prefixed labels and annotations. Keys under the apps.cozystack.io- prefix remain owned by the API and are recomputed from the Application, so label/annotation deletions still propagate. Signed-off-by: mattia-eleuteri <[email protected]>
1 parent d9ed566 commit a18cbc5

2 files changed

Lines changed: 143 additions & 3 deletions

File tree

‎pkg/registry/apps/application/rest.go‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -575,16 +575,30 @@ func (r *REST) Update(ctx context.Context, name string, objInfo rest.UpdatedObje
575575
return nil, false, fmt.Errorf("conversion error: %v", err)
576576
}
577577

578-
// Fetch the live HelmRelease: it backs the ResourceVersion when the
579-
// converted object carries none, and runtime-managed labels and the
580-
// suspension are carried over from it below.
578+
// The conversion above rebuilds the HelmRelease from the Application
579+
// alone, but parts of the HelmRelease metadata are owned by other
580+
// controllers and must survive the update. Fetch the current object
581+
// and carry them over before issuing the full-replace PUT:
582+
// - finalizers: helm-controller's finalizers.fluxcd.io guarantees
583+
// `helm uninstall` runs on deletion. Stripping it here lets a
584+
// subsequent delete remove the HelmRelease instantly, orphaning
585+
// every resource of the release.
586+
// - ownerReferences: garbage collection and lineage.
587+
// - labels/annotations outside the apps.cozystack.io- prefix: set
588+
// directly on the HelmRelease by Flux or other controllers.
589+
// Keys under the prefix are owned by this API: stale ones are dropped
590+
// so deleting a label or annotation on the Application propagates.
581591
cur := &helmv2.HelmRelease{}
582592
if err := r.c.Get(ctx, client.ObjectKey{Namespace: helmRelease.Namespace, Name: helmRelease.Name}, cur, &client.GetOptions{Raw: &metav1.GetOptions{}}); err != nil {
583593
return nil, false, registry.WrapPreservingStatus("failed to fetch current HelmRelease", err, r.gvr.GroupResource(), name)
584594
}
585595
if helmRelease.ResourceVersion == "" {
586596
helmRelease.SetResourceVersion(cur.GetResourceVersion())
587597
}
598+
helmRelease.Finalizers = cur.Finalizers
599+
helmRelease.OwnerReferences = cur.OwnerReferences
600+
helmRelease.Labels = mergeMaps(omitPrefixedMap(cur.Labels, LabelPrefix), helmRelease.Labels)
601+
helmRelease.Annotations = mergeMaps(omitPrefixedMap(cur.Annotations, AnnotationPrefix), helmRelease.Annotations)
588602

589603
// Merge system labels (from config) directly
590604
helmRelease.Labels = mergeMaps(r.releaseConfig.Labels, helmRelease.Labels)
@@ -1232,6 +1246,24 @@ func filterPrefixedMap(original map[string]string, prefix string) map[string]str
12321246
return processed
12331247
}
12341248

1249+
// omitPrefixedMap returns the entries of a map whose keys do NOT carry the
1250+
// predefined prefix, keeping the keys as-is. It is the complement of
1251+
// filterPrefixedMap: prefixed keys are owned by this API and recomputed from
1252+
// the Application on every write, while the remaining keys belong to other
1253+
// controllers and must be preserved.
1254+
func omitPrefixedMap(original map[string]string, prefix string) map[string]string {
1255+
if original == nil {
1256+
return nil
1257+
}
1258+
processed := make(map[string]string)
1259+
for k, v := range original {
1260+
if !strings.HasPrefix(k, prefix) {
1261+
processed[k] = v
1262+
}
1263+
}
1264+
return processed
1265+
}
1266+
12351267
// ConvertHelmReleaseToApplication converts a HelmRelease to an Application.
12361268
func (r *REST) ConvertHelmReleaseToApplication(ctx context.Context, hr *helmv2.HelmRelease) (appsv1alpha1.Application, error) {
12371269
return r.ConvertHelmReleaseToApplicationWithMonitor(ctx, hr, nil)

‎pkg/registry/apps/application/rest_update_metadata_test.go‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
package application
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
helmv2 "github.com/fluxcd/helm-controller/api/v2"
8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
"k8s.io/apimachinery/pkg/types"
10+
"k8s.io/apiserver/pkg/endpoints/request"
11+
"sigs.k8s.io/controller-runtime/pkg/client"
12+
13+
appsv1alpha1 "github.com/cozystack/cozystack/pkg/apis/apps/v1alpha1"
14+
)
15+
16+
// TestUpdate_PreservesForeignHelmReleaseMetadata pins the metadata
17+
// carry-over contract on the Update path. Update rebuilds the
18+
// HelmRelease from the Application and issues a full-replace PUT, so
19+
// without an explicit carry-over every update strips metadata owned by
20+
// other controllers. The most harmful instance is helm-controller's
21+
// finalizers.fluxcd.io finalizer: once stripped, a delete that lands
22+
// before helm-controller re-adds it removes the HelmRelease without
23+
// running `helm uninstall`, orphaning every resource of the release
24+
// (VMs, databases, PVCs). A label patch immediately followed by a
25+
// delete — a common client pattern — hits that window almost every
26+
// time.
27+
func TestUpdate_PreservesForeignHelmReleaseMetadata(t *testing.T) {
28+
existing := &helmv2.HelmRelease{
29+
ObjectMeta: metav1.ObjectMeta{
30+
Name: "postgresql-mydb",
31+
Namespace: "default",
32+
Finalizers: []string{"finalizers.fluxcd.io"},
33+
OwnerReferences: []metav1.OwnerReference{{
34+
APIVersion: "apps.cozystack.io/v1alpha1",
35+
Kind: "Tenant",
36+
Name: "root",
37+
UID: types.UID("owner-uid"),
38+
}},
39+
Labels: map[string]string{
40+
ApplicationKindLabel: "PostgreSQL",
41+
ApplicationGroupLabel: "apps.cozystack.io",
42+
ApplicationNameLabel: "mydb",
43+
"kustomize.toolkit.fluxcd.io/name": "core",
44+
LabelPrefix + "stale": "removed-by-this-update",
45+
},
46+
Annotations: map[string]string{
47+
"reconcile.fluxcd.io/requestedAt": "2026-06-12T00:00:00Z",
48+
AnnotationPrefix + "stale": "removed-by-this-update",
49+
},
50+
},
51+
}
52+
r := newTestRESTWithSchemes(existing)
53+
54+
app := &appsv1alpha1.Application{
55+
TypeMeta: metav1.TypeMeta{
56+
APIVersion: "apps.cozystack.io/v1alpha1",
57+
Kind: "PostgreSQL",
58+
},
59+
ObjectMeta: metav1.ObjectMeta{
60+
Name: "mydb",
61+
Namespace: "default",
62+
Labels: map[string]string{"fresh": "yes"},
63+
},
64+
}
65+
66+
ctx := request.WithNamespace(context.Background(), "default")
67+
if _, _, err := r.Update(
68+
ctx,
69+
"mydb",
70+
newDefaultUpdatedObjectInfo(app),
71+
nil,
72+
nil,
73+
false,
74+
&metav1.UpdateOptions{},
75+
); err != nil {
76+
t.Fatalf("unexpected error: %v", err)
77+
}
78+
79+
got := &helmv2.HelmRelease{}
80+
if err := r.c.Get(ctx, client.ObjectKey{Namespace: "default", Name: "postgresql-mydb"}, got); err != nil {
81+
t.Fatalf("failed to fetch updated HelmRelease: %v", err)
82+
}
83+
84+
if len(got.Finalizers) != 1 || got.Finalizers[0] != "finalizers.fluxcd.io" {
85+
t.Errorf("expected finalizers.fluxcd.io to be preserved, got %v", got.Finalizers)
86+
}
87+
if len(got.OwnerReferences) != 1 || got.OwnerReferences[0].UID != types.UID("owner-uid") {
88+
t.Errorf("expected ownerReferences to be preserved, got %v", got.OwnerReferences)
89+
}
90+
if got.Labels["kustomize.toolkit.fluxcd.io/name"] != "core" {
91+
t.Errorf("expected foreign label to be preserved, got labels %v", got.Labels)
92+
}
93+
if got.Annotations["reconcile.fluxcd.io/requestedAt"] != "2026-06-12T00:00:00Z" {
94+
t.Errorf("expected foreign annotation to be preserved, got annotations %v", got.Annotations)
95+
}
96+
if got.Labels[LabelPrefix+"fresh"] != "yes" {
97+
t.Errorf("expected new prefixed user label to be set, got labels %v", got.Labels)
98+
}
99+
if _, ok := got.Labels[LabelPrefix+"stale"]; ok {
100+
t.Errorf("expected stale prefixed label to be dropped, got labels %v", got.Labels)
101+
}
102+
if _, ok := got.Annotations[AnnotationPrefix+"stale"]; ok {
103+
t.Errorf("expected stale prefixed annotation to be dropped, got annotations %v", got.Annotations)
104+
}
105+
if got.Labels[ApplicationNameLabel] != "mydb" {
106+
t.Errorf("expected application metadata labels to remain, got labels %v", got.Labels)
107+
}
108+
}

0 commit comments

Comments
 (0)