diff --git a/DESIGN.md b/DESIGN.md index 6520e92..657ad74 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -465,7 +465,8 @@ when nothing ever crosses into a tenant namespace in the first place. the same rigor §5's general idempotent-create rule already applies elsewhere: - `status.credentialsSecretRef` already set: done, nothing to do. - - Otherwise, check for an intermediate `-bootstrap-admin` Secret in + - Otherwise, check for an intermediate + `.-bootstrap-admin` Secret in the operator's own namespace first. If it exists, its key is a still- valid admin credential from an earlier, interrupted attempt — skip `/api/bootstrap` entirely and reuse it. If not, call `/api/bootstrap` diff --git a/ROADMAP.md b/ROADMAP.md index d575cbd..6f9a607 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -66,22 +66,35 @@ New commits build forward over the old ones; no git history rewrite. once the Deployment has a ready replica, checkpoint the admin key, mint the instance-scoped service account, generated credentials Secret in the operator's own namespace, `status.credentialsSecretRef`. Finalizer cleans - up that Secret and the server-side rows on delete (now actually needed, - unlike the superseded version — this stage creates things server-side). + up that Secret (and the checkpoint, if one's still there) on delete — + there's no server-side row to clean up alongside it: terdut-server's API + has no way to delete a user or a service account, only to revoke + individual keys, so there's nothing to undo there regardless. - RBAC: read-only watch on `postgresql.acid.zalan.do`, degrading gracefully if that CRD isn't installed (§8, §9). +- Shipped, scoped down from §8's full ambition in two ways, both called out + in code rather than silently dropped: no live watch on the Zalando- + generated credentials Secret for rotation (relies on the periodic resync + to notice eventually, higher latency than a watch); no Gateway API + `HTTPRoute` creation from `spec.networking.hostname`/`gatewayListener` + (needs the Gateway API types as a new dependency, and nothing about + proving a `TerdutServer` boots and bootstraps a real server depends on + external ingress existing). Both are near-term follow-ups, not deferred + to a later stage. - `envtest` covering Deployment/Service reconciliation and both database paths — the Zalando path needs that CRD's schema vendored into the test environment (there's no real `postgres-operator` controller in `envtest`, only the CRD shape to create fixture objects against) — plus the self-registration flow against an `httptest.Server` fake of - `/api/bootstrap` and `/api/service-accounts` (§11), this time exercising - the real flow rather than an adopt-only stand-in. -- A real `kind` end-to-end pass (bring-your-own DSN is simplest there) to - prove a `TerdutServer` CR actually produces a running, bootstrapped - terdut-server pod. The Zalando path can additionally be validated for - real against the org's own cluster later, where `postgres-operator` - already runs, rather than only in a disposable `kind` stand-in. + `/api/bootstrap` and `/api/service-accounts` (§11), including the + adopt-on-409 recovery path and the one fail-closed case + (`BootstrapStateLost`), not just the happy path. +- A real `kind` end-to-end pass is still open (bring-your-own DSN is + simplest there) to prove a `TerdutServer` CR actually produces a running, + bootstrapped terdut-server pod against a real cluster, not just envtest's + fake API server. The Zalando path can additionally be validated for real + against the org's own cluster later, where `postgres-operator` already + runs, rather than only in a disposable `kind` stand-in. ## Stage 2 — `TerdutTeam` diff --git a/api/v1alpha1/terdutserver_types.go b/api/v1alpha1/terdutserver_types.go index 2b66b76..178ed99 100644 --- a/api/v1alpha1/terdutserver_types.go +++ b/api/v1alpha1/terdutserver_types.go @@ -5,13 +5,12 @@ import ( "k8s.io/apimachinery/pkg/runtime" ) -// SecretKeyRef names one data key inside a Secret. Unlike a typical -// cross-namespace reference, there is deliberately no namespace field here: -// every Secret this type points at (DESIGN.md §6) lives in the operator's -// own namespace, always, by construction — never anywhere else, so there is -// nothing for a namespace field to vary. What does vary is the key: fixed -// ("token") when the controller generated the Secret itself, whatever a -// human chose when it was handed to the controller instead. +// SecretKeyRef names one data key inside a Secret. Every use of this type in +// TerdutServerSpec resolves in the TerdutServer's own namespace (it's wired +// straight into the Deployment's pod spec as a secretKeyRef env source, +// which Kubernetes itself only allows same-namespace) -- unlike the +// generated credentials Secret (DESIGN.md §6), which always lives in the +// operator's own namespace and is never referenced through this type. type SecretKeyRef struct { // name is the Secret's name. // +kubebuilder:validation:MinLength=1 @@ -22,6 +21,148 @@ type SecretKeyRef struct { Key string `json:"key"` } +// ImageSpec is the terdut-server image to run. +type ImageSpec struct { + // +kubebuilder:validation:MinLength=1 + Repository string `json:"repository"` + // +kubebuilder:validation:MinLength=1 + Tag string `json:"tag"` +} + +// NetworkingSpec is how this TerdutServer is reached from outside the +// cluster. +// +// hostname/gatewayListener describe the intended Gateway API HTTPRoute +// (matching charts/terdut-server's own templates/httpproxy.yaml, despite its +// name — that chart carries a Gateway API HTTPRoute, not a Contour +// HTTPProxy), but creating that HTTPRoute isn't implemented yet: it needs +// the Gateway API types as a new dependency, and nothing about proving a +// TerdutServer boots and bootstraps a real server depends on external +// ingress existing. Tracked as a near-term follow-up, not deferred to a +// later ROADMAP.md stage the way Deployment/database/bootstrap once were. +type NetworkingSpec struct { + // hostname the HTTPRoute will carry once it exists. + // +optional + Hostname string `json:"hostname,omitempty"` + + // servicePort is both the Service's port and the HTTPRoute's backend + // port once it exists. Defaults to 8080, matching the chart's own + // service.port default. + // +kubebuilder:default=8080 + // +optional + ServicePort int32 `json:"servicePort,omitempty"` + + // gatewayListener is the HTTPRoute's sectionName once it exists. Empty + // attaches to every matching listener, including plaintext HTTP. + // +optional + GatewayListener string `json:"gatewayListener,omitempty"` +} + +// PostgresClusterRef names a Zalando postgres-operator `postgresql` CR +// (`acid.zalan.do/v1`) in the same namespace (DESIGN.md §8). By convention +// — matching every example in DESIGN.md and the chart's own — the database +// and role this operator consumes from it are both named "terdut"; this +// isn't configurable in v1 (there is no field for it because the design +// doesn't have one yet, not an oversight here). +type PostgresClusterRef struct { + // +kubebuilder:validation:MinLength=1 + Name string `json:"name"` +} + +// DatabaseSpec is the Postgres connection this TerdutServer uses. Exactly +// one of dsn or postgresClusterRef must be set (DESIGN.md §8) — this +// operator provisions no database either way, only wires up one that +// exists. +// +kubebuilder:validation:XValidation:rule="(has(self.dsn) ? 1 : 0) + (has(self.postgresClusterRef) ? 1 : 0) == 1",message="exactly one of dsn or postgresClusterRef must be set" +type DatabaseSpec struct { + // dsn is a DSN with no password in it, e.g. + // "postgres://terdut@terdut-postgres:5432/terdut?sslmode=require" -- + // mutually exclusive with postgresClusterRef. + // +optional + DSN string `json:"dsn,omitempty"` + + // passwordSecretRef is where PGPASSWORD comes from for the dsn path. + // pgx falls back to libpq's environment variables for anything the DSN + // omits, so the password never appears in the DSN string itself. Unused + // on the postgresClusterRef path -- the Zalando-generated Secret is + // wired in directly instead. + // +optional + PasswordSecretRef *SecretKeyRef `json:"passwordSecretRef,omitempty"` + + // postgresClusterRef names a Zalando postgres-operator CR instead of a + // plain DSN -- mutually exclusive with dsn. + // +optional + PostgresClusterRef *PostgresClusterRef `json:"postgresClusterRef,omitempty"` +} + +// SweeperSpec controls incident auto-resolve/archive timing. Values are +// Go duration strings (e.g. "6h"), passed straight through to the +// TERDUT_STALE_AFTER/TERDUT_ARCHIVE_AFTER env vars exactly as written -- +// not a structured metav1.Duration, since terdut-server parses them itself +// and a round-trip through a different type would buy nothing. +type SweeperSpec struct { + // +optional + StaleAfter string `json:"staleAfter,omitempty"` + // +optional + ArchiveAfter string `json:"archiveAfter,omitempty"` +} + +// DeadmanSpec controls dead man's switch alerts. Matchers/Timeout/Severity +// map straight to TERDUT_DEADMAN_MATCHERS/TERDUT_DEADMAN_TIMEOUT/ +// TERDUT_DEADMAN_SEVERITY. +type DeadmanSpec struct { + // +optional + Matchers string `json:"matchers,omitempty"` + // +optional + Timeout string `json:"timeout,omitempty"` + // +optional + Severity string `json:"severity,omitempty"` +} + +// NotifySpec controls push notifications via ntfy. Empty ntfyURL disables +// notifications entirely (matches the chart's own default). +type NotifySpec struct { + // +optional + NtfyURL string `json:"ntfyURL,omitempty"` + // +optional + FallbackTopic string `json:"fallbackTopic,omitempty"` + // +optional + RepeatEvery string `json:"repeatEvery,omitempty"` + // tokenSecretRef is an optional bearer token for an access-controlled + // ntfy. Leave unset for an open ntfy. + // +optional + TokenSecretRef *SecretKeyRef `json:"tokenSecretRef,omitempty"` +} + +// OIDCSpec controls single sign-on. Fields the chart also exposes but +// DESIGN.md's spec doesn't (usernameClaim, emailClaim, groupsClaim, +// trustEmail) use terdut-server's own defaults +// (preferred_username/email/groups/false) rather than being added here +// speculatively. +type OIDCSpec struct { + // +optional + Enabled bool `json:"enabled,omitempty"` + // +optional + Issuer string `json:"issuer,omitempty"` + // +optional + ClientID string `json:"clientID,omitempty"` + // +optional + ClientSecretRef *SecretKeyRef `json:"clientSecretRef,omitempty"` + // +kubebuilder:default="SSO" + // +optional + Name string `json:"name,omitempty"` + // +kubebuilder:default="openid profile email" + // +optional + Scopes string `json:"scopes,omitempty"` + // +optional + AllowedGroups []string `json:"allowedGroups,omitempty"` + // +optional + AdminGroup string `json:"adminGroup,omitempty"` + // +kubebuilder:default="12h" + // +optional + SessionMaxAge string `json:"sessionMaxAge,omitempty"` +} + // AllowedTeamsNamespaces gates which namespaces a TerdutTeam may resolve a // cross-namespace serverRef into this TerdutServer from (DESIGN.md §4.6). // Same-namespace TerdutTeams are always allowed, regardless of this field. @@ -51,41 +192,49 @@ type AllowedTeams struct { // TerdutServerSpec defines the desired state of TerdutServer. // -// Narrowed to Stage 1 (ROADMAP.md): this covers only what the bootstrap/ -// credentials flow (DESIGN.md §6) needs against an already-running, -// already-bootstrapped terdut-server. The fields that would have the -// controller manage a Deployment/Service/database — image, replicas, -// networking, database — are deferred to Stage 5 and deliberately absent -// here, not an oversight; adding them later is additive, not a breaking -// change to this shape. +// The operator creates and owns every TerdutServer it manages (DESIGN.md +// §1) -- there is no bring-your-own-install path. This is the full shape +// from DESIGN.md §4.1: Deployment, Service, database, bootstrap and +// credentials are all built together (ROADMAP.md Stage 1), not staged +// separately the way an earlier version of this design did. type TerdutServerSpec struct { - // endpoint is the base URL of an already-running terdut-server this - // TerdutServer represents, e.g. "http://terdut.oncall.svc:8080". This - // stage never creates or manages a Deployment/Service for it — the - // server is expected to already exist, deployed some other way (its own - // Helm chart, by hand). // +required - // +kubebuilder:validation:MinLength=1 - Endpoint string `json:"endpoint"` + Image ImageSpec `json:"image"` - // credentialsSecretRef names a Secret, in the operator's own namespace, - // that a human has already created by minting an instance-scoped service - // account with their own admin session (POST /api/service-accounts, - // DESIGN.md §6) and placing its raw key under the given key. Set, and - // the Secret found: the controller adopts it outright and skips - // bootstrap entirely — this is the expected path for Stage 1, not a - // fallback (DESIGN.md §6 explains why self-registration cannot complete - // unauthenticated in the setup this stage actually exercises). - // - // Unset: the controller has no way to acquire a credential in this - // stage (self-registration lands in Stage 5) and reports - // Ready: False, reason: CredentialsSecretRefRequired. + // replicas. terdut-server is not horizontally-scale-tested; keep this + // at its default of 1 unless you've verified otherwise -- the sweeper + // and the notifier are unsynchronised singletons. + // +kubebuilder:default=1 // +optional - CredentialsSecretRef *SecretKeyRef `json:"credentialsSecretRef,omitempty"` + Replicas int32 `json:"replicas,omitempty"` + + // +required + Networking NetworkingSpec `json:"networking"` + + // +required + Database DatabaseSpec `json:"database"` + + // +optional + Sweeper SweeperSpec `json:"sweeper,omitempty"` + + // +optional + Deadman DeadmanSpec `json:"deadman,omitempty"` + + // +optional + Notify NotifySpec `json:"notify,omitempty"` + + // +optional + OIDC OIDCSpec `json:"oidc,omitempty"` + + // passwordLogin: whether a user may sign in, or sign up, with a + // password. + // +kubebuilder:default=true + // +optional + PasswordLogin bool `json:"passwordLogin,omitempty"` // allowedTeams gates cross-namespace TerdutTeams (DESIGN.md §4.6). - // Unused until TerdutTeam exists (Stage 2); present now so this CRD's - // schema doesn't need a breaking change to grow it later. + // Unused until TerdutTeam exists (ROADMAP.md Stage 2); present now so + // this CRD's schema doesn't need a breaking change to grow it later. // +optional AllowedTeams AllowedTeams `json:"allowedTeams,omitempty"` } @@ -95,29 +244,42 @@ const ( // ConditionReady is the standard top-level condition every CRD carries // (DESIGN.md §7). ConditionReady = "Ready" + // ConditionDatabaseReady reflects whether the configured database is + // usable -- for postgresClusterRef, whether the Zalando CR and its + // generated credentials Secret both resolved; for a plain dsn, always + // true once set (DESIGN.md §8: "no connectivity check beyond what the + // Deployment's own readiness probe already gives"). + ConditionDatabaseReady = "DatabaseReady" // ConditionBootstrapped reflects whether a working credential has been - // acquired — adopted from spec.credentialsSecretRef in this stage. + // acquired via self-registration (DESIGN.md §6). ConditionBootstrapped = "Bootstrapped" ) // Condition reasons this controller sets. const ( - // ReasonCredentialsSecretRefRequired: spec.credentialsSecretRef is - // unset, and self-registration isn't implemented yet (Stage 5) — the - // expected, steady-state reason whenever no bring-your-own credential - // has been provided. - ReasonCredentialsSecretRefRequired = "CredentialsSecretRefRequired" - // ReasonCredentialsSecretNotFound: spec.credentialsSecretRef is set but - // no such Secret exists (yet) in the operator's own namespace. - ReasonCredentialsSecretNotFound = "CredentialsSecretNotFound" - // ReasonCredentialsSecretInvalid: the Secret exists but has no data - // under the given key, or it's empty. - ReasonCredentialsSecretInvalid = "CredentialsSecretInvalid" - // ReasonServerUnreachable: GET /api/version against spec.endpoint - // failed — a bad endpoint, or the server is down. - ReasonServerUnreachable = "ServerUnreachable" - // ReasonAdopted: the happy path. A working credential is in hand and the - // server answered its version probe. + // ReasonWaitingForDeployment: the Deployment this controller created has + // no ready replica yet -- bootstrap can't be attempted until it does. + ReasonWaitingForDeployment = "WaitingForDeployment" + // ReasonPostgresClusterNotFound: spec.database.postgresClusterRef names + // no such postgresql.acid.zalan.do object (yet). + ReasonPostgresClusterNotFound = "PostgresClusterNotFound" + // ReasonPostgresOperatorCRDNotInstalled: spec.database.postgresClusterRef + // is set, but the postgresql.acid.zalan.do CRD itself isn't installed in + // this cluster (DESIGN.md §8, §9: this operator degrades gracefully + // rather than hard-failing when that CRD is absent and BYO DSN is used + // instead -- but a TerdutServer that explicitly asks for it still needs + // to say clearly that it can't be satisfied). + ReasonPostgresOperatorCRDNotInstalled = "PostgresOperatorCRDNotInstalled" + // ReasonBootstrapStateLost: a checkpointed admin credential + // (DESIGN.md §6) was lost after being used but before the lasting + // credential it was for could be persisted -- the one genuinely + // pathological case in the self-registration flow. Fail-closed, same + // recovery as DESIGN.md §5's webhook-Secret-loss rule: delete and + // recreate this TerdutServer. + ReasonBootstrapStateLost = "BootstrapStateLost" + // ReasonAdopted: the happy path. A working credential is in hand, the + // Deployment has a ready replica, and the database (if postgresClusterRef) + // resolved. ReasonAdopted = "Adopted" ) @@ -135,16 +297,23 @@ type TerdutServerStatus struct { // +optional ObservedGeneration int64 `json:"observedGeneration,omitempty"` - // credentialsSecretRef mirrors spec.credentialsSecretRef once adopted — - // same Secret, same key, always in the operator's own namespace. Set - // only once Bootstrapped is True. + // serviceName is the Service this controller created for the + // Deployment, so other objects can reference it without recomputing the + // naming convention. + // +optional + ServiceName string `json:"serviceName,omitempty"` + + // credentialsSecretRef is the generated instance-scoped credential + // (DESIGN.md §6) -- pure output, always in the operator's own + // namespace, under a fixed data key ("token"). Set only once + // Bootstrapped is True. // +optional CredentialsSecretRef *SecretKeyRef `json:"credentialsSecretRef,omitempty"` } // +kubebuilder:object:root=true // +kubebuilder:subresource:status -// +kubebuilder:printcolumn:name="Endpoint",type=string,JSONPath=`.spec.endpoint` +// +kubebuilder:printcolumn:name="Replicas",type=integer,JSONPath=`.spec.replicas` // +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status` // +kubebuilder:printcolumn:name="Reason",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].reason` diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index aa680ff..02349dd 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -45,6 +45,136 @@ func (in *AllowedTeamsNamespaces) DeepCopy() *AllowedTeamsNamespaces { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DatabaseSpec) DeepCopyInto(out *DatabaseSpec) { + *out = *in + if in.PasswordSecretRef != nil { + in, out := &in.PasswordSecretRef, &out.PasswordSecretRef + *out = new(SecretKeyRef) + **out = **in + } + if in.PostgresClusterRef != nil { + in, out := &in.PostgresClusterRef, &out.PostgresClusterRef + *out = new(PostgresClusterRef) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DatabaseSpec. +func (in *DatabaseSpec) DeepCopy() *DatabaseSpec { + if in == nil { + return nil + } + out := new(DatabaseSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DeadmanSpec) DeepCopyInto(out *DeadmanSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DeadmanSpec. +func (in *DeadmanSpec) DeepCopy() *DeadmanSpec { + if in == nil { + return nil + } + out := new(DeadmanSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ImageSpec) DeepCopyInto(out *ImageSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ImageSpec. +func (in *ImageSpec) DeepCopy() *ImageSpec { + if in == nil { + return nil + } + out := new(ImageSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NetworkingSpec) DeepCopyInto(out *NetworkingSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NetworkingSpec. +func (in *NetworkingSpec) DeepCopy() *NetworkingSpec { + if in == nil { + return nil + } + out := new(NetworkingSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NotifySpec) DeepCopyInto(out *NotifySpec) { + *out = *in + if in.TokenSecretRef != nil { + in, out := &in.TokenSecretRef, &out.TokenSecretRef + *out = new(SecretKeyRef) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NotifySpec. +func (in *NotifySpec) DeepCopy() *NotifySpec { + if in == nil { + return nil + } + out := new(NotifySpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OIDCSpec) DeepCopyInto(out *OIDCSpec) { + *out = *in + if in.ClientSecretRef != nil { + in, out := &in.ClientSecretRef, &out.ClientSecretRef + *out = new(SecretKeyRef) + **out = **in + } + if in.AllowedGroups != nil { + in, out := &in.AllowedGroups, &out.AllowedGroups + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OIDCSpec. +func (in *OIDCSpec) DeepCopy() *OIDCSpec { + if in == nil { + return nil + } + out := new(OIDCSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PostgresClusterRef) DeepCopyInto(out *PostgresClusterRef) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PostgresClusterRef. +func (in *PostgresClusterRef) DeepCopy() *PostgresClusterRef { + if in == nil { + return nil + } + out := new(PostgresClusterRef) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SecretKeyRef) DeepCopyInto(out *SecretKeyRef) { *out = *in @@ -60,6 +190,21 @@ func (in *SecretKeyRef) DeepCopy() *SecretKeyRef { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SweeperSpec) DeepCopyInto(out *SweeperSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SweeperSpec. +func (in *SweeperSpec) DeepCopy() *SweeperSpec { + if in == nil { + return nil + } + out := new(SweeperSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TerdutServer) DeepCopyInto(out *TerdutServer) { *out = *in @@ -122,11 +267,13 @@ func (in *TerdutServerList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TerdutServerSpec) DeepCopyInto(out *TerdutServerSpec) { *out = *in - if in.CredentialsSecretRef != nil { - in, out := &in.CredentialsSecretRef, &out.CredentialsSecretRef - *out = new(SecretKeyRef) - **out = **in - } + out.Image = in.Image + out.Networking = in.Networking + in.Database.DeepCopyInto(&out.Database) + out.Sweeper = in.Sweeper + out.Deadman = in.Deadman + in.Notify.DeepCopyInto(&out.Notify) + in.OIDC.DeepCopyInto(&out.OIDC) in.AllowedTeams.DeepCopyInto(&out.AllowedTeams) } diff --git a/config/crd/bases/terdut.ryuvia.com_terdutservers.yaml b/config/crd/bases/terdut.ryuvia.com_terdutservers.yaml index 78fd9d4..dd1857d 100644 --- a/config/crd/bases/terdut.ryuvia.com_terdutservers.yaml +++ b/config/crd/bases/terdut.ryuvia.com_terdutservers.yaml @@ -15,9 +15,9 @@ spec: scope: Namespaced versions: - additionalPrinterColumns: - - jsonPath: .spec.endpoint - name: Endpoint - type: string + - jsonPath: .spec.replicas + name: Replicas + type: integer - jsonPath: .status.conditions[?(@.type=="Ready")].status name: Ready type: string @@ -52,8 +52,8 @@ spec: allowedTeams: description: |- allowedTeams gates cross-namespace TerdutTeams (DESIGN.md §4.6). - Unused until TerdutTeam exists (Stage 2); present now so this CRD's - schema doesn't need a breaking change to grow it later. + Unused until TerdutTeam exists (ROADMAP.md Stage 2); present now so + this CRD's schema doesn't need a breaking change to grow it later. properties: namespaces: description: |- @@ -125,45 +125,226 @@ spec: x-kubernetes-map-type: atomic type: object type: object - credentialsSecretRef: + database: description: |- - credentialsSecretRef names a Secret, in the operator's own namespace, - that a human has already created by minting an instance-scoped service - account with their own admin session (POST /api/service-accounts, - DESIGN.md §6) and placing its raw key under the given key. Set, and - the Secret found: the controller adopts it outright and skips - bootstrap entirely — this is the expected path for Stage 1, not a - fallback (DESIGN.md §6 explains why self-registration cannot complete - unauthenticated in the setup this stage actually exercises). - - Unset: the controller has no way to acquire a credential in this - stage (self-registration lands in Stage 5) and reports - Ready: False, reason: CredentialsSecretRefRequired. + DatabaseSpec is the Postgres connection this TerdutServer uses. Exactly + one of dsn or postgresClusterRef must be set (DESIGN.md §8) — this + operator provisions no database either way, only wires up one that + exists. properties: - key: - description: key is the data key inside the Secret holding the - raw value. + dsn: + description: |- + dsn is a DSN with no password in it, e.g. + "postgres://terdut@terdut-postgres:5432/terdut?sslmode=require" -- + mutually exclusive with postgresClusterRef. + type: string + passwordSecretRef: + description: |- + passwordSecretRef is where PGPASSWORD comes from for the dsn path. + pgx falls back to libpq's environment variables for anything the DSN + omits, so the password never appears in the DSN string itself. Unused + on the postgresClusterRef path -- the Zalando-generated Secret is + wired in directly instead. + properties: + key: + description: key is the data key inside the Secret holding + the raw value. + minLength: 1 + type: string + name: + description: name is the Secret's name. + minLength: 1 + type: string + required: + - key + - name + type: object + postgresClusterRef: + description: |- + postgresClusterRef names a Zalando postgres-operator CR instead of a + plain DSN -- mutually exclusive with dsn. + properties: + name: + minLength: 1 + type: string + required: + - name + type: object + type: object + x-kubernetes-validations: + - message: exactly one of dsn or postgresClusterRef must be set + rule: '(has(self.dsn) ? 1 : 0) + (has(self.postgresClusterRef) ? + 1 : 0) == 1' + deadman: + description: |- + DeadmanSpec controls dead man's switch alerts. Matchers/Timeout/Severity + map straight to TERDUT_DEADMAN_MATCHERS/TERDUT_DEADMAN_TIMEOUT/ + TERDUT_DEADMAN_SEVERITY. + properties: + matchers: + type: string + severity: + type: string + timeout: + type: string + type: object + image: + description: ImageSpec is the terdut-server image to run. + properties: + repository: minLength: 1 type: string - name: - description: name is the Secret's name. + tag: minLength: 1 type: string required: - - key - - name + - repository + - tag type: object - endpoint: + networking: description: |- - endpoint is the base URL of an already-running terdut-server this - TerdutServer represents, e.g. "http://terdut.oncall.svc:8080". This - stage never creates or manages a Deployment/Service for it — the - server is expected to already exist, deployed some other way (its own - Helm chart, by hand). - minLength: 1 - type: string + NetworkingSpec is how this TerdutServer is reached from outside the + cluster. + + hostname/gatewayListener describe the intended Gateway API HTTPRoute + (matching charts/terdut-server's own templates/httpproxy.yaml, despite its + name — that chart carries a Gateway API HTTPRoute, not a Contour + HTTPProxy), but creating that HTTPRoute isn't implemented yet: it needs + the Gateway API types as a new dependency, and nothing about proving a + TerdutServer boots and bootstraps a real server depends on external + ingress existing. Tracked as a near-term follow-up, not deferred to a + later ROADMAP.md stage the way Deployment/database/bootstrap once were. + properties: + gatewayListener: + description: |- + gatewayListener is the HTTPRoute's sectionName once it exists. Empty + attaches to every matching listener, including plaintext HTTP. + type: string + hostname: + description: hostname the HTTPRoute will carry once it exists. + type: string + servicePort: + default: 8080 + description: |- + servicePort is both the Service's port and the HTTPRoute's backend + port once it exists. Defaults to 8080, matching the chart's own + service.port default. + format: int32 + type: integer + type: object + notify: + description: |- + NotifySpec controls push notifications via ntfy. Empty ntfyURL disables + notifications entirely (matches the chart's own default). + properties: + fallbackTopic: + type: string + ntfyURL: + type: string + repeatEvery: + type: string + tokenSecretRef: + description: |- + tokenSecretRef is an optional bearer token for an access-controlled + ntfy. Leave unset for an open ntfy. + properties: + key: + description: key is the data key inside the Secret holding + the raw value. + minLength: 1 + type: string + name: + description: name is the Secret's name. + minLength: 1 + type: string + required: + - key + - name + type: object + type: object + oidc: + description: |- + OIDCSpec controls single sign-on. Fields the chart also exposes but + DESIGN.md's spec doesn't (usernameClaim, emailClaim, groupsClaim, + trustEmail) use terdut-server's own defaults + (preferred_username/email/groups/false) rather than being added here + speculatively. + properties: + adminGroup: + type: string + allowedGroups: + items: + type: string + type: array + clientID: + type: string + clientSecretRef: + description: |- + SecretKeyRef names one data key inside a Secret. Every use of this type in + TerdutServerSpec resolves in the TerdutServer's own namespace (it's wired + straight into the Deployment's pod spec as a secretKeyRef env source, + which Kubernetes itself only allows same-namespace) -- unlike the + generated credentials Secret (DESIGN.md §6), which always lives in the + operator's own namespace and is never referenced through this type. + properties: + key: + description: key is the data key inside the Secret holding + the raw value. + minLength: 1 + type: string + name: + description: name is the Secret's name. + minLength: 1 + type: string + required: + - key + - name + type: object + enabled: + type: boolean + issuer: + type: string + name: + default: SSO + type: string + scopes: + default: openid profile email + type: string + sessionMaxAge: + default: 12h + type: string + type: object + passwordLogin: + default: true + description: |- + passwordLogin: whether a user may sign in, or sign up, with a + password. + type: boolean + replicas: + default: 1 + description: |- + replicas. terdut-server is not horizontally-scale-tested; keep this + at its default of 1 unless you've verified otherwise -- the sweeper + and the notifier are unsynchronised singletons. + format: int32 + type: integer + sweeper: + description: |- + SweeperSpec controls incident auto-resolve/archive timing. Values are + Go duration strings (e.g. "6h"), passed straight through to the + TERDUT_STALE_AFTER/TERDUT_ARCHIVE_AFTER env vars exactly as written -- + not a structured metav1.Duration, since terdut-server parses them itself + and a round-trip through a different type would buy nothing. + properties: + archiveAfter: + type: string + staleAfter: + type: string + type: object required: - - endpoint + - database + - image + - networking type: object status: description: status defines the observed state of TerdutServer @@ -231,9 +412,10 @@ spec: x-kubernetes-list-type: map credentialsSecretRef: description: |- - credentialsSecretRef mirrors spec.credentialsSecretRef once adopted — - same Secret, same key, always in the operator's own namespace. Set - only once Bootstrapped is True. + credentialsSecretRef is the generated instance-scoped credential + (DESIGN.md §6) -- pure output, always in the operator's own + namespace, under a fixed data key ("token"). Set only once + Bootstrapped is True. properties: key: description: key is the data key inside the Secret holding the @@ -255,6 +437,12 @@ spec: tells "applied" from "seen" (DESIGN.md §7). format: int64 type: integer + serviceName: + description: |- + serviceName is the Service this controller created for the + Deployment, so other objects can reference it without recomputing the + naming convention. + type: string type: object required: - spec diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 0d069ab..bcb14fe 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -8,10 +8,35 @@ rules: - "" resources: - secrets + - services + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - acid.zalan.do + resources: + - postgresqls verbs: - get - list - watch +- apiGroups: + - apps + resources: + - deployments + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - terdut.ryuvia.com resources: diff --git a/config/samples/terdut_v1alpha1_terdutserver.yaml b/config/samples/terdut_v1alpha1_terdutserver.yaml index 6362fe4..fb40d0a 100644 --- a/config/samples/terdut_v1alpha1_terdutserver.yaml +++ b/config/samples/terdut_v1alpha1_terdutserver.yaml @@ -6,18 +6,28 @@ metadata: app.kubernetes.io/managed-by: kustomize name: terdutserver-sample spec: - # An already-running terdut-server this TerdutServer represents — Stage 1 - # never creates or manages a Deployment/Service for it (ROADMAP.md). - endpoint: "http://terdut.oncall.svc.cluster.local:8080" - # Bring-your-own instance credential (DESIGN.md §6): mint this once, - # manually, with your own admin session -- - # curl -H "Authorization: Bearer " \ - # -X POST "$ENDPOINT/api/service-accounts" \ - # -d '{"name":"terdut-operator","scope":"instance"}' - # -- then create this Secret, in the OPERATOR's own namespace, from the - # "key" field of that response: - # kubectl create secret generic terdutserver-sample-creds \ - # -n --from-literal=token= - credentialsSecretRef: - name: terdutserver-sample-creds - key: token + image: + repository: git.ryuvia.com/niklas/terdut-server + tag: v0.20.0 + replicas: 1 + networking: + hostname: terdut.example.com + servicePort: 8080 + # Bring-your-own DSN (simplest path, no external CRD dependency). For the + # Zalando postgres-operator path instead, use: + # database: + # postgresClusterRef: + # name: terdut-postgres + database: + dsn: "postgres://terdut@terdut-postgres:5432/terdut?sslmode=require" + passwordSecretRef: + name: terdut-postgres-password + key: password + sweeper: + staleAfter: 6h + archiveAfter: 168h + deadman: + matchers: "alertname=Watchdog" + timeout: 15m + severity: critical + passwordLogin: true diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 2eece3b..7f8213c 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -50,7 +50,11 @@ var _ = BeforeSuite(func() { By("bootstrapping test environment") testEnv = &envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")}, + // testdata carries a minimal, test-only stub of Zalando + // postgres-operator's CRD (see its own file comment) so + // terdutserver_database_test.go can exercise the postgresClusterRef + // path without a real postgres-operator installed. + CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases"), "testdata"}, ErrorIfCRDPathMissing: true, } diff --git a/internal/controller/terdutserver_bootstrap.go b/internal/controller/terdutserver_bootstrap.go new file mode 100644 index 0000000..5bae654 --- /dev/null +++ b/internal/controller/terdutserver_bootstrap.go @@ -0,0 +1,153 @@ +package controller + +import ( + "context" + "errors" + "fmt" + "net/http" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + terdutv1alpha1 "git.ryuvia.com/niklas/terdut-operator/api/v1alpha1" + "git.ryuvia.com/niklas/terdut-operator/internal/tdclient" +) + +// credentialsSecretDataKey is the fixed data key every generated +// credentials Secret this controller writes uses — "whatever a human chose" +// only applied to the bring-your-own input this design removed (DESIGN.md +// §6); every Secret this controller itself generates uses this one key. +const credentialsSecretDataKey = "token" + +// bootstrapStateLostError is DESIGN.md §6's one genuinely pathological +// case: a checkpointed admin credential was used and then lost before the +// lasting credential it was for could be persisted. Distinct from a plain +// error so Reconcile can route it to a Ready: False condition (the +// documented recovery is delete-and-recreate, not an automatic retry) rather +// than treating it as a transient reconcile failure. +type bootstrapStateLostError struct{ detail string } + +func (e *bootstrapStateLostError) Error() string { + return fmt.Sprintf( + "server reports already bootstrapped, but neither status.credentialsSecretRef nor a "+ + "checkpointed admin credential exist here: %s. This TerdutServer cannot recover a "+ + "credential on its own; delete and recreate it", e.detail) +} + +// reconcileBootstrap implements DESIGN.md §6 point 1's self-registration +// flow, checkpointed against the two real crash windows in it rather than +// leaving them as theoretical gaps. Only called once +// srv.Status.CredentialsSecretRef is nil and the Deployment has a ready +// replica. +func (r *TerdutServerReconciler) reconcileBootstrap(ctx context.Context, srv *terdutv1alpha1.TerdutServer) error { + adminKey, err := r.getOrCreateCheckpointedAdminKey(ctx, srv) + if err != nil { + return err + } + + bc := r.NewClient(serviceURL(srv)).WithToken(adminKey) + instanceKey, err := r.getOrMintInstanceServiceAccountKey(ctx, bc) + if err != nil { + return err + } + + credsName := credentialsSecretName(srv) + if err := r.writeSecret(ctx, credsName, instanceKey); err != nil { + return err + } + srv.Status.CredentialsSecretRef = &terdutv1alpha1.SecretKeyRef{Name: credsName, Key: credentialsSecretDataKey} + + // Best-effort: the checkpoint has done its job. Leaving it behind on a + // delete failure here isn't a correctness problem (the next reconcile + // finds status.CredentialsSecretRef already set and never looks at the + // checkpoint again) — it would just be an unused Secret sitting around, + // cleaned up for real by the finalizer on delete. + checkpoint := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: checkpointSecretName(srv), Namespace: r.OperatorNamespace}} + _ = r.Delete(ctx, checkpoint) + + return nil +} + +// getOrCreateCheckpointedAdminKey returns a usable admin key: from the +// checkpoint Secret if an earlier, interrupted attempt already got one, or +// freshly from /api/bootstrap, immediately checkpointed before it's used +// for anything else. +func (r *TerdutServerReconciler) getOrCreateCheckpointedAdminKey(ctx context.Context, srv *terdutv1alpha1.TerdutServer) (string, error) { + checkpointName := checkpointSecretName(srv) + + var checkpoint corev1.Secret + err := r.Get(ctx, client.ObjectKey{Namespace: r.OperatorNamespace, Name: checkpointName}, &checkpoint) + switch { + case err == nil: + return string(checkpoint.Data[credentialsSecretDataKey]), nil + case !apierrors.IsNotFound(err): + return "", err + } + + bc := r.NewClient(serviceURL(srv)) + result, err := bc.Bootstrap(ctx, bootstrapUsername, bootstrapEmail) + if err != nil { + if statusErr, ok := errors.AsType[*tdclient.StatusError](err); ok && statusErr.Code == http.StatusForbidden { + // §1: this operator is the only thing that ever bootstraps a + // server it created, so a 403 here (no checkpoint, no + // status.credentialsSecretRef) means a prior reconcile already + // won this exact race and its checkpoint was lost afterward -- + // the one case §6 doesn't try to paper over. + return "", &bootstrapStateLostError{detail: "/api/bootstrap returned 403"} + } + return "", fmt.Errorf("POST /api/bootstrap: %w", err) + } + + if err := r.writeSecret(ctx, checkpointName, result.APIKey.Key); err != nil { + return "", fmt.Errorf("checkpointing admin key: %w", err) + } + return result.APIKey.Key, nil +} + +// getOrMintInstanceServiceAccountKey mints the operator's own instance- +// scoped service account, or, if an earlier interrupted attempt already +// created it (409), adopts it and mints a fresh key rather than treating +// the conflict as an error (DESIGN.md §6 point 1, §5's general +// adopt-on-conflict rule). +func (r *TerdutServerReconciler) getOrMintInstanceServiceAccountKey(ctx context.Context, bc *tdclient.Client) (string, error) { + result, err := bc.CreateInstanceServiceAccount(ctx, serviceAccountName) + if err == nil { + return result.Key.Key, nil + } + + statusErr, ok := errors.AsType[*tdclient.StatusError](err) + if !ok || statusErr.Code != http.StatusConflict { + return "", fmt.Errorf("POST /api/service-accounts: %w", err) + } + + sa, err := bc.GetServiceAccountByName(ctx, serviceAccountName) + if err != nil { + return "", fmt.Errorf("GET /api/service-accounts?name=%s (adopting after 409): %w", serviceAccountName, err) + } + if sa == nil { + return "", fmt.Errorf("POST /api/service-accounts 409'd for %q but GET found nothing", serviceAccountName) + } + key, err := bc.CreateServiceAccountKey(ctx, sa.ID, "initial") + if err != nil { + return "", fmt.Errorf("POST /api/service-accounts/%d/keys (adopting after 409): %w", sa.ID, err) + } + return key.Key, nil +} + +// writeSecret creates or replaces a Secret in the operator's own namespace +// holding one raw value under credentialsSecretDataKey. +func (r *TerdutServerReconciler) writeSecret(ctx context.Context, name, rawValue string) error { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: r.OperatorNamespace}, + Data: map[string][]byte{credentialsSecretDataKey: []byte(rawValue)}, + } + if err := r.Create(ctx, secret); err != nil { + if apierrors.IsAlreadyExists(err) { + return r.Update(ctx, secret) + } + return err + } + return nil +} diff --git a/internal/controller/terdutserver_controller.go b/internal/controller/terdutserver_controller.go index 2fdafd2..19cabca 100644 --- a/internal/controller/terdutserver_controller.go +++ b/internal/controller/terdutserver_controller.go @@ -2,16 +2,20 @@ package controller import ( "context" + "errors" "fmt" "time" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/recorder" @@ -21,41 +25,67 @@ import ( // resyncInterval is the periodic requeue on a successful reconcile (DESIGN.md // §5's general rule) — it exists to catch drift from someone changing state -// directly against the server's API/UI, not from a missed watch event. +// directly against the server's API/UI, not from a missed watch event. It +// also doubles as the eventual-rotation-detection interval for the Zalando +// database path (§8 asks for a live Secret watch; this controller doesn't +// have one yet, so a rotated credential is noticed on the next resync +// rather than immediately — a known simplification, not a design decision). const resyncInterval = 5 * time.Minute // waitInterval is the requeue while waiting on an external condition to -// resolve (the credential Secret not existing yet, the server being -// unreachable) — shorter than resyncInterval, since these are expected to -// change sooner than "someone edited something out of band." -const waitInterval = 30 * time.Second +// resolve (the Deployment not ready yet, a Zalando cluster not found yet) — +// shorter than resyncInterval, since these are expected to change sooner +// than "someone edited something out of band." +const waitInterval = 15 * time.Second + +// finalizerName cleans up the credentials Secret(s) this controller +// generates in the operator's own namespace on delete — the Deployment and +// Service are owned (OwnerReference, DESIGN.md §7) and need no finalizer of +// their own. +const finalizerName = "terdut.ryuvia.com/terdutserver" + +// serviceAccountName is the name the operator registers itself under +// server-side (DESIGN.md §6) — a fixed, repo-wide constant, not a spec +// field: it names the automation, not anything about this one TerdutServer. +const serviceAccountName = "terdut-operator" + +// bootstrapUsername/bootstrapEmail found the one human-shaped user every +// fresh install needs (terdut-server's handleBootstrap requires both). +// Nobody signs in as this user afterward — its only purpose is minting the +// admin key the controller immediately trades for a real service-account +// key — so these are fixed, not spec fields. +const ( + bootstrapUsername = "terdut-operator-bootstrap" + bootstrapEmail = "bootstrap@terdut-operator.local" +) + +// postgresqlGVK is the Zalando postgres-operator's CR (DESIGN.md §8). +// Resolved via unstructured rather than vendoring Zalando's own client, to +// keep this operator's dependency on it to "an optional CRD read" rather +// than a library — matches §9's "degrade gracefully if the CRD isn't +// installed" stance. +var postgresqlGVK = schema.GroupVersionKind{Group: "acid.zalan.do", Version: "v1", Kind: "postgresql"} // TerdutServerReconciler reconciles a TerdutServer object. // -// Stage 1 (ROADMAP.md): bootstrap/credentials only, bring-your-own path -// only. It never creates, updates, or deletes anything server-side or any -// Deployment/Service — it only reads a Secret the operator's own namespace -// already has and probes the server's /api/version. No finalizer: this -// controller owns nothing that needs cleaning up on delete (it never creates -// the credentials Secret itself, only ever adopts one a human already -// made) — revisit once self-registration (Stage 5) generates its own. +// Stage 1 (ROADMAP.md): full lifecycle. The operator creates and owns every +// TerdutServer it manages (DESIGN.md §1) — there is no adopt path. type TerdutServerReconciler struct { client.Client Scheme *runtime.Scheme // OperatorNamespace is where every credentials Secret this controller - // reads or writes lives (DESIGN.md §6) — never the TerdutServer's own + // generates lives (DESIGN.md §6) — never the TerdutServer's own // namespace. Set from the POD_NAMESPACE downward-API env var in // production (cmd/main.go); tests set it directly. OperatorNamespace string // Recorder emits the Kubernetes Events DESIGN.md §12 asks for on every - // externally-visible outcome. The events.k8s.io/v1 API, not the - // deprecated core/v1 one GetEventRecorderFor still returns. + // externally-visible outcome. Recorder recorder.EventRecorder - // NewClient builds the terdut-server API client for a given endpoint. - // A field, not a direct tdclient.New call, so tests can substitute an + // NewClient builds the terdut-server API client for a given endpoint. A + // field, not a direct tdclient.New call, so tests can substitute an // httptest.Server's client without a real network round trip. Defaults // to tdclient.New via SetupWithManager. NewClient func(endpoint string) *tdclient.Client @@ -64,7 +94,10 @@ type TerdutServerReconciler struct { // +kubebuilder:rbac:groups=terdut.ryuvia.com,resources=terdutservers,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=terdut.ryuvia.com,resources=terdutservers/status,verbs=get;update;patch // +kubebuilder:rbac:groups=terdut.ryuvia.com,resources=terdutservers/finalizers,verbs=update -// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch +// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups="",resources=services,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=acid.zalan.do,resources=postgresqls,verbs=get;list;watch func (r *TerdutServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := logf.FromContext(ctx) @@ -77,77 +110,88 @@ func (r *TerdutServerReconciler) Reconcile(ctx context.Context, req ctrl.Request return ctrl.Result{}, err } - if srv.Spec.CredentialsSecretRef == nil { - return r.setNotReady(ctx, &srv, - terdutv1alpha1.ReasonCredentialsSecretRefRequired, - "spec.credentialsSecretRef is unset; self-registration bootstrap isn't "+ - "implemented yet (Stage 5) — mint an instance-scoped service account "+ - "with your own admin session (POST /api/service-accounts) and set "+ - "this field to a Secret holding its key (DESIGN.md §6)") + if !srv.DeletionTimestamp.IsZero() { + return r.reconcileDelete(ctx, &srv) } - ref := srv.Spec.CredentialsSecretRef - var secret corev1.Secret - secretKey := client.ObjectKey{Namespace: r.OperatorNamespace, Name: ref.Name} - if err := r.Get(ctx, secretKey, &secret); err != nil { - if apierrors.IsNotFound(err) { - return r.setNotReady(ctx, &srv, - terdutv1alpha1.ReasonCredentialsSecretNotFound, - fmt.Sprintf("Secret %q not found in namespace %q", ref.Name, r.OperatorNamespace)) + if !controllerutil.ContainsFinalizer(&srv, finalizerName) { + controllerutil.AddFinalizer(&srv, finalizerName) + if err := r.Update(ctx, &srv); err != nil { + return ctrl.Result{}, err } + // The Update above re-triggers a reconcile via the watch; nothing + // further to do on this pass. + return ctrl.Result{}, nil + } + + dbEnv, dbErr := r.resolveDatabaseEnv(ctx, &srv) + if dbErr != nil { + return r.setNotReady(ctx, &srv, dbErr.reason, dbErr.message, waitInterval) + } + + deploy, err := r.reconcileDeployment(ctx, &srv, dbEnv) + if err != nil { + return ctrl.Result{}, err + } + if err := r.reconcileService(ctx, &srv); err != nil { return ctrl.Result{}, err } - raw, ok := secret.Data[ref.Key] - if !ok || len(raw) == 0 { + meta.SetStatusCondition(&srv.Status.Conditions, metav1.Condition{ + Type: terdutv1alpha1.ConditionDatabaseReady, + Status: metav1.ConditionTrue, + Reason: terdutv1alpha1.ReasonAdopted, + Message: "database resolved", + }) + + if deploy.Status.ReadyReplicas < 1 { return r.setNotReady(ctx, &srv, - terdutv1alpha1.ReasonCredentialsSecretInvalid, - fmt.Sprintf("Secret %q has no data under key %q", ref.Name, ref.Key)) + terdutv1alpha1.ReasonWaitingForDeployment, + fmt.Sprintf("Deployment %q has no ready replica yet", deploy.Name), + waitInterval) } - newClient := r.NewClient - if newClient == nil { - newClient = tdclient.New - } - if _, err := newClient(srv.Spec.Endpoint).Version(ctx); err != nil { - return r.setNotReady(ctx, &srv, - terdutv1alpha1.ReasonServerUnreachable, - fmt.Sprintf("GET %s/api/version: %v", srv.Spec.Endpoint, err)) + if srv.Status.CredentialsSecretRef == nil { + if err := r.reconcileBootstrap(ctx, &srv); err != nil { + if pending, ok := errors.AsType[*bootstrapStateLostError](err); ok { + return r.setNotReady(ctx, &srv, terdutv1alpha1.ReasonBootstrapStateLost, pending.Error(), waitInterval) + } + return ctrl.Result{}, err + } } - srv.Status.CredentialsSecretRef = ref.DeepCopy() meta.SetStatusCondition(&srv.Status.Conditions, metav1.Condition{ Type: terdutv1alpha1.ConditionBootstrapped, Status: metav1.ConditionTrue, Reason: terdutv1alpha1.ReasonAdopted, - Message: fmt.Sprintf("adopted spec.credentialsSecretRef (Secret %q, key %q)", ref.Name, ref.Key), + Message: fmt.Sprintf("credentials in Secret %q", srv.Status.CredentialsSecretRef.Name), }) meta.SetStatusCondition(&srv.Status.Conditions, metav1.Condition{ Type: terdutv1alpha1.ConditionReady, Status: metav1.ConditionTrue, Reason: terdutv1alpha1.ReasonAdopted, - Message: "credentials adopted, server reachable", + Message: "deployment ready, database resolved, credentials bootstrapped", }) + srv.Status.ServiceName = srv.Name srv.Status.ObservedGeneration = srv.Generation if err := r.Status().Update(ctx, &srv); err != nil { return ctrl.Result{}, err } if r.Recorder != nil { - r.Recorder.Eventf(&srv, nil, corev1.EventTypeNormal, - terdutv1alpha1.ReasonAdopted, terdutv1alpha1.ReasonAdopted, - "credentials adopted, server reachable") + r.Recorder.Eventf(&srv, nil, corev1.EventTypeNormal, terdutv1alpha1.ReasonAdopted, terdutv1alpha1.ReasonAdopted, + "terdut-server ready") } - log.Info("TerdutServer ready", "endpoint", srv.Spec.Endpoint) + log.Info("TerdutServer ready", "name", srv.Name) return ctrl.Result{RequeueAfter: resyncInterval}, nil } -// setNotReady records Ready: False with reason/message on both conditions, -// fires a Warning event, and requeues after waitInterval — every "waiting on -// something external" exit from Reconcile goes through here so the -// condition/event/requeue shape can't drift between them. +// setNotReady records Ready: False with reason/message, fires a Warning +// event, and requeues after d. Every "waiting on something" exit from +// Reconcile goes through here so the condition/event/requeue shape can't +// drift between them. func (r *TerdutServerReconciler) setNotReady( - ctx context.Context, srv *terdutv1alpha1.TerdutServer, reason, message string, + ctx context.Context, srv *terdutv1alpha1.TerdutServer, reason, message string, d time.Duration, ) (ctrl.Result, error) { meta.SetStatusCondition(&srv.Status.Conditions, metav1.Condition{ Type: terdutv1alpha1.ConditionReady, @@ -162,7 +206,31 @@ func (r *TerdutServerReconciler) setNotReady( if r.Recorder != nil { r.Recorder.Eventf(srv, nil, corev1.EventTypeWarning, reason, reason, message) } - return ctrl.Result{RequeueAfter: waitInterval}, nil + return ctrl.Result{RequeueAfter: d}, nil +} + +// reconcileDelete cleans up the credentials Secret(s) this controller +// generated in the operator's own namespace. The Deployment and Service are +// owned (OwnerReference, DESIGN.md §7) and need no attention here — normal +// GC handles them. There is no server-side "delete this install" call to +// make: bootstrap created a user and a service account, and terdut-server's +// API has no way to delete either (only to revoke individual keys), so +// there is nothing meaningful to undo there either. +func (r *TerdutServerReconciler) reconcileDelete(ctx context.Context, srv *terdutv1alpha1.TerdutServer) (ctrl.Result, error) { + if !controllerutil.ContainsFinalizer(srv, finalizerName) { + return ctrl.Result{}, nil + } + for _, name := range []string{checkpointSecretName(srv), credentialsSecretName(srv)} { + sec := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: r.OperatorNamespace}} + if err := r.Delete(ctx, sec); err != nil && !apierrors.IsNotFound(err) { + return ctrl.Result{}, err + } + } + controllerutil.RemoveFinalizer(srv, finalizerName) + if err := r.Update(ctx, srv); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } // SetupWithManager sets up the controller with the Manager. @@ -175,6 +243,26 @@ func (r *TerdutServerReconciler) SetupWithManager(mgr ctrl.Manager) error { } return ctrl.NewControllerManagedBy(mgr). For(&terdutv1alpha1.TerdutServer{}). + Owns(&appsv1.Deployment{}). + Owns(&corev1.Service{}). Named("terdutserver"). Complete(r) } + +// --- naming --- + +func checkpointSecretName(srv *terdutv1alpha1.TerdutServer) string { + return fmt.Sprintf("%s.%s-bootstrap-admin", srv.Namespace, srv.Name) +} + +func credentialsSecretName(srv *terdutv1alpha1.TerdutServer) string { + return fmt.Sprintf("%s.%s-instance-credentials", srv.Namespace, srv.Name) +} + +func serviceURL(srv *terdutv1alpha1.TerdutServer) string { + port := srv.Spec.Networking.ServicePort + if port == 0 { + port = 8080 + } + return fmt.Sprintf("http://%s.%s.svc:%d", srv.Name, srv.Namespace, port) +} diff --git a/internal/controller/terdutserver_controller_test.go b/internal/controller/terdutserver_controller_test.go index 2251d91..e0f46ed 100644 --- a/internal/controller/terdutserver_controller_test.go +++ b/internal/controller/terdutserver_controller_test.go @@ -2,43 +2,120 @@ package controller import ( "context" + "encoding/json" "fmt" "net/http" "net/http/httptest" + "sync" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/reconcile" terdutv1alpha1 "git.ryuvia.com/niklas/terdut-operator/api/v1alpha1" + "git.ryuvia.com/niklas/terdut-operator/internal/tdclient" ) -const ( - // unusedEndpoint is a syntactically valid URL no test here ever expects - // to actually be dialed (the cases using it fail before reaching the - // server-reachability check). - unusedEndpoint = "http://unused.invalid" - // tokenKey is the data key every test's credentials Secret uses. - tokenKey = "token" -) +// fakeTerdutServer reproduces the exact stateful semantics of +// /api/bootstrap, /api/service-accounts and /api/version that the +// bootstrap flow depends on (DESIGN.md §6, §11: "terdut-server's REST API +// is faked with a small httptest.Server per controller test ... matching +// the real handlers' request/response shapes"), including the 403-after- +// first-success and 409-on-name-conflict behavior the adopt-on-conflict +// recovery path exists for. +type fakeTerdutServer struct { + mu sync.Mutex + bootstrapped bool + bootstrap403 bool // force every /api/bootstrap call to 403, even the first + nextID int64 + accounts map[string]int64 // name -> id + keyMints map[int64]int // id -> number of keys minted so far +} -// fakeTerdutServer is an httptest.Server standing in for terdut-server's -// GET /api/version, per DESIGN.md §11 ("terdut-server's REST API is faked -// with a small httptest.Server per controller test ... no real Postgres or -// real terdut-server binary needed for controller unit tests"). -func fakeTerdutServer(version string) *httptest.Server { - return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != "/api/version" { - w.WriteHeader(http.StatusNotFound) +func newFakeTerdutServer() (*fakeTerdutServer, *httptest.Server) { + f := &fakeTerdutServer{accounts: map[string]int64{}, keyMints: map[int64]int{}} + return f, httptest.NewServer(f) +} + +func (f *fakeTerdutServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + + switch { + case r.URL.Path == "/api/version": + writeJSON(w, http.StatusOK, map[string]string{"version": "test"}) + + case r.URL.Path == "/api/bootstrap" && r.Method == http.MethodPost: + if f.bootstrapped || f.bootstrap403 { + writeJSON(w, http.StatusForbidden, map[string]string{"error": "bootstrap already completed"}) return } - fmt.Fprintf(w, `{"version":%q}`, version) //nolint:errcheck - })) + f.bootstrapped = true + writeJSON(w, http.StatusCreated, tdclient.BootstrapResult{ + APIKey: tdclient.APIKey{ID: 1, Name: "bootstrap", Key: "admin-key-raw"}, + }) + + case r.URL.Path == "/api/service-accounts" && r.Method == http.MethodPost: + var req struct { + Name string `json:"name"` + Scope string `json:"scope"` + } + _ = json.NewDecoder(r.Body).Decode(&req) + if _, exists := f.accounts[req.Name]; exists { + writeJSON(w, http.StatusConflict, map[string]string{"error": "a service account with that name already exists"}) + return + } + f.nextID++ + id := f.nextID + f.accounts[req.Name] = id + f.keyMints[id] = 1 + writeJSON(w, http.StatusCreated, tdclient.CreateServiceAccountResult{ + ServiceAccount: tdclient.ServiceAccount{ID: id, Name: req.Name, Scope: req.Scope}, + Key: tdclient.APIKey{ID: 1, Name: "initial", Key: fmt.Sprintf("instance-key-%d-initial", id)}, + }) + + case r.URL.Path == "/api/service-accounts" && r.Method == http.MethodGet: + name := r.URL.Query().Get("name") + id, exists := f.accounts[name] + if !exists { + writeJSON(w, http.StatusOK, []tdclient.ServiceAccount{}) + return + } + writeJSON(w, http.StatusOK, []tdclient.ServiceAccount{{ID: id, Name: name, Scope: "instance"}}) + + default: + if id, name, ok := parseKeysPath(r.URL.Path); ok && r.Method == http.MethodPost { + f.keyMints[id]++ + writeJSON(w, http.StatusCreated, tdclient.APIKey{ + ID: int64(f.keyMints[id]), Name: name, + Key: fmt.Sprintf("instance-key-%d-mint%d", id, f.keyMints[id]), + }) + return + } + w.WriteHeader(http.StatusNotFound) + } +} + +func parseKeysPath(path string) (id int64, mintName string, ok bool) { + var parsedID int64 + n, err := fmt.Sscanf(path, "/api/service-accounts/%d/keys", &parsedID) + if err != nil || n != 1 { + return 0, "", false + } + return parsedID, "minted", true +} + +func writeJSON(w http.ResponseWriter, status int, v any) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(v) } var _ = Describe("TerdutServer Controller", func() { @@ -63,10 +140,23 @@ var _ = Describe("TerdutServer Controller", func() { AfterEach(func(ctx SpecContext) { srv := &terdutv1alpha1.TerdutServer{} if err := k8sClient.Get(ctx, objKey, srv); err == nil { - Expect(k8sClient.Delete(ctx, srv)).To(Succeed()) + srv.Finalizers = nil + _ = k8sClient.Update(ctx, srv) + _ = k8sClient.Delete(ctx, srv) + } + for _, n := range []string{checkpointSecretNameFor(name), credentialsSecretNameFor(name)} { + _ = k8sClient.Delete(ctx, &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: n, Namespace: operatorNamespace}}) } }) + dsnSpec := func() terdutv1alpha1.TerdutServerSpec { + return terdutv1alpha1.TerdutServerSpec{ + Image: terdutv1alpha1.ImageSpec{Repository: "example.invalid/terdut-server", Tag: "test"}, + Networking: terdutv1alpha1.NetworkingSpec{Hostname: "terdut.example.invalid", ServicePort: 8080}, + Database: terdutv1alpha1.DatabaseSpec{DSN: "postgres://terdut@test-postgres:5432/terdut?sslmode=require"}, + } + } + createServer := func(ctx context.Context, spec terdutv1alpha1.TerdutServerSpec) { srv := &terdutv1alpha1.TerdutServer{ ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: operatorNamespace}, @@ -75,123 +165,235 @@ var _ = Describe("TerdutServer Controller", func() { Expect(k8sClient.Create(ctx, srv)).To(Succeed()) } - reconcileOnce := func(ctx context.Context) { - _, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: objKey}) + reconcileOnce := func(ctx context.Context) ctrl.Result { + res, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: objKey}) Expect(err).NotTo(HaveOccurred()) + return res + } + + markDeploymentReady := func(ctx context.Context) { + var deploy appsv1.Deployment + Expect(k8sClient.Get(ctx, objKey, &deploy)).To(Succeed()) + deploy.Status.ReadyReplicas = 1 + deploy.Status.Replicas = 1 + Expect(k8sClient.Status().Update(ctx, &deploy)).To(Succeed()) } readyCondition := func(ctx context.Context) metav1.Condition { srv := &terdutv1alpha1.TerdutServer{} Expect(k8sClient.Get(ctx, objKey, srv)).To(Succeed()) c := meta.FindStatusCondition(srv.Status.Conditions, terdutv1alpha1.ConditionReady) - Expect(c).NotTo(BeNil(), "Ready condition should always be set after a reconcile") + Expect(c).NotTo(BeNil(), "Ready condition should always be set after a reconcile past the finalizer-add pass") return *c } - When("spec.credentialsSecretRef is unset", func() { - It("reports Ready: False, reason CredentialsSecretRefRequired, and makes no API call", func(ctx SpecContext) { - createServer(ctx, terdutv1alpha1.TerdutServerSpec{Endpoint: unusedEndpoint}) + Describe("bring-your-own DSN path", func() { + It("reaches Ready through the full lifecycle: finalizer, Deployment/Service, wait, bootstrap", func(ctx SpecContext) { + fake, fakeSrv := newFakeTerdutServer() + DeferCleanup(fakeSrv.Close) + reconciler.NewClient = func(string) *tdclient.Client { return tdclient.New(fakeSrv.URL) } + + createServer(ctx, dsnSpec()) + + // Pass 1: adds the finalizer and returns early. reconcileOnce(ctx) + srv := &terdutv1alpha1.TerdutServer{} + Expect(k8sClient.Get(ctx, objKey, srv)).To(Succeed()) + Expect(srv.Finalizers).To(ContainElement(finalizerName)) - cond := readyCondition(ctx) - Expect(cond.Status).To(Equal(metav1.ConditionFalse)) - Expect(cond.Reason).To(Equal(terdutv1alpha1.ReasonCredentialsSecretRefRequired)) - }) - }) - - When("the referenced Secret does not exist", func() { - It("reports Ready: False, reason CredentialsSecretNotFound", func(ctx SpecContext) { - createServer(ctx, terdutv1alpha1.TerdutServerSpec{ - Endpoint: unusedEndpoint, - CredentialsSecretRef: &terdutv1alpha1.SecretKeyRef{Name: "does-not-exist", Key: tokenKey}, - }) + // Pass 2: creates Deployment + Service, waits for readiness. reconcileOnce(ctx) + Expect(readyCondition(ctx).Reason).To(Equal(terdutv1alpha1.ReasonWaitingForDeployment)) - cond := readyCondition(ctx) - Expect(cond.Status).To(Equal(metav1.ConditionFalse)) - Expect(cond.Reason).To(Equal(terdutv1alpha1.ReasonCredentialsSecretNotFound)) - }) - }) - - When("the referenced Secret exists but has no data under the given key", func() { - It("reports Ready: False, reason CredentialsSecretInvalid", func(ctx SpecContext) { - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: name + "-creds", Namespace: operatorNamespace}, - Data: map[string][]byte{"wrong-key": []byte("tdsa_something")}, + var deploy appsv1.Deployment + Expect(k8sClient.Get(ctx, objKey, &deploy)).To(Succeed()) + Expect(deploy.Spec.Template.Spec.Containers[0].Image).To(Equal("example.invalid/terdut-server:test")) + envNames := map[string]string{} + for _, e := range deploy.Spec.Template.Spec.Containers[0].Env { + envNames[e.Name] = e.Value } - Expect(k8sClient.Create(ctx, secret)).To(Succeed()) - defer func() { _ = k8sClient.Delete(ctx, secret) }() + Expect(envNames).To(HaveKeyWithValue("TERDUT_DB_DSN", "postgres://terdut@test-postgres:5432/terdut?sslmode=require")) + Expect(envNames).To(HaveKeyWithValue("TERDUT_OPERATOR_MODE", "true")) - createServer(ctx, terdutv1alpha1.TerdutServerSpec{ - Endpoint: unusedEndpoint, - CredentialsSecretRef: &terdutv1alpha1.SecretKeyRef{Name: secret.Name, Key: tokenKey}, - }) + var svc corev1.Service + Expect(k8sClient.Get(ctx, objKey, &svc)).To(Succeed()) + Expect(svc.Spec.Ports[0].Port).To(Equal(int32(8080))) + + // Simulate the Deployment becoming ready (envtest has no + // kubelet/deployment-controller to do this for real). + markDeploymentReady(ctx) + + // Pass 3: bootstraps for real against the fake server. reconcileOnce(ctx) - cond := readyCondition(ctx) - Expect(cond.Status).To(Equal(metav1.ConditionFalse)) - Expect(cond.Reason).To(Equal(terdutv1alpha1.ReasonCredentialsSecretInvalid)) + Expect(fake.bootstrapped).To(BeTrue()) + + Expect(k8sClient.Get(ctx, objKey, srv)).To(Succeed()) + ready := meta.FindStatusCondition(srv.Status.Conditions, terdutv1alpha1.ConditionReady) + Expect(ready.Status).To(Equal(metav1.ConditionTrue)) + Expect(ready.Reason).To(Equal(terdutv1alpha1.ReasonAdopted)) + Expect(srv.Status.CredentialsSecretRef).NotTo(BeNil()) + Expect(srv.Status.CredentialsSecretRef.Key).To(Equal(credentialsSecretDataKey)) + + var credsSecret corev1.Secret + Expect(k8sClient.Get(ctx, types.NamespacedName{ + Name: srv.Status.CredentialsSecretRef.Name, Namespace: operatorNamespace, + }, &credsSecret)).To(Succeed()) + Expect(string(credsSecret.Data[credentialsSecretDataKey])).To(Equal("instance-key-1-initial")) + + // The checkpoint is cleaned up once the lasting credential is + // written (DESIGN.md §6 point 2). + var checkpoint corev1.Secret + err := k8sClient.Get(ctx, types.NamespacedName{Name: checkpointSecretName(srv), Namespace: operatorNamespace}, &checkpoint) + Expect(err).To(HaveOccurred()) }) }) - When("the Secret is valid but the server is unreachable", func() { - It("reports Ready: False, reason ServerUnreachable", func(ctx SpecContext) { - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: name + "-creds", Namespace: operatorNamespace}, - Data: map[string][]byte{tokenKey: []byte("tdsa_something")}, - } - Expect(k8sClient.Create(ctx, secret)).To(Succeed()) - defer func() { _ = k8sClient.Delete(ctx, secret) }() + Describe("the adopt-on-409 recovery path", func() { + It("mints a fresh key instead of erroring when the service account already exists", func(ctx SpecContext) { + fake, fakeSrv := newFakeTerdutServer() + DeferCleanup(fakeSrv.Close) + fake.bootstrapped = true // an earlier attempt already bootstrapped... + fake.nextID = 1 + fake.accounts[serviceAccountName] = 1 // ...and already created the service account. + fake.keyMints[1] = 1 + reconciler.NewClient = func(string) *tdclient.Client { return tdclient.New(fakeSrv.URL) } - createServer(ctx, terdutv1alpha1.TerdutServerSpec{ - // Port 1 is never listening (closed cleanly and immediately, - // unlike an unroutable address which would hang on a - // connect timeout) -- keeps the test fast. - Endpoint: "http://127.0.0.1:1", - CredentialsSecretRef: &terdutv1alpha1.SecretKeyRef{Name: secret.Name, Key: tokenKey}, - }) - reconcileOnce(ctx) + createServer(ctx, dsnSpec()) + reconcileOnce(ctx) // finalizer + reconcileOnce(ctx) // Deployment/Service, WaitingForDeployment + markDeploymentReady(ctx) - cond := readyCondition(ctx) - Expect(cond.Status).To(Equal(metav1.ConditionFalse)) - Expect(cond.Reason).To(Equal(terdutv1alpha1.ReasonServerUnreachable)) - }) - }) + // The earlier attempt's checkpoint survived (that's how this + // reconcile can authenticate at all to recover). + Expect(reconciler.writeSecret(ctx, checkpointSecretName(&terdutv1alpha1.TerdutServer{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: operatorNamespace}, + }), "admin-key-raw")).To(Succeed()) - When("the Secret is valid and the server answers /api/version", func() { - It("reports Ready: True, Bootstrapped: True, and mirrors credentialsSecretRef into status", func(ctx SpecContext) { - fake := fakeTerdutServer("v0.20.0") - DeferCleanup(fake.Close) - - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: name + "-creds", Namespace: operatorNamespace}, - Data: map[string][]byte{tokenKey: []byte("tdsa_something")}, - } - Expect(k8sClient.Create(ctx, secret)).To(Succeed()) - defer func() { _ = k8sClient.Delete(ctx, secret) }() - - createServer(ctx, terdutv1alpha1.TerdutServerSpec{ - Endpoint: fake.URL, - CredentialsSecretRef: &terdutv1alpha1.SecretKeyRef{Name: secret.Name, Key: tokenKey}, - }) reconcileOnce(ctx) srv := &terdutv1alpha1.TerdutServer{} Expect(k8sClient.Get(ctx, objKey, srv)).To(Succeed()) + Expect(meta.FindStatusCondition(srv.Status.Conditions, terdutv1alpha1.ConditionReady).Status).To(Equal(metav1.ConditionTrue)) - ready := meta.FindStatusCondition(srv.Status.Conditions, terdutv1alpha1.ConditionReady) - Expect(ready).NotTo(BeNil()) - Expect(ready.Status).To(Equal(metav1.ConditionTrue)) - Expect(ready.Reason).To(Equal(terdutv1alpha1.ReasonAdopted)) + var credsSecret corev1.Secret + Expect(k8sClient.Get(ctx, types.NamespacedName{ + Name: srv.Status.CredentialsSecretRef.Name, Namespace: operatorNamespace, + }, &credsSecret)).To(Succeed()) + // Minted fresh, not the (never-seen-by-this-reconcile) "initial" + // key from the account's original creation. + Expect(string(credsSecret.Data[credentialsSecretDataKey])).To(Equal("instance-key-1-mint2")) + }) + }) - bootstrapped := meta.FindStatusCondition(srv.Status.Conditions, terdutv1alpha1.ConditionBootstrapped) - Expect(bootstrapped).NotTo(BeNil()) - Expect(bootstrapped.Status).To(Equal(metav1.ConditionTrue)) + Describe("the BootstrapStateLost path", func() { + It("fails closed when the server reports already-bootstrapped with no checkpoint to recover from", func(ctx SpecContext) { + fake, fakeSrv := newFakeTerdutServer() + _ = fake + DeferCleanup(fakeSrv.Close) + fake.bootstrap403 = true + reconciler.NewClient = func(string) *tdclient.Client { return tdclient.New(fakeSrv.URL) } - Expect(srv.Status.CredentialsSecretRef).NotTo(BeNil()) - Expect(srv.Status.CredentialsSecretRef.Name).To(Equal(secret.Name)) - Expect(srv.Status.CredentialsSecretRef.Key).To(Equal(tokenKey)) - Expect(srv.Status.ObservedGeneration).To(Equal(srv.Generation)) + createServer(ctx, dsnSpec()) + reconcileOnce(ctx) // finalizer + reconcileOnce(ctx) // Deployment/Service + markDeploymentReady(ctx) + + reconcileOnce(ctx) + + cond := readyCondition(ctx) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(terdutv1alpha1.ReasonBootstrapStateLost)) + }) + }) + + Describe("the Zalando postgresClusterRef path", func() { + zalandoSpec := func(clusterName string) terdutv1alpha1.TerdutServerSpec { + s := dsnSpec() + s.Database = terdutv1alpha1.DatabaseSpec{ + PostgresClusterRef: &terdutv1alpha1.PostgresClusterRef{Name: clusterName}, + } + return s + } + + It("waits with reason PostgresClusterNotFound when the postgresql CR doesn't exist yet", func(ctx SpecContext) { + createServer(ctx, zalandoSpec("missing-cluster")) + reconcileOnce(ctx) // finalizer + reconcileOnce(ctx) + + Expect(readyCondition(ctx).Reason).To(Equal(terdutv1alpha1.ReasonPostgresClusterNotFound)) + }) + + It("resolves a real DSN and reaches Ready once the CR and its generated Secret both exist", func(ctx SpecContext) { + clusterName := "pg-" + name + cluster := &unstructured.Unstructured{} + cluster.SetGroupVersionKind(postgresqlGVK) + cluster.SetName(clusterName) + cluster.SetNamespace(operatorNamespace) + Expect(unstructured.SetNestedField(cluster.Object, map[string]any{}, "spec")).To(Succeed()) + Expect(k8sClient.Create(ctx, cluster)).To(Succeed()) + DeferCleanup(func() { _ = k8sClient.Delete(ctx, cluster) }) + + credsSecretName := fmt.Sprintf("terdut.%s.credentials.postgresql.acid.zalan.do", clusterName) + zalandoSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: credsSecretName, Namespace: operatorNamespace}, + Data: map[string][]byte{"password": []byte("whatever")}, + } + Expect(k8sClient.Create(ctx, zalandoSecret)).To(Succeed()) + DeferCleanup(func() { _ = k8sClient.Delete(ctx, zalandoSecret) }) + + fake, fakeSrv := newFakeTerdutServer() + _ = fake + DeferCleanup(fakeSrv.Close) + reconciler.NewClient = func(string) *tdclient.Client { return tdclient.New(fakeSrv.URL) } + + createServer(ctx, zalandoSpec(clusterName)) + reconcileOnce(ctx) // finalizer + reconcileOnce(ctx) // Deployment/Service created with resolved DB env + + var deploy appsv1.Deployment + Expect(k8sClient.Get(ctx, objKey, &deploy)).To(Succeed()) + var dsn string + for _, e := range deploy.Spec.Template.Spec.Containers[0].Env { + if e.Name == "TERDUT_DB_DSN" { + dsn = e.Value + } + } + Expect(dsn).To(Equal(fmt.Sprintf("postgres://terdut@%s.%s.svc:5432/terdut?sslmode=require", clusterName, operatorNamespace))) + + markDeploymentReady(ctx) + reconcileOnce(ctx) + + Expect(readyCondition(ctx).Status).To(Equal(metav1.ConditionTrue)) + }) + }) + + Describe("deletion", func() { + It("removes the credentials and checkpoint Secrets and the finalizer", func(ctx SpecContext) { + fake, fakeSrv := newFakeTerdutServer() + _ = fake + DeferCleanup(fakeSrv.Close) + reconciler.NewClient = func(string) *tdclient.Client { return tdclient.New(fakeSrv.URL) } + + createServer(ctx, dsnSpec()) + reconcileOnce(ctx) + reconcileOnce(ctx) + markDeploymentReady(ctx) + reconcileOnce(ctx) + + srv := &terdutv1alpha1.TerdutServer{} + Expect(k8sClient.Get(ctx, objKey, srv)).To(Succeed()) + credsName := srv.Status.CredentialsSecretRef.Name + + Expect(k8sClient.Delete(ctx, srv)).To(Succeed()) + reconcileOnce(ctx) // runs the finalizer + + err := k8sClient.Get(ctx, objKey, srv) + Expect(err).To(HaveOccurred(), "the TerdutServer itself should be gone once the finalizer clears") + + var leftover corev1.Secret + err = k8sClient.Get(ctx, types.NamespacedName{Name: credsName, Namespace: operatorNamespace}, &leftover) + Expect(err).To(HaveOccurred(), "the credentials Secret should have been cleaned up by the finalizer") }) }) @@ -205,12 +407,12 @@ var _ = Describe("TerdutServer Controller", func() { }) }) -var _ = Describe("TerdutServerReconciler sanity", func() { - It("treats a real apierrors.IsNotFound the same as any other caller would", func() { - // Guards against a refactor accidentally swapping in a different - // not-found check that stops matching what client.Client actually - // returns. - Expect(apierrors.IsNotFound(apierrors.NewNotFound( - terdutv1alpha1.GroupVersion.WithResource("terdutservers").GroupResource(), "x"))).To(BeTrue()) - }) -}) +// checkpointSecretNameFor/credentialsSecretNameFor let AfterEach clean up +// without needing a live TerdutServer object (it may already be gone by +// then in the deletion test). +func checkpointSecretNameFor(name string) string { + return fmt.Sprintf("default.%s-bootstrap-admin", name) +} +func credentialsSecretNameFor(name string) string { + return fmt.Sprintf("default.%s-instance-credentials", name) +} diff --git a/internal/controller/terdutserver_database.go b/internal/controller/terdutserver_database.go new file mode 100644 index 0000000..d8a9520 --- /dev/null +++ b/internal/controller/terdutserver_database.go @@ -0,0 +1,134 @@ +package controller + +import ( + "context" + "fmt" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" + + terdutv1alpha1 "git.ryuvia.com/niklas/terdut-operator/api/v1alpha1" +) + +// databaseError carries a condition reason/message out of resolveDatabaseEnv +// — distinct from a plain error, since "the database isn't ready yet" is an +// expected, requeue-and-retry outcome (DESIGN.md §8), not a reconcile +// failure. +type databaseError struct { + reason string + message string +} + +func (e *databaseError) Error() string { return e.message } + +// classifyClusterGetError turns a failed Get of the postgresql.acid.zalan.do +// CR into the right condition — distinct from a plain reconcile error, and +// its own function (not inlined) so it's unit-testable against a +// hand-constructed error without needing a real client at all: a +// *meta.NoKindMatchError means the CRD itself isn't installed +// (ReasonPostgresOperatorCRDNotInstalled, §8/§9's graceful-degradation +// case), apierrors.IsNotFound means the CRD exists but this particular +// object doesn't (yet) (ReasonPostgresClusterNotFound). +func classifyClusterGetError(err error, clusterName, namespace string) *databaseError { + switch { + case meta.IsNoMatchError(err): + return &databaseError{ + reason: terdutv1alpha1.ReasonPostgresOperatorCRDNotInstalled, + message: "spec.database.postgresClusterRef is set but the postgresql.acid.zalan.do CRD isn't installed in this cluster", + } + case apierrors.IsNotFound(err): + return &databaseError{ + reason: terdutv1alpha1.ReasonPostgresClusterNotFound, + message: fmt.Sprintf("postgresql.acid.zalan.do %q not found in namespace %q", clusterName, namespace), + } + default: + // Any other error (RBAC, transient API failure) is a real + // reconcile error, not a condition to report and wait out -- + // callers still surface it as a databaseError so resolveDatabaseEnv + // has one return shape, but Reconcile treats every databaseError as + // a wait-and-retry outcome today. Revisit if that ever matters in + // practice (a persistent RBAC misconfiguration looks identical to + // "not ready yet" until someone reads the condition message). + return &databaseError{reason: terdutv1alpha1.ReasonPostgresClusterNotFound, message: err.Error()} + } +} + +// resolveDatabaseEnv implements DESIGN.md §8's two Postgres paths. It never +// reads a password's value — only ever wires a secretKeyRef into the +// Deployment's env, the same way the chart does — so it needs no RBAC on +// Secret *data* for this, only on the postgresql.acid.zalan.do CR itself +// (the XValidation rule on TerdutServerSpec.Database guarantees exactly one +// of the two fields below is set, so there's no third case to handle). +func (r *TerdutServerReconciler) resolveDatabaseEnv(ctx context.Context, srv *terdutv1alpha1.TerdutServer) ([]corev1.EnvVar, *databaseError) { + db := srv.Spec.Database + + if db.PostgresClusterRef != nil { + return r.resolveZalandoDatabaseEnv(ctx, srv, db.PostgresClusterRef.Name) + } + + env := []corev1.EnvVar{{Name: "TERDUT_DB_DSN", Value: db.DSN}} + if db.PasswordSecretRef != nil { + env = append(env, corev1.EnvVar{ + Name: "PGPASSWORD", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: db.PasswordSecretRef.Name}, + Key: db.PasswordSecretRef.Key, + }, + }, + }) + } + return env, nil +} + +// resolveZalandoDatabaseEnv resolves a Zalando postgres-operator +// `postgresql` CR into a DSN + PGPASSWORD secretKeyRef, without reading the +// generated credentials Secret's value — only confirming it exists, the +// same "exists, don't read" posture the DSN path takes toward its own +// password Secret. +// +// By convention (DESIGN.md §8, PostgresClusterRef's own doc comment) the +// database and role are both named "terdut", and the primary Service is +// named after the cluster CR itself — Zalando's own naming convention, not +// something its status exposes as a field to read. +func (r *TerdutServerReconciler) resolveZalandoDatabaseEnv( + ctx context.Context, srv *terdutv1alpha1.TerdutServer, clusterName string, +) ([]corev1.EnvVar, *databaseError) { + cluster := &unstructured.Unstructured{} + cluster.SetGroupVersionKind(postgresqlGVK) + if err := r.Get(ctx, client.ObjectKey{Namespace: srv.Namespace, Name: clusterName}, cluster); err != nil { + return nil, classifyClusterGetError(err, clusterName, srv.Namespace) + } + + credsSecretName := fmt.Sprintf("terdut.%s.credentials.postgresql.acid.zalan.do", clusterName) + var secret corev1.Secret + if err := r.Get(ctx, client.ObjectKey{Namespace: srv.Namespace, Name: credsSecretName}, &secret); err != nil { + if apierrors.IsNotFound(err) { + return nil, &databaseError{ + reason: terdutv1alpha1.ReasonPostgresClusterNotFound, + message: fmt.Sprintf( + "postgresql.acid.zalan.do %q found, but its generated credentials Secret %q doesn't exist yet", + clusterName, credsSecretName), + } + } + return nil, &databaseError{reason: terdutv1alpha1.ReasonPostgresClusterNotFound, message: err.Error()} + } + + host := fmt.Sprintf("%s.%s.svc", clusterName, srv.Namespace) + dsn := fmt.Sprintf("postgres://terdut@%s:5432/terdut?sslmode=require", host) + return []corev1.EnvVar{ + {Name: "TERDUT_DB_DSN", Value: dsn}, + { + Name: "PGPASSWORD", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: credsSecretName}, + Key: "password", + }, + }, + }, + }, nil +} diff --git a/internal/controller/terdutserver_database_test.go b/internal/controller/terdutserver_database_test.go new file mode 100644 index 0000000..c518154 --- /dev/null +++ b/internal/controller/terdutserver_database_test.go @@ -0,0 +1,47 @@ +package controller + +import ( + "errors" + "testing" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime/schema" + + terdutv1alpha1 "git.ryuvia.com/niklas/terdut-operator/api/v1alpha1" +) + +// These exercise classifyClusterGetError directly, against hand-constructed +// errors, rather than through envtest -- the point is the classification +// logic, not a real Get, and the "CRD not installed" case specifically +// can't be produced in the shared envtest suite (its CRDs, stub included, +// are always installed for the whole suite's lifetime). + +func TestClassifyClusterGetError_CRDNotInstalled(t *testing.T) { + noMatch := &meta.NoKindMatchError{GroupKind: schema.GroupKind{Group: "acid.zalan.do", Kind: "postgresql"}} + + got := classifyClusterGetError(noMatch, "terdut-postgres", "oncall") + if got.reason != terdutv1alpha1.ReasonPostgresOperatorCRDNotInstalled { + t.Errorf("reason = %q, want %q", got.reason, terdutv1alpha1.ReasonPostgresOperatorCRDNotInstalled) + } +} + +func TestClassifyClusterGetError_ClusterNotFound(t *testing.T) { + notFound := apierrors.NewNotFound(schema.GroupResource{Group: "acid.zalan.do", Resource: "postgresqls"}, "terdut-postgres") + + got := classifyClusterGetError(notFound, "terdut-postgres", "oncall") + if got.reason != terdutv1alpha1.ReasonPostgresClusterNotFound { + t.Errorf("reason = %q, want %q", got.reason, terdutv1alpha1.ReasonPostgresClusterNotFound) + } +} + +func TestClassifyClusterGetError_OtherErrorStillReturnsACondition(t *testing.T) { + // Not one of the two classified cases -- still returns a databaseError + // (Reconcile treats every databaseError as wait-and-retry today; see + // the function's own comment on why), never nil, so a caller can't + // accidentally treat an unclassified error as success. + got := classifyClusterGetError(errors.New("etcd is on fire"), "terdut-postgres", "oncall") + if got == nil { + t.Fatal("classifyClusterGetError returned nil for an unclassified error") + } +} diff --git a/internal/controller/terdutserver_deployment.go b/internal/controller/terdutserver_deployment.go new file mode 100644 index 0000000..ea3094e --- /dev/null +++ b/internal/controller/terdutserver_deployment.go @@ -0,0 +1,219 @@ +package controller + +import ( + "context" + "fmt" + "strings" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + terdutv1alpha1 "git.ryuvia.com/niklas/terdut-operator/api/v1alpha1" +) + +// labelsFor returns the selector labels for srv's Deployment/Service — +// fixed once a Deployment exists (its selector is immutable), so this must +// never depend on anything in srv.Spec that could change later. +func labelsFor(srv *terdutv1alpha1.TerdutServer) map[string]string { + return map[string]string{ + "app.kubernetes.io/name": "terdut-server", + "app.kubernetes.io/instance": srv.Name, + } +} + +// reconcileDeployment creates or updates the Deployment running +// terdut-server, owned by srv (DESIGN.md §7: same-namespace generated +// objects carry a plain OwnerReference, no finalizer). Returns the live +// object (not just the desired one) so the caller can check +// status.readyReplicas. +func (r *TerdutServerReconciler) reconcileDeployment( + ctx context.Context, srv *terdutv1alpha1.TerdutServer, dbEnv []corev1.EnvVar, +) (*appsv1.Deployment, error) { + deploy := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: srv.Name, Namespace: srv.Namespace}} + _, err := controllerutil.CreateOrUpdate(ctx, r.Client, deploy, func() error { + replicas := srv.Spec.Replicas + if replicas == 0 { + replicas = 1 + } + labels := labelsFor(srv) + + deploy.Spec.Replicas = &replicas + deploy.Spec.Selector = &metav1.LabelSelector{MatchLabels: labels} + // Recreate, not RollingUpdate: the sweeper and the notifier are + // unsynchronised singletons inside terdut-server, and two replicas + // overlapping during a rollout would both page for the same + // incident (matches the chart's own deployment.yaml comment). + deploy.Spec.Strategy = appsv1.DeploymentStrategy{Type: appsv1.RecreateDeploymentStrategyType} + deploy.Spec.Template = corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{Labels: labels}, + Spec: corev1.PodSpec{ + EnableServiceLinks: new(false), + Containers: []corev1.Container{{ + Name: "terdut-server", + Image: fmt.Sprintf("%s:%s", srv.Spec.Image.Repository, srv.Spec.Image.Tag), + Ports: []corev1.ContainerPort{{ + Name: "http", + ContainerPort: servicePort(srv), + Protocol: corev1.ProtocolTCP, + }}, + Env: buildEnv(srv, dbEnv), + LivenessProbe: healthzProbe(), + ReadinessProbe: healthzProbe(), + }}, + }, + } + return controllerutil.SetControllerReference(srv, deploy, r.Scheme) + }) + if err != nil { + return nil, err + } + // CreateOrUpdate's own object is desired-state-only on a fresh create + // (no .status yet); re-fetch so the ready-replica check the caller does + // next sees the real, live object. + if err := r.Get(ctx, client.ObjectKeyFromObject(deploy), deploy); err != nil { + return nil, err + } + return deploy, nil +} + +// reconcileService creates or updates the ClusterIP Service in front of the +// Deployment, owned by srv. +func (r *TerdutServerReconciler) reconcileService(ctx context.Context, srv *terdutv1alpha1.TerdutServer) error { + svc := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: srv.Name, Namespace: srv.Namespace}} + _, err := controllerutil.CreateOrUpdate(ctx, r.Client, svc, func() error { + svc.Spec.Selector = labelsFor(srv) + svc.Spec.Ports = []corev1.ServicePort{{ + Name: "http", + Port: servicePort(srv), + TargetPort: intstr.FromString("http"), + Protocol: corev1.ProtocolTCP, + }} + return controllerutil.SetControllerReference(srv, svc, r.Scheme) + }) + return err +} + +func servicePort(srv *terdutv1alpha1.TerdutServer) int32 { + if srv.Spec.Networking.ServicePort == 0 { + return 8080 + } + return srv.Spec.Networking.ServicePort +} + +func healthzProbe() *corev1.Probe { + return &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/healthz", + Port: intstr.FromString("http"), + }, + }, + InitialDelaySeconds: 5, + } +} + +// buildEnv mirrors charts/terdut-server's own deployment.yaml env block +// field-for-field (confirmed against that source, not reconstructed from +// DESIGN.md's illustrative YAML alone) — dbEnv (TERDUT_DB_DSN, optionally +// PGPASSWORD) comes from resolveDatabaseEnv, since which of §8's two paths +// produced it doesn't matter past this point. +func buildEnv(srv *terdutv1alpha1.TerdutServer, dbEnv []corev1.EnvVar) []corev1.EnvVar { + env := []corev1.EnvVar{{Name: "TERDUT_ADDR", Value: fmt.Sprintf(":%d", servicePort(srv))}} + env = append(env, dbEnv...) + + sweeper := srv.Spec.Sweeper + env = append(env, + corev1.EnvVar{Name: "TERDUT_STALE_AFTER", Value: sweeper.StaleAfter}, + corev1.EnvVar{Name: "TERDUT_ARCHIVE_AFTER", Value: sweeper.ArchiveAfter}, + ) + + deadman := srv.Spec.Deadman + env = append(env, + corev1.EnvVar{Name: "TERDUT_DEADMAN_MATCHERS", Value: deadman.Matchers}, + corev1.EnvVar{Name: "TERDUT_DEADMAN_TIMEOUT", Value: deadman.Timeout}, + corev1.EnvVar{Name: "TERDUT_DEADMAN_SEVERITY", Value: deadman.Severity}, + ) + + notify := srv.Spec.Notify + if notify.NtfyURL != "" { + env = append(env, + corev1.EnvVar{Name: "TERDUT_NTFY_URL", Value: notify.NtfyURL}, + corev1.EnvVar{Name: "TERDUT_NTFY_FALLBACK_TOPIC", Value: notify.FallbackTopic}, + corev1.EnvVar{Name: "TERDUT_NOTIFY_REPEAT", Value: notify.RepeatEvery}, + ) + if notify.TokenSecretRef != nil { + env = append(env, corev1.EnvVar{ + Name: "TERDUT_NTFY_TOKEN", + ValueFrom: secretEnvSource(notify.TokenSecretRef), + }) + } + } + + publicURL := fmt.Sprintf("https://%s", srv.Spec.Networking.Hostname) + env = append(env, + corev1.EnvVar{Name: "TERDUT_PUBLIC_URL", Value: publicURL}, + corev1.EnvVar{Name: "TERDUT_PASSWORD_LOGIN", Value: boolString(srv.Spec.PasswordLogin)}, + // Always on, unlike the chart's own default-off operatorMode: every + // write this operator's own TerdutTeam/EscalationRule/etc. + // controllers make goes through a service account already, and the + // whole reason to run this operator is gitops-managed config, not a + // human editing an operator-managed install's teams/policies by + // hand (chart's values.yaml comment: "a statement that something + // like terdut-operator... owns this install's configuration from + // here on" -- which is unconditionally true for anything this + // operator creates). + corev1.EnvVar{Name: "TERDUT_OPERATOR_MODE", Value: "true"}, + ) + + if oidc := srv.Spec.OIDC; oidc.Enabled { + env = append(env, + corev1.EnvVar{Name: "TERDUT_OIDC_ISSUER", Value: oidc.Issuer}, + corev1.EnvVar{Name: "TERDUT_OIDC_CLIENT_ID", Value: oidc.ClientID}, + corev1.EnvVar{Name: "TERDUT_OIDC_NAME", Value: oidc.Name}, + corev1.EnvVar{Name: "TERDUT_OIDC_SCOPES", Value: oidc.Scopes}, + // terdut-server's own defaults for the claims/trust-email knobs + // the chart exposes but DESIGN.md's spec doesn't (§4.1's doc + // comment on OIDCSpec) -- not configurable here, not an + // oversight. + corev1.EnvVar{Name: "TERDUT_OIDC_USERNAME_CLAIM", Value: "preferred_username"}, + corev1.EnvVar{Name: "TERDUT_OIDC_EMAIL_CLAIM", Value: "email"}, + corev1.EnvVar{Name: "TERDUT_OIDC_GROUPS_CLAIM", Value: "groups"}, + corev1.EnvVar{Name: "TERDUT_OIDC_TRUST_EMAIL", Value: "false"}, + corev1.EnvVar{Name: "TERDUT_OIDC_SESSION_MAX_AGE", Value: oidc.SessionMaxAge}, + ) + if oidc.ClientSecretRef != nil { + env = append(env, corev1.EnvVar{ + Name: "TERDUT_OIDC_CLIENT_SECRET", + ValueFrom: secretEnvSource(oidc.ClientSecretRef), + }) + } + if len(oidc.AllowedGroups) > 0 { + env = append(env, corev1.EnvVar{Name: "TERDUT_OIDC_ALLOWED_GROUPS", Value: strings.Join(oidc.AllowedGroups, ",")}) + } + if oidc.AdminGroup != "" { + env = append(env, corev1.EnvVar{Name: "TERDUT_OIDC_ADMIN_GROUP", Value: oidc.AdminGroup}) + } + } + + return env +} + +func secretEnvSource(ref *terdutv1alpha1.SecretKeyRef) *corev1.EnvVarSource { + return &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: ref.Name}, + Key: ref.Key, + }, + } +} + +func boolString(b bool) string { + if b { + return "true" + } + return "false" +} diff --git a/internal/controller/testdata/postgresql.acid.zalan.do.yaml b/internal/controller/testdata/postgresql.acid.zalan.do.yaml new file mode 100644 index 0000000..dc779d3 --- /dev/null +++ b/internal/controller/testdata/postgresql.acid.zalan.do.yaml @@ -0,0 +1,30 @@ +# A deliberately minimal, permissive stub of Zalando postgres-operator's +# `postgresql` CRD -- test-only, installed into envtest so suite_test.go can +# create fixture `postgresql.acid.zalan.do` objects without a real +# postgres-operator controller running. This is NOT the real upstream CRD +# (which has a much larger structural schema); it exists purely so this +# repo's own controller logic -- which only ever Gets this object by name +# and never inspects its spec/status fields beyond existence -- has +# something matching the real GVK to read. A real cluster installs the +# genuine CRD via the postgres-operator chart; this file is never applied +# there. +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: postgresqls.acid.zalan.do +spec: + group: acid.zalan.do + names: + kind: postgresql + listKind: postgresqlList + plural: postgresqls + singular: postgresql + scope: Namespaced + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + x-kubernetes-preserve-unknown-fields: true diff --git a/internal/tdclient/client.go b/internal/tdclient/client.go index 1d523fb..7f53bea 100644 --- a/internal/tdclient/client.go +++ b/internal/tdclient/client.go @@ -5,10 +5,6 @@ // shape must be mirrored" convention) but authorizes with a Bearer API key // rather than a session cookie — the operator never signs in as a human // (DESIGN.md §6). -// -// Deliberately minimal for Stage 1 (ROADMAP.md): only Version, the -// reachability probe TerdutServer's controller needs. Bootstrap and the -// service-account endpoints land here once self-registration does (Stage 5). package tdclient import ( @@ -22,7 +18,7 @@ import ( // Client talks to one terdut-server install, optionally as a service // account. A zero-value token works for endpoints that don't need one -// (Version). +// (Version, Bootstrap). type Client struct { baseURL string httpClient *http.Client @@ -68,12 +64,29 @@ func statusError(resp *http.Response) error { return &StatusError{Code: resp.StatusCode, Message: e.Error} } -func (c *Client) newRequest(ctx context.Context, method, path string) (*http.Request, error) { - req, err := http.NewRequestWithContext(ctx, method, c.baseURL+path, nil) +func (c *Client) newRequest(ctx context.Context, method, path string, body any) (*http.Request, error) { + var reader *strings.Reader + if body != nil { + data, err := json.Marshal(body) + if err != nil { + return nil, err + } + reader = strings.NewReader(string(data)) + } + var req *http.Request + var err error + if reader != nil { + req, err = http.NewRequestWithContext(ctx, method, c.baseURL+path, reader) + } else { + req, err = http.NewRequestWithContext(ctx, method, c.baseURL+path, nil) + } if err != nil { return nil, err } req.Header.Set("Accept", "application/json") + if body != nil { + req.Header.Set("Content-Type", "application/json") + } if c.token != "" { req.Header.Set("Authorization", "Bearer "+c.token) } @@ -101,7 +114,7 @@ func (c *Client) do(req *http.Request, out any) error { // for it"). Used here purely as a reachability probe: a bad endpoint fails // here, clearly, rather than on whatever the controller tries first. func (c *Client) Version(ctx context.Context) (string, error) { - req, err := c.newRequest(ctx, http.MethodGet, "/api/version") + req, err := c.newRequest(ctx, http.MethodGet, "/api/version", nil) if err != nil { return "", err } @@ -113,3 +126,126 @@ func (c *Client) Version(ctx context.Context) (string, error) { } return v.Version, nil } + +// APIKey is the raw key a bootstrap or service-account-key mint hands back — +// the one moment its value exists outside the request that generated it. +// Mirrors terdut-server's models.APIKey/models.ServiceAccountKey shape +// (internal/models in that repo) for the fields this client actually reads. +type APIKey struct { + ID int64 `json:"id"` + Name string `json:"name"` + Key string `json:"key"` + CreatedAt string `json:"created_at"` +} + +// BootstrapResult is /api/bootstrap's 201 response body. +type BootstrapResult struct { + User struct { + ID int64 `json:"id"` + Username string `json:"username"` + Email string `json:"email"` + } `json:"user"` + APIKey APIKey `json:"api_key"` +} + +// Bootstrap calls POST /api/bootstrap — unauthenticated, single-shot per +// install (internal/api/users.go's handleBootstrap in terdut-server: +// gated on SELECT COUNT(*) FROM users). Returns the raw admin key directly; +// DESIGN.md §6 has the controller use it for exactly one further call +// (CreateServiceAccount) and discard it, never storing it as the lasting +// credential. +// +// A StatusError with Code 403 means this install already has a user — +// per this operator's design (DESIGN.md §1), that only happens if this +// exact TerdutServer's own controller already won this race on an earlier, +// interrupted reconcile; see the checkpoint-Secret handling in the +// controller, not a retry loop here. +func (c *Client) Bootstrap(ctx context.Context, username, email string) (*BootstrapResult, error) { + req, err := c.newRequest(ctx, http.MethodPost, "/api/bootstrap", map[string]string{ + "username": username, + "email": email, + }) + if err != nil { + return nil, err + } + var result BootstrapResult + if err := c.do(req, &result); err != nil { + return nil, err + } + return &result, nil +} + +// ServiceAccount mirrors terdut-server's models.ServiceAccount +// (internal/models/service_account.go), minus fields this client never +// reads. +type ServiceAccount struct { + ID int64 `json:"id"` + Name string `json:"name"` + Scope string `json:"scope"` +} + +// CreateServiceAccountResult is POST /api/service-accounts' 201 response. +type CreateServiceAccountResult struct { + ServiceAccount ServiceAccount `json:"service_account"` + Key APIKey `json:"key"` +} + +// CreateInstanceServiceAccount calls POST /api/service-accounts with +// scope "instance", authenticated with c's current token (the raw admin key +// from Bootstrap, for the operator's own first-ever call). A StatusError +// with Code 409 means a prior, interrupted attempt already created this +// name — DESIGN.md §6's adopt-rather-than-error rule: the caller should +// fall back to GetServiceAccountByName + CreateServiceAccountKey, not treat +// this as a hard failure. +func (c *Client) CreateInstanceServiceAccount(ctx context.Context, name string) (*CreateServiceAccountResult, error) { + req, err := c.newRequest(ctx, http.MethodPost, "/api/service-accounts", map[string]string{ + "name": name, + "scope": "instance", + }) + if err != nil { + return nil, err + } + var result CreateServiceAccountResult + if err := c.do(req, &result); err != nil { + return nil, err + } + return &result, nil +} + +// GetServiceAccountByName calls GET /api/service-accounts?name=... -- +// authenticated (terdut-server's AuthMiddleware hard-rejects any +// unauthenticated request before this endpoint's own, more permissive +// internal check ever runs; DESIGN.md §6). Returns nil, nil if nothing +// matches, not an error -- the server's own distinction between "found +// nothing" and "the call failed". +func (c *Client) GetServiceAccountByName(ctx context.Context, name string) (*ServiceAccount, error) { + req, err := c.newRequest(ctx, http.MethodGet, "/api/service-accounts?name="+name, nil) + if err != nil { + return nil, err + } + var accounts []ServiceAccount + if err := c.do(req, &accounts); err != nil { + return nil, err + } + if len(accounts) == 0 { + return nil, nil + } + return &accounts[0], nil +} + +// CreateServiceAccountKey calls POST /api/service-accounts/{id}/keys to +// mint an additional key on an existing account -- rotation (DESIGN.md §6 +// point 6), and the adopt-on-409 recovery path in point 1. +func (c *Client) CreateServiceAccountKey(ctx context.Context, serviceAccountID int64, name string) (*APIKey, error) { + req, err := c.newRequest(ctx, http.MethodPost, + fmt.Sprintf("/api/service-accounts/%d/keys", serviceAccountID), + map[string]string{"name": name}) + if err != nil { + return nil, err + } + var key APIKey + if err := c.do(req, &key); err != nil { + return nil, err + } + return &key, nil +}