Merge branch 'evictorero/snapshtos-mt-cleanup-expired' into evictorero/snapshots-dual-write-support

pkg/registry/apis/dashboard/register.go

@@ -122,6 +122,7 @@ type DashboardsAPIBuilder struct {
 	publicDashboardService       publicdashboards.Service
 	snapshotService              dashboardsnapshots.Service
 	snapshotOptions              dashv0.SnapshotSharingOptions
+	snapshotStorage              rest.Storage // for dual-write support in routes
 	namespacer                   request.NamespaceMapper
 	dashboardActivityChannel     live.DashboardActivityChannel
 	isStandalone                 bool // skips any handling including anything to do with legacy storage
@@ -747,15 +748,26 @@ func (b *DashboardsAPIBuilder) storageForVersion(
 		}
 	}
 
-	// Legacy only (for now) and only v0alpha1
+	// Snapshots - only v0alpha1
 	if snapshots != nil && dashboards.GroupVersion().Version == "v0alpha1" {
 		snapshotLegacyStore := &snapshot.SnapshotLegacyStore{
 			ResourceInfo: *snapshots,
 			Service:      b.snapshotService,
 			Namespacer:   b.namespacer,
 		}
-		storage[snapshots.StoragePath()] = snapshotLegacyStore
-		storage[snapshots.StoragePath("dashboard")], err = snapshot.NewDashboardREST(dashboards, b.snapshotService)
+
+		unifiedSnapshotStore, err := grafanaregistry.NewRegistryStore(opts.Scheme, *snapshots, opts.OptsGetter)
+		if err != nil {
+			return err
+		}
+		snapshotGr := snapshots.GroupResource()
+		snapshotDualWrite, err := opts.DualWriteBuilder(snapshotGr, snapshotLegacyStore, unifiedSnapshotStore)
+		if err != nil {
+			return err
+		}
+		storage[snapshots.StoragePath()] = snapshotDualWrite
+		b.snapshotStorage = snapshotDualWrite // store for use in routes
+		storage[snapshots.StoragePath("dashboard")], err = snapshot.NewDashboardREST(snapshotDualWrite)
 		if err != nil {
 			return err
 		}
@@ -979,7 +991,9 @@ func (b *DashboardsAPIBuilder) GetAPIRoutes(gv schema.GroupVersion) *builder.API
 
 	defs := b.GetOpenAPIDefinitions()(func(path string) spec.Ref { return spec.Ref{} })
 	searchAPIRoutes := b.search.GetAPIRoutes(defs)
-	snapshotAPIRoutes := snapshot.GetRoutes(b.snapshotService, b.snapshotOptions, defs)
+	snapshotAPIRoutes := snapshot.GetRoutes(b.snapshotService, b.snapshotOptions, defs, func() rest.Storage {
+		return b.snapshotStorage
+	})
 
 	return &builder.APIRoutes{
 		Namespace: append(searchAPIRoutes.Namespace, snapshotAPIRoutes.Namespace...),

pkg/registry/apis/dashboard/snapshot/conversions.go

@@ -7,6 +7,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	dashV0 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1"
+	common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1"
 	"github.com/grafana/grafana/pkg/apimachinery/utils"
 	"github.com/grafana/grafana/pkg/services/apiserver/endpoints/request"
 	"github.com/grafana/grafana/pkg/services/dashboardsnapshots"
@@ -59,7 +60,10 @@ func convertSnapshotToK8sResource(v *dashboardsnapshots.DashboardSnapshot, names
 			Namespace:         namespacer(v.OrgID),
 		},
 		Spec: dashV0.SnapshotSpec{
-			Title: &v.Name,
+			Title:       &v.Name,
+			Expires:     &expires,
+			External:    &v.External,
+			ExternalUrl: &v.ExternalURL,
 		},
 	}
 
@@ -78,3 +82,68 @@ func convertSnapshotToK8sResource(v *dashboardsnapshots.DashboardSnapshot, names
 	}
 	return snap
 }
+
+// convertK8sResourceToCreateCommand converts a K8s Snapshot to a CreateDashboardSnapshotCommand
+func convertK8sResourceToCreateCommand(snap *dashV0.Snapshot, orgID int64, userID int64) *dashboardsnapshots.CreateDashboardSnapshotCommand {
+	cmd := &dashboardsnapshots.CreateDashboardSnapshotCommand{
+		OrgID:  orgID,
+		UserID: userID,
+	}
+
+	// Map title
+	if snap.Spec.Title != nil {
+		cmd.Name = *snap.Spec.Title
+	}
+
+	// Map dashboard (convert map[string]interface{} to *common.Unstructured)
+	if snap.Spec.Dashboard != nil {
+		cmd.Dashboard = &common.Unstructured{Object: snap.Spec.Dashboard}
+	}
+
+	// Map expires
+	if snap.Spec.Expires != nil {
+		cmd.Expires = *snap.Spec.Expires
+	}
+
+	// Map external settings
+	if snap.Spec.External != nil && *snap.Spec.External {
+		cmd.External = true
+		if snap.Spec.ExternalUrl != nil {
+			cmd.ExternalURL = *snap.Spec.ExternalUrl
+		}
+	}
+
+	return cmd
+}
+
+// convertCreateCmdToK8sSnapshot converts a CreateDashboardSnapshotCommand request to a K8s Snapshot
+// Used by routes.go to create a Snapshot object from the incoming create command
+func convertCreateCmdToK8sSnapshot(cmd *dashboardsnapshots.CreateDashboardSnapshotCommand, namespace string) *dashV0.Snapshot {
+	snap := &dashV0.Snapshot{
+		TypeMeta: dashV0.SnapshotResourceInfo.TypeMeta(),
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: namespace,
+		},
+		Spec: dashV0.SnapshotSpec{
+			Title: &cmd.Name,
+		},
+	}
+
+	// Convert *common.Unstructured to map[string]interface{}
+	if cmd.Dashboard != nil {
+		snap.Spec.Dashboard = cmd.Dashboard.Object
+	}
+
+	if cmd.Expires > 0 {
+		snap.Spec.Expires = &cmd.Expires
+	}
+
+	if cmd.External {
+		snap.Spec.External = &cmd.External
+		if cmd.ExternalURL != "" {
+			snap.Spec.ExternalUrl = &cmd.ExternalURL
+		}
+	}
+
+	return snap
+}

