Enhancement: Add strict format validation for yurthub storage keys

pkg/yurthub/storage/disk/key.go

@@ -133,3 +133,34 @@ func ExtractKeyBuildInfo(key storage.Key) (*storage.KeyBuildInfo, error) {
 
 	return buildInfo, nil
 }
+
+func (k storageKey) Validate() error {
+	if k.path == "" {
+		return storage.ErrKeyIsEmpty
+	}
+
+	path := strings.TrimPrefix(k.path, "/")
+	elems := strings.SplitN(path, "/", 3)
+
+	// A non-root key must always have at least
+	// <Component>/<Resource.Version.Group>/<Namespace or Name>
+	if !k.rootKey && len(elems) < 3 {
+		return fmt.Errorf("invalid disk key %s: expect at least component/resource/name, got %d segments", k.path, len(elems))
+	}
+	if len(elems) < 2 {
+		return fmt.Errorf("invalid disk key %s: missing component or resource segment", k.path)
+	}
+	if elems[0] == "" {
+		return fmt.Errorf("invalid disk key %s: empty component", k.path)
+	}
+
+	gvrElems := strings.Split(elems[1], ".")
+	if len(gvrElems) != 1 && len(gvrElems) != 3 {
+		return fmt.Errorf("invalid disk key %s: invalid resource/version/group format %q", k.path, elems[1])
+	}
+	if gvrElems[0] == "" {
+		return fmt.Errorf("invalid disk key %s: empty resource", k.path)
+	}
+
+	return nil
+}

pkg/yurthub/storage/disk/key_test.go

@@ -275,3 +275,59 @@ func TestExtractKeyBuildInfo(t *testing.T) {
 		})
 	}
 }
+
+func TestStorageKey_Validate(t *testing.T) {
+	cases := []struct {
+		name    string
+		key     storageKey
+		wantErr bool
+	}{
+		{
+			name:    "valid full key",
+			key:     storageKey{path: "kubelet/pods.v1.core/default/nginx", rootKey: false},
+			wantErr: false,
+		},
+		{
+			name:    "valid non-enhancement mode key (no version/group)",
+			key:     storageKey{path: "kubelet/pods/default/nginx", rootKey: false},
+			wantErr: false,
+		},
+		{
+			name:    "valid root key with only component+resource",
+			key:     storageKey{path: "kubelet/pods.v1.core", rootKey: true},
+			wantErr: false,
+		},
+		{
+			name:    "empty path",
+			key:     storageKey{path: "", rootKey: false},
+			wantErr: true,
+		},
+		{
+			name:    "non-root key missing name segment",
+			key:     storageKey{path: "kubelet/pods.v1.core", rootKey: false},
+			wantErr: true,
+		},
+		{
+			name:    "malformed gvr (2 dots-worth of parts, not 1 or 3)",
+			key:     storageKey{path: "kubelet/pods.v1/default/nginx", rootKey: false},
+			wantErr: true,
+		},
+		{
+    		name:    "empty component",
+    		key:     storageKey{path: "//pods.v1.core/default/nginx", rootKey: false},
+    		wantErr: true,
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			err := c.key.Validate()
+			if c.wantErr && err == nil {
+				t.Errorf("expected error, got nil")
+			}
+			if !c.wantErr && err != nil {
+				t.Errorf("expected no error, got %v", err)
+			}
+		})
+	}
+}

pkg/yurthub/storage/key.go

@@ -22,6 +22,10 @@ type Key interface {
 	Key() string
 }
 
