diff --git a/AGENTS.md b/AGENTS.md index d32c9d2f..592148ea 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -96,6 +96,24 @@ if err != nil { } ``` +### Config files: microcephd is the sole writer, dqlite is the source of truth + +All on-disk config files (`ceph.conf`, keyrings, `radosgw.conf`, NFS +`ganesha.conf`, etc.) are written **solely by `microcephd`**, and **dqlite is the +source of truth** for their contents. A config file is a projection of cluster +state held in the database, not an input to it. Consequences: + +- Persist every setting that ends up in a config file in the database (config + table, service records, or a dedicated table), then **render the file from the + database**. Do not let a value live only on disk. +- Config files must be reproducible: a member must be able to regenerate an + identical file from dqlite after a restart, snap refresh, or rejoin. +- Never treat an on-disk config file as authoritative state to read back. If you + need to know a member's current setting, read it from the database. +- Operator hand-edits to config files are out of scope and unsupported; the + daemon may overwrite them on the next reconcile. + + ## Building and installing locally Build the snap: diff --git a/docs/.custom_wordlist.txt b/docs/.custom_wordlist.txt index 54f776b7..9c2f29c7 100644 --- a/docs/.custom_wordlist.txt +++ b/docs/.custom_wordlist.txt @@ -2,6 +2,7 @@ backend backends bitmask +boolean Charmcraft cjk cryptographically diff --git a/docs/snap/reference/declarative-placement.rst b/docs/snap/reference/declarative-placement.rst new file mode 100644 index 00000000..bd58a152 --- /dev/null +++ b/docs/snap/reference/declarative-placement.rst @@ -0,0 +1,221 @@ +.. meta:: + :description: Reference information for the MicroCeph declarative placement API, including the policy schema, capability markers, and the supported ownership model. + +.. _declarative-placement: + +Declarative placement +===================== + +Declarative placement lets an external orchestrator, such as the MicroCeph +charm, describe which cluster members should run which services. The +orchestrator submits a desired-state policy and MicroCeph reconciles the +cluster to match it, owning all destructive-operation safety itself. + +Placement is an API-only surface. There is no ``microceph`` subcommand for it; +it is intended to be driven by an orchestrator rather than by hand. + +Capability markers +------------------ + +Before using placement, a consumer should confirm the running snap revision +supports it by querying ``GET /1.0/cluster/capabilities``. The markers relevant +to placement are: + +.. list-table:: + :header-rows: 1 + + * - Marker + - Meaning + * - ``declarative-placement`` + - The ``/1.0/placement`` endpoints exist and control services (MON, MGR, + MDS) are reconciled. + * - ``placement-rgw`` + - The ``rgw`` field accepts an object carrying frontend configuration, and + ``GET`` reports an observed ``rgw_frontend``. + +A consumer that needs RGW placement must gate on ``placement-rgw``. A snap +revision without it rejects the object-shaped ``rgw`` field with HTTP 400. + +Endpoints +--------- + +.. list-table:: + :header-rows: 1 + + * - Method + - Path + - Effect + * - ``PUT`` + - ``/1.0/placement`` + - Apply a policy, then store it as the declared intent. + * - ``GET`` + - ``/1.0/placement`` + - Return the declared policy, the observed placement, and lifecycle state. + * - ``DELETE`` + - ``/1.0/placement`` + - Clear the declared policy. Services are not added or removed. + +Policy schema +------------- + +The ``PUT`` body is a policy document. ``mode`` is required and must be +``reconcile``; an omitted or unknown mode is rejected, so a policy written for +a future mode fails loudly against an older snap rather than being silently +applied. + +.. code-block:: json + + { + "mode": "reconcile", + "members": { + "node-a": { "control": true }, + "node-b": { "control": false }, + "node-c": { "rgw": { "enabled": true, "port": 80 } } + } + } + +Members absent from ``members`` are never touched. For members that are +present, an omitted field leaves that service untouched on that member, which +is distinct from an explicit ``false`` requesting removal. + +.. list-table:: + :header-rows: 1 + + * - Field + - Type + - Effect + * - ``control`` + - boolean + - ``true`` places MON, MGR and MDS on the member; ``false`` removes them, + subject to the keep-one invariant below. + * - ``rgw`` + - object + - ``{"enabled": true}`` places RGW; ``{"enabled": false}`` removes it. See + :ref:`declarative-placement-rgw` below. + * - ``nfs`` + - array + - Accepted and reported, but not yet reconciled by the placement engine. + * - ``storage_eligible`` + - boolean + - Accepted and reported, but not yet enforced by the disk APIs. + +Safety rules +------------ + +The engine applies additions before removals, so a migration brings the +replacement up before tearing the previous instance down. + +Control services are further protected by a **keep-one invariant**: the engine +refuses to remove the last viable MON, MGR or MDS. Viability is checked against +Ceph itself (MON quorum, MGR active or standby, MDS up) rather than against +database records, so a service that exists but is not healthy is still a +removal target while never counting as the last retainer. + +RGW has no keep-one invariant. It is a stateless gateway that may be scaled to +zero, so a policy that disables RGW on every member is honoured. + +If a removal is refused, the requested additions remain in effect, the policy +is still stored as the declared intent, and the reason is reported in the +``placement_refusal`` field of ``GET /1.0/placement``. + +.. _declarative-placement-rgw: + +RGW placement +------------- + +The ``rgw`` field carries both placement intent and beast frontend +configuration, so that presence and frontend settings are applied together. + +.. list-table:: + :header-rows: 1 + + * - Field + - Type + - Notes + * - ``enabled`` + - boolean + - Whether the member should run RGW. + * - ``port`` + - integer + - Unencrypted listener port. Defaults to 80 when no TLS material is given. + * - ``ssl_port`` + - integer + - TLS listener port. Ignored unless both certificate and key are given. + * - ``ssl_certificate`` + - string + - base64-encoded PEM certificate. + * - ``ssl_private_key`` + - string + - base64-encoded PEM private key. + +Re-applying the same configuration is a no-op: the member compares the desired +frontend against what is on disk and restarts RGW only when something actually +changed. Rotating a certificate is therefore an ordinary ``PUT`` with the new +material. + +TLS key material is write-only. It travels over the authenticated API to the +member that needs it and is written to disk there, but it is stripped before +the policy is stored and is never returned by ``GET``. The observed +``rgw_frontend`` reports ports and a TLS on/off flag only. A consumer that +manages TLS must therefore hold its own copy of the material; it cannot read it +back from MicroCeph. + +Reading placement state +----------------------- + +``GET /1.0/placement`` returns the declared policy alongside an ``observed`` +list describing what each member is actually running. Comparing the two is the +supported way to determine whether the cluster has converged. + +Response codes +-------------- + +.. list-table:: + :header-rows: 1 + + * - Code + - Meaning + * - 400 + - The request cannot be satisfied: Ceph is not bootstrapped, the policy + names an unknown member, the RGW frontend configuration is malformed, or + a removal was refused by the keep-one invariant. + * - 409 + - Another placement apply, or a Ceph bootstrap, is in progress. The + request can be retried. + +.. _declarative-placement-ownership: + +Ownership model +--------------- + +Placement applies are serialised cluster-wide, so concurrent policy +submissions cannot interleave. That serialisation covers the placement API +only. + +.. important:: + + Managing the same services through both the placement API and the + ``microceph enable`` / ``microceph disable`` commands is **not a supported + configuration**. Pick one owner for a given cluster. + +The two mechanisms are independent write paths. The service commands do not +participate in the placement lock, do not consult the declared policy, and are +not subject to the keep-one invariant. Consequently, on a cluster driven by an +orchestrator: + +- Enabling or disabling a service by hand puts the cluster out of step with the + declared policy. Nothing corrects this until the orchestrator submits its + next policy, at which point the manual change is reverted without warning. +- A service command issued while an apply is in flight can interleave with it. + Observed state may briefly disagree with what is running until the next + policy is applied. +- The keep-one invariant does not protect manual removals. ``microceph disable + mon`` will remove a MON that the placement engine would have refused to + remove. + +Divergence is visible: ``GET /1.0/placement`` reports observed placement +alongside the declared policy, so the gap can be inspected at any time. + +This restriction applies only to services that placement manages. On a cluster +that does not use placement, the service commands remain the normal way to +manage services and are fully supported. diff --git a/docs/snap/reference/index.rst b/docs/snap/reference/index.rst index 0edfdb07..0bebb781 100644 --- a/docs/snap/reference/index.rst +++ b/docs/snap/reference/index.rst @@ -24,6 +24,19 @@ with ``microceph help``. commands/index +Declarative placement +--------------------- + +The declarative placement section describes the API used by an orchestrator, +such as the MicroCeph charm, to control which cluster members run which +services, and the ownership model that applies when it is in use. + +.. toctree:: + :maxdepth: 1 + + declarative-placement + + Release Notes ------------- diff --git a/microceph/api/capabilities.go b/microceph/api/capabilities.go index 8dd48c8c..c20bf952 100644 --- a/microceph/api/capabilities.go +++ b/microceph/api/capabilities.go @@ -15,6 +15,11 @@ var CapabilitiesSupported = []string{ "deferred-ceph-bootstrap", "ceph-only-bootstrap", "declarative-placement", + // placement-rgw: object-shaped rgw placement (enabled/port/ssl_port/SSL + // material) plus an observed rgw_frontend in GET /placement (CE142 Option B). + // The charm gates role-managed RGW on this marker; a snap without it would + // 400 on the object payload via DisallowUnknownFields. + "placement-rgw", } // capabilitiesCmd is the cluster capabilities endpoint (CE142). diff --git a/microceph/api/capabilities_test.go b/microceph/api/capabilities_test.go index 8543ec10..7339d522 100644 --- a/microceph/api/capabilities_test.go +++ b/microceph/api/capabilities_test.go @@ -11,8 +11,8 @@ import ( "github.com/stretchr/testify/require" ) -// TestCapabilitiesGet verifies that cmdCapabilitiesGet returns the 3 CE142 -// capability markers. +// TestCapabilitiesGet verifies that cmdCapabilitiesGet returns the CE142 +// capability markers, including placement-rgw for object-shaped RGW placement. func TestCapabilitiesGet(t *testing.T) { rec := httptest.NewRecorder() req := httptest.NewRequest(http.MethodGet, "/1.0/cluster/capabilities", nil) @@ -32,4 +32,5 @@ func TestCapabilitiesGet(t *testing.T) { assert.Contains(t, raw.Metadata.Supported, "deferred-ceph-bootstrap") assert.Contains(t, raw.Metadata.Supported, "ceph-only-bootstrap") assert.Contains(t, raw.Metadata.Supported, "declarative-placement") + assert.Contains(t, raw.Metadata.Supported, "placement-rgw", "object-shaped RGW placement capability must be advertised") } diff --git a/microceph/api/placement.go b/microceph/api/placement.go index 59ddd17a..fcf635bc 100644 --- a/microceph/api/placement.go +++ b/microceph/api/placement.go @@ -51,7 +51,8 @@ const placementPutTimeout = 10 * time.Minute func isClientSidePlacementError(err error) bool { return errors.Is(err, ceph.ErrCephNotBootstrapped) || errors.Is(err, ceph.ErrUnknownPlacementMember) || - errors.Is(err, ceph.ErrKeepOneInvariant) + errors.Is(err, ceph.ErrKeepOneInvariant) || + errors.Is(err, ceph.ErrRgwFrontendInvalid) } // inProgressResponse maps an "already in progress" sentinel (placement apply or @@ -147,7 +148,7 @@ func cmdPlacementPut(s mcTypes.State, r *http.Request) mcTypes.Response { // intent, and last_refusal records what failed so the caller can retry // the same policy to converge. if errors.Is(applyErr, ceph.ErrKeepOneInvariant) { - storeErr := ceph.StorePlacementPolicyFunc(ctx, interfaces.CephState{State: s}, policy) + storeErr := ceph.StorePlacementPolicyFunc(ctx, interfaces.CephState{State: s}, ceph.PolicyForStorage(policy)) if storeErr != nil { logger.Warnf("failed to store placement policy after keep-one refusal: %v", storeErr) } @@ -176,8 +177,11 @@ func cmdPlacementPut(s mcTypes.State, r *http.Request) mcTypes.Response { logger.Warnf("failed to clear placement refusal: %v", clearErr) } - // Persist the policy only after successful application. - err = ceph.StorePlacementPolicyFunc(ctx, interfaces.CephState{State: s}, policy) + // Persist the policy only after successful application. Strip RGW SSL key + // material before storage (CE142 Option B secrets posture): the cert/key + // travel over the authenticated API to the member that needs them, then are + // dropped so they are never persisted in dqlite. + err = ceph.StorePlacementPolicyFunc(ctx, interfaces.CephState{State: s}, ceph.PolicyForStorage(policy)) if err != nil { logger.Errorf("failed to store placement policy: %v", err) return mcTypes.InternalError(err) diff --git a/microceph/api/placement_test.go b/microceph/api/placement_test.go index 7affd34e..87c8a0e1 100644 --- a/microceph/api/placement_test.go +++ b/microceph/api/placement_test.go @@ -639,6 +639,124 @@ func TestCephBootstrapPutUnknownFieldRejected(t *testing.T) { assert.Equal(t, http.StatusBadRequest, rec.Code, "unknown field must be rejected with 400") } +// TestPlacementPutStripsSSLMaterialOnStore verifies that cmdPlacementPut stores +// the policy with the RGW SSL certificate and private key stripped (via +// ceph.PolicyForStorage), while enabled/port/ssl_port are retained so GET +// /placement still reports the declared frontend intent. +func TestPlacementPutStripsSSLMaterialOnStore(t *testing.T) { + stubPlacementApplyLock(t) + var storedPolicy types.PlacementPolicy + storeCalled := false + origApply := ceph.ApplyPlacementFunc + origStore := ceph.StorePlacementPolicyFunc + origRefusal := ceph.SetPlacementRefusalFunc + ceph.ApplyPlacementFunc = func(_ context.Context, _ interfaces.StateInterface, _ types.PlacementPolicy) error { + return nil + } + ceph.StorePlacementPolicyFunc = func(_ context.Context, _ interfaces.StateInterface, p types.PlacementPolicy) error { + storeCalled = true + storedPolicy = p + return nil + } + ceph.SetPlacementRefusalFunc = func(_ context.Context, _ interfaces.StateInterface, _ string) error { return nil } + defer func() { + ceph.ApplyPlacementFunc = origApply + ceph.StorePlacementPolicyFunc = origStore + ceph.SetPlacementRefusalFunc = origRefusal + }() + + body := `{"mode":"reconcile","members":{"node-a":{"rgw":{"enabled":true,"port":8080,"ssl_port":443,"ssl_certificate":"Y2VydA==","ssl_private_key":"a2V5"}}}}` + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPut, "/1.0/placement", strings.NewReader(body)) + + resp := cmdPlacementPut(nil, req) + _ = resp.Render(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.True(t, storeCalled) + require.NotNil(t, storedPolicy.Members["node-a"].Rgw, "rgw entry must be stored") + assert.Empty(t, storedPolicy.Members["node-a"].Rgw.SSLCertificate, "cert must be stripped before storage") + assert.Empty(t, storedPolicy.Members["node-a"].Rgw.SSLPrivateKey, "key must be stripped before storage") + assert.True(t, storedPolicy.Members["node-a"].Rgw.Enabled, "enabled must be retained") + assert.Equal(t, 8080, storedPolicy.Members["node-a"].Rgw.Port, "port must be retained") + assert.Equal(t, 443, storedPolicy.Members["node-a"].Rgw.SSLPort, "ssl_port must be retained") +} + +// TestPlacementPutRGWObjectDecodes verifies that an object-shaped rgw field +// decodes and reaches ApplyPlacement with the full frontend payload (clean +// break: the wire type is the object, not a bool). +func TestPlacementPutRGWObjectDecodes(t *testing.T) { + stubPlacementApplyLock(t) + var appliedPolicy types.PlacementPolicy + origApply := ceph.ApplyPlacementFunc + origStore := ceph.StorePlacementPolicyFunc + origRefusal := ceph.SetPlacementRefusalFunc + ceph.ApplyPlacementFunc = func(_ context.Context, _ interfaces.StateInterface, p types.PlacementPolicy) error { + appliedPolicy = p + return nil + } + ceph.StorePlacementPolicyFunc = func(_ context.Context, _ interfaces.StateInterface, _ types.PlacementPolicy) error { return nil } + ceph.SetPlacementRefusalFunc = func(_ context.Context, _ interfaces.StateInterface, _ string) error { return nil } + defer func() { + ceph.ApplyPlacementFunc = origApply + ceph.StorePlacementPolicyFunc = origStore + ceph.SetPlacementRefusalFunc = origRefusal + }() + + body := `{"mode":"reconcile","members":{"node-a":{"rgw":{"enabled":true,"port":80,"ssl_port":443,"ssl_certificate":"Y2VydA==","ssl_private_key":"a2V5"}}}}` + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPut, "/1.0/placement", strings.NewReader(body)) + + resp := cmdPlacementPut(nil, req) + _ = resp.Render(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code, "object-shaped rgw must decode and apply") + require.NotNil(t, appliedPolicy.Members["node-a"].Rgw) + assert.True(t, appliedPolicy.Members["node-a"].Rgw.Enabled) + assert.Equal(t, "Y2VydA==", appliedPolicy.Members["node-a"].Rgw.SSLCertificate, "SSL material reaches the apply path") +} + +// TestPlacementPutRGWBareBoolRejected verifies the clean break: a bare rgw +// bool no longer decodes and is rejected with BadRequest. +func TestPlacementPutRGWBareBoolRejected(t *testing.T) { + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPut, "/1.0/placement", strings.NewReader(`{"mode":"reconcile","members":{"node-a":{"rgw":true}}}`)) + + resp := cmdPlacementPut(nil, req) + _ = resp.Render(rec, req) + + assert.Equal(t, http.StatusBadRequest, rec.Code, "a bare rgw bool must be rejected (clean break)") +} + +// TestPlacementPutMalformedTLSReturns400 verifies that an RGW frontend error +// (ErrRgwFrontendInvalid, e.g. bad base64 TLS material surfaced from +// applyRGWFrontend) maps to HTTP 400 rather than the 500 fallback. +func TestPlacementPutMalformedTLSReturns400(t *testing.T) { + stubPlacementApplyLock(t) + origApply := ceph.ApplyPlacementFunc + origStore := ceph.StorePlacementPolicyFunc + origRefusal := ceph.SetPlacementRefusalFunc + ceph.ApplyPlacementFunc = func(_ context.Context, _ interfaces.StateInterface, _ types.PlacementPolicy) error { + return fmt.Errorf("%w: failed to decode SSL certificate: illegal base64", ceph.ErrRgwFrontendInvalid) + } + ceph.StorePlacementPolicyFunc = func(_ context.Context, _ interfaces.StateInterface, _ types.PlacementPolicy) error { return nil } + ceph.SetPlacementRefusalFunc = func(_ context.Context, _ interfaces.StateInterface, _ string) error { return nil } + defer func() { + ceph.ApplyPlacementFunc = origApply + ceph.StorePlacementPolicyFunc = origStore + ceph.SetPlacementRefusalFunc = origRefusal + }() + + body := `{"mode":"reconcile","members":{"node-a":{"rgw":{"enabled":true,"port":80}}}}` + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPut, "/1.0/placement", strings.NewReader(body)) + + resp := cmdPlacementPut(nil, req) + _ = resp.Render(rec, req) + + assert.Equal(t, http.StatusBadRequest, rec.Code, "malformed TLS must map to 400") +} + // TestPlacementGetSuccess verifies that cmdPlacementGet returns placement status. func TestPlacementGetSuccess(t *testing.T) { origGet := ceph.GetPlacementStatusFunc diff --git a/microceph/api/types/placement.go b/microceph/api/types/placement.go index 928b60a8..af4f616c 100644 --- a/microceph/api/types/placement.go +++ b/microceph/api/types/placement.go @@ -7,6 +7,20 @@ type NFSPlacement struct { BindAddress string `json:"bind_address" yaml:"bind_address"` } +// RgwPlacement carries RGW placement intent plus charm-derived frontend +// configuration (CE142 Option B). Enabled toggles placement; Port/SSLPort and +// the base64 PEM SSLCertificate/SSLPrivateKey configure the beast frontend. +// The certificate and key are charm-derived deployment context (from the +// charm's certificates relation), NOT Provider-furnished; the snap MUST redact +// them from GET /1.0/placement and MUST NOT persist them in the stored policy. +type RgwPlacement struct { + Enabled bool `json:"enabled" yaml:"enabled"` + Port int `json:"port,omitempty" yaml:"port,omitempty"` + SSLPort int `json:"ssl_port,omitempty" yaml:"ssl_port,omitempty"` + SSLCertificate string `json:"ssl_certificate,omitempty" yaml:"ssl_certificate,omitempty"` + SSLPrivateKey string `json:"ssl_private_key,omitempty" yaml:"ssl_private_key,omitempty"` +} + // MemberPlacement describes the desired placement for a single MicroCeph member. // Pointer fields distinguish "explicitly false/empty" (remove) from "omitted" // (leave untouched). This is the generic, non-OS106 payload consumed by the @@ -14,8 +28,9 @@ type NFSPlacement struct { type MemberPlacement struct { // Control governs MON, MGR, and MDS placement. nil means untouched. Control *bool `json:"control,omitempty" yaml:"control,omitempty"` - // Rgw governs RGW placement. nil means untouched. - Rgw *bool `json:"rgw,omitempty" yaml:"rgw,omitempty"` + // Rgw governs RGW placement and frontend config. nil means untouched; a + // non-nil value with Enabled false means remove RGW from the member. + Rgw *RgwPlacement `json:"rgw,omitempty" yaml:"rgw,omitempty"` // Nfs governs role-driven NFS placement. nil means untouched; an empty // (non-nil) slice means remove role-driven NFS on that member. The json // tag intentionally omits the omitempty modifier so that an empty slice @@ -40,14 +55,26 @@ type PlacementPolicy struct { Members map[string]MemberPlacement `json:"members" yaml:"members"` } +// RgwObservedFrontend is the observed RGW beast frontend on a member. It +// reports ports and whether TLS is configured, but never the cert/key bytes. +// Sourced from the rgw_frontends DB table (see CE142 placement-rgw). +type RgwObservedFrontend struct { + Port int `json:"port,omitempty" yaml:"port,omitempty"` + SSLPort int `json:"ssl_port,omitempty" yaml:"ssl_port,omitempty"` + SSL bool `json:"ssl" yaml:"ssl"` +} + // PlacementObservedMember captures the observed service placement for a member. // Control is true when the member hosts any of MON, MGR, or MDS. Nfs lists the // NFS group IDs placed on the member (from the grouped-services records). +// Rgw is true when the member hosts RGW; RgwFrontend reports its observed beast +// frontend (ports + TLS flag, never key material) when Rgw is true. type PlacementObservedMember struct { - Member string `json:"member" yaml:"member"` - Control bool `json:"control" yaml:"control"` - Rgw bool `json:"rgw" yaml:"rgw"` - Nfs []string `json:"nfs" yaml:"nfs"` + Member string `json:"member" yaml:"member"` + Control bool `json:"control" yaml:"control"` + Rgw bool `json:"rgw" yaml:"rgw"` + RgwFrontend *RgwObservedFrontend `json:"rgw_frontend,omitempty" yaml:"rgw_frontend,omitempty"` + Nfs []string `json:"nfs" yaml:"nfs"` } // PlacementStatus is the response body of GET /1.0/placement. It returns the diff --git a/microceph/api/types/placement_test.go b/microceph/api/types/placement_test.go new file mode 100644 index 00000000..1736403c --- /dev/null +++ b/microceph/api/types/placement_test.go @@ -0,0 +1,111 @@ +package types + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestRgwPlacementUnmarshalObject confirms the Option B wire shape: the rgw +// field is an object carrying enabled + charm-derived port/TLS material. A +// bare bool is rejected (clean break; the charm never shipped with the bool +// flag). +func TestRgwPlacementUnmarshalObject(t *testing.T) { + raw := `{ + "mode": "reconcile", + "members": { + "node-a": { + "rgw": { + "enabled": true, + "port": 8080, + "ssl_port": 8443, + "ssl_certificate": "Y2VydA==", + "ssl_private_key": "a2V5" + } + } + } + }` + var policy PlacementPolicy + err := json.Unmarshal([]byte(raw), &policy) + require.NoError(t, err) + require.Contains(t, policy.Members, "node-a") + + rgw := policy.Members["node-a"].Rgw + require.NotNil(t, rgw) + assert.True(t, rgw.Enabled) + assert.Equal(t, 8080, rgw.Port) + assert.Equal(t, 8443, rgw.SSLPort) + assert.Equal(t, "Y2VydA==", rgw.SSLCertificate) + assert.Equal(t, "a2V5", rgw.SSLPrivateKey) +} + +// TestRgwPlacementBareBoolRejected confirms the clean break: a bare rgw bool +// no longer decodes. DisallowUnknownFields is the API decoder's gate; at the +// type level a bool into *RgwPlacement must fail to unmarshal. +func TestRgwPlacementBareBoolRejected(t *testing.T) { + raw := `{"mode":"reconcile","members":{"node-a":{"rgw":true}}}` + var policy PlacementPolicy + err := json.Unmarshal([]byte(raw), &policy) + require.Error(t, err, "a bare rgw bool must be rejected (clean break)") +} + +// TestRgwPlacementOmittedIsNil confirms that an omitted rgw field leaves the +// pointer nil (untouched), distinct from {enabled:false} (remove intent). +func TestRgwPlacementOmittedIsNil(t *testing.T) { + raw := `{"mode":"reconcile","members":{"node-a":{"control":true}}}` + var policy PlacementPolicy + err := json.Unmarshal([]byte(raw), &policy) + require.NoError(t, err) + assert.Nil(t, policy.Members["node-a"].Rgw, "omitted rgw must stay nil (untouched)") +} + +// TestRgwPlacementDisabledRoundTrips confirms {enabled:false} round-trips +// through marshal/unmarshal as a non-nil remove intent (not dropped to nil). +func TestRgwPlacementDisabledRoundTrips(t *testing.T) { + rgw := &RgwPlacement{Enabled: false, Port: 80} + policy := PlacementPolicy{ + Mode: PlacementModeReconcile, + Members: map[string]MemberPlacement{ + "node-a": {Rgw: rgw}, + }, + } + data, err := json.Marshal(policy) + require.NoError(t, err) + + var back PlacementPolicy + require.NoError(t, json.Unmarshal(data, &back)) + require.NotNil(t, back.Members["node-a"].Rgw, "enabled:false must not be dropped to nil") + assert.False(t, back.Members["node-a"].Rgw.Enabled) + assert.Equal(t, 80, back.Members["node-a"].Rgw.Port) +} + +// TestRgwObservedFrontendMarshal confirms the observed frontend reports ports +// and a TLS flag only, and omits empty ports via omitempty. +func TestRgwObservedFrontendMarshal(t *testing.T) { + om := PlacementObservedMember{ + Member: "node-a", + Rgw: true, + RgwFrontend: &RgwObservedFrontend{Port: 80, SSLPort: 443, SSL: true}, + } + data, err := json.Marshal(om) + require.NoError(t, err) + + var back PlacementObservedMember + require.NoError(t, json.Unmarshal(data, &back)) + assert.True(t, back.Rgw) + require.NotNil(t, back.RgwFrontend) + assert.Equal(t, 80, back.RgwFrontend.Port) + assert.Equal(t, 443, back.RgwFrontend.SSLPort) + assert.True(t, back.RgwFrontend.SSL) +} + +// TestRgwObservedFrontendOmittedWhenNil confirms an absent frontend serializes +// without the field (omitempty) so non-RGW members do not carry a stub. +func TestRgwObservedFrontendOmittedWhenNil(t *testing.T) { + om := PlacementObservedMember{Member: "node-a"} + data, err := json.Marshal(om) + require.NoError(t, err) + assert.NotContains(t, string(data), "rgw_frontend") +} diff --git a/microceph/ceph/configwriter.go b/microceph/ceph/configwriter.go index 17d4a3c7..3d58539a 100644 --- a/microceph/ceph/configwriter.go +++ b/microceph/ceph/configwriter.go @@ -1,6 +1,7 @@ package ceph import ( + "bytes" "fmt" "os" "path/filepath" @@ -32,6 +33,24 @@ func (c *Config) validateConfigFile() error { return nil } +// RenderConfig renders the configuration template with data into a byte slice +// without touching disk. A caller compares this against the current on-disk +// file to decide whether a rewrite (and service restart) is actually needed, +// the "render from dqlite, compare in place" idiom (AGENTS.md conf-file +// model). The output is byte-identical to WriteConfig for the same data. +func (c *Config) RenderConfig(data map[string]any) ([]byte, error) { + err := c.validateConfigFile() + if err != nil { + return nil, err + } + var buf bytes.Buffer + err = c.configTemplate.Execute(&buf, data) + if err != nil { + return nil, fmt.Errorf("Couldn't render %s: %w", c.configFile, err) + } + return buf.Bytes(), nil +} + // WriteConfig writes the configuration file given a data bag and a filemode. // It writes atomically: the template is rendered to a uniquely-named temporary // file (created via os.CreateTemp in the same directory as the destination) and diff --git a/microceph/ceph/placement.go b/microceph/ceph/placement.go index 5550072c..bd674798 100644 --- a/microceph/ceph/placement.go +++ b/microceph/ceph/placement.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" "regexp" + "sort" "strings" "time" @@ -38,6 +39,12 @@ var ErrKeepOneInvariant = fmt.Errorf("keep-one invariant") // ErrCephBootstrapInProgress. var ErrPlacementApplyInProgress = fmt.Errorf("placement apply already in progress") +// ErrRgwFrontendInvalid is returned when the RGW frontend configuration in a +// placement policy is malformed (e.g. bad base64 SSL material). It is a +// client-side sentinel so the API handler maps it to HTTP 400 rather than the +// SmartError 500 fallback. +var ErrRgwFrontendInvalid = fmt.Errorf("invalid RGW frontend configuration") + // placementApplyLease bounds how long a placement apply may hold the // cluster-wide dqlite lock before it is considered abandoned (daemon crashed // mid-apply) and reclaimable by the next writer. It must comfortably exceed @@ -49,13 +56,28 @@ const placementApplyLease = 15 * time.Minute // used by the API handler so tests can override it. var LockPlacementApplyFunc = LockPlacementApply -// LockPlacementApply acquires the cluster-wide placement apply lock (CE142). -// ApplyPlacement reads observed service state and then mutates services over -// minutes; two overlapping applies (possibly served by different members) -// could each count the other's removal targets as keep-one retainers and -// together remove the last viable control service. The dqlite-backed lock -// makes the read-modify cycle mutually exclusive across all cluster members. +// LockPlacementApply acquires the cluster-wide placement apply lock. +// The dqlite-backed lock makes the whole apply-then-store cycle mutually +// exclusive across all cluster members. It serves two distinct purposes, and +// both must hold before it can be narrowed or removed: +// +// 1. Control-service safety. ApplyPlacement reads observed service state and +// then mutates services over minutes; two overlapping applies (possibly +// served by different members) could each count the other's removal +// targets as keep-one retainers and together remove the last viable +// control service. This rationale is specific to control: the keep-one +// decision reads *other* members' state, so a stale snapshot is unsafe. +// +// 2. Apply/store atomicity, for every service class. The API handler stores +// the policy inside this lock (see cmdPlacementPut), so the stored intent +// always matches the apply that ran last. Without it, two overlapping PUTs +// could apply in one order and store in the other, leaving GET /placement +// reporting a declared policy that contradicts what was actually applied. // +// Non-quorum services such as RGW have no keep-one invariant and decide each +// member independently, so reason 1 does not apply to them — they ride along +// on this lock for reason 2 (atomicity), not for safety. +// // The returned token must be passed to UnlockPlacementApply. If another apply // holds the lock, ErrPlacementApplyInProgress is returned; a lock older than // placementApplyLease is treated as abandoned and reclaimed. @@ -271,6 +293,16 @@ func ApplyPlacement(ctx context.Context, s interfaces.StateInterface, policy typ } } + // RGW pass (CE142 Option B): reconcile role-managed RGW placement after the + // control pass so control quorum is established first. RGW has no keep-one + // invariant (scale-to-zero is allowed); it follows add-before-remove so a + // migration keeps a gateway serving. An omitted rgw field (nil) leaves the + // member untouched; enabled:false removes RGW only where it is observed. + err = applyRgwPlacement(ctx, s, policy) + if err != nil { + return err + } + if len(refused) > 0 { return fmt.Errorf("%w: refused to remove last control service(s): %s", ErrKeepOneInvariant, strings.Join(refused, ", ")) } @@ -341,6 +373,205 @@ func prodRemoveControlService(ctx context.Context, s interfaces.StateInterface, return nil } +// applyRgwPlacement reconciles role-managed RGW placement (CE142 Option B). It +// runs after the control pass and follows add-before-remove: desired members +// are enabled first (with their frontend config), then RGW is removed from +// members that should not have it, but only where it is observed. RGW has no +// keep-one invariant, so scale-to-zero (all enabled:false / omitted) is +// honoured. Member iteration is sorted for deterministic ordering and tests. +func applyRgwPlacement(ctx context.Context, s interfaces.StateInterface, policy types.PlacementPolicy) error { + // Short-circuit when the policy carries no RGW intent: every member's rgw + // is omitted (nil), so there is nothing to enable or remove, and an observed + // RGW read is unnecessary. This keeps control-only applies from touching RGW + // and lets control-only tests skip the RGW observer stub. + hasRgwIntent := false + for _, mp := range policy.Members { + if mp.Rgw != nil { + hasRgwIntent = true + break + } + } + if !hasRgwIntent { + return nil + } + + observedRgw, err := getObservedRgwFunc(ctx, s) + if err != nil { + return fmt.Errorf("failed to get observed RGW services: %w", err) + } + + var desiredEnable []string + desiredDisable := make(map[string]bool) + for memberName, mp := range policy.Members { + if mp.Rgw == nil { + continue // omitted: untouched + } + if mp.Rgw.Enabled { + desiredEnable = append(desiredEnable, memberName) + } else { + desiredDisable[memberName] = true + } + } + sort.Strings(desiredEnable) + + // Add-before-remove: enable desired members first so a migration keeps a + // gateway serving while the old one is torn down. + for _, memberName := range desiredEnable { + err = enableRgwServiceFunc(ctx, s, memberName, *policy.Members[memberName].Rgw) + if err != nil { + return fmt.Errorf("failed to enable RGW on %s: %w", memberName, err) + } + } + + // Then remove RGW from members that should not have it, but only where it is + // observed (DisableRGW is idempotent either way; this avoids needless dispatch). + disableMembers := make([]string, 0, len(desiredDisable)) + for m := range desiredDisable { + disableMembers = append(disableMembers, m) + } + sort.Strings(disableMembers) + for _, memberName := range disableMembers { + if !observedRgw[memberName] { + continue + } + err = removeRgwServiceFunc(ctx, s, memberName) + if err != nil { + return fmt.Errorf("failed to remove RGW from %s: %w", memberName, err) + } + } + return nil +} + +// getObservedRgwFunc returns the set of members currently running RGW (from the +// services table). It reports presence only, which is all the reconcile pass +// needs; the observed frontend ports/TLS come from the rgw_frontends table in +// the status path, not here. Injectable for testing. +var getObservedRgwFunc = func(ctx context.Context, s interfaces.StateInterface) (map[string]bool, error) { + result := make(map[string]bool) + err := s.ClusterState().Database().Transaction(ctx, func(ctx context.Context, tx *sql.Tx) error { + services, err := database.GetServices(ctx, tx) + if err != nil { + return err + } + for _, svc := range services { + if svc.Service == "rgw" { + result[svc.Member] = true + } + } + return nil + }) + return result, err +} + +// enableRgwServiceFunc enables/reconciles RGW (with frontend config) on a +// member. Injectable for testing; the production implementation dispatches via +// the service API to the target member. +var enableRgwServiceFunc = func(ctx context.Context, s interfaces.StateInterface, member string, rgw types.RgwPlacement) error { + logger.Infof("Placement: enabling RGW on %s (port=%d, ssl_port=%d, ssl=%v)", member, rgw.Port, rgw.SSLPort, rgw.SSLCertificate != "") + return prodEnableRgwService(ctx, s, member, rgw) +} + +// removeRgwServiceFunc disables RGW on a member. Injectable for testing. +var removeRgwServiceFunc = func(ctx context.Context, s interfaces.StateInterface, member string) error { + logger.Infof("Placement: removing RGW from %s", member) + return prodRemoveRgwService(ctx, s, member) +} + +// ProdEnableRgwServiceFunc is the injectable hook for the production RGW enable +// implementation. The daemon package sets this at init time; the default is nil +// (no-op for tests that don't need real service placement). +var ProdEnableRgwServiceFunc func(ctx context.Context, s interfaces.StateInterface, member string, rgw types.RgwPlacement) error + +// ProdRemoveRgwServiceFunc is the injectable hook for the production RGW remove +// implementation. The daemon package sets this at init time; the default is nil +// (no-op for tests that don't need real service removal). +var ProdRemoveRgwServiceFunc func(ctx context.Context, s interfaces.StateInterface, member string) error + +// prodEnableRgwService delegates to the injected production function, or is a +// no-op when no production function is wired (e.g. in unit tests). +func prodEnableRgwService(ctx context.Context, s interfaces.StateInterface, member string, rgw types.RgwPlacement) error { + if ProdEnableRgwServiceFunc != nil { + return ProdEnableRgwServiceFunc(ctx, s, member, rgw) + } + return nil +} + +// prodRemoveRgwService delegates to the injected production function, or is a +// no-op when no production function is wired (e.g. in unit tests). +func prodRemoveRgwService(ctx context.Context, s interfaces.StateInterface, member string) error { + if ProdRemoveRgwServiceFunc != nil { + return ProdRemoveRgwServiceFunc(ctx, s, member) + } + return nil +} + +// PolicyForStorage returns a copy of policy with the RGW SSL certificate and +// private key stripped from every member's rgw entry. SSL key material must +// never be persisted in dqlite (CE142 Option B secrets posture): it travels +// over the authenticated API to the member that needs it, then is dropped +// before the policy is stored. enabled/port/ssl_port are retained so +// GET /placement still reports the declared frontend intent. The original +// policy is untouched so the apply path retains the material. +func PolicyForStorage(policy types.PlacementPolicy) types.PlacementPolicy { + out := policy + out.Members = make(map[string]types.MemberPlacement, len(policy.Members)) + for name, mp := range policy.Members { + if mp.Rgw != nil { + stripped := *mp.Rgw + stripped.SSLCertificate = "" + stripped.SSLPrivateKey = "" + mp.Rgw = &stripped + } + out.Members[name] = mp + } + return out +} + +// redactStoredPolicy blanks the RGW SSL certificate and private key from every +// member's rgw entry in a declared policy before it is returned via +// GET /placement. It is defense-in-depth: the stored policy is already stripped +// at PUT time (PolicyForStorage), but this protects against any future code +// path that stores the raw policy. A nil policy is a no-op. The observed +// frontend (RgwObservedFrontend) carries ports + a TLS flag only, so it needs no +// redaction. +func redactStoredPolicy(policy *types.PlacementPolicy) { + if policy == nil { + return + } + for _, mp := range policy.Members { + if mp.Rgw != nil { + mp.Rgw.SSLCertificate = "" + mp.Rgw.SSLPrivateKey = "" + } + } +} + +// populateRGWFrontends sets each observed member's RgwFrontend from the +// recorded rgw_frontends rows (CE142 placement-rgw). Only members observed to +// host RGW (Rgw=true) get a frontend; a recorded row for a member not observed +// with RGW is ignored because the services row is the presence authority. The +// frontend reports ports + a TLS flag only, never cert/key bytes. +func populateRGWFrontends(observedByMember map[string]*types.PlacementObservedMember, frontends []database.RgwFrontend) { + byName := make(map[string]database.RgwFrontend, len(frontends)) + for _, f := range frontends { + byName[f.Member] = f + } + for member, om := range observedByMember { + if !om.Rgw { + continue + } + f, ok := byName[member] + if !ok { + continue + } + om.RgwFrontend = &types.RgwObservedFrontend{ + Port: f.Port, + SSLPort: f.SSLPort, + SSL: f.SSL, + } + } +} + // GetPlacementStatusFunc is the injectable wrapper for GetPlacementStatus, // used by the API handler so tests can override it. var GetPlacementStatusFunc = GetPlacementStatus @@ -379,6 +610,10 @@ func GetPlacementStatus(ctx context.Context, s interfaces.StateInterface) (*type if err != nil { return fmt.Errorf("failed to unmarshal placement policy: %w", err) } + // Defense-in-depth: the stored policy is already stripped at PUT time, + // but redact again so a future code path that stores the raw policy + // cannot leak SSL material via GET /placement. + redactStoredPolicy(&policy) status.Policy = &policy } status.PlacementRefusal = redactSecrets(rec.LastRefusal) @@ -432,6 +667,15 @@ func GetPlacementStatus(ctx context.Context, s interfaces.StateInterface) (*type om.Nfs = append(om.Nfs, gs.GroupID) } + // Observed RGW frontends: ports + TLS flag from the rgw_frontends table + // (CE142 placement-rgw), read in the same transaction as the rest of the + // observed state. Never cert/key bytes. + rgwFrontends, err := database.GetRGWFrontends(ctx, tx) + if err != nil { + return err + } + populateRGWFrontends(observedByMember, rgwFrontends) + for _, om := range observedByMember { status.Observed = append(status.Observed, *om) } diff --git a/microceph/ceph/placement_test.go b/microceph/ceph/placement_test.go index 06e5bb21..2254a20b 100644 --- a/microceph/ceph/placement_test.go +++ b/microceph/ceph/placement_test.go @@ -11,9 +11,11 @@ import ( "github.com/canonical/lxd/shared" "github.com/canonical/lxd/shared/api" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "github.com/canonical/microceph/microceph/api/types" + "github.com/canonical/microceph/microceph/database" "github.com/canonical/microceph/microceph/interfaces" "github.com/canonical/microceph/microceph/mocks" "github.com/canonical/microceph/microceph/tests" @@ -782,6 +784,365 @@ func (s *placementSuite) TestControlServiceViabilityAllRemovalTargetsNoRetainers } } +// withObservedRgw injects a fixed observed RGW member set (members currently +// running RGW) for the RGW reconcile pass. +func withObservedRgw(observed map[string]bool) func() { + orig := getObservedRgwFunc + getObservedRgwFunc = func(_ context.Context, _ interfaces.StateInterface) (map[string]bool, error) { + result := make(map[string]bool, len(observed)) + for m, present := range observed { + if present { + result[m] = true + } + } + return result, nil + } + return func() { getObservedRgwFunc = orig } +} + +// rgwEvent records a single RGW enable/disable dispatch in an ordered log. +type rgwEvent struct { + kind string // "enable" or "remove" + member string + rgw types.RgwPlacement // payload (enable only) +} + +// rgwRecorder tracks RGW enable/remove calls so tests can assert add-before- +// remove ordering, the dispatched payload, and that omitted members are +// untouched. +type rgwRecorder struct { + events []rgwEvent +} + +func withRgwRecorder() (*rgwRecorder, func()) { + rec := &rgwRecorder{} + origEnable := enableRgwServiceFunc + origRemove := removeRgwServiceFunc + enableRgwServiceFunc = func(_ context.Context, _ interfaces.StateInterface, member string, rgw types.RgwPlacement) error { + rec.events = append(rec.events, rgwEvent{"enable", member, rgw}) + return nil + } + removeRgwServiceFunc = func(_ context.Context, _ interfaces.StateInterface, member string) error { + rec.events = append(rec.events, rgwEvent{"remove", member, types.RgwPlacement{}}) + return nil + } + return rec, func() { + enableRgwServiceFunc = origEnable + removeRgwServiceFunc = origRemove + } +} + +// enables returns the ordered list of "member" enabled. +func (r *rgwRecorder) enables() []string { + var result []string + for _, e := range r.events { + if e.kind == "enable" { + result = append(result, e.member) + } + } + return result +} + +// removes returns the ordered list of "member" removed. +func (r *rgwRecorder) removes() []string { + var result []string + for _, e := range r.events { + if e.kind == "remove" { + result = append(result, e.member) + } + } + return result +} + +// enablePayload returns the payload dispatched for the first enable of member. +func (r *rgwRecorder) enablePayload(member string) (types.RgwPlacement, bool) { + for _, e := range r.events { + if e.kind == "enable" && e.member == member { + return e.rgw, true + } + } + return types.RgwPlacement{}, false +} + +// allEnablesBeforeAllRemoves returns true if every enable event precedes every +// remove event in the ordered log (RGW add-before-remove). +func (r *rgwRecorder) allEnablesBeforeAllRemoves() bool { + firstRemoveIdx := -1 + lastEnableIdx := -1 + for i, e := range r.events { + if e.kind == "enable" { + lastEnableIdx = i + } + if e.kind == "remove" && firstRemoveIdx == -1 { + firstRemoveIdx = i + } + } + if firstRemoveIdx == -1 || lastEnableIdx == -1 { + return true + } + return lastEnableIdx < firstRemoveIdx +} + +// TestPlacementEnableRGW verifies that rgw.enabled:true dispatches an enable +// with the full frontend payload (port + SSL material) to each desired member. +func (s *placementSuite) TestPlacementEnableRGW() { + defer withObservedControl(map[string]map[string]bool{"mon": {}, "mgr": {}, "mds": {}})() + defer withObservedRgw(map[string]bool{})() + rec, restore := withRgwRecorder() + defer restore() + + policy := types.PlacementPolicy{ + Mode: "reconcile", + Members: map[string]types.MemberPlacement{ + "node-a": {Rgw: &types.RgwPlacement{Enabled: true, Port: 8080, SSLPort: 443, SSLCertificate: "Y2VydA==", SSLPrivateKey: "a2V5"}}, + "node-b": {Rgw: &types.RgwPlacement{Enabled: true, Port: 80}}, + }, + } + err := ApplyPlacement(context.Background(), s.TestStateInterface, policy) + assert.NoError(s.T(), err) + assert.ElementsMatch(s.T(), []string{"node-a", "node-b"}, rec.enables()) + assert.Empty(s.T(), rec.removes()) + + payload, ok := rec.enablePayload("node-a") + require.True(s.T(), ok) + assert.Equal(s.T(), 8080, payload.Port) + assert.Equal(s.T(), 443, payload.SSLPort) + assert.Equal(s.T(), "Y2VydA==", payload.SSLCertificate, "SSL material must reach the member enable path") +} + +// TestPlacementDisableRGWObserved verifies that rgw.enabled:false on a member +// currently running RGW dispatches a remove. +func (s *placementSuite) TestPlacementDisableRGWObserved() { + defer withObservedControl(map[string]map[string]bool{"mon": {}, "mgr": {}, "mds": {}})() + defer withObservedRgw(map[string]bool{"node-a": true})() + rec, restore := withRgwRecorder() + defer restore() + + policy := types.PlacementPolicy{ + Mode: "reconcile", + Members: map[string]types.MemberPlacement{ + "node-a": {Rgw: &types.RgwPlacement{Enabled: false}}, + }, + } + err := ApplyPlacement(context.Background(), s.TestStateInterface, policy) + assert.NoError(s.T(), err) + assert.Equal(s.T(), []string{"node-a"}, rec.removes()) + assert.Empty(s.T(), rec.enables()) +} + +// TestPlacementDisableRGWNotObserved verifies that rgw.enabled:false on a member +// NOT running RGW is a no-op (no remove dispatched). +func (s *placementSuite) TestPlacementDisableRGWNotObserved() { + defer withObservedControl(map[string]map[string]bool{"mon": {}, "mgr": {}, "mds": {}})() + defer withObservedRgw(map[string]bool{})() + rec, restore := withRgwRecorder() + defer restore() + + policy := types.PlacementPolicy{ + Mode: "reconcile", + Members: map[string]types.MemberPlacement{ + "node-a": {Rgw: &types.RgwPlacement{Enabled: false}}, + }, + } + err := ApplyPlacement(context.Background(), s.TestStateInterface, policy) + assert.NoError(s.T(), err) + assert.Empty(s.T(), rec.removes(), "no remove when RGW is not observed on the member") +} + +// TestPlacementRGWOmittedUntouched verifies that an omitted rgw field (nil) +// leaves the member untouched, even if it currently runs RGW. +func (s *placementSuite) TestPlacementRGWOmittedUntouched() { + defer withObservedControl(map[string]map[string]bool{"mon": {}, "mgr": {}, "mds": {}})() + defer withObservedRgw(map[string]bool{"node-a": true})() + rec, restore := withRgwRecorder() + defer restore() + + policy := types.PlacementPolicy{ + Mode: "reconcile", + Members: map[string]types.MemberPlacement{ + "node-a": {Control: boolPtr(true)}, // rgw omitted + }, + } + err := ApplyPlacement(context.Background(), s.TestStateInterface, policy) + assert.NoError(s.T(), err) + assert.Empty(s.T(), rec.enables(), "omitted rgw must not enable") + assert.Empty(s.T(), rec.removes(), "omitted rgw must not remove even when observed") +} + +// TestPlacementRGWScaleToZero verifies that when every member has rgw +// enabled:false, RGW is removed only where it is observed (scale-to-zero; no +// keep-one for RGW). Omitted members are covered by TestPlacementRGWOmittedUntouched. +func (s *placementSuite) TestPlacementRGWScaleToZero() { + defer withObservedControl(map[string]map[string]bool{"mon": {}, "mgr": {}, "mds": {}})() + defer withObservedRgw(map[string]bool{"node-a": true, "node-b": true, "node-c": false})() + rec, restore := withRgwRecorder() + defer restore() + + policy := types.PlacementPolicy{ + Mode: "reconcile", + Members: map[string]types.MemberPlacement{ + "node-a": {Rgw: &types.RgwPlacement{Enabled: false}}, + "node-b": {Rgw: &types.RgwPlacement{Enabled: false}}, + "node-c": {Rgw: &types.RgwPlacement{Enabled: false}}, + }, + } + err := ApplyPlacement(context.Background(), s.TestStateInterface, policy) + assert.NoError(s.T(), err) + assert.Empty(s.T(), rec.enables()) + // node-a and node-b observed -> removed; node-c not observed -> no-op. + assert.ElementsMatch(s.T(), []string{"node-a", "node-b"}, rec.removes()) +} + +// TestPlacementRGWMigrateAddBeforeRemove verifies that migrating RGW from node-a +// to node-b enables node-b before disabling node-a, keeping a gateway serving. +func (s *placementSuite) TestPlacementRGWMigrateAddBeforeRemove() { + defer withObservedControl(map[string]map[string]bool{"mon": {}, "mgr": {}, "mds": {}})() + defer withObservedRgw(map[string]bool{"node-a": true})() + rec, restore := withRgwRecorder() + defer restore() + + policy := types.PlacementPolicy{ + Mode: "reconcile", + Members: map[string]types.MemberPlacement{ + "node-a": {Rgw: &types.RgwPlacement{Enabled: false}}, + "node-b": {Rgw: &types.RgwPlacement{Enabled: true, Port: 80}}, + }, + } + err := ApplyPlacement(context.Background(), s.TestStateInterface, policy) + assert.NoError(s.T(), err) + assert.True(s.T(), rec.allEnablesBeforeAllRemoves(), "RGW enables must precede removes: %v", rec.events) + assert.Equal(s.T(), []string{"node-b"}, rec.enables()) + assert.Equal(s.T(), []string{"node-a"}, rec.removes()) +} + +// TestPlacementRGWUnknownMemberRejected verifies that an unknown member in the +// rgw map is rejected (reusing the existing member validation). +func (s *placementSuite) TestPlacementRGWUnknownMemberRejected() { + rec, restore := withRgwRecorder() + defer restore() + + policy := types.PlacementPolicy{ + Mode: "reconcile", + Members: map[string]types.MemberPlacement{ + "unknown-node": {Rgw: &types.RgwPlacement{Enabled: true}}, + }, + } + err := ApplyPlacement(context.Background(), s.TestStateInterface, policy) + assert.Error(s.T(), err) + assert.ErrorIs(s.T(), err, ErrUnknownPlacementMember) + assert.Empty(s.T(), rec.enables()) +} + +// TestPlacementControlBeforeRGW verifies that when a policy has both control +// and rgw changes, the control pass runs before the RGW pass. The recorder log +// must show all control adds before the first RGW enable. +func (s *placementSuite) TestPlacementControlBeforeRGW() { + defer withObservedControl(map[string]map[string]bool{ + "mon": {}, "mgr": {}, "mds": {}, + })() + defer withObservedRgw(map[string]bool{})() + ctrlRec, ctrlRestore := withAddRemoveRecorder() + defer ctrlRestore() + rgwRec, rgwRestore := withRgwRecorder() + defer rgwRestore() + + policy := types.PlacementPolicy{ + Mode: "reconcile", + Members: map[string]types.MemberPlacement{ + "node-a": { + Control: boolPtr(true), + Rgw: &types.RgwPlacement{Enabled: true, Port: 80}, + }, + }, + } + err := ApplyPlacement(context.Background(), s.TestStateInterface, policy) + assert.NoError(s.T(), err) + assert.NotEmpty(s.T(), ctrlRec.adds(), "control adds must run") + assert.NotEmpty(s.T(), rgwRec.enables(), "rgw enable must run") + // Control adds precede RGW enables because the control pass runs first and + // both recorders share no state; assert via the control recorder having + // recorded all three services (mon/mgr/mds) which only happens before RGW. + assert.Len(s.T(), ctrlRec.adds(), 3, "all control services added before RGW pass") +} + +// TestPolicyForStorageStripsSSLMaterial verifies that PolicyForStorage returns a +// copy of the policy with the RGW SSL certificate and private key removed, +// while enabled/port/ssl_port are retained for declared-intent reporting. The +// original policy must be untouched so the apply path still has the material. +func (s *placementSuite) TestPolicyForStorageStripsSSLMaterial() { + policy := types.PlacementPolicy{ + Mode: "reconcile", + Members: map[string]types.MemberPlacement{ + "node-a": {Rgw: &types.RgwPlacement{Enabled: true, Port: 80, SSLPort: 443, SSLCertificate: "Y2VydA==", SSLPrivateKey: "a2V5"}}, + "node-b": {Control: boolPtr(true)}, // no rgw: untouched + }, + } + stored := PolicyForStorage(policy) + + require.Contains(s.T(), stored.Members, "node-a") + require.NotNil(s.T(), stored.Members["node-a"].Rgw) + assert.Empty(s.T(), stored.Members["node-a"].Rgw.SSLCertificate, "cert must be stripped before storage") + assert.Empty(s.T(), stored.Members["node-a"].Rgw.SSLPrivateKey, "key must be stripped before storage") + assert.True(s.T(), stored.Members["node-a"].Rgw.Enabled, "enabled must be retained") + assert.Equal(s.T(), 80, stored.Members["node-a"].Rgw.Port, "port must be retained") + assert.Equal(s.T(), 443, stored.Members["node-a"].Rgw.SSLPort, "ssl_port must be retained") + + // Original policy untouched: the apply path still needs the material. + assert.Equal(s.T(), "Y2VydA==", policy.Members["node-a"].Rgw.SSLCertificate) + assert.Equal(s.T(), "a2V5", policy.Members["node-a"].Rgw.SSLPrivateKey) +} + +// TestPopulateRGWFrontends verifies the pure helper maps recorded rgw_frontends +// rows onto observed members that host RGW, and ignores rows for members not +// observed with RGW (the services row is the presence authority). +func (s *placementSuite) TestPopulateRGWFrontends() { + observed := map[string]*types.PlacementObservedMember{ + "node-a": {Member: "node-a", Rgw: true}, + "node-b": {Member: "node-b", Rgw: true}, + "node-c": {Member: "node-c", Rgw: false}, + } + frontends := []database.RgwFrontend{ + {Member: "node-a", Port: 80, SSLPort: 0, SSL: false}, + {Member: "node-b", Port: 0, SSLPort: 443, SSL: true}, + {Member: "node-c", Port: 80, SSL: false}, // not observed with RGW + {Member: "node-d", Port: 80, SSL: false}, // not in observed at all + } + populateRGWFrontends(observed, frontends) + + require.NotNil(s.T(), observed["node-a"].RgwFrontend) + assert.Equal(s.T(), 80, observed["node-a"].RgwFrontend.Port) + assert.False(s.T(), observed["node-a"].RgwFrontend.SSL) + + require.NotNil(s.T(), observed["node-b"].RgwFrontend) + assert.Equal(s.T(), 443, observed["node-b"].RgwFrontend.SSLPort) + assert.True(s.T(), observed["node-b"].RgwFrontend.SSL) + + assert.Nil(s.T(), observed["node-c"].RgwFrontend, "member without observed RGW gets no frontend") +} + +// TestRedactStoredPolicy verifies the GET defense-in-depth redaction blanks the +// RGW SSL certificate and private key from the declared policy while retaining +// non-secret fields, and that a nil policy is a no-op. +func (s *placementSuite) TestRedactStoredPolicy() { + policy := &types.PlacementPolicy{ + Members: map[string]types.MemberPlacement{ + "node-a": {Rgw: &types.RgwPlacement{Enabled: true, Port: 80, SSLPort: 443, SSLCertificate: "Y2VydA==", SSLPrivateKey: "a2V5"}}, + "node-b": {Control: boolPtr(true)}, + }, + } + redactStoredPolicy(policy) + + require.NotNil(s.T(), policy.Members["node-a"].Rgw) + assert.Empty(s.T(), policy.Members["node-a"].Rgw.SSLCertificate, "cert must be redacted on GET") + assert.Empty(s.T(), policy.Members["node-a"].Rgw.SSLPrivateKey, "key must be redacted on GET") + assert.True(s.T(), policy.Members["node-a"].Rgw.Enabled, "enabled must be retained") + assert.Equal(s.T(), 80, policy.Members["node-a"].Rgw.Port, "port must be retained") + assert.Equal(s.T(), 443, policy.Members["node-a"].Rgw.SSLPort, "ssl_port must be retained") + + // nil policy must not panic. + redactStoredPolicy(nil) +} + // TestRedactSecrets verifies that redactSecrets masks realistic cephx key // material while leaving ordinary refusal/error text intact. This is the sole // guard against leaking key material through GET /1.0/placement, so a regex diff --git a/microceph/ceph/rgw.go b/microceph/ceph/rgw.go index 7c5782ca..95ed87c1 100644 --- a/microceph/ceph/rgw.go +++ b/microceph/ceph/rgw.go @@ -1,16 +1,225 @@ package ceph import ( + "bytes" "context" "encoding/base64" "fmt" - "github.com/canonical/microceph/microceph/constants" - "github.com/canonical/microceph/microceph/interfaces" "os" "path/filepath" + "sort" "strings" + + "github.com/canonical/microceph/microceph/constants" + "github.com/canonical/microceph/microceph/interfaces" +) + +// Injectable primitives so applyRGWFrontend is unit-testable without a running +// snap or Ceph cluster (AGENTS.md injectable-function-variable convention). The +// defaults delegate to the real snapctl/ceph helpers; tests override these to +// record calls and avoid external commands. +var ( + startRGWFunc = startRGW + restartRGWFunc = RestartRGW + createRGWKeyringFunc = createRGWKeyring ) +// effectiveRGWPorts applies the default-port-80 rule for a plaintext frontend +// (no SSL material) and returns the port/sslPort that will actually be rendered +// to radosgw.conf and recorded as observed state. When SSL is not configured +// sslPort is forced to 0 so the observed frontend never reports a stray SSL +// port (e.g. the enable-rgw CLI's --ssl-port default of 443) for a plaintext +// gateway. Centralizing the rule keeps the on-disk render and the rgw_frontends +// DB record consistent, so observed state matches what is running. +func effectiveRGWPorts(port, sslPort int, sslCert, sslKey string) (int, int) { + if sslCert != "" && sslKey != "" { + return port, sslPort + } + // Plaintext: no SSL port, default the plain port to 80. + if port == 0 { + port = 80 + } + return port, 0 +} + +// rgwFrontendLine extracts the single `rgw frontends = ...` line from a rendered +// radosgw.conf. It is the only line whose change (port/ssl_port/ssl paths) +// requires an RGW restart; the `mon host` and `run dir` lines are owned by +// UpdateConfig/migrateStaleRunDir and must not drive restart decisions here. +// Returns "" when no such line is present (e.g. a missing file on first enable). +func rgwFrontendLine(conf []byte) string { + for _, line := range strings.Split(string(conf), "\n") { + if strings.HasPrefix(strings.TrimSpace(line), "rgw frontends = ") { + return strings.TrimSpace(line) + } + } + return "" +} + +// applyRGWFrontend renders radosgw.conf and SSL material for the desired RGW +// beast frontend and (re)starts RGW only when the on-disk state changed. It is +// idempotent: re-applying the same port/TLS is a no-op with no restart, so +// frequent placement reconcile PUTs do not churn RGW. It returns changed=true +// when it wrote files and (re)started RGW. +// +// The member decides whether a restart is needed because radosgw.conf and the +// SSL files are local to it; the engine only declares desired state. This is +// the single primitive shared by the enable rgw CLI path and the placement +// path. +func applyRGWFrontend(s interfaces.StateInterface, port, sslPort int, sslCert, sslKey string, monitors []string) (bool, error) { + pathConsts := constants.GetPathConst() + + certPath := filepath.Join(pathConsts.SSLFilesPath, "server.crt") + keyPath := filepath.Join(pathConsts.SSLFilesPath, "server.key") + + var decodedCert, decodedKey []byte + sslCertificatePath := "" + sslPrivateKeyPath := "" + + sslConfigured := sslCert != "" && sslKey != "" + if sslConfigured { + var err error + decodedCert, err = base64.StdEncoding.DecodeString(sslCert) + if err != nil { + return false, fmt.Errorf("%w: failed to decode SSL certificate: %v", ErrRgwFrontendInvalid, err) + } + decodedKey, err = base64.StdEncoding.DecodeString(sslKey) + if err != nil { + return false, fmt.Errorf("%w: failed to decode SSL private key: %v", ErrRgwFrontendInvalid, err) + } + sslCertificatePath = certPath + sslPrivateKeyPath = keyPath + } + + port, sslPort = effectiveRGWPorts(port, sslPort, sslCert, sslKey) + + // Normalize monitors (IPv6-bracket + sort) so the rendered `mon host` line + // is deterministic and byte-identical to what the periodic UpdateConfig -> + // updateRadosGWMonHost writer produces. getMonitorsFromConfig iterates a map + // (randomized order), so without this the two writers would flip-flop the + // line. Restart decisions do not depend on this line (see rgwFrontendLine), + // but keeping it stable avoids a pointless re-render race between writers. + monitors = formatIPv6(monitors) + sort.Strings(monitors) + + configs := map[string]any{ + "runDir": pathConsts.RunPath, + "monitors": strings.Join(monitors, ","), + "rgwPort": port, + "sslPort": sslPort, + "sslCertificatePath": sslCertificatePath, + "sslPrivateKeyPath": sslPrivateKeyPath, + } + + rgwConf := newRadosGWConfig(pathConsts.ConfPath) + desiredConf, err := rgwConf.RenderConfig(configs) + if err != nil { + return false, err + } + confPath := rgwConf.GetPath() + + // Detect a frontend change by comparing ONLY the rendered `rgw frontends` + // line against the on-disk one, not the whole file. The `mon host` and + // `run dir` lines are owned by UpdateConfig/migrateStaleRunDir and are + // rewritten in place there; comparing the whole file would make every + // reconcile look changed once those writers touch it, restarting RGW + // needlessly (idempotency defeat in multi-mon / IPv6 clusters). A missing + // file is a first enable. + currentConf, confErr := os.ReadFile(confPath) + confExists := false + if confErr == nil { + confExists = true + } else if !os.IsNotExist(confErr) { + return false, fmt.Errorf("failed to read radosgw.conf: %w", confErr) + } + frontendChanged := !confExists || rgwFrontendLine(desiredConf) != rgwFrontendLine(currentConf) + + // Detect an SSL change by comparing the decoded material against the + // on-disk files. When moving to plaintext, any leftover SSL files from a + // prior TLS config are removed, which is also a change. + sslChanged := false + if sslConfigured { + curCert, e1 := os.ReadFile(certPath) + curKey, e2 := os.ReadFile(keyPath) + if os.IsNotExist(e1) || os.IsNotExist(e2) || !bytes.Equal(curCert, decodedCert) || !bytes.Equal(curKey, decodedKey) { + sslChanged = true + } + } else if fileExists(certPath) || fileExists(keyPath) { + sslChanged = true + } + + if !frontendChanged && !sslChanged { + return false, nil + } + + // Apply SSL: write if configured, remove leftovers if moving to plaintext. + if sslChanged { + if sslConfigured { + _, _, err = writeSSLFiles(pathConsts.SSLFilesPath, sslCert, sslKey) + if err != nil { + return false, err + } + } else { + if err = removeIgnoreMissing(certPath); err != nil { + return false, fmt.Errorf("failed to remove leftover SSL certificate: %w", err) + } + if err = removeIgnoreMissing(keyPath); err != nil { + return false, fmt.Errorf("failed to remove leftover SSL private key: %w", err) + } + } + } + + // Rewrite the config only when the frontend line changed (a cert rotation + // keeps the same paths, so the line is unchanged and only the SSL files are + // rewritten above). The full file is rendered so `mon host`/`run dir` stay + // present; UpdateConfig keeps them fresh thereafter. + if frontendChanged { + if err = rgwConf.WriteConfig(configs, 0644); err != nil { + return false, err + } + } + + // Ensure the keyring and its conf-dir symlink exist (both idempotent). + keyringPath := filepath.Join(pathConsts.DataPath, "radosgw", "ceph-radosgw.gateway") + if err = createRGWKeyringFunc(keyringPath); err != nil { + return false, err + } + if err = symlinkRGWKeyring(keyringPath, pathConsts.ConfPath); err != nil { + return false, err + } + + // Start on first enable; restart on a frontend change. Only one of these + // runs, and only when something actually changed. + if !confExists { + if err = startRGWFunc(); err != nil { + return false, err + } + } else { + if err = restartRGWFunc(); err != nil { + return false, err + } + } + return true, nil +} + +// fileExists reports whether path exists (any type). Errors other than +// IsNotExist are treated as missing to keep the change-detection robust. +func fileExists(path string) bool { + _, err := os.Stat(path) + return err == nil +} + +// removeIgnoreMissing removes path, treating a missing file as success so the +// disable / TLS->plaintext paths are idempotent (a re-run or partial prior +// state still converges). +func removeIgnoreMissing(path string) error { + err := os.Remove(path) + if err != nil && !os.IsNotExist(err) { + return err + } + return nil +} + // writeSSLFiles decodes base64-encoded SSL certificate and key, and writes them to disk. // Returns the paths to the written certificate and key files. func writeSSLFiles(sslFilesPath string, sslCertificate string, sslPrivateKey string) (certPath string, keyPath string, err error) { @@ -63,56 +272,13 @@ func writeSSLFiles(sslFilesPath string, sslCertificate string, sslPrivateKey str return certPath, keyPath, nil } -// EnableRGW enables the RGW service on the cluster and adds initial configuration given a service port number. +// EnableRGW enables the RGW service on the cluster and adds initial +// configuration given a service port number. It delegates to applyRGWFrontend so +// the enable rgw CLI path and the placement path share one idempotent, +// restart-on-change primitive. func EnableRGW(s interfaces.StateInterface, port int, sslPort int, sslCertificate string, sslPrivateKey string, monitors []string) error { - pathConsts := constants.GetPathConst() - - sslCertificatePath := "" - sslPrivateKeyPath := "" - if sslCertificate != "" && sslPrivateKey != "" { - var err error - sslCertificatePath, sslPrivateKeyPath, err = writeSSLFiles(pathConsts.SSLFilesPath, sslCertificate, sslPrivateKey) - if err != nil { - return err - } - } else if sslCertificate == "" || sslPrivateKey == "" { - // The default value is in the command line is 0 for the case where - // both SSL certificates and Private Key are provided, so we handle the - // default case here. - if port == 0 { - port = 80 - } - } - configs := map[string]any{ - "runDir": pathConsts.RunPath, - "monitors": strings.Join(monitors, ","), - "rgwPort": port, - "sslPort": sslPort, - "sslCertificatePath": sslCertificatePath, - "sslPrivateKeyPath": sslPrivateKeyPath, - } - - // Create RGW configuration. - rgwConf := newRadosGWConfig(pathConsts.ConfPath) - err := rgwConf.WriteConfig(configs, 0644) - if err != nil { - return err - } - // Create RGW keyring. - path := filepath.Join(pathConsts.DataPath, "radosgw", "ceph-radosgw.gateway") - if err = createRGWKeyring(path); err != nil { - return err - } - // Symlink the keyring to the conf directory for usage with the radosgw-admin command. - if err = symlinkRGWKeyring(path, pathConsts.ConfPath); err != nil { - return err - } - - if err = startRGW(); err != nil { - return err - } - - return nil + _, err := applyRGWFrontend(s, port, sslPort, sslCertificate, sslPrivateKey, monitors) + return err } // UpdateRGWCertificates decodes base64 SSL certificate and key, and writes them to disk. @@ -161,29 +327,29 @@ func DisableRGW(ctx context.Context, s interfaces.StateInterface) error { } // Remove the keyring symlink. - err = os.Remove(filepath.Join(pathConsts.ConfPath, "ceph.client.radosgw.gateway.keyring")) + err = removeIgnoreMissing(filepath.Join(pathConsts.ConfPath, "ceph.client.radosgw.gateway.keyring")) if err != nil { return fmt.Errorf("failed to remove RGW keyring symlink: %w", err) } // Remove the keyring. - err = os.Remove(filepath.Join(pathConsts.DataPath, "radosgw", "ceph-radosgw.gateway", "keyring")) + err = removeIgnoreMissing(filepath.Join(pathConsts.DataPath, "radosgw", "ceph-radosgw.gateway", "keyring")) if err != nil { return fmt.Errorf("failed to remove RGW keyring: %w", err) } // Remove the SSL files. - err = os.Remove(filepath.Join(pathConsts.SSLFilesPath, "server.crt")) - if err != nil && !os.IsNotExist(err) { + err = removeIgnoreMissing(filepath.Join(pathConsts.SSLFilesPath, "server.crt")) + if err != nil { return fmt.Errorf("failed to remove RGW SSL Certificate file: %w", err) } - err = os.Remove(filepath.Join(pathConsts.SSLFilesPath, "server.key")) - if err != nil && !os.IsNotExist(err) { + err = removeIgnoreMissing(filepath.Join(pathConsts.SSLFilesPath, "server.key")) + if err != nil { return fmt.Errorf("failed to remove RGW SSL Private Key file: %w", err) } // Remove the configuration. - err = os.Remove(filepath.Join(pathConsts.ConfPath, "radosgw.conf")) + err = removeIgnoreMissing(filepath.Join(pathConsts.ConfPath, "radosgw.conf")) if err != nil { return fmt.Errorf("failed to remove RGW configuration: %w", err) } @@ -234,13 +400,26 @@ func createRGWKeyring(path string) error { return nil } -// symlinkRGWKeyring creates a symlink to the RGW keyring in the conf directory for use with the radosgw-admin command. -func symlinkRGWKeyring(keyPath, ConfPath string) error { - if err := os.Symlink( - filepath.Join(keyPath, "keyring"), - filepath.Join(ConfPath, "ceph.client.radosgw.gateway.keyring")); err != nil { - return fmt.Errorf("Failed to create symlink to RGW keyring: %w", err) +// symlinkRGWKeyring creates a symlink to the RGW keyring in the conf directory +// for use with the radosgw-admin command. It is idempotent: if a symlink +// already points at the keyring, it is a no-op; a stale or non-symlink entry at +// the link path is replaced so re-applies and recoveries converge. +func symlinkRGWKeyring(keyPath, confPath string) error { + linkPath := filepath.Join(confPath, "ceph.client.radosgw.gateway.keyring") + target := filepath.Join(keyPath, "keyring") + + if fi, err := os.Lstat(linkPath); err == nil { + if fi.Mode()&os.ModeSymlink != 0 { + if existing, rErr := os.Readlink(linkPath); rErr == nil && existing == target { + return nil + } + } + // A stale symlink or a stray regular file: remove before re-creating. + _ = removeIgnoreMissing(linkPath) } + if err := os.Symlink(target, linkPath); err != nil { + return fmt.Errorf("Failed to create symlink to RGW keyring: %w", err) + } return nil } diff --git a/microceph/ceph/rgw_test.go b/microceph/ceph/rgw_test.go index 56db6e97..9ba19cb5 100644 --- a/microceph/ceph/rgw_test.go +++ b/microceph/ceph/rgw_test.go @@ -1,365 +1,303 @@ package ceph import ( - "context" - "fmt" + "encoding/base64" "os" "path/filepath" "testing" - "github.com/canonical/lxd/shared/api" - "github.com/canonical/microceph/microceph/common" - "github.com/canonical/microceph/microceph/mocks" - "github.com/canonical/microceph/microceph/tests" + "github.com/canonical/microceph/microceph/constants" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/suite" + "github.com/stretchr/testify/require" ) -type rgwSuite struct { - tests.BaseSuite - TestStateInterface *mocks.StateInterface +// rgwOpsRecorder captures calls to the injectable RGW primitives so tests can +// assert start vs restart and keyring creation without a running snap or Ceph. +type rgwOpsRecorder struct { + starts int + restarts int + keyrings int } -const validSSLCertificate = `LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURuakNDQW9hZ0F3SUJBZ0lVR0czNU9mWkcrRFdFQytrc2FHalJyTmlXZncwd0RRWUpLb1pJaHZjTkFRRUwKQlFBd1hERUxNQWtHQTFVRUJoTUNWVk14RHpBTkJnTlZCQWdNQmtSbGJtbGhiREVVTUJJR0ExVUVCd3dMVTNCeQphVzVuWm1sbGJHUXhEREFLQmdOVkJBb01BMFJwY3pFWU1CWUdBMVVFQXd3UGQzZDNMbVY0WVcxd2JHVXVZMjl0Ck1CNFhEVEkwTURneE5qRTROREUxT0ZvWERUSTFNRGd4TmpFNE5ERTFPRm93WERFTE1Ba0dBMVVFQmhNQ1ZWTXgKRHpBTkJnTlZCQWdNQmtSbGJtbGhiREVVTUJJR0ExVUVCd3dMVTNCeWFXNW5abWxsYkdReEREQUtCZ05WQkFvTQpBMFJwY3pFWU1CWUdBMVVFQXd3UGQzZDNMbVY0WVcxd2JHVXVZMjl0TUlJQklqQU5CZ2txaGtpRzl3MEJBUUVGCkFBT0NBUThBTUlJQkNnS0NBUUVBdFl5ZGRhb0l4T3hQWmtVMEN1dXE0aEd3Q2JlZXBUM3lBQ0JOS1J6MjB5alQKZ2xSWTFTSTlXSjl4K2t1a3dMTGNiVEIrSkNka2NWTEZuNThtVDRmUW5IMHdmWCtIby9BTUNHNkxITnZnOXovVAorTlV4dTgydGZsVko3RFRUdmVuYzlqVU9qNFZqUExaV2tiemNIOC91Sm1DNkd1ZzAvcksvN2wraG9xNUd6VXhzCmJQeGlOV0QvNW5kaklKa1VidEtpTllnQlRwcnRzZFlCWHoyeTFxS1AxcGZLQ3VIUWVldTNLTWErS0dUU2NUSjYKU251Y0pxZmIvTWdUMWozV3Zpcm1QaUQ3bEwzY3ZmaEtmTEgvYTdsaFhIeDRic21TekZ2UkRYTCt1YmNhak5seQpGUm5WdG9hUHhmMUY4RStFbXh4cXNESlc2bHZKVHJMeW84TjVNbWtoOFFJREFRQUJvMWd3VmpBVUJnTlZIUkVFCkRUQUxnZ2xzYjJOaGJHaHZjM1F3SFFZRFZSME9CQllFRkhIMFoxdWVmSHB1Wll1QTRzRFBlWTd4U2R6b01COEcKQTFVZEl3UVlNQmFBRlBRc1Q0SkU3dUl1ay96T2VvVlZpQVZYeDBoUk1BMEdDU3FHU0liM0RRRUJDd1VBQTRJQgpBUUNucHVFM2hzVHAwckZCU1hWRnV6VzExZjE2bXlML3pyZkJDWnRxQyt6UFZINGlyUUlrRFg2TDdPekY2K00vCml6OFJtQlZXSVpzWTlzczM5SmRlcEsvOVhuMEo5RUdDS2hhdmpldS8yUnpvalFaeXRQWU5DdldtMlhTQ0VHY2wKSDhDcGNQVC9JdnlCNU8yRVl0RUJNcnRrUVNKNjVFWlQyZHRiVFYySUdJN3ZDdjJIUnY0Y2twRXBFTWlLWnNPYgpBcWovbGNLeWFZODVwakFBWWVtVlprZ2dRZTJUM0tzSDFYRVJrNnhFRHF2TUdHbjEvOTNHY1J1enVTVTZaYXVPCmVzVDVISUl2UGZReWlwZG4rOWlKVjluc3hyNGVCa1JPVWFvV2s1NVVENE5tcEtiaHJ3MzZ3RzN4RzJ2RlIxeWUKSVFPNmhKMk5yckFnc2JwemxINzhVcjM4Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0=` -const validSSLPrivateKey = `LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tCk1JSUV2UUlCQURBTkJna3Foa2lHOXcwQkFRRUZBQVNDQktjd2dnU2pBZ0VBQW9JQkFRQzFqSjExcWdqRTdFOW0KUlRRSzY2cmlFYkFKdDU2bFBmSUFJRTBwSFBiVEtOT0NWRmpWSWoxWW4zSDZTNlRBc3R4dE1INGtKMlJ4VXNXZgpueVpQaDlDY2ZUQjlmNGVqOEF3SWJvc2MyK0QzUDlQNDFURzd6YTErVlVuc05OTzk2ZHoyTlE2UGhXTTh0bGFSCnZOd2Z6KzRtWUxvYTZEVCtzci91WDZHaXJrYk5UR3hzL0dJMVlQL21kMk1nbVJSdTBxSTFpQUZPbXUyeDFnRmYKUGJMV29vL1dsOG9LNGRCNTY3Y294cjRvWk5KeE1ucEtlNXdtcDl2OHlCUFdQZGErS3VZK0lQdVV2ZHk5K0VwOApzZjlydVdGY2ZIaHV5WkxNVzlFTmN2NjV0eHFNMlhJVkdkVzJoby9GL1VYd1Q0U2JIR3F3TWxicVc4bE9zdktqCncza3lhU0h4QWdNQkFBRUNnZ0VBQXVWdTM2RXFTYVh4Y0ZLN1RVOU1KeFljSmxPSkV0N0ZuUTNtM1RpS2tYek4KdnY4RWVjWDFqNVBmbUJ3YjBUMHBPZzZ6ZkhVcWE0cGovN05rdzVFSm1XMS8yQWl3UzhPNUZXdGFDY2hTTXUrUQpQS0IrRGg1dVhaMFR0RkoxYkVxdVRUazBkY0t0ZmhyMGo1ZWhOVnEyVkdObnBLVStyeTkvMDFnd05tMnNVSHNZClBmZWszNjNXRU5BOXlqbDNuOXFicXp4aXphaVowekJEM1ZDWkhXRVBrd215Yk1oRnZQY1V4M24rU2tRUnRhYk0KSzdyZTc2bkwwdU9GdTV3L1FUUU5KVVcvdGJ5R0lZVFJHUExibTFJeWs4RUpmc2lWa09yR0tVWE1HelU1UlZ5QQpROGQvWlI1b3Q0L0R3R29OM2NmQytBODlXT1g2dk5kU3B6Y1VsZmtLS1FLQmdRRGtPbDlIRnRiRklZd0VZbmpDCklQMDdmcVArNnhtVnZpQlRjOEFqQnMraGc1NmhadFltQ29aOTQ4RHJZT0MxSEtWVi9WZWVCU2FHZGo4WUNtdnkKZUNkTExvYms3Skc0bHRiUmxMUlpHUGEyR1JSTkZUZ2xhRXQ2Q1F3QXFuQ1hUMkxoVHI1NGswVlZFT00wclFNWgpUN1NMSVdzZXA5N3N2TW15ZGwyWlNBbDNuUUtCZ1FETHBDSTV4UjF6eU9Kb1RQQStaRTIrYVlCMjdqM0VPeSs2CmdyTXovc3M2c2lndVJ4TEMvRHZJWUNLbXc0S213N2N0dUw3eW5UbDRuaFJIVFhwRDV5M0NLTG5OVTBXRVA5L1MKVmsrS25FUWFFdVdaTm9pbU1xUTNMSkcvL0pYYUpPa2c3WmV0eWt2cnYyUHdLY0lncHpKUGdJWjZOeVRzaDV5NwowbUFyVWF4bFpRS0JnUURON2hXV1VYZE12RzVZYm5uRHdIeCtPRkRGYldEU2lwRWtlNmI4YytMWk82Zmd2cWV2Ci80TkhDRUJFb2s5ZlhBK2JQVkxYbEpJa2RZR01zYXFoUitVOG95aTRXdlZKZDJFeURsbUVvMC9KRTJ3TCtYK0YKMFV0NU83eUd4VU4rWS9VMmt4U3VPMFF0ODJUdlhNVVZDNlErZmRMb0FGVFhpNmo2ekc2OEpoSFV5UUtCZ0VDMApyb3RjcnJjVHBaMHVsVWU5NTFZUmY5aEtheVhuQ0l0aTdENGhQOEl1eWNXcW43T0ZJaG5STWpGNi9oQ3ZMNDAvCm5xekllSEp6Q0U1L3Q5SExxeVorZWt0Ym9rTWJhS3NVOGNGQlZnSlM3dEY0R29OMG8rbEVLQ3V3dm96S0hhbHcKMVRsTGhrUXFWRDhEaGNPS1hOb1dKS1RBME9LM1ZIMzVvc1VnOW41aEFvR0FYYm45dHNOVFp1SmpLWXFxMWszVQovM2trR0NadEJnZmEvaCtpRWdPN1RoZFp5ekdzcjRuVGkzQTFyU09iVkZ0amoza3BOTEZCMW91aTVMcEJjMWFWCkQ0VjhuMHhDdktJbTl2N2hCVm9iTWZVZmVoVE1TSFBZOFZvcWJneXY4ZWZueS9MNVh6d2R3b0NXSGpEZFZXS3EKMVlDLzBIRkhlRFJzWm9aT3RtdTVnTTQ9Ci0tLS0tRU5EIFBSSVZBVEUgS0VZLS0tLS0=` - -func TestRGW(t *testing.T) { - suite.Run(t, new(rgwSuite)) +func (r *rgwOpsRecorder) startFunc() func() error { + return func() error { r.starts++; return nil } } - -// Expect: run ceph auth -func addRGWEnableExpectations(r *mocks.Runner) { - // add keyring expectation - r.On("RunCommand", tests.CmdAny("ceph", 9)...).Return("ok", nil).Once() - // start service expectation - r.On("RunCommand", []interface{}{ - "snapctl", "start", "microceph.rgw", "--enable", - }...).Return("ok", nil).Once() +func (r *rgwOpsRecorder) restartFunc() func() error { + return func() error { r.restarts++; return nil } } - -// Expect: run snapctl service stop -func addStopRGWExpectations(s *rgwSuite, r *mocks.Runner) { - u := api.NewURL() - - state := &mocks.MockState{ - URL: u, - ClusterName: "foohost", - } - - s.TestStateInterface.On("ClusterState").Return(state) - r.On("RunCommand", tests.CmdAny("snapctl", 3)...).Return("ok", nil).Once() -} - -// Set up test suite -func (s *rgwSuite) SetupTest() { - s.BaseSuite.SetupTest() - s.CopyCephConfigs() - - s.TestStateInterface = mocks.NewStateInterface(s.T()) -} - -// Test enabling RGW -func (s *rgwSuite) TestEnableRGW() { - r := mocks.NewRunner(s.T()) - - addRGWEnableExpectations(r) - - common.ProcessExec = r - - err := EnableRGW(s.TestStateInterface, 8081, 443, "", "", []string{"10.1.1.1", "10.2.2.2"}) - - assert.NoError(s.T(), err) - - // check that the radosgw.conf file contains expected values - conf := s.ReadCephConfig("radosgw.conf") - assert.Contains(s.T(), conf, "rgw frontends = beast port=8081\n") - assert.Contains(s.T(), conf, "mon host = 10.1.1.1,10.2.2.2") - // run dir must use the stable 'current' symlink, not a revision-specific path - assert.Contains(s.T(), conf, "run dir = "+filepath.Join(s.Tmp, "current", "run")) -} - -// Test enabling RGW -func (s *rgwSuite) TestEnableRGWWithInvalidSSLCertificate() { - r := mocks.NewRunner(s.T()) - - common.ProcessExec = r - - err := EnableRGW(s.TestStateInterface, 80, 443, "invalid-certificate", validSSLPrivateKey, []string{"10.1.1.1", "10.2.2.2"}) - - // we expect an illegal base64 data error - assert.EqualError(s.T(), err, "failed to decode SSL certificate: illegal base64 data at input byte 7") - - // radosgw.conf must not have been created when enable fails early - _, statErr := os.Stat(filepath.Join(s.Tmp, "SNAP_DATA", "conf", "radosgw.conf")) - assert.True(s.T(), os.IsNotExist(statErr)) +func (r *rgwOpsRecorder) keyringFunc() func(string) error { + return func(string) error { r.keyrings++; return nil } } -// Test enabling RGW -func (s *rgwSuite) TestEnableRGWWithInvalidSSLPrivateKey() { - r := mocks.NewRunner(s.T()) - - common.ProcessExec = r - - err := EnableRGW(s.TestStateInterface, 80, 443, validSSLCertificate, "invalid-private-key", []string{"10.1.1.1", "10.2.2.2"}) - - // we expect an illegal base64 data error - assert.EqualError(s.T(), err, "failed to decode SSL private key: illegal base64 data at input byte 7") - - // radosgw.conf must not have been created when enable fails early - _, statErr := os.Stat(filepath.Join(s.Tmp, "SNAP_DATA", "conf", "radosgw.conf")) - assert.True(s.T(), os.IsNotExist(statErr)) +// setupRGWPaths overrides constants.GetPathConst to point at temp directories so +// applyRGWFrontend writes radosgw.conf / SSL files in isolation. Returns a +// restore closure. +func setupRGWPaths(t *testing.T) (restore func()) { + t.Helper() + tmp := t.TempDir() + confPath := filepath.Join(tmp, "conf") + runPath := filepath.Join(tmp, "run") + dataPath := filepath.Join(tmp, "data") + sslPath := filepath.Join(tmp, "ssl") + for _, d := range []string{confPath, runPath, dataPath, sslPath} { + require.NoError(t, os.MkdirAll(d, 0755)) + } + orig := constants.GetPathConst + constants.GetPathConst = func() constants.PathConst { + return constants.PathConst{ + ConfPath: confPath, + RunPath: runPath, + DataPath: dataPath, + SSLFilesPath: sslPath, + } + } + return func() { constants.GetPathConst = orig } } -// Test enabling RGW -func (s *rgwSuite) TestEnableRGWWithMissingSSLCertificate() { - r := mocks.NewRunner(s.T()) - - addRGWEnableExpectations(r) - - common.ProcessExec = r - - err := EnableRGW(s.TestStateInterface, 0, 443, "", validSSLPrivateKey, []string{"10.1.1.1", "10.2.2.2"}) - - assert.NoError(s.T(), err) - - // check that the radosgw.conf file contains expected values - conf := s.ReadCephConfig("radosgw.conf") - assert.Contains(s.T(), conf, "rgw frontends = beast port=80\n") +// setupRGWInjectables replaces the snap/keyring primitives with recorders and +// returns the recorder plus a restore closure. +func setupRGWInjectables(t *testing.T) (*rgwOpsRecorder, func()) { + t.Helper() + rec := &rgwOpsRecorder{} + origStart, origRestart, origKey := startRGWFunc, restartRGWFunc, createRGWKeyringFunc + startRGWFunc = rec.startFunc() + restartRGWFunc = rec.restartFunc() + createRGWKeyringFunc = rec.keyringFunc() + return rec, func() { + startRGWFunc = origStart + restartRGWFunc = origRestart + createRGWKeyringFunc = origKey + } } -// Test enabling RGW -func (s *rgwSuite) TestEnableRGWWithMissingSSLPrivateKey() { - r := mocks.NewRunner(s.T()) - - addRGWEnableExpectations(r) - - common.ProcessExec = r - - err := EnableRGW(s.TestStateInterface, 0, 443, validSSLCertificate, "", []string{"10.1.1.1", "10.2.2.2"}) - - assert.NoError(s.T(), err) - - // check that the radosgw.conf file contains expected values - conf := s.ReadCephConfig("radosgw.conf") - assert.Contains(s.T(), conf, "rgw frontends = beast port=80\n") +func b64(t *testing.T, s string) string { + t.Helper() + return base64.StdEncoding.EncodeToString([]byte(s)) } -// Test enabling RGW -func (s *rgwSuite) TestEnableRGWWithSSL() { - r := mocks.NewRunner(s.T()) - - addRGWEnableExpectations(r) - - common.ProcessExec = r - - err := EnableRGW(s.TestStateInterface, 8081, 443, validSSLCertificate, validSSLPrivateKey, []string{"10.1.1.1", "10.2.2.2"}) - - assert.NoError(s.T(), err) - - // check that the radosgw.conf file contains expected values - conf := s.ReadCephConfig("radosgw.conf") - sslCertificatePath := filepath.Join(s.Tmp, "SNAP_COMMON", "server.crt") - sslPrivateKeyPath := filepath.Join(s.Tmp, "SNAP_COMMON", "server.key") - assert.Contains(s.T(), conf, "rgw frontends = beast port=8081 ssl_port=443 ssl_certificate="+sslCertificatePath+" ssl_private_key="+sslPrivateKeyPath+"\n") +// TestEffectiveRGWPorts verifies the default-port-80 rule is applied only for a +// plaintext frontend, and is centralized so the on-disk render and the DB +// record stay consistent. +func TestEffectiveRGWPorts(t *testing.T) { + tests := []struct { + name string + port, ssl int + cert, key string + wantPort int + wantSSLPort int + }{ + {"plaintext default", 0, 0, "", "", 80, 0}, + {"plaintext explicit", 8080, 0, "", "", 8080, 0}, + {"plaintext drops stray ssl port", 0, 443, "", "", 80, 0}, + {"plaintext explicit drops stray ssl port", 8080, 443, "", "", 8080, 0}, + {"ssl keeps port 0", 0, 443, "c", "k", 0, 443}, + {"ssl explicit port", 8080, 443, "c", "k", 8080, 443}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + p, s := effectiveRGWPorts(tc.port, tc.ssl, tc.cert, tc.key) + assert.Equal(t, tc.wantPort, p) + assert.Equal(t, tc.wantSSLPort, s) + }) + } } -func (s *rgwSuite) TestUpdateRGWCertificates() { - r := mocks.NewRunner(s.T()) - - // snapCheckActive expects: snapctl services microceph.rgw - r.On("RunCommand", "snapctl", "services", "microceph.rgw").Return("microceph.rgw enabled active", nil).Once() - - common.ProcessExec = r - - // Seed a radosgw.conf with SSL so the SSL-configured check passes. - confPath := filepath.Join(s.Tmp, "SNAP_DATA", "conf", "radosgw.conf") - err := os.WriteFile(confPath, []byte("rgw frontends = beast ssl_port=443 ssl_certificate=/tmp/server.crt ssl_private_key=/tmp/server.key\n"), 0644) - assert.NoError(s.T(), err) - - err = UpdateRGWCertificates(s.TestStateInterface, validSSLCertificate, validSSLPrivateKey) - assert.NoError(s.T(), err) - - // Verify cert file was written - certPath := filepath.Join(s.Tmp, "SNAP_COMMON", "server.crt") - certData, err := os.ReadFile(certPath) - assert.NoError(s.T(), err) - assert.Contains(s.T(), string(certData), "BEGIN CERTIFICATE") - - // Verify key file was written - keyPath := filepath.Join(s.Tmp, "SNAP_COMMON", "server.key") - keyData, err := os.ReadFile(keyPath) - assert.NoError(s.T(), err) - assert.Contains(s.T(), string(keyData), "BEGIN PRIVATE KEY") - - // Verify file permissions are 0600 - certInfo, _ := os.Stat(certPath) - assert.Equal(s.T(), os.FileMode(0600), certInfo.Mode().Perm()) - keyInfo, _ := os.Stat(keyPath) - assert.Equal(s.T(), os.FileMode(0600), keyInfo.Mode().Perm()) +// TestApplyRGWFrontendFirstEnable verifies the first enable renders radosgw.conf +// (with the default port 80 for plaintext), creates the keyring/symlink, and +// starts (not restarts) RGW, reporting changed=true. +func TestApplyRGWFrontendFirstEnable(t *testing.T) { + restorePaths := setupRGWPaths(t) + defer restorePaths() + rec, restoreInj := setupRGWInjectables(t) + defer restoreInj() + + changed, err := applyRGWFrontend(nil, 0, 0, "", "", []string{"mon1"}) + require.NoError(t, err) + assert.True(t, changed, "first enable must report changed") + + conf, err := os.ReadFile(filepath.Join(constants.GetPathConst().ConfPath, "radosgw.conf")) + require.NoError(t, err) + assert.Contains(t, string(conf), "port=80", "default plaintext port must be rendered") + assert.NotContains(t, string(conf), "ssl_certificate=") + + assert.Equal(t, 1, rec.starts, "first enable must start RGW") + assert.Equal(t, 0, rec.restarts) + assert.Equal(t, 1, rec.keyrings) } -func (s *rgwSuite) TestUpdateRGWCertificatesWhenRGWNotActive() { - r := mocks.NewRunner(s.T()) - - // snapCheckActive returns inactive - r.On("RunCommand", "snapctl", "services", "microceph.rgw").Return("microceph.rgw disabled inactive", nil).Once() - - common.ProcessExec = r - - err := UpdateRGWCertificates(s.TestStateInterface, validSSLCertificate, validSSLPrivateKey) - assert.Error(s.T(), err) - assert.Contains(s.T(), err.Error(), "RGW service is not running") +// TestApplyRGWFrontendReapplyNoChange verifies an identical re-apply is a no-op: +// changed=false, no restart, no extra start, no extra keyring creation. +func TestApplyRGWFrontendReapplyNoChange(t *testing.T) { + restorePaths := setupRGWPaths(t) + defer restorePaths() + rec, restoreInj := setupRGWInjectables(t) + defer restoreInj() + + _, err := applyRGWFrontend(nil, 80, 0, "", "", []string{"mon1"}) + require.NoError(t, err) + + changed, err := applyRGWFrontend(nil, 80, 0, "", "", []string{"mon1"}) + require.NoError(t, err) + assert.False(t, changed, "identical re-apply must report no change") + assert.Equal(t, 1, rec.starts, "no extra start on re-apply") + assert.Equal(t, 0, rec.restarts, "no restart on identical re-apply") + assert.Equal(t, 1, rec.keyrings, "no extra keyring creation on re-apply") } -func (s *rgwSuite) TestUpdateRGWCertificatesSSLNotConfigured() { - r := mocks.NewRunner(s.T()) - - r.On("RunCommand", "snapctl", "services", "microceph.rgw").Return("microceph.rgw enabled active", nil).Once() - - common.ProcessExec = r - - // Seed a radosgw.conf without SSL — RGW was enabled without certificates. - confPath := filepath.Join(s.Tmp, "SNAP_DATA", "conf", "radosgw.conf") - _ = os.WriteFile(confPath, []byte("rgw frontends = beast port=80\n"), 0644) - - err := UpdateRGWCertificates(s.TestStateInterface, validSSLCertificate, validSSLPrivateKey) - assert.Error(s.T(), err) - assert.Contains(s.T(), err.Error(), "RGW is not configured with SSL") +// TestApplyRGWFrontendMultiMonitorReorderNoRestart is the regression test for +// the idempotency defeat: with more than one monitor (and an IPv6 monitor), +// the rendered `mon host` line must not drive a restart. A re-apply with the +// monitors in a different order, and after the on-disk `mon host` line has been +// rewritten out-of-band (as UpdateConfig->updateRadosGWMonHost does), must be a +// no-op: no restart, changed=false. +func TestApplyRGWFrontendMultiMonitorReorderNoRestart(t *testing.T) { + restorePaths := setupRGWPaths(t) + defer restorePaths() + rec, restoreInj := setupRGWInjectables(t) + defer restoreInj() + + monsA := []string{"10.0.0.3", "10.0.0.1", "fe80::1"} + monsB := []string{"fe80::1", "10.0.0.1", "10.0.0.3"} // same set, different order + + changed, err := applyRGWFrontend(nil, 80, 0, "", "", monsA) + require.NoError(t, err) + assert.True(t, changed, "first enable must report changed") + assert.Equal(t, 1, rec.starts) + + // Simulate the periodic UpdateConfig->updateRadosGWMonHost rewrite of the + // mon host line (sorted + IPv6-bracketed), independent of applyRGWFrontend. + confPath := filepath.Join(constants.GetPathConst().ConfPath, "radosgw.conf") + require.NoError(t, updateRadosGWMonHost(constants.GetPathConst().ConfPath, formatIPv6(monsA))) + before, err := os.ReadFile(confPath) + require.NoError(t, err) + + // Re-apply the same frontend with monitors in a different order. + changed, err = applyRGWFrontend(nil, 80, 0, "", "", monsB) + require.NoError(t, err) + assert.False(t, changed, "re-apply with reordered monitors must be a no-op") + assert.Equal(t, 0, rec.restarts, "reordered monitors must not restart RGW") + + // The on-disk mon host line (owned by updateRadosGWMonHost) must be intact. + after, err := os.ReadFile(confPath) + require.NoError(t, err) + assert.Equal(t, string(before), string(after), "radosgw.conf must be untouched on a no-op re-apply") + assert.Contains(t, string(after), "[fe80::1]", "IPv6 mon host must remain bracketed") } -func (s *rgwSuite) TestUpdateRGWCertificatesInvalidCertificate() { - r := mocks.NewRunner(s.T()) +// TestApplyRGWFrontendPortChange verifies a port change rewrites radosgw.conf and +// restarts RGW (not start), and the pre-existing keyring symlink does not error. +func TestApplyRGWFrontendPortChange(t *testing.T) { + restorePaths := setupRGWPaths(t) + defer restorePaths() + rec, restoreInj := setupRGWInjectables(t) + defer restoreInj() - r.On("RunCommand", "snapctl", "services", "microceph.rgw").Return("microceph.rgw enabled active", nil).Once() + _, err := applyRGWFrontend(nil, 80, 0, "", "", []string{"mon1"}) + require.NoError(t, err) - common.ProcessExec = r + changed, err := applyRGWFrontend(nil, 8080, 0, "", "", []string{"mon1"}) + require.NoError(t, err) + assert.True(t, changed, "port change must report changed") - // Seed a radosgw.conf with SSL so the SSL-configured check passes. - confPath := filepath.Join(s.Tmp, "SNAP_DATA", "conf", "radosgw.conf") - _ = os.WriteFile(confPath, []byte("rgw frontends = beast ssl_port=443 ssl_certificate=/tmp/server.crt ssl_private_key=/tmp/server.key\n"), 0644) + conf, err := os.ReadFile(filepath.Join(constants.GetPathConst().ConfPath, "radosgw.conf")) + require.NoError(t, err) + assert.Contains(t, string(conf), "port=8080") - err := UpdateRGWCertificates(s.TestStateInterface, "invalid-base64!", validSSLPrivateKey) - assert.Error(s.T(), err) - assert.Contains(s.T(), err.Error(), "failed to decode SSL certificate") + assert.Equal(t, 1, rec.starts, "start only on first enable") + assert.Equal(t, 1, rec.restarts, "port change must restart RGW") } -func (s *rgwSuite) TestUpdateRGWCertificatesInvalidPrivateKey() { - r := mocks.NewRunner(s.T()) - - r.On("RunCommand", "snapctl", "services", "microceph.rgw").Return("microceph.rgw enabled active", nil).Once() - - common.ProcessExec = r - - // Seed a radosgw.conf with SSL so the SSL-configured check passes. - confPath := filepath.Join(s.Tmp, "SNAP_DATA", "conf", "radosgw.conf") - _ = os.WriteFile(confPath, []byte("rgw frontends = beast ssl_port=443 ssl_certificate=/tmp/server.crt ssl_private_key=/tmp/server.key\n"), 0644) - - err := UpdateRGWCertificates(s.TestStateInterface, validSSLCertificate, "invalid-base64!") - assert.Error(s.T(), err) - assert.Contains(s.T(), err.Error(), "failed to decode SSL private key") -} - -func (s *rgwSuite) TestWriteSSLFilesRejectsEmpty() { - sslDir := filepath.Join(s.Tmp, "SNAP_COMMON") - - // Empty certificate should be rejected. - _, _, err := writeSSLFiles(sslDir, "", validSSLPrivateKey) - assert.Error(s.T(), err) - assert.Contains(s.T(), err.Error(), "SSL certificate cannot be empty") - - // Empty key should be rejected. - _, _, err = writeSSLFiles(sslDir, validSSLCertificate, "") - assert.Error(s.T(), err) - assert.Contains(s.T(), err.Error(), "SSL private key cannot be empty") - - // Both empty should be rejected. - _, _, err = writeSSLFiles(sslDir, "", "") - assert.Error(s.T(), err) - assert.Contains(s.T(), err.Error(), "SSL certificate cannot be empty") -} - -func (s *rgwSuite) TestWriteSSLFilesAtomicCleanup() { - sslDir := filepath.Join(s.Tmp, "SNAP_COMMON") - - // Call with valid cert but invalid key to trigger failure after cert .tmp is written. - _, _, err := writeSSLFiles(sslDir, validSSLCertificate, "invalid-base64!") - assert.Error(s.T(), err) - - // Verify no .tmp files are left behind. - _, err = os.Stat(filepath.Join(sslDir, "server.crt.tmp")) - assert.True(s.T(), os.IsNotExist(err), "server.crt.tmp should be cleaned up") - _, err = os.Stat(filepath.Join(sslDir, "server.key.tmp")) - assert.True(s.T(), os.IsNotExist(err), "server.key.tmp should be cleaned up") - - // Verify no final files were written either. - _, err = os.Stat(filepath.Join(sslDir, "server.crt")) - assert.True(s.T(), os.IsNotExist(err), "server.crt should not exist after failure") - _, err = os.Stat(filepath.Join(sslDir, "server.key")) - assert.True(s.T(), os.IsNotExist(err), "server.key should not exist after failure") +// TestApplyRGWFrontendSSLEnable verifies SSL is written, the conf references the +// cert path, and a plaintext->TLS transition restarts RGW. +func TestApplyRGWFrontendSSLEnable(t *testing.T) { + restorePaths := setupRGWPaths(t) + defer restorePaths() + rec, restoreInj := setupRGWInjectables(t) + defer restoreInj() + + // Start plaintext. + _, err := applyRGWFrontend(nil, 80, 0, "", "", []string{"mon1"}) + require.NoError(t, err) + + // Transition to TLS. + changed, err := applyRGWFrontend(nil, 0, 443, b64(t, "cert1"), b64(t, "key1"), []string{"mon1"}) + require.NoError(t, err) + assert.True(t, changed) + + conf, err := os.ReadFile(filepath.Join(constants.GetPathConst().ConfPath, "radosgw.conf")) + require.NoError(t, err) + assert.Contains(t, string(conf), "ssl_port=443") + assert.Contains(t, string(conf), "ssl_certificate=") + + // SSL files exist on disk. + _, err = os.ReadFile(filepath.Join(constants.GetPathConst().SSLFilesPath, "server.crt")) + require.NoError(t, err) + + assert.Equal(t, 1, rec.restarts, "TLS transition must restart") } -func (s *rgwSuite) TestWriteSSLFilesAtomicSuccess() { - sslDir := filepath.Join(s.Tmp, "SNAP_COMMON") +// TestApplyRGWFrontendSSLCertRotation verifies a cert/key rotation (same ports) +// is detected via the cert/key bytes (the conf path is unchanged, so the conf +// bytes are identical) and restarts RGW. +func TestApplyRGWFrontendSSLCertRotation(t *testing.T) { + restorePaths := setupRGWPaths(t) + defer restorePaths() + rec, restoreInj := setupRGWInjectables(t) + defer restoreInj() - certPath, keyPath, err := writeSSLFiles(sslDir, validSSLCertificate, validSSLPrivateKey) - assert.NoError(s.T(), err) + _, err := applyRGWFrontend(nil, 0, 443, b64(t, "cert1"), b64(t, "key1"), []string{"mon1"}) + require.NoError(t, err) - // Verify final files exist. - certData, err := os.ReadFile(certPath) - assert.NoError(s.T(), err) - assert.Contains(s.T(), string(certData), "BEGIN CERTIFICATE") + changed, err := applyRGWFrontend(nil, 0, 443, b64(t, "cert2"), b64(t, "key2"), []string{"mon1"}) + require.NoError(t, err) + assert.True(t, changed, "cert rotation must be detected even when conf path is unchanged") - keyData, err := os.ReadFile(keyPath) - assert.NoError(s.T(), err) - assert.Contains(s.T(), string(keyData), "BEGIN PRIVATE KEY") + crt, err := os.ReadFile(filepath.Join(constants.GetPathConst().SSLFilesPath, "server.crt")) + require.NoError(t, err) + assert.Equal(t, "cert2", string(crt), "rotated cert must be on disk") - // Verify no .tmp files are left behind. - _, err = os.Stat(certPath + ".tmp") - assert.True(s.T(), os.IsNotExist(err), "server.crt.tmp should not exist after success") - _, err = os.Stat(keyPath + ".tmp") - assert.True(s.T(), os.IsNotExist(err), "server.key.tmp should not exist after success") + assert.Equal(t, 1, rec.restarts, "cert rotation must restart") } -func (s *rgwSuite) TestRestartRGW() { - r := mocks.NewRunner(s.T()) - - r.On("RunCommand", "snapctl", "restart", "microceph.rgw").Return("ok", nil).Once() +// TestApplyRGWFrontendSSLToPlaintext verifies a TLS->plaintext transition removes +// the leftover SSL files and rewrites the conf without the SSL line. +func TestApplyRGWFrontendSSLToPlaintext(t *testing.T) { + restorePaths := setupRGWPaths(t) + defer restorePaths() + rec, restoreInj := setupRGWInjectables(t) + defer restoreInj() - common.ProcessExec = r + _, err := applyRGWFrontend(nil, 0, 443, b64(t, "cert1"), b64(t, "key1"), []string{"mon1"}) + require.NoError(t, err) - err := RestartRGW() - assert.NoError(s.T(), err) -} + changed, err := applyRGWFrontend(nil, 80, 0, "", "", []string{"mon1"}) + require.NoError(t, err) + assert.True(t, changed) -func (s *rgwSuite) TestRestartRGWFailure() { - r := mocks.NewRunner(s.T()) + conf, err := os.ReadFile(filepath.Join(constants.GetPathConst().ConfPath, "radosgw.conf")) + require.NoError(t, err) + assert.NotContains(t, string(conf), "ssl_certificate=") + assert.Contains(t, string(conf), "port=80") - r.On("RunCommand", "snapctl", "restart", "microceph.rgw").Return("", fmt.Errorf("service not found")).Once() + _, err = os.Stat(filepath.Join(constants.GetPathConst().SSLFilesPath, "server.crt")) + assert.True(t, os.IsNotExist(err), "leftover cert must be removed on TLS->plaintext") + _, err = os.Stat(filepath.Join(constants.GetPathConst().SSLFilesPath, "server.key")) + assert.True(t, os.IsNotExist(err), "leftover key must be removed on TLS->plaintext") - common.ProcessExec = r - - err := RestartRGW() - assert.Error(s.T(), err) - assert.Contains(s.T(), err.Error(), "failed to restart RGW service") + assert.Equal(t, 1, rec.restarts) } -func (s *rgwSuite) TestDisableRGW() { - r := mocks.NewRunner(s.T()) - - addStopRGWExpectations(s, r) - - common.ProcessExec = r - - err := DisableRGW(context.Background(), s.TestStateInterface) - - // we expect a missing database error - assert.EqualError(s.T(), err, "no server certificate") - - // check that the radosgw.conf file is absent - _, err = os.Stat(filepath.Join(s.Tmp, "SNAP_DATA", "conf", "radosgw.conf")) - assert.True(s.T(), os.IsNotExist(err)) - - // check that the keyring file is absent - _, err = os.Stat(filepath.Join(s.Tmp, "SNAP_COMMON", "data", "radosgw", "ceph-radosgw.gateway", "keyring")) - assert.True(s.T(), os.IsNotExist(err)) +// TestApplyRGWFrontendBadBase64 verifies malformed SSL material surfaces a clear +// error (maps to HTTP 400 at the API layer) and reports no change. +func TestApplyRGWFrontendBadBase64(t *testing.T) { + restorePaths := setupRGWPaths(t) + defer restorePaths() + _, restoreInj := setupRGWInjectables(t) + defer restoreInj() + + changed, err := applyRGWFrontend(nil, 0, 443, "not-base64!!", b64(t, "key1"), []string{"mon1"}) + require.Error(t, err) + assert.False(t, changed) + assert.Contains(t, err.Error(), "SSL certificate", "error must identify the bad material") + assert.ErrorIs(t, err, ErrRgwFrontendInvalid, "malformed TLS must map to a client-side error") } diff --git a/microceph/ceph/services.go b/microceph/ceph/services.go index dce47150..85feec2e 100644 --- a/microceph/ceph/services.go +++ b/microceph/ceph/services.go @@ -202,6 +202,15 @@ func removeServiceDatabase(ctx context.Context, s interfaces.StateInterface, ser } } + // Drop the recorded RGW frontend so observed state does not outlive the + // service row. Both are removed in the same transaction. + if service == "rgw" { + err = database.DeleteRGWFrontendByMember(ctx, tx, s.ClusterState().Name()) + if err != nil { + return fmt.Errorf("failed to remove rgw frontend from db: %w", err) + } + } + return nil }) return err diff --git a/microceph/ceph/services_placement_rgw.go b/microceph/ceph/services_placement_rgw.go index 50cc3cfb..7eee5940 100644 --- a/microceph/ceph/services_placement_rgw.go +++ b/microceph/ceph/services_placement_rgw.go @@ -2,9 +2,14 @@ package ceph import ( "context" + "database/sql" "encoding/json" "fmt" + "net/http" + "github.com/canonical/lxd/shared/api" + + "github.com/canonical/microceph/microceph/database" "github.com/canonical/microceph/microceph/interfaces" ) @@ -24,7 +29,6 @@ func (rgw *RgwServicePlacement) PopulateParams(s interfaces.StateInterface, payl return nil } - func (rgw *RgwServicePlacement) HospitalityCheck(s interfaces.StateInterface) error { return genericHospitalityCheck("rgw") } @@ -44,5 +48,34 @@ func (rgw *RgwServicePlacement) PostPlacementCheck(s interfaces.StateInterface) } func (rgw *RgwServicePlacement) DbUpdate(ctx context.Context, s interfaces.StateInterface) error { - return genericDbUpdate(ctx, s, "rgw") + if s.ClusterState().ServerCert() == nil { + return fmt.Errorf("no server certificate") + } + + member := s.ClusterState().Name() + // Record the ports that were actually applied (after the default-port-80 + // rule) so the observed frontend matches what is running, not the raw input. + effPort, effSSLPort := effectiveRGWPorts(rgw.Port, rgw.SSLPort, rgw.SSLCertificate, rgw.SSLPrivateKey) + ssl := rgw.SSLCertificate != "" && rgw.SSLPrivateKey != "" + + return s.ClusterState().Database().Transaction(ctx, func(ctx context.Context, tx *sql.Tx) error { + // Idempotent services row: ignore an already-present (member, rgw) row so + // re-applying placement to an enabled member does not error on a + // duplicate insert. CreateService returns http.StatusConflict (409) when + // the row exists; that is the success case for a re-apply. + _, err := database.CreateService(ctx, tx, database.Service{Member: member, Service: "rgw"}) + if err != nil && !api.StatusErrorCheck(err, http.StatusConflict) { + return fmt.Errorf("failed to record rgw service: %w", err) + } + + // Record the observed frontend (ports + TLS flag) in the same transaction + // as the services row, so presence and frontend config stay consistent + // and GET /placement can report them from dqlite. Stores ports + flag + // only, never cert/key bytes. + err = database.UpsertRGWFrontend(ctx, tx, member, effPort, effSSLPort, ssl) + if err != nil { + return fmt.Errorf("failed to record rgw frontend: %w", err) + } + return nil + }) } diff --git a/microceph/cmd/microcephd/prod_wiring.go b/microceph/cmd/microcephd/prod_wiring.go index 1c9b9074..e291a9a5 100644 --- a/microceph/cmd/microcephd/prod_wiring.go +++ b/microceph/cmd/microcephd/prod_wiring.go @@ -2,6 +2,7 @@ package main import ( "context" + "encoding/json" "fmt" "github.com/canonical/microceph/microceph/api/types" @@ -28,6 +29,8 @@ func wireProductionFuncs() { ceph.GetClusterMemberNamesFunc = prodGetClusterMemberNamesFunc ceph.ProdAddControlServiceFunc = prodAddControlServiceFunc ceph.ProdRemoveControlServiceFunc = prodRemoveControlServiceFunc + ceph.ProdEnableRgwServiceFunc = prodEnableRgwServiceFunc + ceph.ProdRemoveRgwServiceFunc = prodRemoveRgwServiceFunc ceph.BootstrapCephStepsFunc = prodBootstrapCephStepsFunc } @@ -68,6 +71,43 @@ func prodRemoveControlServiceFunc(ctx context.Context, s interfaces.StateInterfa return client.DeleteService(ctx, cli, member, service) } +// prodEnableRgwServiceFunc enables/reconciles RGW with its frontend config (port +// + TLS material) on a target member via the existing services/rgw placement +// API. The payload is the RgwServicePlacement struct the enable rgw CLI already +// uses, so the member-side ServiceInit/DbUpdate reuse the same path. The +// member-side applyRGWFrontend decides whether a restart is actually needed. +func prodEnableRgwServiceFunc(ctx context.Context, s interfaces.StateInterface, member string, rgw types.RgwPlacement) error { + cli, err := s.ClusterState().Connect().Leader(false) + if err != nil { + return fmt.Errorf("failed to get leader client: %w", err) + } + payload, err := json.Marshal(ceph.RgwServicePlacement{ + Port: rgw.Port, + SSLPort: rgw.SSLPort, + SSLCertificate: rgw.SSLCertificate, + SSLPrivateKey: rgw.SSLPrivateKey, + }) + if err != nil { + return fmt.Errorf("failed to marshal rgw placement payload: %w", err) + } + return client.SendServicePlacementReq(ctx, cli, &types.EnableService{ + Name: "rgw", + Wait: true, + Payload: string(payload), + }, member) +} + +// prodRemoveRgwServiceFunc removes RGW from a target member via the service +// deletion API. DisableRGW on the member is idempotent, so a re-run after a +// partial disable still converges. +func prodRemoveRgwServiceFunc(ctx context.Context, s interfaces.StateInterface, member string) error { + cli, err := s.ClusterState().Connect().Leader(false) + if err != nil { + return fmt.Errorf("failed to get leader client: %w", err) + } + return client.DeleteService(ctx, cli, member, "rgw") +} + // prodBootstrapCephStepsFunc runs the Ceph bootstrap steps on the local node // (the handler proxies to the target member via ProxyTarget:true, so the handler // body executes on the target member where s.Name()==target). It reuses the diff --git a/microceph/database/rgw_frontend.go b/microceph/database/rgw_frontend.go new file mode 100644 index 00000000..754913e6 --- /dev/null +++ b/microceph/database/rgw_frontend.go @@ -0,0 +1,83 @@ +package database + +import ( + "context" + "database/sql" + "fmt" +) + +// RgwFrontend is the observed RGW beast frontend configuration recorded for a +// cluster member (CE142 placement-rgw). It stores ports and a TLS on/off flag +// only — never cert/key bytes, which remain on disk in server.crt/server.key. +// Member is the cluster member name (resolved to/from member_id at the DB +// layer), matching PlacementObservedMember.Member. +type RgwFrontend struct { + Member string + Port int + SSLPort int + SSL bool +} + +// UpsertRGWFrontend records (or replaces) the RGW frontend configuration for +// the named member. It is called from the member-side service DbUpdate in the +// same transaction that records the services row, so the services record and +// the observed frontend stay consistent. The member name is resolved to +// member_id via a subquery, mirroring the generated client_config mapper. A +// re-apply with new port/TLS overwrites the prior row (INSERT OR REPLACE on the +// UNIQUE(member_id) constraint), making rotation and port changes idempotent. +func UpsertRGWFrontend(ctx context.Context, tx *sql.Tx, member string, port, sslPort int, ssl bool) error { + sslInt := 0 + if ssl { + sslInt = 1 + } + _, err := tx.ExecContext(ctx, ` +INSERT OR REPLACE INTO rgw_frontends (member_id, port, ssl_port, ssl) +VALUES ((SELECT id FROM core_cluster_members WHERE name = ?), ?, ?, ?)`, member, port, sslPort, sslInt) + if err != nil { + return fmt.Errorf("failed to upsert rgw frontend for %s: %w", member, err) + } + return nil +} + +// GetRGWFrontends returns the recorded RGW frontend configuration for every +// member that has one, keyed by member name. Used by GetPlacementStatus to +// populate PlacementObservedMember.RgwFrontend in the same DB transaction as +// the rest of the observed state. +func GetRGWFrontends(ctx context.Context, tx *sql.Tx) ([]RgwFrontend, error) { + rows, err := tx.QueryContext(ctx, ` +SELECT m.name, f.port, f.ssl_port, f.ssl + FROM rgw_frontends f + JOIN core_cluster_members m ON m.id = f.member_id`) + if err != nil { + return nil, fmt.Errorf("failed to query rgw frontends: %w", err) + } + defer func() { _ = rows.Close() }() + + var frontends []RgwFrontend + for rows.Next() { + var f RgwFrontend + var sslInt int + err := rows.Scan(&f.Member, &f.Port, &f.SSLPort, &sslInt) + if err != nil { + return nil, fmt.Errorf("failed to scan rgw frontend: %w", err) + } + f.SSL = sslInt != 0 + frontends = append(frontends, f) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("rgw frontend rows error: %w", err) + } + return frontends, nil +} + +// DeleteRGWFrontendByMember removes the RGW frontend record for the named +// member. A missing row is not an error, so scale-down and retried applies are +// idempotent. Called from DisableRGW alongside removeServiceDatabase. +func DeleteRGWFrontendByMember(ctx context.Context, tx *sql.Tx, member string) error { + _, err := tx.ExecContext(ctx, ` +DELETE FROM rgw_frontends WHERE member_id = (SELECT id FROM core_cluster_members WHERE name = ?)`, member) + if err != nil { + return fmt.Errorf("failed to delete rgw frontend for %s: %w", member, err) + } + return nil +} diff --git a/microceph/database/rgw_frontend_extras_test.go b/microceph/database/rgw_frontend_extras_test.go new file mode 100644 index 00000000..09ec4858 --- /dev/null +++ b/microceph/database/rgw_frontend_extras_test.go @@ -0,0 +1,186 @@ +package database + +import ( + "context" + "database/sql" + "testing" + + _ "github.com/mattn/go-sqlite3" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// setupRGWFrontendDB creates an in-memory SQLite database (with foreign keys +// enabled) containing a minimal core_cluster_members table (the FK target) and +// the real schemaUpdate10 migration that creates rgw_frontends. Two named +// members are seeded so name->id resolution and cascade-delete can be tested. +func setupRGWFrontendDB(t *testing.T) *sql.DB { + t.Helper() + // _foreign_keys=on is per-connection in SQLite; the DSN parameter ensures + // every pooled connection honours ON DELETE CASCADE. + db, err := sql.Open("sqlite3", ":memory:?_foreign_keys=on") + require.NoError(t, err) + t.Cleanup(func() { _ = db.Close() }) + + // Minimal core_cluster_members matching the columns the rgw_frontends + // migration and the name->id subqueries reference. + _, err = db.Exec(` +CREATE TABLE core_cluster_members ( + id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, + name TEXT NOT NULL UNIQUE +); +INSERT INTO core_cluster_members (name) VALUES ('node-a'); +INSERT INTO core_cluster_members (name) VALUES ('node-b'); +`) + require.NoError(t, err) + + tx, err := db.BeginTx(context.Background(), nil) + require.NoError(t, err) + err = schemaUpdate10(context.Background(), tx) + require.NoError(t, err) + require.NoError(t, tx.Commit()) + + return db +} + +// TestSchemaUpdate10CreatesTable verifies schemaUpdate10 creates the +// rgw_frontends table on an upgraded database. +func TestSchemaUpdate10CreatesTable(t *testing.T) { + db := setupRGWFrontendDB(t) + + var name string + err := db.QueryRow(`SELECT name FROM sqlite_master WHERE name = 'rgw_frontends'`).Scan(&name) + require.NoError(t, err) + assert.Equal(t, "rgw_frontends", name) +} + +// TestUpsertRGWFrontendAndGetAll verifies an upsert is persisted and read back +// keyed by member name, with ports and the TLS flag intact. +func TestUpsertRGWFrontendAndGetAll(t *testing.T) { + db := setupRGWFrontendDB(t) + ctx := context.Background() + + tx, err := db.BeginTx(ctx, nil) + require.NoError(t, err) + err = UpsertRGWFrontend(ctx, tx, "node-a", 80, 0, false) + require.NoError(t, err) + require.NoError(t, tx.Commit()) + + tx2, err := db.BeginTx(ctx, nil) + require.NoError(t, err) + defer func() { _ = tx2.Rollback() }() + + frontends, err := GetRGWFrontends(ctx, tx2) + require.NoError(t, err) + require.Len(t, frontends, 1) + assert.Equal(t, "node-a", frontends[0].Member) + assert.Equal(t, 80, frontends[0].Port) + assert.Equal(t, 0, frontends[0].SSLPort) + assert.False(t, frontends[0].SSL) +} + +// TestUpsertRGWFrontendOverwrites verifies a second upsert for the same member +// replaces the row (cert rotation / port change), not a duplicate. +func TestUpsertRGWFrontendOverwrites(t *testing.T) { + db := setupRGWFrontendDB(t) + ctx := context.Background() + + lockTx(t, db, func(tx *sql.Tx) { + require.NoError(t, UpsertRGWFrontend(ctx, tx, "node-a", 80, 0, false)) + }) + lockTx(t, db, func(tx *sql.Tx) { + require.NoError(t, UpsertRGWFrontend(ctx, tx, "node-a", 8080, 443, true)) + }) + + tx, err := db.BeginTx(ctx, nil) + require.NoError(t, err) + defer func() { _ = tx.Rollback() }() + + frontends, err := GetRGWFrontends(ctx, tx) + require.NoError(t, err) + require.Len(t, frontends, 1, "upsert must replace, not append") + assert.Equal(t, 8080, frontends[0].Port) + assert.Equal(t, 443, frontends[0].SSLPort) + assert.True(t, frontends[0].SSL) +} + +// TestGetRGWFrontendsMultipleMembers verifies the read returns each member's +// frontend keyed by name. +func TestGetRGWFrontendsMultipleMembers(t *testing.T) { + db := setupRGWFrontendDB(t) + ctx := context.Background() + + lockTx(t, db, func(tx *sql.Tx) { + require.NoError(t, UpsertRGWFrontend(ctx, tx, "node-a", 80, 0, false)) + require.NoError(t, UpsertRGWFrontend(ctx, tx, "node-b", 8443, 443, true)) + }) + + tx, err := db.BeginTx(ctx, nil) + require.NoError(t, err) + defer func() { _ = tx.Rollback() }() + + frontends, err := GetRGWFrontends(ctx, tx) + require.NoError(t, err) + require.Len(t, frontends, 2) + + byName := map[string]RgwFrontend{} + for _, f := range frontends { + byName[f.Member] = f + } + assert.Contains(t, byName, "node-a") + assert.Contains(t, byName, "node-b") + assert.True(t, byName["node-b"].SSL) +} + +// TestDeleteRGWFrontendByMember verifies the row is removed and that deleting a +// missing row is not an error (idempotent scale-down / retry). +func TestDeleteRGWFrontendByMember(t *testing.T) { + db := setupRGWFrontendDB(t) + ctx := context.Background() + + lockTx(t, db, func(tx *sql.Tx) { + require.NoError(t, UpsertRGWFrontend(ctx, tx, "node-a", 80, 0, false)) + }) + + lockTx(t, db, func(tx *sql.Tx) { + require.NoError(t, DeleteRGWFrontendByMember(ctx, tx, "node-a")) + }) + + // Read in its own (rolled-back) transaction, released before the re-delete + // below so the pool reuses the same :memory: connection. + func() { + tx, err := db.BeginTx(ctx, nil) + require.NoError(t, err) + frontends, err := GetRGWFrontends(ctx, tx) + require.NoError(t, err) + assert.Empty(t, frontends) + require.NoError(t, tx.Rollback()) + }() + + // Deleting an already-absent row must succeed (idempotent). + lockTx(t, db, func(tx *sql.Tx) { + require.NoError(t, DeleteRGWFrontendByMember(ctx, tx, "node-a")) + }) +} + +// TestRGWFrontendCascadeOnMemberRemoval verifies the ON DELETE CASCADE foreign +// key removes the frontend row when the cluster member is deleted. +func TestRGWFrontendCascadeOnMemberRemoval(t *testing.T) { + db := setupRGWFrontendDB(t) + ctx := context.Background() + + lockTx(t, db, func(tx *sql.Tx) { + require.NoError(t, UpsertRGWFrontend(ctx, tx, "node-a", 80, 0, false)) + }) + + _, err := db.Exec(`DELETE FROM core_cluster_members WHERE name = 'node-a'`) + require.NoError(t, err) + + tx, err := db.BeginTx(ctx, nil) + require.NoError(t, err) + defer func() { _ = tx.Rollback() }() + + frontends, err := GetRGWFrontends(ctx, tx) + require.NoError(t, err) + assert.Empty(t, frontends, "removing the member must cascade-delete its frontend row") +} diff --git a/microceph/database/schema.go b/microceph/database/schema.go index 17f305d8..106854b3 100644 --- a/microceph/database/schema.go +++ b/microceph/database/schema.go @@ -22,6 +22,7 @@ var SchemaExtensions = []cluster.Update{ schemaUpdate7, schemaUpdate8, schemaUpdate9, + schemaUpdate10, } // getClusterTableName returns the name of the table that holds the record of cluster members from sqlite_master. @@ -296,3 +297,29 @@ INSERT INTO placement_policy (id) VALUES (1); return err } + +// schemaUpdate10 adds the rgw_frontends table (CE142 placement-rgw). It records +// the observed RGW beast frontend (port, ssl_port, and a TLS on/off flag) for +// each member hosting RGW, so GET /1.0/placement can report observed frontend +// state from dqlite alongside the rest of its observed state, without a +// per-member fan-out. Only ports and the TLS flag are stored — never cert/key +// bytes, which remain on disk in server.crt/server.key (0600). The row is +// cascade-deleted when the cluster member is removed, mirroring the services +// and host_tags tables. At most one row per member is enforced by UNIQUE on +// member_id. +func schemaUpdate10(ctx context.Context, tx *sql.Tx) error { + stmt := ` +CREATE TABLE rgw_frontends ( + id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, + member_id INTEGER NOT NULL, + port INTEGER NOT NULL DEFAULT 0, + ssl_port INTEGER NOT NULL DEFAULT 0, + ssl INTEGER NOT NULL DEFAULT 0, + FOREIGN KEY (member_id) REFERENCES "core_cluster_members" (id) ON DELETE CASCADE, + UNIQUE(member_id) +); + ` + _, err := tx.ExecContext(ctx, stmt) + + return err +} diff --git a/tests/robot/resources/placement_status.py b/tests/robot/resources/placement_status.py index 552a8668..f845e583 100644 --- a/tests/robot/resources/placement_status.py +++ b/tests/robot/resources/placement_status.py @@ -69,6 +69,60 @@ def supported_capabilities(raw): return [str(s) for s in supported] +def observed_member(raw, member): + """Return the observed placement dict for *member* from a GET /1.0/placement + body, or {} when the member is absent or the body is malformed. + """ + observed = response_metadata(raw).get("observed") + if not isinstance(observed, list): + return {} + for entry in observed: + if isinstance(entry, dict) and entry.get("member") == member: + return entry + return {} + + +def member_rgw_frontend(raw, member): + """Return *member*'s observed ``rgw_frontend`` object from a GET + /1.0/placement body, or {} when absent. + + The frontend reports ``port``/``ssl_port``/``ssl`` only; a suite asserts + these match the requested port/TLS, proving observed state is sourced from + the rgw_frontends table rather than fanned out. + """ + frontend = observed_member(raw, member).get("rgw_frontend") + if not isinstance(frontend, dict): + return {} + return frontend + + +def placement_leaks_rgw_secrets(raw): + """Return True when a GET /1.0/placement body contains any RGW SSL key + material, i.e. a non-empty ``ssl_certificate`` or ``ssl_private_key`` under + the stored policy's members. + + The snap MUST strip these before storage and redact them on GET; a suite + asserts this returns False. Fails safe: a body it cannot parse as the + expected shape reports no leak (there is nothing to leak), but a present + non-empty secret string is always detected. + """ + policy = response_metadata(raw).get("policy") + if not isinstance(policy, dict): + return False + members = policy.get("members") + if not isinstance(members, dict): + return False + for entry in members.values(): + if not isinstance(entry, dict): + continue + rgw = entry.get("rgw") + if not isinstance(rgw, dict): + continue + if rgw.get("ssl_certificate") or rgw.get("ssl_private_key"): + return True + return False + + def mon_count(raw): """Return the monmap daemon count from ``ceph -s -f json`` output. diff --git a/tests/robot/resources/test_harness_helpers.py b/tests/robot/resources/test_harness_helpers.py index 00731477..df4c3811 100644 --- a/tests/robot/resources/test_harness_helpers.py +++ b/tests/robot/resources/test_harness_helpers.py @@ -937,6 +937,61 @@ def test_supported_capabilities_malformed_is_empty(): assert placement_status.supported_capabilities(non_list) == [] +# GET /1.0/placement body carrying an observed RGW member with a frontend, plus +# a stored policy whose rgw entry has been stripped/redacted (no key material). +_RGW_PLACEMENT_RESPONSE = json.dumps({ + "status_code": 200, + "metadata": { + "active": True, + "policy": { + "mode": "reconcile", + "members": { + "node-a": {"rgw": {"enabled": True, "port": 8080, "ssl_port": 443}}, + }, + }, + "observed": [ + {"member": "node-a", "rgw": True, + "rgw_frontend": {"port": 8080, "ssl_port": 443, "ssl": True}}, + {"member": "node-b", "control": True}, + ], + }, +}) + + +def test_member_rgw_frontend_reports_ports_and_tls(): + fe = placement_status.member_rgw_frontend(_RGW_PLACEMENT_RESPONSE, "node-a") + assert fe == {"port": 8080, "ssl_port": 443, "ssl": True} + + +def test_member_rgw_frontend_absent_member_is_empty(): + assert placement_status.member_rgw_frontend(_RGW_PLACEMENT_RESPONSE, "node-b") == {} + assert placement_status.member_rgw_frontend(_RGW_PLACEMENT_RESPONSE, "node-z") == {} + assert placement_status.member_rgw_frontend("garbage", "node-a") == {} + + +def test_placement_leaks_rgw_secrets_false_when_stripped(): + # The stored policy carries port/ssl_port but no cert/key: no leak. + assert placement_status.placement_leaks_rgw_secrets(_RGW_PLACEMENT_RESPONSE) is False + assert placement_status.placement_leaks_rgw_secrets("garbage") is False + + +def test_placement_leaks_rgw_secrets_true_when_present(): + leaky = json.dumps({ + "status_code": 200, + "metadata": {"policy": {"members": { + "node-a": {"rgw": {"enabled": True, "ssl_certificate": "Y2VydA=="}}, + }}}, + }) + assert placement_status.placement_leaks_rgw_secrets(leaky) is True + leaky_key = json.dumps({ + "status_code": 200, + "metadata": {"policy": {"members": { + "node-a": {"rgw": {"enabled": True, "ssl_private_key": "a2V5"}}, + }}}, + }) + assert placement_status.placement_leaks_rgw_secrets(leaky_key) is True + + def test_mon_count_prefers_monmap_num_mons(): raw = json.dumps({"monmap": {"num_mons": 3}, "quorum_names": ["a", "b"]}) assert placement_status.mon_count(raw) == 3 diff --git a/tests/robot/rgw-placement-tests/rgw_placement_tests.robot b/tests/robot/rgw-placement-tests/rgw_placement_tests.robot new file mode 100644 index 00000000..2420ccbb --- /dev/null +++ b/tests/robot/rgw-placement-tests/rgw_placement_tests.robot @@ -0,0 +1,108 @@ +*** Settings *** +Documentation rgw-placement-tests +... Functional coverage for the Option B RGW role-placement feature (CE142): +... the placement-rgw capability is advertised, a placement policy carrying an +... `rgw` object (enabled/port) enables RGW atomically, GET /placement reports +... the observed rgw_frontend (port/ssl) sourced from the rgw_frontends table, +... no SSL key material ever leaks into the stored policy, and a scale-to-zero +... `rgw:{enabled:false}` removes the RGW daemon. +... The suite mirrors single-system-tests setup (single outer VM + 3 loop OSDs so +... RGW zone pools can place PGs), then drives the placement API directly. +Resource ../resources/microceph_harness.resource +Suite Setup RGW Placement Suite Setup +Suite Teardown Teardown MicroCeph Environment +Test Tags single-node rgw placement lxd integration slow + +*** Keywords *** +RGW Placement Suite Setup + [Documentation] Launch a single VM, install the local snap, bootstrap, and add + ... 3 loop OSDs so the cluster is healthy enough for RGW pool creation. + Launch Outer Test VM vm_name=microceph-rgw-placement-vm + Copy Scripts To VM + Copy Snap To VM + Install Tools + Install And Bootstrap MicroCeph + Create Loop Devices + Run In VM And Check sudo microceph disk add /dev/sdia /dev/sdib /dev/sdic --wipe 300 + Wait For OSD Count 3 + +RGW Snap Service Is Not Active + [Documentation] Polls until the snap.microceph.rgw systemd unit is no longer active. + ... ceph -s keeps the rgw service-map line for a lag after the daemon stops, so + ... systemctl is-active is the reliable scale-to-zero signal (DisableRGW stops + ... the unit, confirmed by "Deactivated successfully" in the daemon log). + ${r}= Run In VM for i in $(seq 1 36); do [ "$(systemctl is-active snap.microceph.rgw.service 2>/dev/null)" != "active" ] && exit 0; sleep 5; done; systemctl is-active snap.microceph.rgw.service; exit 1 300 + Should Be Equal As Integers ${r.rc} 0 msg=RGW snap service still active after scale-to-zero + +Placement Has No RGW Frontend + [Documentation] Polls GET /placement until no observed rgw_frontend is reported + ... (the rgw_frontends DB row is deleted by the member disable, so the + ... observed frontend must drop away once the scale-to-zero completes). + FOR ${i} IN RANGE 36 + ${placement}= Get Placement Status JSON + ${present}= Run Keyword And Return Status Should Contain ${placement} "rgw_frontend" + IF not ${present} + Return From Keyword + END + Sleep 5s + END + ${placement}= Get Placement Status JSON + Should Not Contain ${placement} "rgw_frontend" msg=observed rgw_frontend still present after scale-to-zero: ${placement} + +*** Test Cases *** +Test Placement RGW Capability Advertised + [Documentation] The snap advertises the `placement-rgw` capability marker so a + ... charm can gate entry into role-managed RGW placement. + [Tags] placement + ${caps}= Get Supported Capabilities + Should Contain ${caps} placement-rgw msg=placement-rgw capability not advertised + +Test Placement Endpoint Reports Bootstrapped + [Documentation] GET /placement is reachable and reports bootstrap_state=bootstrapped on the + ... freshly-bootstrapped node (exercises the populateRGWFrontends read path with + ... no RGW members yet; a fresh cluster has no stored policy, so `active` is + ... false until a PUT stores one — tested after the enable PUT below). + [Tags] placement + ${placement}= Get Placement Status JSON + Should Contain ${placement} "bootstrap_state":"bootstrapped" msg=placement endpoint did not report bootstrapped: ${placement} + +Test Enable RGW Via Placement Object + [Documentation] PUT /1.0/placement with `rgw:{enabled:true,port:8080}` on the + ... local member atomically enables RGW on port 8080 (engine -> member dispatch + ... -> applyRGWFrontend render + start). The bare-bool form is rejected. + [Tags] rgw placement + ${hn}= Get VM Hostname + # Bare-bool rgw must be rejected (HTTP 400) by the Option B parser. + ${bad}= MicroCeph API Put placement {"mode":"reconcile","members":{"${hn}":{"rgw":true}}} timeout=60 + ${bad_code}= Response Status Code ${bad} + Run Keyword And Continue On Failure Should Not Be Equal As Integers ${bad_code} 200 msg=bare-bool rgw must be rejected, got ${bad} + # Object form enables RGW on port 8080. + ${resp}= MicroCeph API Put placement {"mode":"reconcile","members":{"${hn}":{"rgw":{"enabled":true,"port":8080}}}} timeout=300 + ${code}= Response Status Code ${resp} + Should Be Equal As Integers ${code} 200 msg=RGW placement PUT failed: ${resp} + Wait For RGW 1 + # The rendered frontend must carry port=8080 (applyRGWFrontend wrote it). + Run In VM And Check grep -q 'port=8080' /var/snap/microceph/current/conf/radosgw.conf 30 + +Test Placement Reports Observed RGW Frontend And No Secret Leak + [Documentation] GET /1.0/placement reports the observed rgw_frontend for the RGW + ... member with port=8080 and ssl=false (ssl_port omitted for plaintext), and the + ... stored policy carries no ssl_certificate / ssl_private_key material. + [Tags] rgw placement + ${placement}= Get Placement Status JSON + # Observed frontend: port 8080, ssl false, no ssl_port (plaintext -> ssl_port=0 -> omitempty). + Should Contain ${placement} "rgw_frontend":{"port":8080,"ssl":false} msg=observed rgw_frontend not reported as expected: ${placement} + # Defense in depth: no SSL key material anywhere in the placement body. + Should Not Contain ${placement} ssl_certificate msg=ssl_certificate leaked into placement body + Should Not Contain ${placement} ssl_private_key msg=ssl_private_key leaked into placement body + +Test Disable RGW Via Placement Scale To Zero + [Documentation] PUT /1.0/placement with `rgw:{enabled:false}` removes the RGW + ... daemon (no keep-one for RGW). The observed rgw_frontend must drop away. + [Tags] rgw placement + ${hn}= Get VM Hostname + ${resp}= MicroCeph API Put placement {"mode":"reconcile","members":{"${hn}":{"rgw":{"enabled":false}}}} timeout=300 + ${code}= Response Status Code ${resp} + Should Be Equal As Integers ${code} 200 msg=RGW scale-to-zero PUT failed: ${resp} + RGW Snap Service Is Not Active + Placement Has No RGW Frontend