feat(tls): cert-manager-managed TLS for RedisFailover - #1

Replace github.com/go-redis/redis/v8 with github.com/redis/go-redis/v9
(v9.19.0). The v8 module is no longer receiving updates.

No code changes required beyond the import path: every API used by
service/redis/client.go (NewClient, Options, NewSentinelClient,
NewBoolCmd, NewIntCmd, NewStatusCmd, NewSliceCmd, Process, Info,
SlaveOf, CkQuorum, ConfigSet) is preserved between v8 and v9. The
v9 breaking changes (Pipeline thread-safety, hook interface, *redis.Z,
MaxConnAge, IdleTimeout, WithContext, Pipeline.Close) are not used by
the operator.

Required for native TLS support: v9 receives ongoing maintenance and
the tls.Config option remains available on Options.

Signed-off-by: Arsolitt <[email protected]>

Adopt vendor-neutral naming for container build files
(Dockerfile -> Containerfile).

- docker/app: bump golang:1.20-alpine -> golang:1.23-alpine3.20,
  pin alpine:latest -> alpine:3.20
- docker/development: bump golang:1.20-alpine -> golang:1.23-alpine3.20
  (code-generator and mockery versions left untouched; refresh is a
  separate concern)
- Makefile: point image build targets at the renamed files
- CI workflow:
  - fix push trigger to use the actual default branch (master, not main)
  - bump actions/checkout@v3 -> @v4, actions/setup-go@v4 -> @v5,
    golangci-lint-action@v3 -> @v6 (version v1.61 supports Go 1.23),
    medyagh/[email protected] -> @v0.0.18,
    azure/setup-helm@v3 -> @v4 (helm v3.15.4)
  - bump Kubernetes test matrix to 1.29/1.30/1.31 to match the k8s
    client-go version pulled in by the previous deps bump
  - expand short flags (--yes, --filename) per project convention

Signed-off-by: Arsolitt <[email protected]>

Introduce spec.tls to opt into TLS for the Redis failover cluster.
TLS is provisioned either by cert-manager (operator creates a
Certificate CR) or by referencing an existing Secret, never both.

CRD shape:

  spec.tls.enabled           bool
  spec.tls.authClients       "no" | "optional" | "yes" (default "no")
  spec.tls.certManager.issuerRef          cmmeta.IssuerReference
  spec.tls.certManager.secretName         string  (optional override)
  spec.tls.certManager.duration           *metav1.Duration
  spec.tls.certManager.renewBefore        *metav1.Duration
  spec.tls.certManager.privateKey         *cmapi.CertificatePrivateKey
  spec.tls.certificateSecret.secretName   string

Validation:
- enabled=true requires exactly one of certManager / certificateSecret
- certManager.issuerRef.name is required; kind defaults to "Issuer"
  and group to "cert-manager.io" when empty
- certificateSecret.secretName is required
- authClients must be one of "no"/"optional"/"yes" when set

Deps:
- github.com/cert-manager/cert-manager v1.15.5 (typed CRD API; the
  operator never requires cert-manager to be installed at runtime
  when TLS is disabled, the import only brings in Go types)

DeepCopy methods for TLSSettings, CertManagerSettings and
LocalSecretReference are added by hand (the project's existing
zz_generated file is hand-maintained rather than regenerated each
commit); the cert-manager imports needed for these methods are
also wired into the existing zz_generated file.

The operator does not yet consume the new fields. Subsequent commits
plumb the settings through to the Redis/Sentinel pods, the issued
Certificate resource and the operator's own redis client.

Signed-off-by: Arsolitt <[email protected]>

When spec.tls.enabled is true the operator now emits a configuration
that drives Redis (server, replication and Sentinel traffic) entirely
over TLS:

- redis.conf and sentinel.conf get a TLS block (port 0, tls-port, the
  three /tls/{tls.crt,tls.key,ca.crt} paths, tls-auth-clients from
  spec, tls-replication yes). The plaintext port is closed
