Correlations: Add DeleteCorrelation HTTP API (#51801)

* Correlations: add DeleteCorrelation HTTP API

* fix error message copy

* add readonly check

* add source_uid in delete condition

* make path singular

* Revert "make path singular"

This reverts commit d15be89578e202e5cb64a3e964ee09521b72d87c.

* add tests

* fix lint errors

* fix lint errors

* change casing

* update spec

* Remove transaction

* change casing in param name in docs

docs/sources/developers/http_api/correlations.md

@@ -68,3 +68,36 @@ Status codes:
 - **403** – Forbidden, source data source is read-only
 - **404** – Not found, either source or target data source could not be found
 - **500** – Internal error
+
+## Delete correlations
+
+`DELETE /api/datasources/uid/:sourceUID/correlations/:correlationUID`
+
+Deletes a correlation.
+
+**Example request:**
+
+```http
+DELETE /api/datasources/uid/uyBf2637k/correlations/J6gn7d31L HTTP/1.1
+Accept: application/json
+Content-Type: application/json
+Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk
+```
+
+**Example response:**
+
+```http
+HTTP/1.1 200
+Content-Type: application/json
+{
+  "message": "Correlation deleted"
+}
+```
+
+Status codes:
+
+- **200** – OK
+- **401** – Unauthorized
+- **403** – Forbidden, data source is read-only
+- **404** – Correlation not found
+- **500** – Internal error

pkg/api/docs/definitions/correlations.go

@@ -31,3 +31,30 @@ type CreateCorrelationResponse struct {
 	// in: body
 	Body correlations.CreateCorrelationResponse `json:"body"`
 }
+
+// swagger:route DELETE /datasources/uid/{uid}/correlations/{correlationUID} correlations deleteCorrelation
+//
+// Delete a correlation.
+//
+// Responses:
+// 200: deleteCorrelationResponse
+// 401: unauthorisedError
+// 403: forbiddenError
+// 404: notFoundError
+// 500: internalServerError
+
+// swagger:parameters deleteCorrelation
+type DeleteCorrelationParams struct {
+	// in:path
+	// required:true
+	DatasourceUID string `json:"uid"`
+	// in:path
+	// required:true
+	CorrelationUID string `json:"correlationUID"`
+}
+
+//swagger:response deleteCorrelationResponse
+type DeleteCorrelationResponse struct {
+	// in: body
+	Body correlations.DeleteCorrelationResponse `json:"body"`
+}

pkg/services/correlations/api.go

@@ -20,6 +20,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))
 	})
 }
 
@@ -47,3 +48,31 @@ func (s *CorrelationsService) createHandler(c *models.ReqContext) response.Respo
 
 	return response.JSON(http.StatusOK, CreateCorrelationResponse{Result: correlation, Message: "Correlation created"})
 }
+
+// deleteHandler handles DELETE /datasources/uid/:uid/correlations/:correlationUID
+func (s *CorrelationsService) deleteHandler(c *models.ReqContext) response.Response {
+	cmd := DeleteCorrelationCommand{
+		UID:       web.Params(c.Req)[":correlationUID"],
+		SourceUID: web.Params(c.Req)[":uid"],
+		OrgId:     c.OrgId,
+	}
+
+	err := s.DeleteCorrelation(c.Req.Context(), cmd)
+	if err != nil {
+		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 delete correlation", err)
+	}
+
+	return response.JSON(http.StatusOK, DeleteCorrelationResponse{Message: "Correlation deleted"})
+}

pkg/services/correlations/correlations.go

