Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion pkg/yurthub/storage/disk/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ func (ds *diskStorage) Create(key storage.Key, content []byte) error {
if err := utils.ValidateKey(key, storageKey{}); err != nil {
return err
}
if err := utils.ValidateDiskKey(key); err != nil {
return err
}
storageKey := key.(storageKey)

if !storageKey.isRootKey() && len(content) == 0 {
Expand Down Expand Up @@ -137,6 +140,9 @@ func (ds *diskStorage) Delete(key storage.Key) error {
if err := utils.ValidateKey(key, storageKey{}); err != nil {
return err
}
if err := utils.ValidateDiskKey(key); err != nil {
return err
}
storageKey := key.(storageKey)

ds.lockKey(storageKey)
Expand All @@ -158,7 +164,10 @@ func (ds *diskStorage) Delete(key storage.Key) error {
// If key points to a dir, return ErrKeyHasNoContent.
func (ds *diskStorage) Get(key storage.Key) ([]byte, error) {
if err := utils.ValidateKey(key, storageKey{}); err != nil {
return []byte{}, storage.ErrKeyIsEmpty
return []byte{}, err
}
if err := utils.ValidateDiskKey(key); err != nil {
return []byte{}, err
}
storageKey := key.(storageKey)

Expand All @@ -185,6 +194,9 @@ func (ds *diskStorage) List(key storage.Key) ([][]byte, error) {
if err := utils.ValidateKey(key, storageKey{}); err != nil {
return [][]byte{}, err
}
if err := utils.ValidateDiskKey(key); err != nil {
return [][]byte{}, err
}
storageKey := key.(storageKey)

ds.lockKey(storageKey)
Expand Down
12 changes: 6 additions & 6 deletions pkg/yurthub/storage/disk/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ var _ = Describe("Test DiskStorage Exposed Functions", func() {
})
It("should return ErrKeyIsEmpty on Get when key type is unrecognized", func() {
_, err = store.Get(unrecognized)
Expect(err).To(Equal(storage.ErrKeyIsEmpty))
Expect(err).To(Equal(storage.ErrUnrecognizedKey))
})
It("should return ErrUnrecognizedKey on List", func() {
_, err = store.List(unrecognized)
Expand Down Expand Up @@ -408,7 +408,7 @@ var _ = Describe("Test DiskStorage Exposed Functions", func() {
})
It("should return ErrKeyIsEmpty if key is empty", func() {
err = store.Create(storageKey{}, podBytes)
Expect(err).To(Equal(storage.ErrKeyIsEmpty))
Expect(err).To(Equal(storage.ErrUnrecognizedKey))
})
It("should return ErrKeyExists if key exists", func() {
err = writeFileAt(filepath.Join(baseDir, podKey.Key()), podBytes)
Expand Down Expand Up @@ -473,7 +473,7 @@ var _ = Describe("Test DiskStorage Exposed Functions", func() {
})
It("should return ErrKeyIsEmpty if key is empty", func() {
err = store.Delete(storageKey{})
Expect(err).To(Equal(storage.ErrKeyIsEmpty))
Expect(err).To(Equal(storage.ErrUnrecognizedKey))
})
})

Expand Down Expand Up @@ -503,7 +503,7 @@ var _ = Describe("Test DiskStorage Exposed Functions", func() {
})
It("should return ErrKeyIsEmpty if key is empty", func() {
_, err = store.Get(storageKey{})
Expect(err).To(Equal(storage.ErrKeyIsEmpty))
Expect(err).To(Equal(storage.ErrUnrecognizedKey))
})
It("should return ErrStorageNotFound if key does not exist", func() {
newPodKey, err := store.KeyFunc(storage.KeyBuildInfo{
Expand Down Expand Up @@ -615,7 +615,7 @@ var _ = Describe("Test DiskStorage Exposed Functions", func() {
})
It("should return ErrKeyIsEmpty if key is empty", func() {
_, err = store.List(storageKey{})
Expect(err).To(Equal(storage.ErrKeyIsEmpty))
Expect(err).To(Equal(storage.ErrUnrecognizedKey))
})
It("should return ErrStorageNotFound if the rootKey does no exist", func() {
rootKeyInfo.Resources = "services"
Expand Down Expand Up @@ -707,7 +707,7 @@ var _ = Describe("Test DiskStorage Exposed Functions", func() {
})
It("should return ErrKeyIsEmpty if key is empty", func() {
_, err = store.Update(storageKey{}, comingPodBytes, comingPodRvUint64)
Expect(err).To(Equal(storage.ErrKeyIsEmpty))
Expect(err).To(Equal(storage.ErrUnrecognizedKey))
})
It("should return ErrStorageNotFound if key does not exist", func() {
newPodKey, err := store.KeyFunc(storage.KeyBuildInfo{
Expand Down
61 changes: 60 additions & 1 deletion pkg/yurthub/storage/utils/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@

import (
"reflect"
"strings"

"github.com/openyurtio/openyurt/pkg/yurthub/storage"
)

// TODO: should also valid the key format
// ValidateKey validates a storage.Key is non-nil/non-empty and has the correct type.
func ValidateKey(key storage.Key, validKeyType interface{}) error {
if key == nil || key.Key() == "" {
return storage.ErrKeyIsEmpty
Expand All @@ -33,6 +34,64 @@
return nil
}

// ValidateDiskKey validates the internal format of a disk storage key.
// A valid disk key has the format: /<Component>/<Resource[.Version[.Group]]>/<Namespace>/<Name>
// or /<Component>/<Resource[.Version[.Group]]>/<Namespace> for list keys.
// Keys without a leading slash are accepted for backward compatibility.
func ValidateDiskKey(key storage.Key) error {

Check failure on line 41 in pkg/yurthub/storage/utils/validate.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=openyurtio_openyurt&issues=AaBMjiv1kc3VX7mN5B_8&open=AaBMjiv1kc3VX7mN5B_8&pullRequest=2785
if key == nil || key.Key() == "" {
return storage.ErrKeyIsEmpty
}
path := key.Key()
// Strip leading slash if present
if strings.HasPrefix(path, "/") {
path = strings.TrimPrefix(path, "/")
}
// Split into at most 3 parts: component, gvr, namespace/name
parts := strings.SplitN(path, "/", 3)
if len(parts) < 2 {
return storage.ErrKeyIsEmpty
}
// Validate component (must not be empty)
if parts[0] == "" {
return storage.ErrKeyIsEmpty
}
// Validate GVR component: either a single resource name (non-enhancement mode)
// or resource.version.group (enhancement mode)
gvr := parts[1]
gvrParts := strings.SplitN(gvr, ".", 3)
switch len(gvrParts) {
case 1:
// Non-enhancement mode: just resource name
if gvrParts[0] == "" {
return storage.ErrKeyIsEmpty
}
case 3:
// Enhancement mode: resource.version.group
for _, p := range gvrParts {
if p == "" {
return storage.ErrKeyIsEmpty
}
}
default:
return storage.ErrKeyIsEmpty
}
// For object keys (non-root), validate namespace/name segment exists
if len(parts) == 3 {
nn := parts[2]
if nn == "" {
return storage.ErrKeyIsEmpty
}
nnParts := strings.SplitN(nn, "/", 2)
for _, p := range nnParts {
if p == "" {
return storage.ErrKeyIsEmpty
}
}
}
return nil
}

func ValidateKV(key storage.Key, content []byte, validKeyType interface{}) error {
if err := ValidateKey(key, validKeyType); err != nil {
return err
Expand Down
95 changes: 95 additions & 0 deletions pkg/yurthub/storage/utils/validate_disk_key_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package utils

import (
"testing"

"github.com/openyurtio/openyurt/pkg/yurthub/storage"
)

type testDiskKey struct {
path string
}

func (k testDiskKey) Key() string {
return k.path
}

func TestValidateDiskKey(t *testing.T) {
cases := map[string]struct {
key storage.Key
expectedErr bool
}{
"nil key": {
key: nil,
expectedErr: true,
},
"empty key": {
key: testDiskKey{path: ""},
expectedErr: true,
},
"valid key without leading slash": {
key: testDiskKey{path: "kubelet/pods.v1.core/default/nginx"},
expectedErr: false,
},
"missing leading slash but invalid GVR": {
key: testDiskKey{path: "kubelet/pods/default/nginx"},
expectedErr: false,
},
"valid object key (enhancement mode)": {
key: testDiskKey{path: "/kubelet/pods.v1.core/default/nginx"},
expectedErr: false,
},
"valid object key (non-enhancement mode)": {
key: testDiskKey{path: "/kubelet/pods/default/nginx"},
expectedErr: false,
},
"valid list key (enhancement mode)": {
key: testDiskKey{path: "/kubelet/pods.v1.core/default"},
expectedErr: false,
},
"valid list key non-namespaced": {
key: testDiskKey{path: "/kubelet/nodes.v1.core/edge-worker"},
expectedErr: false,
},
"valid resource list key": {
key: testDiskKey{path: "/kubelet/pods.v1.core"},
expectedErr: false,
},
"empty component": {
key: testDiskKey{path: "//pods.v1.core/default/nginx"},
expectedErr: true,
},
"empty GVR": {
key: testDiskKey{path: "/kubelet//default/nginx"},
expectedErr: true,
},
"invalid GVR format (2 parts)": {
key: testDiskKey{path: "/kubelet/pods.v1/default/nginx"},
expectedErr: true,
},
"empty namespace in object key": {
key: testDiskKey{path: "/kubelet/pods.v1.core//nginx"},
expectedErr: true,
},
"empty name in object key": {
key: testDiskKey{path: "/kubelet/pods.v1.core/default/"},
expectedErr: true,
},
"only one slash": {
key: testDiskKey{path: "/kubelet"},
expectedErr: true,
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
err := ValidateDiskKey(tc.key)
if tc.expectedErr && err == nil {
t.Errorf("ValidateDiskKey() expected error, got nil")
}
if !tc.expectedErr && err != nil {
t.Errorf("ValidateDiskKey() unexpected error: %v", err)
}
})
}
}