- The Redis StatefulSet and Sentinel Deployment mount the TLS Secret
  read-only at /tls. The Secret name is resolved with a clear
  precedence: spec.tls.certificateSecret.secretName takes priority,
  then spec.tls.certManager.secretName, then a generated default
  (rftls-<name>) used in the cert-manager-managed path
- The shutdown.sh and ready.sh scripts in the operator-managed
  ConfigMaps inject the matching --tls --cert --key --cacert flags
  into every redis-cli invocation
- The inline exec probes (liveness on the redis container, both
  probes on the sentinel container) get the same flags
- The redis_exporter sidecars receive REDIS_EXPORTER_TLS_* env vars
  pointing at the mounted certificate; REDIS_ADDR switches to
  rediss:// so the exporter dials TLS as well

The operator does not yet create the cert-manager Certificate
resource; that lands in a separate commit. spec.tls.certificateSecret
(bring-your-own) is already wired through.

New tests under generator_tls_test.go cover the config files, the
volume + mount layout, the exporter env, the probe scripts and the
GetTLSSecretName precedence rules. With TLS disabled all existing
test fixtures continue to pass unchanged.

Signed-off-by: Arsolitt <[email protected]>

…vers

When spec.tls.enabled is true and spec.tls.certManager is set, the
operator now creates (and reconciles) a cert-manager Certificate that
issues into the Secret the pods already mount. The Certificate is a
no-op for the bring-your-own-secret mode (spec.tls.certificateSecret)
and obviously for TLS-disabled failovers.

Wiring:
- service/k8s: new Certificate service interface and CertificateService
  implementation talking to cert-manager's typed clientset; added to
  Services so existing call sites pick it up via embedding
- cmd: CreateKubernetesClients now also builds the cert-manager client;
  main.go passes it into k8s.New
- service generator: generateRedisCertificate builds a Certificate with
  the IssuerRef from spec.tls.certManager, the resolved Secret name,
  optional duration / renewBefore / privateKey overrides, and the
  ServerAuth + ClientAuth key usages
- ensurer: EnsureRedisCertificate runs before the ConfigMaps and pods
  so the Secret cert-manager populates exists by the time pods mount it

SAN coverage on the issued certificate:
- the four Service names (redis headless, redis master, redis slave,
  sentinel) each in short, namespaced, .svc and .svc.cluster.local form
- a wildcard against the redis headless service so per-pod DNS records
  (<pod>.rfr-<name>.<ns>.svc.cluster.local) validate without re-issuing
  when replica count changes
- no IP SANs — pod IPs are unstable; the operator client validates
  pod-IP-targeted dials via ServerName override against the headless
  DNS SAN

Mocks for Services and RedisFailoverClient are extended by hand with
the new methods (the project's mocks/ tree is hand-maintained between
mockery runs).

Tests:
- EnsureRedisCertificate creates a Certificate with the right SANs,
  issuer ref, secret name, duration/renewBefore and usages
- EnsureRedisCertificate is a no-op when TLS is disabled
- EnsureRedisCertificate is a no-op for bring-your-own-secret mode
- The existing TestEnsure suite now stubs EnsureRedisCertificate

The operator's internal redis client still uses plaintext; TLS in
service/redis/client.go lands in the next commit.

Signed-off-by: Arsolitt <[email protected]>

The operator's own redis/sentinel client (service/redis/client.go)
now dials TLS when the failover has spec.tls.enabled.

Approach: every Client method gains a final *tls.Config parameter
that is fed straight into rediscli.Options.TLSConfig. A nil config
preserves the historical plaintext behavior, so disabled-TLS clusters
are unaffected.

Plumbing:
- service/redis/tls.go: new BuildTLSConfig helper. Loads the CA out
  of the ca.crt key of the cluster Secret (the one cert-manager or
  the user populates), produces a tls.Config with the supplied
  ServerName for SAN-based validation, MinVersion TLS 1.2, no
  InsecureSkipVerify.
