K8s/Playlist: Support full CRUD from k8s to existing storage (#75709)

pkg/apis/playlist/conversions.go

@@ -76,6 +76,26 @@ func convertToK8sResource(v *playlist.PlaylistDTO, namespacer request.NamespaceM
 	}
 }
 
+func convertToLegacyUpdateCommand(p *Playlist, orgId int64) (*playlist.UpdatePlaylistCommand, error) {
+	spec := p.Spec
+	cmd := &playlist.UpdatePlaylistCommand{
+		UID:      p.Name,
+		Name:     spec.Title,
+		Interval: spec.Interval,
+		OrgId:    orgId,
+	}
+	for _, item := range spec.Items {
+		if item.Type == ItemTypeDashboardById {
+			return nil, fmt.Errorf("unsupported item type: %s", item.Type)
+		}
+		cmd.Items = append(cmd.Items, playlist.PlaylistItem{
+			Type:  string(item.Type),
+			Value: item.Value,
+		})
+	}
+	return cmd, nil
+}
+
 // Read legacy ID from metadata annotations
 func getLegacyID(item *unstructured.Unstructured) int64 {
 	meta := kinds.GrafanaResourceMetadata{

pkg/apis/playlist/legacy_storage.go

@@ -3,6 +3,7 @@ package playlist
 import (
 	"context"
 	"errors"
+	"fmt"
 
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/internalversion"
@@ -21,6 +22,9 @@ var (
 	_ rest.Getter               = (*legacyStorage)(nil)
 	_ rest.Lister               = (*legacyStorage)(nil)
 	_ rest.Storage              = (*legacyStorage)(nil)
+	_ rest.Creater              = (*legacyStorage)(nil)
+	_ rest.Updater              = (*legacyStorage)(nil)
+	_ rest.GracefulDeleter      = (*legacyStorage)(nil)
 )
 
 type legacyStorage struct {
@@ -110,3 +114,96 @@ func (s *legacyStorage) Get(ctx context.Context, name string, options *metav1.Ge
 
 	return convertToK8sResource(dto, s.namespacer), nil
 }
+
+func (s *legacyStorage) Create(ctx context.Context,
+	obj runtime.Object,
+	createValidation rest.ValidateObjectFunc,
+	options *metav1.CreateOptions,
+) (runtime.Object, error) {
+	info, err := request.NamespaceInfoFrom(ctx, true)
+	if err != nil {
+		return nil, err
+	}
+
+	p, ok := obj.(*Playlist)
+	if !ok {
+		return nil, fmt.Errorf("expected playlist?")
+	}
+	cmd, err := convertToLegacyUpdateCommand(p, info.OrgID)
+	if err != nil {
+		return nil, err
+	}
+	out, err := s.service.Create(ctx, &playlist.CreatePlaylistCommand{
+		UID:      p.Name,
+		Name:     cmd.Name,
+		Interval: cmd.Interval,
+		Items:    cmd.Items,
+		OrgId:    cmd.OrgId,
+	})
+	if err != nil {
+		return nil, err
+	}
+	return s.Get(ctx, out.UID, nil)
+}
+
+func (s *legacyStorage) Update(ctx context.Context,
+	name string,
+	objInfo rest.UpdatedObjectInfo,
+	createValidation rest.ValidateObjectFunc,
+	updateValidation rest.ValidateObjectUpdateFunc,
+	forceAllowCreate bool,
+	options *metav1.UpdateOptions,
+) (runtime.Object, bool, error) {
+	info, err := request.NamespaceInfoFrom(ctx, true)
+	if err != nil {
+		return nil, false, err
+	}
+
+	created := false
+	old, err := s.Get(ctx, name, nil)
+	if err != nil {
+		return old, created, err
+	}
+
+	obj, err := objInfo.UpdatedObject(ctx, old)
+	if err != nil {
+		return old, created, err
+	}
+	p, ok := obj.(*Playlist)
+	if !ok {
+		return nil, created, fmt.Errorf("expected playlist after update")
+	}
+
+	cmd, err := convertToLegacyUpdateCommand(p, info.OrgID)
+	if err != nil {
+		return old, created, err
+	}
+	_, err = s.service.Update(ctx, cmd)
+	if err != nil {
+		return nil, false, err
+	}
+
+	r, err := s.Get(ctx, name, nil)
+	return r, created, err
+}
+
+// GracefulDeleter
+func (s *legacyStorage) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) {
+	v, err := s.Get(ctx, name, &metav1.GetOptions{})
+	if err != nil {
+		return v, false, err // includes the not-found error
+	}
+	info, err := request.NamespaceInfoFrom(ctx, true)
+	if err != nil {
+		return nil, false, err
+	}
+	p, ok := v.(*Playlist)
+	if !ok {
+		return v, false, fmt.Errorf("expected a playlist response from Get")
+	}
+	err = s.service.Delete(ctx, &playlist.DeletePlaylistCommand{
+		UID:   name,
+		OrgId: info.OrgID,
+	})
+	return p, true, err // true is instant delete
+}

pkg/services/grafana-apiserver/registry/generic/strategy.go

@@ -39,11 +39,11 @@ func (genericStrategy) Validate(ctx context.Context, obj runtime.Object) field.E
 func (genericStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { return nil }
 
 func (genericStrategy) AllowCreateOnUpdate() bool {
-	return false
+	return true
 }
 
 func (genericStrategy) AllowUnconditionalUpdate() bool {
-	return false
+	return true
 }
 
 func (genericStrategy) Canonicalize(obj runtime.Object) {}

pkg/tests/apis/helper.go

@@ -126,7 +126,16 @@ func (c *K8sTestHelper) AsStatusError(err error) *errors.StatusError {
 func (c *K8sResourceClient) SanitizeJSON(v *unstructured.Unstructured) string {
 	c.t.Helper()
 
-	copy := v.DeepCopy().Object
+	deep := v.DeepCopy()
+	anno := deep.GetAnnotations()
+	if anno["grafana.app/originKey"] != "" {
+		anno["grafana.app/originKey"] = "${originKey}"
+	}
+	if anno["grafana.app/updatedTimestamp"] != "" {
+		anno["grafana.app/updatedTimestamp"] = "${updatedTimestamp}"
+	}
+	deep.SetAnnotations(anno)
+	copy := deep.Object
 	meta, ok := copy["metadata"].(map[string]any)
 	require.True(c.t, ok)
 
@@ -139,6 +148,7 @@ func (c *K8sResourceClient) SanitizeJSON(v *unstructured.Unstructured) string {
 	}
 
 	out, err := json.MarshalIndent(copy, "", "  ")
+	//fmt.Printf("%s", out)
 	require.NoError(c.t, err)
 	return string(out)
 }

pkg/tests/apis/playlist/playlist_test.go

@@ -1,13 +1,17 @@
 package playlist
 
 import (
+	"cmp"
 	"context"
+	"encoding/json"
 	"net/http"
+	"slices"
 	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	"github.com/grafana/grafana/pkg/services/playlist"
@@ -127,6 +131,11 @@ func TestPlaylist(t *testing.T) {
 			"apiVersion": "playlist.grafana.app/v0alpha1",
 			"kind": "Playlist",
 			"metadata": {
+			  "annotations": {
+				"grafana.app/originKey": "${originKey}",
+				"grafana.app/originName": "SQL",
+				"grafana.app/updatedTimestamp": "${updatedTimestamp}"
+			  },
 			  "creationTimestamp": "${creationTimestamp}",
 			  "name": "` + uid + `",
 			  "namespace": "default",
@@ -134,7 +143,6 @@ func TestPlaylist(t *testing.T) {
 			  "uid": "${uid}"
 			},
 			"spec": {
-			  "title": "Test",
 			  "interval": "20s",
 			  "items": [
 				{
@@ -145,7 +153,8 @@ func TestPlaylist(t *testing.T) {
 				  "type": "dashboard_by_tag",
 				  "value": "graph-ng"
 				}
-			  ]
+			  ],
+			  "title": "Test"
 			}
 		  }`
 
@@ -191,4 +200,172 @@ func TestPlaylist(t *testing.T) {
 		require.Nil(t, found)
 		require.Equal(t, metav1.StatusReasonNotFound, statusError.Status().Reason)
 	})
+
+	t.Run("Do CRUD via k8s (and check that legacy api still works)", func(t *testing.T) {
+		client := helper.GetResourceClient(apis.ResourceClientArgs{
+			User: helper.Org1.Editor,
+			GVR:  gvr,
+		})
+
+		// Create the playlist "test"
+		first, err := client.Resource.Create(context.Background(),
+			helper.LoadYAMLOrJSONFile("testdata/playlist-test-create.yaml"),
+			metav1.CreateOptions{},
+		)
+		require.NoError(t, err)
+		require.Equal(t, "test", first.GetName())
+		uids := []string{first.GetName()}
+
+		// Create (with name generation) two playlists
+		for i := 0; i < 2; i++ {
+			out, err := client.Resource.Create(context.Background(),
+				helper.LoadYAMLOrJSONFile("testdata/playlist-generate.yaml"),
+				metav1.CreateOptions{},
+			)
+			require.NoError(t, err)
+			uids = append(uids, out.GetName())
+		}
+		slices.Sort(uids) // make list compare stable
+
+		// Check that everything is returned from the List command
+		list, err := client.Resource.List(context.Background(), metav1.ListOptions{})
+		require.NoError(t, err)
+		require.Equal(t, uids, SortSlice(Map(list.Items, func(item unstructured.Unstructured) string {
+			return item.GetName()
+		})))
+
+		// The legacy endpoint has the same results
+		searchResponse := apis.DoRequest(helper, apis.RequestParams{
+			User:   client.Args.User,
+			Method: http.MethodGet,
+			Path:   "/api/playlists",
+		}, &playlist.Playlists{})
+		require.NotNil(t, searchResponse.Result)
+		require.Equal(t, uids, SortSlice(Map(*searchResponse.Result, func(item *playlist.Playlist) string {
+			return item.UID
+		})))
+
+		// Check all playlists
+		for _, uid := range uids {
+			getFromBothAPIs(t, helper, client, uid, nil)
+		}
+
+		// PUT :: Update the title (full payload)
+		updated, err := client.Resource.Update(context.Background(),
+			helper.LoadYAMLOrJSONFile("testdata/playlist-test-replace.yaml"),
+			metav1.UpdateOptions{},
+		)
+		require.NoError(t, err)
+		require.Equal(t, first.GetName(), updated.GetName())
+		require.Equal(t, first.GetUID(), updated.GetUID())
+		require.Less(t, first.GetResourceVersion(), updated.GetResourceVersion())
+		out := getFromBothAPIs(t, helper, client, "test", &playlist.PlaylistDTO{
+			Name:     "Test playlist (replaced from k8s; 22m; 1 items; PUT)",
+			Interval: "22m",
+		})
+		require.Equal(t, updated.GetResourceVersion(), out.GetResourceVersion())
+
+		// PATCH :: apply only some fields
+		updated, err = client.Resource.Apply(context.Background(), "test",
+			helper.LoadYAMLOrJSONFile("testdata/playlist-test-apply.yaml"),
+			metav1.ApplyOptions{
+				Force:        true,
+				FieldManager: "testing",
+			},
+		)
+		require.NoError(t, err)
+		require.Equal(t, first.GetName(), updated.GetName())
+		require.Equal(t, first.GetUID(), updated.GetUID())
+		require.Less(t, first.GetResourceVersion(), updated.GetResourceVersion())
+		getFromBothAPIs(t, helper, client, "test", &playlist.PlaylistDTO{
+			Name:     "Test playlist (apply from k8s; ??m; ?? items; PATCH)",
+			Interval: "22m", // has not changed from previous update
+		})
+
+		// Now delete all playlist (three)
+		for _, uid := range uids {
+			err := client.Resource.Delete(context.Background(), uid, metav1.DeleteOptions{})
+			require.NoError(t, err)
+
+			// Second call is not found!
+			err = client.Resource.Delete(context.Background(), uid, metav1.DeleteOptions{})
+			statusError := helper.AsStatusError(err)
+			require.Equal(t, metav1.StatusReasonNotFound, statusError.Status().Reason)
+
+			// Not found from k8s getter
+			_, err = client.Resource.Get(context.Background(), uid, metav1.GetOptions{})
+			statusError = helper.AsStatusError(err)
+			require.Equal(t, metav1.StatusReasonNotFound, statusError.Status().Reason)
+		}
+
+		// Check that they are all gone
+		list, err = client.Resource.List(context.Background(), metav1.ListOptions{})
+		require.NoError(t, err)
+		require.Empty(t, list.Items)
+	})
+}
+
+// typescript style map function
+func Map[A any, B any](input []A, m func(A) B) []B {
+	output := make([]B, len(input))
+	for i, element := range input {
+		output[i] = m(element)
+	}
+	return output
+}
+
+func SortSlice[A cmp.Ordered](input []A) []A {
+	slices.Sort(input)
+	return input
+}
+
+// This does a get with both k8s and legacy API, and verifies the results are the same
+func getFromBothAPIs(t *testing.T,
+	helper *apis.K8sTestHelper,
+	client *apis.K8sResourceClient,
+	uid string,
+	// Optionally match some expect some values
+	expect *playlist.PlaylistDTO,
+) *unstructured.Unstructured {
+	t.Helper()
+
+	found, err := client.Resource.Get(context.Background(), uid, metav1.GetOptions{})
+	require.NoError(t, err)
+	require.Equal(t, uid, found.GetName())
+
+	dto := apis.DoRequest(helper, apis.RequestParams{
+		User:   client.Args.User,
+		Method: http.MethodGet,
+		Path:   "/api/playlists/" + uid,
+	}, &playlist.PlaylistDTO{}).Result
+	require.NotNil(t, dto)
+	require.Equal(t, uid, dto.Uid)
+
+	spec, ok := found.Object["spec"].(map[string]any)
+	require.True(t, ok)
+	require.Equal(t, dto.Uid, found.GetName())
+	require.Equal(t, dto.Name, spec["title"])
+	require.Equal(t, dto.Interval, spec["interval"])
+
+	a, errA := json.Marshal(spec["items"])
+	b, errB := json.Marshal(dto.Items)
+	require.NoError(t, errA)
+	require.NoError(t, errB)
+	require.JSONEq(t, string(a), string(b))
+
+	if expect != nil {
+		if expect.Name != "" {
+			require.Equal(t, expect.Name, dto.Name)
+			require.Equal(t, expect.Name, spec["title"])
+		}
+		if expect.Interval != "" {
+			require.Equal(t, expect.Interval, dto.Interval)
+			require.Equal(t, expect.Interval, spec["interval"])
+		}
+		if expect.Uid != "" {
+			require.Equal(t, expect.Uid, dto.Uid)
+			require.Equal(t, expect.Uid, found.GetName())
+		}
+	}
+	return found
 }

pkg/tests/apis/playlist/testdata/playlist-generate.yaml

@@ -0,0 +1,12 @@
+apiVersion: playlist.grafana.app/v0alpha1
+kind: Playlist
+metadata:
+  generateName: x # anything is ok here... except yes or true -- they become boolean!
+spec:
+  title: Playlist with auto generated UID
+  interval: 5m
+  items:
+  - type: dashboard_by_tag
+    value: panel-tests
+  - type: dashboard_by_uid
+    value: vmie2cmWz # dashboard from devenv

pkg/tests/apis/playlist/testdata/playlist-test-apply.yaml

@@ -0,0 +1,6 @@
+apiVersion: playlist.grafana.app/v0alpha1
+kind: Playlist
+metadata:
+  name: test
+spec:
+  title: Test playlist (apply from k8s; ??m; ?? items; PATCH)

pkg/tests/apis/playlist/testdata/playlist-test-create.yaml

@@ -0,0 +1,12 @@
+apiVersion: playlist.grafana.app/v0alpha1
+kind: Playlist
+metadata:
+  name: test
+spec:
+  title: Test playlist (created from k8s; 2 items; POST)
+  interval: 5m
+  items:
+  - type: dashboard_by_tag
+    value: panel-tests
+  - type: dashboard_by_uid
+    value: vmie2cmWz # dashboard from devenv

pkg/tests/apis/playlist/testdata/playlist-test-replace.yaml

@@ -0,0 +1,10 @@
+apiVersion: playlist.grafana.app/v0alpha1
+kind: Playlist
+metadata:
+  name: test
+spec:
+  title: Test playlist (replaced from k8s; 22m; 1 items; PUT)
+  interval: 22m
+  items:
+  - type: dashboard_by_tag
+    value: panel-tests

public/app/features/playlist/api.ts

@@ -57,15 +57,10 @@ interface K8sPlaylist {
 class K8sAPI implements PlaylistAPI {
   readonly apiVersion = 'playlist.grafana.app/v0alpha1';
   readonly url: string;
-  readonly legacy: PlaylistAPI | undefined;
 
   constructor() {
     const ns = contextSrv.user.orgId === 1 ? 'default' : `org-${contextSrv.user.orgId}`;
     this.url = `/apis/${this.apiVersion}/namespaces/${ns}/playlists`;
-
-    // When undefined, this will use k8s for all CRUD features
-    // if (!config.featureToggles.grafanaAPIServerWithExperimentalAPIs) {
-    this.legacy = new LegacyAPI();
   }
 
   async getAllPlaylist(): Promise<Playlist[]> {
@@ -81,25 +76,16 @@ class K8sAPI implements PlaylistAPI {
   }
 
   async createPlaylist(playlist: Playlist): Promise<void> {
-    if (this.legacy) {
-      return this.legacy.createPlaylist(playlist);
-    }
     const body = this.playlistAsK8sResource(playlist);
     await withErrorHandling(() => getBackendSrv().post(this.url, body));
   }
 
   async updatePlaylist(playlist: Playlist): Promise<void> {
-    if (this.legacy) {
-      return this.legacy.updatePlaylist(playlist);
-    }
     const body = this.playlistAsK8sResource(playlist);
     await withErrorHandling(() => getBackendSrv().put(`${this.url}/${playlist.uid}`, body));
   }
 
   async deletePlaylist(uid: string): Promise<void> {
-    if (this.legacy) {
-      return this.legacy.deletePlaylist(uid);
-    }
     await withErrorHandling(() => getBackendSrv().delete(`${this.url}/${uid}`), 'Playlist deleted');
   }