fix(rest): refuse props edits on create doors, honour definitions_only, drop tracker numbers

cmd/controller/main.go

@@ -71,7 +71,7 @@ func main() {
 	var controllerNamespace string
 	var tlsOpts []func(*tls.Config)
 	flag.BoolVar(&enableRestAPI, "enable-rest-api", false,
-		"Mount the LINSTOR-compatible REST API alongside the reconcilers. Default is OFF — the apiserver runs as a separate Deployment (cmd/apiserver) since Phase 11.x. Set to true only for the legacy single-binary deployment.")
+		"Mount the LINSTOR-compatible REST API alongside the reconcilers. Default is off: the REST API runs as the separate apiserver Deployment (cmd/apiserver). Set it only for a single-binary deployment.")
 	flag.StringVar(&restAddr, "rest-bind-address", defaultPlainBindAddress,
 		"The address the plain-HTTP LINSTOR-compatible REST API binds to. It has no authentication, so the default is loopback, "+
 			"reachable through `kubectl port-forward` only. Ignored when --enable-rest-api=false.")

pkg/api/v1/resource_definition.go

@@ -224,9 +224,9 @@ type ResourceDefinitionCreate struct {
 	// bypasses the LUKS-prereq gate.
 	LayerList []string `json:"layer_list,omitempty"`
 
-	// OverrideProps mirrors GenericPropsModify — golinstor stuffs RG
-	// spawn defaults into this when creating an RD via spawn, so the
-	// REST handler must accept it even if we don't yet diff the values.
+	// The props-modify fields are declared so the decoder can name them;
+	// a non-empty one is refused, since a create has no props to modify.
+	// Props for a new definition go in resource_definition.props.
 	OverrideProps   map[string]string `json:"override_props,omitempty"`
 	DeleteProps     []string          `json:"delete_props,omitempty"`
 	DeleteNamespace []string          `json:"delete_namespaces,omitempty"`

pkg/api/v1/resource_group.go

@@ -97,10 +97,12 @@ type AutoSelectFilter struct {
 	OverrideVlmID           string           `json:"override_vlm_id,omitempty"`
 }
 
-// ResourceGroupSpawn is the payload for POST /resource-groups/{rg}/spawn —
-// the call linstor-csi uses to actually create a Resource from a group.
-// Upstream LINSTOR reuses GenericPropsModify here too, so we accept the
-// override_props / delete_props fields even though we don't consume them.
+// ResourceGroupSpawn is the payload for POST /resource-groups/{rg}/spawn,
+// behind `linstor rg spawn`: it creates a definition, its volumes and,
+// unless definitions_only is set, its replicas from a group.
+// The props-modify fields are declared so the decoder can name them, and a
+// non-empty one is refused: a spawn creates the definition, there are no
+// props on it to modify yet.
 type ResourceGroupSpawn struct {
 	ResourceDefinitionName     string            `json:"resource_definition_name,omitempty"`
 	ResourceDefinitionExternal string            `json:"resource_definition_external_name,omitempty"`

pkg/rest/autoplace.go

@@ -1707,7 +1707,7 @@ func (s *Server) refuseResourceCreateOnSPDeletedRace(w http.ResponseWriter, r *h
 
 		writeError(w, http.StatusNotFound,
 			"storage pool '"+pool+"' was deleted on node '"+res.NodeName+
-				"' concurrently with the resource create (Bug 145): "+
+				"' concurrently with the resource create: "+
 				"retry after re-creating the pool, or pick a different pool")
 
 		return false
@@ -1745,7 +1745,7 @@ func (s *Server) refuseResourceCreateOnNodeDeletedRace(w http.ResponseWriter, r
 
 		writeError(w, http.StatusNotFound,
 			"node '"+res.NodeName+"' was deleted concurrently with "+
-				"the resource create (Bug 174): retry after re-creating "+
+				"the resource create: retry after re-creating "+
 				"the node, or pick a different node")
 
 		return false

pkg/rest/create_doors_props_modify_test.go

@@ -0,0 +1,177 @@
+// 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 (
+	"encoding/json"
+	"errors"
+	"io"
+	"net/http"
+	"strings"
+	"testing"
+
+	apiv1 "github.com/cozystack/blockstor/pkg/api/v1"
+	"github.com/cozystack/blockstor/pkg/store"
+)
+
+// The props-modify fields edit props a definition already has. On a door
+// that creates the definition there is nothing to edit, and answering
+// success while dropping them claimed an edit that never happened.
+func propsModifyCases() map[string]func(override *map[string]string, del, ns *[]string) {
+	return map[string]func(override *map[string]string, del, ns *[]string){
+		"override_props":    func(o *map[string]string, _, _ *[]string) { *o = map[string]string{"Aux/k": "v"} },
+		"delete_props":      func(_ *map[string]string, d, _ *[]string) { *d = []string{"Aux/k"} },
+		"delete_namespaces": func(_ *map[string]string, _, n *[]string) { *n = []string{"Aux"} },
+	}
+}
+
+func TestRDCreateRefusesPropsModifyFields(t *testing.T) {
+	t.Parallel()
+
+	for field, set := range propsModifyCases() {
+		t.Run(field, func(t *testing.T) {
+			t.Parallel()
+
+			st := store.NewInMemory()
+			base, stop := startServerWithStore(t, st)
+			defer stop()
+
+			body := apiv1.ResourceDefinitionCreate{
+				ResourceDefinition: apiv1.ResourceDefinition{Name: "rd-modify"},
+			}
+			set(&body.OverrideProps, &body.DeleteProps, &body.DeleteNamespace)
+
+			assertRefusedNamingField(t, base+"/v1/resource-definitions", body, field)
+
+			_, err := st.ResourceDefinitions().Get(t.Context(), "rd-modify")
+			if !errors.Is(err, store.ErrNotFound) {
+				t.Fatalf("a refused create left the definition behind: err=%v", err)
+			}
+		})
+	}
+}
+
+func TestSpawnRefusesPropsModifyFields(t *testing.T) {
+	t.Parallel()
+
+	for field, set := range propsModifyCases() {
+		t.Run(field, func(t *testing.T) {
+			t.Parallel()
+
+			st := store.NewInMemory()
+			if err := st.ResourceGroups().Create(t.Context(), &apiv1.ResourceGroup{Name: "rg-modify"}); err != nil {
+				t.Fatalf("seed group: %v", err)
+			}
+
+			base, stop := startServerWithStore(t, st)
+			defer stop()
+
+			body := apiv1.ResourceGroupSpawn{ResourceDefinitionName: "rd-modify", VolumeSizes: []int64{4096}}
+			set(&body.OverrideProps, &body.DeleteProps, &body.DeleteNamespace)
+
+			assertRefusedNamingField(t, base+"/v1/resource-groups/rg-modify/spawn", body, field)
+
+			_, err := st.ResourceDefinitions().Get(t.Context(), "rd-modify")
+			if !errors.Is(err, store.ErrNotFound) {
+				t.Fatalf("a refused spawn left the definition behind: err=%v", err)
+			}
+		})
+	}
+}
+
+// `linstor rg spawn --definition-only` asks for the definition and its
+// volumes and no placement. The handler placed replicas anyway.
+func TestSpawnDefinitionsOnlyPlacesNothing(t *testing.T) {
+	t.Parallel()
+
+	st := store.NewInMemory()
+	ctx := t.Context()
+
+	if err := st.ResourceGroups().Create(ctx, &apiv1.ResourceGroup{
+		Name:         "rg-defs",
+		SelectFilter: apiv1.AutoSelectFilter{PlaceCount: 2, StoragePool: "pool"},
+	}); err != nil {
+		t.Fatalf("seed group: %v", err)
+	}
+
+	for _, node := range []string{"n1", "n2"} {
+		if err := st.StoragePools().Create(ctx, &apiv1.StoragePool{
+			StoragePoolName: "pool", NodeName: node, ProviderKind: apiv1.StoragePoolKindLVMThin,
+			FreeCapacity: 1_000_000, TotalCapacity: 10_000_000,
+		}); err != nil {
+			t.Fatalf("seed pool: %v", err)
+		}
+	}
+
+	base, stop := startServerWithStore(t, st)
+	defer stop()
+
+	raw, err := json.Marshal(apiv1.ResourceGroupSpawn{
+		ResourceDefinitionName: "rd-defs",
+		VolumeSizes:            []int64{4096},
+		DefinitionsOnly:        true,
+	})
+	if err != nil {
+		t.Fatalf("marshal: %v", err)
+	}
+
+	resp := httpPost(t, base+"/v1/resource-groups/rg-defs/spawn", raw)
+	defer func() { _ = resp.Body.Close() }()
+
+	if resp.StatusCode != http.StatusCreated {
+		b, _ := io.ReadAll(resp.Body)
+		t.Fatalf("status %d, want 201: %s", resp.StatusCode, b)
+	}
+
+	vds, err := st.VolumeDefinitions().List(ctx, "rd-defs")
+	if err != nil || len(vds) != 1 {
+		t.Fatalf("volume definitions: %+v, err=%v; want one", vds, err)
+	}
+
+	replicas, err := st.Resources().ListByDefinition(ctx, "rd-defs")
+	if err != nil {
+		t.Fatalf("list replicas: %v", err)
+	}
+
+	if len(replicas) != 0 {
+		t.Fatalf("definitions_only spawn placed %d replicas, want none", len(replicas))
+	}
+}
+
+func assertRefusedNamingField(t *testing.T, url string, body any, field string) {
+	t.Helper()
+
+	raw, err := json.Marshal(body)
+	if err != nil {
+		t.Fatalf("marshal: %v", err)
+	}
+
+	resp := httpPost(t, url, raw)
+	defer func() { _ = resp.Body.Close() }()
+
+	got, _ := io.ReadAll(resp.Body)
+
+	if resp.StatusCode != http.StatusBadRequest {
+		t.Fatalf("status %d, want 400: %s", resp.StatusCode, got)
+	}
+
+	if !strings.Contains(string(got), field) {
+		t.Fatalf("the refusal does not name %s: %s", field, got)
+	}
+}

pkg/rest/flags_validation.go

@@ -156,7 +156,7 @@ func init() {
 
 	for _, p := range pairs {
 		if p.local != p.exported {
-			panic("Bug-167 flag-allowlist desync: local " + p.name + " != apiv1 constant")
+			panic("flag allowlist out of sync with apiv1: local " + p.name + " != apiv1 constant")
 		}
 	}
 }

pkg/rest/kv_store.go

@@ -212,7 +212,7 @@ func decodeKVModifyBody(w http.ResponseWriter, r *http.Request) (apiv1.GenericPr
 
 	for key := range raw {
 		switch key {
-		case "override_props", "delete_props", "delete_namespaces":
+		case fieldOverrideProps, fieldDeleteProps, fieldDeleteNamespaces:
 			continue
 		default:
 			writeError(w, http.StatusBadRequest,
@@ -224,7 +224,7 @@ func decodeKVModifyBody(w http.ResponseWriter, r *http.Request) (apiv1.GenericPr
 		}
 	}
 
-	if rawOverride, ok := raw["override_props"]; ok {
+	if rawOverride, ok := raw[fieldOverrideProps]; ok {
 		err = json.Unmarshal(rawOverride, &patch.OverrideProps)
 		if err != nil {
 			writeError(w, http.StatusBadRequest, "decode override_props: "+err.Error())
@@ -233,7 +233,7 @@ func decodeKVModifyBody(w http.ResponseWriter, r *http.Request) (apiv1.GenericPr
 		}
 	}
 
-	if rawDelete, ok := raw["delete_props"]; ok {
+	if rawDelete, ok := raw[fieldDeleteProps]; ok {
 		err = json.Unmarshal(rawDelete, &patch.DeleteProps)
 		if err != nil {
 			writeError(w, http.StatusBadRequest, "decode delete_props: "+err.Error())
@@ -242,7 +242,7 @@ func decodeKVModifyBody(w http.ResponseWriter, r *http.Request) (apiv1.GenericPr
 		}
 	}
 
-	if rawNS, ok := raw["delete_namespaces"]; ok {
+	if rawNS, ok := raw[fieldDeleteNamespaces]; ok {
 		err = json.Unmarshal(rawNS, &patch.DeleteNamespace)
 		if err != nil {
 			writeError(w, http.StatusBadRequest, "decode delete_namespaces: "+err.Error())

pkg/rest/operator_messages_test.go

@@ -0,0 +1,90 @@
+// 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 (
+	"fmt"
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"io/fs"
+	"path/filepath"
+	"regexp"
+	"strconv"
+	"strings"
+	"testing"
+)
+
+// internalTrackerRef matches the project's internal tracker spellings in
+// any case, "Bug 174", "Bug-020", "BUG-048" and "Phase 12". They mean nothing to an operator
+// reading an error, an event or a log line, so no string literal in the
+// shipped code carries one; comments may.
+var internalTrackerRef = regexp.MustCompile(`(?i)\bBug[- ][0-9]|\bPhase [0-9]`)
+
+func TestShippedStringsCarryNoInternalTrackerNumbers(t *testing.T) {
+	t.Parallel()
+
+	var hits []string
+
+	for _, root := range []string{"../../pkg", "../../internal", "../../cmd"} {
+		err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
+			if err != nil {
+				return err
+			}
+
+			if d.IsDir() || !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") {
+				return nil
+			}
+
+			fset := token.NewFileSet()
+
+			file, parseErr := parser.ParseFile(fset, path, nil, parser.SkipObjectResolution)
+			if parseErr != nil {
+				return fmt.Errorf("parse %s: %w", path, parseErr)
+			}
+
+			ast.Inspect(file, func(n ast.Node) bool {
+				lit, ok := n.(*ast.BasicLit)
+				if !ok || lit.Kind != token.STRING {
+					return true
+				}
+
+				value, unquoteErr := strconv.Unquote(lit.Value)
+				if unquoteErr != nil {
+					value = lit.Value
+				}
+
+				if internalTrackerRef.MatchString(value) {
+					hits = append(hits, fset.Position(lit.Pos()).String()+": "+value)
+				}
+
+				return true
+			})
+
+			return nil
+		})
+		if err != nil {
+			t.Fatalf("walk %s: %v", root, err)
+		}
+	}
+
+	for _, hit := range hits {
+		t.Errorf("string literal carries an internal tracker number: %s", hit)
+	}
+}