fix(vertical-pod-autoscaler): migrate admission-controller TLS to cert-manager to stop reconcile-driven ReplicaSet churn - #3178

…real name

certificate.yaml renders the Certificate with name <fullname>-tls (its
secretName matches the legacy chart-owned Secret exactly), but the
rollout hook was passing plain <fullname> to `kubectl wait` — so
every post-install/post-upgrade cycle sat 3 minutes on NotFound,
failed, and burned through the Job's backoffLimit. cozystack-platform
then aborted its HR reconcile because the VPA release's post-upgrade
hook was Failed.

Snapshot from the first CI run of #3178 confirms:
  - Certificate object: vertical-pod-autoscaler-admission-controller-tls
    (namespace cozy-vertical-pod-autoscaler)
  - Rollout Job pods: 10+ BackOff / BackoffLimitExceeded
  - HR condition: post-upgrade hooks failed
  - Admission-controller Deployment itself: 1 ReplicaSet, 1 Ready pod
    (the actual chart change works — churn is gone; only the rollout
    trigger was broken)

Fix propagates the -tls suffix through both places that reference the
Certificate:
  * The rollout container's `cert` variable used in `kubectl wait`.
  * The Role's resourceNames on cert-manager.io/certificates so RBAC
    still admits the get/watch/list against the actual object.

Regression test in vpa_test.yaml asserts rules[0].resourceNames[0]
carries the -tls suffix so this exact off-by-suffix bug cannot come
back through a future rename.

Signed-off-by: Ivan Okhotnikov <[email protected]>

…real name

certificate.yaml renders the Certificate with name <fullname>-tls (its
secretName matches the legacy chart-owned Secret exactly), but the
rollout hook was passing plain <fullname> to `kubectl wait` — so
every post-install/post-upgrade cycle sat 3 minutes on NotFound,
failed, and burned through the Job's backoffLimit. cozystack-platform
then aborted its HR reconcile because the VPA release's post-upgrade
hook was Failed.

Snapshot from the first CI run of #3178 confirms:
  - Certificate object: vertical-pod-autoscaler-admission-controller-tls
    (namespace cozy-vertical-pod-autoscaler)
  - Rollout Job pods: 10+ BackOff / BackoffLimitExceeded
  - HR condition: post-upgrade hooks failed
  - Admission-controller Deployment itself: 1 ReplicaSet, 1 Ready pod
    (the actual chart change works — churn is gone; only the rollout
    trigger was broken)

Fix propagates the -tls suffix through both places that reference the
Certificate:
  * The rollout container's `cert` variable used in `kubectl wait`.
  * The Role's resourceNames on cert-manager.io/certificates so RBAC
    still admits the get/watch/list against the actual object.

Regression test in vpa_test.yaml asserts rules[0].resourceNames[0]
carries the -tls suffix so this exact off-by-suffix bug cannot come
back through a future rename.

Signed-off-by: Ivan Okhotnikov <[email protected]>

