CloudWatch: fix variable query migration with json template variables (#51207)

* CloudWatch: fix variable query migration with json template variables

* fix error messages

* changes for reviews

* fix lint

* fix betterer

.betterer.results

@@ -3923,7 +3923,7 @@ exports[`no type assertions`] = {
       [41, 15, 78, "Do not use any type assertions.", "2265747900"],
       [52, 13, 63, "Do not use any type assertions.", "1673299780"]
     ],
-    "public/app/plugins/datasource/cloudwatch/datasource.test.ts:1906445626": [
+    "public/app/plugins/datasource/cloudwatch/datasource.test.ts:3643150425": [
       [30, 42, 53, "Do not use any type assertions.", "1293523870"],
       [30, 67, 16, "Do not use any type assertions.", "1416388343"],
       [40, 42, 65, "Do not use any type assertions.", "3968570046"],
@@ -3938,9 +3938,9 @@ exports[`no type assertions`] = {
       [369, 8, 43, "Do not use any type assertions.", "2822747014"],
       [415, 8, 54, "Do not use any type assertions.", "3062027045"],
       [456, 50, 101, "Do not use any type assertions.", "2285906034"],
-      [571, 19, 83, "Do not use any type assertions.", "55035528"]
+      [570, 19, 83, "Do not use any type assertions.", "55035528"]
     ],
-    "public/app/plugins/datasource/cloudwatch/datasource.ts:1003611696": [
+    "public/app/plugins/datasource/cloudwatch/datasource.ts:1299576841": [
       [218, 26, 85, "Do not use any type assertions.", "598275619"],
       [347, 23, 52, "Do not use any type assertions.", "3463806655"],
       [396, 43, 28, "Do not use any type assertions.", "426490724"],
@@ -3949,8 +3949,8 @@ exports[`no type assertions`] = {
       [630, 16, 26, "Do not use any type assertions.", "1259697827"],
       [865, 11, 47, "Do not use any type assertions.", "3491676769"],
       [865, 11, 19, "Do not use any type assertions.", "2012170033"],
-      [900, 23, 47, "Do not use any type assertions.", "3514668393"],
+      [898, 23, 47, "Do not use any type assertions.", "3514668393"],
-      [900, 23, 19, "Do not use any type assertions.", "3095385657"]
+      [898, 23, 19, "Do not use any type assertions.", "3095385657"]
     ],
     "public/app/plugins/datasource/cloudwatch/dynamic-labels/CompletionItemProvider.test.ts:2308040365": [
       [13, 17, 20, "Do not use any type assertions.", "3987315101"],
@@ -10727,7 +10727,7 @@ exports[`no explicit any`] = {
     "public/app/plugins/datasource/cloudwatch/datasource.d.ts:561167771": [
       [0, 34, 3, "Unexpected any. Specify a different type.", "193409811"]
     ],
-    "public/app/plugins/datasource/cloudwatch/datasource.test.ts:1906445626": [
+    "public/app/plugins/datasource/cloudwatch/datasource.test.ts:3643150425": [
       [30, 92, 3, "Unexpected any. Specify a different type.", "193409811"],
       [40, 104, 3, "Unexpected any. Specify a different type.", "193409811"],
       [74, 15, 3, "Unexpected any. Specify a different type.", "193409811"],
@@ -10740,9 +10740,9 @@ exports[`no explicit any`] = {
       [369, 48, 3, "Unexpected any. Specify a different type.", "193409811"],
       [415, 59, 3, "Unexpected any. Specify a different type.", "193409811"],
       [459, 11, 3, "Unexpected any. Specify a different type.", "193409811"],
-      [577, 7, 3, "Unexpected any. Specify a different type.", "193409811"]
+      [576, 7, 3, "Unexpected any. Specify a different type.", "193409811"]
     ],
-    "public/app/plugins/datasource/cloudwatch/datasource.ts:1003611696": [
+    "public/app/plugins/datasource/cloudwatch/datasource.ts:1299576841": [
       [93, 12, 3, "Unexpected any. Specify a different type.", "193409811"],
       [94, 17, 3, "Unexpected any. Specify a different type.", "193409811"],
       [531, 53, 3, "Unexpected any. Specify a different type.", "193409811"],
@@ -10761,7 +10761,7 @@ exports[`no explicit any`] = {
       [806, 71, 3, "Unexpected any. Specify a different type.", "193409811"],
       [834, 32, 3, "Unexpected any. Specify a different type.", "193409811"],
       [834, 46, 3, "Unexpected any. Specify a different type.", "193409811"],
-      [968, 26, 3, "Unexpected any. Specify a different type.", "193409811"]
+      [966, 26, 3, "Unexpected any. Specify a different type.", "193409811"]
     ],
     "public/app/plugins/datasource/cloudwatch/language_provider.test.ts:609679032": [
       [114, 7, 3, "Unexpected any. Specify a different type.", "193409811"],

public/app/plugins/datasource/cloudwatch/datasource.test.ts

@@ -500,8 +500,7 @@ describe('datasource', () => {
   describe('convertMultiFiltersFormat', () => {
     const ds = setupMockedDataSource({ variables: [labelsVariable, dimensionVariable], mockGetVariableName: false });
     it('converts keys and values correctly', () => {
-      // the json in this line doesn't matter, but it makes sure that old queries will be parsed
+      const filters = { $dimension: ['b'], a: ['$labels', 'bar'] };
-      const filters = { $dimension: ['b'], a: ['${labels:json}', 'bar'] };
       const result = ds.datasource.convertMultiFilterFormat(filters);
       expect(result).toStrictEqual({
         env: ['b'],

public/app/plugins/datasource/cloudwatch/datasource.ts

@@ -237,7 +237,7 @@ export class CloudWatchDatasource
               options,
               this.timeSrv.timeRange(),
               this.replace.bind(this),
-              this.getVariableValue.bind(this),
+              this.expandVariableToArray.bind(this),
               this.getActualRegion.bind(this),
               this.tracingDataSourceUid
             );
@@ -650,7 +650,7 @@ export class CloudWatchDatasource
             if (Array.isArray(anyQuery[fieldName])) {
               anyQuery[fieldName] = anyQuery[fieldName].flatMap((val: string) => {
                 if (fieldName === 'logGroupNames') {
-                  return this.getVariableValue(val, options.scopedVars || {});
+                  return this.expandVariableToArray(val, options.scopedVars || {});
                 }
                 return this.replace(val, options.scopedVars, true, fieldName);
               });
@@ -851,22 +851,20 @@ export class CloudWatchDatasource
         return { ...result, [key]: null };
       }
 
-      const newValues = this.getVariableValue(value, scopedVars);
+      const newValues = this.expandVariableToArray(value, scopedVars);
       return { ...result, [key]: newValues };
     }, {});
   }
 
   // get the value for a given template variable
-  getVariableValue(value: string, scopedVars: ScopedVars): string[] {
+  expandVariableToArray(value: string, scopedVars: ScopedVars): string[] {
     const variableName = this.templateSrv.getVariableName(value);
     const valueVar = this.templateSrv.getVariables().find(({ name }) => {
       return name === variableName;
     });
     if (variableName && valueVar) {
       if ((valueVar as unknown as VariableWithMultiSupport).multi) {
-        // rebuild the variable name to handle old migrated queries
+        return this.templateSrv.replace(value, scopedVars, 'pipe').split('|');
-        const values = this.templateSrv.replace('$' + variableName, scopedVars, 'pipe').split('|');
-        return values;
       }
       return [this.templateSrv.replace(value, scopedVars)];
     }
@@ -881,7 +879,7 @@ export class CloudWatchDatasource
       }
       const initialVal: string[] = [];
       const newValues = values.reduce((result, value) => {
-        const vals = this.getVariableValue(value, {});
+        const vals = this.expandVariableToArray(value, {});
         return [...result, ...vals];
       }, initialVal);
       return { ...result, [key]: newValues };

public/app/plugins/datasource/cloudwatch/migrations/variableQueryMigrations.test.ts

@@ -55,6 +55,12 @@ describe('variableQueryMigrations', () => {
           expect(query.dimensionKey).toBe('DBInstanceIdentifier');
           expect(query.dimensionFilters).toStrictEqual({ InstanceId: '$instance_id' });
         });
+        it('should migrate json template variables', () => {
+          const query = migrateVariableQuery(
+            'dimension_values(us-east-1,AWS/RDS,CPUUtilization,DBInstanceIdentifier,{"role":${role:json},"pop":${pop:json}})'
+          );
+          expect(query.dimensionFilters).toStrictEqual({ role: '$role', pop: '$pop' });
+        });
       });
     });
   });
@@ -68,6 +74,12 @@ describe('variableQueryMigrations', () => {
       expect(query.resourceType).toBe('elasticloadbalancing:loadbalancer');
       expect(query.tags).toStrictEqual({ 'elasticbeanstalk:environment-name': ['myApp-dev', 'myApp-prod'] });
     });
+    it('should migrate json template variables', () => {
+      const query = migrateVariableQuery(
+        'resource_arns(eu-west-1,elasticloadbalancing:loadbalancer,{"elasticbeanstalk:environment-name":[${jsonVar:json},"test-$singleVar"]})'
+      );
+      expect(query.tags).toStrictEqual({ 'elasticbeanstalk:environment-name': ['$jsonVar', 'test-$singleVar'] });
+    });
     it('should parse a empty array for tags', () => {
       const query = migrateVariableQuery('resource_arns(eu-west-1,elasticloadbalancing:loadbalancer, [])');
       expect(query.tags).toStrictEqual({});
@@ -81,6 +93,10 @@ describe('variableQueryMigrations', () => {
       expect(query.attributeName).toBe('rds:db');
       expect(query.ec2Filters).toStrictEqual({ environment: ['$environment'] });
     });
+    it('should migrate json template variables', () => {
+      const query = migrateVariableQuery('ec2_instance_attribute(us-east-1,rds:db,{"environment":${env:json}})');
+      expect(query.ec2Filters).toStrictEqual({ environment: ['$env'] });
+    });
     it('should parse an empty array for filters', () => {
       const query = migrateVariableQuery('ec2_instance_attribute(us-east-1,rds:db,[])');
       expect(query.ec2Filters).toStrictEqual({});

public/app/plugins/datasource/cloudwatch/migrations/variableQueryMigrations.ts

@@ -1,11 +1,29 @@
 import { omit } from 'lodash';
 
-import { VariableQuery, VariableQueryType, OldVariableQuery } from '../types';
+import { Dimensions, VariableQuery, VariableQueryType, OldVariableQuery, MultiFilters } from '../types';
+
+const jsonVariable = /\${(\w+):json}/g;
 
 function isVariableQuery(rawQuery: string | VariableQuery | OldVariableQuery): rawQuery is VariableQuery {
   return typeof rawQuery !== 'string' && typeof rawQuery.ec2Filters !== 'string' && typeof rawQuery.tags !== 'string';
 }
 
+function migrateMultiFilters(oldFilters: string): MultiFilters {
+  const tempFilters = oldFilters.replace(jsonVariable, '"$$$1"');
+  const parsedFilters: Dimensions = JSON.parse(tempFilters);
+  const newFilters: MultiFilters = {};
+  // if the old filter was {key:value} transform it to {key:[value]}
+  Object.keys(parsedFilters).forEach((key) => {
+    const value = parsedFilters[key];
+    if (typeof value === 'string') {
+      newFilters[key] = [value];
+    } else if (value !== undefined) {
+      newFilters[key] = value;
+    }
+  });
+  return newFilters;
+}
+
 export function migrateVariableQuery(rawQuery: string | VariableQuery | OldVariableQuery): VariableQuery {
   if (isVariableQuery(rawQuery)) {
     return rawQuery;
@@ -19,22 +37,23 @@ export function migrateVariableQuery(rawQuery: string | VariableQuery | OldVaria
     newQuery.tags = {};
 
     if (rawQuery.dimensionFilters !== '' && rawQuery.ec2Filters !== '[]') {
+      const tempFilters = rawQuery.dimensionFilters.replace(jsonVariable, '"$$$1"');
       try {
-        newQuery.dimensionFilters = JSON.parse(rawQuery.dimensionFilters);
+        newQuery.dimensionFilters = JSON.parse(tempFilters);
       } catch {
         throw new Error(`unable to migrate poorly formed filters: ${rawQuery.dimensionFilters}`);
       }
     }
     if (rawQuery.ec2Filters !== '' && rawQuery.ec2Filters !== '[]') {
       try {
-        newQuery.ec2Filters = JSON.parse(rawQuery.ec2Filters);
+        newQuery.ec2Filters = migrateMultiFilters(rawQuery.ec2Filters);
       } catch {
         throw new Error(`unable to migrate poorly formed filters: ${rawQuery.ec2Filters}`);
       }
     }
     if (rawQuery.tags !== '' && rawQuery.tags !== '[]') {
       try {
-        newQuery.tags = JSON.parse(rawQuery.tags);
+        newQuery.tags = migrateMultiFilters(rawQuery.tags);
       } catch {
         throw new Error(`unable to migrate poorly formed filters: ${rawQuery.tags}`);
       }
@@ -94,8 +113,9 @@ export function migrateVariableQuery(rawQuery: string | VariableQuery | OldVaria
     newQuery.dimensionKey = dimensionValuesQuery[4];
     newQuery.dimensionFilters = {};
     if (!!dimensionValuesQuery[6] && dimensionValuesQuery[6] !== '[]') {
+      const tempFilters = dimensionValuesQuery[6].replace(jsonVariable, '"$$$1"');
       try {
-        newQuery.dimensionFilters = JSON.parse(dimensionValuesQuery[6]);
+        newQuery.dimensionFilters = JSON.parse(tempFilters);
       } catch {
         throw new Error(`unable to migrate poorly formed filters: ${dimensionValuesQuery[6]}`);
       }
@@ -118,7 +138,7 @@ export function migrateVariableQuery(rawQuery: string | VariableQuery | OldVaria
     newQuery.attributeName = ec2InstanceAttributeQuery[2];
     if (ec2InstanceAttributeQuery[3] && ec2InstanceAttributeQuery[3] !== '[]') {
       try {
-        newQuery.ec2Filters = JSON.parse(ec2InstanceAttributeQuery[3]);
+        newQuery.ec2Filters = migrateMultiFilters(ec2InstanceAttributeQuery[3]);
       } catch {
         throw new Error(`unable to migrate poorly formed filters: ${ec2InstanceAttributeQuery[3]}`);
       }
@@ -133,7 +153,7 @@ export function migrateVariableQuery(rawQuery: string | VariableQuery | OldVaria
     newQuery.resourceType = resourceARNsQuery[2];
     if (resourceARNsQuery[3] && resourceARNsQuery[3] !== '[]') {
       try {
-        newQuery.tags = JSON.parse(resourceARNsQuery[3]);
+        newQuery.tags = migrateMultiFilters(resourceARNsQuery[3]);
       } catch {
         throw new Error(`unable to migrate poorly formed filters: ${resourceARNsQuery[3]}`);
       }