From 4ce766a3a621677f3e720b18eebf1af58a60e7bc Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Sat, 4 Jul 2026 00:54:10 +0200 Subject: [PATCH 1/3] fix(rest): validate layer_stack on rg-modify + rd inherit (Bug 434) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The RD/RG create paths validate the layer stack (validateLayerStack: allowlist {DRBD,LUKS,STORAGE}, DRBD first, STORAGE terminal), but two sibling paths bypassed it: 1. `rg modify` (handleRGUpdate) gated only place_count, so an invalid select_filter.layer_stack was merged and persisted unvalidated. 2. handleRDCreate ran validateRDCreateBody (which validates an EXPLICIT layer_list) BEFORE inheritLayerStackFromRG, so an RD created against such an RG inherited the invalid stack unchecked. Net: an invalid, unmaterialisable layer stack the direct create path refuses (400) reached a persisted RD spec via the rg-modify -> inherit chain; on the stand the satellite can't materialise it (drbdadm/LVM error) -> spawn failure / hot-loop. Same validate-on-create, bypass-on-modify/inherit asymmetry class as the resize-bounds bypass. Fix: (1) extend the rg-update wire gate (rgUpdatePlaceCountGate -> rgUpdateWireGate) to reject an invalid layer_stack with the same 400 the create path returns, and (2) defense-in-depth re-validate the resolved (possibly inherited) stack in handleRDCreate after the RG inherit. Either gate alone closes the hole; both together also stop an already-invalid RG (from any prior path) from smuggling a bad stack into an RD. Regression (fail-on-bug): TestBug434* drive the real handler mux over an in-memory store — the rg-modify rejection and the rd-inherit rejection both FAIL on the pre-fix tree (accepted / persisted) and PASS with the gates. Healthy-path (valid re-order, absent field) still succeeds. Co-Authored-By: Claude Signed-off-by: Andrei Kvapil --- .../bug_434_rg_layer_stack_validation_test.go | 229 ++++++++++++++++++ pkg/rest/resource_definitions.go | 13 + pkg/rest/resource_groups.go | 32 ++- 3 files changed, 266 insertions(+), 8 deletions(-) create mode 100644 pkg/rest/bug_434_rg_layer_stack_validation_test.go diff --git a/pkg/rest/bug_434_rg_layer_stack_validation_test.go b/pkg/rest/bug_434_rg_layer_stack_validation_test.go new file mode 100644 index 00000000..7b63f353 --- /dev/null +++ b/pkg/rest/bug_434_rg_layer_stack_validation_test.go @@ -0,0 +1,229 @@ +// SPDX-License-Identifier: Apache-2.0 + +/* +Copyright 2026 Cozystack contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rest + +import ( + "bytes" + "encoding/json" + "net/http" + "testing" + + apiv1 "github.com/cozystack/blockstor/pkg/api/v1" + "github.com/cozystack/blockstor/pkg/store" +) + +// Bug 434 (P2 correctness+availability): the RD/RG CREATE paths validate +// the layer stack (validateLayerStack: allowlist {DRBD,LUKS,STORAGE}, +// DRBD first, STORAGE terminal), but two sibling paths bypassed it: +// +// 1. `rg modify` (handleRGUpdate) gated only place_count — an invalid +// select_filter.layer_stack was merged and persisted unvalidated. +// 2. handleRDCreate ran validateRDCreateBody (which validates an +// EXPLICIT layer_list) BEFORE inheritLayerStackFromRG, so an RD +// created against such an RG inherited the invalid stack unchecked. +// +// Net: an invalid, unmaterialisable layer stack the DIRECT create path +// refuses (400) reached a persisted RD spec via the rg-modify → inherit +// chain. Same asymmetry class as the resize-bounds bypass (create +// validates; a sibling modify/inherit path bypasses). +// +// Fix: (1) validate the layer stack in the rg-update wire gate, and +// (2) defense-in-depth re-validate the resolved (possibly inherited) +// stack in handleRDCreate after the RG inherit. +// +// These FAIL on the pre-fix tree (the invalid stack is accepted / +// inherited) and PASS with the two gates. + +// bug434InvalidStack is STORAGE-before-DRBD: STORAGE must be terminal and +// DRBD must be first, so this is refused by the create path. +var bug434InvalidStack = []string{"STORAGE", "DRBD"} //nolint:gochecknoglobals // shared test fixture + +// TestBug434RGUpdateRefusesInvalidLayerStack — FAIL-on-bug (Fix 1). +// +// `rg modify` with an invalid select_filter.layer_stack must be rejected +// with the same 400 the create path returns, and the stored RG must keep +// its valid stack (the gate runs BEFORE PatchResourceGroup). +func TestBug434RGUpdateRefusesInvalidLayerStack(t *testing.T) { + t.Parallel() + + st := store.NewInMemory() + ctx := t.Context() + + if err := st.ResourceGroups().Create(ctx, &apiv1.ResourceGroup{ + Name: "rg-b434", + SelectFilter: apiv1.AutoSelectFilter{LayerStack: []string{"DRBD", "STORAGE"}}, + }); err != nil { + t.Fatalf("seed RG: %v", err) + } + + base, stop := startServerWithStore(t, st) + defer stop() + + body, _ := json.Marshal(map[string]any{ + "select_filter": map[string]any{"layer_stack": bug434InvalidStack}, + }) + + req, _ := http.NewRequestWithContext(ctx, http.MethodPut, base+"/v1/resource-groups/rg-b434", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("PUT: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusBadRequest { + got, _ := readAllBody(resp) + t.Fatalf("status: got %d, want 400 (Bug 434: rg modify must refuse an invalid layer_stack). Body: %s", + resp.StatusCode, got) + } + + // The rejected update must not reach the store: the RG keeps its + // valid stack. + stored, err := st.ResourceGroups().Get(ctx, "rg-b434") + if err != nil { + t.Fatalf("re-fetch RG: %v", err) + } + + if got := stored.SelectFilter.LayerStack; len(got) != 2 || got[0] != "DRBD" || got[1] != "STORAGE" { + t.Errorf("stored LayerStack changed after rejection: got %v, want [DRBD STORAGE]", got) + } +} + +// TestBug434RGUpdateValidLayerStackAccepted pins the healthy path: a +// valid re-order still lands. +func TestBug434RGUpdateValidLayerStackAccepted(t *testing.T) { + t.Parallel() + + st := store.NewInMemory() + ctx := t.Context() + + if err := st.ResourceGroups().Create(ctx, &apiv1.ResourceGroup{ + Name: "rg-b434-ok", + SelectFilter: apiv1.AutoSelectFilter{LayerStack: []string{"DRBD", "STORAGE"}}, + }); err != nil { + t.Fatalf("seed RG: %v", err) + } + + base, stop := startServerWithStore(t, st) + defer stop() + + body, _ := json.Marshal(map[string]any{ + "select_filter": map[string]any{"layer_stack": []string{"DRBD", "LUKS", "STORAGE"}}, + }) + + req, _ := http.NewRequestWithContext(ctx, http.MethodPut, base+"/v1/resource-groups/rg-b434-ok", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("PUT: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + got, _ := readAllBody(resp) + t.Fatalf("status: got %d, want 200 (valid layer_stack must be accepted). Body: %s", + resp.StatusCode, got) + } +} + +// TestBug434RGUpdateAbsentLayerStackUntouched pins "field absent = leave +// alone": a PUT that doesn't mention layer_stack must NOT fire the gate. +func TestBug434RGUpdateAbsentLayerStackUntouched(t *testing.T) { + t.Parallel() + + st := store.NewInMemory() + ctx := t.Context() + + if err := st.ResourceGroups().Create(ctx, &apiv1.ResourceGroup{ + Name: "rg-b434-absent", + SelectFilter: apiv1.AutoSelectFilter{LayerStack: []string{"DRBD", "STORAGE"}}, + }); err != nil { + t.Fatalf("seed RG: %v", err) + } + + base, stop := startServerWithStore(t, st) + defer stop() + + body, _ := json.Marshal(map[string]any{"description": "operator note"}) + + req, _ := http.NewRequestWithContext(ctx, http.MethodPut, base+"/v1/resource-groups/rg-b434-absent", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("PUT: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + got, _ := readAllBody(resp) + t.Fatalf("status: got %d, want 200 (absent layer_stack must NOT fire the gate). Body: %s", + resp.StatusCode, got) + } +} + +// TestBug434RDCreateRefusesInheritedInvalidLayerStack — FAIL-on-bug (Fix 2). +// +// Even if an RG already carries an invalid layer stack (seeded directly +// here to model an RG that predates the rg-modify gate, or was persisted +// by any other path), an RD created against it — inheriting the stack +// with no explicit layer_list — must be refused, and no RD may persist. +// handleRDCreate validates the EXPLICIT body before the RG inherit, so +// without the post-inherit re-validation the invalid stack reaches the RD. +func TestBug434RDCreateRefusesInheritedInvalidLayerStack(t *testing.T) { + t.Parallel() + + st := store.NewInMemory() + ctx := t.Context() + + // Seed an RG that ALREADY holds the invalid stack (store Create does + // not validate — validation lives in the REST handlers). + if err := st.ResourceGroups().Create(ctx, &apiv1.ResourceGroup{ + Name: "rg-b434-bad", + SelectFilter: apiv1.AutoSelectFilter{LayerStack: bug434InvalidStack}, + }); err != nil { + t.Fatalf("seed bad RG: %v", err) + } + + base, stop := startServerWithStore(t, st) + defer stop() + + body, _ := json.Marshal(map[string]any{ + "resource_definition": map[string]any{ + "name": "rd-b434-inherit", + "resource_group_name": "rg-b434-bad", + }, + }) + + resp := httpPost(t, base+"/v1/resource-definitions", body) + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusBadRequest { + got, _ := readAllBody(resp) + t.Fatalf("status: got %d, want 400 (Bug 434: RD must not inherit an invalid layer stack). Body: %s", + resp.StatusCode, got) + } + + // No RD may persist with the invalid inherited stack. + if _, err := st.ResourceDefinitions().Get(ctx, "rd-b434-inherit"); err == nil { + t.Errorf("RD rd-b434-inherit persisted after rejection — must not (invalid inherited layer stack)") + } +} diff --git a/pkg/rest/resource_definitions.go b/pkg/rest/resource_definitions.go index 0ff2931f..df7c5c57 100644 --- a/pkg/rest/resource_definitions.go +++ b/pkg/rest/resource_definitions.go @@ -310,6 +310,19 @@ func (s *Server) handleRDCreate(w http.ResponseWriter, r *http.Request) { return } + // Bug 434 (defense-in-depth): validateRDCreateBody validates an + // EXPLICIT layer_list, but it runs BEFORE the RG inherit above — so an + // invalid stack inherited from an RG (e.g. one that predates the + // rg-modify validation gate) would reach the store unchecked. Re-validate + // the resolved (possibly inherited) stack here, mirroring the create + // path's 400, so an unmaterialisable layer chain never persists onto an RD. + lsErr := validateLayerStack(rd.LayerStack) + if lsErr != nil { + writeError(w, http.StatusBadRequest, lsErr.Error()) + + return + } + // Bug 262 (P2): stand-caught — `linstor rd lp | grep // quorum` reported `DrbdOptions/Resource/quorum off` on freshly- // created RDs because the RG-create surface never seeded the diff --git a/pkg/rest/resource_groups.go b/pkg/rest/resource_groups.go index d28f6640..b7e47692 100644 --- a/pkg/rest/resource_groups.go +++ b/pkg/rest/resource_groups.go @@ -215,11 +215,12 @@ func (s *Server) handleRGUpdate(w http.ResponseWriter, r *http.Request) { // PUT's contribution. var rebalanceScheduled bool - // Bug 367 / 361: refuse negative or absurdly-large place_count - // on the patch body BEFORE we hand it to PatchResourceGroup. See - // rgUpdatePlaceCountGate — keeps the wire-validation rule out of - // the store error path and the handler under the funlen budget. - gateErr := rgUpdatePlaceCountGate(raw, &patch) + // Bug 367 / 361 + Bug 434: refuse a negative/absurd place_count OR an + // invalid select_filter.layer_stack on the patch body BEFORE we hand + // it to PatchResourceGroup. See rgUpdateWireGate — keeps the + // wire-validation rules out of the store error path and the handler + // under the funlen budget. + gateErr := rgUpdateWireGate(raw, &patch) if gateErr != nil { writeError(w, http.StatusBadRequest, gateErr.Error()) @@ -895,8 +896,9 @@ func rgDeleteRefusedMessage(name string, count int) string { // allocates against an absurd target. const rgPlaceCountSanityCeiling = 1_000_000 -// rgUpdatePlaceCountGate runs the Bug 367 / 361 wire-validation -// rule on the PUT patch body BEFORE PatchResourceGroup sees it. +// rgUpdateWireGate runs the PUT patch body's wire-validation rules +// (Bug 367 / 361 place_count + Bug 434 layer_stack) BEFORE +// PatchResourceGroup sees it. // // Extracted from handleRGUpdate to keep the handler under the funlen // budget; the split tracks the natural "validate → patch → persist" @@ -910,7 +912,7 @@ const rgPlaceCountSanityCeiling = 1_000_000 // validator's actionable message; routing the error through the // PatchResourceGroup callback would demote it to a 500 (writeStoreError // has no band for "bad request"). -func rgUpdatePlaceCountGate(raw []byte, patch *apiv1.ResourceGroup) error { +func rgUpdateWireGate(raw []byte, patch *apiv1.ResourceGroup) error { mentioned := rgSelectFilterKeys(raw) if _, ok := mentioned["place_count"]; ok { err := validateRGSelectFilterPlaceCount(patch.SelectFilter.PlaceCount, 0) @@ -926,6 +928,20 @@ func rgUpdatePlaceCountGate(raw []byte, patch *apiv1.ResourceGroup) error { } } + // Bug 434: `rg modify` must reject an invalid select_filter.layer_stack + // with the same 400 the create path (handleRGCreate) returns, instead + // of storing it unvalidated for a child RD to inherit + // (handleRDCreate → inheritLayerStackFromRG). Validate exactly the + // value mergeRGSelectFilterLists would persist (patch.LayerStack != + // nil); a nil/empty stack is the legitimate "clear to default" and + // validateLayerStack accepts it. + if patch.SelectFilter.LayerStack != nil { + err := validateLayerStack(patch.SelectFilter.LayerStack) + if err != nil { + return err + } + } + return nil } From f58c9c590073321fa4474229bf96a4353a5a349d Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Sat, 4 Jul 2026 00:54:21 +0200 Subject: [PATCH 2/3] test(integration): rd must not inherit an invalid rg layer_stack (Bug 434) End-to-end regression driving the whole REST surface (rg create -> rg modify -> rd create inherit). Fix-agnostic: passes whether the rg-modify gate rejects the invalid stack or the rd-create post-inherit re-validation refuses the RD. On the pre-fix tree the RD is persisted with the invalid [STORAGE,DRBD] stack inherited from the modified RG -> FAIL; with the fix no persisted RD carries the invalid ordering. Co-Authored-By: Claude Signed-off-by: Andrei Kvapil --- .../group_blue_rg_layer_bug434_test.go | 192 ++++++++++++++++++ 1 file changed, 192 insertions(+) create mode 100644 tests/integration/group_blue_rg_layer_bug434_test.go diff --git a/tests/integration/group_blue_rg_layer_bug434_test.go b/tests/integration/group_blue_rg_layer_bug434_test.go new file mode 100644 index 00000000..f7ee24c1 --- /dev/null +++ b/tests/integration/group_blue_rg_layer_bug434_test.go @@ -0,0 +1,192 @@ +// SPDX-License-Identifier: Apache-2.0 + +//go:build integration + +/* +Copyright 2026 Cozystack contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Bug 434 — end-to-end regression for the layer-stack validation +// asymmetry. +// +// The RD/RG CREATE paths validate the layer stack (validateLayerStack: +// allowlist {DRBD,LUKS,STORAGE}, DRBD first, STORAGE terminal). But +// `rg modify` (handleRGUpdate) gated only place_count, so an invalid +// select_filter.layer_stack was merged and persisted unvalidated; and +// handleRDCreate ran validateRDCreateBody BEFORE inheritLayerStackFromRG, +// so an RD created against that RG inherited the invalid stack unchecked. +// Net: an invalid, unmaterialisable layer stack the DIRECT create path +// refuses (400) reached a persisted RD spec via the rg-modify → inherit +// chain. +// +// This drives the whole REST surface (rg create → rg modify → rd create +// inherit) and is fix-agnostic: it passes whether the RG-modify gate +// rejects the invalid stack OR the RD-create post-inherit re-validation +// refuses the RD. On the pre-fix tree the RD is persisted with the +// invalid [STORAGE,DRBD] stack → FAIL. +package integration + +import ( + "bytes" + "context" + "encoding/json" + "io" + "net/http" + "testing" + "time" + + "k8s.io/apimachinery/pkg/types" + + blockstoriov1alpha1 "github.com/cozystack/blockstor/api/v1alpha1" + "github.com/cozystack/blockstor/tests/integration/harness" +) + +// b434HTTP issues an HTTP request with a JSON body and returns +// (status, body). +func b434HTTP(t *testing.T, method, url string, payload any) (int, []byte) { + t.Helper() + + raw, err := json.Marshal(payload) + if err != nil { + t.Fatalf("marshal: %v", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, method, url, bytes.NewReader(raw)) + if err != nil { + t.Fatalf("build %s: %v", method, err) + } + + req.Header.Set("Content-Type", "application/json") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("%s %s: %v", method, url, err) + } + defer func() { _ = resp.Body.Close() }() + + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("read body: %v", err) + } + + return resp.StatusCode, body +} + +// b434LayerStackInvalidOrder reports whether stack has STORAGE appearing +// before DRBD — the specific invalid ordering this test injects (STORAGE +// must be terminal, DRBD must be first). Mirrors the create-path rejection. +func b434LayerStackInvalidOrder(stack []string) bool { + storageIdx, drbdIdx := -1, -1 + + for i, l := range stack { + switch l { + case "STORAGE": + if storageIdx == -1 { + storageIdx = i + } + case "DRBD": + if drbdIdx == -1 { + drbdIdx = i + } + } + } + + return storageIdx != -1 && drbdIdx != -1 && storageIdx < drbdIdx +} + +// TestBug434RGLayerModifyNoInvalidRDInherit — FAIL-on-bug regression. +// +// An invalid layer stack the DIRECT rd-create path refuses (400) must not +// reach a persisted RD via the `rg modify` (unvalidated) → inherit chain. +// Fix-agnostic: it passes whether Blue validates the RG modify (rg stays +// valid, RD inherits a valid stack) OR re-validates the RD after inherit +// (RD create is refused). On the pre-fix tree the RD is persisted with the +// invalid [STORAGE,DRBD] stack → FAIL. +func TestBug434RGLayerModifyNoInvalidRDInherit(t *testing.T) { + stack := harness.StartStack(t) + harness.SeedThreeNodeCluster(t, stack) + + const ( + rgName = "b434-rglayer-inherit" + rdName = "b434-rd-layer-inherit" + ) + + invalid := []string{"STORAGE", "DRBD"} + + // Control: the DIRECT create path MUST refuse this stack. If it ever + // starts accepting it, the whole premise (and the fix) is moot. + dst, dbody := b434HTTP(t, http.MethodPost, stack.RestURL+"/v1/resource-definitions", map[string]any{ + "resource_definition": map[string]any{"name": "b434-rd-layer-control"}, + "layer_list": invalid, + }) + if dst >= 200 && dst < 300 { + t.Fatalf("premise broken: direct rd-create ACCEPTED the invalid layer stack %v (status=%d body=%s)", invalid, dst, string(dbody)) + } + + // 1. Create an RG with a VALID layer stack. + if s, b := b434HTTP(t, http.MethodPost, stack.RestURL+"/v1/resource-groups", map[string]any{ + "name": rgName, + "select_filter": map[string]any{"layer_stack": []string{"DRBD", "STORAGE"}}, + }); s < 200 || s >= 300 { + t.Fatalf("RG create: status=%d body=%s", s, string(b)) + } + + // 2. `rg modify` with the INVALID layer stack (retry past seed cache-lag). + rgURL := stack.RestURL + "/v1/resource-groups/" + rgName + + deadline := time.Now().Add(15 * time.Second) + for time.Now().Before(deadline) { + s, _ := b434HTTP(t, http.MethodPut, rgURL, map[string]any{ + "select_filter": map[string]any{"layer_stack": invalid}, + }) + if s != http.StatusNotFound { + break + } + + time.Sleep(200 * time.Millisecond) + } + + // Let the RG (whatever its final stored stack) become cache-visible so + // the RD-create inherit lookup observes it. + time.Sleep(1500 * time.Millisecond) + + // 3. Create an RD inheriting from the RG (no explicit layer_list). + b434HTTP(t, http.MethodPost, stack.RestURL+"/v1/resource-definitions", map[string]any{ + "resource_definition": map[string]any{ + "name": rdName, + "resource_group_name": rgName, + }, + }) + + // 4. Invariant: no persisted RD may carry the invalid layer ordering. + var rd blockstoriov1alpha1.ResourceDefinition + + err := stack.Env.Client.Get(context.Background(), types.NamespacedName{Name: rdName}, &rd) + if err != nil { + return // RD not persisted (create refused) — invariant holds. + } + + if b434LayerStackInvalidOrder(rd.Spec.LayerStack) { + t.Fatalf("RD %q persisted with an INVALID layer stack %v inherited from a modified RG. "+ + "The direct rd-create path refuses this exact stack (STORAGE must be terminal, DRBD first), "+ + "but pre-fix `rg modify` stored select_filter.layer_stack unvalidated and handleRDCreate "+ + "validated BEFORE inheritLayerStackFromRG — so the invalid stack reached the RD spec. On the "+ + "stand this yields an unmaterialisable layer chain.", + rdName, rd.Spec.LayerStack) + } +} From 33e0f094d83c12117f38b390c391e6ce68098aba Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Sat, 4 Jul 2026 00:57:53 +0200 Subject: [PATCH 3/3] test(harness): L6 cli-matrix + L7 replay for rg-modify layer reject (Bug 434) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the CLI-bug-fix protocol, author the operator-CLI codification for the `rg modify` layer-stack gate: - rg-modify-invalid-layer-rejected.sh (L6): rg c --layer-list drbd,storage → `rg modify --layer-list storage,drbd` MUST be rejected → the RG keeps its valid stack → an RD inheriting from it gets the valid [DRBD,STORAGE], never [STORAGE,DRBD] (read off the RD/RG CRD spec). - rg-modify-invalid-layer-rejected.yaml (L7): the same operator sequence, codified via exit-code contract (the runner has no layer-stack await kind; the RD-inherit invariant is asserted at the L6 layer, mirroring rg-delete-with-rds-rejected). On-stand run is DEFERRED (the DRBD stand is unavailable this session); the rest-handler unit and the envtest integration are the deterministic fail-on-bug proof. Co-Authored-By: Claude Signed-off-by: Andrei Kvapil --- tests/e2e/cli-matrix/README.md | 1 + .../rg-modify-invalid-layer-rejected.sh | 107 ++++++++++++++++++ .../rg-modify-invalid-layer-rejected.yaml | 51 +++++++++ 3 files changed, 159 insertions(+) create mode 100755 tests/e2e/cli-matrix/rg-modify-invalid-layer-rejected.sh create mode 100644 tests/operator-harness/replay/rg-modify-invalid-layer-rejected.yaml diff --git a/tests/e2e/cli-matrix/README.md b/tests/e2e/cli-matrix/README.md index b07682b8..b55fb856 100644 --- a/tests/e2e/cli-matrix/README.md +++ b/tests/e2e/cli-matrix/README.md @@ -44,6 +44,7 @@ A cell counts as **FAIL** if either leg times out, the `linstor` CLI exits non-z | `rd-clone-vd-data-plane.sh` | 020 | `linstor rd clone ` on a VD-bearing source (plain CLI body, no `use_zfs_clone`) AND the raw-REST `use_zfs_clone=true` body linstor-csi sends both materialise a real clone: 2 replicas UpToDate, marker bytes from the source present on EVERY clone replica (promote each in turn), clone status COMPLETE, internal `clone-` snapshot visible on the source. Pre-fix: 400 on `use_zfs_clone`, 501 on VD-bearing sources (Bug 114 gate). | | `encryption-passphrase-luks-rd.sh` | 023 | Secret-only LUKS flow: `linstor encryption create-passphrase` alone (legacy `DrbdOptions/EncryptPassphrase` controller prop asserted ABSENT throughout) unlocks `rd c -l drbd,luks,storage` + autoplace to UpToDate, and the Secret-backed passphrase actually opens the LUKS header on each replica's backing device. Requires the Bug-023 fix (PR #143); pre-fix the rd-create is rejected with "LUKS layer requires DrbdOptions/EncryptPassphrase to be set first". | | `vd-modify-preserves-drbd-minor.sh` | 433 | A legal in-bounds VD-scoped modify (`vd set-size` grow / `vd set-property`) must NOT change the per-volume DRBDMinor — the /dev/drbd device identity. Creates rd-a (lowest minor Ma) + rd-b (next minor Mb), frees Ma by deleting rd-a, then modifies rd-b: its DRBDMinor AND device path must stay Mb over a settle window. Pre-fix `wireToCRDVD` dropped the minor on the VD-scoped write-back and the allocator re-stamped the freed lower Ma — a permanent device-identity change on a live resized volume. | +| `rg-modify-invalid-layer-rejected.sh` | 434 | `rg modify --layer-list storage,drbd` (STORAGE-before-DRBD, the ordering the create path refuses) must be rejected, the RG must keep its valid `[DRBD,STORAGE]` stack, and an RD inheriting from the RG must get the valid stack — never `[STORAGE,DRBD]`. Pre-fix `rg modify` gated only place_count and `handleRDCreate` validated the explicit layer_list BEFORE the RG inherit, so an invalid stack reached a persisted RD via the modify→inherit chain. | ## Running diff --git a/tests/e2e/cli-matrix/rg-modify-invalid-layer-rejected.sh b/tests/e2e/cli-matrix/rg-modify-invalid-layer-rejected.sh new file mode 100755 index 00000000..7fac24b3 --- /dev/null +++ b/tests/e2e/cli-matrix/rg-modify-invalid-layer-rejected.sh @@ -0,0 +1,107 @@ +#!/usr/bin/env bash +# +# usage: rg-modify-invalid-layer-rejected.sh WORK_DIR +# +# L6 cli-matrix cell — Bug 434: `rg modify` must reject an invalid +# select_filter.layer_stack the create path already refuses, and no RD +# created against the RG may inherit an invalid stack. +# +# Pre-fix, handleRGUpdate gated only place_count, so an invalid +# layer_stack was merged + persisted unvalidated; and handleRDCreate +# validated an EXPLICIT layer_list BEFORE inheritLayerStackFromRG, so an +# RD created against such an RG inherited the invalid stack unchecked. On +# the stand the satellite/dispatcher cannot materialise an out-of-order +# layer chain (drbdadm / LVM config error) → spawn failure / hot-loop. +# +# Sequence (STORAGE-before-DRBD is the invalid ordering — STORAGE must be +# terminal, DRBD must be first): +# 1. rg c --layer-list drbd,storage (valid, accepted) +# 2. rg m --layer-list storage,drbd MUST be rejected +# 3. the RG keeps its valid stack (the reject never persisted) +# 4. rd c --resource-group (inherit) → RD stack is the valid +# [DRBD,STORAGE], never [STORAGE,DRBD] + +set -euo pipefail + +WORK_DIR=${1:?work_dir required} +export KUBECONFIG="$WORK_DIR/kubeconfig" + +SCRIPT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=lib.sh +source "$SCRIPT_DIR/lib.sh" + +require_workers 1 + +linstor_cli_setup + +RG=cli-b434-rg +RD=cli-b434-rd + +cleanup() { + delete_rd "$RD" 2>/dev/null || true + "${LCTL[@]}" resource-group delete "$RG" >/dev/null 2>&1 || true + linstor_cli_teardown +} +trap cleanup EXIT + +# rg_layer_stack / rd_layer_stack render the stored layer stack as a +# comma-joined string (empty when unset). +rg_layer_stack() { + kubectl get resourcegroup "$1" -o json 2>/dev/null \ + | jq -r '.spec.selectFilter.layerStack // [] | join(",")' 2>/dev/null || echo "" +} +rd_layer_stack() { + kubectl get resourcedefinition "$1" -o json 2>/dev/null \ + | jq -r '.spec.layerStack // [] | join(",")' 2>/dev/null || echo "" +} + +# ---- 1. RG with a VALID layer stack --------------------------------------- +echo ">> rg c $RG --layer-list drbd,storage" +_out=$("${LCTL[@]}" resource-group create "$RG" --layer-list drbd,storage 2>&1) \ + || { echo "FAIL: rg c $RG: $_out" >&2; exit 1; } + +# ---- 2. rg modify to an INVALID stack MUST be rejected -------------------- +echo ">> rg m $RG --layer-list storage,drbd (must be rejected)" +out=$(mktemp) +set +e +"${LCTL[@]}" resource-group modify "$RG" --layer-list storage,drbd >"$out" 2>&1 +rc=$? +set -e +if (( rc == 0 )); then + echo "FAIL (Bug 434): rg modify ACCEPTED an invalid layer stack storage,drbd" >&2 + cat "$out" >&2; rm -f "$out"; exit 1 +fi +if ! grep -qiE 'invalid layer order|layer' "$out"; then + echo "FAIL (Bug 434): rg-modify rejection must mention the layer fault; got:" >&2 + cat "$out" >&2; rm -f "$out"; exit 1 +fi +rm -f "$out" +echo ">> rg modify rejected OK" + +# ---- 3. the rejected modify must NOT have persisted ----------------------- +stored=$(rg_layer_stack "$RG") +if [[ "$stored" != "DRBD,STORAGE" ]]; then + echo "FAIL (Bug 434): RG $RG layer stack changed after a REJECTED modify: got '$stored', want 'DRBD,STORAGE'" >&2 + exit 1 +fi +echo ">> RG stack still 'DRBD,STORAGE' OK" + +# ---- 4. an RD inheriting from the RG gets the VALID stack ------------------ +echo ">> rd c $RD --resource-group $RG (inherit)" +_out=$("${LCTL[@]}" resource-definition create "$RD" --resource-group "$RG" 2>&1) \ + || { echo "FAIL: rd c $RD inherit: $_out" >&2; exit 1; } + +rd_stack="" +deadline=$(( $(date +%s) + 30 )) +while (( $(date +%s) < deadline )); do + rd_stack=$(rd_layer_stack "$RD") + [[ -n "$rd_stack" ]] && break + sleep 2 +done +if [[ "$rd_stack" == "STORAGE,DRBD" ]]; then + echo "FAIL (Bug 434): RD $RD inherited the INVALID layer stack [$rd_stack]" >&2 + exit 1 +fi +echo ">> RD inherited stack '[$rd_stack]' (not STORAGE,DRBD) OK" + +echo ">> rg-modify-invalid-layer-rejected (Bug 434) OK" diff --git a/tests/operator-harness/replay/rg-modify-invalid-layer-rejected.yaml b/tests/operator-harness/replay/rg-modify-invalid-layer-rejected.yaml new file mode 100644 index 00000000..cf981c35 --- /dev/null +++ b/tests/operator-harness/replay/rg-modify-invalid-layer-rejected.yaml @@ -0,0 +1,51 @@ +name: rg-modify-invalid-layer-rejected +description: | + Bug 434 (P2 correctness+availability) — `rg modify` must reject an + invalid select_filter.layer_stack the create path already refuses, + instead of storing it unvalidated for a child RD to inherit. + + Pre-fix, handleRGUpdate gated only place_count (so the invalid stack was + merged + persisted), and handleRDCreate validated an EXPLICIT layer_list + BEFORE inheritLayerStackFromRG (so an RD created against the RG inherited + the invalid stack unchecked). On the stand the satellite/dispatcher can't + materialise an out-of-order layer chain (drbdadm / LVM config error). + + Codified purely via exit-code contracts (the runner has no layer-stack + await kind; the RD-inherit invariant — the RD gets the VALID stack, never + [STORAGE,DRBD] — is asserted at the L6 layer in + tests/e2e/cli-matrix/rg-modify-invalid-layer-rejected.sh, which reads the + RD's spec.layerStack). Sequence: rg c --layer-list drbd,storage → + `rg m --layer-list storage,drbd` MUST be rejected → rd c inheriting + from the (still-valid) rg succeeds. + +prerequisites: + min_nodes: 1 + storage_pool: stand + +vars: + sp: stand + rd: b434-inherit-rd + +steps: + - name: create-rg-valid-layer + cmd: ["resource-group", "create", "b434-rg", "--layer-list", "drbd,storage"] + expect_exit: 0 + # ---- Contract: rg modify to STORAGE-before-DRBD is refused ---- + - name: rg-modify-invalid-layer-rejected + cmd: ["resource-group", "modify", "b434-rg", "--layer-list", "storage,drbd"] + expect_exit: [1, 10] + # The RG survived the refused modify and still resolves. + - name: rg-still-present + cmd: ["resource-group", "list", "--resource-groups", "b434-rg"] + expect_exit: 0 + # ---- An RD inheriting from the (still-valid) RG is accepted ---- + - name: create-rd-inherits-valid + cmd: ["resource-definition", "create", "{{rd}}", "--resource-group", "b434-rg"] + expect_exit: 0 + +teardown: + - cmd: ["resource-definition", "delete", "{{rd}}"] + - cmd: ["resource-group", "delete", "b434-rg"] + +invariants: + - no_orphans