🐛 fix(api): snapshot.enabled: false is unsettable and never cleans up

[thilak009]

Re-raises #6 against main.

#6 was stacked on fix/snapshot-cronjob-scheduling (the branch behind #5). #5 was squash-merged, so main gained a single new commit rather than that branch's history, and #6 — merged into its original base — never reached main. GitHub auto-retargets a stacked PR when its base branch is deleted on merge, but delete_branch_on_merge is off for this repo, so it kept pointing at the stale branch.

No code changes: the three commits from #6 are cherry-picked onto main, and the resulting tree is byte-identical to the reviewed branch (git diff origin/fix/snapshot-enabled-false --stat is empty). Cherry-picked cleanly with no conflicts.

The original review discussion lives on #6.

What this contains

1. snapshot.enabled: false is unsettable. SnapshotSpec.Enabled was a non-pointer bool with omitempty and +kubebuilder:default=true. false is Go's zero value, so it is dropped on marshal and the API server re-applies the default:

enabled=false marshals to: {"schedule":"0 * * * *","retentionCount":5}
enabled=true  marshals to: {"enabled":true,"schedule":"0 * * * *","retentionCount":5}

It therefore cannot survive the operator's own full-object write when adding the finalizer. Enabled becomes *bool with an IsEnabled() accessor treating nil as true, matching the CRD default. The CRD schema is unchanged — *bool and bool both render as type: boolean with default: true; only the field description differs.

S3BackupSpec.Enabled is deliberately left a plain bool: it has no kubebuilder default, so absent and explicit-false are already equivalent there.

2. The cleanup path was unreachable. The call site gated on Spec.Snapshot.Enabled, but the delete branch lives inside the function being gated — so it could only run when snapshots were enabled, and its first act was to check whether they were disabled. reconcileSnapshotCronJob is now called unconditionally.

Deployment note

⚠️ Any cluster whose CR declares snapshot.enabled: false will have its snapshot CronJob deleted on the first reconcile after upgrade. That is the intended fix, but it is a behaviour change on upgrade rather than a pure bugfix, hence the minor version bump to 0.3.0.

This should ship together with or ahead of #5, not after it. #5 alone repairs the CronJobs without removing them, which starts snapshot jobs on clusters that have disabled them.

Testing

make ci passes — lint 0 issues, all packages green, build succeeds. api/v1alpha1 coverage at 90.2%.

🤖 Generated with Claude Code