From ee76613eaee845aa10ebdc40ddc3581959080e62 Mon Sep 17 00:00:00 2001 From: Andreas Christou Date: Tue, 23 Jul 2024 17:31:22 +0700 Subject: [PATCH] [v11.0.x] Prometheus: Reintroduce Azure audience override feature flag (#90558) Prometheus: Reintroduce Azure audience override feature flag (#90339) * Re-add feature flag with deprecation note * Hide the field in frontend if ff disabled * Block scope overriding if ff is disabled in backend - Update promlib to forward logger to extendOptions - Add warning - Update tests * Default toggle to true for now * Update description * Update prom tests * Fix lint (cherry picked from commit 2616366a0a90de214a30393d4681ea193204d3f6) # Conflicts: # packages/grafana-data/src/types/featureToggles.gen.ts # pkg/services/featuremgmt/registry.go # pkg/services/featuremgmt/toggles_gen.csv # pkg/services/featuremgmt/toggles_gen.go # pkg/services/featuremgmt/toggles_gen.json # pkg/tsdb/prometheus/prometheus.go --- .../src/types/featureToggles.gen.ts | 1 + pkg/promlib/heuristics_test.go | 3 +- pkg/promlib/library.go | 4 +- pkg/promlib/library_test.go | 5 +- pkg/services/featuremgmt/registry.go | 7 +++ pkg/services/featuremgmt/toggles_gen.csv | 1 + pkg/services/featuremgmt/toggles_gen.go | 4 ++ pkg/services/featuremgmt/toggles_gen.json | 12 ++++ pkg/tsdb/prometheus/azureauth/azure.go | 12 +++- pkg/tsdb/prometheus/azureauth/azure_test.go | 60 +++++++++++++++++-- pkg/tsdb/prometheus/prometheus.go | 7 ++- pkg/tsdb/prometheus/prometheus_test.go | 4 +- .../configuration/AzureAuthSettings.tsx | 43 +++++++------ 13 files changed, 126 insertions(+), 37 deletions(-) diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 1d27dc64036..dc5b693e7ef 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -177,4 +177,5 @@ export interface FeatureToggles { ssoSettingsSAML?: boolean; usePrometheusFrontendPackage?: boolean; oauthRequireSubClaim?: boolean; + prometheusAzureOverrideAudience?: boolean; } diff --git a/pkg/promlib/heuristics_test.go b/pkg/promlib/heuristics_test.go index 48b6f380ad8..21831e43247 100644 --- a/pkg/promlib/heuristics_test.go +++ b/pkg/promlib/heuristics_test.go @@ -14,6 +14,7 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/backend/datasource" sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" + "github.com/grafana/grafana-plugin-sdk-go/backend/log" ) type heuristicsSuccessRoundTripper struct { @@ -41,7 +42,7 @@ func newHeuristicsSDKProvider(hrt heuristicsSuccessRoundTripper) *sdkhttpclient. return sdkhttpclient.NewProvider(sdkhttpclient.ProviderOptions{Middlewares: []sdkhttpclient.Middleware{mid}}) } -func mockExtendClientOpts(ctx context.Context, settings backend.DataSourceInstanceSettings, clientOpts *sdkhttpclient.Options) error { +func mockExtendClientOpts(ctx context.Context, settings backend.DataSourceInstanceSettings, clientOpts *sdkhttpclient.Options, log log.Logger) error { return nil } diff --git a/pkg/promlib/library.go b/pkg/promlib/library.go index 1a441e9305e..bd7d48b2fc1 100644 --- a/pkg/promlib/library.go +++ b/pkg/promlib/library.go @@ -32,7 +32,7 @@ type instance struct { versionCache *cache.Cache } -type ExtendOptions func(ctx context.Context, settings backend.DataSourceInstanceSettings, clientOpts *sdkhttpclient.Options) error +type ExtendOptions func(ctx context.Context, settings backend.DataSourceInstanceSettings, clientOpts *sdkhttpclient.Options, log log.Logger) error func NewService(httpClientProvider *sdkhttpclient.Provider, plog log.Logger, extendOptions ExtendOptions) *Service { if httpClientProvider == nil { @@ -53,7 +53,7 @@ func newInstanceSettings(httpClientProvider *sdkhttpclient.Provider, log log.Log } if extendOptions != nil { - err = extendOptions(ctx, settings, opts) + err = extendOptions(ctx, settings, opts, log) if err != nil { return nil, fmt.Errorf("error extending transport options: %v", err) } diff --git a/pkg/promlib/library_test.go b/pkg/promlib/library_test.go index 04529d608d0..9bdf5d504bd 100644 --- a/pkg/promlib/library_test.go +++ b/pkg/promlib/library_test.go @@ -10,6 +10,7 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/backend" sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" + "github.com/grafana/grafana-plugin-sdk-go/backend/log" "github.com/stretchr/testify/require" ) @@ -60,7 +61,7 @@ func getMockPromTestSDKProvider(f *fakeHTTPClientProvider) *sdkhttpclient.Provid return sdkhttpclient.NewProvider(sdkhttpclient.ProviderOptions{Middlewares: []sdkhttpclient.Middleware{mid}}) } -func mockExtendTransportOptions(ctx context.Context, settings backend.DataSourceInstanceSettings, clientOpts *sdkhttpclient.Options) error { +func mockExtendTransportOptions(ctx context.Context, settings backend.DataSourceInstanceSettings, clientOpts *sdkhttpclient.Options, log log.Logger) error { return nil } @@ -103,7 +104,7 @@ func TestService(t *testing.T) { t.Run("extendOptions function provided", func(t *testing.T) { f := &fakeHTTPClientProvider{} httpProvider := getMockPromTestSDKProvider(f) - service := NewService(httpProvider, backend.NewLoggerWith("logger", "test"), func(ctx context.Context, settings backend.DataSourceInstanceSettings, clientOpts *sdkhttpclient.Options) error { + service := NewService(httpProvider, backend.NewLoggerWith("logger", "test"), func(ctx context.Context, settings backend.DataSourceInstanceSettings, clientOpts *sdkhttpclient.Options, log log.Logger) error { fmt.Println(ctx, settings, clientOpts) require.NotNil(t, ctx) require.NotNil(t, settings) diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index b8f3bbb646d..8e8a018c474 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1190,6 +1190,13 @@ var ( HideFromDocs: true, HideFromAdminPage: true, }, + { + Name: "prometheusAzureOverrideAudience", + Description: "Deprecated. Allow override default AAD audience for Azure Prometheus endpoint. Enabled by default. This feature should no longer be used and will be removed in the future.", + Stage: FeatureStageDeprecated, + Owner: grafanaPartnerPluginsSquad, + Expression: "true", // Enabled by default for now + }, } ) diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 339cd7dc90f..3f4a697bc51 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -158,3 +158,4 @@ scopeFilters,experimental,@grafana/dashboards-squad,false,false,false ssoSettingsSAML,experimental,@grafana/identity-access-team,false,false,false usePrometheusFrontendPackage,experimental,@grafana/observability-metrics,false,false,true oauthRequireSubClaim,experimental,@grafana/identity-access-team,false,false,false +prometheusAzureOverrideAudience,deprecated,@grafana/partner-datasources,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 4e8adf7f9db..7ce7fd6db68 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -642,4 +642,8 @@ const ( // FlagOauthRequireSubClaim // Require that sub claims is present in oauth tokens. FlagOauthRequireSubClaim = "oauthRequireSubClaim" + + // FlagPrometheusAzureOverrideAudience + // Deprecated. Allow override default AAD audience for Azure Prometheus endpoint. Enabled by default. This feature should no longer be used and will be removed in the future. + FlagPrometheusAzureOverrideAudience = "prometheusAzureOverrideAudience" ) diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 2f132630d10..f95bc3c571e 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -2052,6 +2052,18 @@ "hideFromAdminPage": true, "hideFromDocs": true } + }, + { + "metadata": { + "name": "prometheusAzureOverrideAudience", + "resourceVersion": "1721244365188", + "creationTimestamp": "2024-07-17T19:26:05Z" + }, + "spec": { + "description": "Deprecated. Allow override default AAD audience for Azure Prometheus endpoint. Enabled by default. This feature should no longer be used and will be removed in the future.", + "stage": "deprecated", + "codeowner": "@grafana/partner-datasources" + } } ] } \ No newline at end of file diff --git a/pkg/tsdb/prometheus/azureauth/azure.go b/pkg/tsdb/prometheus/azureauth/azure.go index 7bdedf5e448..d9259a9e971 100644 --- a/pkg/tsdb/prometheus/azureauth/azure.go +++ b/pkg/tsdb/prometheus/azureauth/azure.go @@ -10,12 +10,13 @@ import ( "github.com/grafana/grafana-azure-sdk-go/v2/azsettings" "github.com/grafana/grafana-plugin-sdk-go/backend" sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" + "github.com/grafana/grafana-plugin-sdk-go/backend/log" "github.com/grafana/grafana-plugin-sdk-go/data/utils/maputil" "github.com/grafana/grafana/pkg/promlib/utils" ) -func ConfigureAzureAuthentication(settings backend.DataSourceInstanceSettings, azureSettings *azsettings.AzureSettings, clientOpts *sdkhttpclient.Options) error { +func ConfigureAzureAuthentication(settings backend.DataSourceInstanceSettings, azureSettings *azsettings.AzureSettings, clientOpts *sdkhttpclient.Options, audienceOverride bool, log log.Logger) error { jsonData, err := utils.GetJsonData(settings) if err != nil { return fmt.Errorf("failed to get jsonData: %w", err) @@ -29,7 +30,7 @@ func ConfigureAzureAuthentication(settings backend.DataSourceInstanceSettings, a if credentials != nil { var scopes []string - if scopes, err = getOverriddenScopes(jsonData); err != nil { + if scopes, err = getOverriddenScopes(jsonData, audienceOverride, log); err != nil { return err } @@ -47,7 +48,7 @@ func ConfigureAzureAuthentication(settings backend.DataSourceInstanceSettings, a return nil } -func getOverriddenScopes(jsonData map[string]any) ([]string, error) { +func getOverriddenScopes(jsonData map[string]any, audienceOverride bool, log log.Logger) ([]string, error) { resourceIdStr, err := maputil.GetStringOptional(jsonData, "azureEndpointResourceId") if err != nil { err = fmt.Errorf("overridden resource ID (audience) invalid") @@ -56,6 +57,11 @@ func getOverriddenScopes(jsonData map[string]any) ([]string, error) { return nil, nil } + if !audienceOverride { + log.Warn("Specifying an audience override requires the prometheusAzureOverrideAudience feature toggle to be enabled. This functionality is deprecated and will be removed in a future release.") + return nil, nil + } + resourceId, err := url.Parse(resourceIdStr) if err != nil || resourceId.Scheme == "" || resourceId.Host == "" { err = fmt.Errorf("overridden endpoint resource ID (audience) '%s' invalid", resourceIdStr) diff --git a/pkg/tsdb/prometheus/azureauth/azure_test.go b/pkg/tsdb/prometheus/azureauth/azure_test.go index 7de7de193cb..a8d66667b47 100644 --- a/pkg/tsdb/prometheus/azureauth/azure_test.go +++ b/pkg/tsdb/prometheus/azureauth/azure_test.go @@ -1,18 +1,39 @@ package azureauth import ( + "bytes" + "context" "testing" "github.com/grafana/grafana-azure-sdk-go/v2/azcredentials" "github.com/grafana/grafana-azure-sdk-go/v2/azsettings" "github.com/grafana/grafana-plugin-sdk-go/backend" sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" + "github.com/grafana/grafana-plugin-sdk-go/backend/log" + "github.com/hashicorp/go-hclog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +type fakeLogger struct { + hclog.Logger + + level log.Level +} + +func (l fakeLogger) Level() log.Level { + return l.level +} +func (l fakeLogger) FromContext(ctx context.Context) log.Logger { + return fakeLogger{} +} +func (l fakeLogger) With(args ...interface{}) log.Logger { + return fakeLogger{} +} + func TestConfigureAzureAuthentication(t *testing.T) { azureSettings := &azsettings.AzureSettings{} + testLogger := backend.Logger t.Run("should set Azure middleware when JsonData contains valid credentials", func(t *testing.T) { settings := backend.DataSourceInstanceSettings{ @@ -26,7 +47,7 @@ func TestConfigureAzureAuthentication(t *testing.T) { var opts = &sdkhttpclient.Options{CustomOptions: map[string]any{}} - err := ConfigureAzureAuthentication(settings, azureSettings, opts) + err := ConfigureAzureAuthentication(settings, azureSettings, opts, false, testLogger) require.NoError(t, err) require.NotNil(t, opts.Middlewares) @@ -40,7 +61,7 @@ func TestConfigureAzureAuthentication(t *testing.T) { var opts = &sdkhttpclient.Options{CustomOptions: map[string]any{}} - err := ConfigureAzureAuthentication(settings, azureSettings, opts) + err := ConfigureAzureAuthentication(settings, azureSettings, opts, false, testLogger) require.NoError(t, err) assert.NotContains(t, opts.CustomOptions, "_azureCredentials") @@ -55,7 +76,7 @@ func TestConfigureAzureAuthentication(t *testing.T) { } var opts = &sdkhttpclient.Options{CustomOptions: map[string]any{}} - err := ConfigureAzureAuthentication(settings, azureSettings, opts) + err := ConfigureAzureAuthentication(settings, azureSettings, opts, false, testLogger) assert.Error(t, err) }) @@ -71,7 +92,7 @@ func TestConfigureAzureAuthentication(t *testing.T) { } var opts = &sdkhttpclient.Options{CustomOptions: map[string]any{}} - err := ConfigureAzureAuthentication(settings, azureSettings, opts) + err := ConfigureAzureAuthentication(settings, azureSettings, opts, true, testLogger) require.NoError(t, err) require.NotNil(t, opts.Middlewares) @@ -87,7 +108,7 @@ func TestConfigureAzureAuthentication(t *testing.T) { } var opts = &sdkhttpclient.Options{CustomOptions: map[string]any{}} - err := ConfigureAzureAuthentication(settings, azureSettings, opts) + err := ConfigureAzureAuthentication(settings, azureSettings, opts, true, testLogger) require.NoError(t, err) if opts.Middlewares != nil { @@ -108,9 +129,36 @@ func TestConfigureAzureAuthentication(t *testing.T) { var opts = &sdkhttpclient.Options{CustomOptions: map[string]any{}} - err := ConfigureAzureAuthentication(settings, azureSettings, opts) + err := ConfigureAzureAuthentication(settings, azureSettings, opts, true, testLogger) assert.Error(t, err) }) + t.Run("should warn if an audience is specified and the feature toggle is not enabled", func(t *testing.T) { + settings := backend.DataSourceInstanceSettings{ + JSONData: []byte(`{ + "httpMethod": "POST", + "azureCredentials": { + "authType": "msi" + }, + "azureEndpointResourceId": "https://api.example.com/abd5c4ce-ca73-41e9-9cb2-bed39aa2adb5" + }`), + } + + var opts = &sdkhttpclient.Options{CustomOptions: map[string]any{}} + var buf bytes.Buffer + testLogger := hclog.New(&hclog.LoggerOptions{ + Name: "test", + Output: &buf, + }) + log := fakeLogger{ + Logger: testLogger, + } + + err := ConfigureAzureAuthentication(settings, azureSettings, opts, false, log) + str := buf.String() + t.Log(str) + assert.NoError(t, err) + assert.Contains(t, str, "Specifying an audience override requires the prometheusAzureOverrideAudience feature toggle to be enabled. This functionality is deprecated and will be removed in a future release.") + }) } func TestGetPrometheusScopes(t *testing.T) { diff --git a/pkg/tsdb/prometheus/prometheus.go b/pkg/tsdb/prometheus/prometheus.go index ef4e1df3105..423c3a1f132 100644 --- a/pkg/tsdb/prometheus/prometheus.go +++ b/pkg/tsdb/prometheus/prometheus.go @@ -7,6 +7,7 @@ import ( "github.com/grafana/grafana-azure-sdk-go/v2/azsettings" "github.com/grafana/grafana-plugin-sdk-go/backend" sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" + "github.com/grafana/grafana-plugin-sdk-go/backend/log" "github.com/grafana/grafana/pkg/promlib" "github.com/grafana/grafana/pkg/tsdb/prometheus/azureauth" @@ -45,7 +46,7 @@ func (s *Service) CheckHealth(ctx context.Context, req *backend.CheckHealthReque return s.lib.CheckHealth(ctx, req) } -func extendClientOpts(ctx context.Context, settings backend.DataSourceInstanceSettings, clientOpts *sdkhttpclient.Options) error { +func extendClientOpts(ctx context.Context, settings backend.DataSourceInstanceSettings, clientOpts *sdkhttpclient.Options, plog log.Logger) error { // Set SigV4 service namespace if clientOpts.SigV4 != nil { clientOpts.SigV4.Service = "aps" @@ -56,9 +57,11 @@ func extendClientOpts(ctx context.Context, settings backend.DataSourceInstanceSe return fmt.Errorf("failed to read Azure settings from Grafana: %v", err) } + audienceOverride := backend.GrafanaConfigFromContext(ctx).FeatureToggles().IsEnabled("prometheusAzureOverrideAudience") + // Set Azure authentication if azureSettings.AzureAuthEnabled { - err = azureauth.ConfigureAzureAuthentication(settings, azureSettings, clientOpts) + err = azureauth.ConfigureAzureAuthentication(settings, azureSettings, clientOpts, audienceOverride, plog) if err != nil { return fmt.Errorf("error configuring Azure auth: %v", err) } diff --git a/pkg/tsdb/prometheus/prometheus_test.go b/pkg/tsdb/prometheus/prometheus_test.go index f778956115e..7bf2db10fec 100644 --- a/pkg/tsdb/prometheus/prometheus_test.go +++ b/pkg/tsdb/prometheus/prometheus_test.go @@ -28,7 +28,7 @@ func TestExtendClientOpts(t *testing.T) { } ctx := backend.WithGrafanaConfig(context.Background(), cfg) opts := &sdkhttpclient.Options{} - err := extendClientOpts(ctx, settings, opts) + err := extendClientOpts(ctx, settings, opts, backend.Logger) require.NoError(t, err) require.Equal(t, 1, len(opts.Middlewares)) }) @@ -47,7 +47,7 @@ func TestExtendClientOpts(t *testing.T) { SecretKey: "secretkey", }, } - err := extendClientOpts(context.Background(), settings, opts) + err := extendClientOpts(context.Background(), settings, opts, backend.Logger) require.NoError(t, err) require.Equal(t, "aps", opts.SigV4.Service) }) diff --git a/public/app/plugins/datasource/prometheus/configuration/AzureAuthSettings.tsx b/public/app/plugins/datasource/prometheus/configuration/AzureAuthSettings.tsx index 7327aa6996a..6ac2ae7e1dd 100644 --- a/public/app/plugins/datasource/prometheus/configuration/AzureAuthSettings.tsx +++ b/public/app/plugins/datasource/prometheus/configuration/AzureAuthSettings.tsx @@ -13,6 +13,7 @@ import { AzureCredentialsForm } from './AzureCredentialsForm'; export const AzureAuthSettings = (props: HttpSettingsBaseProps) => { const { dataSourceConfig, onChange } = props; + const [overrideAudienceAllowed] = useState(!!config.featureToggles.prometheusAzureOverrideAudience); const [overrideAudienceChecked, setOverrideAudienceChecked] = useState( !!dataSourceConfig.jsonData.azureEndpointResourceId ); @@ -64,25 +65,29 @@ export const AzureAuthSettings = (props: HttpSettingsBaseProps) => { onCredentialsChange={onCredentialsChange} disabled={dataSourceConfig.readOnly} /> -
Azure configuration
-
- - - - - - {overrideAudienceChecked && ( - - - - - - )} -
+ {overrideAudienceAllowed && ( + <> +
Azure configuration
+
+ + + + + + {overrideAudienceChecked && ( + + + + + + )} +
+ + )} ); };