Glue: Add configuration support to PATCH (#56117)

* feat: add config to tests v0

* feat: add config to UpdateCorrelationCommand

* refactor: repair some tests

* refactor: repair another test

* refactor: repair last test

* refactor: remove possible test

* refactor: add comments

* refactor: add changes from code review

* refactor: implement more detailed patch

* feat: add tests for partial config update

* refactor: make error handling more detailed

Co-authored-by: Giordano Ricci <me@giordanoricci.com>

Co-authored-by: Giordano Ricci <me@giordanoricci.com>

docs/sources/developers/http_api/correlations.md

@@ -161,6 +161,7 @@ Content-Type: application/json
 Status codes:
 
 - **200** – OK
+- **400** – Bad request
 - **401** – Unauthorized
 - **403** – Forbidden, source data source is read-only
 - **404** – Not found, either source or target data source could not be found

pkg/services/correlations/api.go

@@ -160,7 +160,7 @@ func (s *CorrelationsService) updateHandler(c *models.ReqContext) response.Respo
 	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)
+			return response.Error(http.StatusBadRequest, "At least one of label, description or config is required", err)
 		}
 
 		if errors.Is(err, ErrSourceDataSourceDoesNotExists) {

pkg/services/correlations/database.go

@@ -99,32 +99,42 @@ func (s CorrelationsService) updateCorrelation(ctx context.Context, cmd UpdateCo
 			return ErrSourceDataSourceReadOnly
 		}
 
-		if cmd.Label == nil && cmd.Description == nil {
+		if cmd.Label == nil && cmd.Description == nil && (cmd.Config == nil || (cmd.Config.Field == nil && cmd.Config.Target == nil && cmd.Config.Type == 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 {
+		found, err := session.Get(&correlation)
+		if !found {
 			return ErrCorrelationNotFound
 		}
 		if err != nil {
 			return err
 		}
 
-		found, err := session.Get(&correlation)
-		if !found {
-			return ErrCorrelationNotFound
+		if cmd.Label != nil {
+			correlation.Label = *cmd.Label
+			session.MustCols("label")
+		}
+		if cmd.Description != nil {
+			correlation.Description = *cmd.Description
+			session.MustCols("description")
+		}
+		if cmd.Config != nil {
+			session.MustCols("config")
+			if cmd.Config.Field != nil {
+				correlation.Config.Field = *cmd.Config.Field
+			}
+			if cmd.Config.Type != nil {
+				correlation.Config.Type = *cmd.Config.Type
+			}
+			if cmd.Config.Target != nil {
+				correlation.Config.Target = *cmd.Config.Target
+			}
 		}
 
+		updateCount, err := session.Where("uid = ? AND source_uid = ?", correlation.UID, correlation.SourceUID).Limit(1).Update(correlation)
+		if updateCount == 0 {
+			return ErrCorrelationNotFound
+		}
 		return err
 	})
 

pkg/services/correlations/models.go

@@ -58,6 +58,18 @@ func (c CorrelationConfig) MarshalJSON() ([]byte, error) {
 	})
 }
 
+type CorrelationConfigUpdateDTO struct {
+	// Field used to attach the correlation link
+	// required:true
+	Field *string `json:"field"`
+	// Target type
+	// required:true
+	Type *CorrelationConfigType `json:"type"`
+	// Target data query
+	// required:true
+	Target *map[string]interface{} `json:"target"`
+}
+
 // Correlation is the model for correlations definitions
 // swagger:model
 type Correlation struct {
@@ -77,7 +89,7 @@ type Correlation struct {
 	// example: Logs to Traces
 	Description string `json:"description" xorm:"description"`
 	// Correlation Configuration
-	// example: { field: "job", target: { query: "job=app" } }
+	// example: { field: "job", type: "query", target: { query: "job=app" } }
 	Config CorrelationConfig `json:"config" xorm:"jsonb config"`
 }
 
@@ -106,7 +118,7 @@ type CreateCorrelationCommand struct {
 	// example: Logs to Traces
 	Description string `json:"description"`
 	// Arbitrary configuration object handled in frontend
-	// example: { field: "job", target: { query: "job=app" } }
+	// example: { field: "job", type: "query", target: { query: "job=app" } }
 	Config CorrelationConfig `json:"config" binding:"Required"`
 }
 
@@ -154,6 +166,9 @@ type UpdateCorrelationCommand struct {
 	// Optional description of the correlation
 	// example: Logs to Traces
 	Description *string `json:"description"`
+	// Correlation Configuration
+	// example: { field: "job", type: "query", target: { query: "job=app" } }
+	Config *CorrelationConfigUpdateDTO `json:"config"`
 }
 
 // GetCorrelationQuery is the query to retrieve a single correlation

pkg/tests/api/correlations/correlations_update_test.go

@@ -162,7 +162,7 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
 		require.NoError(t, res.Body.Close())
 	})
 
-	t.Run("updating a without data should result in a 400", func(t *testing.T) {
+	t.Run("updating a correlation without data should result in a 400", func(t *testing.T) {
 		correlation := ctx.createCorrelation(correlations.CreateCorrelationCommand{
 			SourceUID: writableDs,
 			TargetUID: &writableDs,
@@ -184,7 +184,7 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
 		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, "At least one of label, description or config is required", response.Message)
 		require.Equal(t, correlations.ErrUpdateCorrelationEmptyParams.Error(), response.Error)
 		require.NoError(t, res.Body.Close())
 
@@ -202,7 +202,7 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
 		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, "At least one of label, description or config is required", response.Message)
 		require.Equal(t, correlations.ErrUpdateCorrelationEmptyParams.Error(), response.Error)
 		require.NoError(t, res.Body.Close())
 
@@ -212,7 +212,8 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
 			user: adminUser,
 			body: `{
 						"label": null,
-						"description": null
+						"description": null,
+						"config": null
 					}`,
 		})
 		require.Equal(t, http.StatusBadRequest, res.StatusCode)
@@ -223,7 +224,7 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
 		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, "At least one of label, description or config is required", response.Message)
 		require.Equal(t, correlations.ErrUpdateCorrelationEmptyParams.Error(), response.Error)
 		require.NoError(t, res.Body.Close())
 	})
