fix(etcd): complete v1alpha2 transition for in-cluster 1.5→1.6 upgrades - #3270
Conversation
📝 WalkthroughWalkthroughChangesBackup endpoint resolution
Etcd adoption migration
Etcd operator upgrade handling
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
size/L
dosubot
Bot
added
the
area/testing
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request hardens the v1alpha2 transition for etcd by addressing critical deployment hook failures and improving security through image digest-pinning. It introduces extensive test coverage to prevent future regressions and refactors S3 endpoint resolution to provide consistent behavior across different backup strategies. Additionally, it enhances the reliability of migration scripts in restricted cluster environments. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces robust SAN checks and in-cluster kubeconfig synthesis for the etcd migration script, adds transitional headless service tests, and implements a helper to dynamically resolve S3 endpoints for backup strategies. It also pins the kubectl image digest and configures numeric user execution for the etcd-operator pre-upgrade hook. Feedback suggests trimming potential whitespaces when base64-decoding the backup endpoint secret, and adding resource requests and limits to the selector-fix hook container to ensure predictable scheduling.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| {{- else -}} | ||
| {{- $secret := lookup "v1" "Secret" .Values.backupStorage.namespace .Values.backupStorage.systemSecretName -}} | ||
| {{- if and $secret $secret.data (index $secret.data "endpoint") -}} | ||
| {{- printf "https://%s" (b64dec (index $secret.data "endpoint") | trimPrefix "https://" | trimPrefix "http://") -}} |
There was a problem hiding this comment.
When base64-decoding the endpoint from the Secret, there may be leading or trailing whitespaces or newlines (for example, if the secret was created with a trailing newline). It is safer to use the trim function before stripping the prefixes to ensure the endpoint URL is clean and doesn't cause malformed URLs in the generated resources.
{{- printf "https://%s" (b64dec (index $secret.data "endpoint") | trim | trimPrefix "https://" | trimPrefix "http://") -}}
| - name: selector-fix | ||
| image: "{{ .Values.kubectlImage.repository }}:{{ .Values.kubectlImage.tag }}" | ||
| image: {{ $img | quote }} | ||
| imagePullPolicy: {{ .Values.kubectlImage.pullPolicy }} | ||
| securityContext: | ||
| allowPrivilegeEscalation: false |
There was a problem hiding this comment.
The selector-fix container is missing resource requests and limits. According to the repository style guide (rule 99), new workloads should have resource requests/limits defined to ensure predictable scheduling and prevent unbounded resource consumption.
- name: selector-fix
image: {{ $img | quote }}
imagePullPolicy: {{ .Values.kubectlImage.pullPolicy }}
resources:
limits:
cpu: 100m
memory: 128Mi
requests:
cpu: 50m
memory: 64Mi
securityContext:
allowPrivilegeEscalation: falseReferences
- Missing resource requests/limits on new workloads. (link)
NOT LGTMDiff scope: merge-base Blocking1. The headline change (the new
Lines 28–41 and the values table must be updated to describe the derive-from-Secret-and-force-https behavior and the fallback semantics, mirroring the new 2. The tests added prove the manifests, not the contracts the fixes are about. The genuinely load-bearing claims in this PR are all runtime behaviors, but the new helm-unittest cases only assert static render output:
These belong in an e2e that drives the 1.5→1.6 upgrade / adoption / backup-upload flow. The migration-50 bats tests are the right shape by contrast — they run the actual Non-blocking / verified fine
Design note (not a code defect): the endpoint fix rests on the assumption that the COSI bucket's advertised |
github-actions
Bot
added
size/XL
|
Andrey Kolkov (@androndo) thanks for the thorough pass — both points are fair. #1 (doc) — fixed in #2 (tests) — agreed. helm-unittest can only assert render output: For the record, all three runtime contracts were validated end-to-end on a real 1.5→1.6 upgrade during review:
(Also pinned the floating |
VerdictLGTM with non-blocking notes The three fixes described in the body (numeric Findings[MINOR] PR body under-describes the actual diff — the highest-risk change is undocumented The body lists only three items (hook Claim mismatches[OK-for-record] "matches the postgres-operator webhook-ready hook that runs the same image" — verified: digest [PARTIAL] "extra/etcd/tests/... asserts the transitional etcd-headless Service" — the test is added by this PR but the Service itself lives in the base branch, so this PR's extra/etcd contribution is test-only. Body wording is accurate; noting for scope clarity only. Caveats
Recommended follow-ups
|
Andrey Kolkov (androndo)
left a comment
Andrey Kolkov (androndo)
left a comment
There was a problem hiding this comment.
Review — LGTM ✅
Reviewed in an isolated worktree pinned to the PR head; every load-bearing claim was executed rather than taken on trust. No bugs, no security issues, no regressions.
Verified (all passed)
- helm-unittest — etcd-operator 17/17, extra/etcd 18/18, backupstrategy-controller 11/11 (the last is omitted from the PR body but passes).
- migration-50 bats — 13/13, including the two new tests (exact SAN match, synthesized in-cluster kubeconfig).
- kubectl digest —
sha256:b9ef7d8…4fe05cis the same image already pinned by postgres-operator/mariadb/harbor. ✔ - workflow SHA pins —
actions/checkout@34e1148…==v4,actions/create-github-app-token@d72941d…==v1. No supply-chain substitution. - S3 endpoint derivation (the riskiest change) — traced by hand since
lookupreturns nil underhelm template: the producer (bucket/templates/user-credentials.yaml) writes a scheme-strippedendpointkey intobucket-cozy-backups-system-credentials(tenant-root), and the new helper (_helpers.tpl:92-103) reads exactly that key/namespace/format and re-forceshttps://. Key, namespace, and format all match. - fix vs. regression — SeaweedFS ships
enableSecurity: trueand consumers dialhttps://…:8333, so the old static defaulthttp://seaweedfs-s3.tenant-root.svc…:8333(wrong namespace + plaintext against a TLS listener) could not have worked forprovisionBucket: true. This is a fix, not a regression; the external-S3 (provisionBucket: false) path is unchanged and used verbatim. - docs —
backup-classes.mdupdated and accurate; no leaked secrets (fixtures use fake tokens).
⚠️ Process flag (worth addressing before merge)
The PR description covers ~6 of the 21 changed files. Three substantial changes land on merge without being described — the backupstrategy-controller S3-endpoint derivation (behavior change for every default backup strategy CR + Velero BSL), migration-50 robustness, and workflow SHA-pinning. This comes from the PR being stacked on #3265. Either land #3265 first so the diff narrows to what the body describes, or expand the body so the backups behavior change isn't rubber-stamped.
Recommendations (non-blocking)
- The live endpoint-derivation path (
provisionBucket: true → read Secret → force https) has no automated coverage — helm-unittest can't exercise it offline (lookupis nil). Best home is the backup e2e suite: assert the strategy CRs pick up the endpoint from the provisioned bucket. Verified manually here, which is why this is a recommendation and not a blocker. - Minor: the helper's
trimPrefix "https://" | trimPrefix "http://"is dead-defensive since the producer already strips the scheme — harmless insurance, just noting it never fires today.
|
Pushed
On the red E2E check — transient sandbox flake, not this PRThe two failing suites are Evidence it's unrelated to this PR:
A rerun should clear it. |
22d837d to
d49b853
Compare
…option etcd-migrate adopts legacy clusters IN PLACE, so the Pods keep their original spec.subdomain: etcd-headless and are dialed at etcd-<i>.etcd-headless.<ns>.svc until they eventually roll onto the operator's native <member>.etcd.<ns>.svc domain. The v1alpha2 operator only creates the native `etcd` Service, and the legacy `etcd-headless` Service is pruned during the transition, so those names stop resolving (no such host), the operator's MemberList fails, and status.readyMembers never populates -- the EtcdCluster never goes Ready even though the etcd processes are healthy and in quorum. Ship a chart-managed transitional headless `etcd-headless` Service (selector mirrors the operator's native `etcd` Service via etcd-operator.cozystack.io/cluster, publishNotReadyAddresses: true). This is the DNS counterpart of the legacy *.etcd-headless.<ns>.svc wildcard already kept in the server/peer cert SANs for the same transition window -- the TLS half of the compat was done, the DNS half was missing. Safe to remove, together with that SAN, once the members have rolled onto the native `etcd` subdomain. Verified live: recreating this Service on an adopted 3-node cluster restored per-pod DNS and the cluster went readyMembers=3 / Available=True. Refs: #3243 Signed-off-by: Andrey Kolkov <androndo@gmail.com>
…OM floor) Two defects that break the etcd-operator on an in-cluster 1.5 -> 1.6 upgrade (#2859 swapped the upstream chart for the cozystack-authored one): - #3242: the controller Deployment's spec.selector.matchLabels changed ({instance,name} -> {name,control-plane}). spec.selector is immutable, so `helm upgrade` cannot patch the existing Deployment and the whole HelmRelease upgrade fails ("field is immutable") -- the operator and the v1alpha2 CRDs it serves never come up, which blocks the etcd v1alpha2 adoption. Fresh installs use the new selector directly, so fresh-install CI does not catch it. Add a pre-upgrade hook that deletes the Deployment ONLY when its live selector is the pre-1.6 one (lacks control-plane=controller-manager), so Helm recreates it cleanly. No-op when the selector already matches (rc.1 -> later) and never runs on a fresh install (pre-upgrade only). Keeping the selector stable in the chart is not an option: rc.1 already shipped the new selector, so aligning it back would merely move the immutable break to rc.1 -> next. - Raise the manager's cold-start memory limit floor 128Mi -> 256Mi (and the VPA minAllowed to match). The steady-state working set is ~250Mi (the VPA's own recommendation); at 128Mi a Pod that starts before the VPA admission webhook rewrites it (e.g. a Deployment recreated out of band, or the webhook briefly unavailable during upgrade) OOMKills into a crash loop. Defense in depth so the operator never depends on VPA timing merely to avoid crashing; the VPA still scales it further under load up to maxAllowed. Verified live on a 1.5 -> 1.6 adoption: the delete-Deployment step plus the VPA re-applying 256Mi+ let the operator come up and the adoption complete. Refs: #3242, #3243 Signed-off-by: Andrey Kolkov <androndo@gmail.com>
…bectl, tests Addresses review on #3265: - pre-upgrade-selector-fix hook: add runAsUser: 65532. clastix/kubectl's image user is the non-numeric name `nonroot`, which the kubelet cannot verify against runAsNonRoot: true — so the Pod fails admission and the hook never runs, silently blocking the very 1.5->1.6 upgrade it exists to unblock. Matches postgres-operator's webhook-ready hook (same image). - Digest-pin the kubectl image (the comment already claimed digest-pinning but shipped a floating v1.32 tag). Reuse the digest postgres-operator already vendors, add the renovate annotation, and template repo:tag@digest. - Tests (both charts have CI helm-unittest suites, none previously covered these): - etcd-operator/tests/selector-fix-hook_test.yaml: hook wiring, namespaced least-privilege RBAC, numeric-non-root securityContext (guards the runAsUser regression above), digest-pinned image. - etcd-operator/tests/deployment_test.yaml: assert the 256Mi cold-start memory floor so it can't silently drop back to an OOMKilling value. - extra/etcd/tests/etcd-cluster_test.yaml: assert the transitional etcd-headless Service (headless, publishNotReadyAddresses, member selector, client/peer ports). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
…luster Migration 50 (etcd.aenix.io -> etcd-operator.cozystack.io/v1alpha2 adoption) had two defects that blocked every in-cluster 1.5 -> 1.6 upgrade on a cluster with an existing etcd: 1. Cert-SAN wait treated transient kubectl failures as "SAN absent". ensure_wildcard_sans checked/awaited the wildcard SAN with `kubectl get ... 2>/dev/null | grep`, so any transient GET failure (API discovery refresh, apiserver blip, throttling) produced an empty string indistinguishable from a genuine absence -> false miss, and the 120s wait never recovered. Replace the two ad-hoc checks with a _san_present helper that retries on an empty read (a real Certificate/Secret never has empty dnsNames/alt-names) and accepts the native wildcard from EITHER the issued Secret's cert-manager.io/alt-names annotation OR the Certificate spec.dnsNames (the source of truth for what cert-manager will issue). 2. etcd-migrate had no kubeconfig in-cluster. etcd-migrate only reads a kubeconfig file (-k/--kubeconfig, default /root/.kube/config) and, unlike kubectl, does not fall back to the mounted in-cluster ServiceAccount. The hook Job set no KUBECONFIG and passed no --kubeconfig, so both the dry-run and --apply aborted with "error building kubeconfig: stat /root/.kube/config: no such file". Synthesize an in-cluster kubeconfig from the mounted ServiceAccount and pass --kubeconfig to both etcd-migrate invocations. Verified end-to-end on a 1.5.2 -> 1.6.0-rc.1 upgrade: the adoption now completes in-place (pods never restarted, data intact) and the cluster reaches readyMembers=3 / Available=True. Refs: #3243, #3255 Signed-off-by: Andrey Kolkov <androndo@gmail.com>
…t-gated wait, IPv6 kubeconfig) Review feedback on #3261 (gemini-code-assist, coderabbitai, myasnikovdaniil): - Exact SAN match. _san_present read the Certificate SANs as {.spec.dnsNames} (a bracketed JSON blob that `tr ','` cannot split) and matched with a substring `grep -qF`, so a wildcard that is a substring of a longer SAN (e.g. *.etcd.<ns>.svc inside *.etcd.<ns>.svc.cluster.local) could false-positive and skip the re-issue. Read {.spec.dnsNames[*]} (space separated) and match exactly with `grep -qxF`, matching the Secret-annotation branch. - Wait on the re-issued Secret, not the patched spec. The post-patch wait loop called _san_present, which returns as soon as the Certificate spec.dnsNames contains the wildcard -- but the patch just added it there, so the loop broke on the first iteration before cert-manager re-issued the Secret, and the "re-issued" log printed unconfirmed. Add _secret_has_san (Secret alt-names annotation only, same empty-read retry + exact match) and gate the wait on it. - IPv6-safe kubeconfig server. The synthesized kubeconfig used https://$KUBERNETES_SERVICE_HOST:$KUBERNETES_SERVICE_PORT, which is an invalid URL on an IPv6-only cluster (a bare IPv6 host must be bracketed). Use https://kubernetes.default.svc -- IP-family-agnostic, validated by the mounted SA CA, and the same server URL kubectl synthesizes in-cluster. Tests (the review blocker): extend the bats suite (and its fake kubectl) with two cases pinning these contracts -- a superstring dnsName must NOT skip the re-issue patch, and etcd-migrate must be invoked with a synthesized in-cluster kubeconfig whose server is kubernetes.default.svc (not the bare IPv6 host), authenticating via the SA token file + CA. Both the kubeconfig path and the SA directory are now env-overridable (ETCD_MIGRATE_KUBECONFIG, ETCD_ADOPT_SA_DIR) so the synthesis is exercisable off-cluster; production defaults are unchanged. Signed-off-by: Andrey Kolkov <androndo@gmail.com>
The default backupStorage.endpoint is http://seaweedfs-s3...svc:8333, but Cozystack ships SeaweedFS with global.seaweedfs.enableSecurity=true, so its in-cluster S3 serves TLS on :8333 behind the self-signed "SeaweedFS CA". Every cozy-default backup (etcd/mariadb/velero/fdb/cnpg) therefore hits a TLS listener over plaintext and fails the handshake. This is fatal for the etcd v1alpha2 adoption migration: its mandatory pre-upgrade safety snapshot writes to this endpoint, so migration 50 hard-fails and blocks the 1.5->1.6 upgrade on any cluster with a legacy etcd. The Etcd Strategy S3 schema has no caCert/insecureSkipVerify field, so it cannot target the self-signed in-cluster endpoint at all. Add a "backupstrategy-controller.endpoint" helper that, for a provisioned bucket, resolves the endpoint from the COSI bucket's system credentials Secret (backupStorage.systemSecretName) — the external S3 ingress with an ACME cert, the same trusted endpoint COSI advertises and every backup operator can verify — and forces the https:// scheme. All Strategy CRs, the Velero BackupStorageLocation and the controller Deployment env are routed through it. It falls back to .Values.backupStorage.endpoint for external S3 (provisionBucket=false) and for offline `helm template`/unit renders and the pre-reconcile first install, where the Secret lookup returns nothing (Flux re-renders on spec.interval once the Secret exists). helm-unittest extended to assert every consumer picks up the resolved endpoint (CNPG/Etcd/Velero keep the full URL, MariaDB/FDB strip the scheme and derive their secure flag) plus the external-S3 verbatim fallback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The `backupstrategy-controller.endpoint` helper changed how the S3 endpoint is resolved (for a provisioned bucket it derives from the COSI system Secret and forces https://, with `backupStorage.endpoint` demoted to a fallback), but docs/operations/backup-classes.md still described the old single-verbatim-value behavior and directly contradicted the code: - the "Endpoint format per driver" intro said the templates adapt "the single backupStorage.endpoint value"; - it claimed drivers "pull from backupStorage.endpoint in chart values, not from the Secret" — now precisely backwards for provisioned buckets; - the values-table `endpoint` row omitted the fallback/derive semantics. Rewrite those to match the new helper + the values.yaml comment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
…k resources - backupstrategy-controller: trim() the decoded S3 endpoint before scheme stripping so a trailing newline in the Secret can't yield a malformed URL - etcd-operator: give the pre-upgrade selector-fix hook container resource requests/limits (matches postgres-operator webhook-ready), +unittest Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
The base branch was changed.
d49b853 to
4af5a82
Compare
myasnikovdaniil
changed the title
|
Rebased PR on main to use new e2e testsuite Andrey Kolkov (@androndo) please rereview this it is basically same diff but tested over fresh main |
Andrey Kolkov (androndo)
left a comment
Andrey Kolkov (androndo)
left a comment
There was a problem hiding this comment.
LGTM
This is a carefully-scoped, well-documented set of fixes for the in-cluster 1.5→1.6 etcd upgrade path. I traced every load-bearing wiring assumption to the actual code and found no bugs, no security issues, no regressions, and no documentation drift.
What I verified (the claims that would silently no-op the fix if wrong)
- etcd-headless Service selector is correct. The transitional Service selects
etcd-operator.cozystack.io/cluster: etcd. TheEtcdClusterinpackages/extra/etcd/templates/etcd-cluster.yaml:14is hardcodedname: etcd, and the operator's selector form is confirmed by the CRD doc (packages/system/etcd-operator-crds/templates/etcdclusters.yaml:3289—etcd-operator.cozystack.io/cluster=<name>). So the selector resolves the real member Pods. Headless +publishNotReadyAddresses: trueis right for restoringetcd-<i>.etcd-headless.<ns>.svcper-pod records. No name collision with the operator-owned nativeetcdService. - Pre-upgrade hook targets the real Deployment.
etcd-operator-controller-managermatchesdeployment.yaml:6. The 1.6 selector carriescontrol-plane: controller-manager(deployment.yaml:15-16), and the hook deletes only when that label is absent — correct no-op on 1.6→later, correct delete on pre-1.6.runAsUser: 65532fix is legitimate (clastix/kubectl'snonrootuser is non-numeric and fails the kubeletrunAsNonRootcheck without it). - 256Mi floor actually flows. The manager container consumes
.Values.resources(deployment.yaml:71), andcontainers[0]ismanager, so thedeployment_test.yamlassertion targets the right container. - The endpoint helper's Secret key exists.
index $secret.data "endpoint"is a real key:packages/system/bucket/templates/user-credentials.yamlwrites a scheme-stripped bare-hostendpointintobucket-cozy-backups-system-credentialsinbackupStorage.namespace(tenant-root) — the exact{namespace, name}the helper looks up. Theb64dec | trim | trimPrefix | printf "https://%s"chain correctly reconstructshttps://<host>, and the render-timelookupis the same pattern already used by the establishedbackupstrategy-controller.bucketNamehelper (run by Flux's helm-controller, which has the cluster read access). - Migration-50 shell logic is sound.
_san_present/_secret_has_sanretry only on a genuinely-empty read (never legitimately empty on a real object) and match exactly withgrep -qxF— the superstring false-positive is really closed. The "patch when unsure" direction is safe (patch is idempotent). The synthesized kubeconfig useskubernetes.default.svc(IPv6-safe) andtokenFile/certificate-authorityfrom the SA dir; heredoc expansion is correct.#!/bin/bash, soreturn(bare) returning the last command's status behaves as intended.
Test coverage
Good for everything that is unit-testable. Both valid and fallback/edge paths are covered: the endpoint suite asserts full-URL delivery to CNPG/Etcd/Velero, scheme-flip for MariaDB/FDB, and the external-S3 verbatim path; the migration bats suite adds the exact-SAN superstring guard and the synthesized-kubeconfig contract (including the "no bare IPv6 in the URL" negative assertion); the hook and Service suites pin the rendered shape.
Notes / recommendations (not blockers)
- Two runtime contracts have no automated regression guard, by nature. (a) The endpoint derive-from-Secret path relies on helm
lookup, which returns nothing under helm-unittest — the unit tests exercise only the fallback, as the suite's own NOTE admits. (b) The hook's delete-if-pre-1.6 shell logic can't be exercised by helm-unittest (it only renders). Both are genuinely e2e-only contracts, verified live on a 1.5.2→1.6.0-rc.1 adoption per the PR body, and consistent with this repo's philosophy that the plugin↔cluster contract belongs in e2e, not in manifest-parsing unit tests. Recommendation: make sure the named 1.5→1.6 e2e upgrade job is actually wired into CI as the authoritative guard the PR body claims it to be — that is the only thing standing behind both behaviors. - The endpoint helper unconditionally forces
https://for provisioned buckets. Correct for the SeaweedFS/COSI ACME-ingress assumption this PR documents, but the producer strips the scheme into the Secret, so the original scheme is lost and a hypothetical future COSI driver advertising a plain-http endpoint would be silently upgraded to https. Documented assumption; fine for the supported config, worth a mental note if external-COSI support widens.
3 participants
Consolidates the etcd
v1alpha2transition fix for in-cluster 1.5 → 1.6 upgrades into a single PR, rebased on currentmain. Supersedes #3265 and #3261 — their commits are carried here (authorship preserved), so those PRs can be closed once this lands. Fresh installs use the new shapes directly and are unaffected, so fresh-install CI does not catch these; the 1.5 → 1.6 e2e upgrade path is the authoritative regression guard.What this PR does
1. Keep the legacy
etcd-headlessService alive during adoption —packages/extra/etcdetcd-migrateadopts legacy clusters in place: the Pods keep their originalspec.subdomain: etcd-headlessand are dialed atetcd-<i>.etcd-headless.<ns>.svcuntil they roll onto the operator's native<member>.etcd.<ns>.svcdomain. The v1alpha2 operator only creates the nativeetcdService and the legacyetcd-headlessService is pruned, so those per-pod names stop resolving (no such host),MemberListfails, andstatus.readyMembersnever populates — theEtcdClusternever goesReadyeven though etcd is healthy and in quorum. We ship a chart-managed transitional headlessetcd-headlessService (selector mirrors the operator's nativeetcdService viaetcd-operator.cozystack.io/cluster,publishNotReadyAddresses: true) — the DNS counterpart of the legacy*.etcd-headless.<ns>.svcSAN already kept for this window. Removable together with that SAN once members roll onto the native subdomain.2. Survive the immutable controller-Deployment selector on upgrade —
packages/system/etcd-operator(#3242)#2859 replaced the upstream etcd-operator chart with the cozystack-authored one, changing
Deployment.spec.selector.matchLabels.spec.selectoris immutable, sohelm upgradecannot patch the existing Deployment and the whole HelmRelease upgrade fails (field is immutable). A pre-upgrade hook (templates/pre-upgrade-selector-fix.yaml: ServiceAccount + Role + RoleBinding + Job) deletes the Deployment only when its live selector is the pre-1.6 one, so Helm recreates it cleanly. No-op when the selector already matches, never runs on fresh install.3. Raise the operator's memory cold-start floor —
packages/system/etcd-operatorSteady-state working set is ~250Mi; the static
limits.memory: 128MiOOMKills a Pod that starts before the VPA admission webhook rewrites it. Raise the floor to256Mi(and VPAminAllowedto match) as defense in depth so the operator never depends on VPA timing to avoid crashing.4. Make migration 50 (etcd adoption) robust in-cluster —
packages/core/platform/images/migrations/migrations/50(was #3261)Exact server/peer cert-SAN match, Secret-gated wait on the adoption Secret, and in-cluster (IPv6-safe) kubeconfig handling, so the migration script drives the adoption reliably from inside the cluster. Covered by
hack/migration-50-etcd-adopt.bats.5. Hardening / review fixes (this PR's original scope)
runAsUser: 65532.pre-upgrade-selector-fix.yamlsetrunAsNonRoot: truebut no numericrunAsUser;clastix/kubectl's image user is the non-numeric namenonroot, which the kubelet cannot verify againstrunAsNonRoot— so the hook Pod fails admission and silently blocks the very upgrade it exists to unblock. AddsrunAsUser: 65532, matching the postgres-operator webhook-ready hook that runs the same image.v1.32tag. Reuses the digest postgres-operator vendors, adds therenovateannotation, and templatesrepo:tag@digest.etcd-operator/tests/selector-fix-hook_test.yaml(hook wiring, weight ordering, namespaced least-privilege RBAC, numeric-non-root security context, digest-pinned image),etcd-operator/tests/deployment_test.yaml(256Mi cold-start floor),extra/etcd/tests/etcd-cluster_test.yaml(transitionaletcd-headlessService). Each assertion was mutation-tested.6. Derive the default S3 endpoint from the provisioned bucket —
packages/system/backupstrategy-controllerDerive the default S3 endpoint (and per-driver scheme / TLS /
secure_connection) from the provisioned bucket Secret instead of requiring it to be hand-set, so etcd (and other) backup strategies get a working endpoint by default. Docs indocs/operations/backup-classes.md; covered bytests/endpoint_form_test.yaml.Verification
helm unittestgreen on currentmain: etcd-operator 18/18, extra/etcd 18/18, backupstrategy-controller 11/11.etcd-headlessService took the adopted cluster toreadyMembers=3 / Available=True.Summary by CodeRabbit
New Features
Bug Fixes
Documentation