Merge pull request #110311 from logicalhan/fix-continue-tests

add explicit typing for continue tests

staging/src/k8s.io/apiserver/pkg/storage/continue.go

@@ -19,11 +19,19 @@ package storage
 import (
 	"encoding/base64"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"path"
 	"strings"
 )
 
+var (
+	ErrInvalidStartRV             = errors.New("continue key is not valid: incorrect encoded start resourceVersion (version meta.k8s.io/v1)")
+	ErrEmptyStartKey              = errors.New("continue key is not valid: encoded start key empty (version meta.k8s.io/v1)")
+	ErrGenericInvalidKey          = errors.New("continue key is not valid")
+	ErrUnrecognizedEncodedVersion = errors.New("continue key is not valid: server does not recognize this encoded version")
+)
+
 // continueToken is a simple structured object for encoding the state of a continue token.
 // TODO: if we change the version of the encoded from, we can't start encoding the new version
 // until all other servers are upgraded (i.e. we need to support rolling schema)
@@ -39,19 +47,19 @@ type continueToken struct {
 func DecodeContinue(continueValue, keyPrefix string) (fromKey string, rv int64, err error) {
 	data, err := base64.RawURLEncoding.DecodeString(continueValue)
 	if err != nil {
-		return "", 0, fmt.Errorf("continue key is not valid: %v", err)
+		return "", 0, fmt.Errorf("%w: %v", ErrGenericInvalidKey, err)
 	}
 	var c continueToken
 	if err := json.Unmarshal(data, &c); err != nil {
-		return "", 0, fmt.Errorf("continue key is not valid: %v", err)
+		return "", 0, fmt.Errorf("%w: %v", ErrGenericInvalidKey, err)
 	}
 	switch c.APIVersion {
 	case "meta.k8s.io/v1":
 		if c.ResourceVersion == 0 {
-			return "", 0, fmt.Errorf("continue key is not valid: incorrect encoded start resourceVersion (version meta.k8s.io/v1)")
+			return "", 0, ErrInvalidStartRV
 		}
 		if len(c.StartKey) == 0 {
-			return "", 0, fmt.Errorf("continue key is not valid: encoded start key empty (version meta.k8s.io/v1)")
+			return "", 0, ErrEmptyStartKey
 		}
 		// defend against path traversal attacks by clients - path.Clean will ensure that startKey cannot
 		// be at a higher level of the hierarchy, and so when we append the key prefix we will end up with
@@ -63,11 +71,11 @@ func DecodeContinue(continueValue, keyPrefix string) (fromKey string, rv int64,
 		}
 		cleaned := path.Clean(key)
 		if cleaned != key {
-			return "", 0, fmt.Errorf("continue key is not valid: %s", c.StartKey)
+			return "", 0, fmt.Errorf("%w: %v", ErrGenericInvalidKey, c.StartKey)
 		}
 		return keyPrefix + cleaned[1:], c.ResourceVersion, nil
 	default:
-		return "", 0, fmt.Errorf("continue key is not valid: server does not recognize this encoded version %q", c.APIVersion)
+		return "", 0, fmt.Errorf("%w %v", ErrUnrecognizedEncodedVersion, c.APIVersion)
 	}
 }
 

staging/src/k8s.io/apiserver/pkg/storage/continue_test.go

@@ -19,6 +19,7 @@ package storage
 import (
 	"encoding/base64"
 	"encoding/json"
+	"errors"
 	"testing"
 )
 
@@ -40,23 +41,65 @@ func Test_decodeContinue(t *testing.T) {
 		args        args
 		wantFromKey string
 		wantRv      int64
-		wantErr     bool
+		wantErr     error
 	}{
-		{name: "valid", args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "key"), keyPrefix: "/test/"}, wantRv: 1, wantFromKey: "/test/key"},
+		{
-		{name: "root path", args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "/"), keyPrefix: "/test/"}, wantRv: 1, wantFromKey: "/test/"},
+			name:        "valid",
-
+			args:        args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "key"), keyPrefix: "/test/"},
-		{name: "empty version", args: args{continueValue: encodeContinueOrDie("", 1, "key"), keyPrefix: "/test/"}, wantErr: true},
+			wantRv:      1,
-		{name: "invalid version", args: args{continueValue: encodeContinueOrDie("v1", 1, "key"), keyPrefix: "/test/"}, wantErr: true},
+			wantFromKey: "/test/key",
-
+		},
-		{name: "path traversal - parent", args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "../key"), keyPrefix: "/test/"}, wantErr: true},
+		{
-		{name: "path traversal - local", args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "./key"), keyPrefix: "/test/"}, wantErr: true},
+			name:        "root path",
-		{name: "path traversal - double parent", args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "./../key"), keyPrefix: "/test/"}, wantErr: true},
+			args:        args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "/"), keyPrefix: "/test/"},
-		{name: "path traversal - after parent", args: args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "key/../.."), keyPrefix: "/test/"}, wantErr: true},
+			wantRv:      1,
+			wantFromKey: "/test/",
+		},
+		{
+			name:    "empty version",
+			args:    args{continueValue: encodeContinueOrDie("", 1, "key"), keyPrefix: "/test/"},
+			wantErr: ErrUnrecognizedEncodedVersion,
+		},
+		{
+			name:    "invalid version",
+			args:    args{continueValue: encodeContinueOrDie("v1", 1, "key"), keyPrefix: "/test/"},
+			wantErr: ErrUnrecognizedEncodedVersion,
+		},
+		{
+			name:    "invalid RV",
+			args:    args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 0, "key"), keyPrefix: "/test/"},
+			wantErr: ErrInvalidStartRV,
+		},
+		{
+			name:    "no start Key",
+			args:    args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, ""), keyPrefix: "/test/"},
+			wantErr: ErrEmptyStartKey,
+		},
+		{
+			name:    "path traversal - parent",
+			args:    args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "../key"), keyPrefix: "/test/"},
+			wantErr: ErrGenericInvalidKey,
+		},
+		{
+			name:    "path traversal - local",
+			args:    args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "./key"), keyPrefix: "/test/"},
+			wantErr: ErrGenericInvalidKey,
+		},
+		{
+			name:    "path traversal - double parent",
+			args:    args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "./../key"), keyPrefix: "/test/"},
+			wantErr: ErrGenericInvalidKey,
+		},
+		{
+			name:    "path traversal - after parent",
+			args:    args{continueValue: encodeContinueOrDie("meta.k8s.io/v1", 1, "key/../.."), keyPrefix: "/test/"},
+			wantErr: ErrGenericInvalidKey,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			gotFromKey, gotRv, err := DecodeContinue(tt.args.continueValue, tt.args.keyPrefix)
-			if (err != nil) != tt.wantErr {
+			if !errors.Is(err, tt.wantErr) {
 				t.Errorf("decodeContinue() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}