@@ -31,6 +31,7 @@ func ProvideService(sqlStore *sqlstore.SQLStore, routeRegister routing.RouteRegi
 
 type Service interface {
 	CreateCorrelation(ctx context.Context, cmd CreateCorrelationCommand) (Correlation, error)
+	DeleteCorrelation(ctx context.Context, cmd DeleteCorrelationCommand) error
 	DeleteCorrelationsBySourceUID(ctx context.Context, cmd DeleteCorrelationsBySourceUIDCommand) error
 	DeleteCorrelationsByTargetUID(ctx context.Context, cmd DeleteCorrelationsByTargetUIDCommand) error
 }
@@ -47,6 +48,10 @@ func (s CorrelationsService) CreateCorrelation(ctx context.Context, cmd CreateCo
 	return s.createCorrelation(ctx, cmd)
 }
 
+func (s CorrelationsService) DeleteCorrelation(ctx context.Context, cmd DeleteCorrelationCommand) error {
+	return s.deleteCorrelation(ctx, cmd)
+}
+
 func (s CorrelationsService) DeleteCorrelationsBySourceUID(ctx context.Context, cmd DeleteCorrelationsBySourceUIDCommand) error {
 	return s.deleteCorrelationsBySourceUID(ctx, cmd)
 }

pkg/services/correlations/database.go

@@ -55,6 +55,28 @@ func (s CorrelationsService) createCorrelation(ctx context.Context, cmd CreateCo
 	return correlation, nil
 }
 
+func (s CorrelationsService) deleteCorrelation(ctx context.Context, cmd DeleteCorrelationCommand) error {
+	return s.SQLStore.WithDbSession(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
+		}
+
+		deletedCount, err := session.Delete(&Correlation{UID: cmd.UID, SourceUID: cmd.SourceUID})
+		if deletedCount == 0 {
+			return ErrCorrelationNotFound
+		}
+		return err
+	})
+}
+
 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

@@ -10,6 +10,7 @@ var (
 	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")
 )
 
 // Correlation is the model for correlations definitions
@@ -57,6 +58,20 @@ type CreateCorrelationCommand struct {
 	Description string `json:"description"`
 }
 
+// swagger:model
+type DeleteCorrelationResponse struct {
+	// example: Correlation deleted
+	Message string `json:"message"`
+}
+
+// DeleteCorrelationCommand is the command for deleting a correlation
+type DeleteCorrelationCommand struct {
+	// UID of the correlation to be deleted.
+	UID       string
+	SourceUID string
+	OrgId     int64
+}
+
 type DeleteCorrelationsBySourceUIDCommand struct {
 	SourceUID string
 }

pkg/tests/api/correlations/common_test.go

@@ -8,12 +8,18 @@ import (
 	"testing"
 
 	"github.com/grafana/grafana/pkg/server"
+	"github.com/grafana/grafana/pkg/services/correlations"
 	"github.com/grafana/grafana/pkg/services/datasources"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/tests/testinfra"
 	"github.com/stretchr/testify/require"
 )
 
