Skip to content

Commit 8433fcb

Browse files
kvapsclaude
kvaps
and
claude
committed
feat(store): slugify k8s metadata.Name for non-RFC1123 LINSTOR names
csi-sanity drives volume/snapshot names like "DeleteSnapshot-volume-1-target" through the LINSTOR REST API. Java LINSTOR accepts those, but we persist them as k8s CRDs whose metadata.Name must satisfy DNS-1123. Add a deterministic Name() slugifier and an annotation-based round-trip so the wire-side name stays exactly what the user gave us. Also align /v1/remotes/* with golinstor's RemoteList (envelope, not bare array) and accept the override_props/delete_props/delete_namespaces fields on RD-create payloads from RG-spawn. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
1 parent 940efe2 commit 8433fcb

10 files changed

Lines changed: 302 additions & 37 deletions

File tree

‎pkg/api/v1/resource_definition.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,4 +59,11 @@ type ResourceDefinitionCreate struct {
5959
ResourceDefinition ResourceDefinition `json:"resource_definition"`
6060
DrbdSecret string `json:"drbd_secret,omitempty"`
6161
ExternalName string `json:"external_name,omitempty"`
62+
63+
// OverrideProps mirrors GenericPropsModify — golinstor stuffs RG
64+
// spawn defaults into this when creating an RD via spawn, so the
65+
// REST handler must accept it even if we don't yet diff the values.
66+
OverrideProps map[string]string `json:"override_props,omitempty"`
67+
DeleteProps []string `json:"delete_props,omitempty"`
68+
DeleteNamespace []string `json:"delete_namespaces,omitempty"`
6269
}

‎pkg/rest/remotes.go‎

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,31 @@ import (
2121
)
2222

2323
// registerRemotes wires the LINSTOR `remotes` endpoints. Cozystack
24-
// doesn't use cross-cluster snapshot shipping, so the typed-by-kind
25-
// stubs return empty arrays — golinstor's snapshot list path calls
26-
// `/v1/remotes/s3` before enumerating snapshots, and a 404 there
27-
// surfaces as `failed to list available s3 remotes` from linstor-csi.
28-
//
29-
// Empty `[]` keeps the wire format LINSTOR-shaped without exposing
30-
// any remote-shipping surface area we don't implement.
24+
// doesn't use cross-cluster snapshot shipping, so we return an empty
25+
// `RemoteList` envelope — golinstor's `client.RemoteList` is an
26+
// object with typed-array fields, NOT a bare array. Returning a raw
27+
// `[]` surfaces as `cannot unmarshal array into Go value of type
28+
// client.RemoteList` on every snapshot-list call.
3129
func (s *Server) registerRemotes(mux *http.ServeMux) {
32-
mux.HandleFunc("GET /v1/remotes", handleEmptyArray)
33-
mux.HandleFunc("GET /v1/remotes/s3", handleEmptyArray)
34-
mux.HandleFunc("GET /v1/remotes/linstor", handleEmptyArray)
35-
mux.HandleFunc("GET /v1/remotes/ebs", handleEmptyArray)
30+
mux.HandleFunc("GET /v1/remotes", handleEmptyRemotes)
31+
mux.HandleFunc("GET /v1/remotes/s3", handleEmptyRemotes)
32+
mux.HandleFunc("GET /v1/remotes/linstor", handleEmptyRemotes)
33+
mux.HandleFunc("GET /v1/remotes/ebs", handleEmptyRemotes)
3634
}
3735

38-
func handleEmptyArray(w http.ResponseWriter, _ *http.Request) {
39-
writeJSON(w, http.StatusOK, []map[string]string{})
36+
// emptyRemoteList is upstream LINSTOR's `RemoteList` zero-value: an
37+
// object with three named arrays, all empty. golinstor decodes the
38+
// response body into this shape unconditionally.
39+
type emptyRemoteList struct {
40+
S3Remotes []map[string]string `json:"s3_remotes"`
41+
LinstorRemotes []map[string]string `json:"linstor_remotes"`
42+
EbsRemotes []map[string]string `json:"ebs_remotes"`
43+
}
44+
45+
func handleEmptyRemotes(w http.ResponseWriter, _ *http.Request) {
46+
writeJSON(w, http.StatusOK, emptyRemoteList{
47+
S3Remotes: []map[string]string{},
48+
LinstorRemotes: []map[string]string{},
49+
EbsRemotes: []map[string]string{},
50+
})
4051
}

‎pkg/store/k8s/crdname.go‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/*
2+
Copyright 2026 Cozystack contributors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package k8s
18+
19+
import (
20+
"crypto/sha256"
21+
"encoding/hex"
22+
"regexp"
23+
"strings"
24+
25+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
26+
)
27+
28+
// AnnotationLinstorName preserves the original LINSTOR-side name when
29+
// it had to be slugified to satisfy DNS-1123 metadata.Name rules.
30+
// Pass-through names are not annotated.
31+
const AnnotationLinstorName = "blockstor.io/linstor-name"
32+
33+
// SetOriginalName tags meta with the original LINSTOR name when it
34+
// disagrees with meta.Name (i.e. the slugifier rewrote it). No-op when
35+
// they match — keeps the annotation map empty for the common case.
36+
func SetOriginalName(meta *metav1.ObjectMeta, original string) {
37+
if meta.Name == original {
38+
return
39+
}
40+
41+
if meta.Annotations == nil {
42+
meta.Annotations = map[string]string{}
43+
}
44+
45+
meta.Annotations[AnnotationLinstorName] = original
46+
}
47+
48+
// OriginalName returns the LINSTOR name the user gave us. If the
49+
// annotation is set we trust it; otherwise meta.Name was already
50+
// rfc1123-clean and is the original.
51+
func OriginalName(meta *metav1.ObjectMeta) string {
52+
if v, ok := meta.Annotations[AnnotationLinstorName]; ok {
53+
return v
54+
}
55+
56+
return meta.Name
57+
}
58+
59+
// Name turns a LINSTOR-shaped name (which Java accepts in mixed case
60+
// and with characters k8s rejects) into something the API server will
61+
// store without complaining. If the input already conforms it passes
62+
// through unchanged; otherwise we lowercase, replace non-allowed runs
63+
// with '-', and append a short SHA-256 prefix so distinct LINSTOR
64+
// names never collide on the same k8s name.
65+
//
66+
// linstor-csi names PVCs like `vol-<uuid>` (already lowercase) but
67+
// csi-sanity uses `DeleteSnapshot-volume-1-…` style — both must work.
68+
func Name(in string) string {
69+
if in != "" && len(in) <= maxK8sName && rfc1123.MatchString(in) {
70+
return in
71+
}
72+
73+
lower := strings.ToLower(in)
74+
75+
var builder strings.Builder
76+
77+
prevDash := true
78+
79+
for _, r := range lower {
80+
switch {
81+
case r >= 'a' && r <= 'z', r >= '0' && r <= '9':
82+
builder.WriteRune(r)
83+
84+
prevDash = false
85+
default:
86+
if !prevDash {
87+
builder.WriteByte('-')
88+
89+
prevDash = true
90+
}
91+
}
92+
}
93+
94+
slug := strings.Trim(builder.String(), "-")
95+
96+
digest := sha256.Sum256([]byte(in))
97+
prefix := hex.EncodeToString(digest[:hashPrefixBytes])
98+
out := prefix + "-" + slug
99+
100+
if len(out) > maxK8sName {
101+
out = out[:maxK8sName]
102+
}
103+
104+
return out
105+
}
106+
107+
// rfc1123 covers k8s metadata.Name validation; declared once at
108+
// package init so the regex compile cost amortises.
109+
var rfc1123 = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`)
110+
111+
const (
112+
// maxK8sName is the metadata.Name length cap (DNS-1123).
113+
maxK8sName = 253
114+
115+
// hashPrefixBytes drives K8sName's collision-resistant prefix
116+
// length. 4 bytes → 8 hex chars → 2^32 distinct slugs, plenty
117+
// for storage objects.
118+
hashPrefixBytes = 4
119+
)

‎pkg/store/k8s/crdname_test.go‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/*
2+
Copyright 2026 Cozystack contributors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package k8s_test
18+
19+
import (
20+
"strings"
21+
"testing"
22+
23+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
24+
25+
"github.com/cozystack/blockstor/pkg/store/k8s"
26+
)
27+
28+
func TestK8sName_PassThroughForValidNames(t *testing.T) {
29+
t.Parallel()
30+
31+
cases := []string{
32+
"vol-3a8c1d4e",
33+
"node-1",
34+
"pvc-bc01.fff",
35+
"a",
36+
}
37+
for _, in := range cases {
38+
if got := k8s.Name(in); got != in {
39+
t.Errorf("Name(%q) = %q, want pass-through", in, got)
40+
}
41+
}
42+
}
43+
44+
func TestK8sName_SlugifiesInvalidNames(t *testing.T) {
45+
t.Parallel()
46+
47+
cases := []string{
48+
"DeleteSnapshot-volume-1-target",
49+
"Foo_Bar",
50+
"weird name with spaces",
51+
"UPPER",
52+
}
53+
for _, in := range cases {
54+
got := k8s.Name(in)
55+
if got == in {
56+
t.Errorf("Name(%q) = %q, expected slugified", in, got)
57+
}
58+
59+
if strings.ToLower(got) != got {
60+
t.Errorf("Name(%q) = %q, expected all lowercase", in, got)
61+
}
62+
63+
if len(got) > 253 {
64+
t.Errorf("Name(%q) = %q (%d chars), expected ≤253", in, got, len(got))
65+
}
66+
}
67+
}
68+
69+
func TestK8sName_DistinctInputsProduceDistinctOutputs(t *testing.T) {
70+
t.Parallel()
71+
72+
a := k8s.Name("Foo-Bar")
73+
b := k8s.Name("foo_bar")
74+
c := k8s.Name("FOO BAR")
75+
76+
if a == b || b == c || a == c {
77+
t.Errorf("collision: a=%q b=%q c=%q", a, b, c)
78+
}
79+
}
80+
81+
func TestK8sName_DeterministicForSameInput(t *testing.T) {
82+
t.Parallel()
83+
84+
in := "DeleteSnapshot-volume-1-source"
85+
first := k8s.Name(in)
86+
second := k8s.Name(in)
87+
88+
if first != second {
89+
t.Errorf("Name(%q) non-deterministic: %q vs %q", in, first, second)
90+
}
91+
}
92+
93+
func TestSetAndOriginalName_RoundTrip(t *testing.T) {
94+
t.Parallel()
95+
96+
original := "DeleteSnapshot-volume-1-target"
97+
meta := metav1.ObjectMeta{Name: k8s.Name(original)}
98+
k8s.SetOriginalName(&meta, original)
99+
100+
if got := k8s.OriginalName(&meta); got != original {
101+
t.Errorf("OriginalName = %q, want %q", got, original)
102+
}
103+
}
104+
105+
func TestSetOriginalName_PassThroughDoesNotAnnotate(t *testing.T) {
106+
t.Parallel()
107+
108+
original := "vol-3a8c1d4e"
109+
meta := metav1.ObjectMeta{Name: k8s.Name(original)}
110+
k8s.SetOriginalName(&meta, original)
111+
112+
if _, ok := meta.Annotations[k8s.AnnotationLinstorName]; ok {
113+
t.Errorf("expected no annotation for pass-through name; got %v", meta.Annotations)
114+
}
115+
116+
if got := k8s.OriginalName(&meta); got != original {
117+
t.Errorf("OriginalName = %q, want %q", got, original)
118+
}
119+
}

‎pkg/store/k8s/nodes.go‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func (n *nodes) List(ctx context.Context) ([]apiv1.Node, error) {
5959
func (n *nodes) Get(ctx context.Context, name string) (apiv1.Node, error) {
6060
var crd crdv1alpha1.Node
6161

62-
err := n.c.Get(ctx, types.NamespacedName{Name: name}, &crd)
62+
err := n.c.Get(ctx, types.NamespacedName{Name: Name(name)}, &crd)
6363
if err != nil {
6464
if apierrors.IsNotFound(err) {
6565
return apiv1.Node{}, errors.Wrapf(store.ErrNotFound, "node %q", name)
@@ -100,7 +100,7 @@ func (n *nodes) Update(ctx context.Context, in *apiv1.Node) error {
100100

101101
var existing crdv1alpha1.Node
102102

103-
err := n.c.Get(ctx, types.NamespacedName{Name: in.Name}, &existing)
103+
err := n.c.Get(ctx, types.NamespacedName{Name: Name(in.Name)}, &existing)
104104
if err != nil {
105105
if apierrors.IsNotFound(err) {
106106
return errors.Wrapf(store.ErrNotFound, "node %q", in.Name)
@@ -127,7 +127,7 @@ func (n *nodes) Update(ctx context.Context, in *apiv1.Node) error {
127127
func (n *nodes) SetConnectionStatus(ctx context.Context, name, status string) error {
128128
var existing crdv1alpha1.Node
129129

130-
err := n.c.Get(ctx, types.NamespacedName{Name: name}, &existing)
130+
err := n.c.Get(ctx, types.NamespacedName{Name: Name(name)}, &existing)
131131
if err != nil {
132132
if apierrors.IsNotFound(err) {
133133
return errors.Wrapf(store.ErrNotFound, "node %q", name)
@@ -148,7 +148,7 @@ func (n *nodes) SetConnectionStatus(ctx context.Context, name, status string) er
148148

149149
// Delete removes the named Node CRD.
150150
func (n *nodes) Delete(ctx context.Context, name string) error {
151-
crd := &crdv1alpha1.Node{ObjectMeta: metav1.ObjectMeta{Name: name}}
151+
crd := &crdv1alpha1.Node{ObjectMeta: metav1.ObjectMeta{Name: Name(name)}}
152152

153153
err := n.c.Delete(ctx, crd)
154154
if err != nil {
@@ -165,7 +165,7 @@ func (n *nodes) Delete(ctx context.Context, name string) error {
165165
// crdToWireNode flattens a Node CRD into the LINSTOR REST shape.
166166
func crdToWireNode(crd *crdv1alpha1.Node) apiv1.Node {
167167
out := apiv1.Node{
168-
Name: crd.Name,
168+
Name: OriginalName(&crd.ObjectMeta),
169169
Type: crd.Spec.Type,
170170
Props: crd.Spec.Props,
171171
ConnectionStatus: crd.Status.ConnectionStatus,
@@ -197,10 +197,13 @@ func crdToWireNode(crd *crdv1alpha1.Node) apiv1.Node {
197197

198198
// wireToCRDNode builds a fresh Node CRD from an apiv1.Node — used by Create.
199199
func wireToCRDNode(in *apiv1.Node) *crdv1alpha1.Node {
200-
return &crdv1alpha1.Node{
201-
ObjectMeta: metav1.ObjectMeta{Name: in.Name},
200+
crd := &crdv1alpha1.Node{
201+
ObjectMeta: metav1.ObjectMeta{Name: Name(in.Name)},
202202
Spec: wireToCRDNodeSpec(in),
203203
}
204+
SetOriginalName(&crd.ObjectMeta, in.Name)
205+
206+
return crd
204207
}
205208

206209
// wireToCRDNodeSpec is the spec-only converter used by both Create and Update.

0 commit comments

Comments
 (0)