Common authentication settings UI

[kostrse]

Azure authentication settings UI same as in Azure Monitor and Prometheus datasources.
Supports migration from legacy credentials format.

Fixes #361

Migration of client secret

Since secret cannot be migrated, the existing legacy client secret field clientSecret is being preserved and will continued to be used until user updates the value. Once new value set, it will be saved to the new field azureClientSecret.

Users should not notice the fact of migration.

Depends on

• Support legacy client secret for AzureClientSecretCredentials and OBO grafana-azure-sdk-go#23

For future

The UI code can be moved into a shared JS library and shared between Azure Monitor, Prometheus and Azure Data Explorer.

[andresmgot]

Thanks for the PR Sergey! I have several inline comments but in general, I think there are two things that we can do here:

• Avoid the breaking change for the client secret. This means that we should support a partially migrated config, which is not going to be elegant from the code perspective but it would be nice for users.
• Remove the default subscription code from this PR. I know this is to refactor this config editor with the Azure Monitor one but we can move that to a different component, so ADX doesn't depend on it.

[aangelisc]

Nice work on this PR @kostrse. I echo @andresmgot comments but I actually think if we're making this change to better align the current Azure data sources then I would move this work to the Azure React repo. This will allow us to better maintain all common components required for Azure credentials.

Wdyt? cc: @andresmgot

[andresmgot]

Nice work on this PR @kostrse. I echo @andresmgot comments but I actually think if we're making this change to better align the current Azure data sources then I would move this work to the Azure React repo. This will allow us to better maintain all common components required for Azure credentials.

Wdyt? cc: @andresmgot

yes, I believe this is an intermediate step before moving the code to the SDK

[kostrse]

Nice work on this PR @kostrse. I echo @andresmgot comments but I actually think if we're making this change to better align the current Azure data sources then I would move this work to the Azure React repo. This will allow us to better maintain all common components required for Azure credentials.
Wdyt? cc: @andresmgot

yes, I believe this is an intermediate step before moving the code to the SDK

The end goal so to move everything to the shared library.

I would extract Ui to the shared library after Azure Monitor also migrated to the common 'azureCredentials' credentials format. I would look at Azure Monitor after Azure Data Explorer is done.

[kostrse]

I would want OBO to be considered a distinct authentication type, not a subset of App Registration.
One is service identity, another is user identity, substantial difference.

Please let me know what needs to be changed in the wording of the message.
Also, do we need a link to documentation? What would the the best target for it then?

[andresmgot]

Thanks for the changes, I have tested the changes and it's working for me, I just have some minor comments.

[andresmgot]

Please let me know what needs to be changed in the wording of the message.
Also, do we need a link to documentation? What would the the best target for it then?

I think this is fine then, I would just change the text of the link.

[andresmgot]

LGTM, @aangelisc do you want to do a final check too?

BTW, this will break the e2e in main because the UI changes but I can send a quick PR to fix it.

[aangelisc]

This all looks good to me as well. R.E. E2E tests, @kostrse would you be able to re-add the data-testid props that were in the original ConfigEditor? This should be a straightforward change that will ensure the E2E tests do not break. If not we can add another PR for this later 😊

[andresmgot]

This all looks good to me as well. R.E. E2E tests, @kostrse would you be able to re-add the data-testid props that were in the original ConfigEditor? This should be a straightforward change that will ensure the E2E tests do not break. If not we can add another PR for this later blush

There is one extra change needed, apart from the test-id. I have prepared a quick PR we can merge after this.