Correlations: Add UpdateCorrelation HTTP API (#52444)

* Correlations: add UpdateCorrelation HTTP API

* handle correlation not found

* add tests

* fix lint errors

* add bad request to API spec

* change casing

* fix casing in docs

* fix tests

* update spec

pkg/services/correlations/api.go

@@ -21,6 +21,7 @@ func (s *CorrelationsService) registerAPIEndpoints() {
 	s.RouteRegister.Group("/api/datasources/uid/:uid/correlations", func(entities routing.RouteRegister) {
 		entities.Post("/", middleware.ReqSignedIn, authorize(ac.ReqOrgAdmin, ac.EvalPermission(datasources.ActionWrite, uidScope)), routing.Wrap(s.createHandler))
 		entities.Delete("/:correlationUID", middleware.ReqSignedIn, authorize(ac.ReqOrgAdmin, ac.EvalPermission(datasources.ActionWrite, uidScope)), routing.Wrap(s.deleteHandler))
+		entities.Patch("/:correlationUID", middleware.ReqSignedIn, authorize(ac.ReqOrgAdmin, ac.EvalPermission(datasources.ActionWrite, uidScope)), routing.Wrap(s.updateHandler))
 	})
 }
 
@@ -127,3 +128,66 @@ type DeleteCorrelationResponse struct {
 	// in: body
 	Body DeleteCorrelationResponseBody `json:"body"`
 }
+
+// swagger:route PATCH /datasources/uid/{sourceUID}/correlations/{correlationUID} correlations updateCorrelation
+//
+// Updates a correlation.
+//
+// Responses:
+// 200: updateCorrelationResponse
+// 400: badRequestError
+// 401: unauthorisedError
+// 403: forbiddenError
+// 404: notFoundError
+// 500: internalServerError
+func (s *CorrelationsService) updateHandler(c *models.ReqContext) response.Response {
+	cmd := UpdateCorrelationCommand{}
+	if err := web.Bind(c.Req, &cmd); err != nil {
+		return response.Error(http.StatusBadRequest, "bad request data", err)
+	}
+
+	cmd.UID = web.Params(c.Req)[":correlationUID"]
+	cmd.SourceUID = web.Params(c.Req)[":uid"]
+	cmd.OrgId = c.OrgId
+
+	correlation, err := s.UpdateCorrelation(c.Req.Context(), cmd)
+	if err != nil {
+		if errors.Is(err, ErrUpdateCorrelationEmptyParams) {
+			return response.Error(http.StatusBadRequest, "At least one of label, description is required", err)
+		}
+
+		if errors.Is(err, ErrSourceDataSourceDoesNotExists) {
+			return response.Error(http.StatusNotFound, "Data source not found", err)
+		}
+
+		if errors.Is(err, ErrCorrelationNotFound) {
+			return response.Error(http.StatusNotFound, "Correlation not found", err)
+		}
+
+		if errors.Is(err, ErrSourceDataSourceReadOnly) {
+			return response.Error(http.StatusForbidden, "Data source is read only", err)
+		}
+
+		return response.Error(http.StatusInternalServerError, "Failed to update correlation", err)
+	}
+
+	return response.JSON(http.StatusOK, UpdateCorrelationResponseBody{Message: "Correlation updated", Result: correlation})
+}
+
+// swagger:parameters updateCorrelation
+type UpdateCorrelationParams struct {
+	// in:path
+	// required:true
+	DatasourceUID string `json:"sourceUID"`
+	// in:path
+	// required:true
+	CorrelationUID string `json:"correlationUID"`
+	// in: body
+	Body UpdateCorrelationCommand `json:"body"`
+}
+
+//swagger:response updateCorrelationResponse
+type UpdateCorrelationResponse struct {
+	// in: body
+	Body UpdateCorrelationResponseBody `json:"body"`
+}

pkg/services/correlations/correlations.go

@@ -52,6 +52,10 @@ func (s CorrelationsService) DeleteCorrelation(ctx context.Context, cmd DeleteCo
 	return s.deleteCorrelation(ctx, cmd)
 }
 
+func (s CorrelationsService) UpdateCorrelation(ctx context.Context, cmd UpdateCorrelationCommand) (Correlation, error) {
+	return s.updateCorrelation(ctx, cmd)
+}
+
 func (s CorrelationsService) DeleteCorrelationsBySourceUID(ctx context.Context, cmd DeleteCorrelationsBySourceUIDCommand) error {
 	return s.deleteCorrelationsBySourceUID(ctx, cmd)
 }

pkg/services/correlations/database.go

@@ -77,6 +77,61 @@ func (s CorrelationsService) deleteCorrelation(ctx context.Context, cmd DeleteCo
 	})
 }
 
