Skip to content

Commit 46ecb80

Browse files
authored
Fix mount path generation (#1409)
Signed-off-by: Alexey Makhov <amakhov@mirantis.com> Signed-off-by: makhov <amakhov@mirantis.com>
1 parent 893917b commit 46ecb80

2 files changed

Lines changed: 145 additions & 1 deletion

File tree

internal/controller/k0smotron.io/k0smotroncluster_statefulset.go

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package k0smotronio
1818

1919
import (
2020
"context"
21+
"crypto/sha256"
2122
"fmt"
2223
"maps"
2324
"reflect"
@@ -36,6 +37,7 @@ import (
3637
apierrors "k8s.io/apimachinery/pkg/api/errors"
3738
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3839
"k8s.io/apimachinery/pkg/types"
40+
"k8s.io/apimachinery/pkg/util/validation"
3941
"k8s.io/kubernetes/pkg/controller"
4042
"sigs.k8s.io/controller-runtime/pkg/client"
4143
)
@@ -48,6 +50,7 @@ const (
4850
)
4951

5052
var versionRegex = regexp.MustCompile(`v\d+.\d+.\d+-k0s.\d+`)
53+
var invalidVolumeNameCharsRegex = regexp.MustCompile(`[^a-z0-9-]+`)
5154

5255
// findStatefulSetPod returns a first running pod from a StatefulSet
5356
func findStatefulSetPod(ctx context.Context, statefulSet string, namespace string, clientSet *kubernetes.Clientset) (*v1.Pod, error) {
@@ -253,7 +256,7 @@ func (scope *kmcScope) generateStatefulSet(ctx context.Context, kmc *km.Cluster)
253256
}
254257

255258
for _, file := range kmc.Spec.Mounts {
256-
volumeName := strings.Replace(file.Path[1:], "/", "-", -1)
259+
volumeName := pathToVolumeName(file.Path)
257260
statefulSet.Spec.Template.Spec.Volumes = append(statefulSet.Spec.Template.Spec.Volumes, v1.Volume{Name: volumeName, VolumeSource: file.VolumeSource})
258261

259262
if file.VolumeSource.ConfigMap != nil || file.VolumeSource.Secret != nil {
@@ -476,6 +479,37 @@ func mountSecrets(kmc *km.Cluster, sfs *apps.StatefulSet) {
476479
})
477480
}
478481

482+
// pathToVolumeName converts a mount path to a valid Kubernetes volume name (DNS label).
483+
// For paths that already produce a valid name with the simple transformation (strip leading
484+
// slash, replace slashes with dashes), that name is returned unchanged for backward
485+
// compatibility. Otherwise the path is sanitized and a short hash suffix is appended to
486+
// ensure uniqueness across paths that may collide after sanitization.
487+
func pathToVolumeName(path string) string {
488+
trimmed := strings.TrimPrefix(path, "/")
489+
// Try the simple, backward-compatible transformation first.
490+
simpleName := strings.ReplaceAll(trimmed, "/", "-")
491+
if len(validation.IsDNS1123Label(simpleName)) == 0 {
492+
return simpleName
493+
}
494+
495+
// Path needs sanitization (contains dots, underscores, uppercase, etc.).
496+
name := strings.ToLower(trimmed)
497+
name = invalidVolumeNameCharsRegex.ReplaceAllString(name, "-")
498+
name = strings.Trim(name, "-")
499+
500+
hash := fmt.Sprintf("%x", sha256.Sum256([]byte(path)))[:5]
501+
502+
// DNS label max length is 63; reserve 6 chars for "-" + 5-char hash.
503+
const maxBase = 57
504+
if len(name) > maxBase {
505+
name = strings.TrimRight(name[:maxBase], "-")
506+
}
507+
if name == "" {
508+
return hash
509+
}
510+
return name + "-" + hash
511+
}
512+
479513
func addMonitoringStack(kmc *km.Cluster, statefulSet *apps.StatefulSet) {
480514
nginxConfCMName := kmc.GetMonitoringNginxConfigMapName()
481515
statefulSet.Spec.Template.Spec.Containers = append(statefulSet.Spec.Template.Spec.Containers, v1.Container{
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
//go:build !envtest
2+
3+
/*
4+
Copyright 2023.
5+
6+
Licensed under the Apache License, Version 2.0 (the "License");
7+
you may not use this file except in compliance with the License.
8+
You may obtain a copy of the License at
9+
10+
http://www.apache.org/licenses/LICENSE-2.0
11+
12+
Unless required by applicable law or agreed to in writing, software
13+
distributed under the License is distributed on an "AS IS" BASIS,
14+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
See the License for the specific language governing permissions and
16+
limitations under the License.
17+
*/
18+
19+
package k0smotronio
20+
21+
import (
22+
"strings"
23+
"testing"
24+
25+
"github.com/stretchr/testify/assert"
26+
)
27+
28+
func Test_pathToVolumeName(t *testing.T) {
29+
tests := []struct {
30+
path string
31+
want string // exact expected name for simple/clean paths (no hash)
32+
wantPrefix string // expected prefix for sanitized paths (ends with "-")
33+
}{
34+
{
35+
// Simple path: already valid, no hash added (backward-compatible)
36+
path: "/etc/kubernetes/pki",
37+
want: "etc-kubernetes-pki",
38+
},
39+
{
40+
// Simple path used in e2e upgrade test - must stay stable across upgrades
41+
path: "/tmp/test",
42+
want: "tmp-test",
43+
},
44+
{
45+
// Underscore is invalid -> sanitized with hash
46+
path: "/my_config/file.conf",
47+
wantPrefix: "my-config-file-conf-",
48+
},
49+
{
50+
// Dot is invalid -> sanitized with hash
51+
path: "/etc/ssl/certs/ca-certificates.crt",
52+
wantPrefix: "etc-ssl-certs-ca-certificates-crt-",
53+
},
54+
{
55+
// Uppercase is invalid -> sanitized with hash
56+
path: "/VAR/lib/K0s",
57+
wantPrefix: "var-lib-k0s-",
58+
},
59+
{
60+
// Single alphanumeric: valid, no hash
61+
path: "/a",
62+
want: "a",
63+
},
64+
{
65+
// Dot in path segment -> sanitized with hash
66+
path: "/root/.aws/credentials",
67+
wantPrefix: "root-aws-credentials-",
68+
},
69+
}
70+
71+
for _, tt := range tests {
72+
t.Run(tt.path, func(t *testing.T) {
73+
got := pathToVolumeName(tt.path)
74+
75+
// Must be at most 63 chars (DNS label limit)
76+
assert.LessOrEqual(t, len(got), 63, "volume name exceeds DNS label limit")
77+
78+
// Must be a valid DNS label
79+
assert.Regexp(t, `^[a-z0-9]([a-z0-9-]*[a-z0-9])?$`, got, "volume name must be a valid DNS label")
80+
81+
if tt.want != "" {
82+
assert.Equal(t, tt.want, got)
83+
} else {
84+
assert.True(t, strings.HasPrefix(got, tt.wantPrefix), "expected prefix %q, got %q", tt.wantPrefix, got)
85+
}
86+
})
87+
}
88+
89+
t.Run("unique names for paths that sanitize to the same string", func(t *testing.T) {
90+
paths := []string{
91+
"/my_path",
92+
"/my.path",
93+
}
94+
names := make(map[string]string)
95+
for _, p := range paths {
96+
name := pathToVolumeName(p)
97+
for prev, prevName := range names {
98+
assert.NotEqual(t, prevName, name, "paths %q and %q produced the same volume name %q", prev, p, name)
99+
}
100+
names[p] = name
101+
}
102+
})
103+
104+
t.Run("long path is truncated to 63 chars", func(t *testing.T) {
105+
longPath := "/a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p/q/r/s/t/u/v/w/x/y/z/aa/bb/cc/dd/ee/ff"
106+
got := pathToVolumeName(longPath)
107+
assert.LessOrEqual(t, len(got), 63)
108+
assert.Regexp(t, `^[a-z0-9]([a-z0-9-]*[a-z0-9])?$`, got)
109+
})
110+
}

0 commit comments

Comments
 (0)