fix: add admission control to block finalizers removal not made by the operator service account #2575
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| package admission | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "net/http" | ||
|
|
||
| admissionv1 "k8s.io/api/admission/v1" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| ctrl "sigs.k8s.io/controller-runtime" | ||
| "sigs.k8s.io/controller-runtime/pkg/webhook" | ||
| "sigs.k8s.io/controller-runtime/pkg/webhook/admission" | ||
|
|
||
| "github.com/weka/weka-operator/internal/config" | ||
| "github.com/weka/weka-operator/internal/consts" | ||
| ) | ||
|
|
||
| // FinalizerProtectionHandler is a raw admission handler that blocks | ||
| // removal of protected finalizers by non-operator service accounts. | ||
| // Unlike other validators, it has NO objectSelector escape hatch. | ||
| type FinalizerProtectionHandler struct { | ||
| decoder admission.Decoder | ||
| } | ||
|
|
||
| // protectedFinalizers is the set of finalizers that may only be removed | ||
| // by the operator's own service account. | ||
| var protectedFinalizers = []string{ | ||
| consts.WekaFinalizer, | ||
| } | ||
|
|
||
| func (h *FinalizerProtectionHandler) Handle(_ context.Context, req admission.Request) admission.Response { | ||
| // Only guard UPDATE; allow everything else unconditionally. | ||
| if req.Operation != admissionv1.Update { | ||
| return admission.Allowed("") | ||
| } | ||
|
|
||
| oldMeta := &metav1.PartialObjectMetadata{} | ||
| if err := h.decoder.DecodeRaw(req.OldObject, oldMeta); err != nil { | ||
| return admission.Errored(http.StatusBadRequest, | ||
| fmt.Errorf("failed to decode old object metadata: %w", err)) | ||
| } | ||
|
|
||
| newMeta := &metav1.PartialObjectMetadata{} | ||
| if err := h.decoder.DecodeRaw(req.Object, newMeta); err != nil { | ||
| return admission.Errored(http.StatusBadRequest, | ||
| fmt.Errorf("failed to decode new object metadata: %w", err)) | ||
| } | ||
|
|
||
| removed := removedFinalizers(oldMeta.Finalizers, newMeta.Finalizers) | ||
| if len(removed) == 0 { | ||
| return admission.Allowed("") | ||
| } | ||
|
|
||
| // Check whether the caller is the operator's own SA. | ||
| if config.Config.OperatorPodNamespace == "" || config.Config.OperatorServiceAccountName == "" { | ||
| return admission.Denied("operator namespace or service account name is not configured; cannot verify caller identity") | ||
| } | ||
| expected := fmt.Sprintf("system:serviceaccount:%s:%s", | ||
| config.Config.OperatorPodNamespace, config.Config.OperatorServiceAccountName) | ||
| if req.UserInfo.Username == expected { | ||
| return admission.Allowed("") | ||
| } | ||
|
|
||
| // Non-operator caller removed a protected finalizer — deny. | ||
| return admission.Denied(fmt.Sprintf( | ||
| "removal of finalizer(s) %v is not allowed; they are managed by the weka-operator", | ||
| removed)) | ||
| } | ||
|
|
||
| // removedFinalizers returns protected finalizers present in old but absent in new. | ||
| func removedFinalizers(old, current []string) []string { | ||
| newSet := make(map[string]struct{}, len(current)) | ||
| for _, f := range current { | ||
| newSet[f] = struct{}{} | ||
| } | ||
|
|
||
| var removed []string | ||
| for _, f := range old { | ||
| if _, ok := newSet[f]; ok { | ||
| continue | ||
| } | ||
| for _, pf := range protectedFinalizers { | ||
| if f == pf { | ||
| removed = append(removed, f) | ||
| break | ||
| } | ||
| } | ||
| } | ||
| return removed | ||
| } | ||
|
|
||
| // RegisterFinalizerProtectionWebhook registers the finalizer-protection | ||
| // admission handler with the manager's webhook server. | ||
| func RegisterFinalizerProtectionWebhook(mgr ctrl.Manager) { | ||
| mgr.GetWebhookServer().Register(FinalizerProtectionWebhookPath, &webhook.Admission{ | ||
| Handler: &FinalizerProtectionHandler{ | ||
| decoder: admission.NewDecoder(mgr.GetScheme()), | ||
| }, | ||
| }) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug — bypass is too permissive. This prefix check matches any service account in the operator's namespace, not just the operator's own SA. The operator namespace also hosts (at minimum)
{{prefix}}-maintenance(seecharts/weka-operator/templates/maintenance_service_account.yaml), the CSI controller SA, and the CSI node SA. Anything running under those identities could remove the protected finalizers and defeat the guard.Match the exact operator SA instead, e.g.:
(
OperatorServiceAccountNamewould need to be plumbed through env the same wayMaintenanceSaNamealready is — seeinternal/config/env.go:392.)