mirror of https://github.com/grafana/grafana.git
				
				
				
			Azure Monitor: Add a feature flag to toggle user auth for Azure Monitor only (#96858)
* Azure Monitor: Add a feature flag to toggle user auth for Azure Monitor only * Fix condition for userIdentityEnabled * Re-add removed test * Remove unused prop * Refactor onAuthTypeChange in AzureCredentialsForm * Add frontend unit tests * Lint
This commit is contained in:
		
							parent
							
								
									d18cdae3e2
								
							
						
					
					
						commit
						b898a4540d
					
				|  | @ -81,6 +81,7 @@ Most [generally available](https://grafana.com/docs/release-life-cycle/#general- | |||
| | `azureMonitorDisableLogLimit`          | Disables the log limit restriction for Azure Monitor when true. The limit is enabled by default.                                                                   |                    | | ||||
| | `preinstallAutoUpdate`                 | Enables automatic updates for pre-installed plugins                                                                                                                | Yes                | | ||||
| | `alertingUIOptimizeReducer`            | Enables removing the reducer from the alerting UI when creating a new alert rule and using instant query                                                           | Yes                | | ||||
| | `azureMonitorEnableUserAuth`           | Enables user auth for Azure Monitor datasource only                                                                                                                | Yes                | | ||||
| 
 | ||||
| ## Public preview feature toggles | ||||
| 
 | ||||
|  |  | |||
|  | @ -241,5 +241,6 @@ export interface FeatureToggles { | |||
|   jaegerBackendMigration?: boolean; | ||||
|   reportingUseRawTimeRange?: boolean; | ||||
|   alertingUIOptimizeReducer?: boolean; | ||||
|   azureMonitorEnableUserAuth?: boolean; | ||||
|   alertingNotificationsStepMode?: boolean; | ||||
| } | ||||
|  |  | |||
|  | @ -1666,6 +1666,13 @@ var ( | |||
| 			Owner:        grafanaAlertingSquad, | ||||
| 			Expression:   "true", // enabled by default
 | ||||
| 		}, | ||||
| 		{ | ||||
| 			Name:        "azureMonitorEnableUserAuth", | ||||
| 			Description: "Enables user auth for Azure Monitor datasource only", | ||||
| 			Stage:       FeatureStageGeneralAvailability, | ||||
| 			Owner:       grafanaPartnerPluginsSquad, | ||||
| 			Expression:  "true", // Enabled by default for now
 | ||||
| 		}, | ||||
| 		{ | ||||
| 			Name:         "alertingNotificationsStepMode", | ||||
| 			Description:  "Enables simplified step mode in the notifications section", | ||||
|  |  | |||
|  | @ -222,4 +222,5 @@ crashDetection,experimental,@grafana/observability-traces-and-profiling,false,fa | |||
| jaegerBackendMigration,experimental,@grafana/oss-big-tent,false,false,false | ||||
| reportingUseRawTimeRange,preview,@grafana/sharing-squad,false,false,false | ||||
| alertingUIOptimizeReducer,GA,@grafana/alerting-squad,false,false,true | ||||
| azureMonitorEnableUserAuth,GA,@grafana/partner-datasources,false,false,false | ||||
| alertingNotificationsStepMode,experimental,@grafana/alerting-squad,false,false,true | ||||
|  |  | |||
| 
 | 
|  | @ -899,6 +899,10 @@ const ( | |||
| 	// Enables removing the reducer from the alerting UI when creating a new alert rule and using instant query
 | ||||
| 	FlagAlertingUIOptimizeReducer = "alertingUIOptimizeReducer" | ||||
| 
 | ||||
| 	// FlagAzureMonitorEnableUserAuth
 | ||||
| 	// Enables user auth for Azure Monitor datasource only
 | ||||
| 	FlagAzureMonitorEnableUserAuth = "azureMonitorEnableUserAuth" | ||||
| 
 | ||||
| 	// FlagAlertingNotificationsStepMode
 | ||||
| 	// Enables simplified step mode in the notifications section
 | ||||
| 	FlagAlertingNotificationsStepMode = "alertingNotificationsStepMode" | ||||
|  |  | |||
|  | @ -660,6 +660,22 @@ | |||
|         "expression": "false" | ||||
|       } | ||||
|     }, | ||||
|     { | ||||
|       "metadata": { | ||||
|         "name": "azureMonitorEnableUserAuth", | ||||
|         "resourceVersion": "1732189410576", | ||||
|         "creationTimestamp": "2024-11-21T11:42:29Z", | ||||
|         "annotations": { | ||||
|           "grafana.app/updatedTimestamp": "2024-11-21 11:43:30.576196 +0000 UTC" | ||||
|         } | ||||
|       }, | ||||
|       "spec": { | ||||
|         "description": "Enables user auth for Azure Monitor datasource only", | ||||
|         "stage": "GA", | ||||
|         "codeowner": "@grafana/partner-datasources", | ||||
|         "expression": "true" | ||||
|       } | ||||
|     }, | ||||
|     { | ||||
|       "metadata": { | ||||
|         "name": "azureMonitorLogLimit", | ||||
|  |  | |||
|  | @ -10,6 +10,7 @@ import ( | |||
| 	"net/http" | ||||
| 	"strconv" | ||||
| 
 | ||||
| 	"github.com/grafana/grafana-azure-sdk-go/v2/azcredentials" | ||||
| 	"github.com/grafana/grafana-azure-sdk-go/v2/azsettings" | ||||
| 	"github.com/grafana/grafana-azure-sdk-go/v2/azusercontext" | ||||
| 	"github.com/grafana/grafana-plugin-sdk-go/backend" | ||||
|  | @ -139,6 +140,10 @@ func NewInstanceSettings(clientProvider *httpclient.Provider, executors map[stri | |||
| 			return nil, err | ||||
| 		} | ||||
| 
 | ||||
| 		if credentials.AzureAuthType() == azcredentials.AzureAuthCurrentUserIdentity && !backend.GrafanaConfigFromContext(ctx).FeatureToggles().IsEnabled("azureMonitorEnableUserAuth") { | ||||
| 			return nil, backend.DownstreamError(errors.New("current user authentication is not enabled for azure monitor")) | ||||
| 		} | ||||
| 
 | ||||
| 		model := types.DatasourceInfo{ | ||||
| 			Credentials:             credentials, | ||||
| 			Settings:                azMonitorSettings, | ||||
|  |  | |||
|  | @ -17,6 +17,7 @@ import ( | |||
| 	"github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" | ||||
| 	"github.com/grafana/grafana-plugin-sdk-go/backend/instancemgmt" | ||||
| 	"github.com/grafana/grafana-plugin-sdk-go/backend/log" | ||||
| 	"github.com/grafana/grafana-plugin-sdk-go/experimental/featuretoggles" | ||||
| 
 | ||||
| 	"github.com/grafana/grafana/pkg/tsdb/azuremonitor/types" | ||||
| 
 | ||||
|  | @ -59,9 +60,29 @@ func TestNewInstanceSettings(t *testing.T) { | |||
| 	tests := []struct { | ||||
| 		name          string | ||||
| 		settings      backend.DataSourceInstanceSettings | ||||
| 		expectedModel types.DatasourceInfo | ||||
| 		expectedModel *types.DatasourceInfo | ||||
| 		Err           require.ErrorAssertionFunc | ||||
| 		setupContext  func(ctx context.Context) context.Context | ||||
| 	}{ | ||||
| 		{ | ||||
| 			name: "current user authentication disabled by feature toggle", | ||||
| 			settings: backend.DataSourceInstanceSettings{ | ||||
| 				JSONData:                []byte(`{"azureAuthType":"currentuser"}`), | ||||
| 				DecryptedSecureJSONData: map[string]string{}, | ||||
| 				ID:                      60, | ||||
| 			}, | ||||
| 			expectedModel: nil, | ||||
| 			Err: func(t require.TestingT, err error, _ ...interface{}) { | ||||
| 				require.Error(t, err) | ||||
| 				require.Contains(t, err.Error(), "current user authentication is not enabled for azure monitor") | ||||
| 			}, | ||||
| 			setupContext: func(ctx context.Context) context.Context { | ||||
| 				featureToggles := backend.NewGrafanaCfg(map[string]string{ | ||||
| 					featuretoggles.EnabledFeatures: "", // No enabled features
 | ||||
| 				}) | ||||
| 				return backend.WithGrafanaConfig(ctx, featureToggles) | ||||
| 			}, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name: "creates an instance", | ||||
| 			settings: backend.DataSourceInstanceSettings{ | ||||
|  | @ -69,7 +90,7 @@ func TestNewInstanceSettings(t *testing.T) { | |||
| 				DecryptedSecureJSONData: map[string]string{"key": "value"}, | ||||
| 				ID:                      40, | ||||
| 			}, | ||||
| 			expectedModel: types.DatasourceInfo{ | ||||
| 			expectedModel: &types.DatasourceInfo{ | ||||
| 				Credentials:             &azcredentials.AzureManagedIdentityCredentials{}, | ||||
| 				Settings:                types.AzureMonitorSettings{}, | ||||
| 				Routes:                  testRoutes, | ||||
|  | @ -87,7 +108,7 @@ func TestNewInstanceSettings(t *testing.T) { | |||
| 				DecryptedSecureJSONData: map[string]string{"clientSecret": "secret"}, | ||||
| 				ID:                      50, | ||||
| 			}, | ||||
| 			expectedModel: types.DatasourceInfo{ | ||||
| 			expectedModel: &types.DatasourceInfo{ | ||||
| 				Credentials: &azcredentials.AzureClientSecretCredentials{ | ||||
| 					AzureCloud:   "AzureCustomizedCloud", | ||||
| 					ClientSecret: "secret", | ||||
|  | @ -117,11 +138,23 @@ func TestNewInstanceSettings(t *testing.T) { | |||
| 
 | ||||
| 	for _, tt := range tests { | ||||
| 		t.Run(tt.name, func(t *testing.T) { | ||||
| 			ctx := context.Background() | ||||
| 			if tt.setupContext != nil { | ||||
| 				ctx = tt.setupContext(ctx) | ||||
| 			} | ||||
| 
 | ||||
| 			factory := NewInstanceSettings(&httpclient.Provider{}, map[string]azDatasourceExecutor{}, log.DefaultLogger) | ||||
| 			instance, err := factory(context.Background(), tt.settings) | ||||
| 			instance, err := factory(ctx, tt.settings) | ||||
| 
 | ||||
| 			tt.Err(t, err) | ||||
| 			if !cmp.Equal(instance, tt.expectedModel) { | ||||
| 				t.Errorf("Unexpected instance: %v", cmp.Diff(instance, tt.expectedModel)) | ||||
| 
 | ||||
| 			if tt.expectedModel == nil { | ||||
| 				require.Nil(t, instance, "Expected instance to be nil") | ||||
| 			} else { | ||||
| 				require.NotNil(t, instance, "Expected instance to be created") | ||||
| 				if !cmp.Equal(instance, *tt.expectedModel) { | ||||
| 					t.Errorf("Unexpected instance: %v", cmp.Diff(instance, *tt.expectedModel)) | ||||
| 				} | ||||
| 			} | ||||
| 		}) | ||||
| 	} | ||||
|  |  | |||
|  | @ -73,17 +73,27 @@ export const AzureCredentialsForm = (props: Props) => { | |||
|   }, [managedIdentityEnabled, workloadIdentityEnabled, userIdentityEnabled]); | ||||
| 
 | ||||
|   const onAuthTypeChange = (selected: SelectableValue<AzureAuthType>) => { | ||||
|     const defaultAuthType = managedIdentityEnabled | ||||
|       ? 'msi' | ||||
|       : workloadIdentityEnabled | ||||
|         ? 'workloadidentity' | ||||
|         : userIdentityEnabled | ||||
|           ? 'currentuser' | ||||
|           : 'clientsecret'; | ||||
|     const defaultAuthType = (() => { | ||||
|       if (managedIdentityEnabled) { | ||||
|         return 'msi'; | ||||
|       } | ||||
| 
 | ||||
|       if (workloadIdentityEnabled) { | ||||
|         return 'workloadidentity'; | ||||
|       } | ||||
| 
 | ||||
|       if (userIdentityEnabled) { | ||||
|         return 'currentuser'; | ||||
|       } | ||||
| 
 | ||||
|       return 'clientsecret'; | ||||
|     })(); | ||||
| 
 | ||||
|     const updated: AzureCredentials = { | ||||
|       ...credentials, | ||||
|       authType: selected.value || defaultAuthType, | ||||
|     }; | ||||
| 
 | ||||
|     onCredentialsChange(updated); | ||||
|   }; | ||||
| 
 | ||||
|  | @ -122,7 +132,6 @@ export const AzureCredentialsForm = (props: Props) => { | |||
|           disabled={disabled} | ||||
|           managedIdentityEnabled={managedIdentityEnabled} | ||||
|           workloadIdentityEnabled={workloadIdentityEnabled} | ||||
|           userIdentityEnabled={userIdentityEnabled} | ||||
|         /> | ||||
|       )} | ||||
|     </ConfigSection> | ||||
|  |  | |||
|  | @ -10,7 +10,6 @@ const setup = (propsFunc?: (props: Props) => Props) => { | |||
|   let props: Props = { | ||||
|     managedIdentityEnabled: true, | ||||
|     workloadIdentityEnabled: true, | ||||
|     userIdentityEnabled: true, | ||||
|     credentials: { authType: 'currentuser' }, | ||||
|     azureCloudOptions: [ | ||||
|       { value: 'AzureCloud', label: 'Azure' }, | ||||
|  |  | |||
|  | @ -13,7 +13,6 @@ import { AppRegistrationCredentials } from './AppRegistrationCredentials'; | |||
| export interface Props { | ||||
|   managedIdentityEnabled: boolean; | ||||
|   workloadIdentityEnabled: boolean; | ||||
|   userIdentityEnabled: boolean; | ||||
|   credentials: AadCurrentUserCredentials; | ||||
|   azureCloudOptions?: SelectableValue[]; | ||||
|   onCredentialsChange: (updatedCredentials: AzureCredentials) => void; | ||||
|  |  | |||
|  | @ -1,21 +1,37 @@ | |||
| import { render, screen, waitFor } from '@testing-library/react'; | ||||
| import { render, screen, waitFor, fireEvent, cleanup } from '@testing-library/react'; | ||||
| 
 | ||||
| import { config } from '@grafana/runtime'; | ||||
| 
 | ||||
| import { createMockDatasourceSettings } from '../../__mocks__/datasourceSettings'; | ||||
| 
 | ||||
| import { MonitorConfig, Props } from './MonitorConfig'; | ||||
| 
 | ||||
| const mockDatasourceSettings = createMockDatasourceSettings(); | ||||
| 
 | ||||
| const defaultProps: Props = { | ||||
|   options: mockDatasourceSettings, | ||||
|   options: createMockDatasourceSettings(), | ||||
|   updateOptions: jest.fn(), | ||||
|   getSubscriptions: jest.fn().mockResolvedValue([]), | ||||
| }; | ||||
| 
 | ||||
| describe('MonitorConfig', () => { | ||||
|   beforeEach(() => { | ||||
|     config.azure = { | ||||
|       ...config.azure, | ||||
|       managedIdentityEnabled: false, | ||||
|       workloadIdentityEnabled: false, | ||||
|       userIdentityEnabled: false, | ||||
|     }; | ||||
|     config.featureToggles = { | ||||
|       azureMonitorEnableUserAuth: false, | ||||
|     }; | ||||
|   }); | ||||
| 
 | ||||
|   afterEach(() => { | ||||
|     cleanup(); | ||||
|     jest.clearAllMocks(); | ||||
|   }); | ||||
| 
 | ||||
|   it('should render component', () => { | ||||
|     render(<MonitorConfig {...defaultProps} />); | ||||
| 
 | ||||
|     expect(screen.getByText('Azure Cloud')).toBeInTheDocument(); | ||||
|   }); | ||||
| 
 | ||||
|  | @ -29,6 +45,7 @@ describe('MonitorConfig', () => { | |||
|     expect(defaultProps.updateOptions).toHaveBeenCalled(); | ||||
|     expect(screen.getByText('Azure Cloud')).toBeInTheDocument(); | ||||
|   }); | ||||
| 
 | ||||
|   expect(defaultProps.options.jsonData.azureAuthType).toBe('clientsecret'); | ||||
| 
 | ||||
|   it('should render component and set the default subscription if specified', async () => { | ||||
|  | @ -39,4 +56,41 @@ describe('MonitorConfig', () => { | |||
|     expect(screen.getByText('Azure Cloud')).toBeInTheDocument(); | ||||
|     await waitFor(() => expect(screen.getByText('Test Sub')).toBeInTheDocument()); | ||||
|   }); | ||||
| 
 | ||||
|   it('should render with user identity enabled when feature toggle is true', async () => { | ||||
|     config.azure.userIdentityEnabled = true; | ||||
|     config.featureToggles.azureMonitorEnableUserAuth = true; | ||||
| 
 | ||||
|     const optionsWithUserAuth = createMockDatasourceSettings({ | ||||
|       jsonData: { azureAuthType: 'currentuser' }, | ||||
|     }); | ||||
| 
 | ||||
|     render(<MonitorConfig {...defaultProps} options={optionsWithUserAuth} />); | ||||
| 
 | ||||
|     const authDropdownInput = screen.getByTestId('data-testid auth-type').querySelector('input'); | ||||
| 
 | ||||
|     if (authDropdownInput) { | ||||
|       fireEvent.mouseDown(authDropdownInput); | ||||
|     } | ||||
| 
 | ||||
|     await waitFor(() => { | ||||
|       expect( | ||||
|         screen.getByText( | ||||
|           (content, element) => element?.tagName?.toLowerCase() === 'span' && /Current User/i.test(content) | ||||
|         ) | ||||
|       ).toBeInTheDocument(); | ||||
|     }); | ||||
|   }); | ||||
| 
 | ||||
|   it('should render with user identity disabled when feature toggle is false', async () => { | ||||
|     config.azure.userIdentityEnabled = true; | ||||
|     config.featureToggles.azureMonitorEnableUserAuth = false; | ||||
| 
 | ||||
|     render(<MonitorConfig {...defaultProps} />); | ||||
| 
 | ||||
|     await waitFor(() => { | ||||
|       expect(screen.getByText('Authentication')).toBeInTheDocument(); | ||||
|       expect(screen.queryByText(/Current User/i)).not.toBeInTheDocument(); | ||||
|     }); | ||||
|   }); | ||||
| }); | ||||
|  |  | |||
|  | @ -53,7 +53,7 @@ export const MonitorConfig = (props: Props) => { | |||
|       <AzureCredentialsForm | ||||
|         managedIdentityEnabled={config.azure.managedIdentityEnabled} | ||||
|         workloadIdentityEnabled={config.azure.workloadIdentityEnabled} | ||||
|         userIdentityEnabled={config.azure.userIdentityEnabled} | ||||
|         userIdentityEnabled={config.azure.userIdentityEnabled && !!config.featureToggles.azureMonitorEnableUserAuth} | ||||
|         credentials={credentials} | ||||
|         azureCloudOptions={getAzureCloudOptions()} | ||||
|         onCredentialsChange={onCredentialsChange} | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue