Trash: Keep provisioned objects out of the trash (#111714)

This commit is contained in:
Stephanie Hingtgen
2025-09-28 20:33:57 +00:00
committed by GitHub
parent 5869fcb97a
commit e01e61a156
4 changed files with 131 additions and 13 deletions
+5
View File
@@ -1097,6 +1097,11 @@ func (s *server) isTrashItemAuthorized(ctx context.Context, iter ListIterator, t
return false
}
// provisioned objects should not be retrievable in the trash
if obj.GetAnnotation(utils.AnnoKeyManagerKind) != "" {
return false
}
// Trash is only accessible to admins or the user who deleted the object
return obj.GetUpdatedBy() == user.GetUID() || trashChecker(iter.Name(), iter.Folder())
}
+19 -12
View File
@@ -889,12 +889,13 @@ func (k *kvStorageBackend) processTrashEntries(ctx context.Context, req *resourc
pagedKeys := applyPagination(filteredKeys, lastSeenRV, sortAscending)
iter := kvHistoryIterator{
keys: pagedKeys,
currentIndex: -1,
ctx: ctx,
listRV: listRV,
sortAscending: sortAscending,
dataStore: k.dataStore,
keys: pagedKeys,
currentIndex: -1,
ctx: ctx,
listRV: listRV,
sortAscending: sortAscending,
dataStore: k.dataStore,
skipProvisioned: true,
}
err = fn(&iter)
@@ -907,12 +908,13 @@ func (k *kvStorageBackend) processTrashEntries(ctx context.Context, req *resourc
// kvHistoryIterator implements ListIterator for KV storage history
type kvHistoryIterator struct {
ctx context.Context
keys []DataKey
currentIndex int
listRV int64
sortAscending bool
dataStore *dataStore
ctx context.Context
keys []DataKey
currentIndex int
listRV int64
sortAscending bool
skipProvisioned bool
dataStore *dataStore
// current
rv int64
@@ -962,6 +964,11 @@ func (i *kvHistoryIterator) Next() bool {
i.folder = meta.GetFolder()
i.err = nil
// if the resource is provisioned and we are skipping provisioned resources, continue onto the next one
if i.skipProvisioned && meta.GetAnnotation(utils.AnnoKeyManagerKind) != "" {
return i.Next()
}
return true
}
@@ -1041,6 +1041,36 @@ func TestKvStorageBackend_ListTrash_Success(t *testing.T) {
rv2, err := backend.WriteEvent(ctx, writeEvent)
require.NoError(t, err)
// Do the same for a provisioned object
provisionedObj, err := createTestObjectWithName("provisioned-obj", appsNamespace, "test-data")
require.NoError(t, err)
metaAccessorProvisioned, err := utils.MetaAccessor(provisionedObj)
require.NoError(t, err)
metaAccessorProvisioned.SetAnnotation(utils.AnnoKeyManagerKind, "repo")
writeEventProvisioned := WriteEvent{
Type: resourcepb.WatchEvent_ADDED,
Key: &resourcepb.ResourceKey{
Namespace: "default",
Group: "apps",
Resource: "resources",
Name: "provisioned-obj",
},
Value: objectToJSONBytes(t, provisionedObj),
Object: metaAccessorProvisioned,
PreviousRV: 0,
}
rv3, err := backend.WriteEvent(ctx, writeEventProvisioned)
require.NoError(t, err)
writeEventProvisioned.Type = resourcepb.WatchEvent_DELETED
writeEventProvisioned.PreviousRV = rv3
writeEventProvisioned.Object = metaAccessorProvisioned
writeEventProvisioned.ObjectOld = metaAccessorProvisioned
_, err = backend.WriteEvent(ctx, writeEventProvisioned)
require.NoError(t, err)
// List the trash (deleted items)
listReq := &resourcepb.ListRequest{
Options: &resourcepb.ListOptions{
@@ -1081,7 +1111,7 @@ func TestKvStorageBackend_ListTrash_Success(t *testing.T) {
require.NoError(t, err)
require.Greater(t, rv, int64(0))
require.Len(t, trashItems, 1) // Should have the deleted item
require.Len(t, trashItems, 1) // Should have the non-provisioned deleted item
// Verify the trash item
require.Equal(t, "test-resource", trashItems[0].name)
@@ -83,6 +83,11 @@ func TestIntegrationDashboardAPIValidation(t *testing.T) {
org1Ctx := createTestContext(t, helper, helper.Org1, dualWriterMode)
// trash is supported through unified storage only
if dualWriterMode == rest.Mode5 {
runDashboardTrashTests(t, org1Ctx)
}
t.Run("Dashboard validation tests", func(t *testing.T) {
runDashboardValidationTests(t, org1Ctx)
})
@@ -2551,3 +2556,74 @@ func postHelper(t *testing.T, ctx *TestContext, path string, body interface{}, u
return result, nil
}
func runDashboardTrashTests(t *testing.T, ctx TestContext) {
t.Helper()
adminClient := getResourceClient(t, ctx.Helper, ctx.AdminUser, getDashboardGVR())
editorClient := getResourceClient(t, ctx.Helper, ctx.EditorUser, getDashboardGVR())
viewerClient := getResourceClient(t, ctx.Helper, ctx.ViewerUser, getDashboardGVR())
t.Run("regular dashboards appear in trash but provisioned ones do not", func(t *testing.T) {
// create two dashboards, one that is provisioned and one that is not
regularDash, err := createDashboard(t, adminClient, "Regular Dashboard for Trash Comparison", nil, nil)
require.NoError(t, err)
regularDashUID := regularDash.GetName()
provisionedDash, err := createDashboard(t, adminClient, "Provisioned Dashboard for Trash Comparison", nil, nil)
require.NoError(t, err)
provisionedDashUID := provisionedDash.GetName()
meta, err := utils.MetaAccessor(provisionedDash)
require.NoError(t, err)
meta.SetAnnotation(utils.AnnoKeyManagerKind, "repo")
meta.SetManagerProperties(utils.ManagerProperties{
Kind: utils.ManagerKindTerraform,
})
updatedProvisionedDash, err := adminClient.Resource.Update(context.Background(), provisionedDash, v1.UpdateOptions{})
require.NoError(t, err)
require.NotNil(t, updatedProvisionedDash)
// delete both dashboards
err = adminClient.Resource.Delete(context.Background(), regularDashUID, v1.DeleteOptions{})
require.NoError(t, err)
err = adminClient.Resource.Delete(context.Background(), provisionedDashUID, v1.DeleteOptions{})
require.NoError(t, err)
// trash should only contain the regular dashboard
trashList, err := adminClient.Resource.List(context.Background(), v1.ListOptions{
LabelSelector: utils.LabelKeyGetTrash + "=true",
})
require.NoError(t, err)
require.Len(t, trashList.Items, 1, "Trash should only contain the regular dashboard")
require.Equal(t, regularDashUID, trashList.Items[0].GetName(), "Trash should only contain the regular dashboard")
})
t.Run("permission checks - admin can see everything, users can see their own deleted items", func(t *testing.T) {
dash, err := createDashboard(t, editorClient, "Dashboard for Trash Test", nil, nil)
require.NoError(t, err)
dashUID := dash.GetName()
err = editorClient.Resource.Delete(context.Background(), dashUID, v1.DeleteOptions{})
require.NoError(t, err)
// although editor deleted it, admin can still see it
trashList, err := adminClient.Resource.List(context.Background(), v1.ListOptions{
LabelSelector: utils.LabelKeyGetTrash + "=true",
})
require.NoError(t, err)
require.GreaterOrEqual(t, len(trashList.Items), 1, "Admin should see deleted dashboards in trash")
require.Equal(t, dashUID, trashList.Items[0].GetName(), "Admin should find the deleted dashboard in trash")
// editor can see
editorTrashList, err := editorClient.Resource.List(context.Background(), v1.ListOptions{
LabelSelector: utils.LabelKeyGetTrash + "=true",
})
require.NoError(t, err)
require.GreaterOrEqual(t, len(editorTrashList.Items), 1, "Admin should see deleted dashboards in trash")
require.Equal(t, dashUID, editorTrashList.Items[0].GetName(), "Admin should find the deleted dashboard in trash")
// viewer should not see the editor's deleted dashboard
viewerTrashList, err := viewerClient.Resource.List(context.Background(), v1.ListOptions{
LabelSelector: utils.LabelKeyGetTrash + "=true",
})
require.NoError(t, err)
require.Len(t, viewerTrashList.Items, 0, "Viewer should not see any trash items")
})
}