From bfaec17c51cbdf56e48a9d95d0a1ccda6899cc05 Mon Sep 17 00:00:00 2001
From: Andrej Ocenas <mr.ocenas@gmail.com>
Date: Wed, 6 May 2020 17:54:24 +0200
Subject: [PATCH] CloudWatch logs: Fix default region interpolation and reset
 log groups on region change (#24346)

---
 .../components/LogsQueryField.test.tsx        | 61 +++++++++++++++++++
 .../cloudwatch/components/LogsQueryField.tsx  | 29 +++++----
 .../datasource/cloudwatch/datasource.test.ts  | 29 +++++++++
 .../datasource/cloudwatch/datasource.ts       | 16 +++--
 4 files changed, 117 insertions(+), 18 deletions(-)
 create mode 100644 public/app/plugins/datasource/cloudwatch/components/LogsQueryField.test.tsx
 create mode 100644 public/app/plugins/datasource/cloudwatch/datasource.test.ts

diff --git a/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.test.tsx b/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.test.tsx
new file mode 100644
index 00000000000..8e9cbae368d
--- /dev/null
+++ b/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.test.tsx
@@ -0,0 +1,61 @@
+import React from 'react';
+import { shallow } from 'enzyme';
+import { CloudWatchLogsQueryField } from './LogsQueryField';
+import { ExploreId } from '../../../../types';
+
+describe('CloudWatchLogsQueryField', () => {
+  it('updates upstream query log groups on region change', async () => {
+    const onChange = jest.fn();
+    const wrapper = shallow(
+      <CloudWatchLogsQueryField
+        history={[]}
+        absoluteRange={{ from: 1, to: 10 }}
+        syntaxLoaded={false}
+        syntax={{} as any}
+        exploreId={ExploreId.left}
+        datasource={
+          {
+            getRegions() {
+              return Promise.resolve([
+                {
+                  label: 'region1',
+                  value: 'region1',
+                  text: 'region1',
+                },
+                {
+                  label: 'region2',
+                  value: 'region2',
+                  text: 'region2',
+                },
+              ]);
+            },
+            describeLogGroups(params: any) {
+              if (params.region === 'region1') {
+                return Promise.resolve(['log_group_1']);
+              } else {
+                return Promise.resolve(['log_group_2']);
+              }
+            },
+          } as any
+        }
+        query={{} as any}
+        onRunQuery={() => {}}
+        onChange={onChange}
+      />
+    );
+    const getRegionSelect = () => wrapper.find({ label: 'Region' }).props().inputEl;
+    const getLogGroupSelect = () => wrapper.find({ label: 'Log Groups' }).props().inputEl;
+
+    getLogGroupSelect().props.onChange([{ value: 'log_group_1' }]);
+    expect(getLogGroupSelect().props.value.length).toBe(1);
+    expect(getLogGroupSelect().props.value[0].value).toBe('log_group_1');
+
+    // We select new region where the selected log group does not exist
+    await getRegionSelect().props.onChange({ value: 'region2' });
+
+    // We clear the select
+    expect(getLogGroupSelect().props.value.length).toBe(0);
+    // Make sure we correctly updated the upstream state
+    expect(onChange.mock.calls[1][0]).toEqual({ region: 'region2', logGroupNames: [] });
+  });
+});
diff --git a/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.tsx b/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.tsx
index 61081294870..a6840f3203e 100644
--- a/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.tsx
+++ b/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.tsx
@@ -186,22 +186,25 @@ export class CloudWatchLogsQueryField extends React.PureComponent<CloudWatchLogs
 
     const logGroups = await this.fetchLogGroupOptions(v.value!);
 
-    this.setState(state => ({
-      availableLogGroups: logGroups,
-      selectedLogGroups: intersection(state.selectedLogGroups, logGroups),
-      loadingLogGroups: false,
-    }));
+    this.setState(state => {
+      const selectedLogGroups = intersection(state.selectedLogGroups, logGroups);
 
-    const { onChange, query } = this.props;
+      const { onChange, query } = this.props;
+      if (onChange) {
+        const nextQuery = {
+          ...query,
+          region: v.value,
+          logGroupNames: selectedLogGroups.map(group => group.value),
+        };
 
-    if (onChange) {
-      const nextQuery = {
-        ...query,
-        region: v.value,
+        onChange(nextQuery);
+      }
+      return {
+        availableLogGroups: logGroups,
+        selectedLogGroups: selectedLogGroups,
+        loadingLogGroups: false,
       };
-
-      onChange(nextQuery);
-    }
+    });
   };
 
   onTypeahead = async (typeahead: TypeaheadInput): Promise<TypeaheadOutput> => {
diff --git a/public/app/plugins/datasource/cloudwatch/datasource.test.ts b/public/app/plugins/datasource/cloudwatch/datasource.test.ts
new file mode 100644
index 00000000000..fc5e8bbc8f5
--- /dev/null
+++ b/public/app/plugins/datasource/cloudwatch/datasource.test.ts
@@ -0,0 +1,29 @@
+import { CloudWatchDatasource } from './datasource';
+import { TemplateSrv } from '../../../features/templating/template_srv';
+import { setBackendSrv } from '@grafana/runtime';
+import { DefaultTimeRange } from '@grafana/data';
+
+describe('datasource', () => {
+  describe('describeLogGroup', () => {
+    it('replaces region correctly in the query', async () => {
+      const datasource = new CloudWatchDatasource(
+        { jsonData: { defaultRegion: 'us-west-1' } } as any,
+        new TemplateSrv(),
+        {
+          timeRange() {
+            return DefaultTimeRange;
+          },
+        } as any
+      );
+      const datasourceRequestMock = jest.fn();
+      datasourceRequestMock.mockResolvedValue({ data: [] });
+      setBackendSrv({ datasourceRequest: datasourceRequestMock } as any);
+
+      await datasource.describeLogGroups({ region: 'default' });
+      expect(datasourceRequestMock.mock.calls[0][0].data.queries[0].region).toBe('us-west-1');
+
+      await datasource.describeLogGroups({ region: 'eu-east' });
+      expect(datasourceRequestMock.mock.calls[1][0].data.queries[0].region).toBe('eu-east');
+    });
+  });
+});
diff --git a/public/app/plugins/datasource/cloudwatch/datasource.ts b/public/app/plugins/datasource/cloudwatch/datasource.ts
index 3cfcc6f1ebb..c7ce5491684 100644
--- a/public/app/plugins/datasource/cloudwatch/datasource.ts
+++ b/public/app/plugins/datasource/cloudwatch/datasource.ts
@@ -470,7 +470,7 @@ export class CloudWatchDatasource extends DataSourceApi<CloudWatchQuery, CloudWa
   makeLogActionRequest(
     subtype: LogAction,
     queryParams: any[],
-    scopedVars?: any,
+    scopedVars?: ScopedVars,
     makeReplacements = true
   ): Observable<DataFrame[]> {
     const range = this.timeSrv.timeRange();
@@ -490,9 +490,10 @@ export class CloudWatchDatasource extends DataSourceApi<CloudWatchQuery, CloudWa
     };
 
     if (makeReplacements) {
-      requestParams.queries.forEach(
-        query => (query.region = this.replace(this.getActualRegion(this.defaultRegion), scopedVars, true, 'region'))
-      );
+      requestParams.queries.forEach(query => {
+        query.region = this.replace(query.region, scopedVars, true, 'region');
+        query.region = this.getActualRegion(query.region);
+      });
     }
 
     const resultsToDataFrames = (val: any): DataFrame[] => toDataQueryResponse(val).data || [];
@@ -785,7 +786,12 @@ export class CloudWatchDatasource extends DataSourceApi<CloudWatchQuery, CloudWa
     }, {});
   }
 
-  replace(target: string, scopedVars: ScopedVars, displayErrorIfIsMultiTemplateVariable?: boolean, fieldName?: string) {
+  replace(
+    target: string,
+    scopedVars: ScopedVars | undefined,
+    displayErrorIfIsMultiTemplateVariable?: boolean,
+    fieldName?: string
+  ) {
     if (displayErrorIfIsMultiTemplateVariable) {
       const variable = this.templateSrv
         .getVariables()