Skip to content

fix(chart): webhook cert SAN must use .Release.Namespace, not .Values.service.namespace - #1828

Open
grkml wants to merge 1 commit into
OT-CONTAINER-KIT:mainfrom
grkml:fix/webhook-cert-san-release-namespace
Open

fix(chart): webhook cert SAN must use .Release.Namespace, not .Values.service.namespace#1828
grkml wants to merge 1 commit into
OT-CONTAINER-KIT:mainfrom
grkml:fix/webhook-cert-san-release-namespace

Conversation

@grkml

@grkml grkml commented Jun 30, 2026

Copy link
Copy Markdown

Description

The redis-operator chart builds the cert-manager Certificate dnsNames from .Values.service.namespace (default redis-operator), but the webhook Service and the MutatingWebhookConfiguration clientConfig both use .Release.Namespace:

Template Namespace source Resolves to (release ns = ot-operators)
templates/cert-manager.yaml (Certificate SAN) .Values.service.namespace webhook-service.redis-operator.svc
templates/service.yaml (Service) .Release.Namespace webhook-service.ot-operators.svc
templates/mutating-webhook-configuration.yaml (clientConfig) .Release.Namespace webhook-service.ot-operators.svc

When the chart is installed into any namespace other than the service.namespace default, the Certificate SAN no longer matches the DNS name the API server actually dials, so the webhook TLS handshake fails the SAN check. Because the mutating webhook is failurePolicy: Fail, this is silent at install time and only surfaces as a 503 (x509/SAN error) on the first Redis CR admission.

This PR pins the SAN to .Release.Namespace so it always matches the Service the webhook is reached at.

Reproduction

```console
$ helm template redis-operator charts/redis-operator -n ot-operators
--set redisOperator.webhook=true --set certmanager.enabled=true
```

Before — Certificate SAN vs. webhook Service disagree:
```yaml

Certificate

dnsNames:

  • webhook-service.redis-operator.svc # .Values.service.namespace

MutatingWebhookConfiguration -> clientConfig.service

name: webhook-service
namespace: ot-operators # .Release.Namespace
```

After this change — they match:
```yaml
dnsNames:

  • webhook-service.ot-operators.svc
  • webhook-service.ot-operators.svc.cluster.local
    ```

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • Functionality/bugs have been confirmed to be unchanged or fixed (verified via helm template into a non-default namespace).
  • I have performed a self-review of my own code.
  • Tests have been added/modified and all tests pass. (chart template change; no unit tests exist for the cert template — happy to add a helm unittest case if the maintainers want one.)
  • Documentation updated where necessary.

Additional Context

After this change .Values.service.namespace is no longer referenced by any template. I left the value in place to keep this a minimal, non-breaking fix — glad to remove it (and the README row) in a follow-up if you'd prefer.

….service.namespace

The cert-manager Certificate's dnsNames were built from
`.Values.service.namespace` (default `redis-operator`), while the webhook
Service (templates/service.yaml) and the MutatingWebhookConfiguration
clientConfig (templates/mutating-webhook-configuration.yaml) both use
`.Release.Namespace`.

When the chart is installed into any namespace other than the
`service.namespace` default, the Certificate SAN becomes
`webhook-service.<service.namespace>.svc` but the API server connects to
`webhook-service.<release-namespace>.svc`, so the TLS SAN check fails. With
the mutating webhook's failurePolicy=Fail this surfaces as a 503 on the
first Redis CR admission — silent until then.

Pin the SAN to `.Release.Namespace` so it always matches the Service the
webhook is actually reached at.

Note: `service.namespace` (values.yaml) is now unused. Left in place to keep
this a minimal non-breaking fix; happy to drop it in a follow-up if preferred.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@iamabhishek-dubey

Copy link
Copy Markdown
Member

Please work on DCO check for commit sign off

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants