Skip to content

Commit

Permalink
Merge pull request #127513 from tkashem/delete-undecryptable
Browse files Browse the repository at this point in the history
KEP-3926: unsafe deletion of corrupt objects

Kubernetes-commit: 4d10ae8fdc579e2bb09789507cae7b8d32cbd306
  • Loading branch information
k8s-publishing-bot committed Nov 8, 2024
2 parents f983148 + fbb5ab0 commit 0b01a72
Show file tree
Hide file tree
Showing 21 changed files with 1,371 additions and 29 deletions.
6 changes: 3 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ require (
gopkg.in/evanphx/json-patch.v4 v4.12.0
gopkg.in/natefinch/lumberjack.v2 v2.2.1
gopkg.in/square/go-jose.v2 v2.6.0
k8s.io/api v0.0.0-20241108114313-789a813a3da8
k8s.io/apimachinery v0.0.0-20241106231735-d941d9fb4c83
k8s.io/client-go v0.0.0-20241108115821-fe3db7fea609
k8s.io/api v0.0.0-20241108114314-0869e9d258da
k8s.io/apimachinery v0.0.0-20241108022104-96b97de8d6ba
k8s.io/client-go v0.0.0-20241108115824-fcfb2ba0165b
k8s.io/component-base v0.0.0-20241108123300-35b74786c418
k8s.io/klog/v2 v2.130.1
k8s.io/kms v0.0.0-20241107031913-7a7a59ea9c74
Expand Down
12 changes: 6 additions & 6 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -365,12 +365,12 @@ gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
k8s.io/api v0.0.0-20241108114313-789a813a3da8 h1:+3HQBAIjBgEx+fcUE7qou+b97GTD0FwsRvdivPD4Fk8=
k8s.io/api v0.0.0-20241108114313-789a813a3da8/go.mod h1:h7yaPC7+0KxMELdLjLoo6n6m3EWq6AeHEY25PjH4cPI=
k8s.io/apimachinery v0.0.0-20241106231735-d941d9fb4c83 h1:4KfMPmiiRIpvYJQ8cBYFEFht59EKysW1anuJWzHLHNg=
k8s.io/apimachinery v0.0.0-20241106231735-d941d9fb4c83/go.mod h1:HqhdaJUgQqky29T1V0o2yFkt/pZqLFIDyn9Zi/8rxoY=
k8s.io/client-go v0.0.0-20241108115821-fe3db7fea609 h1:AcsofjKUH8NSsiWP98f+aiOqWGNSNI4UY80QwXRjftU=
k8s.io/client-go v0.0.0-20241108115821-fe3db7fea609/go.mod h1:tGXgc1bj5IDFTTFfLPieQm+C+mO7h5UaU7CFVds/P6M=
k8s.io/api v0.0.0-20241108114314-0869e9d258da h1:0IBD100isGkMfDCeLrXtOqsmKKeiy3Uj85sI50Aff7U=
k8s.io/api v0.0.0-20241108114314-0869e9d258da/go.mod h1:jw6pQTESH9mdZL2vOK3twojvpPxipl5TpLZpPyl5ZYU=
k8s.io/apimachinery v0.0.0-20241108022104-96b97de8d6ba h1:ghB5Iygt6Ge8UyIwW7C1kJx4kP7AUTCL9Qg6GCsUUOY=
k8s.io/apimachinery v0.0.0-20241108022104-96b97de8d6ba/go.mod h1:HqhdaJUgQqky29T1V0o2yFkt/pZqLFIDyn9Zi/8rxoY=
k8s.io/client-go v0.0.0-20241108115824-fcfb2ba0165b h1:lS1fax+8b6ixc5eVrH2wdX0ZsJPL3aUgi5/S4WlESgI=
k8s.io/client-go v0.0.0-20241108115824-fcfb2ba0165b/go.mod h1:luyD5FCao3u3FeN1L5HS36g83otuzs6cdlUNsYdQ6dw=
k8s.io/component-base v0.0.0-20241108123300-35b74786c418 h1:QwfJ5xnagesBw1NsW9GQ1egcdSfNsWOyPb/2RkdZUic=
k8s.io/component-base v0.0.0-20241108123300-35b74786c418/go.mod h1:xoGVbsAaeyCMTUz4Ck1TA3Tz7SSE9Suryx+nCTkuChM=
k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk=
Expand Down
129 changes: 127 additions & 2 deletions pkg/endpoints/handlers/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,14 @@ import (
metainternalversionvalidation "k8s.io/apimachinery/pkg/apis/meta/internalversion/validation"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/validation"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/admission"
"k8s.io/apiserver/pkg/audit"
"k8s.io/apiserver/pkg/authorization/authorizer"
"k8s.io/apiserver/pkg/endpoints/handlers/finisher"
requestmetrics "k8s.io/apiserver/pkg/endpoints/handlers/metrics"
"k8s.io/apiserver/pkg/endpoints/handlers/negotiation"
Expand All @@ -44,6 +48,9 @@ import (
"k8s.io/apiserver/pkg/util/dryrun"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/component-base/tracing"

"k8s.io/klog/v2"
"k8s.io/utils/ptr"
)

// DeleteResource returns a function that will handle a resource deletion
Expand Down Expand Up @@ -116,17 +123,46 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope *RequestSc
}
}
}
if !utilfeature.DefaultFeatureGate.Enabled(features.AllowUnsafeMalformedObjectDeletion) && options != nil {
options.IgnoreStoreReadErrorWithClusterBreakingPotential = nil
}
if errs := validation.ValidateDeleteOptions(options); len(errs) > 0 {
err := errors.NewInvalid(schema.GroupKind{Group: metav1.GroupName, Kind: "DeleteOptions"}, "", errs)
scope.err(err, w, req)
return
}
options.TypeMeta.SetGroupVersionKind(metav1.SchemeGroupVersion.WithKind("DeleteOptions"))

span.AddEvent("About to delete object from database")
wasDeleted := true
userInfo, _ := request.UserFrom(ctx)
staticAdmissionAttrs := admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Delete, options, dryrun.IsDryRun(options.DryRun), userInfo)

if utilfeature.DefaultFeatureGate.Enabled(features.AllowUnsafeMalformedObjectDeletion) {
if options != nil && ptr.Deref(options.IgnoreStoreReadErrorWithClusterBreakingPotential, false) {
// let's make sure that the audit will reflect that this delete request
// was tried with ignoreStoreReadErrorWithClusterBreakingPotential enabled
audit.AddAuditAnnotation(ctx, "apiserver.k8s.io/unsafe-delete-ignore-read-error", "")

p, ok := r.(rest.CorruptObjectDeleterProvider)
if !ok || p.GetCorruptObjDeleter() == nil {
// this is a developer error
scope.err(errors.NewInternalError(fmt.Errorf("no unsafe deleter provided, can not honor ignoreStoreReadErrorWithClusterBreakingPotential")), w, req)
return
}
if scope.Authorizer == nil {
scope.err(errors.NewInternalError(fmt.Errorf("no authorizer provided, unable to authorize unsafe delete")), w, req)
return
}
if err := authorizeUnsafeDelete(ctx, staticAdmissionAttrs, scope.Authorizer); err != nil {
scope.err(err, w, req)
return
}

r = p.GetCorruptObjDeleter()
}
}

span.AddEvent("About to delete object from database")
wasDeleted := true
result, err := finisher.FinishRequest(ctx, func() (runtime.Object, error) {
obj, deleted, err := r.Delete(ctx, name, rest.AdmissionToValidateObjectDeleteFunc(admit, staticAdmissionAttrs, scope), options)
wasDeleted = deleted
Expand Down Expand Up @@ -260,11 +296,26 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope *RequestSc
}
}
}
if !utilfeature.DefaultFeatureGate.Enabled(features.AllowUnsafeMalformedObjectDeletion) && options != nil {
options.IgnoreStoreReadErrorWithClusterBreakingPotential = nil
}
if errs := validation.ValidateDeleteOptions(options); len(errs) > 0 {
err := errors.NewInvalid(schema.GroupKind{Group: metav1.GroupName, Kind: "DeleteOptions"}, "", errs)
scope.err(err, w, req)
return
}

if utilfeature.DefaultFeatureGate.Enabled(features.AllowUnsafeMalformedObjectDeletion) {
if options != nil && ptr.Deref(options.IgnoreStoreReadErrorWithClusterBreakingPotential, true) {
fieldErrList := field.ErrorList{
field.Invalid(field.NewPath("ignoreStoreReadErrorWithClusterBreakingPotential"), true, "is not allowed with DELETECOLLECTION, try again after removing the option"),
}
err := errors.NewInvalid(schema.GroupKind{Group: metav1.GroupName, Kind: "DeleteOptions"}, "", fieldErrList)
scope.err(err, w, req)
return
}
}

options.TypeMeta.SetGroupVersionKind(metav1.SchemeGroupVersion.WithKind("DeleteOptions"))

admit = admission.WithAudit(admit)
Expand Down Expand Up @@ -295,3 +346,77 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope *RequestSc
transformResponseObject(ctx, scope, req, w, http.StatusOK, outputMediaType, result)
}
}

// authorizeUnsafeDelete ensures that the user has permission to do
// 'unsafe-delete-ignore-read-errors' on the resource being deleted when
// ignoreStoreReadErrorWithClusterBreakingPotential is enabled
func authorizeUnsafeDelete(ctx context.Context, attr admission.Attributes, authz authorizer.Authorizer) (err error) {
if attr.GetOperation() != admission.Delete || attr.GetOperationOptions() == nil {
return nil
}
options, ok := attr.GetOperationOptions().(*metav1.DeleteOptions)
if !ok {
return errors.NewInternalError(fmt.Errorf("expected an option of type: %T, but got: %T", &metav1.DeleteOptions{}, attr.GetOperationOptions()))
}
if !ptr.Deref(options.IgnoreStoreReadErrorWithClusterBreakingPotential, false) {
return nil
}

requestInfo, found := request.RequestInfoFrom(ctx)
if !found {
return admission.NewForbidden(attr, fmt.Errorf("no RequestInfo found in the context"))
}
if !requestInfo.IsResourceRequest || len(attr.GetSubresource()) > 0 {
return admission.NewForbidden(attr, fmt.Errorf("ignoreStoreReadErrorWithClusterBreakingPotential delete option is not allowed on a subresource or non-resource request"))
}

// if we are here, IgnoreStoreReadErrorWithClusterBreakingPotential
// is set to true in the delete options, the user must have permission
// to do 'unsafe-delete-ignore-read-errors' on the given resource.
record := authorizer.AttributesRecord{
User: attr.GetUserInfo(),
Verb: "unsafe-delete-ignore-read-errors",
Namespace: attr.GetNamespace(),
Name: attr.GetName(),
APIGroup: attr.GetResource().Group,
APIVersion: attr.GetResource().Version,
Resource: attr.GetResource().Resource,
ResourceRequest: true,
}
// TODO: can't use ResourceAttributesFrom from k8s.io/kubernetes/pkg/registry/authorization/util
// due to prevent staging --> k8s.io/kubernetes dep issue
if utilfeature.DefaultFeatureGate.Enabled(features.AuthorizeWithSelectors) {
if len(requestInfo.FieldSelector) > 0 {
fieldSelector, err := fields.ParseSelector(requestInfo.FieldSelector)
if err != nil {
record.FieldSelectorRequirements, record.FieldSelectorParsingErr = nil, err
} else {
if requirements := fieldSelector.Requirements(); len(requirements) > 0 {
record.FieldSelectorRequirements, record.FieldSelectorParsingErr = fieldSelector.Requirements(), nil
}
}
}
if len(requestInfo.LabelSelector) > 0 {
labelSelector, err := labels.Parse(requestInfo.LabelSelector)
if err != nil {
record.LabelSelectorRequirements, record.LabelSelectorParsingErr = nil, err
} else {
if requirements, _ /*selectable*/ := labelSelector.Requirements(); len(requirements) > 0 {
record.LabelSelectorRequirements, record.LabelSelectorParsingErr = requirements, nil
}
}
}
}

decision, reason, err := authz.Authorize(ctx, record)
if err != nil {
err = fmt.Errorf("error while checking permission for %q, %w", record.Verb, err)
klog.FromContext(ctx).V(1).Error(err, "failed to authorize")
return admission.NewForbidden(attr, err)
}
if decision == authorizer.DecisionAllow {
return nil
}

return admission.NewForbidden(attr, fmt.Errorf("not permitted to do %q, reason: %s", record.Verb, reason))
}
Loading

0 comments on commit 0b01a72

Please sign in to comment.