- operator/redisfailover/service/check.go: new tlsConfigFor helper
  reads the Secret named by GetTLSSecretName and feeds BuildTLSConfig
  with ServerName = GetRedisName(rf) (the redis headless service,
  which is the SAN every issued cert covers).
- check.go and heal.go: every method that already loaded the password
  now also calls tlsConfigFor and passes the resulting config to the
  client. tlsConfigFor returns nil for TLS-disabled failovers so the
  pre-existing call sites keep their plaintext semantics.

Interface changes:
- service/redis.Client: each method gains a trailing *tls.Config
- service.RedisFailoverCheck.CheckSentinelMonitor: gains a leading
  *redisfailoverv1.RedisFailover so tlsConfigFor has access to it
- service.RedisFailoverHeal.RestoreSentinel: same, leading rf

Callers in operator/redisfailover/checker.go updated to pass rf.

Why ServerName, not InsecureSkipVerify: pod IPs are unstable, but the
issued cert covers the headless service DNS name. Dialing pod IP +
ServerName override gives strong validation against the SAN list
without juggling IP SANs that would churn on every reschedule.

Mocks (mocks/service/redis/Client.go, the two RedisFailover* mocks)
are updated by hand to match the new signatures. The check_test and
heal_test fixtures are updated en masse (mock.Anything for the new
trailing tlsConfig arg). New tests cover BuildTLSConfig: nil/empty
CA, missing ServerName, malformed PEM, success path.

Signed-off-by: Arsolitt <[email protected]>

…xample

The operator now requires write access to cert-manager Certificate
resources whenever a RedisFailover opts into TLS with the
spec.tls.certManager block. Grant create/delete/get/list/patch/
update/watch on cert-manager.io/certificates from the operator's
ClusterRole. The rule is always present; the operator only issues
requests when a failover has TLS enabled, so installations that do
not have cert-manager installed remain unaffected.

Add example/redisfailover/tls.yaml that demonstrates the cert-manager
managed path and documents the bring-your-own-Secret alternative in
a comment.

Signed-off-by: Arsolitt <[email protected]>

Bump to the current upstream stable versions:
- Go directive: 1.26.0 (was 1.23). The previous CODEGEN_IMAGE could
  not parse three-digit minor versions, which broke generate-crd.
- k8s.io/{api,apimachinery,client-go,apiextensions-apiserver}:
  0.31.0 -> 0.36.1
- kooper: 2.7.0 -> 2.10.0
- cert-manager and prometheus client_golang were already at latest
  via earlier transitive bumps

Update the codegen tooling:
- CODEGEN_IMAGE: slok/kube-code-generator v1.27.0 -> v0.9.0. The
  upstream project rewrote its CLI in the v0 line (drop update-*.sh
  scripts, single Go binary configured with flags), so the Makefile
  targets are rewritten to call it directly via flags rather than
  through env-vars + a bash wrapper. The new image's bundled Go is
  older than our go.mod directive, so GOTOOLCHAIN=auto is set on the
  docker run to let it download a matching toolchain on demand.

Regenerate the CRD against the new TLSSettings types so installs
through helm/kubectl pick up the new spec.tls fields out of the box.
The chart's crds/ copy and the kustomize base copy are also updated
by the same generate-crd target.

Drive-by: fix non-constant format string in GetNumberMasters which
go vet started flagging under newer toolchains.

Signed-off-by: Arsolitt <[email protected]>

Adds an env: [] knob to charts/redisoperator/values.yaml and renders the
corresponding env block in the Deployment when set. Documents
KUBE_FEATURE_WatchListClient=false as the workaround when running
against API servers that do not deliver BOOKMARK events on the initial
list — without it the controller hangs forever in
"Waiting for caches to sync".

Signed-off-by: Arsolitt <[email protected]>