@@ -264,6 +265,11 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
 			OrgId:       writableDsOrgId,
 			Label:       "0",
 			Description: "0",
+			Config: correlations.CorrelationConfig{
+				Field:  "fieldName",
+				Type:   "query",
+				Target: map[string]interface{}{"expr": "foo"},
+			},
 		})
 
 		// updating all
@@ -272,7 +278,12 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
 			user: adminUser,
 			body: `{
 				"label": "1",
-				"description": "1"
+				"description": "1",
+				"config": {
+					"field": "field",
+					"type": "query",
+					"target": { "expr": "bar" }
+				}
 			}`,
 		})
 		require.Equal(t, http.StatusOK, res.StatusCode)
@@ -286,7 +297,9 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
 
 		require.Equal(t, "Correlation updated", response.Message)
 		require.Equal(t, "1", response.Result.Label)
-		require.Equal(t, "1", response.Result.Label)
+		require.Equal(t, "1", response.Result.Description)
+		require.Equal(t, "field", response.Result.Config.Field)
+		require.Equal(t, map[string]interface{}{"expr": "bar"}, response.Result.Config.Target)
 		require.NoError(t, res.Body.Close())
 
 		// partially updating only label
@@ -308,6 +321,8 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
 		require.Equal(t, "Correlation updated", response.Message)
 		require.Equal(t, "2", response.Result.Label)
 		require.Equal(t, "1", response.Result.Description)
+		require.Equal(t, "field", response.Result.Config.Field)
+		require.Equal(t, map[string]interface{}{"expr": "bar"}, response.Result.Config.Target)
 		require.NoError(t, res.Body.Close())
 
 		// partially updating only description
@@ -329,15 +344,97 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
 		require.Equal(t, "Correlation updated", response.Message)
 		require.Equal(t, "2", response.Result.Label)
 		require.Equal(t, "2", response.Result.Description)
+		require.Equal(t, "field", response.Result.Config.Field)
+		require.Equal(t, map[string]interface{}{"expr": "bar"}, response.Result.Config.Target)
 		require.NoError(t, res.Body.Close())
 
-		// setting both to empty strings (testing wether empty strings are handled correctly)
+		// partially updating whole config
+		res = ctx.Patch(PatchParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", correlation.SourceUID, correlation.UID),
+			user: adminUser,
+			body: `{
+				"config": {
+					"field": "name",
+					"type": "query",
+					"target": { "expr": "baz" }
+				}
+			}`,
+		})
+		require.Equal(t, http.StatusOK, res.StatusCode)
+
+		responseBody, err = io.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.Equal(t, "name", response.Result.Config.Field)
+		require.Equal(t, map[string]interface{}{"expr": "baz"}, response.Result.Config.Target)
+		require.NoError(t, res.Body.Close())
+
+		// partially updating only config field
+		res = ctx.Patch(PatchParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", correlation.SourceUID, correlation.UID),
+			user: adminUser,
+			body: `{
+				"config": {
+					"field": "newName"
+				}
+			}`,
+		})
+		require.Equal(t, http.StatusOK, res.StatusCode)
+
+		responseBody, err = io.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.Equal(t, "newName", response.Result.Config.Field)
+		require.Equal(t, map[string]interface{}{"expr": "baz"}, response.Result.Config.Target)
+		require.NoError(t, res.Body.Close())
+
+		// partially updating only config target
+		res = ctx.Patch(PatchParams{
+			url:  fmt.Sprintf("/api/datasources/uid/%s/correlations/%s", correlation.SourceUID, correlation.UID),
+			user: adminUser,
+			body: `{
+				"config": {
+					"target": { "expr": "foo" }
+				}
+			}`,
+		})
+		require.Equal(t, http.StatusOK, res.StatusCode)
+
+		responseBody, err = io.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.Equal(t, "newName", response.Result.Config.Field)
+		require.Equal(t, map[string]interface{}{"expr": "foo"}, response.Result.Config.Target)
+		require.NoError(t, res.Body.Close())
+
+		// setting label, description and config field to empty strings (testing whether 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": ""
+				"description": "",
+				"config": {
+					"field": ""
+				}
 			}`,
 		})
 		require.Equal(t, http.StatusOK, res.StatusCode)
@@ -351,6 +448,7 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
 		require.Equal(t, "Correlation updated", response.Message)
 		require.Equal(t, "", response.Result.Label)
 		require.Equal(t, "", response.Result.Description)
+		require.Equal(t, "", response.Result.Config.Field)
 		require.NoError(t, res.Body.Close())
 	})
 }