diff --git a/pkg/yurthub/storage/disk/key.go b/pkg/yurthub/storage/disk/key.go index ec5588ec5e9..9a643d0303f 100644 --- a/pkg/yurthub/storage/disk/key.go +++ b/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 + // // + 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 +} diff --git a/pkg/yurthub/storage/disk/key_test.go b/pkg/yurthub/storage/disk/key_test.go index 8f91be5181c..94378136d08 100644 --- a/pkg/yurthub/storage/disk/key_test.go +++ b/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) + } + }) + } +} \ No newline at end of file diff --git a/pkg/yurthub/storage/key.go b/pkg/yurthub/storage/key.go index dac0a3be64b..cc0b257654f 100644 --- a/pkg/yurthub/storage/key.go +++ b/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 diff --git a/pkg/yurthub/storage/utils/validate.go b/pkg/yurthub/storage/utils/validate.go index b2448fa035d..fb03a850b88 100644 --- a/pkg/yurthub/storage/utils/validate.go +++ b/pkg/yurthub/storage/utils/validate.go @@ -22,7 +22,6 @@ 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 @@ -30,6 +29,11 @@ func ValidateKey(key storage.Key, validKeyType interface{}) error { 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 } diff --git a/pkg/yurthub/storage/utils/validate_test.go b/pkg/yurthub/storage/utils/validate_test.go index b27eda92d48..675c9380dc9 100644 --- a/pkg/yurthub/storage/utils/validate_test.go +++ b/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,25 +115,54 @@ 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, }, } @@ -105,9 +170,40 @@ func TestValidateKV(t *testing.T) { 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) } }) } -} +} \ No newline at end of file