diff --git a/pkg/apis/playlist/conversions.go b/pkg/apis/playlist/conversions.go index bbe77f689fa..adffda00343 100644 --- a/pkg/apis/playlist/conversions.go +++ b/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{ diff --git a/pkg/apis/playlist/legacy_storage.go b/pkg/apis/playlist/legacy_storage.go index dab3ce7f813..7bad9013a47 100644 --- a/pkg/apis/playlist/legacy_storage.go +++ b/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 +} diff --git a/pkg/services/grafana-apiserver/registry/generic/strategy.go b/pkg/services/grafana-apiserver/registry/generic/strategy.go index 797f2acc4fb..95bf1424e04 100644 --- a/pkg/services/grafana-apiserver/registry/generic/strategy.go +++ b/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) {} diff --git a/pkg/tests/apis/helper.go b/pkg/tests/apis/helper.go index 52fc57bd574..b24fa49900d 100644 --- a/pkg/tests/apis/helper.go +++ b/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) } diff --git a/pkg/tests/apis/playlist/playlist_test.go b/pkg/tests/apis/playlist/playlist_test.go index 9d475d82d5a..59a47e85612 100644 --- a/pkg/tests/apis/playlist/playlist_test.go +++ b/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 } diff --git a/pkg/tests/apis/playlist/testdata/playlist-generate.yaml b/pkg/tests/apis/playlist/testdata/playlist-generate.yaml new file mode 100644 index 00000000000..07dbe1efe32 --- /dev/null +++ b/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 diff --git a/pkg/tests/apis/playlist/testdata/playlist-test-apply.yaml b/pkg/tests/apis/playlist/testdata/playlist-test-apply.yaml new file mode 100644 index 00000000000..098dc08e86a --- /dev/null +++ b/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) diff --git a/pkg/tests/apis/playlist/testdata/playlist-test-create.yaml b/pkg/tests/apis/playlist/testdata/playlist-test-create.yaml new file mode 100644 index 00000000000..4839fa07669 --- /dev/null +++ b/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 diff --git a/pkg/tests/apis/playlist/testdata/playlist-test-replace.yaml b/pkg/tests/apis/playlist/testdata/playlist-test-replace.yaml new file mode 100644 index 00000000000..1984ef2c1df --- /dev/null +++ b/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 diff --git a/public/app/features/playlist/api.ts b/public/app/features/playlist/api.ts index 27ea44fdf96..d8a40b649ae 100644 --- a/public/app/features/playlist/api.ts +++ b/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 { @@ -81,25 +76,16 @@ class K8sAPI implements PlaylistAPI { } async createPlaylist(playlist: Playlist): Promise { - 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 { - 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 { - if (this.legacy) { - return this.legacy.deletePlaylist(uid); - } await withErrorHandling(() => getBackendSrv().delete(`${this.url}/${uid}`), 'Playlist deleted'); }