github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-30 15:05:27 +00:00
Code Issues Projects Releases Wiki Activity

Object creation with generateName should return a proper error

Browse Source 
Signed-off-by: Vince Prignano <vincepri@vmware.com>
This commit is contained in:
Vince Prignano 2021-08-31 18:04:59 -07:00
parent eae38bbe9e
commit 8a9d61278f
6 changed files with 52 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
pkg/registry/core/service/storage/rest.go
View File

@ -247,7 +247,7 @@ func (rs *REST) Create(ctx context.Context, obj runtime.Object, createValidation
out, err := rs.services.Create(ctx, service, createValidation, options)
if err != nil {
err = rest.CheckGeneratedNameError(rs.strategy, err, service)
err = rest.CheckGeneratedNameError(ctx, rs.strategy, err, service)
}
if err == nil {

10
pkg/registry/core/service/strategy_test.go
View File

@ -48,19 +48,23 @@ func newStrategy(cidr string, hasSecondary bool) (testStrategy Strategy, testSta
}
func TestCheckGeneratedNameError(t *testing.T) {
ctx := genericapirequest.WithRequestInfo(genericapirequest.NewContext(), &genericapirequest.RequestInfo{
Resource: "foos",
})
testStrategy, _ := newStrategy("10.0.0.0/16", false)
expect := errors.NewNotFound(api.Resource("foos"), "bar")
if err := rest.CheckGeneratedNameError(testStrategy, expect, &api.Service{}); err != expect {
if err := rest.CheckGeneratedNameError(ctx, testStrategy, expect, &api.Service{}); err != expect {
t.Errorf("NotFoundError should be ignored: %v", err)
}
expect = errors.NewAlreadyExists(api.Resource("foos"), "bar")
if err := rest.CheckGeneratedNameError(testStrategy, expect, &api.Service{}); err != expect {
if err := rest.CheckGeneratedNameError(ctx, testStrategy, expect, &api.Service{}); err != expect {
t.Errorf("AlreadyExists should be returned when no GenerateName field: %v", err)
}
expect = errors.NewAlreadyExists(api.Resource("foos"), "bar")
if err := rest.CheckGeneratedNameError(testStrategy, expect, &api.Service{ObjectMeta: metav1.ObjectMeta{GenerateName: "foo"}}); err == nil || !errors.IsServerTimeout(err) {
if err := rest.CheckGeneratedNameError(ctx, testStrategy, expect, &api.Service{ObjectMeta: metav1.ObjectMeta{GenerateName: "foo"}}); err == nil || !errors.IsAlreadyExists(err) {
t.Errorf("expected try again later error: %v", err)
}
}

19
staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go
View File

@ -170,6 +170,25 @@ func NewAlreadyExists(qualifiedResource schema.GroupResource, name string) *Stat
}}
}
// NewGenerateNameConflict returns an error indicating the server
// was not able to generate a valid name for a resource.
func NewGenerateNameConflict(qualifiedResource schema.GroupResource, name string, retryAfterSeconds int) *StatusError {
return &StatusError{metav1.Status{
Status: metav1.StatusFailure,
Code: http.StatusConflict,
Reason: metav1.StatusReasonAlreadyExists,
Details: &metav1.StatusDetails{
Group: qualifiedResource.Group,
Kind: qualifiedResource.Resource,
Name: name,
RetryAfterSeconds: int32(retryAfterSeconds),
},
Message: fmt.Sprintf(
"%s %q already exists, the server was not able to generate a unique name for the object",
qualifiedResource.String(), name),
}}
}
// NewUnauthorized returns an error indicating the client is not authorized to perform the requested
// action.
func NewUnauthorized(reason string) *StatusError {

9
staging/src/k8s.io/apimachinery/pkg/api/errors/errors_test.go
View File

@ -64,7 +64,7 @@ func TestErrorNew(t *testing.T) {
}
if !IsConflict(NewConflict(resource("tests"), "2", errors.New("message"))) {
t.Errorf("expected to be conflict")
t.Errorf("expected to be %s", metav1.StatusReasonAlreadyExists)
}
if !IsNotFound(NewNotFound(resource("tests"), "3")) {
t.Errorf("expected to be %s", metav1.StatusReasonNotFound)
@ -88,6 +88,13 @@ func TestErrorNew(t *testing.T) {
t.Errorf("expected to be %s", metav1.StatusReasonMethodNotAllowed)
}
if !IsAlreadyExists(NewGenerateNameConflict(resource("tests"), "3", 1)) {
t.Errorf("expected to be %s", metav1.StatusReasonAlreadyExists)
}
if time, ok := SuggestsClientDelay(NewGenerateNameConflict(resource("tests"), "3", 1)); time != 1 || !ok {
t.Errorf("unexpected %d", time)
}
if time, ok := SuggestsClientDelay(NewServerTimeout(resource("tests"), "doing something", 10)); time != 10 || !ok {
t.Errorf("unexpected %d", time)
}

4
staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go
View File

@ -404,7 +404,7 @@ func (e *Store) Create(ctx context.Context, obj runtime.Object, createValidation
out := e.NewFunc()
if err := e.Storage.Create(ctx, key, obj, out, ttl, dryrun.IsDryRun(options.DryRun)); err != nil {
err = storeerr.InterpretCreateError(err, qualifiedResource, name)
err = rest.CheckGeneratedNameError(e.CreateStrategy, err, obj)
err = rest.CheckGeneratedNameError(ctx, e.CreateStrategy, err, obj)
if !apierrors.IsAlreadyExists(err) {
return nil, err
}
@ -659,7 +659,7 @@ func (e *Store) Update(ctx context.Context, name string, objInfo rest.UpdatedObj
}
if creating {
err = storeerr.InterpretCreateError(err, qualifiedResource, name)
err = rest.CheckGeneratedNameError(e.CreateStrategy, err, creatingObj)
err = rest.CheckGeneratedNameError(ctx, e.CreateStrategy, err, creatingObj)
} else {
err = storeerr.InterpretUpdateError(err, qualifiedResource, name)
}

18
staging/src/k8s.io/apiserver/pkg/registry/rest/create.go
View File

@ -28,6 +28,7 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/admission"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/features"
"k8s.io/apiserver/pkg/storage/names"
utilfeature "k8s.io/apiserver/pkg/util/feature"
@ -109,6 +110,7 @@ func BeforeCreate(strategy RESTCreateStrategy, ctx context.Context, obj runtime.
objectMeta.SetDeletionGracePeriodSeconds(nil)
strategy.PrepareForCreate(ctx, obj)
FillObjectMetaSystemFields(objectMeta)
if len(objectMeta.GetGenerateName()) > 0 && len(objectMeta.GetName()) == 0 {
objectMeta.SetName(strategy.GenerateName(objectMeta.GetGenerateName()))
}
@ -145,21 +147,31 @@ func BeforeCreate(strategy RESTCreateStrategy, ctx context.Context, obj runtime.
// CheckGeneratedNameError checks whether an error that occurred creating a resource is due
// to generation being unable to pick a valid name.
func CheckGeneratedNameError(strategy RESTCreateStrategy, err error, obj runtime.Object) error {
func CheckGeneratedNameError(ctx context.Context, strategy RESTCreateStrategy, err error, obj runtime.Object) error {
if !errors.IsAlreadyExists(err) {
return err
}
objectMeta, kind, kerr := objectMetaAndKind(strategy, obj)
objectMeta, gvk, kerr := objectMetaAndKind(strategy, obj)
if kerr != nil {
return kerr
}
if len(objectMeta.GetGenerateName()) == 0 {
// If we don't have a generated name, return the original error (AlreadyExists).
// When we're here, the user picked a name that is causing a conflict.
return err
}
return errors.NewServerTimeoutForKind(kind.GroupKind(), "POST", 0)
// Get the group resource information from the context, if populated.
gr := schema.GroupResource{}
if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found {
gr = schema.GroupResource{Group: gvk.Group, Resource: requestInfo.Resource}
}
// If we have a name and generated name, the server picked a name
// that already exists.
return errors.NewGenerateNameConflict(gr, objectMeta.GetName(), 1)
}
// objectMetaAndKind retrieves kind and ObjectMeta from a runtime object, or returns an error.