Authorization / Access Control

Prerequisites: A Kubernetes cluster with Grafana provisioning CRDs installed and an admission controller that enforces pending-delete checks (as per the commit). You need a provisioning resource (e.g., a Repository) with the pending-delete label present on the object. 1) Create a pending-delete Repository (pre-fix behavior would allow updates; post-fix behavior blocks updates when both old and new carry the label). Example (using kubectl and assuming the pending-delete label key is the one used by Grafana's admission guard, referenced in code as LabelPendingDelete): # Step 1: Create a repository with the pending-delete label apiVersion: provisioning.grafana.io/v0alpha1 kind: Repository metadata: name: test-repo labels: <pending-delete-label-key>: "true" spec: title: "Test Repo" type: GitHubRepositoryType kubectl apply -f test-repo-pending-delete.yaml # Step 2: Attempt an update while the label remains (modify a field, keep the label) # This simulates an attacker attempting to mutate a resource that is still pending deletion kubectl patch repository test-repo \ -p '{"metadata":{"labels":{"<pending-delete-label-key>":"true"}},"spec":{"title":"Test Repo (Updated)"}}' \ --type=merge # Expected (with fix): 403 Forbidden with message indicating namespace is pending deletion # Expected (without fix): The update may succeed and mutate the resource while pending deletion is in effect # Step 3: Remedy by removing the pending-delete label (explicit unlock) kubectl patch repository test-repo \ -p '{"metadata":{"labels":{"<pending-delete-label-key>":null}},"spec":{"title":"Test Repo (Unlocked)"}}' \ --type=merge # Verification: After removing the label, updates should proceed as allowed

diff --git a/apps/provisioning/pkg/apis/admission/pending_delete.go b/apps/provisioning/pkg/apis/admission/pending_delete.go new file mode 100644 index 0000000000000..2318a7ebbb35f --- /dev/null +++ b/apps/provisioning/pkg/apis/admission/pending_delete.go @@ -0,0 +1,38 @@ +package admission + +import ( + "errors" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apiserver/pkg/admission" + + appcontroller "github.com/grafana/grafana/apps/provisioning/pkg/controller" + "github.com/grafana/grafana/pkg/apimachinery/utils" +) + +// ValidatePendingDeletion blocks mutations on resources whose namespace is pending deletion. +// On Update it returns Forbidden when both the old and new object carry the pending-delete +// label, allowing updates that remove the label (explicit unlock). +// On Create it returns Forbidden when the incoming object already carries the label. +func ValidatePendingDeletion(a admission.Attributes, meta utils.GrafanaMetaAccessor) error { + switch a.GetOperation() { + case admission.Update: + if a.GetSubresource() != "" { + return nil // status (and other subresource) patches are allowed + } + if old := a.GetOldObject(); old != nil { + if oldMeta, err := utils.MetaAccessor(old); err == nil && appcontroller.IsPendingDelete(oldMeta.GetLabels()) { + if appcontroller.IsPendingDelete(meta.GetLabels()) { + return apierrors.NewForbidden(a.GetResource().GroupResource(), a.GetName(), errors.New("namespace is pending deletion")) + } + } + } + case admission.Create: + if appcontroller.IsPendingDelete(meta.GetLabels()) { + return apierrors.NewForbidden(a.GetResource().GroupResource(), a.GetName(), errors.New("namespace is pending deletion")) + } + case admission.Delete, admission.Connect: + // no checks needed + } + return nil +} diff --git a/apps/provisioning/pkg/apis/admission/pending_delete_test.go b/apps/provisioning/pkg/apis/admission/pending_delete_test.go new file mode 100644 index 0000000000000..4e3d5993d8adf --- /dev/null +++ b/apps/provisioning/pkg/apis/admission/pending_delete_test.go @@ -0,0 +1,138 @@ +package admission + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/authentication/user" + + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + appcontroller "github.com/grafana/grafana/apps/provisioning/pkg/controller" + "github.com/grafana/grafana/pkg/apimachinery/utils" +) + +func newPendingDeleteAttributes(obj, old runtime.Object, op admission.Operation, subresource string) admission.Attributes { + return admission.NewAttributesRecord( + obj, + old, + provisioning.RepositoryResourceInfo.GroupVersionKind(), + "default", + "test-repo", + provisioning.SchemeGroupVersion.WithResource("repositories"), + subresource, + op, + nil, + false, + &user.DefaultInfo{}, + ) +} + +func pendingDeleteRepo(withLabel bool) *provisioning.Repository { + labels := map[string]string{} + if withLabel { + labels[appcontroller.LabelPendingDelete] = "true" + } + return &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{Name: "test-repo", Labels: labels}, + } +} + +func TestValidatePendingDeletion(t *testing.T) { + tests := []struct { + name string + obj runtime.Object + old runtime.Object + op admission.Operation + subresource string + wantErr bool + wantForbidden bool + }{ + { + name: "Create without pending-delete label is allowed", + obj: pendingDeleteRepo(false), + op: admission.Create, + wantErr: false, + }, + { + name: "Create with pending-delete label is forbidden", + obj: pendingDeleteRepo(true), + op: admission.Create, + wantErr: true, + wantForbidden: true, + }, + { + name: "Update: old without label, new without label is allowed", + obj: pendingDeleteRepo(false), + old: pendingDeleteRepo(false), + op: admission.Update, + wantErr: false, + }, + { + name: "Update: old with label, new without label is allowed (explicit unlock)", + obj: pendingDeleteRepo(false), + old: pendingDeleteRepo(true), + op: admission.Update, + wantErr: false, + }, + { + name: "Update: old without label, new with label is allowed (label being set)", + obj: pendingDeleteRepo(true), + old: pendingDeleteRepo(false), + op: admission.Update, + wantErr: false, + }, + { + name: "Update: both old and new have pending-delete label is forbidden", + obj: pendingDeleteRepo(true), + old: pendingDeleteRepo(true), + op: admission.Update, + wantErr: true, + wantForbidden: true, + }, + { + name: "Update status subresource: both have pending-delete label is allowed", + obj: pendingDeleteRepo(true), + old: pendingDeleteRepo(true), + op: admission.Update, + subresource: "status", + wantErr: false, + }, + { + name: "Update status subresource: old with label, new with label is allowed", + obj: pendingDeleteRepo(true), + old: pendingDeleteRepo(true), + op: admission.Update, + subresource: "status", + wantErr: false, + }, + { + name: "Delete operation is not checked", + obj: pendingDeleteRepo(true), + op: admission.Delete, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + attr := newPendingDeleteAttributes(tt.obj, tt.old, tt.op, tt.subresource) + meta, err := utils.MetaAccessor(tt.obj) + require.NoError(t, err) + + gotErr := ValidatePendingDeletion(attr, meta) + + if tt.wantErr { + require.Error(t, gotErr) + if tt.wantForbidden { + assert.Contains(t, gotErr.Error(), "namespace is pending deletion") + } + } else { + assert.NoError(t, gotErr) + } + }) + } +} diff --git a/apps/provisioning/pkg/connection/validator.go b/apps/provisioning/pkg/connection/validator.go index c9e67a2a06d34..46c319e71b881 100644 --- a/apps/provisioning/pkg/connection/validator.go +++ b/apps/provisioning/pkg/connection/validator.go @@ -9,6 +9,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/admission" + provisioningadmission "github.com/grafana/grafana/apps/provisioning/pkg/apis/admission" provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/apimachinery/utils" ) @@ -50,6 +51,12 @@ func (v *AdmissionValidator) Validate(ctx context.Context, a admission.Attribute return nil } + // Block creations of pending-deleted resources and mutations on resources whose namespace is pending deletion. + // Allows for updates that remove the pending-delete label (explicit unlock). + if err := provisioningadmission.ValidatePendingDeletion(a, meta); err != nil { + return err + } + c, ok := obj.(*provisioning.Connection) if !ok { return fmt.Errorf("expected connection configuration, got %T", obj) diff --git a/apps/provisioning/pkg/connection/validator_test.go b/apps/provisioning/pkg/connection/validator_test.go index aa599c11170a8..4a6beaecb604a 100644 --- a/apps/provisioning/pkg/connection/validator_test.go +++ b/apps/provisioning/pkg/connection/validator_test.go @@ -16,6 +16,7 @@ import ( "k8s.io/apiserver/pkg/authentication/user" provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + appcontroller "github.com/grafana/grafana/apps/provisioning/pkg/controller" common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" ) @@ -181,6 +182,90 @@ func TestAdmissionValidator_Validate(t *testing.T) { factoryErrors: field.ErrorList{}, wantErr: false, }, + { + name: "blocks UPDATE when both old and new objects have the pending-delete label", + obj: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Labels: map[string]string{appcontroller.LabelPendingDelete: "true"}, + }, + Spec: provisioning.ConnectionSpec{ + Title: "Test Connection (modified)", + Type: provisioning.GithubConnectionType, + }, + }, + old: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Labels: map[string]string{appcontroller.LabelPendingDelete: "true"}, + }, + Spec: provisioning.ConnectionSpec{ + Title: "Test Connection", + Type: provisioning.GithubConnectionType, + }, + }, + operation: admission.Update, + wantErr: true, + wantErrContains: "namespace is pending deletion", + }, + { + name: "allows UPDATE that removes the pending-delete label (explicit unlock)", + obj: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + Spec: provisioning.ConnectionSpec{ + Title: "Test Connection", + Type: provisioning.GithubConnectionType, + }, + }, + old: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Labels: map[string]string{appcontroller.LabelPendingDelete: "true"}, + }, + Spec: provisioning.ConnectionSpec{ + Title: "Test Connection", + Type: provisioning.GithubConnectionType, + }, + }, + operation: admission.Update, + factoryErrors: field.ErrorList{}, + wantErr: false, + }, + { + name: "allows the UPDATE that sets the pending-delete label (old without label → new with label)", + obj: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Labels: map[string]string{appcontroller.LabelPendingDelete: "true"}, + }, + Spec: provisioning.ConnectionSpec{ + Title: "Test Connection", + Type: provisioning.GithubConnectionType, + }, + }, + old: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + }, + operation: admission.Update, + factoryErrors: field.ErrorList{}, + wantErr: false, + }, + { + name: "blocks CREATE when incoming object carries the pending-delete label", + obj: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Labels: map[string]string{appcontroller.LabelPendingDelete: "true"}, + }, + Spec: provisioning.ConnectionSpec{ + Title: "Test Connection", + Type: provisioning.GithubConnectionType, + }, + }, + operation: admission.Create, + wantErr: true, + wantErrContains: "namespace is pending deletion", + }, } for _, tt := range tests { diff --git a/apps/provisioning/pkg/repository/validator.go b/apps/provisioning/pkg/repository/validator.go index 0f1161e2f53cc..903a0c03f7bbf 100644 --- a/apps/provisioning/pkg/repository/validator.go +++ b/apps/provisioning/pkg/repository/validator.go @@ -13,6 +13,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/admission" + provisioningadmission "github.com/grafana/grafana/apps/provisioning/pkg/apis/admission" provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/apimachinery/utils" ) @@ -225,6 +226,12 @@ func (v *AdmissionValidator) Validate(ctx context.Context, a admission.Attribute return nil } + // Block creations of pending-deleted resources and mutations on resources whose namespace is pending deletion. + // Allows for updates that remove the pending-delete label (explicit unlock). + if err := provisioningadmission.ValidatePendingDeletion(a, meta); err != nil { + return err + } + r, ok := obj.(*provisioning.Repository) if !ok { return fmt.Errorf("expected repository configuration, got %T", obj) diff --git a/apps/provisioning/pkg/repository/validator_test.go b/apps/provisioning/pkg/repository/validator_test.go index a31419685ae58..eca4dde0df4ed 100644 --- a/apps/provisioning/pkg/repository/validator_test.go +++ b/apps/provisioning/pkg/repository/validator_test.go @@ -15,6 +15,7 @@ import ( "k8s.io/apiserver/pkg/authentication/user" provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + appcontroller "github.com/grafana/grafana/apps/provisioning/pkg/controller" common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" ) @@ -541,6 +542,106 @@ func TestAdmissionValidator_Validate(t *testing.T) { wantErr: true, wantErrContains: "Changing sync target after running sync is not supported", }, + { + name: "blocks UPDATE when both old and new objects have the pending-delete label", + obj: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Finalizers: []string{CleanFinalizer}, + Labels: map[string]string{appcontroller.LabelPendingDelete: "true"}, + }, + Spec: provisioning.RepositorySpec{ + Title: "Test Repo (modified)", + Type: provisioning.GitHubRepositoryType, + }, + }, + old: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Labels: map[string]string{appcontroller.LabelPendingDelete: "true"}, + }, + Spec: provisioning.RepositorySpec{ + Title: "Test Repo", + Type: provisioning.GitHubRepositoryType, + }, + }, + operation: admission.Update, + wantErr: true, + wantErrContains: "namespace is pending deletion", + }, + { + name: "allows UPDATE that removes the pending-delete label (explicit unlock)", + obj: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Finalizers: []string{CleanFinalizer}, + }, + Spec: provisioning.RepositorySpec{ + Title: "Test Repo", + Type: provisioning.GitHubRepositoryType, + Sync: provisioning.SyncOptions{IntervalSeconds: 60}, + }, + }, + old: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Finalizers: []string{CleanFinalizer}, + Labels: map[string]string{appcontroller.LabelPendingDelete: "true"}, + }, + Spec: provisioning.RepositorySpec{ + Title: "Test Repo", + Type: provisioning.GitHubRepositoryType, + Sync: provisioning.SyncOptions{IntervalSeconds: 60}, + }, + }, + operation: admission.Update, + wantErr: false, + }, + { + name: "allows the UPDATE that sets the pending-delete label (old without label → new with label)", + obj: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Finalizers: []string{CleanFinalizer}, + Labels: map[string]string{appcontroller.LabelPendingDelete: "true"}, + }, + Spec: provisioning.RepositorySpec{ + Title: "Test Repo", + Type: provisioning.GitHubRepositoryType, + Sync: provisioning.SyncOptions{IntervalSeconds: 60}, + }, + }, + old: &provisioning.Reposit ... [truncated]