AzureSettings reader and writer

[kostrse]

This PR adds reader and writer of azsetttings.AzureSettings for external plugins.

Both reader and writer encapsulated into single packages so it should be easier to extend the settings structure in future and have consistent validation.

azsettings.ReadFromEnv will be used here (by an external plugin):
grafana/azure-data-explorer-datasource/pkg/azuredx/datasource.go#L47

azsettings.WriteToEnvStr will be used here (by the Grafana plugin manager):
grafana/grafana/pkg/plugins/manager/loader/initializer/initializer.go#L85

I decided to rename the underlying environment variables to make them more plugin specific and eliminate a chance of clash with machine wide variables (e.g. AZURE_CLOUD too generic name and could be used on a host machine). I prefixed with GF_PLUGIN_ but open for other naming approaches.

While the existing environment variables were introduced long time ago, they never been used by any Azure plugin, so they are OK to be changed.

The old variables can be removed in future (e.g. after release of Grafana 9.x).

Fixes #4

[kostrse]

@wbrowne could you please have a look? Would want to hear your opinion on naming of environment variables passed to plugins.

[wbrowne]

Thanks for looping me in @kostrse - have left a comment

[wbrowne]

We use the GF_PLUGIN_ prefix as a convention where Grafana auto creates an ENV vars based on plugin settings provided via the ini.

For example:

[plugin.grafana-azure-monitor-datasource]
cloud = true

becomes export GF_PLUGIN_GRAFANA_AZURE_MONITOR_DATASOURCE_CLOUD=true, which is subsequently accessible via a plugin. Further docs - see here.

For the AWS SDK, it's not using that mechanism. Instead it looks like so:

• Grafana manually setting the env vars based on keys exposed in the AWS SDK
• The AWS SDK reading those variables

Unless I'm forgetting something, the former should suit your cases - which would mean this line should be

envAzureCloud = "GF_PLUGIN_{CORRECT_PLUGIN_ID}_CLOUD"

[kostrse]

If user set some settings for the host (via config or env vars), I would not want these raw settings to be automatically transferred to a plugin as is.

AzureSettings structure is very simple now but it will grow in complexity in future and will have state which is not direct user input but calculation of combinations of different user inputs and hosting environment (like combination of OAuth settings with Azure settings etc.).

Settings provided by a user are being validated/sanitized by the host (/pkg/setting/setting_azure.go - it is simple now) and resulting AzureSettings being used by built-in Azure datasources too.

So I want transfer of the computed AzureSettings structure between host and plugin. This means serialization/deserialization of AzureSettings via env vars should be isolated from other effects.

I will make prefix unique for this package WriteToEnvStr/ReadFromEnv to GFAZPL_ so it's guaranteed to not have another meaning in Grafana like (GF_PLUGIN_ does):

GFAZPL_AZURE_CLOUD=
GFAZPL_AZURE_MANAGED_IDENTITY_ENABLED=
GFAZPL_AZURE_MANAGED_IDENTITY_CLIENT_ID=

I could even use UUIDs but it would complicate troubleshooting.

[andresmgot]

Settings provided by a user are being validated/sanitized by the host (/pkg/setting/setting_azure.go - it is simple now) and resulting AzureSettings being used by built-in Azure datasources too.

So I want transfer of the computed AzureSettings structure between host and plugin. This means serialization/deserialization of AzureSettings via env vars should be isolated from other effects.

I don't really understand this, for example, since cloud is already a setting in the configuration
, it can be set using a custom config file, and the environment variable GF_AZURE_CLOUD. Would that be different if the user set GFAZPL_AZURE_CLOUD (or AZURE_CLOUD since that seems to be another alternative)? Is the goal to remove that configuration?

[kostrse]

Use can set cloud via GF_AZURE_CLOUD or via config file and this value goes through validation here.

So the value stored in cfg.Azure.Cloud can be different from what user initially provided.

Built-in plugins won't see user's original value but the final value which is stored in cfg.Azure.Cloud (i.e. azsettings.AzureSettings).

When transferring contents of the azsettings.AzureSettings structure from host to external plugins, I want them to not be unintentionally mixed with other environment variables set on the machine. So I want the env keys to be unique enough. So, external plugins would received the same contents of azsettings.AzureSettings as it is on host.

[andresmgot]

ah, I get it now, the problem is that plugins don't have access to the cfg information so the mechanism to allow this is to expose certain information as environment variables. The goal is for grafana/grafana to expose GFAZPL_AZURE_CLOUD so this SDK can read it, right? (not for the user to set that variable, that's why the name should be unique enough).

[sunker]

Looks good to me. For housekeeping reasons, you may want to unset environment variables used in your unit test. E.g

err := os.Setenv("ENV_VAR", "value")
defer os.Unsetenv("ENV_VAR")

[andresmgot]

LGTM, it feels weird that the Grafana plugin SDK doesn't expose a function to directly read the configuration but I guess this is how it works. Thanks for the explanation.