AzureMonitor: reset default subscription when credentials change (#34707)

* Update default subscription on credentials change * Fix secret reset when subscription selected * Remove unused exports
2025-02-25 18:55:37 -06:00 · 2021-05-27 11:03:36 -07:00 · 2021-05-27 11:03:36 -07:00 · de86114b66
commit de86114b66
parent f07366690e
8 changed files with 74 additions and 95 deletions
--- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/azure_monitor_datasource.ts
+++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/azure_monitor_datasource.ts
@ -56,7 +56,7 @@ export default class AzureMonitorDatasource extends DataSourceWithBackend<AzureM
    super(instanceSettings);

    this.timeSrv = getTimeSrv();
-    this.subscriptionId = instanceSettings.jsonData.subscriptionId;
+    this.subscriptionId = instanceSettings.jsonData.subscriptionId!;

    const cloud = getAzureCloud(instanceSettings);
    const route = getManagementApiRoute(cloud);
--- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/AnalyticsConfig.tsx
+++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/AnalyticsConfig.tsx
@ -24,8 +24,8 @@ export const AnalyticsConfig: FunctionComponent<Props> = (props: Props) => {
  const primaryCredentials = useMemo(() => getCredentials(props.options), [props.options]);
  const logAnalyticsCredentials = useMemo(() => getLogAnalyticsCredentials(props.options), [props.options]);
  const subscriptionId = logAnalyticsCredentials
-    ? props.options.jsonData.logAnalyticsSubscriptionId
-    : props.options.jsonData.subscriptionId;
+    ? logAnalyticsCredentials.defaultSubscriptionId
+    : primaryCredentials.defaultSubscriptionId;

  const credentialsEnabled = primaryCredentials.authType === 'clientsecret';

@ -99,18 +99,6 @@ export const AnalyticsConfig: FunctionComponent<Props> = (props: Props) => {
    setSameAsSwitched(true);
  };

-  const onLogAnalyticsDefaultSubscriptionChange = (subscriptionId: string | undefined) => {
-    updateOptions((options) => {
-      return {
-        ...options,
-        jsonData: {
-          ...options.jsonData,
-          logAnalyticsSubscriptionId: subscriptionId || '',
-        },
-      };
-    });
-  };
-
  const onDefaultWorkspaceChange = (selected: SelectableValue<string>) => {
    updateOptions((options) => {
      return {
@ -157,9 +145,7 @@ export const AnalyticsConfig: FunctionComponent<Props> = (props: Props) => {
            <AzureCredentialsForm
              managedIdentityEnabled={false}
              credentials={logAnalyticsCredentials}
-              defaultSubscription={subscriptionId}
              onCredentialsChange={onCredentialsChange}
-              onDefaultSubscriptionChange={onLogAnalyticsDefaultSubscriptionChange}
              getSubscriptions={getSubscriptions}
            />
          )}
--- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/AzureCredentialsForm.test.tsx
+++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/AzureCredentialsForm.test.tsx
@ -11,8 +11,8 @@ const setup = (propsFunc?: (props: Props) => Props) => {
      tenantId: 'e7f3f661-a933-3h3f-0294-31c4f962ec48',
      clientId: '34509fad-c0r9-45df-9e25-f1ee34af6900',
      clientSecret: undefined,
+      defaultSubscriptionId: '44987801-6nn6-49he-9b2d-9106972f9789',
    },
-    defaultSubscription: '44987801-6nn6-49he-9b2d-9106972f9789',
    azureCloudOptions: [
      { value: 'azuremonitor', label: 'Azure' },
      { value: 'govazuremonitor', label: 'Azure US Government' },
@ -20,7 +20,6 @@ const setup = (propsFunc?: (props: Props) => Props) => {
      { value: 'chinaazuremonitor', label: 'Azure China' },
    ],
    onCredentialsChange: jest.fn(),
-    onDefaultSubscriptionChange: jest.fn(),
    getSubscriptions: jest.fn(),
  };

--- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/AzureCredentialsForm.tsx
+++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/AzureCredentialsForm.tsx
@ -8,10 +8,8 @@ const { Select, Input } = LegacyForms;
 export interface Props {
  managedIdentityEnabled: boolean;
  credentials: AzureCredentials;
-  defaultSubscription?: string;
  azureCloudOptions?: SelectableValue[];
  onCredentialsChange: (updatedCredentials: AzureCredentials) => void;
-  onDefaultSubscriptionChange?: (subscriptionId: string | undefined) => void;
  getSubscriptions?: () => Promise<SelectableValue[]>;
 }

@ -27,14 +25,7 @@ const authTypeOptions: Array<SelectableValue<AzureAuthType>> = [
 ];

 export const AzureCredentialsForm: FunctionComponent<Props> = (props: Props) => {
-  const {
-    credentials,
-    defaultSubscription,
-    azureCloudOptions,
-    onCredentialsChange,
-    onDefaultSubscriptionChange,
-    getSubscriptions,
-  } = props;
+  const { credentials, azureCloudOptions, onCredentialsChange, getSubscriptions } = props;
  const hasRequiredFields = isCredentialsComplete(credentials);

  const [subscriptions, setSubscriptions] = useState<Array<SelectableValue<string>>>([]);
@ -59,15 +50,15 @@ export const AzureCredentialsForm: FunctionComponent<Props> = (props: Props) =>

  const updateSubscriptions = (received: Array<SelectableValue<string>>) => {
    setSubscriptions(received);
-    if (onDefaultSubscriptionChange) {
-      if (!defaultSubscription && received.length > 0) {
+    if (getSubscriptions) {
+      if (!credentials.defaultSubscriptionId && received.length > 0) {
        // Setting the default subscription if subscriptions received but no default subscription selected
-        onDefaultSubscriptionChange(received[0].value);
-      } else if (defaultSubscription) {
-        const found = received.find((opt) => opt.value === defaultSubscription);
+        onSubscriptionChange(received[0]);
+      } else if (credentials.defaultSubscriptionId) {
+        const found = received.find((opt) => opt.value === credentials.defaultSubscriptionId);
        if (!found) {
          // Unsetting the default found if it isn't found among the received subscriptions
-          onDefaultSubscriptionChange(undefined);
+          onSubscriptionChange(undefined);
        }
      }
    }
@ -75,9 +66,11 @@ export const AzureCredentialsForm: FunctionComponent<Props> = (props: Props) =>

  const onAuthTypeChange = (selected: SelectableValue<AzureAuthType>) => {
    if (onCredentialsChange) {
+      setSubscriptions([]);
      const updated: AzureCredentials = {
        ...credentials,
        authType: selected.value || 'msi',
+        defaultSubscriptionId: undefined,
      };
      onCredentialsChange(updated);
    }
@ -85,9 +78,11 @@ export const AzureCredentialsForm: FunctionComponent<Props> = (props: Props) =>

  const onAzureCloudChange = (selected: SelectableValue<string>) => {
    if (onCredentialsChange && credentials.authType === 'clientsecret') {
+      setSubscriptions([]);
      const updated: AzureCredentials = {
        ...credentials,
        azureCloud: selected.value,
+        defaultSubscriptionId: undefined,
      };
      onCredentialsChange(updated);
    }
@ -95,9 +90,11 @@ export const AzureCredentialsForm: FunctionComponent<Props> = (props: Props) =>

  const onTenantIdChange = (event: ChangeEvent<HTMLInputElement>) => {
    if (onCredentialsChange && credentials.authType === 'clientsecret') {
+      setSubscriptions([]);
      const updated: AzureCredentials = {
        ...credentials,
        tenantId: event.target.value,
+        defaultSubscriptionId: undefined,
      };
      onCredentialsChange(updated);
    }
@ -105,9 +102,11 @@ export const AzureCredentialsForm: FunctionComponent<Props> = (props: Props) =>

  const onClientIdChange = (event: ChangeEvent<HTMLInputElement>) => {
    if (onCredentialsChange && credentials.authType === 'clientsecret') {
+      setSubscriptions([]);
      const updated: AzureCredentials = {
        ...credentials,
        clientId: event.target.value,
+        defaultSubscriptionId: undefined,
      };
      onCredentialsChange(updated);
    }
@ -115,9 +114,11 @@ export const AzureCredentialsForm: FunctionComponent<Props> = (props: Props) =>

  const onClientSecretChange = (event: ChangeEvent<HTMLInputElement>) => {
    if (onCredentialsChange && credentials.authType === 'clientsecret') {
+      setSubscriptions([]);
      const updated: AzureCredentials = {
        ...credentials,
        clientSecret: event.target.value,
+        defaultSubscriptionId: undefined,
      };
      onCredentialsChange(updated);
    }
@ -125,17 +126,23 @@ export const AzureCredentialsForm: FunctionComponent<Props> = (props: Props) =>

  const onClientSecretReset = () => {
    if (onCredentialsChange && credentials.authType === 'clientsecret') {
+      setSubscriptions([]);
      const updated: AzureCredentials = {
        ...credentials,
        clientSecret: '',
+        defaultSubscriptionId: undefined,
      };
      onCredentialsChange(updated);
    }
  };

-  const onSubscriptionChange = (selected: SelectableValue<string>) => {
-    if (onDefaultSubscriptionChange) {
-      onDefaultSubscriptionChange(selected?.value);
+  const onSubscriptionChange = (selected: SelectableValue<string> | undefined) => {
+    if (onCredentialsChange) {
+      const updated: AzureCredentials = {
+        ...credentials,
+        defaultSubscriptionId: selected?.value,
+      };
+      onCredentialsChange(updated);
    }
  };

@ -230,14 +237,18 @@ export const AzureCredentialsForm: FunctionComponent<Props> = (props: Props) =>
          )}
        </>
      )}
-      {getSubscriptions && onDefaultSubscriptionChange && (
+      {getSubscriptions && (
        <>
          <div className="gf-form-inline">
            <div className="gf-form">
              <InlineFormLabel className="width-12">Default Subscription</InlineFormLabel>
              <div className="width-25">
                <Select
-                  value={subscriptions.find((opt) => opt.value === defaultSubscription)}
+                  value={
+                    credentials.defaultSubscriptionId
+                      ? subscriptions.find((opt) => opt.value === credentials.defaultSubscriptionId)
+                      : undefined
+                  }
                  options={subscriptions}
                  onChange={onSubscriptionChange}
                />
--- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MonitorConfig.tsx
+++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MonitorConfig.tsx
@ -3,7 +3,7 @@ import { SelectableValue } from '@grafana/data';
 import { config } from '@grafana/runtime';
 import { AzureCredentialsForm } from './AzureCredentialsForm';
 import { AzureDataSourceSettings, AzureCredentials } from '../types';
-import { getCredentials, updateCredentials, isLogAnalyticsSameAs } from '../credentials';
+import { getCredentials, updateCredentials } from '../credentials';

 const azureClouds = [
  { value: 'azuremonitor', label: 'Azure' },
@ -21,44 +21,19 @@ export interface Props {
 export const MonitorConfig: FunctionComponent<Props> = (props: Props) => {
  const { updateOptions, getSubscriptions } = props;
  const credentials = useMemo(() => getCredentials(props.options), [props.options]);
-  const subscriptionId = props.options.jsonData.subscriptionId;

  const onCredentialsChange = (credentials: AzureCredentials): void => {
    updateOptions((options) => updateCredentials(options, credentials));
  };

-  const onDefaultSubscriptionChange = (subscriptionId: string | undefined) => {
-    updateOptions((options) => {
-      options = {
-        ...options,
-        jsonData: {
-          ...options.jsonData,
-          subscriptionId: subscriptionId || '',
-        },
-      };
-      if (isLogAnalyticsSameAs(options)) {
-        options = {
-          ...options,
-          jsonData: {
-            ...options.jsonData,
-            logAnalyticsSubscriptionId: subscriptionId || '',
-          },
-        };
-      }
-      return options;
-    });
-  };
-
  return (
    <>
      <h3 className="page-heading">Authentication</h3>
      <AzureCredentialsForm
        managedIdentityEnabled={config.azure.managedIdentityEnabled}
        credentials={credentials}
-        defaultSubscription={subscriptionId}
        azureCloudOptions={azureClouds}
        onCredentialsChange={onCredentialsChange}
-        onDefaultSubscriptionChange={onDefaultSubscriptionChange}
        getSubscriptions={getSubscriptions}
      />
    </>
--- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/snapshots/AnalyticsConfig.test.tsx.snap
+++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/snapshots/AnalyticsConfig.test.tsx.snap
@ -103,14 +103,13 @@ exports[`Render should enable azure log analytics load workspaces button 1`] = `
        "azureCloud": "azuremonitor",
        "clientId": "44693801-6ee6-49de-9b2d-9106972f9572",
        "clientSecret": undefined,
+        "defaultSubscriptionId": "e3fe4fde-ad5e-4d60-9974-e2f3562ffdf2",
        "tenantId": "e7f3f661-a933-4b3f-8176-51c4f982ec48",
      }
    }
-    defaultSubscription="e3fe4fde-ad5e-4d60-9974-e2f3562ffdf2"
    getSubscriptions={[MockFunction]}
    managedIdentityEnabled={false}
    onCredentialsChange={[Function]}
-    onDefaultSubscriptionChange={[Function]}
  />
  <div
    className="gf-form-group"
@ -202,13 +201,13 @@ exports[`Render should render component 1`] = `
        "azureCloud": "azuremonitor",
        "clientId": undefined,
        "clientSecret": undefined,
+        "defaultSubscriptionId": undefined,
        "tenantId": "",
      }
    }
    getSubscriptions={[MockFunction]}
    managedIdentityEnabled={false}
    onCredentialsChange={[Function]}
-    onDefaultSubscriptionChange={[Function]}
  />
  <div
    className="gf-form-group"
--- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/credentials.ts
+++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/credentials.ts
@ -74,7 +74,7 @@ function getLogAnalyticsSecret(options: AzureDataSourceSettings): undefined | st
  }
 }

-export function isLogAnalyticsSameAs(options: AzureDataSourceSettings | AzureDataSourceInstanceSettings): boolean {
+function isLogAnalyticsSameAs(options: AzureDataSourceSettings | AzureDataSourceInstanceSettings): boolean {
  return typeof options.jsonData.azureLogAnalyticsSameAs !== 'boolean' || options.jsonData.azureLogAnalyticsSameAs;
 }

@ -94,6 +94,7 @@ export function getCredentials(options: AzureDataSourceSettings): AzureCredentia
      if (config.azure.managedIdentityEnabled) {
        return {
          authType: 'msi',
+          defaultSubscriptionId: options.jsonData.subscriptionId,
        };
      } else {
        // If authentication type is managed identity but managed identities were disabled in Grafana config,
@ -110,6 +111,7 @@ export function getCredentials(options: AzureDataSourceSettings): AzureCredentia
        tenantId: options.jsonData.tenantId,
        clientId: options.jsonData.clientId,
        clientSecret: getSecret(options),
+        defaultSubscriptionId: options.jsonData.subscriptionId,
      };
  }
 }
@ -133,6 +135,7 @@ export function getLogAnalyticsCredentials(options: AzureDataSourceSettings): Az
    tenantId: options.jsonData.logAnalyticsTenantId,
    clientId: options.jsonData.logAnalyticsClientId,
    clientSecret: getLogAnalyticsSecret(options),
+    defaultSubscriptionId: options.jsonData.logAnalyticsSubscriptionId,
  };
 }

@ -151,11 +154,14 @@ export function updateCredentials(
        jsonData: {
          ...options.jsonData,
          azureAuthType: 'msi',
+          subscriptionId: credentials.defaultSubscriptionId,
        },
      };

      if (!isLogAnalyticsSameAs(options)) {
        options = updateLogAnalyticsSameAs(options, true);
+      } else {
+        options = updateLogAnalyticsCredentials(options, credentials);
      }

      return options;
@ -169,6 +175,7 @@ export function updateCredentials(
          cloudName: credentials.azureCloud || getDefaultAzureCloud(),
          tenantId: credentials.tenantId,
          clientId: credentials.clientId,
+          subscriptionId: credentials.defaultSubscriptionId,
        },
        secureJsonData: {
          ...options.secureJsonData,
@ -179,7 +186,7 @@ export function updateCredentials(
        },
        secureJsonFields: {
          ...options.secureJsonFields,
-          clientSecret: typeof credentials.clientSecret === 'object',
+          clientSecret: typeof credentials.clientSecret === 'symbol',
        },
      };

@ -218,6 +225,15 @@ export function updateLogAnalyticsCredentials(
    };
  }

+  // Default subscription
+  options = {
+    ...options,
+    jsonData: {
+      ...options.jsonData,
+      logAnalyticsSubscriptionId: credentials.defaultSubscriptionId,
+    },
+  };
+
  return options;
 }

@ -236,28 +252,16 @@ export function updateLogAnalyticsSameAs(options: AzureDataSourceSettings, sameA
      // Get the primary credentials
      let credentials = getCredentials(options);

-      // Log Analytics credentials only used if primary credentials are App Registration (client secret)
-      if (credentials.authType === 'clientsecret') {
-        // Check whether the client secret is concealed
-        if (typeof credentials.clientSecret === 'symbol') {
-          // Log Analytics credentials need to be synchronized but the client secret is concealed,
-          // so we have to reset the primary client secret to ensure that user enters a new secret
-          credentials.clientSecret = undefined;
-          options = updateCredentials(options, credentials);
-        }
-
-        // Synchronize the Log Analytics credentials with primary credentials
-        options = updateLogAnalyticsCredentials(options, credentials);
+      // Check whether the primary client secret is concealed
+      if (credentials.authType === 'clientsecret' && typeof credentials.clientSecret === 'symbol') {
+        // Log Analytics credentials need to be synchronized but the client secret is concealed,
+        // so we have to reset the primary client secret to ensure that user enters a new secret
+        credentials.clientSecret = undefined;
+        options = updateCredentials(options, credentials);
      }

-      // Synchronize default subscription
-      options = {
-        ...options,
-        jsonData: {
-          ...options.jsonData,
-          logAnalyticsSubscriptionId: options.jsonData.subscriptionId,
-        },
-      };
+      // Synchronize the Log Analytics credentials with primary credentials
+      options = updateLogAnalyticsCredentials(options, credentials);
    }
  }

--- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/types/index.ts
+++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/types/index.ts
@ -54,13 +54,16 @@ export type AzureAuthType = 'msi' | 'clientsecret';

 export type ConcealedSecret = symbol;

-export type AzureCredentials = AzureManagedIdentityCredentials | AzureClientSecretCredentials;
+interface AzureCredentialsBase {
+  authType: AzureAuthType;
+  defaultSubscriptionId?: string;
+}

-export interface AzureManagedIdentityCredentials {
+export interface AzureManagedIdentityCredentials extends AzureCredentialsBase {
  authType: 'msi';
 }

-export interface AzureClientSecretCredentials {
+export interface AzureClientSecretCredentials extends AzureCredentialsBase {
  authType: 'clientsecret';
  azureCloud?: string;
  tenantId?: string;
@ -68,6 +71,8 @@ export interface AzureClientSecretCredentials {
  clientSecret?: string | ConcealedSecret;
 }

+export type AzureCredentials = AzureManagedIdentityCredentials | AzureClientSecretCredentials;
+
 export interface AzureDataSourceJsonData extends DataSourceJsonData {
  cloudName: string;
  azureAuthType?: AzureAuthType;
@ -75,7 +80,7 @@ export interface AzureDataSourceJsonData extends DataSourceJsonData {
  // monitor
  tenantId?: string;
  clientId?: string;
-  subscriptionId: string;
+  subscriptionId?: string;

  // logs
  azureLogAnalyticsSameAs?: boolean;