diff --git a/.changes/unreleased/charts-redpanda-Fixed-20260713-094500.yaml b/.changes/unreleased/charts-redpanda-Fixed-20260713-094500.yaml new file mode 100644 index 000000000..99513f372 --- /dev/null +++ b/.changes/unreleased/charts-redpanda-Fixed-20260713-094500.yaml @@ -0,0 +1,15 @@ +project: charts/redpanda +kind: Fixed +body: | + `helm upgrade` no longer fails under Helm 4's default server-side apply when + SASL is enabled. To keep the generated bootstrap-user password stable, the + chart looks up the existing `*-bootstrap-user` Secret on upgrade and + re-renders it — previously verbatim, including the server-set + `metadata.managedFields`, which the API server rejects with + `metadata.managedFields must be nil` when Helm 4 applies manifests + server-side. The rejected apply also polluted release history: manifests + stored by any successful client-side upgrade carried `managedFields`, so + later server-side rollbacks failed the same way. The lookup now strips + `managedFields`, `resourceVersion`, and `uid` before the Secret is + re-rendered. +time: 2026-07-13T09:45:00.000000-07:00 diff --git a/acceptance/features/helm-chart.feature b/acceptance/features/helm-chart.feature index f4da206f0..9e90c8ba2 100644 --- a/acceptance/features/helm-chart.feature +++ b/acceptance/features/helm-chart.feature @@ -37,3 +37,55 @@ Feature: Redpanda Helm Chart 3 active 2 - ``` + + Scenario: Bootstrap user Secret survives upgrades without server-set metadata + Given I helm install "sasl-upgrade" "../charts/redpanda/chart" with values: + ```yaml + fullnameOverride: saslupgrade + + statefulset: + replicas: 1 + sideCars: + image: + tag: dev + repository: localhost/redpanda-operator + + external: + enabled: false + + console: + enabled: false + + auth: + sasl: + enabled: true + users: + - name: admin + password: sasl-password + ``` + When I helm upgrade "sasl-upgrade" "../charts/redpanda/chart" with values: + ```yaml + fullnameOverride: saslupgrade + + statefulset: + replicas: 1 + sideCars: + image: + tag: dev + repository: localhost/redpanda-operator + + external: + enabled: false + + console: + enabled: false + + auth: + sasl: + enabled: true + users: + - name: admin + password: sasl-password + ``` + Then the stored manifest for release "sasl-upgrade" re-renders Secret "saslupgrade-bootstrap-user" without server-set metadata + And the stored manifest for release "sasl-upgrade" re-renders Secret "saslupgrade-bootstrap-user" with the password in use diff --git a/acceptance/steps/helm.go b/acceptance/steps/helm.go index 3ee7d7a21..2c66973fe 100644 --- a/acceptance/steps/helm.go +++ b/acceptance/steps/helm.go @@ -10,14 +10,20 @@ package steps import ( + "bytes" + "compress/gzip" "context" + "encoding/base64" + "encoding/json" "fmt" + "io" "strings" "github.com/cucumber/godog" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/yaml" framework "github.com/redpanda-data/redpanda-operator/harpoon" @@ -61,3 +67,65 @@ func iDeleteHelmReleaseSecret(ctx context.Context, t framework.TestingT, helmRel }, })) } + +// helmStoredManifestSecret returns the named Secret as re-rendered into the +// stored manifest of the release's currently deployed revision. +func helmStoredManifestSecret(ctx context.Context, t framework.TestingT, release, secretName string) *corev1.Secret { + var storageSecrets corev1.SecretList + require.NoError(t, t.List(ctx, &storageSecrets, client.InNamespace(t.Namespace()), client.MatchingLabels{ + "owner": "helm", + "name": release, + "status": "deployed", + })) + require.Lenf(t, storageSecrets.Items, 1, "expected exactly one deployed release storage secret for %q", release) + + // Helm stores each revision as base64(gzip(json)) under the "release" key. + payload, err := base64.StdEncoding.DecodeString(string(storageSecrets.Items[0].Data["release"])) + require.NoError(t, err) + reader, err := gzip.NewReader(bytes.NewReader(payload)) + require.NoError(t, err) + decoded, err := io.ReadAll(reader) + require.NoError(t, err) + + var stored struct { + Manifest string `json:"manifest"` + } + require.NoError(t, json.Unmarshal(decoded, &stored)) + + for _, doc := range strings.Split(stored.Manifest, "\n---") { + var secret corev1.Secret + if err := yaml.Unmarshal([]byte(doc), &secret); err != nil { + continue + } + if secret.Kind == "Secret" && secret.Name == secretName { + return &secret + } + } + require.Failf(t, "secret not found", "Secret %q is not part of the stored manifest of release %q", secretName, release) + return nil +} + +func helmRerendersSecretWithoutServerSetMetadata(ctx context.Context, t framework.TestingT, release, secretName string) { + secret := helmStoredManifestSecret(ctx, t, release, secretName) + + // Server-populated metadata must not round-trip from the chart's + // bootstrap user lookup into the rendered manifest: Helm 4 server-side + // applies rendered manifests and the API server rejects objects carrying + // managedFields, while uid and resourceVersion act as apply + // preconditions. See https://github.com/redpanda-data/redpanda-operator/issues/1648. + require.Empty(t, secret.ManagedFields) + require.Empty(t, secret.UID) + require.Empty(t, secret.ResourceVersion) +} + +func helmRerendersSecretWithLivePassword(ctx context.Context, t framework.TestingT, release, secretName string) { + secret := helmStoredManifestSecret(ctx, t, release, secretName) + + var live corev1.Secret + require.NoError(t, t.Get(ctx, t.ResourceKey(secretName), &live)) + + // The re-render must carry the password already in use — anything else + // means the upgrade regenerated it. + require.NotEmpty(t, live.Data["password"]) + require.Equal(t, live.Data["password"], secret.Data["password"]) +} diff --git a/acceptance/steps/register.go b/acceptance/steps/register.go index 62f031708..991869731 100644 --- a/acceptance/steps/register.go +++ b/acceptance/steps/register.go @@ -97,6 +97,10 @@ func init() { // I can helm upgrade "release-name" "chart/path" with values: // I helm upgrade "release-name" "chart/path" --version v1.2.3 with values: framework.RegisterStep(`I(?: can)? helm upgrade "([^"]+)" "([^"]+)"(?: --version (\S+))? with values:`, iHelmUpgrade) + // the stored manifest for release "release-name" re-renders Secret "secret-name" without server-set metadata + framework.RegisterStep(`^the stored manifest for release "([^"]+)" re-renders Secret "([^"]+)" without server-set metadata$`, helmRerendersSecretWithoutServerSetMetadata) + // the stored manifest for release "release-name" re-renders Secret "secret-name" with the password in use + framework.RegisterStep(`^the stored manifest for release "([^"]+)" re-renders Secret "([^"]+)" with the password in use$`, helmRerendersSecretWithLivePassword) // Helm migration scenario steps framework.RegisterStep(`^the Kubernetes object of type "([^"]*)" with name "([^"]*)" has an OwnerReference pointing to the cluster "([^"]*)"$`, kubernetesObjectHasClusterOwner) diff --git a/charts/redpanda/chart/templates/_render_state.go.tpl b/charts/redpanda/chart/templates/_render_state.go.tpl index 4ea221887..3d6f545b1 100644 --- a/charts/redpanda/chart/templates/_render_state.go.tpl +++ b/charts/redpanda/chart/templates/_render_state.go.tpl @@ -15,11 +15,14 @@ {{- $existing_1 := (index $_76_existing_1_ok_2 0) -}} {{- $ok_2 := (index $_76_existing_1_ok_2 1) -}} {{- if $ok_2 -}} +{{- $_ := (set $existing_1.metadata "managedFields" (coalesce nil)) -}} +{{- $_ := (set $existing_1.metadata "resourceVersion" "") -}} +{{- $_ := (set $existing_1.metadata "uid" "") -}} {{- $_ := (set $existing_1 "immutable" true) -}} {{- $_ := (set $r "BootstrapUserSecret" $existing_1) -}} -{{- $_92_data_3_found_4 := (get (fromJson (include "_shims.dicttest" (dict "a" (list $existing_1.data $selector.key (coalesce nil))))) "r") -}} -{{- $data_3 := (index $_92_data_3_found_4 0) -}} -{{- $found_4 := (index $_92_data_3_found_4 1) -}} +{{- $_104_data_3_found_4 := (get (fromJson (include "_shims.dicttest" (dict "a" (list $existing_1.data $selector.key (coalesce nil))))) "r") -}} +{{- $data_3 := (index $_104_data_3_found_4 0) -}} +{{- $found_4 := (index $_104_data_3_found_4 1) -}} {{- if $found_4 -}} {{- $_ := (set $r "BootstrapUserPassword" (toString $data_3)) -}} {{- end -}} @@ -32,9 +35,9 @@ {{- range $_ := (list 1) -}} {{- $_is_returning := false -}} {{- if $r.Release.IsUpgrade -}} -{{- $_107_existing_5_ok_6 := (get (fromJson (include "_shims.lookup" (dict "a" (list "apps/v1" "StatefulSet" $r.Release.Namespace (get (fromJson (include "redpanda.Fullname" (dict "a" (list $r)))) "r"))))) "r") -}} -{{- $existing_5 := (index $_107_existing_5_ok_6 0) -}} -{{- $ok_6 := (index $_107_existing_5_ok_6 1) -}} +{{- $_119_existing_5_ok_6 := (get (fromJson (include "_shims.lookup" (dict "a" (list "apps/v1" "StatefulSet" $r.Release.Namespace (get (fromJson (include "redpanda.Fullname" (dict "a" (list $r)))) "r"))))) "r") -}} +{{- $existing_5 := (index $_119_existing_5_ok_6 0) -}} +{{- $ok_6 := (index $_119_existing_5_ok_6 1) -}} {{- if (and $ok_6 (gt ((get (fromJson (include "_shims.len" (dict "a" (list $existing_5.spec.template.metadata.labels)))) "r") | int) (0 | int))) -}} {{- $_ := (set $r "StatefulSetPodLabels" $existing_5.spec.template.metadata.labels) -}} {{- $_ := (set $r "StatefulSetSelector" $existing_5.spec.selector.matchLabels) -}} @@ -63,9 +66,9 @@ {{- $adminTLS = (get (fromJson (include "redpanda.InternalTLS.ToCommonTLS" (dict "a" (list $r.Values.listeners.admin.tls $r $r.Values.tls)))) "r") -}} {{- end -}} {{- $adminAuth := (coalesce nil) -}} -{{- $_155_adminAuthEnabled__ := (get (fromJson (include "_shims.typetest" (dict "a" (list "bool" (index $r.Values.config.cluster "admin_api_require_auth") false)))) "r") -}} -{{- $adminAuthEnabled := (index $_155_adminAuthEnabled__ 0) -}} -{{- $_ := (index $_155_adminAuthEnabled__ 1) -}} +{{- $_167_adminAuthEnabled__ := (get (fromJson (include "_shims.typetest" (dict "a" (list "bool" (index $r.Values.config.cluster "admin_api_require_auth") false)))) "r") -}} +{{- $adminAuthEnabled := (index $_167_adminAuthEnabled__ 0) -}} +{{- $_ := (index $_167_adminAuthEnabled__ 1) -}} {{- if $adminAuthEnabled -}} {{- $adminAuth = (mustMergeOverwrite (dict) (dict "username" $username "passwordSecretRef" (mustMergeOverwrite (dict) (dict "namespace" $r.Release.Namespace "secretKeyRef" (mustMergeOverwrite (dict "key" "") (mustMergeOverwrite (dict) (dict "name" $passwordRef.name)) (dict "key" $passwordRef.key)))))) -}} {{- end -}} diff --git a/charts/redpanda/render_state.go b/charts/redpanda/render_state.go index 7e8ac3bf3..48da50ff7 100644 --- a/charts/redpanda/render_state.go +++ b/charts/redpanda/render_state.go @@ -83,6 +83,15 @@ func (r *RenderState) FetchBootstrapUser() { // that a password be explicitly set? // See also: https://github.com/redpanda-data/helm-charts/issues/1596 if existing, ok := helmette.Lookup[corev1.Secret](r.Dot, r.Release.Namespace, selector.Name); ok { + // This object is re-rendered into the chart's output, so server + // populated metadata must be stripped: Helm 4 server-side applies + // rendered manifests and the API server rejects any apply request + // carrying metadata.managedFields, while resourceVersion and uid act + // as update preconditions we must not re-assert. + // See: https://github.com/redpanda-data/redpanda-operator/issues/1648 + existing.ObjectMeta.ManagedFields = nil + existing.ObjectMeta.ResourceVersion = "" + existing.ObjectMeta.UID = "" // make any existing secret immutable existing.Immutable = ptr.To(true) r.BootstrapUserSecret = existing diff --git a/charts/redpanda/render_state_test.go b/charts/redpanda/render_state_test.go index 3f7ed1d50..1ff1a9a50 100644 --- a/charts/redpanda/render_state_test.go +++ b/charts/redpanda/render_state_test.go @@ -213,6 +213,12 @@ func TestFetchBootstrapUser(t *testing.T) { require.Equal(t, tc.wantPassword, state.BootstrapUserPassword) if tc.wantSecret { require.NotNil(t, state.BootstrapUserSecret) + // The secret is re-rendered into the chart's output, so + // server populated metadata must not survive the lookup or + // Helm 4's server-side apply will reject the manifest. + require.Nil(t, state.BootstrapUserSecret.ManagedFields) + require.Empty(t, state.BootstrapUserSecret.ResourceVersion) + require.Empty(t, state.BootstrapUserSecret.UID) } else { require.Nil(t, state.BootstrapUserSecret) }