From a4744a8f5e5e8e06e5d575bd25360f4f5176ab78 Mon Sep 17 00:00:00 2001 From: Alyssa Bull <58453566+alyssabull@users.noreply.github.com> Date: Thu, 2 Nov 2023 14:30:18 -0600 Subject: [PATCH] Azure Monitor: buildResourceURI function incorrectly migrating (#77178) --- .../metrics/azuremonitor-datasource.go | 7 ++- pkg/tsdb/azuremonitor/metrics/url-builder.go | 22 ++++--- .../azuremonitor/metrics/url-builder_test.go | 62 +++++++++++++++---- 3 files changed, 69 insertions(+), 22 deletions(-) diff --git a/pkg/tsdb/azuremonitor/metrics/azuremonitor-datasource.go b/pkg/tsdb/azuremonitor/metrics/azuremonitor-datasource.go index cae6134a2b6..b3c9fcc88d7 100644 --- a/pkg/tsdb/azuremonitor/metrics/azuremonitor-datasource.go +++ b/pkg/tsdb/azuremonitor/metrics/azuremonitor-datasource.go @@ -112,7 +112,10 @@ func (e *AzureMonitorDatasource) buildQueries(queries []backend.DataQuery, dsInf azureURL = ub.BuildMetricsURL() // POST requests are only supported at the subscription level filterInBody = false - resourceUri := ub.buildResourceURI() + resourceUri, err := ub.buildResourceURI() + if err != nil { + return nil, err + } if resourceUri != nil { resourceMap[*resourceUri] = dataquery.AzureMonitorResource{ResourceGroup: resourceGroup, ResourceName: resourceName} } @@ -125,7 +128,7 @@ func (e *AzureMonitorDatasource) buildQueries(queries []backend.DataQuery, dsInf MetricNamespace: azJSONModel.MetricNamespace, ResourceName: r.ResourceName, } - resourceUri := ub.buildResourceURI() + resourceUri, _ := ub.buildResourceURI() if resourceUri != nil { resourceMap[*resourceUri] = r } diff --git a/pkg/tsdb/azuremonitor/metrics/url-builder.go b/pkg/tsdb/azuremonitor/metrics/url-builder.go index 2d38ce835a4..5785b94f976 100644 --- a/pkg/tsdb/azuremonitor/metrics/url-builder.go +++ b/pkg/tsdb/azuremonitor/metrics/url-builder.go @@ -15,11 +15,12 @@ type urlBuilder struct { ResourceGroup *string MetricNamespace *string ResourceName *string + MetricDefinition *string } -func (params *urlBuilder) buildResourceURI() *string { +func (params *urlBuilder) buildResourceURI() (*string, error) { if params.ResourceURI != nil && *params.ResourceURI != "" { - return params.ResourceURI + return params.ResourceURI, nil } subscription := params.Subscription @@ -28,11 +29,16 @@ func (params *urlBuilder) buildResourceURI() *string { subscription = params.DefaultSubscription } - if params.MetricNamespace == nil || *params.MetricNamespace == "" { - return nil + metricNamespace := params.MetricNamespace + + if metricNamespace == nil || *metricNamespace == "" { + if params.MetricDefinition == nil || *params.MetricDefinition == "" { + return nil, fmt.Errorf("no metricNamespace or metricDefiniton value provided") + } + metricNamespace = params.MetricDefinition } - metricNamespaceArray := strings.Split(*params.MetricNamespace, "/") + metricNamespaceArray := strings.Split(*metricNamespace, "/") var resourceNameArray []string if params.ResourceName != nil && *params.ResourceName != "" { resourceNameArray = strings.Split(*params.ResourceName, "/") @@ -40,7 +46,7 @@ func (params *urlBuilder) buildResourceURI() *string { provider := metricNamespaceArray[0] metricNamespaceArray = metricNamespaceArray[1:] - if strings.HasPrefix(strings.ToLower(*params.MetricNamespace), "microsoft.storage/storageaccounts/") && + if strings.HasPrefix(strings.ToLower(*metricNamespace), "microsoft.storage/storageaccounts/") && params.ResourceName != nil && !strings.HasSuffix(*params.ResourceName, "default") { resourceNameArray = append(resourceNameArray, "default") @@ -64,7 +70,7 @@ func (params *urlBuilder) buildResourceURI() *string { } resourceURI := strings.Join(urlArray, "/") - return &resourceURI + return &resourceURI, nil } // BuildMetricsURL checks the metric properties to see which form of the url @@ -74,7 +80,7 @@ func (params *urlBuilder) BuildMetricsURL() string { // Prior to Grafana 9, we had a legacy query object rather than a resourceURI, so we manually create the resource URI if resourceURI == nil || *resourceURI == "" { - resourceURI = params.buildResourceURI() + resourceURI, _ = params.buildResourceURI() } return fmt.Sprintf("%s/providers/microsoft.insights/metrics", *resourceURI) diff --git a/pkg/tsdb/azuremonitor/metrics/url-builder_test.go b/pkg/tsdb/azuremonitor/metrics/url-builder_test.go index e16385f6bfb..09e9a3a9b71 100644 --- a/pkg/tsdb/azuremonitor/metrics/url-builder_test.go +++ b/pkg/tsdb/azuremonitor/metrics/url-builder_test.go @@ -91,18 +91,56 @@ func TestURLBuilder(t *testing.T) { url := ub.BuildMetricsURL() assert.Equal(t, "/subscriptions/default-sub/resourceGroups/rg/providers/Microsoft.NetApp/netAppAccounts/rn1/capacityPools/rn2/volumes/rn3/providers/microsoft.insights/metrics", url) }) - - t.Run("when metric definition is Microsoft.Storage/storageAccounts/blobServices", func(t *testing.T) { - ub := &urlBuilder{ - DefaultSubscription: strPtr("default-sub"), - ResourceGroup: strPtr("rg"), - MetricNamespace: strPtr("Microsoft.Storage/storageAccounts/blobServices"), - ResourceName: strPtr("rn1"), - } - - url := *ub.buildResourceURI() - assert.Equal(t, "/subscriptions/default-sub/resourceGroups/rg/providers/Microsoft.Storage/storageAccounts/rn1/blobServices/default", url) - }) + }) + }) +} + +func TestBuildResourceURI(t *testing.T) { + t.Run("AzureMonitor Resource URI Builder", func(t *testing.T) { + t.Run("when there is no resource uri", func(t *testing.T) { + ub := &urlBuilder{ + DefaultSubscription: strPtr("default-sub"), + MetricDefinition: strPtr("Microsoft.Web/serverFarms"), + ResourceGroup: strPtr("rg"), + ResourceName: strPtr("rn1"), + } + + result, err := ub.buildResourceURI() + if err != nil { + return + } + url := *result + assert.Equal(t, "/subscriptions/default-sub/resourceGroups/rg/providers/Microsoft.Web/serverFarms/rn1", url) + }) + + t.Run("when metric definition is Microsoft.Storage/storageAccounts/blobServices", func(t *testing.T) { + ub := &urlBuilder{ + DefaultSubscription: strPtr("default-sub"), + ResourceGroup: strPtr("rg"), + MetricNamespace: strPtr("Microsoft.Storage/storageAccounts/blobServices"), + ResourceName: strPtr("rn1"), + } + + result, err := ub.buildResourceURI() + if err != nil { + return + } + url := *result + assert.Equal(t, "/subscriptions/default-sub/resourceGroups/rg/providers/Microsoft.Storage/storageAccounts/rn1/blobServices/default", url) + }) + + t.Run("when metricDefinition or metricNamespace is not defined an error is thrown", func(t *testing.T) { + ub := &urlBuilder{} + + _, err := ub.buildResourceURI() + if err == nil { + t.Errorf("Expected an error, but got nil") + } else { + expectedErrorMessage := "no metricNamespace or metricDefiniton value provided" + if err.Error() != expectedErrorMessage { + t.Errorf("Expected error message %s, but got %s", expectedErrorMessage, err.Error()) + } + } }) }) }