+type errorResponseBody struct {
+	Message string `json:"message"`
+	Error   string `json:"error"`
+}
+
 type TestContext struct {
 	env server.TestEnv
 	t   *testing.T
@@ -46,18 +52,10 @@ type PostParams struct {
 func (c TestContext) Post(params PostParams) *http.Response {
 	c.t.Helper()
 	buf := bytes.NewReader([]byte(params.body))
-	baseUrl := fmt.Sprintf("http://%s", c.env.Server.HTTPServer.Listener.Addr())
-	if params.user.username != "" && params.user.password != "" {
-		baseUrl = fmt.Sprintf("http://%s:%s@%s", params.user.username, params.user.password, c.env.Server.HTTPServer.Listener.Addr())
-	}
 
 	// nolint:gosec
 	resp, err := http.Post(
-		fmt.Sprintf(
-			"%s%s",
-			baseUrl,
-			params.url,
-		),
+		c.getURL(params.url, params.user),
 		"application/json",
 		buf,
 	)
@@ -66,6 +64,37 @@ func (c TestContext) Post(params PostParams) *http.Response {
 	return resp
 }
 
+type DeleteParams struct {
+	url  string
+	user User
+}
+
+func (c TestContext) Delete(params DeleteParams) *http.Response {
+	c.t.Helper()
+
+	req, err := http.NewRequest("DELETE", c.getURL(params.url, params.user), nil)
+	require.NoError(c.t, err)
+	resp, err := http.DefaultClient.Do(req)
+	require.NoError(c.t, err)
+
+	return resp
+}
+
+func (c TestContext) getURL(url string, user User) string {
+	c.t.Helper()
+
+	baseUrl := fmt.Sprintf("http://%s", c.env.Server.HTTPServer.Listener.Addr())
+	if user.username != "" && user.password != "" {
+		baseUrl = fmt.Sprintf("http://%s:%s@%s", user.username, user.password, c.env.Server.HTTPServer.Listener.Addr())
+	}
+
+	return fmt.Sprintf(
+		"%s%s",
+		baseUrl,
+		url,
+	)
+}
+
 func (c TestContext) createUser(cmd user.CreateUserCommand) {
 	c.t.Helper()
 
@@ -82,3 +111,11 @@ func (c TestContext) createDs(cmd *datasources.AddDataSourceCommand) {
 	err := c.env.SQLStore.AddDataSource(context.Background(), cmd)
 	require.NoError(c.t, err)
 }
+
+func (c TestContext) createCorrelation(cmd correlations.CreateCorrelationCommand) correlations.Correlation {
+	c.t.Helper()
+	correlation, err := c.env.Server.HTTPServer.CorrelationsService.CreateCorrelation(context.Background(), cmd)
+
+	require.NoError(c.t, err)
+	return correlation
+}

pkg/tests/api/correlations/correlations_create_test.go

@@ -14,11 +14,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-type errorResponseBody struct {
-	Message string `json:"message"`
-	Error   string `json:"error"`
-}
-
 func TestIntegrationCreateCorrelation(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping integration test")

pkg/tests/api/correlations/correlations_delete_test.go

@@ -0,0 +1,222 @@
+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 TestIntegrationDeleteCorrelation(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 delete correlations", func(t *testing.T) {
+		res := ctx.Delete(DeleteParams{
+			url: fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", "some-ds-uid", "some-correlation-uid"),
+		})
+		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 delete correlations", func(t *testing.T) {
+		res := ctx.Delete(DeleteParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", "some-ds-uid", "some-correlation-uid"),
+			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.Delete(DeleteParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", "nonexistent-ds-uid", "some-correlation-uid"),
+			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.Delete(DeleteParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", writableDs, "nonexistent-correlation-uid"),
+			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, "Correlation not found", response.Message)
+		require.Equal(t, correlations.ErrCorrelationNotFound.Error(), response.Error)
+
+		require.NoError(t, res.Body.Close())
+	})
+
+	t.Run("deleting a correlation originating from a read-only data source should result in a 403", func(t *testing.T) {
+		res := ctx.Delete(DeleteParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", readOnlyDS, "nonexistent-correlation-uid"),
+			user: adminUser,
+		})
+		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("deleting 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,
+		})
+
+		res := ctx.Delete(DeleteParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", correlation.SourceUID, correlation.UID),
+			user: adminUser,
+		})
+		require.Equal(t, http.StatusOK, res.StatusCode)
+
+		responseBody, err := ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+
+		var response correlations.CreateCorrelationResponse
+		err = json.Unmarshal(responseBody, &response)
+		require.NoError(t, err)
+
+		require.Equal(t, "Correlation deleted", response.Message)
+		require.NoError(t, res.Body.Close())
+
+		// trying to delete the same correlation a second time should result in a 404
+		res = ctx.Delete(DeleteParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", correlation.SourceUID, correlation.UID),
+			user: adminUser,
+		})
+		require.NoError(t, res.Body.Close())
+		require.Equal(t, http.StatusNotFound, res.StatusCode)
+	})
+
+	t.Run("should correctly delete a correlation", func(t *testing.T) {
+		correlation := ctx.createCorrelation(correlations.CreateCorrelationCommand{
+			SourceUID: writableDs,
+			TargetUID: readOnlyDS,
+			OrgId:     writableDsOrgId,
+		})
+
+		res := ctx.Delete(DeleteParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", correlation.SourceUID, correlation.UID),
+			user: adminUser,
+		})
+		require.Equal(t, http.StatusOK, res.StatusCode)
+
+		responseBody, err := ioutil.ReadAll(res.Body)
+		require.NoError(t, err)
+
+		var response correlations.CreateCorrelationResponse
+		err = json.Unmarshal(responseBody, &response)
+		require.NoError(t, err)
+
+		require.Equal(t, "Correlation deleted", response.Message)
+		require.NoError(t, res.Body.Close())
+
+		// trying to delete the same correlation a second time should result in a 404
+		res = ctx.Delete(DeleteParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", correlation.SourceUID, correlation.UID),
+			user: adminUser,
+		})
+		require.NoError(t, res.Body.Close())
+		require.Equal(t, http.StatusNotFound, res.StatusCode)
+	})
+}

