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
This commit is contained in:
Andreas Christou 2024-07-17 21:09:55 +07:00 committed by GitHub
parent 7839903fef
commit 2616366a0a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 131 additions and 37 deletions

View File

@ -199,4 +199,5 @@ export interface FeatureToggles {
cloudWatchRoundUpEndTime?: boolean;
bodyScrolling?: boolean;
cloudwatchMetricInsightsCrossAccount?: boolean;
prometheusAzureOverrideAudience?: boolean;
}

View File

@ -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
}

View File

@ -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)
}

View File

@ -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)

View File

@ -1366,6 +1366,13 @@ var (
Owner: awsDatasourcesSquad,
FrontendOnly: 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
},
}
)

View File

@ -180,3 +180,4 @@ dashboardRestoreUI,experimental,@grafana/grafana-frontend-platform,false,false,f
cloudWatchRoundUpEndTime,GA,@grafana/aws-datasources,false,false,false
bodyScrolling,experimental,@grafana/grafana-frontend-platform,false,false,true
cloudwatchMetricInsightsCrossAccount,experimental,@grafana/aws-datasources,false,false,true
prometheusAzureOverrideAudience,deprecated,@grafana/partner-datasources,false,false,false

1 Name Stage Owner requiresDevMode RequiresRestart FrontendOnly
180 cloudWatchRoundUpEndTime GA @grafana/aws-datasources false false false
181 bodyScrolling experimental @grafana/grafana-frontend-platform false false true
182 cloudwatchMetricInsightsCrossAccount experimental @grafana/aws-datasources false false true
183 prometheusAzureOverrideAudience deprecated @grafana/partner-datasources false false false

View File

@ -730,4 +730,8 @@ const (
// FlagCloudwatchMetricInsightsCrossAccount
// Enables cross account observability for Cloudwatch Metric Insights
FlagCloudwatchMetricInsightsCrossAccount = "cloudwatchMetricInsightsCrossAccount"
// 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"
)

View File

@ -2019,6 +2019,23 @@
"codeowner": "@grafana/observability-metrics"
}
},
{
"metadata": {
"name": "prometheusAzureOverrideAudience",
"resourceVersion": "1721046541163",
"creationTimestamp": "2022-05-30T15:43:32Z",
"deletionTimestamp": "2023-07-16T21:30:14Z",
"annotations": {
"grafana.app/updatedTimestamp": "2024-07-15 12:29:01.163772 +0000 UTC"
}
},
"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",
"expression": "true"
}
},
{
"metadata": {
"name": "prometheusCodeModeMetricNamesSearch",

View File

@ -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)

View File

@ -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) {

View File

@ -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"
@ -56,7 +57,7 @@ func (s *Service) ConvertObject(ctx context.Context, req *backend.ConversionRequ
return s.lib.ConvertObject(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"
@ -67,9 +68,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)
}

View File

@ -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)
})

View File

@ -13,6 +13,7 @@ import { AzureCredentialsForm } from './AzureCredentialsForm';
export const AzureAuthSettings = (props: HttpSettingsBaseProps) => {
const { dataSourceConfig, onChange } = props;
const [overrideAudienceAllowed] = useState<boolean>(!!config.featureToggles.prometheusAzureOverrideAudience);
const [overrideAudienceChecked, setOverrideAudienceChecked] = useState<boolean>(
!!dataSourceConfig.jsonData.azureEndpointResourceId
);
@ -64,25 +65,29 @@ export const AzureAuthSettings = (props: HttpSettingsBaseProps) => {
onCredentialsChange={onCredentialsChange}
disabled={dataSourceConfig.readOnly}
/>
<h6>Azure configuration</h6>
<div className="gf-form-group">
<InlineFieldRow>
<InlineField labelWidth={labelWidth} label="Override AAD audience" disabled={dataSourceConfig.readOnly}>
<InlineSwitch value={overrideAudienceChecked} onChange={onOverrideAudienceChange} />
</InlineField>
</InlineFieldRow>
{overrideAudienceChecked && (
<InlineFieldRow>
<InlineField labelWidth={labelWidth} label="Resource ID" disabled={dataSourceConfig.readOnly}>
<Input
className={cx(prometheusConfigOverhaulAuth ? 'width-20' : 'width-30')}
value={dataSourceConfig.jsonData.azureEndpointResourceId || ''}
onChange={onResourceIdChange}
/>
</InlineField>
</InlineFieldRow>
)}
</div>
{overrideAudienceAllowed && (
<>
<h6>Azure configuration</h6>
<div className="gf-form-group">
<InlineFieldRow>
<InlineField labelWidth={labelWidth} label="Override AAD audience" disabled={dataSourceConfig.readOnly}>
<InlineSwitch value={overrideAudienceChecked} onChange={onOverrideAudienceChange} />
</InlineField>
</InlineFieldRow>
{overrideAudienceChecked && (
<InlineFieldRow>
<InlineField labelWidth={labelWidth} label="Resource ID" disabled={dataSourceConfig.readOnly}>
<Input
className={cx(prometheusConfigOverhaulAuth ? 'width-20' : 'width-30')}
value={dataSourceConfig.jsonData.azureEndpointResourceId || ''}
onChange={onResourceIdChange}
/>
</InlineField>
</InlineFieldRow>
)}
</div>
</>
)}
</>
);
};