Aligns the operator image with the go 1.26 toolchain required by go.mod
and refreshes the alpine base in both the build and runtime stages.
Earlier golang:1.23-alpine3.20 / alpine:3.20 stack could no longer
compile after the dependency bump to k8s 0.36 / cert-manager 1.20.

Signed-off-by: Arsolitt <[email protected]>

Adds spec.tls.certManager.extraSANs: a list of additional Subject
Alternative Names appended to the service and per-pod SANs the operator
already computes. Entries are auto-classified by net.ParseIP — IPv4 and
IPv6 strings become IP SANs, everything else becomes a DNS SAN. The
merge is purely additive so in-cluster connectivity is preserved when
operators add external hostnames or LoadBalancer VIPs for off-cluster
clients.

Includes regenerated deepcopy (now produced by controller-gen, which
also reformats prior entries) and the regenerated CRD manifest in all
three published locations: chart crds, kustomize base, and manifests
root.

The go directive moves from 1.26.0 to 1.26.3 as a side-effect of
running the generator under that toolchain.

Signed-off-by: Arsolitt <[email protected]>

slok/kube-code-generator v0.9.0 dropped the explicit --client-package
flag; the generator now infers the client package from --go-gen-out
combined with the go.mod module path. Passing the removed flag caused
update-codegen to fail with "unknown long flag '--client-package'".

Signed-off-by: Arsolitt <[email protected]>

Re-runs make update-codegen and make generate-crd against the canonical
toolchain after the Makefile fix. The diff captures three shifts:

  - deepcopy: refreshed by deepcopy-gen, replacing the temporary
    controller-gen output that landed alongside the extraSANs change.
    Format and import aliases match the upstream style again.
  - clientset: regenerated; the public surface contracts in Get/List/
    Watch/Create/Update/Patch are unchanged, but the typed fake clients
    shrunk to the generic client-go testing wrappers and a stale doc.go
    is dropped.
  - informers / listers: newly generated tree under client/k8s. Not
    consumed by the operator itself yet, but kept versioned to keep
    downstream consumers and future controller refactors free of an
    extra generation step.
  - CRD manifest: regenerated and synced to manifests/, kustomize base
    and the chart crds directory.

Signed-off-by: Arsolitt <[email protected]>

Apply goimports with -local=github.com/spotahome/redis-operator and the
gofmt struct-literal alignment that golangci-lint v2 enforces by default.
Pure formatting — no behaviour or API changes.

The realignments in service/redis/client.go are a follow-up to the TLS
plumbing change (62ac27b): adding TLSConfig to the existing rediscli
options literals shifted the longest field name, which gofmt then expects
all sibling colons to track.

Signed-off-by: Arsolitt <[email protected]>

golangci-lint v2 reorganised its CLI and configuration schema. Inline
flags like '-E goimports --timeout 3m' no longer work, and v2's default
'standard' linter set bundles many staticcheck families (QF/ST/S1/SA1019)
that the previous v1.61 default excluded. Out-of-the-box this would
surface a flood of issues in upstream code that pre-dates this fork and
is unrelated to the work happening here.

Lock the staticcheck check list down to the SA* bug-class families (minus
SA1019, since the kooper-style SlaveOf usage in service/redis/client.go
is deliberate) so the noise level matches what v1.61 produced. Move the
in-repo errcheck exception for upstream's deferred rClient.Close() calls
into the same config. Goimports becomes a v2 'formatter' rather than a
'-E' flag, configured with the project's local prefix.

scripts/check.sh stops passing v1-only flags and just delegates to the
new config.

Signed-off-by: Arsolitt <[email protected]>

