IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Plugins: Improve grafana-cli UX + API response messaging for plugin install incompatibility scenario (#36556)

Browse Source 
* improve UX for plugin install incompatability

* refactor test
This commit is contained in:
Will Browne 2021-07-13 00:58:46 -07:00 committed by GitHub
parent 81511e34d9
commit e06335ffe9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 38 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
pkg/api/plugins.go
View File

@ -387,8 +387,9 @@ func (hs *HTTPServer) InstallPlugin(c *models.ReqContext, dto dtos.InstallPlugin
if errors.As(err, &versionNotFoundErr) {
return response.Error(http.StatusNotFound, "Plugin version not found", err)
}
if errors.Is(err, installer.ErrPluginNotFound) {
return response.Error(http.StatusNotFound, "Plugin not found", err)
var clientError installer.Response4xxError
if errors.As(err, &clientError) {
return response.Error(clientError.StatusCode, clientError.Message, err)
}
if errors.Is(err, plugins.ErrInstallCorePlugin) {
return response.Error(http.StatusForbidden, "Cannot install or change a Core plugin", err)

43
pkg/plugins/manager/installer/installer.go
View File

@ -41,20 +41,23 @@ const (
)
var (
ErrPluginNotFound = errors.New("plugin not found")
reGitBuild = regexp.MustCompile("^[a-zA-Z0-9_.-]*/")
reGitBuild = regexp.MustCompile("^[a-zA-Z0-9_.-]*/")
)
type BadRequestError struct {
Message string
Status string
type Response4xxError struct {
Message string
StatusCode int
SystemInfo string
}
func (e *BadRequestError) Error() string {
func (e Response4xxError) Error() string {
if len(e.Message) > 0 {
return fmt.Sprintf("%s: %s", e.Status, e.Message)
if len(e.SystemInfo) > 0 {
return fmt.Sprintf("%s (%s)", e.Message, e.SystemInfo)
}
return fmt.Sprintf("%d: %s", e.StatusCode, e.Message)
}
return e.Status
return fmt.Sprintf("%d", e.StatusCode)
}
type ErrVersionUnsupported struct {
@ -248,7 +251,7 @@ func (i *Installer) DownloadFile(pluginID string, tmpFile *os.File, url string,
// slow network. As this is CLI operation hanging is not a big of an issue as user can just abort.
bodyReader, err := i.sendRequestWithoutTimeout(url)
if err != nil {
return errutil.Wrap("Failed to send request", err)
return err
}
defer func() {
if err := bodyReader.Close(); err != nil {
@ -274,11 +277,7 @@ func (i *Installer) getPluginMetadataFromPluginRepo(pluginID, pluginRepoURL stri
i.log.Debugf("Fetching metadata for plugin \"%s\" from repo %s", pluginID, pluginRepoURL)
body, err := i.sendRequestGetBytes(pluginRepoURL, "repo", pluginID)
if err != nil {
if errors.Is(err, ErrPluginNotFound) {
i.log.Errorf("failed to find plugin '%s' in plugin repository. Please check if plugin ID is correct", pluginID)
return Plugin{}, err
}
return Plugin{}, errutil.Wrap("Failed to send request", err)
return Plugin{}, err
}
var data Plugin
@ -354,14 +353,6 @@ func (i *Installer) createRequest(URL string, subPaths ...string) (*http.Request
}
func (i *Installer) handleResponse(res *http.Response) (io.ReadCloser, error) {
if res.StatusCode == 404 {
return nil, ErrPluginNotFound
}
if res.StatusCode/100 != 2 && res.StatusCode/100 != 4 {
return nil, fmt.Errorf("API returned invalid status: %s", res.Status)
}
if res.StatusCode/100 == 4 {
body, err := ioutil.ReadAll(res.Body)
defer func() {
@ -370,7 +361,7 @@ func (i *Installer) handleResponse(res *http.Response) (io.ReadCloser, error) {
}
}()
if err != nil || len(body) == 0 {
return nil, &BadRequestError{Status: res.Status}
return nil, Response4xxError{StatusCode: res.StatusCode}
}
var message string
var jsonBody map[string]string
@ -380,7 +371,11 @@ func (i *Installer) handleResponse(res *http.Response) (io.ReadCloser, error) {
} else {
message = jsonBody["message"]
}
return nil, &BadRequestError{Status: res.Status, Message: message}
return nil, Response4xxError{StatusCode: res.StatusCode, Message: message, SystemInfo: i.fullSystemInfoString()}
}
if res.StatusCode/100 != 2 {
return nil, fmt.Errorf("API returned invalid status: %s", res.Status)
}
return res.Body, nil

26
pkg/tests/api/plugins/api_install_test.go
View File

@ -3,6 +3,7 @@ package plugins
import (
"bytes"
"context"
"encoding/json"
"fmt"
"io/ioutil"
"net/http"
@ -36,23 +37,24 @@ func TestPluginInstallAccess(t *testing.T) {
createUser(t, store, usernameAdmin, defaultPassword, true)
t.Run("Request is forbidden if not from an admin", func(t *testing.T) {
statusCode, body := makePostRequest(t, grafanaAPIURL(usernameNonAdmin, grafanaListedAddr, "plugins/grafana-plugin/install"))
assert.Equal(t, 403, statusCode)
assert.JSONEq(t, "{\"message\": \"Permission denied\"}", body)
status, body := makePostRequest(t, grafanaAPIURL(usernameNonAdmin, grafanaListedAddr, "plugins/grafana-plugin/install"))
assert.Equal(t, 403, status)
assert.Equal(t, "Permission denied", body["message"])
statusCode, body = makePostRequest(t, grafanaAPIURL(usernameNonAdmin, grafanaListedAddr, "plugins/grafana-plugin/uninstall"))
assert.Equal(t, 403, statusCode)
assert.JSONEq(t, "{\"message\": \"Permission denied\"}", body)
status, body = makePostRequest(t, grafanaAPIURL(usernameNonAdmin, grafanaListedAddr, "plugins/grafana-plugin/uninstall"))
assert.Equal(t, 403, status)
assert.Equal(t, "Permission denied", body["message"])
})
t.Run("Request is not forbidden if from an admin", func(t *testing.T) {
statusCode, body := makePostRequest(t, grafanaAPIURL(usernameAdmin, grafanaListedAddr, "plugins/test/install"))
assert.Equal(t, 404, statusCode)
assert.JSONEq(t, "{\"error\":\"plugin not found\", \"message\":\"Plugin not found\"}", body)
assert.Equal(t, "Plugin not found", body["message"])
statusCode, body = makePostRequest(t, grafanaAPIURL(usernameAdmin, grafanaListedAddr, "plugins/test/uninstall"))
assert.Equal(t, 404, statusCode)
assert.JSONEq(t, "{\"error\":\"plugin is not installed\", \"message\":\"Plugin not installed\"}", body)
assert.Equal(t, "Plugin not installed", body["message"])
})
}
@ -68,7 +70,7 @@ func createUser(t *testing.T, store *sqlstore.SQLStore, username, password strin
require.NoError(t, err)
}
func makePostRequest(t *testing.T, URL string) (int, string) {
func makePostRequest(t *testing.T, URL string) (int, map[string]interface{}) {
t.Helper()
// nolint:gosec
@ -81,7 +83,11 @@ func makePostRequest(t *testing.T, URL string) (int, string) {
b, err := ioutil.ReadAll(resp.Body)
require.NoError(t, err)
return resp.StatusCode, string(b)
var body = make(map[string]interface{})
err = json.Unmarshal(b, &body)
require.NoError(t, err)
return resp.StatusCode, body
}
func grafanaAPIURL(username string, grafanaListedAddr string, path string) string {