+func (s CorrelationsService) updateCorrelation(ctx context.Context, cmd UpdateCorrelationCommand) (Correlation, error) {
+	correlation := Correlation{
+		UID:       cmd.UID,
+		SourceUID: cmd.SourceUID,
+	}
+
+	err := s.SQLStore.WithTransactionalDbSession(ctx, func(session *sqlstore.DBSession) error {
+		query := &datasources.GetDataSourceQuery{
+			OrgId: cmd.OrgId,
+			Uid:   cmd.SourceUID,
+		}
+		if err := s.DataSourceService.GetDataSource(ctx, query); err != nil {
+			return ErrSourceDataSourceDoesNotExists
+		}
+
+		if query.Result.ReadOnly {
+			return ErrSourceDataSourceReadOnly
+		}
+
+		if cmd.Label == nil && cmd.Description == nil {
+			return ErrUpdateCorrelationEmptyParams
+		}
+		update := Correlation{}
+		if cmd.Label != nil {
+			update.Label = *cmd.Label
+			session.MustCols("label")
+		}
+		if cmd.Description != nil {
+			update.Description = *cmd.Description
+			session.MustCols("description")
+		}
+
+		updateCount, err := session.Where("uid = ? AND source_uid = ?", correlation.UID, correlation.SourceUID).Limit(1).Update(update)
+		if updateCount == 0 {
+			return ErrCorrelationNotFound
+		}
+		if err != nil {
+			return err
+		}
+
+		found, err := session.Get(&correlation)
+		if !found {
+			return ErrCorrelationNotFound
+		}
+
+		return err
+	})
+
+	if err != nil {
+		return Correlation{}, err
+	}
+
+	return correlation, nil
+}
+
 func (s CorrelationsService) deleteCorrelationsBySourceUID(ctx context.Context, cmd DeleteCorrelationsBySourceUIDCommand) error {
 	return s.SQLStore.WithDbSession(ctx, func(session *sqlstore.DBSession) error {
 		_, err := session.Delete(&Correlation{SourceUID: cmd.SourceUID})

pkg/services/correlations/models.go

@@ -9,8 +9,8 @@ var (
 	ErrSourceDataSourceDoesNotExists      = errors.New("source data source does not exist")
 	ErrTargetDataSourceDoesNotExists      = errors.New("target data source does not exist")
 	ErrCorrelationFailedGenerateUniqueUid = errors.New("failed to generate unique correlation UID")
-	ErrCorrelationIdentifierNotSet        = errors.New("source identifier and org id are needed to be able to edit correlations")
 	ErrCorrelationNotFound                = errors.New("correlation not found")
+	ErrUpdateCorrelationEmptyParams       = errors.New("not enough parameters to edit correlation")
 )
 
 // Correlation is the model for correlations definitions
@@ -72,6 +72,28 @@ type DeleteCorrelationCommand struct {
 	OrgId     int64
 }
 
+// swagger:model
+type UpdateCorrelationResponseBody struct {
+	Result Correlation `json:"result"`
+	// example: Correlation updated
+	Message string `json:"message"`
+}
+
+// UpdateCorrelationCommand is the command for updating a correlation
+type UpdateCorrelationCommand struct {
+	// UID of the correlation to be deleted.
+	UID       string `json:"-"`
+	SourceUID string `json:"-"`
+	OrgId     int64  `json:"-"`
+
+	// Optional label identifying the correlation
+	// example: My label
+	Label *string `json:"label"`
+	// Optional description of the correlation
+	// example: Logs to Traces
+	Description *string `json:"description"`
+}
+
 type DeleteCorrelationsBySourceUIDCommand struct {
 	SourceUID string
 }

pkg/tests/api/correlations/common_test.go

@@ -64,6 +64,25 @@ func (c TestContext) Post(params PostParams) *http.Response {
 	return resp
 }
 
+type PatchParams struct {
+	url  string
+	body string
+	user User
+}
+
+func (c TestContext) Patch(params PatchParams) *http.Response {
+	c.t.Helper()
+
+	req, err := http.NewRequest(http.MethodPatch, c.getURL(params.url, params.user), bytes.NewBuffer([]byte(params.body)))
+	req.Header.Set("Content-Type", "application/json")
+	require.NoError(c.t, err)
+	resp, err := http.DefaultClient.Do(req)
+	require.NoError(c.t, err)
+	require.NoError(c.t, err)
+
+	return resp
+}
+
 type DeleteParams struct {
 	url  string
 	user User

pkg/tests/api/correlations/correlations_update_test.go

@@ -0,0 +1,356 @@
+package correlations
+
+import (
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"testing"
+
+	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/correlations"
+	"github.com/grafana/grafana/pkg/services/datasources"
+	"github.com/grafana/grafana/pkg/services/user"
+	"github.com/stretchr/testify/require"
+)
+
+func TestIntegrationUpdateCorrelation(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
+	ctx := NewTestEnv(t)
+
+	adminUser := User{
+		username: "admin",
+		password: "admin",
+	}
+	editorUser := User{
+		username: "editor",
+		password: "editor",
+	}
+
+	ctx.createUser(user.CreateUserCommand{
+		DefaultOrgRole: string(models.ROLE_EDITOR),
+		Password:       editorUser.password,
+		Login:          editorUser.username,
+	})
+	ctx.createUser(user.CreateUserCommand{
+		DefaultOrgRole: string(models.ROLE_ADMIN),
+		Password:       adminUser.password,
+		Login:          adminUser.username,
+	})
+
+	createDsCommand := &datasources.AddDataSourceCommand{
+		Name:     "read-only",
+		Type:     "loki",
+		ReadOnly: true,
+		OrgId:    1,
+	}
+	ctx.createDs(createDsCommand)
+	readOnlyDS := createDsCommand.Result.Uid
+
+	createDsCommand = &datasources.AddDataSourceCommand{
+		Name:  "writable",
+		Type:  "loki",
+		OrgId: 1,
+	}
+	ctx.createDs(createDsCommand)
+	writableDs := createDsCommand.Result.Uid
+	writableDsOrgId := createDsCommand.Result.OrgId
+
+	t.Run("Unauthenticated users shouldn't be able to update correlations", func(t *testing.T) {
+		res := ctx.Patch(PatchParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", "some-ds-uid", "some-correlation-uid"),
+			body: ``,
+		})
+		require.Equal(t, http.StatusUnauthorized, res.StatusCode)
+
+		responseBody, err := ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+
+		var response errorResponseBody
+		err = json.Unmarshal(responseBody, &response)
+		require.NoError(t, err)
+
+		require.Equal(t, "Unauthorized", response.Message)
+
+		require.NoError(t, res.Body.Close())
+	})
+
+	t.Run("non org admin shouldn't be able to update correlations", func(t *testing.T) {
+		res := ctx.Patch(PatchParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", "some-ds-uid", "some-correlation-uid"),
+			body: `{}`,
+			user: editorUser,
+		})
+		require.Equal(t, http.StatusForbidden, res.StatusCode)
+
+		responseBody, err := ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+
+		var response errorResponseBody
+		err = json.Unmarshal(responseBody, &response)
+		require.NoError(t, err)
+
+		require.Contains(t, response.Message, "Permissions needed: datasources:write")
+
+		require.NoError(t, res.Body.Close())
+	})
+
+	t.Run("inexistent source data source should result in a 404", func(t *testing.T) {
+		res := ctx.Patch(PatchParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", "some-ds-uid", "some-correlation-uid"),
+			body: `{}`,
+			user: adminUser,
+		})
+		require.Equal(t, http.StatusNotFound, res.StatusCode)
+
+		responseBody, err := ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+
+		var response errorResponseBody
+		err = json.Unmarshal(responseBody, &response)
+		require.NoError(t, err)
+
+		require.Equal(t, "Data source not found", response.Message)
+		require.Equal(t, correlations.ErrSourceDataSourceDoesNotExists.Error(), response.Error)
+
+		require.NoError(t, res.Body.Close())
+	})
+
+	t.Run("inexistent correlation should result in a 404", func(t *testing.T) {
+		res := ctx.Patch(PatchParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", writableDs, "nonexistent-correlation-uid"),
+			user: adminUser,
+			body: `{
+				"label": ""
+			}`,
+		})
+		require.Equal(t, http.StatusNotFound, res.StatusCode)
+
+		responseBody, err := ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+
+		var response errorResponseBody
+		err = json.Unmarshal(responseBody, &response)
+		require.NoError(t, err)
+
+		require.Equal(t, "Correlation not found", response.Message)
+		require.Equal(t, correlations.ErrCorrelationNotFound.Error(), response.Error)
+
+		require.NoError(t, res.Body.Close())
+	})
+
+	t.Run("updating a correlation originating from a read-only data source should result in a 403", func(t *testing.T) {
+		res := ctx.Patch(PatchParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", readOnlyDS, "nonexistent-correlation-uid"),
+			user: adminUser,
+			body: `{}`,
+		})
+		require.Equal(t, http.StatusForbidden, res.StatusCode)
+
+		responseBody, err := ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+
+		var response errorResponseBody
+		err = json.Unmarshal(responseBody, &response)
+		require.NoError(t, err)
+
+		require.Equal(t, "Data source is read only", response.Message)
+		require.Equal(t, correlations.ErrSourceDataSourceReadOnly.Error(), response.Error)
+
+		require.NoError(t, res.Body.Close())
+	})
+
+	t.Run("updating a without data should result in a 400", func(t *testing.T) {
+		correlation := ctx.createCorrelation(correlations.CreateCorrelationCommand{
+			SourceUID: writableDs,
+			TargetUID: writableDs,
+			OrgId:     writableDsOrgId,
+		})
+
+		// no params
+		res := ctx.Patch(PatchParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", correlation.SourceUID, correlation.UID),
+			user: adminUser,
+			body: `{}`,
+		})
+		require.Equal(t, http.StatusBadRequest, res.StatusCode)
+
+		responseBody, err := ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+
+		var response errorResponseBody
+		err = json.Unmarshal(responseBody, &response)
+		require.NoError(t, err)
+
+		require.Equal(t, "At least one of label, description is required", response.Message)
+		require.Equal(t, correlations.ErrUpdateCorrelationEmptyParams.Error(), response.Error)
+		require.NoError(t, res.Body.Close())
+
+		// empty body
+		res = ctx.Patch(PatchParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", correlation.SourceUID, correlation.UID),
+			user: adminUser,
+			body: ``,
+		})
+		require.Equal(t, http.StatusBadRequest, res.StatusCode)
+
+		responseBody, err = ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+
+		err = json.Unmarshal(responseBody, &response)
+		require.NoError(t, err)
+
+		require.Equal(t, "At least one of label, description is required", response.Message)
+		require.Equal(t, correlations.ErrUpdateCorrelationEmptyParams.Error(), response.Error)
+		require.NoError(t, res.Body.Close())
+
+		// all set to null
+		res = ctx.Patch(PatchParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", correlation.SourceUID, correlation.UID),
+			user: adminUser,
+			body: `{
+						"label": null,
+						"description": null
+					}`,
+		})
+		require.Equal(t, http.StatusBadRequest, res.StatusCode)
+
+		responseBody, err = ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+
+		err = json.Unmarshal(responseBody, &response)
+		require.NoError(t, err)
+
+		require.Equal(t, "At least one of label, description is required", response.Message)
+		require.Equal(t, correlations.ErrUpdateCorrelationEmptyParams.Error(), response.Error)
+		require.NoError(t, res.Body.Close())
+	})
+
+	t.Run("updating a correlation pointing to a read-only data source should work", func(t *testing.T) {
+		correlation := ctx.createCorrelation(correlations.CreateCorrelationCommand{
+			SourceUID: writableDs,
+			TargetUID: writableDs,
+			OrgId:     writableDsOrgId,
+			Label:     "a label",
+		})
+
+		res := ctx.Patch(PatchParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", correlation.SourceUID, correlation.UID),
+			user: adminUser,
+			body: `{
+				"label": "updated label"
+			}`,
+		})
+		require.Equal(t, http.StatusOK, res.StatusCode)
+
+		responseBody, err := ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+
+		var response correlations.UpdateCorrelationResponseBody
+		err = json.Unmarshal(responseBody, &response)
+		require.NoError(t, err)
+
+		require.Equal(t, "Correlation updated", response.Message)
+		require.Equal(t, "updated label", response.Result.Label)
+		require.NoError(t, res.Body.Close())
+	})
+
+	t.Run("should correctly update correlations", func(t *testing.T) {
+		correlation := ctx.createCorrelation(correlations.CreateCorrelationCommand{
+			SourceUID:   writableDs,
+			TargetUID:   writableDs,
+			OrgId:       writableDsOrgId,
+			Label:       "0",
+			Description: "0",
+		})
+
+		// updating all
+		res := ctx.Patch(PatchParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", correlation.SourceUID, correlation.UID),
+			user: adminUser,
+			body: `{
+				"label": "1",
+				"description": "1"
+			}`,
+		})
+		require.Equal(t, http.StatusOK, res.StatusCode)
+
+		responseBody, err := ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+
+		var response correlations.UpdateCorrelationResponseBody
+		err = json.Unmarshal(responseBody, &response)
+		require.NoError(t, err)
+
+		require.Equal(t, "Correlation updated", response.Message)
+		require.Equal(t, "1", response.Result.Label)
+		require.Equal(t, "1", response.Result.Label)
+		require.NoError(t, res.Body.Close())
+
+		// partially updating only label
+		res = ctx.Patch(PatchParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", correlation.SourceUID, correlation.UID),
+			user: adminUser,
+			body: `{
+				"label": "2"
+			}`,
+		})
+		require.Equal(t, http.StatusOK, res.StatusCode)
+
+		responseBody, err = ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+
+		err = json.Unmarshal(responseBody, &response)
+		require.NoError(t, err)
+
+		require.Equal(t, "Correlation updated", response.Message)
+		require.Equal(t, "2", response.Result.Label)
+		require.Equal(t, "1", response.Result.Description)
+		require.NoError(t, res.Body.Close())
+
+		// partially updating only description
+		res = ctx.Patch(PatchParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", correlation.SourceUID, correlation.UID),
+			user: adminUser,
+			body: `{
+				"description": "2"
+			}`,
+		})
+		require.Equal(t, http.StatusOK, res.StatusCode)
+
+		responseBody, err = ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+
+		err = json.Unmarshal(responseBody, &response)
+		require.NoError(t, err)
+
+		require.Equal(t, "Correlation updated", response.Message)
+		require.Equal(t, "2", response.Result.Label)
+		require.Equal(t, "2", response.Result.Description)
+		require.NoError(t, res.Body.Close())
+
+		// setting both to empty strings (testing wether empty strings are handled correctly)
+		res = ctx.Patch(PatchParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", correlation.SourceUID, correlation.UID),
+			user: adminUser,
+			body: `{
+				"label": "",
+				"description": ""
+			}`,
+		})
+		require.Equal(t, http.StatusOK, res.StatusCode)
+
+		responseBody, err = ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+
+		err = json.Unmarshal(responseBody, &response)
+		require.NoError(t, err)
+
+		require.Equal(t, "Correlation updated", response.Message)
+		require.Equal(t, "", response.Result.Label)
+		require.Equal(t, "", response.Result.Description)
+		require.NoError(t, res.Body.Close())
+	})
+}