+type KeyFormatValidator interface {
+	Validate() error
+}
+
 type KeyBuildInfo struct {
 	Component string
 	Namespace string

pkg/yurthub/storage/utils/validate.go

@@ -22,14 +22,18 @@ import (
 	"github.com/openyurtio/openyurt/pkg/yurthub/storage"
 )
 
-// TODO: should also valid the key format
 func ValidateKey(key storage.Key, validKeyType interface{}) error {
 	if key == nil || key.Key() == "" {
 		return storage.ErrKeyIsEmpty
 	}
 	if reflect.TypeOf(key) != reflect.TypeOf(validKeyType) {
 		return storage.ErrUnrecognizedKey
 	}
+	if v, ok := key.(storage.KeyFormatValidator); ok {
+		if err := v.Validate(); err != nil {
+			return err
+		}
+	}
 	return nil
 }
 

pkg/yurthub/storage/utils/validate_test.go

@@ -23,53 +23,89 @@ import (
 	"github.com/openyurtio/openyurt/pkg/yurthub/storage"
 )
 
-type testKey struct {
+// mockValidKey implements storage.Key + storage.KeyFormatValidator.
+type mockValidKey struct {
+	path      string
+	shouldErr bool
+}
+
+func (k mockValidKey) Key() string {
+	return k.path
+}
+
+func (k mockValidKey) Validate() error {
+	if k.shouldErr {
+		return errors.New("bad format")
+	}
+	return nil
+}
+
+// mockPlainKey implements ONLY storage.Key (no Validate).
+// It simulates ClusterInfoKey.
+type mockPlainKey struct {
 	path string
 }
 
-func (k testKey) Key() string {
+func (k mockPlainKey) Key() string {
 	return k.path
 }
 
 func TestValidateKey(t *testing.T) {
-	cases := map[string]struct {
-		key          storage.Key
-		validKeyType interface{}
-		expectedErr  error
-	}{
-		"nil key": {
-			key:          nil,
-			validKeyType: testKey{},
-			expectedErr:  storage.ErrKeyIsEmpty,
-		},
-		"empty key": {
-			key:          testKey{path: ""},
-			validKeyType: testKey{},
-			expectedErr:  storage.ErrKeyIsEmpty,
-		},
-		"unrecognized key type": {
-			key:          testKey{path: "kubelet/pods.v1.core/default/foo"},
-			validKeyType: storage.ClusterInfoKey{},
-			expectedErr:  storage.ErrUnrecognizedKey,
-		},
-		"valid key": {
-			key:          testKey{path: "kubelet/pods.v1.core/default/foo"},
-			validKeyType: testKey{},
-			expectedErr:  nil,
-		},
-	}
+	validType := mockValidKey{}
+	plainType := mockPlainKey{}
 
-	for name, tc := range cases {
-		t.Run(name, func(t *testing.T) {
-			err := ValidateKey(tc.key, tc.validKeyType)
-			if !errors.Is(err, tc.expectedErr) {
-				t.Errorf("ValidateKey() error = %v, want %v", err, tc.expectedErr)
-			}
-		})
-	}
+	t.Run("nil key returns ErrKeyIsEmpty", func(t *testing.T) {
+		err := ValidateKey(nil, validType)
+		if !errors.Is(err, storage.ErrKeyIsEmpty) {
+			t.Errorf("expected ErrKeyIsEmpty, got %v", err)
+		}
+	})
+
+	t.Run("empty key string returns ErrKeyIsEmpty", func(t *testing.T) {
+		err := ValidateKey(mockValidKey{path: ""}, validType)
+		if !errors.Is(err, storage.ErrKeyIsEmpty) {
+			t.Errorf("expected ErrKeyIsEmpty, got %v", err)
+		}
+	})
+
+	t.Run("wrong concrete type returns ErrUnrecognizedKey", func(t *testing.T) {
+		err := ValidateKey(mockPlainKey{path: "x"}, validType)
+		if !errors.Is(err, storage.ErrUnrecognizedKey) {
+			t.Errorf("expected ErrUnrecognizedKey, got %v", err)
+		}
+	})
+
+	t.Run("valid key with good format passes", func(t *testing.T) {
+		err := ValidateKey(
+			mockValidKey{path: "x", shouldErr: false},
+			validType,
+		)
+		if err != nil {
+			t.Errorf("expected nil, got %v", err)
+		}
+	})
+
+	t.Run("valid type but bad internal format is rejected", func(t *testing.T) {
+		err := ValidateKey(
+			mockValidKey{path: "x", shouldErr: true},
+			validType,
+		)
+		if err == nil {
+			t.Errorf("expected format validation error, got nil")
+		}
+	})
+
+	t.Run("key type without Validate still passes", func(t *testing.T) {
+		err := ValidateKey(mockPlainKey{path: "x"}, plainType)
+		if err != nil {
+			t.Errorf("expected nil (no Validate method), got %v", err)
+		}
+	})
 }
 
 func TestValidateKV(t *testing.T) {
+	validKeyType := mockValidKey{}
+
 	cases := map[string]struct {
 		key          storage.Key
 		content      []byte
@@ -79,35 +115,95 @@ func TestValidateKV(t *testing.T) {
 		"nil key": {
 			key:          nil,
 			content:      []byte("data"),
-			validKeyType: testKey{},
+			validKeyType: validKeyType,
+			expectedErr:  storage.ErrKeyIsEmpty,
+		},
+		"empty key": {
+			key:          mockValidKey{path: ""},
+			content:      []byte("data"),
+			validKeyType: validKeyType,
 			expectedErr:  storage.ErrKeyIsEmpty,
 		},
 		"unrecognized key type": {
-			key:          testKey{path: "kubelet/pods.v1.core/default/foo"},
+			key:          mockPlainKey{
+				path: "kubelet/pods.v1.core/default/foo",
+			},
 			content:      []byte("data"),
-			validKeyType: storage.ClusterInfoKey{},
+			validKeyType: validKeyType,
 			expectedErr:  storage.ErrUnrecognizedKey,
 		},
+		"invalid key format": {
+			key: mockValidKey{
+				path:      "kubelet/pods.v1.core/default/foo",
+				shouldErr: true,
+			},
+			content:      []byte("data"),
+			validKeyType: validKeyType,
+			expectedErr:  errors.New("bad format"),
+		},
 		"empty content": {
-			key:          testKey{path: "kubelet/pods.v1.core/default/foo"},
+			key: mockValidKey{
+				path: "kubelet/pods.v1.core/default/foo",
+			},
 			content:      []byte{},
-			validKeyType: testKey{},
+			validKeyType: validKeyType,
 			expectedErr:  storage.ErrKeyHasNoContent,
 		},
 		"valid key and content": {
-			key:          testKey{path: "kubelet/pods.v1.core/default/foo"},
+			key: mockValidKey{
+				path: "kubelet/pods.v1.core/default/foo",
+			},
+			content:      []byte("data"),
+			validKeyType: validKeyType,
+			expectedErr:  nil,
+		},
+		"plain key without Validate and valid content": {
+			key: mockPlainKey{
+				path: "kubelet/pods.v1.core/default/foo",
+			},
 			content:      []byte("data"),
-			validKeyType: testKey{},
+			validKeyType: mockPlainKey{},
 			expectedErr:  nil,
 		},
 	}
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
 			err := ValidateKV(tc.key, tc.content, tc.validKeyType)
-			if !errors.Is(err, tc.expectedErr) {
-				t.Errorf("ValidateKV() error = %v, want %v", err, tc.expectedErr)
+
+			if tc.expectedErr != nil {
+				if err == nil {
+					t.Errorf(
+						"ValidateKV() error = nil, want %v",
+						tc.expectedErr,
+					)
+					return
+				}
+
+				if tc.expectedErr.Error() == "bad format" {
+					if err.Error() != tc.expectedErr.Error() {
+						t.Errorf(
+							"ValidateKV() error = %v, want %v",
+							err,
+							tc.expectedErr,
+						)
+					}
+					return
+				}
+
+				if !errors.Is(err, tc.expectedErr) {
+					t.Errorf(
+						"ValidateKV() error = %v, want %v",
+						err,
+						tc.expectedErr,
+					)
+				}
+				return
+			}
+
+			if err != nil {
+				t.Errorf("ValidateKV() error = %v, want nil", err)
 			}
 		})
 	}
-}
+}