Kvaps' review on #3178 correctly identified that the pre-upgrade
tls-migrator Job cannot do what its comments claim, for two
independent reasons:

  1. Helm decides what to delete on upgrade from the STORED release
     manifest, not from live labels on the object. The only live-
     object opt-out is the \`helm.sh/resource-policy: keep\`
     annotation — see the \`original.Difference(target)\` loop in
     helm's pkg/kube/client.go, which only checks
     \`ResourcePolicyAnno\`. Stripping \`meta.helm.sh/*\` and
     \`app.kubernetes.io/managed-by\` does not prevent the deletion;
     those markers only matter for adoption checks on resources
     entering a manifest, not on resources leaving it.

     Likewise, helm-controller has no independent label-based prune
     for Helm-managed resources — the \`helm.toolkit.fluxcd.io/*\`
     labels are tracking metadata only, not a deletion gate.

  2. Even if the Secret survived the Helm/Flux delete pass,
     cert-manager still could not adopt it in place: the legacy
     Secret has \`type: Opaque\`, cert-manager writes
     \`type: kubernetes.io/tls\`, and Secret.type is immutable in
     Kubernetes. The Certificate reconcile would fail with
     \"field is immutable\" and never become Ready.

The migration converges anyway because the Job is a no-op: Helm
deletes the old Opaque Secret, cert-manager mints a fresh one of
the right type, and the Deployment rolls in the same upgrade
(its podTemplate carries the new \`checksum/tls-cert\` annotation,
which is the sha of the runtime Secret and therefore differs the
first time cert-manager writes new content). The webhook's
\`failurePolicy: Ignore\` makes the seconds-long window benign.

Delete the migrator entirely: template file, RBAC (ServiceAccount +
Role + RoleBinding), the hardcoded clastix/kubectl image reference,
and the four helm-unittest cases pinning its hook wiring / RBAC
scope. ~200 lines removed, one less pinned image, and the
documented migration flow now matches what actually happens.

Existing helm-unittest suite: 15/15 pass (was 19 with the migrator
tests; removed 4 that referenced the retired template).

Signed-off-by: Ivan Okhotnikov <[email protected]>

…diff on the vendored chart

Kvaps' review on #3178 flagged that this package's \`make update\`
target does \`rm -rf charts && helm pull cowboysysop/vertical-pod-
autoscaler --version 11.1.1\` with no re-apply step. Any local edits
to the vendored subchart were therefore one \`make update\` away
from vanishing without warning — exactly what the fluxcd-operator
package pre-empts via a \`patches/*.diff\` re-apply loop after the
helm pull.

Adopt the same convention here:

  * New file \`patches/cert-manager-admission-tls.diff\` capturing
    all four modifications the cert-manager path makes to the
    vendored subchart — the new \`certificate.yaml\` template, the
    checksum/tls-secret + new checksum/tls-cert annotation gating on
    the admission-controller Deployment, the certManagerEnabled
    guard on the legacy \`tls-secret.yaml\`, and the new
    \`certManagerEnabled: true\` default in the subchart values.

  * Makefile \`update\` target now runs
    \`patch --no-backup-if-mismatch -p1 < patches/*.diff\` after the
    helm pull, so a future upstream bump (or a routine
    \`make update\` today) re-applies the cozystack modifications
    over the freshly-fetched vendored files and leaves the chart in
    the same state git tracks.

Verified end-to-end: with the on-disk chart intact I ran
  cp -r charts/vertical-pod-autoscaler /tmp/reference
  rm -rf charts
  helm pull cowboysysop/vertical-pod-autoscaler --untar --untardir charts --version 11.1.1
  patch --no-backup-if-mismatch -p1 < patches/cert-manager-admission-tls.diff
  diff -r charts/vertical-pod-autoscaler /tmp/reference
The diff is empty — \`make update\` now produces a byte-identical
tree to what git has right now, and the 15 helm-unittest cases still
pass unchanged.

Signed-off-by: Ivan Okhotnikov <[email protected]>

Kvaps' review on #3178 called out three imprecisions in the
supporting comments. Fix them without touching runtime behaviour:

  1. certificate.yaml previously emitted a secretTemplate whose
     labels include \`app.kubernetes.io/managed-by: Helm\`. Helm
     does NOT own the Secret written by cert-manager — the Certificate
     resource does — and stamping that label on it misleads operators
     reading the object with \`kubectl get secret -oyaml\`. Drop
     secretTemplate; cert-manager labels its outputs unambiguously
     (\`cert-manager.io/*\`, \`controller.cert-manager.io/*\`).

  2. deployment.yaml comment on checksum/tls-cert claimed the lookup
     \"resolves at every reconcile\". Not true. \`lookup\` runs only
     on Helm actions (install / upgrade / rollback); helm-controller
     does not re-render a deployed release on interval ticks. Under
     cozystack's release cadence this is fine — any real chart bump,
     values change, or manual \`flux reconcile hr\` triggers a
     re-template within days — but at day ~350 renewal the pod
     continues serving the certificate it loaded at startup until the
     next Helm action. If no Helm action lands in the ~15 days
     between renewal and expiry the webhook silently stops mutating.
     The comment now spells that trade-off out so a future reader can
     find it, and points at a small post-renewal Job as the follow-up
     without re-introducing the ReplicaSet-churn amplifier the whole
     PR retires.

  3. values.yaml comment on certManagerEnabled did not warn that the
     switch is effectively one-way. Once cert-manager has written the
     Secret with \`type: kubernetes.io/tls\`, flipping the flag back
     to false and rolling back to the legacy Opaque path fails —
     \`Secret.type\` is immutable, so the legacy Secret template
     cannot overwrite the cert-manager-owned one in place. The doc
     now says operators must \`kubectl delete\` the Secret first if
     they need to roll back.

Also refreshed patches/cert-manager-admission-tls.diff to reflect
these comment-only edits and re-verified end-to-end that
\`make update\`-style pull + patch produces a byte-identical tree
to what git tracks now.

15/15 helm-unittest cases still pass.

Signed-off-by: Ivan Okhotnikov <[email protected]>

…tus-patch stalls

Symptom
  cozystack-platform HelmRelease times out at 15m on fresh install
  with a single PackageSource stuck in InProgress. Snapshot from
  #3178 CI run 28587656430:

    PackageSource cozystack.vm-disk-application
      Ready: Unknown, reason: Progressing
      lastTransitionTime: 2026-07-02T13:39:26Z
      (snapshot generated 30+ minutes later — no state change)

    ArtifactGenerator cozystack.vm-disk-application
      status.inventory:              2 items with sha256 digests
      status.observedSourcesDigest:  populated
      status.conditions[Ready]:      Unknown, reason: Progressing
      status.conditions[Reconciling]: True, reason: Progressing

  ExternalArtifacts are produced. Downstream Flux consumes them and
  its HelmReleases install fine. But PackageSourceReconciler
  faithfully copies the Unknown Ready condition through, so
  cozystack-platform (which waits on PackageSource.Ready=True) never
  converges.

Root cause
  Traced in fluxcd/source-watcher:v2.1.0 log for the affected
  ArtifactGenerator:

    13:39:28.475  ExternalArtifact vm-disk       reconciled ✅
    13:39:28.641  ExternalArtifact vm-disk-rd    reconciled ✅
    13:39:45.719  ❌ failed to update status: etcdserver: request timed out
    13:39:45.720  ❌ Reconciler error (etcd timeout)
    13:39:46.485  ✅ No drift detected, 2 artifact(s) up to date
    ——— nothing else for this ArtifactGenerator ———

  Under first-install etcd pressure the patchStatus call times out.
  The reconciler returns the etcd error; controller-runtime prints
  the "returned both a result and non-nil error, RequeueAfter
  ignored" warning and requeues. The next reconcile hits the "No
  drift detected" short-circuit (artifacts are already up to date)
  and early-exits before touching status. Result: the Ready
  condition is stuck Unknown until either the process restarts or
  something forces a state change on the ArtifactGenerator. Neither
  happens in a real install run.

  This is an upstream bug in source-watcher's reconcile loop. It has
  been papered over once already at the runtime-tuning level in
  #3163 (source-watcher concurrency + CPU), but a bad etcd second
  during install still lands us here. Cozystack cannot afford to
  wait for an upstream release cycle to unblock every install.

Fix
  PackageSourceReconciler.updateStatus now checks two invariants
  before falling back to copying the (potentially stuck) upstream
  Ready condition:

    1. ArtifactGenerator.status.Inventory is non-empty
    2. ArtifactGenerator.status.ObservedSourcesDigest is set

  Both fields are only written by source-watcher after the artifacts
  have actually been produced — status.Inventory carries the
  ExternalArtifact digests downstream Flux is already consuming. If
  BOTH are present AND the upstream Ready condition is still Unknown
  (or absent), the PackageSource Ready condition is synthesised as
  True with a distinctive reason `ArtifactsGeneratedAwaitingUpstreamStatus`
  and a message that names the upstream bug and the observable state
  it relied on. If the upstream Ready ever catches up (True or False)
  the Owns() watch re-fires the reconciler and the real condition
  overwrites the synthetic one — True stays True, False surfaces the
  real failure. Failures are NEVER masked: the synthesis path only
  triggers when Ready is Unknown or missing.

Safety
  * Never synthesises True over a real False → upstream failure is
    surfaced unchanged.
  * Requires BOTH Inventory non-empty AND ObservedSourcesDigest set
    → a fresh ArtifactGenerator that has literally not started work
    cannot be prematurely marked Ready.
  * ObservedGeneration is stamped so a spec change re-invalidates
    the synthesis until the next observable-artifacts pass.
  * Reason string is greppable so operators can audit which
    PackageSources came up via the workaround.

Tests
  Added TestArtifactGeneratorObservablyReady with 7 cases covering
  every branch of the predicate:
    - fresh generator: false
    - inventory only:  false
    - digest only:     false
    - both set + Unknown Ready:   true  (the actual bug)
    - both set + no Ready cond:   true  (same window pre-stamp)
    - both set + Ready=True:      false (pass real condition)
    - both set + Ready=False:     false (never mask failure)
  All pass; existing operator unit tests are unaffected.

Signed-off-by: Ivan Okhotnikov <[email protected]>

…tus-patch stalls

Symptom
  cozystack-platform HelmRelease times out at 15m on fresh install
  with a single PackageSource stuck in InProgress. Snapshot from
  #3178 CI run 28587656430:

    PackageSource cozystack.vm-disk-application
      Ready: Unknown, reason: Progressing
      lastTransitionTime: 2026-07-02T13:39:26Z
      (snapshot generated 30+ minutes later — no state change)

    ArtifactGenerator cozystack.vm-disk-application
      status.inventory:              2 items with sha256 digests
      status.observedSourcesDigest:  populated
      status.conditions[Ready]:      Unknown, reason: Progressing
      status.conditions[Reconciling]: True, reason: Progressing

  ExternalArtifacts are produced. Downstream Flux consumes them and
  its HelmReleases install fine. But PackageSourceReconciler
  faithfully copies the Unknown Ready condition through, so
  cozystack-platform (which waits on PackageSource.Ready=True) never
  converges.

Root cause
  Traced in fluxcd/source-watcher:v2.1.0 log for the affected
  ArtifactGenerator:

    13:39:28.475  ExternalArtifact vm-disk       reconciled ✅
    13:39:28.641  ExternalArtifact vm-disk-rd    reconciled ✅
    13:39:45.719  ❌ failed to update status: etcdserver: request timed out
    13:39:45.720  ❌ Reconciler error (etcd timeout)
    13:39:46.485  ✅ No drift detected, 2 artifact(s) up to date
    ——— nothing else for this ArtifactGenerator ———

  Under first-install etcd pressure the patchStatus call times out.
  The reconciler returns the etcd error; controller-runtime prints
  the "returned both a result and non-nil error, RequeueAfter
  ignored" warning and requeues. The next reconcile hits the "No
  drift detected" short-circuit (artifacts are already up to date)
  and early-exits before touching status. Result: the Ready
  condition is stuck Unknown until either the process restarts or
  something forces a state change on the ArtifactGenerator. Neither
  happens in a real install run.

  This is an upstream bug in source-watcher's reconcile loop. It has
  been papered over once already at the runtime-tuning level in
  #3163 (source-watcher concurrency + CPU), but a bad etcd second
  during install still lands us here. Cozystack cannot afford to
  wait for an upstream release cycle to unblock every install.

Fix
  PackageSourceReconciler.updateStatus now checks two invariants
  before falling back to copying the (potentially stuck) upstream
  Ready condition:

    1. ArtifactGenerator.status.Inventory is non-empty
    2. ArtifactGenerator.status.ObservedSourcesDigest is set

  Both fields are only written by source-watcher after the artifacts
  have actually been produced — status.Inventory carries the
  ExternalArtifact digests downstream Flux is already consuming. If
  BOTH are present AND the upstream Ready condition is still Unknown
  (or absent), the PackageSource Ready condition is synthesised as
  True with a distinctive reason `ArtifactsGeneratedAwaitingUpstreamStatus`
  and a message that names the upstream bug and the observable state
  it relied on. If the upstream Ready ever catches up (True or False)
  the Owns() watch re-fires the reconciler and the real condition
  overwrites the synthetic one — True stays True, False surfaces the
  real failure. Failures are NEVER masked: the synthesis path only
  triggers when Ready is Unknown or missing.

Safety
  * Never synthesises True over a real False → upstream failure is
    surfaced unchanged.
  * Requires BOTH Inventory non-empty AND ObservedSourcesDigest set
    → a fresh ArtifactGenerator that has literally not started work
    cannot be prematurely marked Ready.
  * ObservedGeneration is stamped so a spec change re-invalidates
    the synthesis until the next observable-artifacts pass.
  * Reason string is greppable so operators can audit which
    PackageSources came up via the workaround.

Tests
  Added TestArtifactGeneratorObservablyReady with 7 cases covering
  every branch of the predicate:
    - fresh generator: false
    - inventory only:  false
    - digest only:     false
    - both set + Unknown Ready:   true  (the actual bug)
    - both set + no Ready cond:   true  (same window pre-stamp)
    - both set + Ready=True:      false (pass real condition)
    - both set + Ready=False:     false (never mask failure)
  All pass; existing operator unit tests are unaffected.

Signed-off-by: Ivan Okhotnikov <[email protected]>

…tus-patch stalls

Symptom
  cozystack-platform HelmRelease times out at 15m on fresh install
  with a single PackageSource stuck in InProgress. Snapshot from
  #3178 CI run 28587656430:

    PackageSource cozystack.vm-disk-application
      Ready: Unknown, reason: Progressing
      lastTransitionTime: 2026-07-02T13:39:26Z
      (snapshot generated 30+ minutes later — no state change)

    ArtifactGenerator cozystack.vm-disk-application
      status.inventory:              2 items with sha256 digests
      status.observedSourcesDigest:  populated
      status.conditions[Ready]:      Unknown, reason: Progressing
      status.conditions[Reconciling]: True, reason: Progressing

  ExternalArtifacts are produced. Downstream Flux consumes them and
  its HelmReleases install fine. But PackageSourceReconciler
  faithfully copies the Unknown Ready condition through, so
  cozystack-platform (which waits on PackageSource.Ready=True) never
  converges.

Root cause
  Traced in fluxcd/source-watcher:v2.1.0 log for the affected
  ArtifactGenerator:

    13:39:28.475  ExternalArtifact vm-disk       reconciled ✅
    13:39:28.641  ExternalArtifact vm-disk-rd    reconciled ✅
    13:39:45.719  ❌ failed to update status: etcdserver: request timed out
    13:39:45.720  ❌ Reconciler error (etcd timeout)
    13:39:46.485  ✅ No drift detected, 2 artifact(s) up to date
    ——— nothing else for this ArtifactGenerator ———

  Under first-install etcd pressure the patchStatus call times out.
  The reconciler returns the etcd error; controller-runtime prints
  the "returned both a result and non-nil error, RequeueAfter
  ignored" warning and requeues. The next reconcile hits the "No
  drift detected" short-circuit (artifacts are already up to date)
  and early-exits before touching status. Result: the Ready
  condition is stuck Unknown until either the process restarts or
  something forces a state change on the ArtifactGenerator. Neither
  happens in a real install run.

  This is an upstream bug in source-watcher's reconcile loop. It has
  been papered over once already at the runtime-tuning level in
  #3163 (source-watcher concurrency + CPU), but a bad etcd second
  during install still lands us here. Cozystack cannot afford to
  wait for an upstream release cycle to unblock every install.

Fix
  PackageSourceReconciler.updateStatus now checks two invariants
  before falling back to copying the (potentially stuck) upstream
  Ready condition:

    1. ArtifactGenerator.status.Inventory is non-empty
    2. ArtifactGenerator.status.ObservedSourcesDigest is set

  Both fields are only written by source-watcher after the artifacts
  have actually been produced — status.Inventory carries the
  ExternalArtifact digests downstream Flux is already consuming. If
  BOTH are present AND the upstream Ready condition is still Unknown
  (or absent), the PackageSource Ready condition is synthesised as
  True with a distinctive reason `ArtifactsGeneratedAwaitingUpstreamStatus`
  and a message that names the upstream bug and the observable state
  it relied on. If the upstream Ready ever catches up (True or False)
  the Owns() watch re-fires the reconciler and the real condition
  overwrites the synthetic one — True stays True, False surfaces the
  real failure. Failures are NEVER masked: the synthesis path only
  triggers when Ready is Unknown or missing.

Safety
  * Never synthesises True over a real False → upstream failure is
    surfaced unchanged.
  * Requires BOTH Inventory non-empty AND ObservedSourcesDigest set
    → a fresh ArtifactGenerator that has literally not started work
    cannot be prematurely marked Ready.
  * ObservedGeneration is stamped so a spec change re-invalidates
    the synthesis until the next observable-artifacts pass.
  * Reason string is greppable so operators can audit which
    PackageSources came up via the workaround.

Tests
  Added TestArtifactGeneratorObservablyReady with 7 cases covering
  every branch of the predicate:
    - fresh generator: false
    - inventory only:  false
    - digest only:     false
    - both set + Unknown Ready:   true  (the actual bug)
    - both set + no Ready cond:   true  (same window pre-stamp)
    - both set + Ready=True:      false (pass real condition)
    - both set + Ready=False:     false (never mask failure)
  All pass; existing operator unit tests are unaffected.

Signed-off-by: Ivan Okhotnikov <[email protected]>