Skip to content

Commit

Permalink
feat(api): Implement delete app functionality as an HTTP endpoint bac…
Browse files Browse the repository at this point in the history
…kend #1223  (#1239)
  • Loading branch information
korniltsev authored Jul 29, 2022
1 parent fcc9330 commit b82f426
Show file tree
Hide file tree
Showing 16 changed files with 305 additions and 89 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/pyroscope-io/pyroscope

go 1.17
go 1.18

require (
github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,6 @@ golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96b
golang.org/x/net v0.0.0-20210421230115-4e50805a0758/go.mod h1:72T/g9IO56b78aLF+1Kcs5dz7/ng1VjMUvfKvpfy+jM=
golang.org/x/net v0.0.0-20210428140749-89ef3d95e781/go.mod h1:OJAsFXCWl8Ukc7SiCT/9KSuxbyM7479/AVlXFRxuMCk=
golang.org/x/net v0.0.0-20210525063256-abc453219eb5/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
golang.org/x/net v0.0.0-20220225172249-27dd8689420f/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
Expand Down Expand Up @@ -1014,7 +1013,6 @@ golang.org/x/sys v0.0.0-20210603081109-ebe580a85c40/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20211019181941-9d821ace8654/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220114195835-da31bd327af9/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220209214540-3681064d5158/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
Expand Down
3 changes: 1 addition & 2 deletions pkg/admin/admin_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ var _ = Describe("integration", func() {

httpServer, err := admin.NewUdsHTTPServer(socketAddr, http)
Expect(err).ToNot(HaveOccurred())
svc := admin.NewService(mockStorage{})
ctrl := admin.NewController(logger, svc, mockUserService{}, mockStorageService{})
ctrl := admin.NewController(logger, mockStorage{}, mockUserService{}, mockStorageService{})
s, err := admin.NewServer(logger, ctrl, httpServer)
Expect(err).ToNot(HaveOccurred())
server = s
Expand Down
3 changes: 2 additions & 1 deletion pkg/admin/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/pyroscope-io/pyroscope/pkg/server"
"io"
"net/http"
"time"
Expand Down Expand Up @@ -67,7 +68,7 @@ func (c *Client) GetAppsNames() (names AppNames, err error) {
func (c *Client) DeleteApp(name string) (err error) {
// we are kinda robbing here
// since the server and client are defined in the same package
payload := DeleteAppInput{
payload := server.DeleteAppInput{
Name: name,
}

Expand Down
52 changes: 13 additions & 39 deletions pkg/admin/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"context"
"encoding/json"
"fmt"
"github.com/pyroscope-io/pyroscope/pkg/server/httputils"
pstorage "github.com/pyroscope-io/pyroscope/pkg/storage"
"net/http"

"github.com/gorilla/mux"
Expand All @@ -14,10 +16,16 @@ import (
"github.com/pyroscope-io/pyroscope/pkg/model"
)

type Controller struct {
log *logrus.Logger
type Storage interface {
pstorage.AppNameGetter
pstorage.AppGetter
pstorage.AppDeleter
}

adminService *AdminService
type Controller struct {
log *logrus.Logger
httpUtils httputils.Utils
storage Storage
userService UserService
storageService StorageService
}
Expand All @@ -32,52 +40,18 @@ type StorageService interface {

func NewController(
log *logrus.Logger,
adminService *AdminService,
storage Storage,
userService UserService,
storageService StorageService) *Controller {
return &Controller{
log: log,

adminService: adminService,
storage: storage,
userService: userService,
storageService: storageService,
}
}

// HandleGetApps handles GET requests
func (ctrl *Controller) HandleGetApps(w http.ResponseWriter, _ *http.Request) {
appNames := ctrl.adminService.GetApps()

w.WriteHeader(http.StatusOK)
ctrl.writeResponseJSON(w, appNames)
}

type DeleteAppInput struct {
Name string `json:"name"`
}

// HandleDeleteApp handles DELETE requests
func (ctrl *Controller) HandleDeleteApp(w http.ResponseWriter, r *http.Request) {
var payload DeleteAppInput

err := json.NewDecoder(r.Body).Decode(&payload)
if err != nil {
ctrl.writeError(w, http.StatusBadRequest, err, "")
return
}

err = ctrl.adminService.DeleteApp(payload.Name)
if err != nil {
// TODO how to distinguish
// it was a bad request
// or an internal server error
ctrl.writeError(w, http.StatusInternalServerError, err, "")
return
}

w.WriteHeader(http.StatusOK)
}

type UpdateUserRequest struct {
Password *string `json:"password"`
IsDisabled *bool `json:"isDisabled"`
Expand Down
14 changes: 10 additions & 4 deletions pkg/admin/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"context"
"encoding/json"
"fmt"
"github.com/pyroscope-io/pyroscope/pkg/server"
"github.com/pyroscope-io/pyroscope/pkg/storage"
"io"
"net/http"
"net/http/httptest"
Expand All @@ -19,13 +21,18 @@ import (

type mockStorage struct {
getAppNamesResult []string
getAppsResult storage.GetAppsOutput
deleteResult error
}

func (m mockStorage) GetAppNames(ctx context.Context) []string {
return m.getAppNamesResult
}

func (m mockStorage) GetApps(ctx context.Context) storage.GetAppsOutput {
return m.getAppsResult
}

func (m mockStorage) DeleteApp(ctx context.Context, appname string) error {
return m.deleteResult
}
Expand Down Expand Up @@ -60,8 +67,7 @@ var _ = Describe("controller", func() {
// create a null logger, since we aren't interested
logger, _ := test.NewNullLogger()

svc := admin.NewService(storage)
ctrl := admin.NewController(logger, svc, mockUserService{}, mockStorageService{})
ctrl := admin.NewController(logger, storage, mockUserService{}, mockStorageService{})
httpServer := &admin.UdsHTTPServer{}
server, err := admin.NewServer(logger, ctrl, httpServer)

Expand Down Expand Up @@ -94,7 +100,7 @@ var _ = Describe("controller", func() {
It("returns StatusOK", func() {
// we are kinda robbing here
// since the server and client are defined in the same package
payload := admin.DeleteAppInput{Name: "myapp"}
payload := server.DeleteAppInput{Name: "myapp"}
marshalledPayload, err := json.Marshal(payload)
request, err := http.NewRequest(http.MethodDelete, "/v1/apps", bytes.NewBuffer(marshalledPayload))
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -125,7 +131,7 @@ var _ = Describe("controller", func() {
It("returns InternalServerError", func() {
// we are kinda robbing here
// since the server and client are defined in the same package
payload := admin.DeleteAppInput{Name: "myapp"}
payload := server.DeleteAppInput{Name: "myapp"}
marshalledPayload, err := json.Marshal(payload)
request, err := http.NewRequest(http.MethodDelete, "/v1/apps", bytes.NewBuffer(marshalledPayload))
Expect(err).ToNot(HaveOccurred())
Expand Down
12 changes: 8 additions & 4 deletions pkg/admin/server.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package admin

import (
"github.com/pyroscope-io/pyroscope/pkg/server"
"github.com/pyroscope-io/pyroscope/pkg/server/httputils"
"net/http"
"os"

Expand Down Expand Up @@ -37,11 +39,13 @@ func NewServer(logger *logrus.Logger, ctrl *Controller, httpServer HTTPServer) (

as.Handler = r

httpUtils := httputils.NewDefaultHelper(logger)
// Routes
r.HandleFunc("/v1/apps", as.ctrl.HandleGetApps).Methods("GET")
r.HandleFunc("/v1/apps", as.ctrl.HandleDeleteApp).Methods("DELETE")
r.HandleFunc("/v1/users/{username}", as.ctrl.UpdateUserHandler).Methods("PATCH")
r.HandleFunc("/v1/storage/cleanup", as.ctrl.StorageCleanupHandler).Methods("PUT")

r.HandleFunc("/v1/apps", server.NewGetAppNamesHandler(ctrl.storage, httpUtils)).Methods("GET")
r.HandleFunc("/v1/apps", server.NewDeleteAppHandler(ctrl.storage, httpUtils)).Methods("DELETE")
r.HandleFunc("/v1/users/{username}", ctrl.UpdateUserHandler).Methods("PATCH")
r.HandleFunc("/v1/storage/cleanup", ctrl.StorageCleanupHandler).Methods("PUT")

// Global middlewares
r.Use(logginMiddleware)
Expand Down
28 changes: 0 additions & 28 deletions pkg/admin/service.go

This file was deleted.

5 changes: 2 additions & 3 deletions pkg/cli/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,8 @@ func newServerService(c *config.Server) (*serverService, error) {
// this needs to happen after storage is initiated!
if svc.config.EnableExperimentalAdmin {
socketPath := svc.config.AdminSocketPath
adminService := admin.NewService(svc.storage)
userService := service.NewUserService(svc.database.DB())
adminCtrl := admin.NewController(svc.logger, adminService, userService, svc.storage)
adminController := admin.NewController(svc.logger, svc.storage, userService, svc.storage)
httpClient, err := admin.NewHTTPOverUDSClient(socketPath)
if err != nil {
return nil, fmt.Errorf("admin: %w", err)
Expand All @@ -134,7 +133,7 @@ func newServerService(c *config.Server) (*serverService, error) {

svc.adminServer, err = admin.NewServer(
svc.logger,
adminCtrl,
adminController,
adminHTTPOverUDS,
)
if err != nil {
Expand Down
78 changes: 78 additions & 0 deletions pkg/server/apps.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package server

import (
"encoding/json"
"github.com/pyroscope-io/pyroscope/pkg/server/httputils"
"github.com/pyroscope-io/pyroscope/pkg/storage"
"net/http"
)

type AppInfo struct {
Name string `json:"name"`
}

type DeleteAppInput struct {
Name string `json:"name"`
}

func (c *Controller) getAppsHandler() http.HandlerFunc {
return NewGetAppsHandler(c.storage, c.httpUtils)
}

func NewGetAppsHandler(s storage.AppGetter, httpUtils httputils.Utils) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
apps, err := s.GetApps(r.Context())
if err != nil {
httpUtils.WriteError(r, w, http.StatusInternalServerError, err, "")
return
}
res := make([]AppInfo, 0, len(apps.Apps))
for _, app := range apps.Apps {
it := AppInfo{
Name: app.Name,
}
res = append(res, it)
}
w.WriteHeader(http.StatusOK)
httpUtils.WriteResponseJSON(r, w, res)
}
}

func (c *Controller) getAppNames() http.HandlerFunc {
return NewGetAppNamesHandler(c.storage, c.httpUtils)
}

func NewGetAppNamesHandler(s storage.AppNameGetter, httpUtils httputils.Utils) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
appNames := s.GetAppNames(r.Context())
w.WriteHeader(http.StatusOK)
httpUtils.WriteResponseJSON(r, w, appNames)
}
}

func (c *Controller) deleteAppsHandler() http.HandlerFunc {
return NewDeleteAppHandler(c.storage, c.httpUtils)
}

func NewDeleteAppHandler(s storage.AppDeleter, httpUtils httputils.Utils) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
var payload DeleteAppInput

err := json.NewDecoder(r.Body).Decode(&payload)
if err != nil {
httpUtils.WriteError(r, w, http.StatusBadRequest, err, "")
return
}

err = s.DeleteApp(r.Context(), payload.Name)
if err != nil {
// TODO how to distinguish
// it was a bad request
// or an internal server error
httpUtils.WriteError(r, w, http.StatusInternalServerError, err, "")
return
}

w.WriteHeader(http.StatusOK)
}
}
10 changes: 10 additions & 0 deletions pkg/server/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"gorm.io/gorm"

adhocserver "github.com/pyroscope-io/pyroscope/pkg/adhoc/server"

"github.com/pyroscope-io/pyroscope/pkg/api"
"github.com/pyroscope-io/pyroscope/pkg/api/authz"
"github.com/pyroscope-io/pyroscope/pkg/api/router"
Expand Down Expand Up @@ -280,6 +281,15 @@ func (ctrl *Controller) serverMux() (http.Handler, error) {
ctrl.drainMiddleware,
ctrl.authMiddleware(nil))

appsRouter := apiRouter.PathPrefix("/").Subrouter()
appsRouter.Use(
api.AuthMiddleware(nil, ctrl.authService, ctrl.httpUtils),
authz.NewAuthorizer(ctrl.log, httputils.NewDefaultHelper(ctrl.log)).RequireOneOf(
authz.Role(model.AdminRole),
))
appsRouter.HandleFunc("/apps", ctrl.getAppsHandler()).Methods("GET")
appsRouter.HandleFunc("/apps", ctrl.deleteAppsHandler()).Methods("DELETE")

// TODO(kolesnikovae):
// Refactor: move mux part to pkg/api/router.
// Make prometheus middleware to support gorilla patterns.
Expand Down
Loading

0 comments on commit b82f426

Please sign in to comment.