Skip to content

Commit ce1f563

Browse files
mattia-eleuterimattia-eleuteri
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 25a6b56 commit ce1f563

2 files changed

Lines changed: 147 additions & 6 deletions

File tree

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

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -541,15 +541,30 @@ func (r *REST) Update(ctx context.Context, name string, objInfo rest.UpdatedObje
541541
return nil, false, fmt.Errorf("conversion error: %v", err)
542542
}
543543

544-
// Ensure ResourceVersion
544+
// The conversion above rebuilds the HelmRelease from the Application
545+
// alone, but parts of the HelmRelease metadata are owned by other
546+
// controllers and must survive the update. Fetch the current object
547+
// and carry them over before issuing the full-replace PUT:
548+
// - finalizers: helm-controller's finalizers.fluxcd.io guarantees
549+
// `helm uninstall` runs on deletion. Stripping it here lets a
550+
// subsequent delete remove the HelmRelease instantly, orphaning
551+
// every resource of the release.
552+
// - ownerReferences: garbage collection and lineage.
553+
// - labels/annotations outside the apps.cozystack.io- prefix: set
554+
// directly on the HelmRelease by Flux or other controllers.
555+
// Keys under the prefix are owned by this API: stale ones are dropped
556+
// so deleting a label or annotation on the Application propagates.
557+
cur := &helmv2.HelmRelease{}
558+
if err := r.c.Get(ctx, client.ObjectKey{Namespace: helmRelease.Namespace, Name: helmRelease.Name}, cur, &client.GetOptions{Raw: &metav1.GetOptions{}}); err != nil {
559+
return nil, false, fmt.Errorf("failed to fetch current HelmRelease: %w", err)
560+
}
545561
if helmRelease.ResourceVersion == "" {
546-
cur := &helmv2.HelmRelease{}
547-
err := r.c.Get(ctx, client.ObjectKey{Namespace: helmRelease.Namespace, Name: helmRelease.Name}, cur, &client.GetOptions{Raw: &metav1.GetOptions{}})
548-
if err != nil {
549-
return nil, false, fmt.Errorf("failed to fetch current HelmRelease: %w", err)
550-
}
551562
helmRelease.SetResourceVersion(cur.GetResourceVersion())
552563
}
564+
helmRelease.Finalizers = cur.Finalizers
565+
helmRelease.OwnerReferences = cur.OwnerReferences
566+
helmRelease.Labels = mergeMaps(omitPrefixedMap(cur.Labels, LabelPrefix), helmRelease.Labels)
567+
helmRelease.Annotations = mergeMaps(omitPrefixedMap(cur.Annotations, AnnotationPrefix), helmRelease.Annotations)
553568

554569
// Merge system labels (from config) directly
555570
helmRelease.Labels = mergeMaps(r.releaseConfig.Labels, helmRelease.Labels)
@@ -1161,6 +1176,24 @@ func filterPrefixedMap(original map[string]string, prefix string) map[string]str
11611176
return processed
11621177
}
11631178

1179+
// omitPrefixedMap returns the entries of a map whose keys do NOT carry the
1180+
// predefined prefix, keeping the keys as-is. It is the complement of
1181+
// filterPrefixedMap: prefixed keys are owned by this API and recomputed from
1182+
// the Application on every write, while the remaining keys belong to other
1183+
// controllers and must be preserved.
1184+
func omitPrefixedMap(original map[string]string, prefix string) map[string]string {
1185+
if original == nil {
1186+
return nil
1187+
}
1188+
processed := make(map[string]string)
1189+
for k, v := range original {
1190+
if !strings.HasPrefix(k, prefix) {
1191+
processed[k] = v
1192+
}
1193+
}
1194+
return processed
1195+
}
1196+
11641197
// ConvertHelmReleaseToApplication converts a HelmRelease to an Application.
11651198
func (r *REST) ConvertHelmReleaseToApplication(ctx context.Context, hr *helmv2.HelmRelease) (appsv1alpha1.Application, error) {
11661199
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)