The fork now builds against Go 1.26 and client-go 0.36, which makes the
CI pipeline that shipped with the upstream repository unusable:

  * golangci-lint v1.61 predates Go 1.26 and rejects the toolchain. Bump
    to v2.12.2 with the matching golangci/golangci-lint-action@v9 (v2
    requires the action's major version 9 or newer).

  * The integration-test matrix targeted Kubernetes 1.29/1.30/1.31. With
    client-go 0.36, the WatchListClient feature gate is enabled by
    default, and against API servers older than 1.36 the initial events
    stream never delivers the BOOKMARK event the client waits for —
    informers hang on cache sync until the job times out. Bump the
    matrix to 1.34.8 / 1.35.5 / 1.36.1 (current supported releases) and
    pass KUBE_FEATURE_WatchListClient=false to the integration-test step
    so older API servers in the matrix still pass.

  * Refresh setup-minikube, setup-helm, setup-go, and actions/checkout
    to their current major versions; bump the helm CLI used by the
    chart-test job to v3.21.0.

Release Charts workflow gets the same setup-helm/checkout/helm CLI
bumps plus chart-releaser-action v1.7.0 so it keeps releasing chart
artefacts after the toolchain upgrade.

Signed-off-by: Arsolitt <[email protected]>

Address review feedback from gemini-code-assist on
service/redis/tls.go:22, service/redis/tls.go:37, and
operator/redisfailover/service/check.go:38:
the operator only loaded the CA bundle from the TLS Secret, so its
internal Redis client could connect to a server that ran in
non-mutual-TLS mode (tls-auth-clients: no) but had no way to satisfy
servers that ran in mutual mode — the handshake would be rejected with
no client certificate to validate.

BuildTLSConfig now takes optional certPEM/keyPEM PEM blobs and attaches
them as tls.Config.Certificates when both are supplied; supplying only
one is rejected. tlsConfigFor reads the leaf certificate and key from
the same cert-manager Secret (the tls.crt / tls.key entries already
populated by the issued Certificate, which carries serverAuth and
clientAuth EKUs) when spec.tls.authClients is yes, and fails fast with
a descriptive error if either entry is missing or empty.

Unit tests cover the new branches: client cert loaded, mismatched
cert/key arguments, and malformed cert/key PEMs.

Signed-off-by: Arsolitt <[email protected]>

Address review feedback from coderabbitai on docker/development/Containerfile:1:
the dev image still pinned golang:1.23-alpine3.20 while go.mod targets
go 1.26.3 and docker/app/Containerfile already used golang:1.26.3-alpine3.23.
Match the dev image so local builds, codegen and lint share the same
toolchain as the production image.

Signed-off-by: Arsolitt <[email protected]>

Address review feedback from coderabbitai on .github/workflows/ci.yaml:23
and .github/workflows/helm.yml:14:

- Pin every third-party action to an immutable commit SHA (with the
  resolved version pinned in a trailing comment for human readability).
  Tag references are mutable and let a compromised action push
  arbitrary code into the workflow on the next run.
- Add a top-level permissions block to each workflow. CI runs with the
  minimum contents: read; the Release Charts workflow needs
  contents: write because chart-releaser-action pushes tags, releases
  and the gh-pages branch. Both replace GitHub's default broad
  GITHUB_TOKEN scope.
- Set persist-credentials: false on every actions/checkout step so the
  short-lived GITHUB_TOKEN is not left behind in .git/config for later
  steps to abuse.

Signed-off-by: Arsolitt <[email protected]>

Update test/integration/redisfailover/creation_test.go call sites that
the TLS work changed but did not migrate:

- CreateKubernetesClients now returns the cert-manager client too;
  destructure the 5th return into cmClient.
- k8s.New takes that cert-manager client as an extra argument.
- IsMaster and GetSentinelMonitor now require a *tls.Config; pass nil
  since the integration suite still talks to Redis in plaintext.

Restores `go test -tags=integration` compile so the integration matrix
turns green again across all three Kubernetes versions.

Signed-off-by: Arsolitt <[email protected]>

CreateOrUpdateService used to overwrite the live Service's metadata
with whatever the operator generated, dropping any labels and
annotations that other actors (admission webhooks, service meshes,
cluster operators stamping metadata) had attached. Merge the stored
metadata with the operator-managed metadata before issuing the update
so external annotations survive every reconcile.

This is the patch we have been carrying as an out-of-tree fix on
in-cluster builds; folding it into the fork so the same image works
without a sidecar patch step.

Signed-off-by: Arsolitt <[email protected]>

Address review feedback from coderabbitai on service/redis/client.go:290:
MakeSlaveOfWithPort dialed the local Redis instance at
net.JoinHostPort(ip, masterPort) and sent SLAVEOF masterIP masterPort,
collapsing two distinct ports into one. In same-cluster failover the
two are equal so the bug was dormant, but SetExternalMasterOnAll
threads the external master's port through this API — when that port
differs from the local Redis port the operator dials the local pod
on the wrong port and the slave-of reconfig fails for every external
master takeover.

The function now takes localPort and masterPort as independent
arguments: the dial uses ip:localPort, the SLAVEOF command keeps
masterIP:masterPort. SetExternalMasterOnAll resolves localPort from
spec.Redis.Port before iterating pods; the same-cluster callers
(SetOldestAsMaster, SetMasterOnAll) pass the same port for both
values since their masterPort always equals localPort.

This supersedes the original WONTFIX on PR #1 review thread 12 — the
fix turned out cheap enough to fold into the TLS branch rather than
defer.

Signed-off-by: Arsolitt <[email protected]>

Address review feedback from coderabbitai on
test/integration/redisfailover/creation_test.go:227:
the loop in testRedisMaster discarded the IsMaster error, so a
connectivity, auth or replication-state failure on any single probe
would silently leave that pod out of the master count, masking the
underlying failure mode. Assert NoError and skip the pod on error so
the failure surfaces in the test report instead of producing a wrong
count downstream.

Signed-off-by: Arsolitt <[email protected]>

Address review feedback from coderabbitai on
test/integration/redisfailover/creation_test.go:250:
GetSentinelMonitor errors were silently dropped in the sentinel
monitoring loop, and the test then read masters[0] unconditionally —
both a transient probe failure and an unexpectedly empty sentinel
list would surface as a confusing index-out-of-range panic rather
than a clear assertion. Assert NoError on each probe, skip the pod
when it fails, and bail out with assert.NotEmpty before indexing.

Signed-off-by: Arsolitt <[email protected]>

Address review feedback from lexfrei (Blocker B2) on
operator/redisfailover/service/generator.go: the FQDN SAN entries
hardcoded ".svc.cluster.local", so on any Kubernetes cluster running
with a non-default --cluster-domain (cozystack defaults to
"cozy.local"; on-prem clusters routinely use "cluster.internal" or a
site-specific value) the cert-manager Certificate produced by the
operator-managed path did not cover the FQDNs clients actually
resolve, and TLS verification failed for every routed FQDN dial.

The bring-your-own-Secret path was unaffected because the chart
renders its own Certificate with the configurable domain, but the
operator-managed path stayed broken for non-default clusters.

Plumbing:

- New `--cluster-domain` CLI flag (default "cluster.local") on the
  operator binary.
- Threaded through the existing `redisfailover.Config` struct, the
  factory, and `NewRedisFailoverKubeClient`, which now takes the
  cluster domain as a constructor argument and stores it on the
  client.
- `generateRedisCertificate` and `redisCertificateSANs` accept the
  cluster domain and template the `.svc.<domain>` FQDN SAN entries
  accordingly. An empty value falls back to "cluster.local" so
  deployments that never set the flag keep working.

Tests:

- All `NewRedisFailoverKubeClient` test callers updated to pass the
  default "cluster.local" — purely mechanical.
- New TestEnsureRedisCertificateUsesConfiguredClusterDomain asserts
  that with --cluster-domain=cozy.local the FQDN SANs use ".cozy.local"
  and that the default ".cluster.local" form is NOT silently added in
  addition (which would mask SAN-mismatch bugs).

Signed-off-by: Arsolitt <[email protected]>