github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-29 14:37:00 +00:00
Code Issues Projects Releases Wiki Activity

Unify ListOptions validation between cache and etcd

Browse Source
This commit is contained in:
Marek Siarkowicz 2025-03-07 12:26:05 +01:00
parent 1c9840c58e
commit ccb607f06b
4 changed files with 200 additions and 174 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go
View File

@ -33,7 +33,6 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/conversion"
"k8s.io/apimachinery/pkg/fields"
@ -645,47 +644,6 @@ func (s *store) ReadinessCheck() error {
return nil
}
// resolveGetListRev is used by GetList to resolve the rev to use in the client.KV.Get request.
func (s *store) resolveGetListRev(continueKey string, continueRV int64, opts storage.ListOptions) (int64, error) {
var withRev int64
// Uses continueRV if this is a continuation request.
if len(continueKey) > 0 {
if len(opts.ResourceVersion) > 0 && opts.ResourceVersion != "0" {
return withRev, apierrors.NewBadRequest("specifying resource version is not allowed when using continue")
}
// If continueRV > 0, the LIST request needs a specific resource version.
// continueRV==0 is invalid.
// If continueRV < 0, the request is for the latest resource version.
if continueRV > 0 {
withRev = continueRV
}
return withRev, nil
}
// Returns 0 if ResourceVersion is not specified.
if len(opts.ResourceVersion) == 0 {
return withRev, nil
}
parsedRV, err := s.versioner.ParseResourceVersion(opts.ResourceVersion)
if err != nil {
return withRev, apierrors.NewBadRequest(fmt.Sprintf("invalid resource version: %v", err))
}
switch opts.ResourceVersionMatch {
case metav1.ResourceVersionMatchNotOlderThan:
// The not older than constraint is checked after we get a response from etcd,
// and returnedRV is then set to the revision we get from the etcd response.
case metav1.ResourceVersionMatchExact:
withRev = int64(parsedRV)
case "": // legacy case
if opts.Recursive && opts.Predicate.Limit > 0 && parsedRV > 0 {
withRev = int64(parsedRV)
}
default:
return withRev, fmt.Errorf("unknown ResourceVersionMatch value: %v", opts.ResourceVersionMatch)
}
return withRev, nil
}
func (s *store) GetCurrentResourceVersion(ctx context.Context) (uint64, error) {
emptyList := s.newListFunc()
pred := storage.SelectionPredicate{
@ -753,15 +711,8 @@ func (s *store) GetList(ctx context.Context, key string, opts storage.ListOption
paging := opts.Predicate.Limit > 0
newItemFunc := getNewItemFunc(listObj, v)
var continueRV, withRev int64
var continueKey string
if opts.Recursive && len(opts.Predicate.Continue) > 0 {
continueKey, continueRV, err = storage.DecodeContinue(opts.Predicate.Continue, keyPrefix)
if err != nil {
return apierrors.NewBadRequest(fmt.Sprintf("invalid continue token: %v", err))
}
}
if withRev, err = s.resolveGetListRev(continueKey, continueRV, opts); err != nil {
withRev, continueKey, err := storage.ValidateListOptions(keyPrefix, s.versioner, opts)
if err != nil {
return err
}

123
staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go
View File

@ -696,129 +696,6 @@ func TestInvalidKeys(t *testing.T) {
expectInvalidKey("Count", countErr)
}
func TestResolveGetListRev(t *testing.T) {
_, store, _ := testSetup(t)
testCases := []struct {
name string
continueKey string
continueRV int64
rv string
rvMatch metav1.ResourceVersionMatch
recursive bool
expectedError string
limit int64
expectedRev int64
}{
{
name: "specifying resource versionwhen using continue",
continueKey: "continue",
continueRV: 100,
rv: "200",
expectedError: "specifying resource version is not allowed when using continue",
},
{
name: "invalid resource version",
rv: "invalid",
expectedError: "invalid resource version",
},
{
name: "unknown ResourceVersionMatch value",
rv: "200",
rvMatch: "unknown",
expectedError: "unknown ResourceVersionMatch value",
},
{
name: "use continueRV",
continueKey: "continue",
continueRV: 100,
rv: "0",
expectedRev: 100,
},
{
name: "use continueRV with empty rv",
continueKey: "continue",
continueRV: 100,
rv: "",
expectedRev: 100,
},
{
name: "continueRV = 0",
continueKey: "continue",
continueRV: 0,
rv: "",
expectedRev: 0,
},
{
name: "continueRV < 0",
continueKey: "continue",
continueRV: -1,
rv: "",
expectedRev: 0,
},
{
name: "default",
expectedRev: 0,
},
{
name: "rev resolve to 0 if ResourceVersionMatchNotOlderThan",
rv: "200",
rvMatch: metav1.ResourceVersionMatchNotOlderThan,
expectedRev: 0,
},
{
name: "specified rev if ResourceVersionMatchExact",
rv: "200",
rvMatch: metav1.ResourceVersionMatchExact,
expectedRev: 200,
},
{
name: "rev resolve to 0 if not recursive",
rv: "200",
limit: 1,
expectedRev: 0,
},
{
name: "rev resolve to 0 if limit unspecified",
rv: "200",
recursive: true,
expectedRev: 0,
},
{
name: "specified rev if recursive with limit",
rv: "200",
recursive: true,
limit: 1,
expectedRev: 200,
},
}
for _, tt := range testCases {
tt := tt
t.Run(tt.name, func(t *testing.T) {
storageOpts := storage.ListOptions{
ResourceVersion: tt.rv,
ResourceVersionMatch: tt.rvMatch,
Predicate: storage.SelectionPredicate{
Limit: tt.limit,
},
Recursive: tt.recursive,
}
rev, err := store.resolveGetListRev(tt.continueKey, tt.continueRV, storageOpts)
if len(tt.expectedError) > 0 {
if err == nil || !strings.Contains(err.Error(), tt.expectedError) {
t.Fatalf("expected error: %s, but got: %v", tt.expectedError, err)
}
return
}
if err != nil {
t.Fatalf("resolveRevForGetList failed: %v", err)
}
if rev != tt.expectedRev {
t.Errorf("%s: expecting rev = %d, but get %d", tt.name, tt.expectedRev, rev)
}
})
}
}
func BenchmarkStore_GetList(b *testing.B) {
generateBigPod := func(index int, total int, expect int) runtime.Object {
l := map[string]string{}

41
staging/src/k8s.io/apiserver/pkg/storage/interfaces.go
View File

@ -20,6 +20,7 @@ import (
"context"
"fmt"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
@ -329,3 +330,43 @@ type DeleteOptions struct {
// object which otherwise can not be deleted using the normal flow
IgnoreStoreReadError bool
}
func ValidateListOptions(keyPrefix string, versioner Versioner, opts ListOptions) (withRev int64, continueKey string, err error) {
if opts.Recursive && len(opts.Predicate.Continue) > 0 {
continueKey, continueRV, err := DecodeContinue(opts.Predicate.Continue, keyPrefix)
if err != nil {
return 0, "", apierrors.NewBadRequest(fmt.Sprintf("invalid continue token: %v", err))
}
if len(opts.ResourceVersion) > 0 && opts.ResourceVersion != "0" {
return 0, "", apierrors.NewBadRequest("specifying resource version is not allowed when using continue")
}
// If continueRV > 0, the LIST request needs a specific resource version.
// continueRV==0 is invalid.
// If continueRV < 0, the request is for the latest resource version.
if continueRV > 0 {
withRev = continueRV
}
return withRev, continueKey, nil
}
if len(opts.ResourceVersion) == 0 {
return withRev, "", nil
}
parsedRV, err := versioner.ParseResourceVersion(opts.ResourceVersion)
if err != nil {
return withRev, "", apierrors.NewBadRequest(fmt.Sprintf("invalid resource version: %v", err))
}
switch opts.ResourceVersionMatch {
case metav1.ResourceVersionMatchNotOlderThan:
// The not older than constraint is checked after we get a response from etcd,
// and returnedRV is then set to the revision we get from the etcd response.
case metav1.ResourceVersionMatchExact:
withRev = int64(parsedRV)
case "": // legacy case
if opts.Recursive && opts.Predicate.Limit > 0 && parsedRV > 0 {
withRev = int64(parsedRV)
}
default:
return withRev, "", fmt.Errorf("unknown ResourceVersionMatch value: %v", opts.ResourceVersionMatch)
}
return withRev, "", nil
}

157
staging/src/k8s.io/apiserver/pkg/storage/interfaces_test.go
View File

@ -20,6 +20,8 @@ import (
"errors"
"strings"
"testing"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
func TestPreconditionsCheckWithNilObject(t *testing.T) {
@ -37,3 +39,158 @@ func TestPreconditionsCheckWithNilObject(t *testing.T) {
t.Errorf("expected error to contain %q", want)
}
}
func TestValidateListOptions(t *testing.T) {
testCases := []struct {
name string
opts ListOptions
expectedError string
expectedRev int64
expectedContinueKey string
}{
{
name: "specifying resource version when using continue",
opts: ListOptions{
Recursive: true,
ResourceVersion: "200",
Predicate: SelectionPredicate{
Continue: encodeContinueOrDie("meta.k8s.io/v1", 100, "continue"),
},
},
expectedError: "specifying resource version is not allowed when using continue",
},
{
name: "invalid resource version",
opts: ListOptions{
ResourceVersion: "invalid",
},
expectedError: "invalid resource version",
},
{
name: "unknown ResourceVersionMatch value",
opts: ListOptions{
ResourceVersion: "200",
ResourceVersionMatch: "unknown",
},
expectedError: "unknown ResourceVersionMatch value",
},
{
name: "use continueRV",
opts: ListOptions{
ResourceVersion: "0",
Recursive: true,
Predicate: SelectionPredicate{
Continue: encodeContinueOrDie("meta.k8s.io/v1", 100, "continue"),
},
},
expectedRev: 100,
expectedContinueKey: "continue",
},
{
name: "use continueRV with empty rv",
opts: ListOptions{
ResourceVersion: "",
Recursive: true,
Predicate: SelectionPredicate{
Continue: encodeContinueOrDie("meta.k8s.io/v1", 100, "continue"),
},
},
expectedRev: 100,
expectedContinueKey: "continue",
},
{
name: "continueRV = 0",
opts: ListOptions{
ResourceVersion: "",
Recursive: true,
Predicate: SelectionPredicate{
Continue: encodeContinueOrDie("meta.k8s.io/v1", 0, "continue"),
},
},
expectedError: "invalid continue token",
},
{
name: "continueRV < 0",
opts: ListOptions{
ResourceVersion: "",
Recursive: true,
Predicate: SelectionPredicate{
Continue: encodeContinueOrDie("meta.k8s.io/v1", -1, "continue"),
},
},
expectedRev: 0,
expectedContinueKey: "continue",
},
{
name: "default",
opts: ListOptions{},
expectedRev: 0,
},
{
name: "rev resolve to 0 if ResourceVersionMatchNotOlderThan",
opts: ListOptions{
ResourceVersion: "200",
ResourceVersionMatch: metav1.ResourceVersionMatchNotOlderThan,
},
expectedRev: 0,
},
{
name: "specified rev if ResourceVersionMatchExact",
opts: ListOptions{
ResourceVersion: "200",
ResourceVersionMatch: metav1.ResourceVersionMatchExact,
},
expectedRev: 200,
},
{
name: "rev resolve to 0 if not recursive",
opts: ListOptions{
ResourceVersion: "200",
Predicate: SelectionPredicate{
Limit: 1,
},
},
expectedRev: 0,
},
{
name: "rev resolve to 0 if limit unspecified",
opts: ListOptions{
ResourceVersion: "200",
Recursive: true,
},
expectedRev: 0,
},
{
name: "specified rev if recursive with limit",
opts: ListOptions{
ResourceVersion: "200",
Recursive: true,
Predicate: SelectionPredicate{
Limit: 1,
},
},
expectedRev: 200,
},
}
for _, tt := range testCases {
tt := tt
t.Run(tt.name, func(t *testing.T) {
withRev, continueKey, err := ValidateListOptions("", APIObjectVersioner{}, tt.opts)
if len(tt.expectedError) > 0 {
if err == nil || !strings.Contains(err.Error(), tt.expectedError) {
t.Fatalf("expected error: %s, but got: %v", tt.expectedError, err)
}
return
}
if err != nil {
t.Fatalf("resolveRevForGetList failed: %v", err)
}
if withRev != tt.expectedRev {
t.Errorf("%s: expecting rev = %d, but get %d", tt.name, tt.expectedRev, withRev)
}
if continueKey != tt.expectedContinueKey {
t.Errorf("%s: expecting continueKey = %q, but get %q", tt.name, tt.expectedContinueKey, continueKey)
}
})
}
}