Panic when clientCertificate configuration is not set in Datastore object definition #732
Description
Context
I'm making some basic tests with Kamaji. In one of them, I tried to create a new Datastore object pointing to an external ECTD cluster. In the definition of the CRD, I did not specify the configuration for clientCertificate param. Because of that, the admission webhook crashed with a panic.
Datastore definition:
apiVersion: kamaji.clastix.io/v1alpha1
kind: DataStore
metadata:
name: tenant01-cluster01
namespace: tenant01
spec:
driver: etcd
endpoints:
- kamaji-etcd-0.tenant01.svc.cluster.local:2379
- kamaji-etcd-1.tenant01.svc.cluster.local:2379
- kamaji-etcd-2.tenant01.svc.cluster.local:2379
tlsConfig:
certificateAuthority:
certificate:
secretReference:
keyPath: ca.crt
name: kamaji-etcd-certs
namespace: tenant01
privateKey:
secretReference:
keyPath: ca.key
name: kamaji-etcd-certs
namespace: tenant01The definition of the CRD can lead to confusion since the parameter is not shown as mandatory. However, the elements that depend on it are indeed required:
CRD visualisation is available through this webpage:
https://doc.crds.dev/github.com/clastix/kamaji/kamaji.clastix.io/DataStore/v1alpha1@edge-24.12.1
Kamaji Controller Logs
2025-03-20T08:25:51Z ERROR admission Observed a panic {"object": {"name":"tenant01-cluster01"}, "namespace": "", "name": "tenant01-cluster01", "resource": {"group":"kamaji.clastix.io","version":"v1alpha
runtime.sigpanic
runtime/signal_unix.go:917
github.com/clastix/kamaji/internal/webhook/handlers.DataStoreValidation.validateTLSConfig
github.com/clastix/kamaji/internal/webhook/handlers/ds_validate.go:107
github.com/clastix/kamaji/internal/webhook/handlers.DataStoreValidation.validate
github.com/clastix/kamaji/internal/webhook/handlers/ds_validate.go:71
github.com/clastix/kamaji/internal/webhook/handlers.(*DataStoreValidation).OnCreate.DataStoreValidation.OnCreate.func1
github.com/clastix/kamaji/internal/webhook/handlers/ds_validate.go:31
github.com/clastix/kamaji/internal/webhook.Register.handlersChainer.Handler.func1.1
github.com/clastix/kamaji/internal/webhook/chainer.go:44
github.com/clastix/kamaji/internal/webhook.Register.handlersChainer.Handler.func1
github.com/clastix/kamaji/internal/webhook/chainer.go:61
sigs.k8s.io/controller-runtime/pkg/webhook/admission.HandlerFunc.Handle
sigs.k8s.io/controller-runtime@v0.19.3/pkg/webhook/admission/webhook.go:114
sigs.k8s.io/controller-runtime/pkg/webhook/admission.(*Webhook).Handle
sigs.k8s.io/controller-runtime@v0.19.3/pkg/webhook/admission/webhook.go:181
sigs.k8s.io/controller-runtime/pkg/webhook/admission.(*Webhook).ServeHTTP
sigs.k8s.io/controller-runtime@v0.19.3/pkg/webhook/admission/http.go:119
sigs.k8s.io/controller-runtime/pkg/webhook/internal/metrics.InstrumentedHook.InstrumentHandlerInFlight.func1
github.com/prometheus/client_golang@v1.19.1/prometheus/promhttp/instrument_server.go:60
net/http.HandlerFunc.ServeHTTP
net/http/server.go:2220
github.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerCounter.func1
github.com/prometheus/client_golang@v1.19.1/prometheus/promhttp/instrument_server.go:147
net/http.HandlerFunc.ServeHTTP
net/http/server.go:2220
github.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerDuration.func2
github.com/prometheus/client_golang@v1.19.1/prometheus/promhttp/instrument_server.go:109
net/http.HandlerFunc.ServeHTTP
net/http/server.go:2220
net/http.(*ServeMux).ServeHTTP
net/http/server.go:2747
net/http.serverHandler.ServeHTTP
net/http/server.go:3210
net/http.(*conn).serve
net/http/server.go:2092
Current behaviour
If clientCertificate param is not set, an unexpected panic occur instead of returning an error.
Expected behaviour
If clientCertificate param is mandatory, it should be shown as such in the CRD definition. In addition to this, we may need to check if the parameter is set before validating its children attributes. If not set, an error should be returned, like:
return fmt.Errorf("Client certificate is required")Additional comments
Before creating a PR, I would like to know your feedback about this issue. If required, I can submit the fix.
Thanks!