etcd-io / etcd-io/etcd-operator

EtcdCluster admission webhooks are scaffolded but never registered

Open
#380 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
164
Forks
72
Avg merge
22h 46m
Merged PRs (30d)
34

Description

## Summary

The operator wires a `webhook.NewServer(...)` into the manager in `cmd/main.go`, and the kustomize scaffolding for `config/webhook` + `config/certmanager` markers exists in `config/default/kustomization.yaml` and `config/crd/kustomization.yaml`. However, **no admission webhook is actually implemented or registered** for `EtcdCluster`. The webhook server starts but serves nothing.

As a result, malformed `EtcdCluster` specs are only caught (if at all) later during reconciliation — e.g. an even `size` (which can never form a quorum), an unparseable `version`, an unsupported skip-minor upgrade/downgrade, or an incoherent TLS surface. The user gets no synchronous, actionable feedback at `kubectl apply` time.

Today the controller logs upgrade-path problems during reconcile (`validateEtcdUpgradePath` in `internal/controller/utils.go`) but still proceeds; there is no admission-time gate.

## Proposal

Implement and **register** admission webhooks for `EtcdCluster`:

**Validating** (`ValidateCreate` / `ValidateUpdate`):
- Reject even `size` (etcd needs an odd member count for a stable quorum).
- Reject `size < 1`.
- Validate `version` parses as semver and (on update) that the current→target transition is a supported single-minor, non-downgrade path — reusing the existing upgrade-path logic.
- Validate TLS surface coherence (provider must be one of the known providers; the matching provider config block must be present; cert-manager provider requires `issuerKind`/`issuerName`).
- Immutable-field guards (e.g. `storageSpec` is immutable once set).

**Defaulting** (`Default`):
- Default an empty `tls.provider` to `auto` when a TLS block is present.

Every rejection must carry a crisp, actionable message telling the user exactly what is wrong and how to fix it.

Wire `SetupWebhookWithManager` in `cmd/main.go`, regenerate `config/webhook` + `config/certmanager` via `make manifests`, and add unit + envtest + e2e coverage (including assertions on the exact error-message text).

I'll open a PR referencing this issue.

Contributor guide

Open the contributing guide

Research direction

Start in cmd/main.go to inspect the existing webhook server wiring, then read internal/controller/utils.go and the kustomization files in config/default and config/crd. Implement and register EtcdCluster validating and defaulting webhooks, regenerate manifests with make manifests, and add the requested unit, envtest, and e2e coverage with exact error-message assertions.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend-api-design, devops
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.