pkg/registry/apis/dashboard/snapshot/routes.go

@@ -6,7 +6,10 @@ import (
 	"net/http"
 
 	"github.com/gorilla/mux"
+	"github.com/grafana/grafana/pkg/setting"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	k8srequest "k8s.io/apiserver/pkg/endpoints/request"
+	"k8s.io/apiserver/pkg/registry/rest"
 	"k8s.io/kube-openapi/pkg/common"
 	"k8s.io/kube-openapi/pkg/spec3"
 	"k8s.io/kube-openapi/pkg/validation/spec"
@@ -14,6 +17,7 @@ import (
 	authlib "github.com/grafana/authlib/types"
 	dashv0 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1"
 	"github.com/grafana/grafana/pkg/apimachinery/identity"
+	"github.com/grafana/grafana/pkg/apimachinery/utils"
 	"github.com/grafana/grafana/pkg/services/apiserver/builder"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
 	"github.com/grafana/grafana/pkg/services/dashboardsnapshots"
@@ -22,7 +26,7 @@ import (
 	"github.com/grafana/grafana/pkg/web"
 )
 
-func GetRoutes(service dashboardsnapshots.Service, options dashv0.SnapshotSharingOptions, defs map[string]common.OpenAPIDefinition) *builder.APIRoutes {
+func GetRoutes(service dashboardsnapshots.Service, options dashv0.SnapshotSharingOptions, defs map[string]common.OpenAPIDefinition, storageGetter func() rest.Storage) *builder.APIRoutes {
 	prefix := dashv0.SnapshotResourceInfo.GroupResource().Resource
 	tags := []string{dashv0.SnapshotResourceInfo.GroupVersionKind().Kind}
 
@@ -97,9 +101,10 @@ func GetRoutes(service dashboardsnapshots.Service, options dashv0.SnapshotSharin
 					},
 				},
 				Handler: func(w http.ResponseWriter, r *http.Request) {
-					user, err := identity.GetRequester(r.Context())
+					ctx := r.Context()
+					user, err := identity.GetRequester(ctx)
 					if err != nil {
-						errhttp.Write(r.Context(), err, w)
+						errhttp.Write(ctx, err, w)
 						return
 					}
 					wrap := &contextmodel.ReqContext{
@@ -107,11 +112,15 @@ func GetRoutes(service dashboardsnapshots.Service, options dashv0.SnapshotSharin
 							Req:  r,
 							Resp: web.NewResponseWriter(r.Method, w),
 						},
-						// SignedInUser: user, ????????????
 					}
 
+					if !options.SnapshotsEnabled {
+						wrap.JsonApiErr(http.StatusForbidden, "Dashboard Snapshots are disabled", nil)
+						return
+					}
 					vars := mux.Vars(r)
-					info, err := authlib.ParseNamespace(vars["namespace"])
+					namespace := vars["namespace"]
+					info, err := authlib.ParseNamespace(namespace)
 					if err != nil {
 						wrap.JsonApiErr(http.StatusBadRequest, "expected namespace", nil)
 						return
@@ -128,8 +137,82 @@ func GetRoutes(service dashboardsnapshots.Service, options dashv0.SnapshotSharin
 						return
 					}
 
-					// Use the existing snapshot service
-					dashboardsnapshots.CreateDashboardSnapshot(wrap, options, cmd, service)
+					if cmd.External && !options.ExternalEnabled {
+						wrap.JsonApiErr(http.StatusForbidden, "External dashboard creation is disabled", nil)
+						return
+					}
+
+					// fill cmd data
+					if cmd.Name == "" {
+						cmd.Name = "Unnamed snapshot"
+					}
+					cmd.OrgID = user.GetOrgID()
+					cmd.UserID, _ = identity.UserIdentifier(user.GetID())
+
+					//originalDashboardURL, err := dashboardsnapshots.CreateOriginalDashboardURL(&cmd)
+
+					// TODO: add logic for external and internal snapshots
+					if cmd.External {
+						// TODO: if it is an external dashboard make a POST to the public snapshot server
+					} else {
+
+					}
+
+					// TODO: validate dashboard exists. Need to call dashboards api, Maybe in a validation hook?
+
+					storage := storageGetter()
+					if storage == nil {
+						errhttp.Write(ctx, fmt.Errorf("snapshot storage not available"), w)
+						return
+					}
+					creater, ok := storage.(rest.Creater)
+					if !ok {
+						errhttp.Write(ctx, fmt.Errorf("snapshot storage does not support create"), w)
+						return
+					}
+
+					// Convert command to K8s Snapshot
+					snapshot := convertCreateCmdToK8sSnapshot(&cmd, namespace)
+
+					snapshot.SetGenerateName("snapshot-")
+
+					// Set namespace in context for k8s storage layer
+					ctx = k8srequest.WithNamespace(ctx, namespace)
+
+					// Create via storage (dual-write mode decides legacy, unified, or both)
+					result, err := creater.Create(ctx, snapshot, nil, &metav1.CreateOptions{})
+					if err != nil {
+						errhttp.Write(ctx, err, w)
+						return
+					}
+
+					// Extract key and deleteKey from result
+					accessor, err := utils.MetaAccessor(result)
+					if err != nil {
+						errhttp.Write(ctx, fmt.Errorf("failed to access result metadata: %w", err), w)
+						return
+					}
+
+					deleteKey, err := util.GetRandomString(32)
+					if err != nil {
+						errhttp.Write(ctx, fmt.Errorf("failed to generate delete key: %w", err), w)
+					}
+
+					key := accessor.GetName()
+					//deleteKey := ""
+					//if annotations := accessor.GetAnnotations(); annotations != nil {
+					//	deleteKey = annotations["grafana.app/delete-key"]
+					//}
+
+					// Build response
+					response := dashv0.DashboardCreateResponse{
+						Key:       key,
+						DeleteKey: deleteKey,
+						URL:       setting.ToAbsUrl("dashboard/snapshot/" + key),
+						DeleteURL: setting.ToAbsUrl("api/snapshots-delete/" + deleteKey),
+					}
+
+					wrap.JSON(http.StatusOK, response)
 				},
 			},
 			{

pkg/registry/apis/dashboard/snapshot/snapshot_legacy_store.go

@@ -2,6 +2,7 @@ package snapshot
 
 import (
 	"context"
+	"fmt"
 
 	"k8s.io/apimachinery/pkg/apis/meta/internalversion"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -20,7 +21,10 @@ var (
 	_ rest.SingularNameProvider = (*SnapshotLegacyStore)(nil)
 	_ rest.Getter               = (*SnapshotLegacyStore)(nil)
 	_ rest.Lister               = (*SnapshotLegacyStore)(nil)
+	_ rest.Creater              = (*SnapshotLegacyStore)(nil)
+	_ rest.Updater              = (*SnapshotLegacyStore)(nil)
 	_ rest.GracefulDeleter      = (*SnapshotLegacyStore)(nil)
+	_ rest.CollectionDeleter    = (*SnapshotLegacyStore)(nil)
 	_ rest.Storage              = (*SnapshotLegacyStore)(nil)
 )
 
@@ -129,3 +133,51 @@ func (s *SnapshotLegacyStore) Get(ctx context.Context, name string, options *met
 	}
 	return nil, s.ResourceInfo.NewNotFound(name)
 }
+
+// Create implements rest.Creater
+func (s *SnapshotLegacyStore) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
+	snap, ok := obj.(*dashV0.Snapshot)
+	if !ok {
+		return nil, fmt.Errorf("expected Snapshot object, got %T", obj)
+	}
+
+	// Run validation if provided
+	if createValidation != nil {
+		if err := createValidation(ctx, obj); err != nil {
+			return nil, err
+		}
+	}
+
+	// Get user identity from context
+	requester, err := identity.GetRequester(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get requester: %w", err)
+	}
+
+	userID, err := requester.GetInternalID()
+	if err != nil {
+		return nil, fmt.Errorf("failed to get user ID: %w", err)
+	}
+
+	// Convert K8s resource to service command
+	cmd := convertK8sResourceToCreateCommand(snap, requester.GetOrgID(), userID)
+
+	// Create the snapshot via service
+	result, err := s.Service.CreateDashboardSnapshot(ctx, cmd)
+	if err != nil {
+		return nil, err
+	}
+
+	// Convert result back to K8s resource
+	return convertSnapshotToK8sResource(result, s.Namespacer), nil
+}
+
+// Update implements rest.Updater - snapshots are immutable, so this returns an error
+func (s *SnapshotLegacyStore) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) {
+	return nil, false, fmt.Errorf("snapshots are immutable and cannot be updated")
+}
+
+// DeleteCollection implements rest.CollectionDeleter
+func (s *SnapshotLegacyStore) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *internalversion.ListOptions) (runtime.Object, error) {
+	return nil, fmt.Errorf("delete collection is not supported for snapshots")
+}

pkg/registry/apis/dashboard/snapshot/sub_dashboard.go

@@ -2,6 +2,7 @@ package snapshot
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -10,22 +11,19 @@ import (
 
 	dashv0 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1"
 	"github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1"
-	"github.com/grafana/grafana/pkg/apimachinery/utils"
 	"github.com/grafana/grafana/pkg/services/apiserver/endpoints/request"
-	"github.com/grafana/grafana/pkg/services/dashboardsnapshots"
 )
 
 // Currently only works with v0alpha1
 type dashboardREST struct {
-	Service dashboardsnapshots.Service
+	getter rest.Getter
 }
 
 func NewDashboardREST(
-	resourceInfo utils.ResourceInfo,
-	service dashboardsnapshots.Service,
+	getter rest.Getter,
 ) (rest.Storage, error) {
 	return &dashboardREST{
-		Service: service,
+		getter: getter,
 	}, nil
 }
 
@@ -58,22 +56,30 @@ func (r *dashboardREST) ProducesObject(verb string) interface{} {
 }
 
 func (r *dashboardREST) Connect(ctx context.Context, name string, opts runtime.Object, responder rest.Responder) (http.Handler, error) {
-	_, err := request.NamespaceInfoFrom(ctx, true)
+	ns, err := request.NamespaceInfoFrom(ctx, true)
 	if err != nil {
 		return nil, err
 	}
-	snap, err := r.Service.GetDashboardSnapshot(ctx, &dashboardsnapshots.GetDashboardSnapshotQuery{Key: name})
+
+	// Get the snapshot from unified storage
+	obj, err := r.getter.Get(ctx, name, &metav1.GetOptions{})
 	if err != nil {
 		return nil, err
 	}
+
+	snap, ok := obj.(*dashv0.Snapshot)
+	if !ok {
+		return nil, fmt.Errorf("expected Snapshot, got %T", obj)
+	}
+
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 		// TODO... support conversions (not required in v0)
 		dash := &dashv0.Dashboard{
 			ObjectMeta: metav1.ObjectMeta{
-				Namespace: name,
+				Namespace: ns.Value,
 			},
 			Spec: v0alpha1.Unstructured{
-				Object: snap.Dashboard.MustMap(),
+				Object: snap.Spec.Dashboard,
 			},
 		}
 		responder.Object(200, dash)

pkg/services/cleanup/cleanup.go

@@ -15,7 +15,9 @@ import (
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/dynamic"
+	"k8s.io/client-go/rest"
 
+	"github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1"
 	"github.com/grafana/grafana/apps/shorturl/pkg/apis/shorturl/v1beta1"
 	"github.com/grafana/grafana/pkg/apimachinery/identity"
 	"github.com/grafana/grafana/pkg/infra/db"
@@ -61,6 +63,7 @@ type CleanUpService struct {
 	orgService                org.Service
 	teamService               team.Service
 	dataSourceService         datasources.DataSourceService
+	dynamicClientFactory      func(*rest.Config) (dynamic.Interface, error)
 }
 
 func ProvideService(cfg *setting.Cfg, Features featuremgmt.FeatureToggles, serverLockService *serverlock.ServerLockService,
@@ -86,6 +89,9 @@ func ProvideService(cfg *setting.Cfg, Features featuremgmt.FeatureToggles, serve
 		orgService:                orgService,
 		teamService:               teamService,
 		dataSourceService:         dataSourceService,
+		dynamicClientFactory: func(c *rest.Config) (dynamic.Interface, error) {
+			return dynamic.NewForConfig(c)
+		},
 	}
 	return s
 }
@@ -230,14 +236,93 @@ func (srv *CleanUpService) shouldCleanupTempFile(filemtime time.Time, now time.T
 
 func (srv *CleanUpService) deleteExpiredSnapshots(ctx context.Context) {
 	logger := srv.log.FromContext(ctx)
-	cmd := dashboardsnapshots.DeleteExpiredSnapshotsCommand{}
-	if err := srv.dashboardSnapshotService.DeleteExpiredSnapshots(ctx, &cmd); err != nil {
-		logger.Error("Failed to delete expired snapshots", "error", err.Error())
+	//nolint:staticcheck // not yet migrated to OpenFeature
+	if srv.Features.IsEnabledGlobally(featuremgmt.FlagKubernetesSnapshots) {
+		srv.deleteKubernetesExpiredSnapshots(ctx)
 	} else {
-		logger.Debug("Deleted expired snapshots", "rows affected", cmd.DeletedRows)
+		cmd := dashboardsnapshots.DeleteExpiredSnapshotsCommand{}
+		if err := srv.dashboardSnapshotService.DeleteExpiredSnapshots(ctx, &cmd); err != nil {
+			logger.Error("Failed to delete expired snapshots", "error", err.Error())
+		} else {
+			logger.Debug("Deleted expired snapshots", "rows affected", cmd.DeletedRows)
+		}
 	}
 }
 
+func (srv *CleanUpService) deleteKubernetesExpiredSnapshots(ctx context.Context) {
+	logger := srv.log.FromContext(ctx)
+	logger.Debug("Starting deleting expired Kubernetes snapshots")
+
+	// Create the dynamic client for Kubernetes API
+	restConfig, err := srv.clientConfigProvider.GetRestConfig(ctx)
+	if err != nil {
+		logger.Error("Failed to get REST config for Kubernetes client", "error", err.Error())
+		return
+	}
+
+	client, err := srv.dynamicClientFactory(restConfig)
+	if err != nil {
+		logger.Error("Failed to create Kubernetes client", "error", err.Error())
+		return
+	}
+
+	// Set up the GroupVersionResource for snapshots
+	gvr := v0alpha1.SnapshotKind().GroupVersionResource()
+
+	// Expiration time is now
+	expirationTime := time.Now()
+	expirationTimestamp := expirationTime.UnixMilli()
+	deletedCount := 0
+
+	// List and delete expired snapshots across all namespaces
+	orgs, err := srv.orgService.Search(ctx, &org.SearchOrgsQuery{})
+	if err != nil {
+		logger.Error("Failed to list organizations", "error", err.Error())
+		return
+	}
+
+	for _, o := range orgs {
+		ctx, _ := identity.WithServiceIdentity(ctx, o.ID)
+		namespaceMapper := request.GetNamespaceMapper(srv.Cfg)
+		snapshots, err := client.Resource(gvr).Namespace(namespaceMapper(o.ID)).List(ctx, v1.ListOptions{})
+		if err != nil {
+			logger.Error("Failed to list snapshots", "error", err.Error())
+			return
+		}
+		// Check each snapshot for expiration
+		for _, item := range snapshots.Items {
+			// Convert unstructured object to Snapshot struct
+			var snapshot v0alpha1.Snapshot
+			err := runtime.DefaultUnstructuredConverter.FromUnstructured(item.Object, &snapshot)
+			if err != nil {
+				logger.Error("Failed to convert unstructured object to snapshot", "name", item.GetName(), "namespace", item.GetNamespace(), "error", err.Error())
+				continue
+			}
+
+			// Only delete expired snapshots
+			if snapshot.Spec.Expires != nil && *snapshot.Spec.Expires < expirationTimestamp {
+				namespace := snapshot.Namespace
+				name := snapshot.Name
+
+				err := client.Resource(gvr).Namespace(namespace).Delete(ctx, name, v1.DeleteOptions{})
+				if err != nil {
+					// Check if it's a "not found" error, which is expected if the resource was already deleted
+					if k8serrors.IsNotFound(err) {
+						logger.Debug("Snapshot already deleted", "name", name, "namespace", namespace)
+					} else {
+						logger.Error("Failed to delete expired snapshot", "name", name, "namespace", namespace, "error", err.Error())
+					}
+				} else {
+					deletedCount++
+					logger.Debug("Successfully deleted expired snapshot", "name", name, "namespace", namespace, "creationTime", snapshot.CreationTimestamp.Unix(), "expirationTime", expirationTimestamp)
+				}
+			}
+		}
+	}
+
+	logger.Debug("Deleted expired Kubernetes snapshots", "count", deletedCount)
+}
+
 func (srv *CleanUpService) deleteExpiredDashboardVersions(ctx context.Context) {
 	logger := srv.log.FromContext(ctx)
 	cmd := dashver.DeleteExpiredVersionsCommand{}
@@ -318,7 +403,7 @@ func (srv *CleanUpService) deleteStaleKubernetesShortURLs(ctx context.Context) {
 		return
 	}
 
-	client, err := dynamic.NewForConfig(restConfig)
+	client, err := srv.dynamicClientFactory(restConfig)
 	if err != nil {
 		logger.Error("Failed to create Kubernetes client", "error", err.Error())
 		return

pkg/services/cleanup/cleanup_test.go

@@ -1,11 +1,30 @@
 package cleanup
 
 import (
+	"context"
+	"errors"
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/watch"
+	"k8s.io/client-go/dynamic"
+	"k8s.io/client-go/rest"
 
+	"github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1"
+	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/services/apiserver"
+	"github.com/grafana/grafana/pkg/services/dashboardsnapshots"
+	"github.com/grafana/grafana/pkg/services/featuremgmt"
+	"github.com/grafana/grafana/pkg/services/org"
+	"github.com/grafana/grafana/pkg/services/org/orgtest"
 	"github.com/grafana/grafana/pkg/setting"
 )
 
@@ -36,3 +55,223 @@ func TestCleanUpTmpFiles(t *testing.T) {
 		require.False(t, service.shouldCleanupTempFile(weekAgo, now))
 	})
 }
+
+func TestDeleteExpiredSnapshots_LegacyMode(t *testing.T) {
+	t.Run("calls DeleteExpiredSnapshots on success", func(t *testing.T) {
+		mockSnapService := dashboardsnapshots.NewMockService(t)
+		mockSnapService.On("DeleteExpiredSnapshots", mock.Anything, mock.Anything).Return(nil)
+
+		service := &CleanUpService{
+			log:                      log.New("cleanup"),
+			Features:                 featuremgmt.WithFeatures(),
+			dashboardSnapshotService: mockSnapService,
+		}
+
+		service.deleteExpiredSnapshots(context.Background())
+
+		mockSnapService.AssertCalled(t, "DeleteExpiredSnapshots", mock.Anything, mock.Anything)
+	})
+
+	t.Run("handles error gracefully", func(t *testing.T) {
+		mockSnapService := dashboardsnapshots.NewMockService(t)
+		mockSnapService.On("DeleteExpiredSnapshots", mock.Anything, mock.Anything).Return(errors.New("db error"))
+
+		service := &CleanUpService{
+			log:                      log.New("cleanup"),
+			Features:                 featuremgmt.WithFeatures(),
+			dashboardSnapshotService: mockSnapService,
+		}
+
+		// Should not panic
+		service.deleteExpiredSnapshots(context.Background())
+	})
+}
+
+func TestDeleteExpiredSnapshots_KubernetesMode(t *testing.T) {
+	t.Run("deletes expired snapshots across multiple orgs", func(t *testing.T) {
+		// Create expired snapshots - one per org
+		expiredTime := time.Now().Add(-time.Hour).UnixMilli()
+		expiredSnapshot1 := createUnstructuredSnapshot("expired-snap-1", "org-1", expiredTime)
+		expiredSnapshot2 := createUnstructuredSnapshot("expired-snap-2", "org-2", expiredTime)
+
+		// Track which namespaces were queried
+		namespacesQueried := make(map[string]bool)
+
+		mockResource := new(mockResourceInterface)
+		mockResource.On("Namespace", mock.Anything).Run(func(args mock.Arguments) {
+			ns := args.Get(0).(string)
+			namespacesQueried[ns] = true
+		}).Return(mockResource)
+		mockResource.On("List", mock.Anything, mock.Anything).Return(&unstructured.UnstructuredList{
+			Items: []unstructured.Unstructured{*expiredSnapshot1, *expiredSnapshot2},
+		}, nil)
+		mockResource.On("Delete", mock.Anything, "expired-snap-1", mock.Anything, mock.Anything).Return(nil)
+		mockResource.On("Delete", mock.Anything, "expired-snap-2", mock.Anything, mock.Anything).Return(nil)
+
+		mockDynClient := new(mockDynamicClient)
+		mockDynClient.On("Resource", mock.Anything).Return(mockResource)
+
+		service := createK8sCleanupService(t, mockDynClient)
+		service.deleteExpiredSnapshots(context.Background())
+
+		// Verify multiple namespaces were queried (one per org)
+		require.GreaterOrEqual(t, len(namespacesQueried), 2, "expected at least 2 namespaces to be queried")
+		// Verify both snapshots were deleted
+		mockResource.AssertCalled(t, "Delete", mock.Anything, "expired-snap-1", mock.Anything, mock.Anything)
+		mockResource.AssertCalled(t, "Delete", mock.Anything, "expired-snap-2", mock.Anything, mock.Anything)
+	})
+
+	t.Run("skips non-expired snapshots", func(t *testing.T) {
+		// Setup with future timestamp
+		futureTime := time.Now().Add(time.Hour).UnixMilli()
+		futureSnapshot := createUnstructuredSnapshot("future-snap", "org-1", futureTime)
+
+		mockResource := new(mockResourceInterface)
+		mockResource.On("Namespace", mock.Anything).Return(mockResource)
+		mockResource.On("List", mock.Anything, mock.Anything).Return(&unstructured.UnstructuredList{
+			Items: []unstructured.Unstructured{*futureSnapshot},
+		}, nil)
+
+		mockDynClient := new(mockDynamicClient)
+		mockDynClient.On("Resource", mock.Anything).Return(mockResource)
+
+		service := createK8sCleanupService(t, mockDynClient)
+		service.deleteExpiredSnapshots(context.Background())
+
+		mockResource.AssertNotCalled(t, "Delete", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+	})
+
+	t.Run("handles REST config error", func(t *testing.T) {
+		service := &CleanUpService{
+			log:                  log.New("cleanup"),
+			Cfg:                  &setting.Cfg{},
+			Features:             featuremgmt.WithFeatures(featuremgmt.FlagKubernetesSnapshots),
+			clientConfigProvider: apiserver.WithoutRestConfig,
+		}
+
+		// Should not panic
+		service.deleteExpiredSnapshots(context.Background())
+	})
+
+	t.Run("handles not found error gracefully", func(t *testing.T) {
+		expiredTime := time.Now().Add(-time.Hour).UnixMilli()
+		expiredSnapshot := createUnstructuredSnapshot("expired-snap", "org-1", expiredTime)
+
+		notFoundErr := k8serrors.NewNotFound(schema.GroupResource{}, "expired-snap")
+
+		mockResource := new(mockResourceInterface)
+		mockResource.On("Namespace", mock.Anything).Return(mockResource)
+		mockResource.On("List", mock.Anything, mock.Anything).Return(&unstructured.UnstructuredList{
+			Items: []unstructured.Unstructured{*expiredSnapshot},
+		}, nil)
+		mockResource.On("Delete", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(notFoundErr)
+
+		mockDynClient := new(mockDynamicClient)
+		mockDynClient.On("Resource", mock.Anything).Return(mockResource)
+
+		service := createK8sCleanupService(t, mockDynClient)
+
+		// Should not panic - not found is expected
+		service.deleteExpiredSnapshots(context.Background())
+		mockResource.AssertExpectations(t)
+	})
+}
+
+// Helper function to create unstructured snapshots for testing
+func createUnstructuredSnapshot(name, namespace string, expiresMillis int64) *unstructured.Unstructured {
+	snapshot := &v0alpha1.Snapshot{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: namespace,
+		},
+		Spec: v0alpha1.SnapshotSpec{
+			Expires: &expiresMillis,
+		},
+	}
+	obj, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(snapshot)
+	return &unstructured.Unstructured{Object: obj}
+}
+
+// Helper to create CleanUpService configured for Kubernetes mode with standard two-org setup
+func createK8sCleanupService(t *testing.T, mockDynClient *mockDynamicClient) *CleanUpService {
+	mockOrgSvc := orgtest.NewMockService(t)
+	mockOrgSvc.On("Search", mock.Anything, mock.Anything).Return([]*org.OrgDTO{
+		{ID: 1, Name: "org1"},
+		{ID: 2, Name: "org2"},
+	}, nil)
+
+	return &CleanUpService{
+		log:      log.New("cleanup"),
+		Cfg:      &setting.Cfg{},
+		Features: featuremgmt.WithFeatures(featuremgmt.FlagKubernetesSnapshots),
+		clientConfigProvider: apiserver.RestConfigProviderFunc(func(ctx context.Context) (*rest.Config, error) {
+			return &rest.Config{}, nil
+		}),
+		orgService: mockOrgSvc,
+		dynamicClientFactory: func(cfg *rest.Config) (dynamic.Interface, error) {
+			return mockDynClient, nil
+		},
+	}
+}
+
+// mockDynamicClient is a minimal mock for dynamic.Interface
+type mockDynamicClient struct {
+	mock.Mock
+}
+
+func (m *mockDynamicClient) Resource(resource schema.GroupVersionResource) dynamic.NamespaceableResourceInterface {
+	args := m.Called(resource)
+	return args.Get(0).(dynamic.NamespaceableResourceInterface)
+}
+
+// mockResourceInterface is a minimal mock for dynamic.ResourceInterface
+type mockResourceInterface struct {
+	mock.Mock
+}
+
+func (m *mockResourceInterface) Namespace(ns string) dynamic.ResourceInterface {
+	args := m.Called(ns)
+	return args.Get(0).(dynamic.ResourceInterface)
+}
+
+func (m *mockResourceInterface) List(ctx context.Context, opts metav1.ListOptions) (*unstructured.UnstructuredList, error) {
+	args := m.Called(ctx, opts)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).(*unstructured.UnstructuredList), args.Error(1)
+}
+
+func (m *mockResourceInterface) Delete(ctx context.Context, name string, opts metav1.DeleteOptions, subresources ...string) error {
+	args := m.Called(ctx, name, opts, subresources)
+	return args.Error(0)
+}
+
+// Unused methods - panic if called unexpectedly
+func (m *mockResourceInterface) Create(ctx context.Context, obj *unstructured.Unstructured, opts metav1.CreateOptions, subresources ...string) (*unstructured.Unstructured, error) {
+	panic("not implemented")
+}
+func (m *mockResourceInterface) Update(ctx context.Context, obj *unstructured.Unstructured, opts metav1.UpdateOptions, subresources ...string) (*unstructured.Unstructured, error) {
+	panic("not implemented")
+}
+func (m *mockResourceInterface) UpdateStatus(ctx context.Context, obj *unstructured.Unstructured, opts metav1.UpdateOptions) (*unstructured.Unstructured, error) {
+	panic("not implemented")
+}
+func (m *mockResourceInterface) DeleteCollection(ctx context.Context, opts metav1.DeleteOptions, listOpts metav1.ListOptions) error {
+	panic("not implemented")
+}
+func (m *mockResourceInterface) Get(ctx context.Context, name string, opts metav1.GetOptions, subresources ...string) (*unstructured.Unstructured, error) {
+	panic("not implemented")
+}
+func (m *mockResourceInterface) Watch(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) {
+	panic("not implemented")
+}
+func (m *mockResourceInterface) Patch(ctx context.Context, name string, pt types.PatchType, data []byte, opts metav1.PatchOptions, subresources ...string) (*unstructured.Unstructured, error) {
+	panic("not implemented")
+}
+func (m *mockResourceInterface) Apply(ctx context.Context, name string, obj *unstructured.Unstructured, opts metav1.ApplyOptions, subresources ...string) (*unstructured.Unstructured, error) {
+	panic("not implemented")
+}
+func (m *mockResourceInterface) ApplyStatus(ctx context.Context, name string, obj *unstructured.Unstructured, opts metav1.ApplyOptions) (*unstructured.Unstructured, error) {
+	panic("not implemented")
+}

pkg/services/dashboardsnapshots/service.go

@@ -37,10 +37,15 @@ var client = &http.Client{
 }
 
 func CreateDashboardSnapshot(c *contextmodel.ReqContext, cfg snapshot.SnapshotSharingOptions, cmd CreateDashboardSnapshotCommand, svc Service) {
+	// perform all validations in the beginning
 	if !cfg.SnapshotsEnabled {
 		c.JsonApiErr(http.StatusForbidden, "Dashboard Snapshots are disabled", nil)
 		return
 	}
+	if cmd.External && !cfg.ExternalEnabled {
+		c.JsonApiErr(http.StatusForbidden, "External dashboard creation is disabled", nil)
+		return
+	}
 
 	uid := cmd.Dashboard.GetNestedString("uid")
 	user, err := identity.GetRequester(c.Req.Context())
@@ -67,17 +72,17 @@ func CreateDashboardSnapshot(c *contextmodel.ReqContext, cfg snapshot.SnapshotSh
 	cmd.ExternalURL = ""
 	cmd.OrgID = user.GetOrgID()
 	cmd.UserID, _ = identity.UserIdentifier(user.GetID())
-	originalDashboardURL, err := createOriginalDashboardURL(&cmd)
+	originalDashboardURL, err := CreateOriginalDashboardURL(&cmd)
 	if err != nil {
 		c.JsonApiErr(http.StatusInternalServerError, "Invalid app URL", err)
 		return
 	}
 
 	if cmd.External {
-		if !cfg.ExternalEnabled {
-			c.JsonApiErr(http.StatusForbidden, "External dashboard creation is disabled", nil)
-			return
-		}
+		//if !cfg.ExternalEnabled {
+		//	c.JsonApiErr(http.StatusForbidden, "External dashboard creation is disabled", nil)
+		//	return
+		//}
 
 		resp, err := createExternalDashboardSnapshot(cmd, cfg.ExternalSnapshotURL)
 		if err != nil {
@@ -203,7 +208,7 @@ func createExternalDashboardSnapshot(cmd CreateDashboardSnapshotCommand, externa
 	return &createSnapshotResponse, nil
 }
 
-func createOriginalDashboardURL(cmd *CreateDashboardSnapshotCommand) (string, error) {
+func CreateOriginalDashboardURL(cmd *CreateDashboardSnapshotCommand) (string, error) {
 	dashUID := cmd.Dashboard.GetNestedString("uid")
 	if ok := util.IsValidShortUID(dashUID); !ok {
 		return "", fmt.Errorf("invalid dashboard UID")

public/app/features/dashboard/services/SnapshotSrv.ts

@@ -1,8 +1,8 @@
-import { lastValueFrom, map } from 'rxjs';
+import { lastValueFrom } from 'rxjs';
 
-import { config, getBackendSrv, FetchResponse } from '@grafana/runtime';
+import { config, getBackendSrv } from '@grafana/runtime';
 import { contextSrv } from 'app/core/services/context_srv';
-import { DashboardDTO, SnapshotSpec } from 'app/types/dashboard';
+import { DashboardDataDTO, DashboardDTO } from 'app/types/dashboard';
 
 import { getAPINamespace } from '../../../api/utils';
 
@@ -82,11 +82,12 @@ interface DashboardSnapshotList {
   items: K8sSnapshotResource[];
 }
 
-interface K8sDashboardSnapshot {
+// Response from the /dashboard subresource - returns a Dashboard with raw dashboard data in spec
+interface K8sDashboardSubresource {
   apiVersion: string;
-  kind: 'Snapshot';
+  kind: 'Dashboard';
   metadata: K8sMetadata;
-  spec: SnapshotSpec;
+  spec: DashboardDataDTO;
 }
 
 class K8sAPI implements DashboardSnapshotSrv {
@@ -128,32 +129,45 @@ class K8sAPI implements DashboardSnapshotSrv {
       const token = `??? TODO, get anon token for snapshots (${contextSrv.user?.name}) ???`;
       headers['Authorization'] = `Bearer ${token}`;
     }
-    return lastValueFrom(
-      getBackendSrv()
-        .fetch<K8sDashboardSnapshot>({
+
+    // Fetch both snapshot metadata and dashboard content in parallel
+    const [snapshotResponse, dashboardResponse] = await Promise.all([
+      lastValueFrom(
+        getBackendSrv().fetch<K8sSnapshotResource>({
           url: this.url + '/' + uid,
           method: 'GET',
           headers: headers,
         })
-        .pipe(
-          map((response: FetchResponse<K8sDashboardSnapshot>) => {
-            return {
-              dashboard: response.data.spec.dashboard,
-              meta: {
-                isSnapshot: true,
-                canSave: false,
-                canEdit: false,
-                canAdmin: false,
-                canStar: false,
-                canShare: false,
-                canDelete: false,
-                isFolder: false,
-                provisioned: false,
-              },
-            };
-          })
-        )
-    );
+      ),
+      lastValueFrom(
+        getBackendSrv().fetch<K8sDashboardSubresource>({
+          url: this.url + '/' + uid + '/dashboard',
+          method: 'GET',
+          headers: headers,
+        })
+      ),
+    ]);
+
+    const snapshot = snapshotResponse.data;
+    const dashboard = dashboardResponse.data;
+
+    return {
+      dashboard: dashboard.spec,
+      meta: {
+        isSnapshot: true,
+        canSave: false,
+        canEdit: false,
+        canAdmin: false,
+        canStar: false,
+        canShare: false,
+        canDelete: false,
+        isFolder: false,
+        provisioned: false,
+        created: snapshot.metadata.creationTimestamp,
+        expires: snapshot.spec.expires?.toString(),
+        k8s: snapshot.metadata,
+      },
+    };
   }
 }