Skip to content

Commit 61529d5

Browse files
authored
Merge pull request kubernetes#130595 from tkashem/omit-admission
KEP-3926: skip admission validation for unsafe delete
2 parents d367e0b + ef3cb5c commit 61529d5

File tree

2 files changed

+52
-2
lines changed

2 files changed

+52
-2
lines changed

staging/src/k8s.io/apiserver/pkg/registry/generic/registry/corrupt_obj_deleter.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,14 @@ func (d *corruptObjectDeleter) Delete(ctx context.Context, name string, deleteVa
107107
klog.FromContext(ctx).V(1).Info("Going to perform unsafe object deletion", "object", klog.KRef(genericapirequest.NamespaceValue(ctx), name))
108108
out := d.store.NewFunc()
109109
storageOpts := storage.DeleteOptions{IgnoreStoreReadError: true}
110-
// dropping preconditions, and keeping the admission
111-
if err := storageBackend.Delete(ctx, key, out, nil, storage.ValidateObjectFunc(deleteValidation), nil, storageOpts); err != nil {
110+
// we don't have the old object in the cache, neither can it be
111+
// retrieved from the storage and decoded into an object
112+
// successfully, so we do the following:
113+
// a) skip preconditions check
114+
// b) skip admission validation, rest.ValidateAllObjectFunc will "admit everything"
115+
var nilPreconditions *storage.Preconditions = nil
116+
var nilCachedExistingObject runtime.Object = nil
117+
if err := storageBackend.Delete(ctx, key, out, nilPreconditions, rest.ValidateAllObjectFunc, nilCachedExistingObject, storageOpts); err != nil {
112118
if storage.IsNotFound(err) {
113119
// the DELETE succeeded, but we don't have the object since it's
114120
// not retrievable from the storage, so we send a nil object

staging/src/k8s.io/apiserver/pkg/registry/generic/registry/corrupt_obj_deleter_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,50 @@ func TestUnsafeDeleteWithUnexpectedError(t *testing.T) {
267267
}
268268
}
269269

270+
func TestUnsafeDeleteWithAdmissionShouldBeSkipped(t *testing.T) {
271+
ctx := genericapirequest.WithNamespace(genericapirequest.NewContext(), "test")
272+
destroyFunc, registry := NewTestGenericStoreRegistry(t)
273+
defer destroyFunc()
274+
275+
// a) create the target object
276+
object := &example.Pod{
277+
ObjectMeta: metav1.ObjectMeta{Name: "foo"},
278+
Spec: example.PodSpec{NodeName: "machine"},
279+
}
280+
_, err := registry.Create(ctx, object, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
281+
if err != nil {
282+
t.Fatalf("Unexpected error: %v", err)
283+
}
284+
285+
// b) wrap the storage layer to return corrupt object error
286+
registry.Storage.Storage = &corruptStorage{
287+
Interface: registry.Storage.Storage,
288+
err: storage.NewCorruptObjError("key", fmt.Errorf("untransformable")),
289+
}
290+
291+
// c) set up a corrupt object deleter for the registry
292+
deleter := NewCorruptObjectDeleter(registry)
293+
294+
// d) try unsafe delete, but pass a validation that always fails
295+
var admissionInvoked int
296+
_, deleted, err := deleter.Delete(ctx, object.Name, func(_ context.Context, _ runtime.Object) error {
297+
admissionInvoked++
298+
return fmt.Errorf("admission was not skipped")
299+
}, &metav1.DeleteOptions{
300+
IgnoreStoreReadErrorWithClusterBreakingPotential: ptr.To[bool](true),
301+
})
302+
303+
if err != nil {
304+
t.Errorf("Unexpected error from Delete: %v", err)
305+
}
306+
if want, got := true, deleted; want != got {
307+
t.Errorf("Expected deleted: %t, but got: %t", want, got)
308+
}
309+
if want, got := 0, admissionInvoked; want != got {
310+
t.Errorf("Expected admission to be invoked %d time(s), but got: %d", want, got)
311+
}
312+
}
313+
270314
type corruptStorage struct {
271315
storage.Interface
272316
err error

0 commit comments

Comments
 (0)