Playlist: Implement the entire API with k8s client (#77596)

pkg/api/playlist.go

@@ -16,127 +16,34 @@ import (
 	internalplaylist "github.com/grafana/grafana/pkg/registry/apis/playlist"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
+	grafanaapiserver "github.com/grafana/grafana/pkg/services/grafana-apiserver"
 	"github.com/grafana/grafana/pkg/services/grafana-apiserver/endpoints/request"
 	"github.com/grafana/grafana/pkg/services/playlist"
 	"github.com/grafana/grafana/pkg/util/errutil/errhttp"
 	"github.com/grafana/grafana/pkg/web"
 )
 
-type playlistAPIHandler struct {
-	SearchPlaylists  []web.Handler
-	GetPlaylist      []web.Handler
-	GetPlaylistItems []web.Handler
-	DeletePlaylist   []web.Handler
-	UpdatePlaylist   []web.Handler
-	CreatePlaylist   []web.Handler
-}
-
-func chainHandlers(h ...web.Handler) []web.Handler {
-	return h
-}
-
 func (hs *HTTPServer) registerPlaylistAPI(apiRoute routing.RouteRegister) {
-	handler := playlistAPIHandler{
-		SearchPlaylists:  chainHandlers(routing.Wrap(hs.SearchPlaylists)),
-		GetPlaylist:      chainHandlers(hs.validateOrgPlaylist, routing.Wrap(hs.GetPlaylist)),
-		GetPlaylistItems: chainHandlers(hs.validateOrgPlaylist, routing.Wrap(hs.GetPlaylistItems)),
-		DeletePlaylist:   chainHandlers(middleware.ReqEditorRole, hs.validateOrgPlaylist, routing.Wrap(hs.DeletePlaylist)),
-		UpdatePlaylist:   chainHandlers(middleware.ReqEditorRole, hs.validateOrgPlaylist, routing.Wrap(hs.UpdatePlaylist)),
-		CreatePlaylist:   chainHandlers(middleware.ReqEditorRole, routing.Wrap(hs.CreatePlaylist)),
-	}
-
-	// Alternative implementations for k8s
-	if hs.Features.IsEnabled(featuremgmt.FlagKubernetesPlaylistsAPI) {
-		namespacer := request.GetNamespaceMapper(hs.Cfg)
-		gvr := schema.GroupVersionResource{
-			Group:    internalplaylist.GroupName,
-			Version:  internalplaylist.VersionID,
-			Resource: "playlists",
-		}
-
-		clientGetter := func(c *contextmodel.ReqContext) (dynamic.ResourceInterface, bool) {
-			dyn, err := dynamic.NewForConfig(hs.clientConfigProvider.GetDirectRestConfig(c))
-			if err != nil {
-				c.JsonApiErr(500, "client", err)
-				return nil, false
-			}
-			return dyn.Resource(gvr).Namespace(namespacer(c.OrgID)), true
-		}
-
-		errorWriter := func(c *contextmodel.ReqContext, err error) {
-			//nolint:errorlint
-			statusError, ok := err.(*errors.StatusError)
-			if ok {
-				c.JsonApiErr(int(statusError.Status().Code),
-					statusError.Status().Message, err)
-				return
-			}
-			errhttp.Write(c.Req.Context(), err, c.Resp)
-		}
-
-		handler.SearchPlaylists = []web.Handler{func(c *contextmodel.ReqContext) {
-			client, ok := clientGetter(c)
-			if !ok {
-				return // error is already sent
-			}
-			out, err := client.List(c.Req.Context(), v1.ListOptions{})
-			if err != nil {
-				errorWriter(c, err)
-				return
-			}
-
-			query := strings.ToUpper(c.Query("query"))
-			playlists := []playlist.Playlist{}
-			for _, item := range out.Items {
-				p := internalplaylist.UnstructuredToLegacyPlaylist(item)
-				if p == nil {
-					continue
-				}
-				if query != "" && !strings.Contains(strings.ToUpper(p.Name), query) {
-					continue // query filter
-				}
-				playlists = append(playlists, *p)
-			}
-			c.JSON(http.StatusOK, playlists)
-		}}
-
-		handler.GetPlaylist = []web.Handler{func(c *contextmodel.ReqContext) {
-			client, ok := clientGetter(c)
-			if !ok {
-				return // error is already sent
-			}
-			uid := web.Params(c.Req)[":uid"]
-			out, err := client.Get(c.Req.Context(), uid, v1.GetOptions{})
-			if err != nil {
-				errorWriter(c, err)
-				return
-			}
-			c.JSON(http.StatusOK, internalplaylist.UnstructuredToLegacyPlaylistDTO(*out))
-		}}
-
-		handler.GetPlaylistItems = []web.Handler{func(c *contextmodel.ReqContext) {
-			client, ok := clientGetter(c)
-			if !ok {
-				return // error is already sent
-			}
-			uid := web.Params(c.Req)[":uid"]
-			out, err := client.Get(c.Req.Context(), uid, v1.GetOptions{})
-			if err != nil {
-				errorWriter(c, err)
-				return
-			}
-			c.JSON(http.StatusOK, internalplaylist.UnstructuredToLegacyPlaylistDTO(*out).Items)
-		}}
-	}
-
 	// Register the actual handlers
 	apiRoute.Group("/playlists", func(playlistRoute routing.RouteRegister) {
-		playlistRoute.Get("/", handler.SearchPlaylists...)
+		if hs.Features.IsEnabled(featuremgmt.FlagKubernetesPlaylistsAPI) {
-		playlistRoute.Get("/:uid", handler.GetPlaylist...)
+			// Use k8s client to implement legacy API
-		playlistRoute.Get("/:uid/items", handler.GetPlaylistItems...)
+			handler := newPlaylistK8sHandler(hs)
-		playlistRoute.Delete("/:uid", handler.DeletePlaylist...)
+			playlistRoute.Get("/", handler.searchPlaylists)
-		playlistRoute.Put("/:uid", handler.UpdatePlaylist...)
+			playlistRoute.Get("/:uid", handler.getPlaylist)
-		playlistRoute.Post("/", handler.CreatePlaylist...)
+			playlistRoute.Get("/:uid/items", handler.getPlaylistItems)
+			playlistRoute.Delete("/:uid", handler.deletePlaylist)
+			playlistRoute.Put("/:uid", handler.updatePlaylist)
+			playlistRoute.Post("/", handler.createPlaylist)
+		} else {
+			// Legacy handlers
+			playlistRoute.Get("/", routing.Wrap(hs.SearchPlaylists))
+			playlistRoute.Get("/:uid", hs.validateOrgPlaylist, routing.Wrap(hs.GetPlaylist))
+			playlistRoute.Get("/:uid/items", hs.validateOrgPlaylist, routing.Wrap(hs.GetPlaylistItems))
+			playlistRoute.Delete("/:uid", middleware.ReqEditorRole, hs.validateOrgPlaylist, routing.Wrap(hs.DeletePlaylist))
+			playlistRoute.Put("/:uid", middleware.ReqEditorRole, hs.validateOrgPlaylist, routing.Wrap(hs.UpdatePlaylist))
+			playlistRoute.Post("/", middleware.ReqEditorRole, routing.Wrap(hs.CreatePlaylist))
+		}
 	})
 }
 
@@ -409,3 +316,156 @@ type CreatePlaylistResponse struct {
 	// in: body
 	Body *playlist.Playlist `json:"body"`
 }
+
+type playlistK8sHandler struct {
+	namespacer           request.NamespaceMapper
+	gvr                  schema.GroupVersionResource
+	clientConfigProvider grafanaapiserver.DirectRestConfigProvider
+}
+
+//-----------------------------------------------------------------------------------------
+// Playlist k8s wrapper functions
+//-----------------------------------------------------------------------------------------
+
+func newPlaylistK8sHandler(hs *HTTPServer) *playlistK8sHandler {
+	return &playlistK8sHandler{
+		gvr: schema.GroupVersionResource{
+			Group:    internalplaylist.GroupName,
+			Version:  "v0alpha1",
+			Resource: "playlists",
+		},
+		namespacer:           request.GetNamespaceMapper(hs.Cfg),
+		clientConfigProvider: hs.clientConfigProvider,
+	}
+}
+
+func (pk8s *playlistK8sHandler) searchPlaylists(c *contextmodel.ReqContext) {
+	client, ok := pk8s.getClient(c)
+	if !ok {
+		return // error is already sent
+	}
+	out, err := client.List(c.Req.Context(), v1.ListOptions{})
+	if err != nil {
+		pk8s.writeError(c, err)
+		return
+	}
+
+	query := strings.ToUpper(c.Query("query"))
+	playlists := []playlist.Playlist{}
+	for _, item := range out.Items {
+		p := internalplaylist.UnstructuredToLegacyPlaylist(item)
+		if p == nil {
+			continue
+		}
+		if query != "" && !strings.Contains(strings.ToUpper(p.Name), query) {
+			continue // query filter
+		}
+		playlists = append(playlists, *p)
+	}
+	c.JSON(http.StatusOK, playlists)
+}
+
+func (pk8s *playlistK8sHandler) getPlaylist(c *contextmodel.ReqContext) {
+	client, ok := pk8s.getClient(c)
+	if !ok {
+		return // error is already sent
+	}
+	uid := web.Params(c.Req)[":uid"]
+	out, err := client.Get(c.Req.Context(), uid, v1.GetOptions{})
+	if err != nil {
+		pk8s.writeError(c, err)
+		return
+	}
+	c.JSON(http.StatusOK, internalplaylist.UnstructuredToLegacyPlaylistDTO(*out))
+}
+
+func (pk8s *playlistK8sHandler) getPlaylistItems(c *contextmodel.ReqContext) {
+	client, ok := pk8s.getClient(c)
+	if !ok {
+		return // error is already sent
+	}
+	uid := web.Params(c.Req)[":uid"]
+	out, err := client.Get(c.Req.Context(), uid, v1.GetOptions{})
+	if err != nil {
+		pk8s.writeError(c, err)
+		return
+	}
+	c.JSON(http.StatusOK, internalplaylist.UnstructuredToLegacyPlaylistDTO(*out).Items)
+}
+
+func (pk8s *playlistK8sHandler) deletePlaylist(c *contextmodel.ReqContext) {
+	client, ok := pk8s.getClient(c)
+	if !ok {
+		return // error is already sent
+	}
+	uid := web.Params(c.Req)[":uid"]
+	err := client.Delete(c.Req.Context(), uid, v1.DeleteOptions{})
+	if err != nil {
+		pk8s.writeError(c, err)
+		return
+	}
+	c.JSON(http.StatusOK, "")
+}
+
+func (pk8s *playlistK8sHandler) updatePlaylist(c *contextmodel.ReqContext) {
+	client, ok := pk8s.getClient(c)
+	if !ok {
+		return // error is already sent
+	}
+	uid := web.Params(c.Req)[":uid"]
+	cmd := playlist.UpdatePlaylistCommand{}
+	if err := web.Bind(c.Req, &cmd); err != nil {
+		c.JsonApiErr(http.StatusBadRequest, "bad request data", err)
+		return
+	}
+	obj := internalplaylist.LegacyUpdateCommandToUnstructured(cmd)
+	obj.SetName(uid)
+	out, err := client.Update(c.Req.Context(), &obj, v1.UpdateOptions{})
+	if err != nil {
+		pk8s.writeError(c, err)
+		return
+	}
+	c.JSON(http.StatusOK, internalplaylist.UnstructuredToLegacyPlaylistDTO(*out))
+}
+
+func (pk8s *playlistK8sHandler) createPlaylist(c *contextmodel.ReqContext) {
+	client, ok := pk8s.getClient(c)
+	if !ok {
+		return // error is already sent
+	}
+	cmd := playlist.UpdatePlaylistCommand{}
+	if err := web.Bind(c.Req, &cmd); err != nil {
+		c.JsonApiErr(http.StatusBadRequest, "bad request data", err)
+		return
+	}
+	obj := internalplaylist.LegacyUpdateCommandToUnstructured(cmd)
+	out, err := client.Create(c.Req.Context(), &obj, v1.CreateOptions{})
+	if err != nil {
+		pk8s.writeError(c, err)
+		return
+	}
+	c.JSON(http.StatusOK, internalplaylist.UnstructuredToLegacyPlaylistDTO(*out))
+}
+
+//-----------------------------------------------------------------------------------------
+// Utility functions
+//-----------------------------------------------------------------------------------------
+
+func (pk8s *playlistK8sHandler) getClient(c *contextmodel.ReqContext) (dynamic.ResourceInterface, bool) {
+	dyn, err := dynamic.NewForConfig(pk8s.clientConfigProvider.GetDirectRestConfig(c))
+	if err != nil {
+		c.JsonApiErr(500, "client", err)
+		return nil, false
+	}
+	return dyn.Resource(pk8s.gvr).Namespace(pk8s.namespacer(c.OrgID)), true
+}
+
+func (pk8s *playlistK8sHandler) writeError(c *contextmodel.ReqContext, err error) {
+	//nolint:errorlint
+	statusError, ok := err.(*errors.StatusError)
+	if ok {
+		c.JsonApiErr(int(statusError.Status().Code), statusError.Status().Message, err)
+		return
+	}
+	errhttp.Write(c.Req.Context(), err, c.Resp)
+}

pkg/registry/apis/playlist/conversions.go

@@ -16,6 +16,27 @@ import (
 	playlistsvc "github.com/grafana/grafana/pkg/services/playlist"
 )
 
+func LegacyUpdateCommandToUnstructured(cmd playlistsvc.UpdatePlaylistCommand) unstructured.Unstructured {
+	items := []map[string]string{}
+	for _, item := range cmd.Items {
+		items = append(items, map[string]string{
+			"type":  item.Type,
+			"value": item.Value,
+		})
+	}
+	obj := unstructured.Unstructured{
+		Object: map[string]interface{}{
+			"spec": map[string]interface{}{
+				"title":    cmd.Name,
+				"interval": cmd.Interval,
+				"items":    items,
+			},
+		},
+	}
+	obj.SetName(cmd.UID)
+	return obj
+}
+
 func UnstructuredToLegacyPlaylist(item unstructured.Unstructured) *playlistsvc.Playlist {
 	spec := item.Object["spec"].(map[string]any)
 	return &playlistsvc.Playlist{

pkg/tests/apis/helper.go

@@ -26,7 +26,6 @@ import (
 	"github.com/grafana/grafana/pkg/server"
 	"github.com/grafana/grafana/pkg/services/auth/identity"
 	"github.com/grafana/grafana/pkg/services/datasources"
-	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/grafana-apiserver/endpoints/request"
 	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/org/orgimpl"
@@ -43,24 +42,16 @@ type K8sTestHelper struct {
 	env        server.TestEnv
 	namespacer request.NamespaceMapper
 
-	Org1 OrgUsers
+	Org1 OrgUsers // default
-	Org2 OrgUsers
+	OrgB OrgUsers // some other id
 
 	// // Registered groups
 	groups []metav1.APIGroup
 }
 
-func NewK8sTestHelper(t *testing.T) *K8sTestHelper {
+func NewK8sTestHelper(t *testing.T, opts testinfra.GrafanaOpts) *K8sTestHelper {
 	t.Helper()
-	dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
+	dir, path := testinfra.CreateGrafDir(t, opts)
-		AppModeProduction: true, // do not start extra port 6443
-		DisableAnonymous:  true,
-		EnableFeatureToggles: []string{
-			featuremgmt.FlagGrafanaAPIServer,
-			featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs,
-		},
-	})
-
 	_, env := testinfra.StartGrafanaEnv(t, dir, path)
 	c := &K8sTestHelper{
 		env:        *env,
@@ -68,8 +59,8 @@ func NewK8sTestHelper(t *testing.T) *K8sTestHelper {
 		namespacer: request.GetNamespaceMapper(nil),
 	}
 
-	c.Org1 = c.createTestUsers(int64(1))
+	c.Org1 = c.createTestUsers("Org1")
-	c.Org2 = c.createTestUsers(int64(2))
+	c.OrgB = c.createTestUsers("OrgB")
 
 	// Read the API groups
 	rsp := DoRequest(c, RequestParams{
@@ -81,6 +72,12 @@ func NewK8sTestHelper(t *testing.T) *K8sTestHelper {
 	return c
 }
 
+func (c *K8sTestHelper) Shutdown() {
+	fmt.Printf("calling shutdown on: %s\n", c.env.Server.HTTPServer.Listener.Addr())
+	err := c.env.Server.Shutdown(context.Background(), "done")
+	require.NoError(c.t, err)
+}
+
 type ResourceClientArgs struct {
 	User      User
 	Namespace string
@@ -327,20 +324,27 @@ func (c *K8sTestHelper) LoadYAMLOrJSON(body string) *unstructured.Unstructured {
 	return &unstructured.Unstructured{Object: unstructuredMap}
 }
 
-func (c K8sTestHelper) createTestUsers(orgId int64) OrgUsers {
+func (c K8sTestHelper) createTestUsers(orgName string) OrgUsers {
 	c.t.Helper()
 
 	store := c.env.SQLStore
-	store.Cfg.AutoAssignOrg = true
+	defer func() {
-	store.Cfg.AutoAssignOrgId = int(orgId)
+		store.Cfg.AutoAssignOrg = false
+		store.Cfg.AutoAssignOrgId = 1 // the default
+	}()
+
 	quotaService := quotaimpl.ProvideService(store, store.Cfg)
 
 	orgService, err := orgimpl.ProvideService(store, store.Cfg, quotaService)
 	require.NoError(c.t, err)
 
-	gotID, err := orgService.GetOrCreate(context.Background(), fmt.Sprintf("Org%d", orgId))
+	orgId := int64(1)
-	require.NoError(c.t, err)
+	if orgName != "Org1" {
-	require.Equal(c.t, orgId, gotID)
+		orgId, err = orgService.GetOrCreate(context.Background(), orgName)
+		require.NoError(c.t, err)
+	}
+	store.Cfg.AutoAssignOrg = true
+	store.Cfg.AutoAssignOrgId = int(orgId)
 
 	teamSvc := teamimpl.ProvideService(store, store.Cfg)
 	cache := localcache.ProvideService()
@@ -354,7 +358,7 @@ func (c K8sTestHelper) createTestUsers(orgId int64) OrgUsers {
 		u, err := userSvc.Create(context.Background(), &user.CreateUserCommand{
 			DefaultOrgRole: string(role),
 			Password:       key,
-			Login:          fmt.Sprintf("%s%d", key, orgId),
+			Login:          fmt.Sprintf("%s-%d", key, orgId),
 			OrgID:          orgId,
 		})
 		require.NoError(c.t, err)

pkg/tests/apis/playlist/playlist_test.go

@@ -14,21 +14,50 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
+	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/playlist"
 	"github.com/grafana/grafana/pkg/tests/apis"
+	"github.com/grafana/grafana/pkg/tests/testinfra"
 )
 
 func TestPlaylist(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping integration test")
 	}
-	helper := apis.NewK8sTestHelper(t)
+
+	t.Run("default setup", func(t *testing.T) {
+		doPlaylistTests(t, apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{
+			AppModeProduction: true, // do not start extra port 6443
+			DisableAnonymous:  true,
+			EnableFeatureToggles: []string{
+				featuremgmt.FlagGrafanaAPIServer,
+			},
+		}))
+	})
+
+	t.Run("with k8s api flag", func(t *testing.T) {
+		doPlaylistTests(t, apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{
+			AppModeProduction: true, // do not start extra port 6443
+			DisableAnonymous:  true,
+			EnableFeatureToggles: []string{
+				featuremgmt.FlagGrafanaAPIServer,
+				featuremgmt.FlagKubernetesPlaylistsAPI, // <<< The change we are testing!
+			},
+		}))
+	})
+}
+
+func doPlaylistTests(t *testing.T, helper *apis.K8sTestHelper) {
 	gvr := schema.GroupVersionResource{
 		Group:    "playlist.grafana.app",
 		Version:  "v0alpha1",
 		Resource: "playlists",
 	}
 
+	defer func() {
+		helper.Shutdown()
+	}()
+
 	t.Run("Check direct List permissions from different org users", func(t *testing.T) {
 		// Check view permissions
 		rsp := helper.List(helper.Org1.Viewer, "default", gvr)
@@ -38,13 +67,13 @@ func TestPlaylist(t *testing.T) {
 		require.Nil(t, rsp.Status)
 
 		// Check view permissions
-		rsp = helper.List(helper.Org2.Viewer, "default", gvr)
+		rsp = helper.List(helper.OrgB.Viewer, "default", gvr)
-		require.Equal(t, 403, rsp.Response.StatusCode) // Org2 can not see default namespace
+		require.Equal(t, 403, rsp.Response.StatusCode) // OrgB can not see default namespace
 		require.Nil(t, rsp.Result)
 		require.Equal(t, metav1.StatusReasonForbidden, rsp.Status.Reason)
 
 		// Check view permissions
-		rsp = helper.List(helper.Org2.Viewer, "org-22", gvr)
+		rsp = helper.List(helper.OrgB.Viewer, "org-22", gvr)
 		require.Equal(t, 403, rsp.Response.StatusCode) // Unknown/not a member
 		require.Nil(t, rsp.Result)
 		require.Equal(t, metav1.StatusReasonForbidden, rsp.Status.Reason)
@@ -63,7 +92,7 @@ func TestPlaylist(t *testing.T) {
 
 		// Check org2 viewer can not see org1 (default namespace)
 		client = helper.GetResourceClient(apis.ResourceClientArgs{
-			User:      helper.Org2.Viewer,
+			User:      helper.OrgB.Viewer,
 			Namespace: "default", // actually org1
 			GVR:       gvr,
 		})
@@ -74,7 +103,7 @@ func TestPlaylist(t *testing.T) {
 
 		// Check invalid namespace
 		client = helper.GetResourceClient(apis.ResourceClientArgs{
-			User:      helper.Org2.Viewer,
+			User:      helper.OrgB.Viewer,
 			Namespace: "org-22", // org 22 does not exist
 			GVR:       gvr,
 		})