From 99b4dfc27dfba338195aa7fead70501dda6c0a87 Mon Sep 17 00:00:00 2001
From: Kevin Yu <kevinwcyu@users.noreply.github.com>
Date: Mon, 14 Mar 2022 09:44:04 -0700
Subject: [PATCH] Dashboard: Validate refId when generating id for cloudwatch
 query (#46182)

* Validate refId when generating id for cloudwatch query

* add test case when refId is a valid metric data id
---
 pkg/tsdb/cloudwatch/request_parser.go      | 10 +++++++++-
 pkg/tsdb/cloudwatch/request_parser_test.go | 17 +++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/pkg/tsdb/cloudwatch/request_parser.go b/pkg/tsdb/cloudwatch/request_parser.go
index c0202c97383..0edb6cf4907 100644
--- a/pkg/tsdb/cloudwatch/request_parser.go
+++ b/pkg/tsdb/cloudwatch/request_parser.go
@@ -10,10 +10,13 @@ import (
 	"strings"
 	"time"
 
+	"github.com/google/uuid"
 	"github.com/grafana/grafana-plugin-sdk-go/backend"
 	"github.com/grafana/grafana/pkg/components/simplejson"
 )
 
+var validMetricDataID = regexp.MustCompile(`^[a-z][a-zA-Z0-9_]*$`)
+
 // parseQueries parses the json queries and returns a map of cloudWatchQueries by region. The cloudWatchQuery has a 1 to 1 mapping to a query editor row
 func (e *cloudWatchExecutor) parseQueries(queries []backend.DataQuery, startTime time.Time, endTime time.Time) (map[string][]*cloudWatchQuery, error) {
 	requestQueries := make(map[string][]*cloudWatchQuery)
@@ -140,7 +143,12 @@ func parseRequestQuery(model *simplejson.Json, refId string, startTime time.Time
 		// Why not just use refId if id is not specified in the frontend? When specifying an id in the editor,
 		// and alphabetical must be used. The id must be unique, so if an id like for example a, b or c would be used,
 		// it would likely collide with some ref id. That's why the `query` prefix is used.
-		id = fmt.Sprintf("query%s", refId)
+		suffix := refId
+		if !validMetricDataID.MatchString(suffix) {
+			uuid := uuid.NewString()
+			suffix = strings.Replace(uuid, "-", "", -1)
+		}
+		id = fmt.Sprintf("query%s", suffix)
 	}
 	expression := model.Get("expression").MustString("")
 	sqlExpression := model.Get("sqlExpression").MustString("")
diff --git a/pkg/tsdb/cloudwatch/request_parser_test.go b/pkg/tsdb/cloudwatch/request_parser_test.go
index cadaff50aa6..cec43f954fc 100644
--- a/pkg/tsdb/cloudwatch/request_parser_test.go
+++ b/pkg/tsdb/cloudwatch/request_parser_test.go
@@ -293,6 +293,23 @@ func TestRequestParser(t *testing.T) {
 			assert.Equal(t, GMDApiModeMathExpression, res.getGMDAPIMode())
 		})
 	})
+
+	t.Run("ID is the string `query` appended with refId if refId is a valid MetricData ID", func(t *testing.T) {
+		query := getBaseJsonQuery()
+		res, err := parseRequestQuery(query, "ref1", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour))
+		require.NoError(t, err)
+		assert.Equal(t, "ref1", res.RefId)
+		assert.Equal(t, "queryref1", res.Id)
+	})
+
+	t.Run("Valid id is generated if ID is not provided and refId is not a valid MetricData ID", func(t *testing.T) {
+		query := getBaseJsonQuery()
+		query.Set("refId", "$$")
+		res, err := parseRequestQuery(query, "$$", time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour))
+		require.NoError(t, err)
+		assert.Equal(t, "$$", res.RefId)
+		assert.Regexp(t, validMetricDataID, res.Id)
+	})
 }
 
 func getBaseJsonQuery() *simplejson.Json {