From c6c4a6b9a7ceb6e407072eb5f244c96701788ce8 Mon Sep 17 00:00:00 2001 From: swiencki Date: Tue, 28 Apr 2026 10:55:19 -0700 Subject: [PATCH 1/5] grafanactl: reconcile ADX datasources --- tools/grafanactl/README.md | 33 ++- tools/grafanactl/cmd/modify/adx.go | 124 ++++++++++ tools/grafanactl/cmd/modify/adx_options.go | 153 ++++++++++++ .../grafanactl/cmd/modify/adx_options_test.go | 234 ++++++++++++++++++ tools/grafanactl/cmd/modify/cmd.go | 21 +- tools/grafanactl/internal/grafana/client.go | 104 +++++++- .../internal/grafana/client_test.go | 92 +++++++ 7 files changed, 752 insertions(+), 9 deletions(-) create mode 100644 tools/grafanactl/cmd/modify/adx.go create mode 100644 tools/grafanactl/cmd/modify/adx_options.go create mode 100644 tools/grafanactl/cmd/modify/adx_options_test.go create mode 100644 tools/grafanactl/internal/grafana/client_test.go diff --git a/tools/grafanactl/README.md b/tools/grafanactl/README.md index 66dd78f9..1c0ff38f 100644 --- a/tools/grafanactl/README.md +++ b/tools/grafanactl/README.md @@ -8,6 +8,7 @@ grafanactl helps maintain Azure Managed Grafana instances by providing tools to: - List all datasources in a Grafana instance - Remove orphaned Azure Monitor Workspace integrations - Clean up stale datasources pointing to deleted resources +- Reconcile Azure Data Explorer datasources - Sync dashboards and folders from git to Grafana This tool is particularly useful when Azure Monitor Workspaces (Prometheus instances) are removed from your infrastructure but their references remain in Grafana, creating stale integrations. @@ -39,6 +40,7 @@ All commands require these basic parameters: - `--subscription` - Azure subscription ID - `--resource-group` - Azure resource group name - `--grafana-name` - Azure Managed Grafana instance name +- `--grafana-resource-id` - Azure Managed Grafana resource ID, as an alternative to subscription/resource group/name - `--output` - Output format: `table` (default) or `json` - `-v, --verbosity` - Set logging verbosity level (0-10) @@ -110,6 +112,36 @@ grafanactl clean fixup-datasources \ --grafana-name "your-grafana-instance" ``` +### Modify Commands + +Modify commands reconcile resources in Azure Managed Grafana. + +#### Reconcile Azure Data Explorer Datasource + +Create or update one Azure Data Explorer datasource using Grafana's REST API: + +```bash +# Preview changes (dry-run) +grafanactl modify datasource reconcile-adx \ + --grafana-resource-id "/subscriptions//resourceGroups//providers/Microsoft.Dashboard/grafana/" \ + --cluster-url "https://example.region.kusto.windows.net" \ + --default-database "ServiceLogs" \ + --datasource-name "kusto-int-uksouth" \ + --dry-run + +# Apply changes +grafanactl modify datasource reconcile-adx \ + --grafana-resource-id "/subscriptions//resourceGroups//providers/Microsoft.Dashboard/grafana/" \ + --cluster-url "https://example.region.kusto.windows.net" \ + --default-database "ServiceLogs" \ + --datasource-name "kusto-int-uksouth" +``` + +The command requires the Azure Data Explorer datasource plugin +(`grafana-azure-data-explorer-datasource`) to be available in Grafana. It uses +the Grafana managed identity for ADX authentication and fails if an existing +datasource with the requested name has a different plugin type. + ### Sync Commands Sync commands help keep your Grafana instance in sync with dashboard definitions stored in git. @@ -148,4 +180,3 @@ The config file (e.g., `observability.yaml`) defines: - The tool includes retry logic for transient Azure API failures - Use `--verbosity` flag to increase logging detail for troubleshooting - Always use `--dry-run` first to preview changes before applying them - diff --git a/tools/grafanactl/cmd/modify/adx.go b/tools/grafanactl/cmd/modify/adx.go new file mode 100644 index 00000000..a1c703f6 --- /dev/null +++ b/tools/grafanactl/cmd/modify/adx.go @@ -0,0 +1,124 @@ +// Copyright 2025 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package modify + +import ( + "context" + "fmt" + + "github.com/go-logr/logr" + "github.com/grafana-tools/sdk" +) + +const adxDatasourceType = "grafana-azure-data-explorer-datasource" + +func (opts *RawReconcileADXDatasourceOptions) Run(ctx context.Context) error { + validated, err := opts.Validate(ctx) + if err != nil { + return fmt.Errorf("validation failed: %w", err) + } + + completed, err := validated.Complete(ctx) + if err != nil { + return fmt.Errorf("completion failed: %w", err) + } + + return completed.Run(ctx) +} + +func (o *CompletedReconcileADXDatasourceOptions) Run(ctx context.Context) error { + logger := logr.FromContextOrDiscard(ctx).WithValues( + "resource-group", o.ResourceGroup, + "grafana-name", o.GrafanaName, + "datasource-name", o.DatasourceName, + ) + + logger.Info("reconcile ADX datasource command executed", "dry-run", o.DryRun) + + dataSourceTypes, err := o.GrafanaClient.ListDataSourceTypes(ctx) + if err != nil { + return fmt.Errorf("failed to list Grafana datasource plugins; verify the caller has Grafana Admin permissions: %w", err) + } + if _, ok := dataSourceTypes[adxDatasourceType]; !ok { + return fmt.Errorf("Grafana datasource plugin %q is not available", adxDatasourceType) + } + + dataSources, err := o.GrafanaClient.ListDataSources(ctx) + if err != nil { + return fmt.Errorf("failed to list Grafana datasources; verify the caller has Grafana Admin permissions: %w", err) + } + + var existing *sdk.Datasource + for i := range dataSources { + if dataSources[i].Name != o.DatasourceName { + continue + } + if existing != nil { + return fmt.Errorf("found multiple datasources named %q", o.DatasourceName) + } + existing = &dataSources[i] + } + + desired := o.desiredDatasource() + if existing == nil { + if o.DryRun { + logger.Info("Dry run - would create ADX datasource") + return nil + } + + logger.Info("Creating ADX datasource") + if err := o.GrafanaClient.CreateDataSource(ctx, desired); err != nil { + return fmt.Errorf("failed to create ADX datasource %q: %w", o.DatasourceName, err) + } + return nil + } + + if existing.Type != adxDatasourceType { + return fmt.Errorf("datasource %q already exists with type %q, expected %q", o.DatasourceName, existing.Type, adxDatasourceType) + } + + desired.ID = existing.ID + desired.UID = existing.UID + desired.OrgID = existing.OrgID + desired.IsDefault = existing.IsDefault + + if o.DryRun { + logger.Info("Dry run - would update ADX datasource", "datasource-id", existing.ID, "datasource-uid", existing.UID) + return nil + } + + logger.Info("Updating ADX datasource", "datasource-id", existing.ID, "datasource-uid", existing.UID) + if err := o.GrafanaClient.UpdateDataSource(ctx, desired); err != nil { + return fmt.Errorf("failed to update ADX datasource %q: %w", o.DatasourceName, err) + } + + return nil +} + +func (o *CompletedReconcileADXDatasourceOptions) desiredDatasource() sdk.Datasource { + return sdk.Datasource{ + Name: o.DatasourceName, + Type: adxDatasourceType, + Access: "proxy", + JSONData: map[string]interface{}{ + "clusterUrl": o.ClusterURL, + "defaultDatabase": o.DefaultDatabase, + "dataConsistency": o.DataConsistency, + "azureCredentials": map[string]interface{}{ + "authType": "msi", + }, + }, + } +} diff --git a/tools/grafanactl/cmd/modify/adx_options.go b/tools/grafanactl/cmd/modify/adx_options.go new file mode 100644 index 00000000..805c38c0 --- /dev/null +++ b/tools/grafanactl/cmd/modify/adx_options.go @@ -0,0 +1,153 @@ +// Copyright 2025 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package modify + +import ( + "context" + "fmt" + "net/url" + "strings" + + "github.com/grafana-tools/sdk" + "github.com/spf13/cobra" + + "github.com/Azure/ARO-Tools/tools/cmdutils" + "github.com/Azure/ARO-Tools/tools/grafanactl/cmd/base" + "github.com/Azure/ARO-Tools/tools/grafanactl/internal/azure" + "github.com/Azure/ARO-Tools/tools/grafanactl/internal/grafana" +) + +const defaultADXDataConsistency = "strongconsistency" + +type adxGrafanaClient interface { + ListDataSources(ctx context.Context) ([]sdk.Datasource, error) + ListDataSourceTypes(ctx context.Context) (map[string]sdk.DatasourceType, error) + CreateDataSource(ctx context.Context, dataSource sdk.Datasource) error + UpdateDataSource(ctx context.Context, dataSource sdk.Datasource) error +} + +// RawReconcileADXDatasourceOptions represents the initial, unvalidated configuration for ADX datasource reconcile operations. +type RawReconcileADXDatasourceOptions struct { + *base.BaseOptions + ClusterURL string + DefaultDatabase string + DatasourceName string + DataConsistency string +} + +type validatedReconcileADXDatasourceOptions struct { + *RawReconcileADXDatasourceOptions +} + +// ValidatedReconcileADXDatasourceOptions represents ADX datasource reconcile configuration that has passed validation. +type ValidatedReconcileADXDatasourceOptions struct { + *validatedReconcileADXDatasourceOptions +} + +// CompletedReconcileADXDatasourceOptions represents fully initialized ADX datasource reconcile configuration. +type CompletedReconcileADXDatasourceOptions struct { + *validatedReconcileADXDatasourceOptions + GrafanaClient adxGrafanaClient +} + +// DefaultReconcileADXDatasourceOptions returns a new RawReconcileADXDatasourceOptions with default values. +func DefaultReconcileADXDatasourceOptions() *RawReconcileADXDatasourceOptions { + return &RawReconcileADXDatasourceOptions{ + BaseOptions: base.DefaultBaseOptions(), + DataConsistency: defaultADXDataConsistency, + } +} + +// BindReconcileADXDatasourceOptions binds command-line flags to the options. +func BindReconcileADXDatasourceOptions(opts *RawReconcileADXDatasourceOptions, cmd *cobra.Command) error { + if err := base.BindBaseOptions(opts.BaseOptions, cmd); err != nil { + return err + } + + flags := cmd.Flags() + flags.StringVar(&opts.ClusterURL, "cluster-url", opts.ClusterURL, "Azure Data Explorer cluster URL") + flags.StringVar(&opts.DefaultDatabase, "default-database", opts.DefaultDatabase, "Default Azure Data Explorer database") + flags.StringVar(&opts.DatasourceName, "datasource-name", opts.DatasourceName, "Grafana datasource name") + flags.StringVar(&opts.DataConsistency, "data-consistency", opts.DataConsistency, "Azure Data Explorer datasource data consistency") + + return nil +} + +// Validate performs validation on the raw options. +func (o *RawReconcileADXDatasourceOptions) Validate(ctx context.Context) (*ValidatedReconcileADXDatasourceOptions, error) { + if err := base.ValidateBaseOptions(o.BaseOptions); err != nil { + return nil, err + } + + clusterURL := strings.TrimSpace(o.ClusterURL) + defaultDatabase := strings.TrimSpace(o.DefaultDatabase) + datasourceName := strings.TrimSpace(o.DatasourceName) + dataConsistency := strings.TrimSpace(o.DataConsistency) + if dataConsistency == "" { + dataConsistency = defaultADXDataConsistency + } + + if clusterURL == "" { + return nil, fmt.Errorf("cluster URL is required") + } + parsedClusterURL, err := url.Parse(clusterURL) + if err != nil { + return nil, fmt.Errorf("invalid cluster URL: %w", err) + } + if parsedClusterURL.Scheme != "https" || parsedClusterURL.Host == "" { + return nil, fmt.Errorf("cluster URL must be an absolute https URL") + } + if defaultDatabase == "" { + return nil, fmt.Errorf("default database is required") + } + if datasourceName == "" { + return nil, fmt.Errorf("datasource name is required") + } + + return &ValidatedReconcileADXDatasourceOptions{ + validatedReconcileADXDatasourceOptions: &validatedReconcileADXDatasourceOptions{ + RawReconcileADXDatasourceOptions: &RawReconcileADXDatasourceOptions{ + BaseOptions: o.BaseOptions, + ClusterURL: clusterURL, + DefaultDatabase: defaultDatabase, + DatasourceName: datasourceName, + DataConsistency: dataConsistency, + }, + }, + }, nil +} + +// Complete performs final initialization to create fully usable ADX datasource reconcile options. +func (o *ValidatedReconcileADXDatasourceOptions) Complete(ctx context.Context) (*CompletedReconcileADXDatasourceOptions, error) { + cred, err := cmdutils.GetAzureTokenCredentials() + if err != nil { + return nil, fmt.Errorf("failed to obtain Azure credentials: %w", err) + } + + managedGrafanaClient, err := azure.NewManagedGrafanaClient(o.SubscriptionID, cred) + if err != nil { + return nil, fmt.Errorf("failed to create managed Grafana client: %w", err) + } + + grafanaClient, err := grafana.NewClient(ctx, cred, managedGrafanaClient, o.SubscriptionID, o.ResourceGroup, o.GrafanaName) + if err != nil { + return nil, fmt.Errorf("failed to create Grafana client: %w", err) + } + + return &CompletedReconcileADXDatasourceOptions{ + validatedReconcileADXDatasourceOptions: o.validatedReconcileADXDatasourceOptions, + GrafanaClient: grafanaClient, + }, nil +} diff --git a/tools/grafanactl/cmd/modify/adx_options_test.go b/tools/grafanactl/cmd/modify/adx_options_test.go new file mode 100644 index 00000000..f58d34ac --- /dev/null +++ b/tools/grafanactl/cmd/modify/adx_options_test.go @@ -0,0 +1,234 @@ +// Copyright 2025 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package modify + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/grafana-tools/sdk" + + "github.com/Azure/ARO-Tools/tools/grafanactl/cmd/base" +) + +type fakeADXGrafanaClient struct { + dataSources []sdk.Datasource + dataSourceTypes map[string]sdk.DatasourceType + createCalls []sdk.Datasource + updateCalls []sdk.Datasource + listTypesErr error + listErr error + createErr error + updateErr error +} + +func (f *fakeADXGrafanaClient) ListDataSources(ctx context.Context) ([]sdk.Datasource, error) { + return f.dataSources, f.listErr +} + +func (f *fakeADXGrafanaClient) ListDataSourceTypes(ctx context.Context) (map[string]sdk.DatasourceType, error) { + return f.dataSourceTypes, f.listTypesErr +} + +func (f *fakeADXGrafanaClient) CreateDataSource(ctx context.Context, dataSource sdk.Datasource) error { + f.createCalls = append(f.createCalls, dataSource) + return f.createErr +} + +func (f *fakeADXGrafanaClient) UpdateDataSource(ctx context.Context, dataSource sdk.Datasource) error { + f.updateCalls = append(f.updateCalls, dataSource) + return f.updateErr +} + +func validRawReconcileADXDatasourceOptions() *RawReconcileADXDatasourceOptions { + baseOptions := base.DefaultBaseOptions() + baseOptions.SubscriptionID = "subscription-id" + baseOptions.ResourceGroup = "resource-group" + baseOptions.GrafanaName = "grafana-name" + + return &RawReconcileADXDatasourceOptions{ + BaseOptions: baseOptions, + ClusterURL: "https://example.kusto.windows.net", + DefaultDatabase: "ServiceLogs", + DatasourceName: "kusto-int-uksouth", + } +} + +func completedReconcileADXDatasourceOptionsForTest(client adxGrafanaClient) *CompletedReconcileADXDatasourceOptions { + raw := validRawReconcileADXDatasourceOptions() + raw.DataConsistency = defaultADXDataConsistency + return &CompletedReconcileADXDatasourceOptions{ + validatedReconcileADXDatasourceOptions: &validatedReconcileADXDatasourceOptions{ + RawReconcileADXDatasourceOptions: raw, + }, + GrafanaClient: client, + } +} + +func TestReconcileADXDatasourceValidateDefaultsDataConsistency(t *testing.T) { + validated, err := validRawReconcileADXDatasourceOptions().Validate(context.Background()) + if err != nil { + t.Fatalf("Validate returned error: %v", err) + } + if validated.DataConsistency != defaultADXDataConsistency { + t.Fatalf("expected default data consistency %q, got %q", defaultADXDataConsistency, validated.DataConsistency) + } +} + +func TestReconcileADXDatasourceValidateRejectsInvalidClusterURL(t *testing.T) { + opts := validRawReconcileADXDatasourceOptions() + opts.ClusterURL = "http://example.kusto.windows.net" + + _, err := opts.Validate(context.Background()) + if err == nil { + t.Fatal("expected validation error") + } + if !strings.Contains(err.Error(), "absolute https URL") { + t.Fatalf("expected https URL error, got %v", err) + } +} + +func TestReconcileADXDatasourceRunCreatesDatasource(t *testing.T) { + client := &fakeADXGrafanaClient{ + dataSourceTypes: map[string]sdk.DatasourceType{ + adxDatasourceType: {}, + }, + } + opts := completedReconcileADXDatasourceOptionsForTest(client) + + if err := opts.Run(context.Background()); err != nil { + t.Fatalf("Run returned error: %v", err) + } + if len(client.createCalls) != 1 { + t.Fatalf("expected one create call, got %d", len(client.createCalls)) + } + if len(client.updateCalls) != 0 { + t.Fatalf("expected no update calls, got %d", len(client.updateCalls)) + } + + created := client.createCalls[0] + if created.Name != opts.DatasourceName { + t.Fatalf("expected datasource name %q, got %q", opts.DatasourceName, created.Name) + } + if created.Type != adxDatasourceType { + t.Fatalf("expected datasource type %q, got %q", adxDatasourceType, created.Type) + } + jsonData, ok := created.JSONData.(map[string]interface{}) + if !ok { + t.Fatalf("expected JSONData map, got %T", created.JSONData) + } + if jsonData["clusterUrl"] != opts.ClusterURL { + t.Fatalf("expected clusterUrl %q, got %v", opts.ClusterURL, jsonData["clusterUrl"]) + } + azureCredentials, ok := jsonData["azureCredentials"].(map[string]interface{}) + if !ok { + t.Fatalf("expected azureCredentials map, got %T", jsonData["azureCredentials"]) + } + if azureCredentials["authType"] != "msi" { + t.Fatalf("expected MSI auth type, got %v", azureCredentials["authType"]) + } +} + +func TestReconcileADXDatasourceRunUpdatesExistingDatasource(t *testing.T) { + client := &fakeADXGrafanaClient{ + dataSourceTypes: map[string]sdk.DatasourceType{ + adxDatasourceType: {}, + }, + dataSources: []sdk.Datasource{ + { + ID: 42, + OrgID: 7, + UID: "existing-uid", + Name: "kusto-int-uksouth", + Type: adxDatasourceType, + IsDefault: true, + }, + }, + } + opts := completedReconcileADXDatasourceOptionsForTest(client) + + if err := opts.Run(context.Background()); err != nil { + t.Fatalf("Run returned error: %v", err) + } + if len(client.createCalls) != 0 { + t.Fatalf("expected no create calls, got %d", len(client.createCalls)) + } + if len(client.updateCalls) != 1 { + t.Fatalf("expected one update call, got %d", len(client.updateCalls)) + } + + updated := client.updateCalls[0] + if updated.ID != 42 || updated.OrgID != 7 || updated.UID != "existing-uid" || !updated.IsDefault { + t.Fatalf("expected existing datasource identity/default to be preserved, got %#v", updated) + } +} + +func TestReconcileADXDatasourceRunRejectsExistingDatasourceWithWrongType(t *testing.T) { + client := &fakeADXGrafanaClient{ + dataSourceTypes: map[string]sdk.DatasourceType{ + adxDatasourceType: {}, + }, + dataSources: []sdk.Datasource{ + { + Name: "kusto-int-uksouth", + Type: "prometheus", + }, + }, + } + opts := completedReconcileADXDatasourceOptionsForTest(client) + + err := opts.Run(context.Background()) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "already exists with type") { + t.Fatalf("expected wrong type error, got %v", err) + } +} + +func TestReconcileADXDatasourceRunRejectsMissingPlugin(t *testing.T) { + client := &fakeADXGrafanaClient{ + dataSourceTypes: map[string]sdk.DatasourceType{}, + } + opts := completedReconcileADXDatasourceOptionsForTest(client) + + err := opts.Run(context.Background()) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "is not available") { + t.Fatalf("expected missing plugin error, got %v", err) + } +} + +func TestReconcileADXDatasourceRunPropagatesMutationErrors(t *testing.T) { + client := &fakeADXGrafanaClient{ + dataSourceTypes: map[string]sdk.DatasourceType{ + adxDatasourceType: {}, + }, + createErr: errors.New("status 403"), + } + opts := completedReconcileADXDatasourceOptionsForTest(client) + + err := opts.Run(context.Background()) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "failed to create ADX datasource") { + t.Fatalf("expected create error, got %v", err) + } +} diff --git a/tools/grafanactl/cmd/modify/cmd.go b/tools/grafanactl/cmd/modify/cmd.go index 3583bd36..f155136e 100644 --- a/tools/grafanactl/cmd/modify/cmd.go +++ b/tools/grafanactl/cmd/modify/cmd.go @@ -30,7 +30,8 @@ import ( const datasourceGroupID = "datasource" func NewModifyCommand(group string) (*cobra.Command, error) { - opts := DefaultAddDatasourceOptions() + addDatasourceOpts := DefaultAddDatasourceOptions() + reconcileADXDatasourceOpts := DefaultReconcileADXDatasourceOptions() modifyCmd := &cobra.Command{ Use: "modify", @@ -56,15 +57,29 @@ func NewModifyCommand(group string) (*cobra.Command, error) { Short: "Reconcile Azure Monitor Workspace datasources in Grafana", Long: "Reconcile Azure Monitor Workspace datasources in the Azure Managed Grafana instance. This integrates the workspaces with Grafana and creates the necessary datasource configuration.", RunE: func(cmd *cobra.Command, args []string) error { - return opts.Run(cmd.Context()) + return addDatasourceOpts.Run(cmd.Context()) }, } - if err := BindAddDatasourceOptions(opts, addDatasourceCmd); err != nil { + reconcileADXDatasourceCmd := &cobra.Command{ + Use: "reconcile-adx", + Short: "Reconcile an Azure Data Explorer datasource in Grafana", + Long: "Reconcile one Azure Data Explorer datasource in the Azure Managed Grafana instance using the Grafana API.", + RunE: func(cmd *cobra.Command, args []string) error { + return reconcileADXDatasourceOpts.Run(cmd.Context()) + }, + } + + if err := BindAddDatasourceOptions(addDatasourceOpts, addDatasourceCmd); err != nil { + return nil, err + } + + if err := BindReconcileADXDatasourceOptions(reconcileADXDatasourceOpts, reconcileADXDatasourceCmd); err != nil { return nil, err } datasourceCmd.AddCommand(addDatasourceCmd) + datasourceCmd.AddCommand(reconcileADXDatasourceCmd) modifyCmd.AddCommand(datasourceCmd) return modifyCmd, nil diff --git a/tools/grafanactl/internal/grafana/client.go b/tools/grafanactl/internal/grafana/client.go index 417d23f5..d933c8ab 100644 --- a/tools/grafanactl/internal/grafana/client.go +++ b/tools/grafanactl/internal/grafana/client.go @@ -15,8 +15,15 @@ package grafana import ( + "bytes" "context" + "encoding/json" "fmt" + "io" + "net/http" + "net/url" + "path" + "strings" "time" "github.com/go-logr/logr" @@ -36,6 +43,9 @@ const ( // Client provides methods to interact with Azure Managed Grafana instances. type Client struct { grafanaClient *sdk.Client + endpoint string + token string + httpClient *http.Client } type retryableLogger struct { @@ -59,18 +69,22 @@ func NewClient(ctx context.Context, credential azcore.TokenCredential, managedGr return nil, fmt.Errorf("failed to get API token: %w", err) } - httpClient := retryablehttp.NewClient() - httpClient.RetryMax = grafanaAPIRetryMax - httpClient.Logger = &retryableLogger{logger: logr.FromContextOrDiscard(ctx).WithName("grafana").WithValues("endpoint", endpoint)} - httpClient.HTTPClient.Timeout = grafanaAPITimeout + retryableClient := retryablehttp.NewClient() + retryableClient.RetryMax = grafanaAPIRetryMax + retryableClient.Logger = &retryableLogger{logger: logr.FromContextOrDiscard(ctx).WithName("grafana").WithValues("endpoint", endpoint)} + retryableClient.HTTPClient.Timeout = grafanaAPITimeout + httpClient := retryableClient.StandardClient() - grafanaClient, err := sdk.NewClient(endpoint, token, httpClient.StandardClient()) + grafanaClient, err := sdk.NewClient(endpoint, token, httpClient) if err != nil { return nil, fmt.Errorf("failed to create Grafana SDK client: %w", err) } return &Client{ grafanaClient: grafanaClient, + endpoint: endpoint, + token: token, + httpClient: httpClient, }, nil } @@ -98,6 +112,86 @@ func (c *Client) ListDataSources(ctx context.Context) ([]sdk.Datasource, error) return datasources, nil } +// ListDataSourceTypes returns all datasource plugins available in the Grafana instance. +func (c *Client) ListDataSourceTypes(ctx context.Context) (map[string]sdk.DatasourceType, error) { + dataSourceTypes, err := c.grafanaClient.GetDatasourceTypes(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get datasource types: %w", err) + } + + return dataSourceTypes, nil +} + +// CreateDataSource creates a datasource and fails on non-successful Grafana API responses. +func (c *Client) CreateDataSource(ctx context.Context, dataSource sdk.Datasource) error { + if _, err := c.doGrafanaRequest(ctx, http.MethodPost, "api/datasources", dataSource, http.StatusOK, http.StatusCreated); err != nil { + return fmt.Errorf("failed to create datasource %q: %w", dataSource.Name, err) + } + + return nil +} + +// UpdateDataSource updates a datasource and fails on non-successful Grafana API responses. +func (c *Client) UpdateDataSource(ctx context.Context, dataSource sdk.Datasource) error { + if dataSource.ID == 0 { + return fmt.Errorf("datasource ID is required for update") + } + + if _, err := c.doGrafanaRequest(ctx, http.MethodPut, fmt.Sprintf("api/datasources/%d", dataSource.ID), dataSource, http.StatusOK); err != nil { + return fmt.Errorf("failed to update datasource %q: %w", dataSource.Name, err) + } + + return nil +} + +func (c *Client) doGrafanaRequest(ctx context.Context, method, apiPath string, body interface{}, allowedStatusCodes ...int) ([]byte, error) { + endpoint, err := url.Parse(c.endpoint) + if err != nil { + return nil, fmt.Errorf("failed to parse Grafana endpoint: %w", err) + } + endpoint.Path = path.Join(endpoint.Path, apiPath) + if !strings.HasPrefix(endpoint.Path, "/") { + endpoint.Path = "/" + endpoint.Path + } + + raw, err := json.Marshal(body) + if err != nil { + return nil, fmt.Errorf("failed to serialize request body: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, method, endpoint.String(), bytes.NewReader(raw)) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + req.Header.Set("Accept", "application/json") + req.Header.Set("Authorization", "Bearer "+c.token) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("User-Agent", "grafanactl") + + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("grafana API %s /%s request failed: %w", method, apiPath, err) + } + defer resp.Body.Close() + + responseBody, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read Grafana API response: %w", err) + } + + for _, allowedStatusCode := range allowedStatusCodes { + if resp.StatusCode == allowedStatusCode { + return responseBody, nil + } + } + + message := fmt.Sprintf("grafana API %s /%s failed with status %d: %s", method, apiPath, resp.StatusCode, strings.TrimSpace(string(responseBody))) + if resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden { + message += "; verify the caller has Grafana Admin permissions" + } + return nil, fmt.Errorf("%s", message) +} + // DeleteDataSource removes a datasource from the Grafana instance by name. func (c *Client) DeleteDataSource(ctx context.Context, dataSourceName string) error { _, err := c.grafanaClient.DeleteDatasourceByName(ctx, dataSourceName) diff --git a/tools/grafanactl/internal/grafana/client_test.go b/tools/grafanactl/internal/grafana/client_test.go new file mode 100644 index 00000000..22e3f7c2 --- /dev/null +++ b/tools/grafanactl/internal/grafana/client_test.go @@ -0,0 +1,92 @@ +// Copyright 2025 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package grafana + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/grafana-tools/sdk" +) + +func TestCreateDataSourceUsesStatusAwareRequest(t *testing.T) { + var gotAuthHeader string + var gotDatasource sdk.Datasource + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Fatalf("expected method %s, got %s", http.MethodPost, r.Method) + } + if r.URL.Path != "/api/datasources" { + t.Fatalf("expected path /api/datasources, got %s", r.URL.Path) + } + gotAuthHeader = r.Header.Get("Authorization") + if err := json.NewDecoder(r.Body).Decode(&gotDatasource); err != nil { + t.Fatalf("failed to decode request body: %v", err) + } + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"message":"created"}`)) + })) + defer server.Close() + + client := &Client{ + endpoint: server.URL, + token: "token", + httpClient: server.Client(), + } + + err := client.CreateDataSource(context.Background(), sdk.Datasource{ + Name: "kusto-int-uksouth", + Type: "grafana-azure-data-explorer-datasource", + }) + if err != nil { + t.Fatalf("CreateDataSource returned error: %v", err) + } + if gotAuthHeader != "Bearer token" { + t.Fatalf("expected bearer token auth header, got %q", gotAuthHeader) + } + if gotDatasource.Name != "kusto-int-uksouth" { + t.Fatalf("expected datasource name in request, got %q", gotDatasource.Name) + } +} + +func TestUpdateDataSourceReportsForbiddenAsAdminPermissionError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"message":"forbidden"}`)) + })) + defer server.Close() + + client := &Client{ + endpoint: server.URL, + token: "token", + httpClient: server.Client(), + } + + err := client.UpdateDataSource(context.Background(), sdk.Datasource{ + ID: 42, + Name: "kusto-int-uksouth", + }) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "Grafana Admin permissions") { + t.Fatalf("expected Grafana Admin permissions error, got %v", err) + } +} From 8eb6dd80341d3f7a39474bce30caa135aee7447a Mon Sep 17 00:00:00 2001 From: swiencki Date: Tue, 28 Apr 2026 11:26:21 -0700 Subject: [PATCH 2/5] grafanactl: address ADX review comments --- tools/grafanactl/cmd/modify/adx.go | 4 +- tools/grafanactl/internal/grafana/client.go | 34 ++++++++++---- .../internal/grafana/client_test.go | 44 +++++++++++++++++++ 3 files changed, 72 insertions(+), 10 deletions(-) diff --git a/tools/grafanactl/cmd/modify/adx.go b/tools/grafanactl/cmd/modify/adx.go index a1c703f6..a67b3c5b 100644 --- a/tools/grafanactl/cmd/modify/adx.go +++ b/tools/grafanactl/cmd/modify/adx.go @@ -49,7 +49,7 @@ func (o *CompletedReconcileADXDatasourceOptions) Run(ctx context.Context) error dataSourceTypes, err := o.GrafanaClient.ListDataSourceTypes(ctx) if err != nil { - return fmt.Errorf("failed to list Grafana datasource plugins; verify the caller has Grafana Admin permissions: %w", err) + return fmt.Errorf("failed to list Grafana datasource plugins: %w", err) } if _, ok := dataSourceTypes[adxDatasourceType]; !ok { return fmt.Errorf("Grafana datasource plugin %q is not available", adxDatasourceType) @@ -57,7 +57,7 @@ func (o *CompletedReconcileADXDatasourceOptions) Run(ctx context.Context) error dataSources, err := o.GrafanaClient.ListDataSources(ctx) if err != nil { - return fmt.Errorf("failed to list Grafana datasources; verify the caller has Grafana Admin permissions: %w", err) + return fmt.Errorf("failed to list Grafana datasources: %w", err) } var existing *sdk.Datasource diff --git a/tools/grafanactl/internal/grafana/client.go b/tools/grafanactl/internal/grafana/client.go index d933c8ab..f39ee4f7 100644 --- a/tools/grafanactl/internal/grafana/client.go +++ b/tools/grafanactl/internal/grafana/client.go @@ -104,21 +104,31 @@ func getGrafanaAPIToken(ctx context.Context, credential azcore.TokenCredential) // ListDataSources returns all datasources configured in the Grafana instance. func (c *Client) ListDataSources(ctx context.Context) ([]sdk.Datasource, error) { - datasources, err := c.grafanaClient.GetAllDatasources(ctx) + raw, err := c.doGrafanaRequest(ctx, http.MethodGet, "api/datasources", nil, http.StatusOK) if err != nil { return nil, fmt.Errorf("failed to get datasources: %w", err) } + var datasources []sdk.Datasource + if err := json.Unmarshal(raw, &datasources); err != nil { + return nil, fmt.Errorf("failed to decode datasources: %w", err) + } + return datasources, nil } // ListDataSourceTypes returns all datasource plugins available in the Grafana instance. func (c *Client) ListDataSourceTypes(ctx context.Context) (map[string]sdk.DatasourceType, error) { - dataSourceTypes, err := c.grafanaClient.GetDatasourceTypes(ctx) + raw, err := c.doGrafanaRequest(ctx, http.MethodGet, "api/datasources/plugins", nil, http.StatusOK) if err != nil { return nil, fmt.Errorf("failed to get datasource types: %w", err) } + dataSourceTypes := map[string]sdk.DatasourceType{} + if err := json.Unmarshal(raw, &dataSourceTypes); err != nil { + return nil, fmt.Errorf("failed to decode datasource types: %w", err) + } + return dataSourceTypes, nil } @@ -154,18 +164,24 @@ func (c *Client) doGrafanaRequest(ctx context.Context, method, apiPath string, b endpoint.Path = "/" + endpoint.Path } - raw, err := json.Marshal(body) - if err != nil { - return nil, fmt.Errorf("failed to serialize request body: %w", err) + var requestBody io.Reader + if body != nil { + raw, err := json.Marshal(body) + if err != nil { + return nil, fmt.Errorf("failed to serialize request body: %w", err) + } + requestBody = bytes.NewReader(raw) } - req, err := http.NewRequestWithContext(ctx, method, endpoint.String(), bytes.NewReader(raw)) + req, err := http.NewRequestWithContext(ctx, method, endpoint.String(), requestBody) if err != nil { return nil, fmt.Errorf("failed to create request: %w", err) } req.Header.Set("Accept", "application/json") req.Header.Set("Authorization", "Bearer "+c.token) - req.Header.Set("Content-Type", "application/json") + if body != nil { + req.Header.Set("Content-Type", "application/json") + } req.Header.Set("User-Agent", "grafanactl") resp, err := c.httpClient.Do(req) @@ -186,7 +202,9 @@ func (c *Client) doGrafanaRequest(ctx context.Context, method, apiPath string, b } message := fmt.Sprintf("grafana API %s /%s failed with status %d: %s", method, apiPath, resp.StatusCode, strings.TrimSpace(string(responseBody))) - if resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden { + if resp.StatusCode == http.StatusUnauthorized { + message += "; verify the caller's authentication token and Grafana endpoint configuration" + } else if resp.StatusCode == http.StatusForbidden { message += "; verify the caller has Grafana Admin permissions" } return nil, fmt.Errorf("%s", message) diff --git a/tools/grafanactl/internal/grafana/client_test.go b/tools/grafanactl/internal/grafana/client_test.go index 22e3f7c2..e662459d 100644 --- a/tools/grafanactl/internal/grafana/client_test.go +++ b/tools/grafanactl/internal/grafana/client_test.go @@ -67,7 +67,14 @@ func TestCreateDataSourceUsesStatusAwareRequest(t *testing.T) { } func TestUpdateDataSourceReportsForbiddenAsAdminPermissionError(t *testing.T) { + var gotAuthHeader string + var gotMethod string + var gotPath string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotMethod = r.Method + gotPath = r.URL.Path + gotAuthHeader = r.Header.Get("Authorization") w.WriteHeader(http.StatusForbidden) _, _ = w.Write([]byte(`{"message":"forbidden"}`)) })) @@ -89,4 +96,41 @@ func TestUpdateDataSourceReportsForbiddenAsAdminPermissionError(t *testing.T) { if !strings.Contains(err.Error(), "Grafana Admin permissions") { t.Fatalf("expected Grafana Admin permissions error, got %v", err) } + if gotMethod != http.MethodPut { + t.Fatalf("expected method %s, got %s", http.MethodPut, gotMethod) + } + if gotPath != "/api/datasources/42" { + t.Fatalf("expected path /api/datasources/42, got %s", gotPath) + } + if gotAuthHeader != "Bearer token" { + t.Fatalf("expected bearer token auth header, got %q", gotAuthHeader) + } +} + +func TestCreateDataSourceReportsUnauthorizedAsAuthError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"message":"unauthorized"}`)) + })) + defer server.Close() + + client := &Client{ + endpoint: server.URL, + token: "token", + httpClient: server.Client(), + } + + err := client.CreateDataSource(context.Background(), sdk.Datasource{ + Name: "kusto-int-uksouth", + Type: "grafana-azure-data-explorer-datasource", + }) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "authentication token") { + t.Fatalf("expected authentication token error, got %v", err) + } + if strings.Contains(err.Error(), "Grafana Admin permissions") { + t.Fatalf("did not expect Grafana Admin permissions error, got %v", err) + } } From 73394d8eacac5ec0ae76d4c7ab15da59c5a437e5 Mon Sep 17 00:00:00 2001 From: swiencki Date: Thu, 30 Apr 2026 13:52:18 -0700 Subject: [PATCH 3/5] grafanactl: align ADX datasource reconcile --- pipelines/types/common.go | 30 ++- pipelines/types/common_test.go | 22 +++ pipelines/types/pipeline.schema.v1.json | 45 +++++ pipelines/types/validation_test.go | 59 ++++++ tools/grafanactl/README.md | 43 +++-- tools/grafanactl/cmd/modify/adx.go | 71 ++++++-- tools/grafanactl/cmd/modify/adx_options.go | 43 ++++- .../grafanactl/cmd/modify/adx_options_test.go | 171 +++++++++++++++++- tools/grafanactl/cmd/modify/cmd.go | 48 +++-- tools/grafanactl/cmd/modify/options.go | 144 ++++++++++++--- tools/grafanactl/internal/grafana/client.go | 18 +- .../internal/grafana/client_test.go | 69 +++++++ 12 files changed, 677 insertions(+), 86 deletions(-) diff --git a/pipelines/types/common.go b/pipelines/types/common.go index 0f085ca6..7f0c0f1b 100644 --- a/pipelines/types/common.go +++ b/pipelines/types/common.go @@ -725,10 +725,28 @@ func (s *GrafanaDashboardsStep) IsWellFormedOverInputs() bool { const StepActionGrafanaDatasources = "GrafanaDatasources" +type GrafanaAzureMonitorDatasources struct { + Enabled *bool `json:"enabled,omitempty"` +} + +type GrafanaADXDatasource struct { + Enabled Value `json:"enabled,omitempty"` + DeleteWhenDisabled bool `json:"deleteWhenDisabled,omitempty"` + ClusterURL Value `json:"clusterUrl,omitempty"` + DefaultDatabase Value `json:"defaultDatabase,omitempty"` + DatasourceName Value `json:"datasourceName,omitempty"` + Geographies Value `json:"geographies,omitempty"` + DataConsistency string `json:"dataConsistency,omitempty"` +} + type GrafanaDatasourcesStep struct { StepMeta `json:",inline"` - GrafanaName string `json:"grafanaName"` + GrafanaName string `json:"grafanaName"` + GrafanaResourceID *Value `json:"grafanaResourceId,omitempty"` + + AzureMonitor *GrafanaAzureMonitorDatasources `json:"azureMonitor,omitempty"` + ADX *GrafanaADXDatasource `json:"adx,omitempty"` // SkipSync indicates whether to skip syncing datasources. It is intended for prow jobs to skip syncing datasources. SkipSync bool `json:"skipSync,omitempty"` @@ -746,6 +764,16 @@ func (s *GrafanaDatasourcesStep) RequiredInputs() []StepDependency { for _, val := range []Input{s.IdentityFrom} { deps = append(deps, val.StepDependency) } + if s.GrafanaResourceID != nil && s.GrafanaResourceID.Input != nil { + deps = append(deps, s.GrafanaResourceID.Input.StepDependency) + } + if s.ADX != nil { + for _, val := range []Value{s.ADX.Enabled, s.ADX.ClusterURL, s.ADX.DefaultDatabase, s.ADX.DatasourceName, s.ADX.Geographies} { + if val.Input != nil { + deps = append(deps, val.Input.StepDependency) + } + } + } slices.SortFunc(deps, SortDependencies) deps = slices.Compact(deps) diff --git a/pipelines/types/common_test.go b/pipelines/types/common_test.go index 8b88cb6d..c431b9c9 100644 --- a/pipelines/types/common_test.go +++ b/pipelines/types/common_test.go @@ -263,6 +263,28 @@ func TestRequiredInputs(t *testing.T) { name: "pav2 empty", input: &Pav2Step{}, }, + { + name: "grafana datasources full", + input: &GrafanaDatasourcesStep{ + IdentityFrom: Input{StepDependency: StepDependency{ResourceGroup: "rg", Step: "identity"}}, + GrafanaResourceID: &Value{Input: &Input{StepDependency: StepDependency{ResourceGroup: "rg", Step: "global"}}}, + ADX: &GrafanaADXDatasource{ + Enabled: Value{Input: &Input{StepDependency: StepDependency{ResourceGroup: "rg", Step: "config"}}}, + ClusterURL: Value{Input: &Input{StepDependency: StepDependency{ResourceGroup: "rg", Step: "kusto"}}}, + DefaultDatabase: Value{Input: &Input{StepDependency: StepDependency{ResourceGroup: "rg", Step: "config"}}}, + DatasourceName: Value{Input: &Input{StepDependency: StepDependency{ResourceGroup: "rg", Step: "name"}}}, + Geographies: Value{Input: &Input{StepDependency: StepDependency{ResourceGroup: "rg", Step: "geo"}}}, + }, + }, + expected: []StepDependency{ + {ResourceGroup: "rg", Step: "config"}, + {ResourceGroup: "rg", Step: "geo"}, + {ResourceGroup: "rg", Step: "global"}, + {ResourceGroup: "rg", Step: "identity"}, + {ResourceGroup: "rg", Step: "kusto"}, + {ResourceGroup: "rg", Step: "name"}, + }, + }, } { t.Run(testCase.name, func(t *testing.T) { if diff := cmp.Diff(testCase.expected, testCase.input.RequiredInputs()); diff != "" { diff --git a/pipelines/types/pipeline.schema.v1.json b/pipelines/types/pipeline.schema.v1.json index c7185918..ea6bf7fa 100644 --- a/pipelines/types/pipeline.schema.v1.json +++ b/pipelines/types/pipeline.schema.v1.json @@ -688,6 +688,42 @@ } ] }, + "grafanaAzureMonitorDatasources": { + "type": "object", + "additionalProperties": false, + "properties": { + "enabled": { + "type": "boolean" + } + } + }, + "grafanaADXDatasource": { + "type": "object", + "additionalProperties": false, + "properties": { + "enabled": { + "$ref": "#/definitions/value" + }, + "deleteWhenDisabled": { + "type": "boolean" + }, + "clusterUrl": { + "$ref": "#/definitions/value" + }, + "defaultDatabase": { + "$ref": "#/definitions/value" + }, + "datasourceName": { + "$ref": "#/definitions/value" + }, + "geographies": { + "$ref": "#/definitions/value" + }, + "dataConsistency": { + "type": "string" + } + } + }, "grafanaDatasourcesStep": { "unevaluatedProperties": false, "allOf": [ @@ -702,6 +738,15 @@ "grafanaName": { "type": "string" }, + "grafanaResourceId": { + "$ref": "#/definitions/value" + }, + "azureMonitor": { + "$ref": "#/definitions/grafanaAzureMonitorDatasources" + }, + "adx": { + "$ref": "#/definitions/grafanaADXDatasource" + }, "skipSync": { "type": "string" }, diff --git a/pipelines/types/validation_test.go b/pipelines/types/validation_test.go index b6b9bea2..67d6f7bb 100644 --- a/pipelines/types/validation_test.go +++ b/pipelines/types/validation_test.go @@ -309,6 +309,65 @@ func TestValidatePipelineSchema(t *testing.T) { }, }, }, + { + name: "valid grafana datasources with adx", + pipeline: map[string]interface{}{ + "serviceGroup": "test", + "rolloutName": "test", + "resourceGroups": []interface{}{ + map[string]interface{}{ + "name": "rg", + "resourceGroup": "rg", + "subscription": "sub", + "steps": []interface{}{ + map[string]interface{}{ + "name": "datasources", + "action": "GrafanaDatasources", + "grafanaName": "grafana", + "grafanaResourceId": map[string]interface{}{ + "input": map[string]interface{}{ + "resourceGroup": "global", + "step": "output", + "name": "grafanaResourceId", + }, + }, + "identityFrom": map[string]interface{}{ + "resourceGroup": "rg", + "step": "deploy", + "name": "msi", + }, + "azureMonitor": map[string]interface{}{ + "enabled": false, + }, + "adx": map[string]interface{}{ + "enabled": map[string]interface{}{ + "configRef": "monitoring.adxDatasourceEnabled", + }, + "deleteWhenDisabled": true, + "clusterUrl": map[string]interface{}{ + "input": map[string]interface{}{ + "resourceGroup": "rg", + "step": "kusto", + "name": "kustoUri", + }, + }, + "defaultDatabase": map[string]interface{}{ + "value": "ServiceLogs", + }, + "datasourceName": map[string]interface{}{ + "value": "kusto-int-uksouth", + }, + "geographies": map[string]interface{}{ + "configRef": "monitoring.adxDatasourceGeographies", + }, + "dataConsistency": "strongconsistency", + }, + }, + }, + }, + }, + }, + }, { name: "invalid", pipeline: map[string]interface{}{ diff --git a/tools/grafanactl/README.md b/tools/grafanactl/README.md index 1c0ff38f..742398a6 100644 --- a/tools/grafanactl/README.md +++ b/tools/grafanactl/README.md @@ -116,31 +116,46 @@ grafanactl clean fixup-datasources \ Modify commands reconcile resources in Azure Managed Grafana. -#### Reconcile Azure Data Explorer Datasource +#### Reconcile Datasources -Create or update one Azure Data Explorer datasource using Grafana's REST API: +Reconcile Azure Monitor Workspace integrations and, when enabled, one Azure Data +Explorer datasource using Grafana's REST API: ```bash # Preview changes (dry-run) -grafanactl modify datasource reconcile-adx \ +grafanactl modify datasource reconcile \ --grafana-resource-id "/subscriptions//resourceGroups//providers/Microsoft.Dashboard/grafana/" \ - --cluster-url "https://example.region.kusto.windows.net" \ - --default-database "ServiceLogs" \ - --datasource-name "kusto-int-uksouth" \ + --azure-monitor-enabled=false \ + --adx-enabled=true \ + --adx-delete-when-disabled=true \ + --adx-cluster-url "https://example.region.kusto.windows.net" \ + --adx-default-database "ServiceLogs" \ + --adx-geographies "uksouth,eastus2" \ + --adx-current-geography "uksouth" \ + --adx-datasource-name "kusto-int-uksouth" \ --dry-run # Apply changes -grafanactl modify datasource reconcile-adx \ +grafanactl modify datasource reconcile \ --grafana-resource-id "/subscriptions//resourceGroups//providers/Microsoft.Dashboard/grafana/" \ - --cluster-url "https://example.region.kusto.windows.net" \ - --default-database "ServiceLogs" \ - --datasource-name "kusto-int-uksouth" + --azure-monitor-enabled=false \ + --adx-enabled=true \ + --adx-delete-when-disabled=true \ + --adx-cluster-url "https://example.region.kusto.windows.net" \ + --adx-default-database "ServiceLogs" \ + --adx-geographies "uksouth,eastus2" \ + --adx-current-geography "uksouth" \ + --adx-datasource-name "kusto-int-uksouth" ``` -The command requires the Azure Data Explorer datasource plugin -(`grafana-azure-data-explorer-datasource`) to be available in Grafana. It uses -the Grafana managed identity for ADX authentication and fails if an existing -datasource with the requested name has a different plugin type. +When `--adx-enabled=false --adx-delete-when-disabled=true` are both set, the +command deletes the named ADX datasource if it exists. ADX create/update requires +the Azure Data Explorer datasource plugin +(`grafana-azure-data-explorer-datasource`) to be available in Grafana. The +datasource uses the Grafana managed identity for ADX authentication and fails if +an existing datasource with the requested name has a different plugin type. When +`--adx-geographies` is set, the command validates the comma-separated geography +allowlist and disables ADX desired state for geographies not in the list. ### Sync Commands diff --git a/tools/grafanactl/cmd/modify/adx.go b/tools/grafanactl/cmd/modify/adx.go index a67b3c5b..03ebced7 100644 --- a/tools/grafanactl/cmd/modify/adx.go +++ b/tools/grafanactl/cmd/modify/adx.go @@ -47,28 +47,21 @@ func (o *CompletedReconcileADXDatasourceOptions) Run(ctx context.Context) error logger.Info("reconcile ADX datasource command executed", "dry-run", o.DryRun) + if !o.Enabled { + return o.deleteDatasourceWhenDisabled(ctx, logger) + } + dataSourceTypes, err := o.GrafanaClient.ListDataSourceTypes(ctx) if err != nil { return fmt.Errorf("failed to list Grafana datasource plugins: %w", err) } if _, ok := dataSourceTypes[adxDatasourceType]; !ok { - return fmt.Errorf("Grafana datasource plugin %q is not available", adxDatasourceType) + return fmt.Errorf("grafana datasource plugin %q is not available", adxDatasourceType) } - dataSources, err := o.GrafanaClient.ListDataSources(ctx) + existing, err := o.findExistingDatasource(ctx) if err != nil { - return fmt.Errorf("failed to list Grafana datasources: %w", err) - } - - var existing *sdk.Datasource - for i := range dataSources { - if dataSources[i].Name != o.DatasourceName { - continue - } - if existing != nil { - return fmt.Errorf("found multiple datasources named %q", o.DatasourceName) - } - existing = &dataSources[i] + return err } desired := o.desiredDatasource() @@ -107,6 +100,56 @@ func (o *CompletedReconcileADXDatasourceOptions) Run(ctx context.Context) error return nil } +func (o *CompletedReconcileADXDatasourceOptions) deleteDatasourceWhenDisabled(ctx context.Context, logger logr.Logger) error { + if !o.DeleteWhenDisabled { + logger.Info("ADX datasource desired state disabled and deletion disabled") + return nil + } + + existing, err := o.findExistingDatasource(ctx) + if err != nil { + return err + } + if existing == nil { + logger.Info("ADX datasource desired state disabled and datasource is already absent") + return nil + } + if existing.Type != adxDatasourceType { + return fmt.Errorf("datasource %q already exists with type %q, expected %q", o.DatasourceName, existing.Type, adxDatasourceType) + } + if o.DryRun { + logger.Info("Dry run - would delete ADX datasource", "datasource-id", existing.ID, "datasource-uid", existing.UID) + return nil + } + + logger.Info("Deleting ADX datasource", "datasource-id", existing.ID, "datasource-uid", existing.UID) + if err := o.GrafanaClient.DeleteDataSource(ctx, o.DatasourceName); err != nil { + return fmt.Errorf("failed to delete ADX datasource %q: %w", o.DatasourceName, err) + } + + return nil +} + +func (o *CompletedReconcileADXDatasourceOptions) findExistingDatasource(ctx context.Context) (*sdk.Datasource, error) { + dataSources, err := o.GrafanaClient.ListDataSources(ctx) + if err != nil { + return nil, fmt.Errorf("failed to list Grafana datasources: %w", err) + } + + var existing *sdk.Datasource + for i := range dataSources { + if dataSources[i].Name != o.DatasourceName { + continue + } + if existing != nil { + return nil, fmt.Errorf("found multiple datasources named %q", o.DatasourceName) + } + existing = &dataSources[i] + } + + return existing, nil +} + func (o *CompletedReconcileADXDatasourceOptions) desiredDatasource() sdk.Datasource { return sdk.Datasource{ Name: o.DatasourceName, diff --git a/tools/grafanactl/cmd/modify/adx_options.go b/tools/grafanactl/cmd/modify/adx_options.go index 805c38c0..a7e2c50a 100644 --- a/tools/grafanactl/cmd/modify/adx_options.go +++ b/tools/grafanactl/cmd/modify/adx_options.go @@ -36,15 +36,18 @@ type adxGrafanaClient interface { ListDataSourceTypes(ctx context.Context) (map[string]sdk.DatasourceType, error) CreateDataSource(ctx context.Context, dataSource sdk.Datasource) error UpdateDataSource(ctx context.Context, dataSource sdk.Datasource) error + DeleteDataSource(ctx context.Context, dataSourceName string) error } // RawReconcileADXDatasourceOptions represents the initial, unvalidated configuration for ADX datasource reconcile operations. type RawReconcileADXDatasourceOptions struct { *base.BaseOptions - ClusterURL string - DefaultDatabase string - DatasourceName string - DataConsistency string + Enabled bool + DeleteWhenDisabled bool + ClusterURL string + DefaultDatabase string + DatasourceName string + DataConsistency string } type validatedReconcileADXDatasourceOptions struct { @@ -66,6 +69,7 @@ type CompletedReconcileADXDatasourceOptions struct { func DefaultReconcileADXDatasourceOptions() *RawReconcileADXDatasourceOptions { return &RawReconcileADXDatasourceOptions{ BaseOptions: base.DefaultBaseOptions(), + Enabled: true, DataConsistency: defaultADXDataConsistency, } } @@ -77,6 +81,8 @@ func BindReconcileADXDatasourceOptions(opts *RawReconcileADXDatasourceOptions, c } flags := cmd.Flags() + flags.BoolVar(&opts.Enabled, "enabled", opts.Enabled, "Reconcile the Azure Data Explorer datasource desired state as present") + flags.BoolVar(&opts.DeleteWhenDisabled, "delete-when-disabled", opts.DeleteWhenDisabled, "Delete the named Azure Data Explorer datasource when desired state is disabled") flags.StringVar(&opts.ClusterURL, "cluster-url", opts.ClusterURL, "Azure Data Explorer cluster URL") flags.StringVar(&opts.DefaultDatabase, "default-database", opts.DefaultDatabase, "Default Azure Data Explorer database") flags.StringVar(&opts.DatasourceName, "datasource-name", opts.DatasourceName, "Grafana datasource name") @@ -99,6 +105,23 @@ func (o *RawReconcileADXDatasourceOptions) Validate(ctx context.Context) (*Valid dataConsistency = defaultADXDataConsistency } + if !o.Enabled { + if o.DeleteWhenDisabled && datasourceName == "" { + return nil, fmt.Errorf("datasource name is required when ADX datasource deletion is enabled") + } + return &ValidatedReconcileADXDatasourceOptions{ + validatedReconcileADXDatasourceOptions: &validatedReconcileADXDatasourceOptions{ + RawReconcileADXDatasourceOptions: &RawReconcileADXDatasourceOptions{ + BaseOptions: o.BaseOptions, + Enabled: false, + DeleteWhenDisabled: o.DeleteWhenDisabled, + DatasourceName: datasourceName, + DataConsistency: dataConsistency, + }, + }, + }, nil + } + if clusterURL == "" { return nil, fmt.Errorf("cluster URL is required") } @@ -119,11 +142,13 @@ func (o *RawReconcileADXDatasourceOptions) Validate(ctx context.Context) (*Valid return &ValidatedReconcileADXDatasourceOptions{ validatedReconcileADXDatasourceOptions: &validatedReconcileADXDatasourceOptions{ RawReconcileADXDatasourceOptions: &RawReconcileADXDatasourceOptions{ - BaseOptions: o.BaseOptions, - ClusterURL: clusterURL, - DefaultDatabase: defaultDatabase, - DatasourceName: datasourceName, - DataConsistency: dataConsistency, + BaseOptions: o.BaseOptions, + Enabled: true, + DeleteWhenDisabled: o.DeleteWhenDisabled, + ClusterURL: clusterURL, + DefaultDatabase: defaultDatabase, + DatasourceName: datasourceName, + DataConsistency: dataConsistency, }, }, }, nil diff --git a/tools/grafanactl/cmd/modify/adx_options_test.go b/tools/grafanactl/cmd/modify/adx_options_test.go index f58d34ac..f9c1665a 100644 --- a/tools/grafanactl/cmd/modify/adx_options_test.go +++ b/tools/grafanactl/cmd/modify/adx_options_test.go @@ -30,10 +30,12 @@ type fakeADXGrafanaClient struct { dataSourceTypes map[string]sdk.DatasourceType createCalls []sdk.Datasource updateCalls []sdk.Datasource + deleteCalls []string listTypesErr error listErr error createErr error updateErr error + deleteErr error } func (f *fakeADXGrafanaClient) ListDataSources(ctx context.Context) ([]sdk.Datasource, error) { @@ -46,7 +48,14 @@ func (f *fakeADXGrafanaClient) ListDataSourceTypes(ctx context.Context) (map[str func (f *fakeADXGrafanaClient) CreateDataSource(ctx context.Context, dataSource sdk.Datasource) error { f.createCalls = append(f.createCalls, dataSource) - return f.createErr + if f.createErr != nil { + return f.createErr + } + if dataSource.UID == "" { + dataSource.UID = "created-uid" + } + f.dataSources = append(f.dataSources, dataSource) + return nil } func (f *fakeADXGrafanaClient) UpdateDataSource(ctx context.Context, dataSource sdk.Datasource) error { @@ -54,6 +63,11 @@ func (f *fakeADXGrafanaClient) UpdateDataSource(ctx context.Context, dataSource return f.updateErr } +func (f *fakeADXGrafanaClient) DeleteDataSource(ctx context.Context, dataSourceName string) error { + f.deleteCalls = append(f.deleteCalls, dataSourceName) + return f.deleteErr +} + func validRawReconcileADXDatasourceOptions() *RawReconcileADXDatasourceOptions { baseOptions := base.DefaultBaseOptions() baseOptions.SubscriptionID = "subscription-id" @@ -62,6 +76,7 @@ func validRawReconcileADXDatasourceOptions() *RawReconcileADXDatasourceOptions { return &RawReconcileADXDatasourceOptions{ BaseOptions: baseOptions, + Enabled: true, ClusterURL: "https://example.kusto.windows.net", DefaultDatabase: "ServiceLogs", DatasourceName: "kusto-int-uksouth", @@ -102,6 +117,33 @@ func TestReconcileADXDatasourceValidateRejectsInvalidClusterURL(t *testing.T) { } } +func TestReconcileADXDatasourceValidateAllowsDisabledWithoutClusterDetails(t *testing.T) { + opts := validRawReconcileADXDatasourceOptions() + opts.Enabled = false + opts.ClusterURL = "" + opts.DefaultDatabase = "" + opts.DatasourceName = "" + + if _, err := opts.Validate(context.Background()); err != nil { + t.Fatalf("Validate returned error: %v", err) + } +} + +func TestReconcileADXDatasourceValidateDisabledDeleteRequiresDatasourceName(t *testing.T) { + opts := validRawReconcileADXDatasourceOptions() + opts.Enabled = false + opts.DeleteWhenDisabled = true + opts.DatasourceName = "" + + _, err := opts.Validate(context.Background()) + if err == nil { + t.Fatal("expected validation error") + } + if !strings.Contains(err.Error(), "datasource name is required") { + t.Fatalf("expected datasource name error, got %v", err) + } +} + func TestReconcileADXDatasourceRunCreatesDatasource(t *testing.T) { client := &fakeADXGrafanaClient{ dataSourceTypes: map[string]sdk.DatasourceType{ @@ -119,7 +161,9 @@ func TestReconcileADXDatasourceRunCreatesDatasource(t *testing.T) { if len(client.updateCalls) != 0 { t.Fatalf("expected no update calls, got %d", len(client.updateCalls)) } - + if len(client.deleteCalls) != 0 { + t.Fatalf("expected no delete calls, got %d", len(client.deleteCalls)) + } created := client.createCalls[0] if created.Name != opts.DatasourceName { t.Fatalf("expected datasource name %q, got %q", opts.DatasourceName, created.Name) @@ -155,6 +199,7 @@ func TestReconcileADXDatasourceRunUpdatesExistingDatasource(t *testing.T) { UID: "existing-uid", Name: "kusto-int-uksouth", Type: adxDatasourceType, + Access: "direct", IsDefault: true, }, }, @@ -177,6 +222,63 @@ func TestReconcileADXDatasourceRunUpdatesExistingDatasource(t *testing.T) { } } +func TestReconcileADXDatasourceRunDeletesWhenDisabled(t *testing.T) { + client := &fakeADXGrafanaClient{ + dataSources: []sdk.Datasource{ + { + ID: 42, + UID: "existing-uid", + Name: "kusto-int-uksouth", + Type: adxDatasourceType, + }, + }, + } + opts := completedReconcileADXDatasourceOptionsForTest(client) + opts.Enabled = false + opts.DeleteWhenDisabled = true + + if err := opts.Run(context.Background()); err != nil { + t.Fatalf("Run returned error: %v", err) + } + if len(client.createCalls) != 0 { + t.Fatalf("expected no create calls, got %d", len(client.createCalls)) + } + if len(client.updateCalls) != 0 { + t.Fatalf("expected no update calls, got %d", len(client.updateCalls)) + } + if len(client.deleteCalls) != 1 || client.deleteCalls[0] != "kusto-int-uksouth" { + t.Fatalf("expected datasource delete call, got %#v", client.deleteCalls) + } +} + +func TestReconcileADXDatasourceRunDisabledDeleteIgnoresAbsentDatasource(t *testing.T) { + client := &fakeADXGrafanaClient{} + opts := completedReconcileADXDatasourceOptionsForTest(client) + opts.Enabled = false + opts.DeleteWhenDisabled = true + + if err := opts.Run(context.Background()); err != nil { + t.Fatalf("Run returned error: %v", err) + } + if len(client.deleteCalls) != 0 { + t.Fatalf("expected no delete calls, got %d", len(client.deleteCalls)) + } +} + +func TestReconcileADXDatasourceRunDisabledWithoutDeleteDoesNothing(t *testing.T) { + client := &fakeADXGrafanaClient{} + opts := completedReconcileADXDatasourceOptionsForTest(client) + opts.Enabled = false + opts.DeleteWhenDisabled = false + + if err := opts.Run(context.Background()); err != nil { + t.Fatalf("Run returned error: %v", err) + } + if len(client.createCalls) != 0 || len(client.updateCalls) != 0 || len(client.deleteCalls) != 0 { + t.Fatalf("expected no mutation calls, got create=%d update=%d delete=%d", len(client.createCalls), len(client.updateCalls), len(client.deleteCalls)) + } +} + func TestReconcileADXDatasourceRunRejectsExistingDatasourceWithWrongType(t *testing.T) { client := &fakeADXGrafanaClient{ dataSourceTypes: map[string]sdk.DatasourceType{ @@ -232,3 +334,68 @@ func TestReconcileADXDatasourceRunPropagatesMutationErrors(t *testing.T) { t.Fatalf("expected create error, got %v", err) } } + +func TestAddDatasourceValidateDisablesADXWhenCurrentGeographyNotAllowed(t *testing.T) { + opts := DefaultAddDatasourceOptions() + opts.SubscriptionID = "subscription-id" + opts.ResourceGroup = "resource-group" + opts.GrafanaName = "grafana-name" + opts.AzureMonitorEnabled = false + opts.ADXEnabled = true + opts.ADXDeleteWhenDisabled = true + opts.ADXDatasourceName = "kusto-int-uksouth" + opts.ADXGeographies = "eus2, wus3" + opts.ADXCurrentGeography = "UKS" + + validated, err := opts.Validate(context.Background()) + if err != nil { + t.Fatalf("Validate returned error: %v", err) + } + if validated.ADXEnabled { + t.Fatal("expected ADX to be disabled for disallowed geography") + } + if !validated.ADXDeleteWhenDisabled { + t.Fatal("expected deleteWhenDisabled to remain enabled") + } +} + +func TestAddDatasourceValidateAllowsADXWhenCurrentGeographyAllowed(t *testing.T) { + opts := DefaultAddDatasourceOptions() + opts.SubscriptionID = "subscription-id" + opts.ResourceGroup = "resource-group" + opts.GrafanaName = "grafana-name" + opts.AzureMonitorEnabled = false + opts.ADXEnabled = true + opts.ADXClusterURL = "https://example.kusto.windows.net" + opts.ADXDefaultDatabase = "ServiceLogs" + opts.ADXDatasourceName = "kusto-int-uksouth" + opts.ADXGeographies = " eus2, UKS " + opts.ADXCurrentGeography = "uks" + + validated, err := opts.Validate(context.Background()) + if err != nil { + t.Fatalf("Validate returned error: %v", err) + } + if !validated.ADXEnabled { + t.Fatal("expected ADX to remain enabled for allowed geography") + } +} + +func TestAddDatasourceValidateRejectsInvalidADXGeographies(t *testing.T) { + opts := DefaultAddDatasourceOptions() + opts.SubscriptionID = "subscription-id" + opts.ResourceGroup = "resource-group" + opts.GrafanaName = "grafana-name" + opts.AzureMonitorEnabled = false + opts.ADXEnabled = true + opts.ADXGeographies = "uks,!" + opts.ADXCurrentGeography = "uks" + + _, err := opts.Validate(context.Background()) + if err == nil { + t.Fatal("expected validation error") + } + if !strings.Contains(err.Error(), "invalid entry") { + t.Fatalf("expected invalid geography error, got %v", err) + } +} diff --git a/tools/grafanactl/cmd/modify/cmd.go b/tools/grafanactl/cmd/modify/cmd.go index f155136e..86924fcf 100644 --- a/tools/grafanactl/cmd/modify/cmd.go +++ b/tools/grafanactl/cmd/modify/cmd.go @@ -31,7 +31,6 @@ const datasourceGroupID = "datasource" func NewModifyCommand(group string) (*cobra.Command, error) { addDatasourceOpts := DefaultAddDatasourceOptions() - reconcileADXDatasourceOpts := DefaultReconcileADXDatasourceOptions() modifyCmd := &cobra.Command{ Use: "modify", @@ -54,32 +53,18 @@ func NewModifyCommand(group string) (*cobra.Command, error) { addDatasourceCmd := &cobra.Command{ Use: "reconcile", - Short: "Reconcile Azure Monitor Workspace datasources in Grafana", - Long: "Reconcile Azure Monitor Workspace datasources in the Azure Managed Grafana instance. This integrates the workspaces with Grafana and creates the necessary datasource configuration.", + Short: "Reconcile datasources in Grafana", + Long: "Reconcile Azure Monitor Workspace integrations and optional Azure Data Explorer datasources in the Azure Managed Grafana instance.", RunE: func(cmd *cobra.Command, args []string) error { return addDatasourceOpts.Run(cmd.Context()) }, } - reconcileADXDatasourceCmd := &cobra.Command{ - Use: "reconcile-adx", - Short: "Reconcile an Azure Data Explorer datasource in Grafana", - Long: "Reconcile one Azure Data Explorer datasource in the Azure Managed Grafana instance using the Grafana API.", - RunE: func(cmd *cobra.Command, args []string) error { - return reconcileADXDatasourceOpts.Run(cmd.Context()) - }, - } - if err := BindAddDatasourceOptions(addDatasourceOpts, addDatasourceCmd); err != nil { return nil, err } - if err := BindReconcileADXDatasourceOptions(reconcileADXDatasourceOpts, reconcileADXDatasourceCmd); err != nil { - return nil, err - } - datasourceCmd.AddCommand(addDatasourceCmd) - datasourceCmd.AddCommand(reconcileADXDatasourceCmd) modifyCmd.AddCommand(datasourceCmd) return modifyCmd, nil @@ -123,8 +108,35 @@ func (o *CompletedAddDatasourceOptions) getMatchingWorkspaceIDs(ctx context.Cont func (o *CompletedAddDatasourceOptions) Run(ctx context.Context) error { logger := logr.FromContextOrDiscard(ctx).WithValues("resource-group", o.ResourceGroup, "grafana-name", o.GrafanaName) - logger.Info("add datasource command executed") + logger.Info("reconcile datasource command executed", "azure-monitor-enabled", o.AzureMonitorEnabled, "adx-enabled", o.ADXEnabled, "adx-delete-when-disabled", o.ADXDeleteWhenDisabled) + + if !o.AzureMonitorEnabled && !o.ADXEnabled && !o.ADXDeleteWhenDisabled { + logger.Info("No datasource reconcile actions enabled") + return nil + } + + if o.AzureMonitorEnabled { + if err := o.reconcileAzureMonitor(ctx, logger); err != nil { + return err + } + } + + if o.ADXEnabled || o.ADXDeleteWhenDisabled { + adxOptions := &CompletedReconcileADXDatasourceOptions{ + validatedReconcileADXDatasourceOptions: &validatedReconcileADXDatasourceOptions{ + RawReconcileADXDatasourceOptions: o.rawADXOptions(), + }, + GrafanaClient: o.GrafanaClient, + } + if err := adxOptions.Run(ctx); err != nil { + return fmt.Errorf("failed to reconcile ADX datasource: %w", err) + } + } + + return nil +} +func (o *CompletedAddDatasourceOptions) reconcileAzureMonitor(ctx context.Context, logger logr.Logger) error { grafana, err := o.ManagedGrafanaClient.GetGrafanaInstance(ctx, o.ResourceGroup, o.GrafanaName) if err != nil { return fmt.Errorf("failed to get Grafana instance: %w", err) diff --git a/tools/grafanactl/cmd/modify/options.go b/tools/grafanactl/cmd/modify/options.go index 79a987b9..20c571e2 100644 --- a/tools/grafanactl/cmd/modify/options.go +++ b/tools/grafanactl/cmd/modify/options.go @@ -17,19 +17,30 @@ package modify import ( "context" "fmt" + "strings" "github.com/spf13/cobra" "github.com/Azure/ARO-Tools/tools/cmdutils" "github.com/Azure/ARO-Tools/tools/grafanactl/cmd/base" "github.com/Azure/ARO-Tools/tools/grafanactl/internal/azure" + "github.com/Azure/ARO-Tools/tools/grafanactl/internal/grafana" ) // RawAddDatasourceOptions represents the initial, unvalidated configuration for add datasource operations. type RawAddDatasourceOptions struct { *base.BaseOptions - TagKey string - TagValue string + TagKey string + TagValue string + AzureMonitorEnabled bool + ADXEnabled bool + ADXDeleteWhenDisabled bool + ADXClusterURL string + ADXDefaultDatabase string + ADXDatasourceName string + ADXGeographies string + ADXCurrentGeography string + ADXDataConsistency string } // validatedAddDatasourceOptions is a private struct that enforces the options validation pattern. @@ -49,14 +60,17 @@ type CompletedAddDatasourceOptions struct { *validatedAddDatasourceOptions MonitorWorkspaceClient *azure.MonitorWorkspaceClient ManagedGrafanaClient *azure.ManagedGrafanaClient + GrafanaClient adxGrafanaClient } // DefaultAddDatasourceOptions returns a new RawAddDatasourceOptions with default values func DefaultAddDatasourceOptions() *RawAddDatasourceOptions { return &RawAddDatasourceOptions{ - BaseOptions: base.DefaultBaseOptions(), - TagKey: "grafanactl-discovery", - TagValue: "true", + BaseOptions: base.DefaultBaseOptions(), + TagKey: "grafanactl-discovery", + TagValue: "true", + AzureMonitorEnabled: true, + ADXDataConsistency: defaultADXDataConsistency, } } @@ -69,6 +83,15 @@ func BindAddDatasourceOptions(opts *RawAddDatasourceOptions, cmd *cobra.Command) flags := cmd.Flags() flags.StringVar(&opts.TagKey, "tag-key", opts.TagKey, "Azure Monitor Workspace tag key to filter by") flags.StringVar(&opts.TagValue, "tag-value", opts.TagValue, "Azure Monitor Workspace tag value to filter by") + flags.BoolVar(&opts.AzureMonitorEnabled, "azure-monitor-enabled", opts.AzureMonitorEnabled, "Reconcile Azure Monitor Workspace integrations") + flags.BoolVar(&opts.ADXEnabled, "adx-enabled", opts.ADXEnabled, "Reconcile the Azure Data Explorer datasource desired state as present") + flags.BoolVar(&opts.ADXDeleteWhenDisabled, "adx-delete-when-disabled", opts.ADXDeleteWhenDisabled, "Delete the named Azure Data Explorer datasource when ADX desired state is disabled") + flags.StringVar(&opts.ADXClusterURL, "adx-cluster-url", opts.ADXClusterURL, "Azure Data Explorer cluster URL") + flags.StringVar(&opts.ADXDefaultDatabase, "adx-default-database", opts.ADXDefaultDatabase, "Default Azure Data Explorer database") + flags.StringVar(&opts.ADXDatasourceName, "adx-datasource-name", opts.ADXDatasourceName, "Grafana Azure Data Explorer datasource name") + flags.StringVar(&opts.ADXGeographies, "adx-geographies", opts.ADXGeographies, "Comma-separated geography short IDs where the Azure Data Explorer datasource should be present") + flags.StringVar(&opts.ADXCurrentGeography, "adx-current-geography", opts.ADXCurrentGeography, "Current geography short ID used with --adx-geographies") + flags.StringVar(&opts.ADXDataConsistency, "adx-data-consistency", opts.ADXDataConsistency, "Azure Data Explorer datasource data consistency") return nil } @@ -79,37 +102,116 @@ func (o *RawAddDatasourceOptions) Validate(ctx context.Context) (*ValidatedAddDa return nil, err } + normalized := *o + if strings.TrimSpace(normalized.ADXGeographies) != "" { + allowed, err := adxGeographyAllowed(normalized.ADXGeographies, normalized.ADXCurrentGeography) + if err != nil { + return nil, err + } + if !allowed { + normalized.ADXEnabled = false + } + } + if normalized.ADXEnabled || normalized.ADXDeleteWhenDisabled { + adxOptions, err := normalized.rawADXOptions().Validate(ctx) + if err != nil { + return nil, fmt.Errorf("invalid ADX datasource options: %w", err) + } + normalized.ADXEnabled = adxOptions.Enabled + normalized.ADXDeleteWhenDisabled = adxOptions.DeleteWhenDisabled + normalized.ADXClusterURL = adxOptions.ClusterURL + normalized.ADXDefaultDatabase = adxOptions.DefaultDatabase + normalized.ADXDatasourceName = adxOptions.DatasourceName + normalized.ADXDataConsistency = adxOptions.DataConsistency + } + return &ValidatedAddDatasourceOptions{ validatedAddDatasourceOptions: &validatedAddDatasourceOptions{ - RawAddDatasourceOptions: &RawAddDatasourceOptions{ - BaseOptions: o.BaseOptions, - TagKey: o.TagKey, - TagValue: o.TagValue, - }, + RawAddDatasourceOptions: &normalized, }, }, nil } -// Complete performs final initialization to create fully usable add datasource options. -func (o *ValidatedAddDatasourceOptions) Complete(ctx context.Context) (*CompletedAddDatasourceOptions, error) { - cred, err := cmdutils.GetAzureTokenCredentials() - if err != nil { - return nil, fmt.Errorf("failed to obtain Azure credentials: %w", err) +func adxGeographyAllowed(geographies, currentGeography string) (bool, error) { + currentGeography = strings.ToLower(strings.TrimSpace(currentGeography)) + if currentGeography == "" { + return false, fmt.Errorf("adx-current-geography is required when adx-geographies is set") + } + if !isValidADXGeography(currentGeography) { + return false, fmt.Errorf("adx-current-geography has invalid value %q", currentGeography) } - managedGrafanaClient, err := azure.NewManagedGrafanaClient(o.SubscriptionID, cred) - if err != nil { - return nil, fmt.Errorf("failed to create managed Grafana client: %w", err) + allowed := false + for _, geography := range strings.Split(geographies, ",") { + normalized := strings.ToLower(strings.TrimSpace(geography)) + if normalized == "" || !isValidADXGeography(normalized) { + return false, fmt.Errorf("adx-geographies has invalid entry %q", geography) + } + if normalized == currentGeography { + allowed = true + } } + return allowed, nil +} - monitorWorkspaceClient, err := azure.NewMonitorWorkspaceClient(o.SubscriptionID, cred) - if err != nil { - return nil, fmt.Errorf("failed to create monitor workspace client: %w", err) +func isValidADXGeography(geography string) bool { + for _, r := range geography { + if (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') || r == '-' { + continue + } + return false + } + return true +} + +func (o *RawAddDatasourceOptions) rawADXOptions() *RawReconcileADXDatasourceOptions { + return &RawReconcileADXDatasourceOptions{ + BaseOptions: o.BaseOptions, + Enabled: o.ADXEnabled, + DeleteWhenDisabled: o.ADXDeleteWhenDisabled, + ClusterURL: o.ADXClusterURL, + DefaultDatabase: o.ADXDefaultDatabase, + DatasourceName: o.ADXDatasourceName, + DataConsistency: o.ADXDataConsistency, + } +} + +// Complete performs final initialization to create fully usable add datasource options. +func (o *ValidatedAddDatasourceOptions) Complete(ctx context.Context) (*CompletedAddDatasourceOptions, error) { + var monitorWorkspaceClient *azure.MonitorWorkspaceClient + var managedGrafanaClient *azure.ManagedGrafanaClient + var grafanaClient adxGrafanaClient + + if o.AzureMonitorEnabled || o.ADXEnabled || o.ADXDeleteWhenDisabled { + cred, err := cmdutils.GetAzureTokenCredentials() + if err != nil { + return nil, fmt.Errorf("failed to obtain Azure credentials: %w", err) + } + + managedGrafanaClient, err = azure.NewManagedGrafanaClient(o.SubscriptionID, cred) + if err != nil { + return nil, fmt.Errorf("failed to create managed Grafana client: %w", err) + } + + if o.AzureMonitorEnabled { + monitorWorkspaceClient, err = azure.NewMonitorWorkspaceClient(o.SubscriptionID, cred) + if err != nil { + return nil, fmt.Errorf("failed to create monitor workspace client: %w", err) + } + } + + if o.ADXEnabled || o.ADXDeleteWhenDisabled { + grafanaClient, err = grafana.NewClient(ctx, cred, managedGrafanaClient, o.SubscriptionID, o.ResourceGroup, o.GrafanaName) + if err != nil { + return nil, fmt.Errorf("failed to create Grafana client: %w", err) + } + } } return &CompletedAddDatasourceOptions{ validatedAddDatasourceOptions: o.validatedAddDatasourceOptions, MonitorWorkspaceClient: monitorWorkspaceClient, ManagedGrafanaClient: managedGrafanaClient, + GrafanaClient: grafanaClient, }, nil } diff --git a/tools/grafanactl/internal/grafana/client.go b/tools/grafanactl/internal/grafana/client.go index f39ee4f7..9c376ff6 100644 --- a/tools/grafanactl/internal/grafana/client.go +++ b/tools/grafanactl/internal/grafana/client.go @@ -18,6 +18,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -188,7 +189,9 @@ func (c *Client) doGrafanaRequest(ctx context.Context, method, apiPath string, b if err != nil { return nil, fmt.Errorf("grafana API %s /%s request failed: %w", method, apiPath, err) } - defer resp.Body.Close() + defer func() { + _ = resp.Body.Close() + }() responseBody, err := io.ReadAll(resp.Body) if err != nil { @@ -202,19 +205,20 @@ func (c *Client) doGrafanaRequest(ctx context.Context, method, apiPath string, b } message := fmt.Sprintf("grafana API %s /%s failed with status %d: %s", method, apiPath, resp.StatusCode, strings.TrimSpace(string(responseBody))) - if resp.StatusCode == http.StatusUnauthorized { + switch resp.StatusCode { + case http.StatusUnauthorized: message += "; verify the caller's authentication token and Grafana endpoint configuration" - } else if resp.StatusCode == http.StatusForbidden { + case http.StatusForbidden: message += "; verify the caller has Grafana Admin permissions" } - return nil, fmt.Errorf("%s", message) + return nil, errors.New(message) } // DeleteDataSource removes a datasource from the Grafana instance by name. func (c *Client) DeleteDataSource(ctx context.Context, dataSourceName string) error { - _, err := c.grafanaClient.DeleteDatasourceByName(ctx, dataSourceName) - if err != nil { - return fmt.Errorf("failed to delete datasource: %w", err) + apiPath := path.Join("api/datasources/name", url.PathEscape(dataSourceName)) + if _, err := c.doGrafanaRequest(ctx, http.MethodDelete, apiPath, nil, http.StatusOK, http.StatusAccepted, http.StatusNotFound); err != nil { + return fmt.Errorf("failed to delete datasource %q: %w", dataSourceName, err) } return nil diff --git a/tools/grafanactl/internal/grafana/client_test.go b/tools/grafanactl/internal/grafana/client_test.go index e662459d..0822b2b4 100644 --- a/tools/grafanactl/internal/grafana/client_test.go +++ b/tools/grafanactl/internal/grafana/client_test.go @@ -134,3 +134,72 @@ func TestCreateDataSourceReportsUnauthorizedAsAuthError(t *testing.T) { t.Fatalf("did not expect Grafana Admin permissions error, got %v", err) } } + +func TestDeleteDataSourceUsesStatusAwareRequest(t *testing.T) { + var gotMethod string + var gotPath string + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotMethod = r.Method + gotPath = r.URL.Path + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"message":"deleted"}`)) + })) + defer server.Close() + + client := &Client{ + endpoint: server.URL, + token: "token", + httpClient: server.Client(), + } + + if err := client.DeleteDataSource(context.Background(), "kusto-int-uksouth"); err != nil { + t.Fatalf("DeleteDataSource returned error: %v", err) + } + if gotMethod != http.MethodDelete { + t.Fatalf("expected method %s, got %s", http.MethodDelete, gotMethod) + } + if gotPath != "/api/datasources/name/kusto-int-uksouth" { + t.Fatalf("expected path /api/datasources/name/kusto-int-uksouth, got %s", gotPath) + } +} + +func TestDeleteDataSourceIgnoresNotFound(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message":"not found"}`)) + })) + defer server.Close() + + client := &Client{ + endpoint: server.URL, + token: "token", + httpClient: server.Client(), + } + + if err := client.DeleteDataSource(context.Background(), "kusto-int-uksouth"); err != nil { + t.Fatalf("DeleteDataSource returned error: %v", err) + } +} + +func TestDeleteDataSourceReportsForbiddenAsAdminPermissionError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"message":"forbidden"}`)) + })) + defer server.Close() + + client := &Client{ + endpoint: server.URL, + token: "token", + httpClient: server.Client(), + } + + err := client.DeleteDataSource(context.Background(), "kusto-int-uksouth") + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "Grafana Admin permissions") { + t.Fatalf("expected Grafana Admin permissions error, got %v", err) + } +} From d74ea09220cc4547677eb8be93ce6060b72d84c9 Mon Sep 17 00:00:00 2001 From: swiencki Date: Thu, 30 Apr 2026 14:11:58 -0700 Subject: [PATCH 4/5] grafanactl: address datasource review comments --- tools/grafanactl/cmd/modify/adx.go | 14 ----- tools/grafanactl/cmd/modify/adx_options.go | 53 ------------------- .../grafanactl/cmd/modify/adx_options_test.go | 18 +++++++ tools/grafanactl/cmd/modify/options.go | 2 +- .../internal/grafana/client_test.go | 38 +++++++++++++ 5 files changed, 57 insertions(+), 68 deletions(-) diff --git a/tools/grafanactl/cmd/modify/adx.go b/tools/grafanactl/cmd/modify/adx.go index 03ebced7..e56891e0 100644 --- a/tools/grafanactl/cmd/modify/adx.go +++ b/tools/grafanactl/cmd/modify/adx.go @@ -24,20 +24,6 @@ import ( const adxDatasourceType = "grafana-azure-data-explorer-datasource" -func (opts *RawReconcileADXDatasourceOptions) Run(ctx context.Context) error { - validated, err := opts.Validate(ctx) - if err != nil { - return fmt.Errorf("validation failed: %w", err) - } - - completed, err := validated.Complete(ctx) - if err != nil { - return fmt.Errorf("completion failed: %w", err) - } - - return completed.Run(ctx) -} - func (o *CompletedReconcileADXDatasourceOptions) Run(ctx context.Context) error { logger := logr.FromContextOrDiscard(ctx).WithValues( "resource-group", o.ResourceGroup, diff --git a/tools/grafanactl/cmd/modify/adx_options.go b/tools/grafanactl/cmd/modify/adx_options.go index a7e2c50a..ef16636c 100644 --- a/tools/grafanactl/cmd/modify/adx_options.go +++ b/tools/grafanactl/cmd/modify/adx_options.go @@ -21,12 +21,8 @@ import ( "strings" "github.com/grafana-tools/sdk" - "github.com/spf13/cobra" - "github.com/Azure/ARO-Tools/tools/cmdutils" "github.com/Azure/ARO-Tools/tools/grafanactl/cmd/base" - "github.com/Azure/ARO-Tools/tools/grafanactl/internal/azure" - "github.com/Azure/ARO-Tools/tools/grafanactl/internal/grafana" ) const defaultADXDataConsistency = "strongconsistency" @@ -65,32 +61,6 @@ type CompletedReconcileADXDatasourceOptions struct { GrafanaClient adxGrafanaClient } -// DefaultReconcileADXDatasourceOptions returns a new RawReconcileADXDatasourceOptions with default values. -func DefaultReconcileADXDatasourceOptions() *RawReconcileADXDatasourceOptions { - return &RawReconcileADXDatasourceOptions{ - BaseOptions: base.DefaultBaseOptions(), - Enabled: true, - DataConsistency: defaultADXDataConsistency, - } -} - -// BindReconcileADXDatasourceOptions binds command-line flags to the options. -func BindReconcileADXDatasourceOptions(opts *RawReconcileADXDatasourceOptions, cmd *cobra.Command) error { - if err := base.BindBaseOptions(opts.BaseOptions, cmd); err != nil { - return err - } - - flags := cmd.Flags() - flags.BoolVar(&opts.Enabled, "enabled", opts.Enabled, "Reconcile the Azure Data Explorer datasource desired state as present") - flags.BoolVar(&opts.DeleteWhenDisabled, "delete-when-disabled", opts.DeleteWhenDisabled, "Delete the named Azure Data Explorer datasource when desired state is disabled") - flags.StringVar(&opts.ClusterURL, "cluster-url", opts.ClusterURL, "Azure Data Explorer cluster URL") - flags.StringVar(&opts.DefaultDatabase, "default-database", opts.DefaultDatabase, "Default Azure Data Explorer database") - flags.StringVar(&opts.DatasourceName, "datasource-name", opts.DatasourceName, "Grafana datasource name") - flags.StringVar(&opts.DataConsistency, "data-consistency", opts.DataConsistency, "Azure Data Explorer datasource data consistency") - - return nil -} - // Validate performs validation on the raw options. func (o *RawReconcileADXDatasourceOptions) Validate(ctx context.Context) (*ValidatedReconcileADXDatasourceOptions, error) { if err := base.ValidateBaseOptions(o.BaseOptions); err != nil { @@ -153,26 +123,3 @@ func (o *RawReconcileADXDatasourceOptions) Validate(ctx context.Context) (*Valid }, }, nil } - -// Complete performs final initialization to create fully usable ADX datasource reconcile options. -func (o *ValidatedReconcileADXDatasourceOptions) Complete(ctx context.Context) (*CompletedReconcileADXDatasourceOptions, error) { - cred, err := cmdutils.GetAzureTokenCredentials() - if err != nil { - return nil, fmt.Errorf("failed to obtain Azure credentials: %w", err) - } - - managedGrafanaClient, err := azure.NewManagedGrafanaClient(o.SubscriptionID, cred) - if err != nil { - return nil, fmt.Errorf("failed to create managed Grafana client: %w", err) - } - - grafanaClient, err := grafana.NewClient(ctx, cred, managedGrafanaClient, o.SubscriptionID, o.ResourceGroup, o.GrafanaName) - if err != nil { - return nil, fmt.Errorf("failed to create Grafana client: %w", err) - } - - return &CompletedReconcileADXDatasourceOptions{ - validatedReconcileADXDatasourceOptions: o.validatedReconcileADXDatasourceOptions, - GrafanaClient: grafanaClient, - }, nil -} diff --git a/tools/grafanactl/cmd/modify/adx_options_test.go b/tools/grafanactl/cmd/modify/adx_options_test.go index f9c1665a..6ff60062 100644 --- a/tools/grafanactl/cmd/modify/adx_options_test.go +++ b/tools/grafanactl/cmd/modify/adx_options_test.go @@ -399,3 +399,21 @@ func TestAddDatasourceValidateRejectsInvalidADXGeographies(t *testing.T) { t.Fatalf("expected invalid geography error, got %v", err) } } + +func TestAddDatasourceValidateIgnoresADXGeographiesWhenADXDisabled(t *testing.T) { + opts := DefaultAddDatasourceOptions() + opts.SubscriptionID = "subscription-id" + opts.ResourceGroup = "resource-group" + opts.GrafanaName = "grafana-name" + opts.AzureMonitorEnabled = false + opts.ADXEnabled = false + opts.ADXGeographies = "uks,!" + + validated, err := opts.Validate(context.Background()) + if err != nil { + t.Fatalf("Validate returned error: %v", err) + } + if validated.ADXEnabled { + t.Fatal("expected ADX to remain disabled") + } +} diff --git a/tools/grafanactl/cmd/modify/options.go b/tools/grafanactl/cmd/modify/options.go index 20c571e2..5e286ede 100644 --- a/tools/grafanactl/cmd/modify/options.go +++ b/tools/grafanactl/cmd/modify/options.go @@ -103,7 +103,7 @@ func (o *RawAddDatasourceOptions) Validate(ctx context.Context) (*ValidatedAddDa } normalized := *o - if strings.TrimSpace(normalized.ADXGeographies) != "" { + if normalized.ADXEnabled && strings.TrimSpace(normalized.ADXGeographies) != "" { allowed, err := adxGeographyAllowed(normalized.ADXGeographies, normalized.ADXCurrentGeography) if err != nil { return nil, err diff --git a/tools/grafanactl/internal/grafana/client_test.go b/tools/grafanactl/internal/grafana/client_test.go index 0822b2b4..2d8799c1 100644 --- a/tools/grafanactl/internal/grafana/client_test.go +++ b/tools/grafanactl/internal/grafana/client_test.go @@ -107,6 +107,44 @@ func TestUpdateDataSourceReportsForbiddenAsAdminPermissionError(t *testing.T) { } } +func TestListDataSourceTypesUsesStatusAwareRequest(t *testing.T) { + var gotAuthHeader string + var gotMethod string + var gotPath string + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotMethod = r.Method + gotPath = r.URL.Path + gotAuthHeader = r.Header.Get("Authorization") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"grafana-azure-data-explorer-datasource":{"name":"Azure Data Explorer","type":"grafana-azure-data-explorer-datasource"}}`)) + })) + defer server.Close() + + client := &Client{ + endpoint: server.URL, + token: "token", + httpClient: server.Client(), + } + + dataSourceTypes, err := client.ListDataSourceTypes(context.Background()) + if err != nil { + t.Fatalf("ListDataSourceTypes returned error: %v", err) + } + if gotMethod != http.MethodGet { + t.Fatalf("expected method %s, got %s", http.MethodGet, gotMethod) + } + if gotPath != "/api/datasources/plugins" { + t.Fatalf("expected path /api/datasources/plugins, got %s", gotPath) + } + if gotAuthHeader != "Bearer token" { + t.Fatalf("expected bearer token auth header, got %q", gotAuthHeader) + } + if dataSourceTypes["grafana-azure-data-explorer-datasource"].Name != "Azure Data Explorer" { + t.Fatalf("expected ADX datasource type to be decoded, got %#v", dataSourceTypes) + } +} + func TestCreateDataSourceReportsUnauthorizedAsAuthError(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusUnauthorized) From ab8153f5bf5278995f1ccaeec9cfa48092323ac5 Mon Sep 17 00:00:00 2001 From: swiencki Date: Wed, 20 May 2026 15:22:55 -0700 Subject: [PATCH 5/5] grafanactl: address review feedback --- pipelines/types/pipeline.schema.v1.json | 2 +- tools/grafanactl/cmd/modify/adx.go | 17 ++++++- .../grafanactl/cmd/modify/adx_options_test.go | 46 +++++++++++++++++++ 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/pipelines/types/pipeline.schema.v1.json b/pipelines/types/pipeline.schema.v1.json index ea6bf7fa..15c7061d 100644 --- a/pipelines/types/pipeline.schema.v1.json +++ b/pipelines/types/pipeline.schema.v1.json @@ -748,7 +748,7 @@ "$ref": "#/definitions/grafanaADXDatasource" }, "skipSync": { - "type": "string" + "type": "boolean" }, "identityFrom": { "$ref": "#/definitions/input" diff --git a/tools/grafanactl/cmd/modify/adx.go b/tools/grafanactl/cmd/modify/adx.go index e56891e0..e4739d32 100644 --- a/tools/grafanactl/cmd/modify/adx.go +++ b/tools/grafanactl/cmd/modify/adx.go @@ -59,7 +59,22 @@ func (o *CompletedReconcileADXDatasourceOptions) Run(ctx context.Context) error logger.Info("Creating ADX datasource") if err := o.GrafanaClient.CreateDataSource(ctx, desired); err != nil { - return fmt.Errorf("failed to create ADX datasource %q: %w", o.DatasourceName, err) + // Handle race condition: if another process created the + // datasource between our list and create calls, re-fetch + // and fall through to the update path. + existing, findErr := o.findExistingDatasource(ctx) + if findErr != nil || existing == nil { + return fmt.Errorf("failed to create ADX datasource %q: %w", o.DatasourceName, err) + } + logger.Info("Datasource was created concurrently, falling back to update", + "datasource-id", existing.ID, "datasource-uid", existing.UID) + desired.ID = existing.ID + desired.UID = existing.UID + desired.OrgID = existing.OrgID + desired.IsDefault = existing.IsDefault + if updateErr := o.GrafanaClient.UpdateDataSource(ctx, desired); updateErr != nil { + return fmt.Errorf("failed to update ADX datasource %q after create conflict: %w", o.DatasourceName, updateErr) + } } return nil } diff --git a/tools/grafanactl/cmd/modify/adx_options_test.go b/tools/grafanactl/cmd/modify/adx_options_test.go index 6ff60062..530134b2 100644 --- a/tools/grafanactl/cmd/modify/adx_options_test.go +++ b/tools/grafanactl/cmd/modify/adx_options_test.go @@ -36,6 +36,10 @@ type fakeADXGrafanaClient struct { createErr error updateErr error deleteErr error + // onCreateErr is called when createErr is non-nil, before returning. + // Use it to simulate race conditions (e.g. appending a datasource + // that a concurrent process created). + onCreateErr func(f *fakeADXGrafanaClient, ds sdk.Datasource) } func (f *fakeADXGrafanaClient) ListDataSources(ctx context.Context) ([]sdk.Datasource, error) { @@ -49,6 +53,9 @@ func (f *fakeADXGrafanaClient) ListDataSourceTypes(ctx context.Context) (map[str func (f *fakeADXGrafanaClient) CreateDataSource(ctx context.Context, dataSource sdk.Datasource) error { f.createCalls = append(f.createCalls, dataSource) if f.createErr != nil { + if f.onCreateErr != nil { + f.onCreateErr(f, dataSource) + } return f.createErr } if dataSource.UID == "" { @@ -171,6 +178,9 @@ func TestReconcileADXDatasourceRunCreatesDatasource(t *testing.T) { if created.Type != adxDatasourceType { t.Fatalf("expected datasource type %q, got %q", adxDatasourceType, created.Type) } + if created.Access != "proxy" { + t.Fatalf("expected Access %q, got %q", "proxy", created.Access) + } jsonData, ok := created.JSONData.(map[string]interface{}) if !ok { t.Fatalf("expected JSONData map, got %T", created.JSONData) @@ -220,6 +230,9 @@ func TestReconcileADXDatasourceRunUpdatesExistingDatasource(t *testing.T) { if updated.ID != 42 || updated.OrgID != 7 || updated.UID != "existing-uid" || !updated.IsDefault { t.Fatalf("expected existing datasource identity/default to be preserved, got %#v", updated) } + if updated.Access != "proxy" { + t.Fatalf("expected Access to be overwritten to %q, got %q", "proxy", updated.Access) + } } func TestReconcileADXDatasourceRunDeletesWhenDisabled(t *testing.T) { @@ -317,6 +330,39 @@ func TestReconcileADXDatasourceRunRejectsMissingPlugin(t *testing.T) { } } +func TestReconcileADXDatasourceRunCreateConflictFallsBackToUpdate(t *testing.T) { + // Simulates a race: create fails because another process created + // the datasource concurrently, but the datasource now exists on + // re-list, so we should fall back to update. + client := &fakeADXGrafanaClient{ + dataSourceTypes: map[string]sdk.DatasourceType{ + adxDatasourceType: {}, + }, + createErr: errors.New("status 409"), + onCreateErr: func(f *fakeADXGrafanaClient, ds sdk.Datasource) { + // Simulate the datasource appearing from a concurrent create + ds.ID = 99 + ds.UID = "conflict-uid" + f.dataSources = append(f.dataSources, ds) + }, + } + opts := completedReconcileADXDatasourceOptionsForTest(client) + + if err := opts.Run(context.Background()); err != nil { + t.Fatalf("expected fallback to update, got error: %v", err) + } + if len(client.createCalls) != 1 { + t.Fatalf("expected one create attempt, got %d", len(client.createCalls)) + } + if len(client.updateCalls) != 1 { + t.Fatalf("expected one update call after conflict, got %d", len(client.updateCalls)) + } + updated := client.updateCalls[0] + if updated.Access != "proxy" { + t.Fatalf("expected Access %q on fallback update, got %q", "proxy", updated.Access) + } +} + func TestReconcileADXDatasourceRunPropagatesMutationErrors(t *testing.T) { client := &fakeADXGrafanaClient{ dataSourceTypes: map[string]sdk.DatasourceType{