public/api-merged.json

@@ -4286,6 +4286,44 @@
         }
       }
     },
+    "/datasources/uid/{uid}/correlations/{correlationUID}": {
+      "delete": {
+        "tags": ["correlations"],
+        "summary": "Delete a correlation.",
+        "operationId": "deleteCorrelation",
+        "parameters": [
+          {
+            "type": "string",
+            "name": "uid",
+            "in": "path",
+            "required": true
+          },
+          {
+            "type": "string",
+            "name": "correlationUID",
+            "in": "path",
+            "required": true
+          }
+        ],
+        "responses": {
+          "200": {
+            "$ref": "#/responses/deleteCorrelationResponse"
+          },
+          "401": {
+            "$ref": "#/responses/unauthorisedError"
+          },
+          "403": {
+            "$ref": "#/responses/forbiddenError"
+          },
+          "404": {
+            "$ref": "#/responses/notFoundError"
+          },
+          "500": {
+            "$ref": "#/responses/internalServerError"
+          }
+        }
+      }
+    },
     "/datasources/uid/{uid}/health": {
       "get": {
         "tags": ["datasources"],
@@ -11588,6 +11626,15 @@
         }
       }
     },
+    "DeleteCorrelationResponse": {
+      "type": "object",
+      "properties": {
+        "message": {
+          "type": "string",
+          "example": "Correlation deleted"
+        }
+      }
+    },
     "DeleteTokenCommand": {
       "type": "object",
       "properties": {
@@ -17859,6 +17906,12 @@
         }
       }
     },
+    "deleteCorrelationResponse": {
+      "description": "(empty)",
+      "schema": {
+        "$ref": "#/definitions/DeleteCorrelationResponse"
+      }
+    },
     "deleteDashboardResponse": {
       "description": "(empty)",
       "schema": {

public/api-spec.json

@@ -3705,6 +3705,44 @@
         }
       }
     },
+    "/datasources/uid/{uid}/correlations/{correlationUID}": {
+      "delete": {
+        "tags": ["correlations"],
+        "summary": "Delete a correlation.",
+        "operationId": "deleteCorrelation",
+        "parameters": [
+          {
+            "type": "string",
+            "name": "uid",
+            "in": "path",
+            "required": true
+          },
+          {
+            "type": "string",
+            "name": "correlationUID",
+            "in": "path",
+            "required": true
+          }
+        ],
+        "responses": {
+          "200": {
+            "$ref": "#/responses/deleteCorrelationResponse"
+          },
+          "401": {
+            "$ref": "#/responses/unauthorisedError"
+          },
+          "403": {
+            "$ref": "#/responses/forbiddenError"
+          },
+          "404": {
+            "$ref": "#/responses/notFoundError"
+          },
+          "500": {
+            "$ref": "#/responses/internalServerError"
+          }
+        }
+      }
+    },
     "/datasources/uid/{uid}/health": {
       "get": {
         "tags": ["datasources"],
@@ -10601,6 +10639,15 @@
       "type": "string",
       "title": "DataTopic is used to identify which topic the frame should be assigned to."
     },
+    "DeleteCorrelationResponse": {
+      "type": "object",
+      "properties": {
+        "message": {
+          "type": "string",
+          "example": "Correlation deleted"
+        }
+      }
+    },
     "DeleteTokenCommand": {
       "type": "object",
       "properties": {
@@ -14079,6 +14126,12 @@
         }
       }
     },
+    "deleteCorrelationResponse": {
+      "description": "",
+      "schema": {
+        "$ref": "#/definitions/DeleteCorrelationResponse"
+      }
+    },
     "deleteDashboardResponse": {
       "description": "",
       "schema": {