Datasources: Simplify the AzureCredentials structure in datasource config (#39209)

Related #35857

Co-authored-by: Marcus Efraimsson <marcus.efraimsson@gmail.com>

packages/grafana-ui/src/components/DataSourceSettings/DataSourceHttpSettings.tsx

@@ -129,6 +129,9 @@ export const DataSourceHttpSettings: React.FC<HttpSettingsProps> = (props) => {
     />
   );
 
+  const azureAuthEnabled: boolean =
+    (azureAuthSettings?.azureAuthSupported && azureAuthSettings.getAzureAuthEnabled(dataSourceConfig)) || false;
+
   return (
     <div className="gf-form-group">
       <>
@@ -219,16 +222,16 @@ export const DataSourceHttpSettings: React.FC<HttpSettingsProps> = (props) => {
             />
           </div>
 
-          {azureAuthSettings?.azureAuthEnabled && (
+          {azureAuthSettings?.azureAuthSupported && (
             <div className="gf-form-inline">
               <Switch
                 label="Azure Authentication"
                 labelClass="width-13"
-                checked={dataSourceConfig.jsonData.azureAuth || false}
+                checked={azureAuthEnabled}
                 onChange={(event) => {
-                  onSettingsChange({
+                  onSettingsChange(
-                    jsonData: { ...dataSourceConfig.jsonData, azureAuth: event!.currentTarget.checked },
+                    azureAuthSettings.setAzureAuthEnabled(dataSourceConfig, event!.currentTarget.checked)
-                  });
+                  );
                 }}
                 tooltip="Use Azure authentication for Azure endpoint."
               />
@@ -267,11 +270,9 @@ export const DataSourceHttpSettings: React.FC<HttpSettingsProps> = (props) => {
           </>
         )}
 
-        {azureAuthSettings?.azureAuthEnabled &&
+        {azureAuthSettings?.azureAuthSupported && azureAuthEnabled && azureAuthSettings.azureSettingsUI && (
-          azureAuthSettings?.azureSettingsUI &&
+          <azureAuthSettings.azureSettingsUI dataSourceConfig={dataSourceConfig} onChange={onChange} />
-          dataSourceConfig.jsonData.azureAuth && (
+        )}
-            <azureAuthSettings.azureSettingsUI dataSourceConfig={dataSourceConfig} onChange={onChange} />
-          )}
 
         {dataSourceConfig.jsonData.sigV4Auth && sigV4AuthToggleEnabled && <SigV4AuthSettings {...props} />}
 

packages/grafana-ui/src/components/DataSourceSettings/types.ts

@@ -2,8 +2,20 @@ import React from 'react';
 import { DataSourceSettings } from '@grafana/data';
 
 export interface AzureAuthSettings {
-  azureAuthEnabled: boolean;
+  /** Set to true if Azure authentication supported by the datasource */
-  azureSettingsUI?: React.ComponentType<HttpSettingsBaseProps>;
+  readonly azureAuthSupported: boolean;
+
+  /** Gets whether the Azure authentication currently enabled for the datasource */
+  readonly getAzureAuthEnabled: (config: DataSourceSettings<any, any>) => boolean;
+
+  /** Enables/disables the Azure authentication from the datasource */
+  readonly setAzureAuthEnabled: (
+    config: DataSourceSettings<any, any>,
+    enabled: boolean
+  ) => Partial<DataSourceSettings<any, any>>;
+
+  /** Optional React component of additional Azure settings UI if authentication is enabled  */
+  readonly azureSettingsUI?: React.ComponentType<HttpSettingsBaseProps>;
 }
 
 export interface HttpSettingsBaseProps<JSONData = any, SecureJSONData = any> {

pkg/infra/httpclient/httpclientprovider/azure_middleware.go

@@ -16,17 +16,11 @@ const azureMiddlewareName = "AzureAuthentication.Provider"
 
 func AzureMiddleware(cfg *setting.Cfg) httpclient.Middleware {
 	return httpclient.NamedMiddlewareFunc(azureMiddlewareName, func(opts httpclient.Options, next http.RoundTripper) http.RoundTripper {
-		if enabled, err := isAzureAuthenticationEnabled(opts.CustomOptions); err != nil {
-			return errorResponse(err)
-		} else if !enabled {
-			return next
-		}
-
 		credentials, err := getAzureCredentials(opts.CustomOptions)
 		if err != nil {
 			return errorResponse(err)
 		} else if credentials == nil {
-			credentials = getDefaultAzureCredentials(cfg)
+			return next
 		}
 
 		tokenProvider, err := aztokenprovider.NewAzureAccessTokenProvider(cfg, credentials)
@@ -49,17 +43,6 @@ func errorResponse(err error) http.RoundTripper {
 	})
 }
 
-func isAzureAuthenticationEnabled(customOptions map[string]interface{}) (bool, error) {
-	if untypedValue, ok := customOptions["_azureAuth"]; !ok {
-		return false, nil
-	} else if value, ok := untypedValue.(bool); !ok {
-		err := fmt.Errorf("the field 'azureAuth' should be a bool")
-		return false, err
-	} else {
-		return value, nil
-	}
-}
-
 func getAzureCredentials(customOptions map[string]interface{}) (azcredentials.AzureCredentials, error) {
 	if untypedValue, ok := customOptions["_azureCredentials"]; !ok {
 		return nil, nil
@@ -71,16 +54,6 @@ func getAzureCredentials(customOptions map[string]interface{}) (azcredentials.Az
 	}
 }
 
-func getDefaultAzureCredentials(cfg *setting.Cfg) azcredentials.AzureCredentials {
-	if cfg.Azure.ManagedIdentityEnabled {
-		return &azcredentials.AzureManagedIdentityCredentials{}
-	} else {
-		return &azcredentials.AzureClientSecretCredentials{
-			AzureCloud: cfg.Azure.Cloud,
-		}
-	}
-}
-
 func getAzureEndpointResourceId(customOptions map[string]interface{}) (*url.URL, error) {
 	var value string
 	if untypedValue, ok := customOptions["azureEndpointResourceId"]; !ok {

pkg/services/datasources/service.go

@@ -277,14 +277,13 @@ func (s *Service) httpClientOptions(ds *models.DataSource) (*sdkhttpclient.Optio
 		}
 	}
 
-	if ds.JsonData != nil && ds.JsonData.Get("azureAuth").MustBool() {
+	if ds.JsonData != nil {
 		credentials, err := azcredentials.FromDatasourceData(ds.JsonData.MustMap(), s.DecryptedValues(ds))
 		if err != nil {
 			err = fmt.Errorf("invalid Azure credentials: %s", err)
 			return nil, err
 		}
 
-		opts.CustomOptions["_azureAuth"] = true
 		if credentials != nil {
 			opts.CustomOptions["_azureCredentials"] = credentials
 		}

pkg/services/datasources/service_test.go

@@ -570,7 +570,7 @@ func TestService_HTTPClientOptions(t *testing.T) {
 	}
 
 	t.Run("Azure authentication", func(t *testing.T) {
-		t.Run("should be disabled if not enabled in JsonData", func(t *testing.T) {
+		t.Run("should be disabled if no Azure credentials configured", func(t *testing.T) {
 			t.Cleanup(func() { ds.JsonData = emptyJsonData; ds.SecureJsonData = emptySecureJsonData })
 
 			secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore())
@@ -579,32 +579,13 @@ func TestService_HTTPClientOptions(t *testing.T) {
 			opts, err := dsService.httpClientOptions(&ds)
 			require.NoError(t, err)
 
-			assert.NotEqual(t, true, opts.CustomOptions["_azureAuth"])
 			assert.NotContains(t, opts.CustomOptions, "_azureCredentials")
 		})
 
-		t.Run("should be enabled if enabled in JsonData without credentials configured", func(t *testing.T) {
+		t.Run("should be enabled if Azure credentials configured", func(t *testing.T) {
 			t.Cleanup(func() { ds.JsonData = emptyJsonData; ds.SecureJsonData = emptySecureJsonData })
 
 			ds.JsonData = simplejson.NewFromAny(map[string]interface{}{
-				"azureAuth": true,
-			})
-
-			secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore())
-			dsService := ProvideService(bus.New(), nil, secretsService, &acmock.Mock{})
-
-			opts, err := dsService.httpClientOptions(&ds)
-			require.NoError(t, err)
-
-			assert.Equal(t, true, opts.CustomOptions["_azureAuth"])
-			assert.NotContains(t, opts.CustomOptions, "_azureCredentials")
-		})
-
-		t.Run("should be enabled if enabled in JsonData with credentials configured", func(t *testing.T) {
-			t.Cleanup(func() { ds.JsonData = emptyJsonData; ds.SecureJsonData = emptySecureJsonData })
-
-			ds.JsonData = simplejson.NewFromAny(map[string]interface{}{
-				"azureAuth": true,
 				"azureCredentials": map[string]interface{}{
 					"authType": "msi",
 				},
@@ -616,39 +597,16 @@ func TestService_HTTPClientOptions(t *testing.T) {
 			opts, err := dsService.httpClientOptions(&ds)
 			require.NoError(t, err)
 
-			assert.Equal(t, true, opts.CustomOptions["_azureAuth"])
-
 			require.Contains(t, opts.CustomOptions, "_azureCredentials")
 			credentials := opts.CustomOptions["_azureCredentials"]
 
 			assert.IsType(t, &azcredentials.AzureManagedIdentityCredentials{}, credentials)
 		})
 
-		t.Run("should be disabled if disabled in JsonData even with credentials configured", func(t *testing.T) {
-			t.Cleanup(func() { ds.JsonData = emptyJsonData; ds.SecureJsonData = emptySecureJsonData })
-
-			ds.JsonData = simplejson.NewFromAny(map[string]interface{}{
-				"azureAuth": false,
-				"azureCredentials": map[string]interface{}{
-					"authType": "msi",
-				},
-			})
-
-			secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore())
-			dsService := ProvideService(bus.New(), nil, secretsService, &acmock.Mock{})
-
-			opts, err := dsService.httpClientOptions(&ds)
-			require.NoError(t, err)
-
-			assert.NotEqual(t, true, opts.CustomOptions["_azureAuth"])
-			assert.NotContains(t, opts.CustomOptions, "_azureCredentials")
-		})
-
 		t.Run("should fail if credentials are invalid", func(t *testing.T) {
 			t.Cleanup(func() { ds.JsonData = emptyJsonData; ds.SecureJsonData = emptySecureJsonData })
 
 			ds.JsonData = simplejson.NewFromAny(map[string]interface{}{
-				"azureAuth":        true,
 				"azureCredentials": "invalid",
 			})
 

public/app/plugins/datasource/prometheus/configuration/AzureCredentialsConfig.ts

@@ -18,16 +18,25 @@ function getSecret(options: DataSourceSettings<any, any>): undefined | string |
   }
 }
 
+export function hasCredentials(options: DataSourceSettings<any, any>): boolean {
+  return !!options.jsonData.azureCredentials;
+}
+
+export function getDefaultCredentials(): AzureCredentials {
+  if (config.azure.managedIdentityEnabled) {
+    return { authType: 'msi' };
+  } else {
+    return { authType: 'clientsecret', azureCloud: getDefaultAzureCloud() };
+  }
+}
+
 export function getCredentials(options: DataSourceSettings<any, any>): AzureCredentials {
   const credentials = options.jsonData.azureCredentials as AzureCredentials | undefined;
 
   // If no credentials saved, then return empty credentials
   // of type based on whether the managed identity enabled
   if (!credentials) {
-    return {
+    return getDefaultCredentials();
-      authType: config.azure.managedIdentityEnabled ? 'msi' : 'clientsecret',
-      azureCloud: getDefaultAzureCloud(),
-    };
   }
 
   switch (credentials.authType) {
@@ -105,3 +114,23 @@ export function updateCredentials(
       return options;
   }
 }
+
+export function setDefaultCredentials(options: DataSourceSettings<any, any>): Partial<DataSourceSettings<any, any>> {
+  return {
+    jsonData: {
+      ...options.jsonData,
+      azureCredentials: getDefaultCredentials(),
+    },
+  };
+}
+
+export function resetCredentials(options: DataSourceSettings<any, any>): Partial<DataSourceSettings<any, any>> {
+  return {
+    jsonData: {
+      ...options.jsonData,
+      azureAuth: undefined,
+      azureCredentials: undefined,
+      azureEndpointResourceId: undefined,
+    },
+  };
+}

public/app/plugins/datasource/prometheus/configuration/ConfigEditor.tsx

@@ -1,10 +1,11 @@
 import React from 'react';
 import { AlertingSettings, DataSourceHttpSettings, Alert } from '@grafana/ui';
-import { DataSourcePluginOptionsEditorProps } from '@grafana/data';
+import { DataSourcePluginOptionsEditorProps, DataSourceSettings } from '@grafana/data';
 import { config } from 'app/core/config';
 import { PromOptions } from '../types';
 import { AzureAuthSettings } from './AzureAuthSettings';
 import { PromSettings } from './PromSettings';
+import { hasCredentials, setDefaultCredentials, resetCredentials } from './AzureCredentialsConfig';
 import { getAllAlertmanagerDataSources } from 'app/features/alerting/unified/utils/alertmanager';
 
 export type Props = DataSourcePluginOptionsEditorProps<PromOptions>;
@@ -13,7 +14,10 @@ export const ConfigEditor = (props: Props) => {
   const alertmanagers = getAllAlertmanagerDataSources();
 
   const azureAuthSettings = {
-    azureAuthEnabled: config.featureToggles['prometheus_azure_auth'] ?? false,
+    azureAuthSupported: config.featureToggles['prometheus_azure_auth'] ?? false,
+    getAzureAuthEnabled: (config: DataSourceSettings<any, any>): boolean => hasCredentials(config),
+    setAzureAuthEnabled: (config: DataSourceSettings<any, any>, enabled: boolean) =>
+      enabled ? setDefaultCredentials(config) : resetCredentials(config),
     azureSettingsUI: AzureAuthSettings,
   };