From c926d4a767ed747bf448da1fd8f31b88773128bd Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Thu, 15 Sep 2022 16:47:34 +0200 Subject: [PATCH 01/43] Lattice: Point to private prerelease of aws-sdk-go (#515) * point to private prerelease of aws-sdk-go * fix build issue --- go.mod | 2 ++ go.sum | 2 ++ 2 files changed, 4 insertions(+) diff --git a/go.mod b/go.mod index 50dbe4cff3c8..d2668d14a68c 100644 --- a/go.mod +++ b/go.mod @@ -394,3 +394,5 @@ replace github.com/prometheus/alertmanager => github.com/grafana/prometheus-aler replace google.golang.org/grpc => google.golang.org/grpc v1.45.0 replace google.golang.org/genproto => google.golang.org/genproto v0.0.0-20220421151946-72621c1f0bd3 + +replace github.com/aws/aws-sdk-go => github.com/grafana/aws-sdk-go-private v1.44.92 diff --git a/go.sum b/go.sum index e5f5e30bf67c..c97e5d274aea 100644 --- a/go.sum +++ b/go.sum @@ -1353,6 +1353,8 @@ github.com/gosimple/slug v1.12.0 h1:xzuhj7G7cGtd34NXnW/yF0l+AGNfWqwgh/IXgFy7dnc= github.com/gosimple/slug v1.12.0/go.mod h1:UiRaFH+GEilHstLUmcBgWcI42viBN7mAb818JrYOeFQ= github.com/gosimple/unidecode v1.0.1 h1:hZzFTMMqSswvf0LBJZCZgThIZrpDHFXux9KeGmn6T/o= github.com/gosimple/unidecode v1.0.1/go.mod h1:CP0Cr1Y1kogOtx0bJblKzsVWrqYaqfNOnHzpgWw4Awc= +github.com/grafana/aws-sdk-go-private v1.44.92 h1:pVbMmf7xFaRtIoScJ4YMiiqIRagIBtZ8+D916Xtc6BY= +github.com/grafana/aws-sdk-go-private v1.44.92/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo= github.com/grafana/codejen v0.0.3 h1:tAWxoTUuhgmEqxJPOLtJoxlPBbMULFwKFOcRsPRPXDw= github.com/grafana/codejen v0.0.3/go.mod h1:zmwwM/DRyQB7pfuBjTWII3CWtxcXh8LTwAYGfDfpR6s= github.com/grafana/cuetsy v0.1.1 h1:+1jaDDYCpvKlcOWJgBRbkc5+VZIClCEn5mbI+4PLZqM= From 45c11ffff1bd41ae92db17a665012fc0b6222913 Mon Sep 17 00:00:00 2001 From: Sarah Zinger Date: Tue, 4 Oct 2022 09:53:25 -0400 Subject: [PATCH 02/43] Lattice: Adding a feature toggle (#549) * Adding a feature toggle for lattice * Change name of feature toggle --- packages/grafana-data/src/types/featureToggles.gen.ts | 1 + pkg/services/featuremgmt/registry.go | 6 ++++++ pkg/services/featuremgmt/toggles_gen.go | 4 ++++ 3 files changed, 11 insertions(+) diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index c4b9823408cf..f859feeb71e7 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -69,6 +69,7 @@ export interface FeatureToggles { objectStore?: boolean; traceqlEditor?: boolean; flameGraph?: boolean; + cloudwatchCrossAccountQuerying?: boolean; redshiftAsyncQueryDataSupport?: boolean; athenaAsyncQueryDataSupport?: boolean; increaseInMemDatabaseQueryCache?: boolean; diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 62352cfe6a34..67d2dbfdf073 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -300,6 +300,12 @@ var ( Description: "Show the flame graph", State: FeatureStateAlpha, }, + { + Name: "cloudwatchCrossAccountQuerying", + Description: "cloudwatchCrossAccountQuerying features", + State: FeatureStateAlpha, + FrontendOnly: true, + }, { Name: "redshiftAsyncQueryDataSupport", Description: "Enable async query data support for Redshift", diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 2441a1ab0feb..45e87605ebbb 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -219,6 +219,10 @@ const ( // Show the flame graph FlagFlameGraph = "flameGraph" + // FlagCloudwatchCrossAccountQuerying + // cloudwatchCrossAccountQuerying features + FlagCloudwatchCrossAccountQuerying = "cloudwatchCrossAccountQuerying" + // FlagRedshiftAsyncQueryDataSupport // Enable async query data support for Redshift FlagRedshiftAsyncQueryDataSupport = "redshiftAsyncQueryDataSupport" From b1b62706a9a8ce81e0a8cbf413bf50ccc81e7166 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Mon, 10 Oct 2022 16:19:44 +0200 Subject: [PATCH 03/43] Lattice: List accounts (#543) * Separate layers * Introduce testify/mock library Co-authored-by: Shirley Leu <4163034+fridgepoet@users.noreply.github.com> --- go.mod | 2 +- go.sum | 33 +--- pkg/tsdb/cloudwatch/cloudwatch.go | 2 + pkg/tsdb/cloudwatch/mocks/accounts_service.go | 16 ++ pkg/tsdb/cloudwatch/mocks/oam_client.go | 20 +++ pkg/tsdb/cloudwatch/models/account.go | 24 +++ pkg/tsdb/cloudwatch/models/http_helpers.go | 22 +++ pkg/tsdb/cloudwatch/models/types.go | 1 + pkg/tsdb/cloudwatch/resource_handler.go | 2 +- pkg/tsdb/cloudwatch/routes/accounts.go | 65 +++++++ pkg/tsdb/cloudwatch/routes/accounts_test.go | 95 ++++++++++ pkg/tsdb/cloudwatch/services/accounts.go | 85 +++++++++ pkg/tsdb/cloudwatch/services/accounts_test.go | 165 ++++++++++++++++++ 13 files changed, 499 insertions(+), 33 deletions(-) create mode 100644 pkg/tsdb/cloudwatch/mocks/accounts_service.go create mode 100644 pkg/tsdb/cloudwatch/mocks/oam_client.go create mode 100644 pkg/tsdb/cloudwatch/models/account.go create mode 100644 pkg/tsdb/cloudwatch/models/http_helpers.go create mode 100644 pkg/tsdb/cloudwatch/routes/accounts.go create mode 100644 pkg/tsdb/cloudwatch/routes/accounts_test.go create mode 100644 pkg/tsdb/cloudwatch/services/accounts.go create mode 100644 pkg/tsdb/cloudwatch/services/accounts_test.go diff --git a/go.mod b/go.mod index d2668d14a68c..a18980529c49 100644 --- a/go.mod +++ b/go.mod @@ -395,4 +395,4 @@ replace google.golang.org/grpc => google.golang.org/grpc v1.45.0 replace google.golang.org/genproto => google.golang.org/genproto v0.0.0-20220421151946-72621c1f0bd3 -replace github.com/aws/aws-sdk-go => github.com/grafana/aws-sdk-go-private v1.44.92 +replace github.com/aws/aws-sdk-go => github.com/grafana/aws-sdk-go-private v1.44.93 diff --git a/go.sum b/go.sum index c97e5d274aea..11245231c8d7 100644 --- a/go.sum +++ b/go.sum @@ -349,32 +349,6 @@ github.com/asaskevich/govalidator v0.0.0-20200907205600-7a23bdc65eef/go.mod h1:W github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d h1:Byv0BzEl3/e6D5CLfI0j/7hiIEtvGVFPCZ7Ei2oq8iQ= github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw= github.com/aws/aws-lambda-go v1.13.3/go.mod h1:4UKl9IzQMoD+QF79YdCuzCwp8VbmG4VAQwij/eHl5CU= -github.com/aws/aws-sdk-go v1.15.11/go.mod h1:mFuSZ37Z9YOHbQEwBWztmVzqXrEkub65tZoCYDt7FT0= -github.com/aws/aws-sdk-go v1.15.27/go.mod h1:mFuSZ37Z9YOHbQEwBWztmVzqXrEkub65tZoCYDt7FT0= -github.com/aws/aws-sdk-go v1.15.78/go.mod h1:E3/ieXAlvM0XWO57iftYVDLLvQ824smPP3ATZkfNZeM= -github.com/aws/aws-sdk-go v1.17.7/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo= -github.com/aws/aws-sdk-go v1.22.4/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo= -github.com/aws/aws-sdk-go v1.25.48/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo= -github.com/aws/aws-sdk-go v1.27.0/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo= -github.com/aws/aws-sdk-go v1.29.16/go.mod h1:1KvfttTE3SPKMpo8g2c6jL3ZKfXtFvKscTgahTma5Xg= -github.com/aws/aws-sdk-go v1.30.12/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0= -github.com/aws/aws-sdk-go v1.31.9/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0= -github.com/aws/aws-sdk-go v1.33.5/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0= -github.com/aws/aws-sdk-go v1.33.12/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0= -github.com/aws/aws-sdk-go v1.34.9/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0= -github.com/aws/aws-sdk-go v1.34.28/go.mod h1:H7NKnBqNVzoTJpGfLrQkkD+ytBA93eiDYi/+8rV9s48= -github.com/aws/aws-sdk-go v1.35.5/go.mod h1:tlPOdRjfxPBpNIwqDj61rmsnA85v9jc0Ps9+muhnW+k= -github.com/aws/aws-sdk-go v1.35.31/go.mod h1:hcU610XS61/+aQV88ixoOzUoG7v3b31pl2zKMmprdro= -github.com/aws/aws-sdk-go v1.37.0/go.mod h1:hcU610XS61/+aQV88ixoOzUoG7v3b31pl2zKMmprdro= -github.com/aws/aws-sdk-go v1.37.8/go.mod h1:hcU610XS61/+aQV88ixoOzUoG7v3b31pl2zKMmprdro= -github.com/aws/aws-sdk-go v1.38.3/go.mod h1:hcU610XS61/+aQV88ixoOzUoG7v3b31pl2zKMmprdro= -github.com/aws/aws-sdk-go v1.38.35/go.mod h1:hcU610XS61/+aQV88ixoOzUoG7v3b31pl2zKMmprdro= -github.com/aws/aws-sdk-go v1.38.60/go.mod h1:hcU610XS61/+aQV88ixoOzUoG7v3b31pl2zKMmprdro= -github.com/aws/aws-sdk-go v1.38.68/go.mod h1:hcU610XS61/+aQV88ixoOzUoG7v3b31pl2zKMmprdro= -github.com/aws/aws-sdk-go v1.40.37/go.mod h1:585smgzpB/KqRA+K3y/NL/oYRqQvpNJYvLm+LY1U59Q= -github.com/aws/aws-sdk-go v1.43.31/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo= -github.com/aws/aws-sdk-go v1.44.109 h1:+Na5JPeS0kiEHoBp5Umcuuf+IDqXqD0lXnM920E31YI= -github.com/aws/aws-sdk-go v1.44.109/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo= github.com/aws/aws-sdk-go-v2 v0.18.0/go.mod h1:JWVYvqSMppoMJC0x5wdwiImzgXTI9FuZwxzkQq9wy+g= github.com/aws/aws-sdk-go-v2 v1.7.0/go.mod h1:tb9wi5s61kTDA5qCkcDbt3KRVV74GGslQkl/DRdX/P4= github.com/aws/aws-sdk-go-v2 v1.16.2 h1:fqlCk6Iy3bnCumtrLz9r3mJ/2gUT0pJ0wLFVIdWh+JA= @@ -890,7 +864,6 @@ github.com/go-git/go-git/v5 v5.4.2/go.mod h1:gQ1kArt6d+n+BGd+/B/I74HwRTLhth2+zti github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= -github.com/go-ini/ini v1.25.4/go.mod h1:ByCAeIL28uOIIG0E3PJtZPDL8WnHpFKFOtgjp+3Ies8= github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= github.com/go-kit/kit v0.9.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= github.com/go-kit/kit v0.10.0/go.mod h1:xUsJbQ/Fp4kEt7AFgCuvyX4a71u8h9jB8tj/ORgOZ7o= @@ -1353,8 +1326,8 @@ github.com/gosimple/slug v1.12.0 h1:xzuhj7G7cGtd34NXnW/yF0l+AGNfWqwgh/IXgFy7dnc= github.com/gosimple/slug v1.12.0/go.mod h1:UiRaFH+GEilHstLUmcBgWcI42viBN7mAb818JrYOeFQ= github.com/gosimple/unidecode v1.0.1 h1:hZzFTMMqSswvf0LBJZCZgThIZrpDHFXux9KeGmn6T/o= github.com/gosimple/unidecode v1.0.1/go.mod h1:CP0Cr1Y1kogOtx0bJblKzsVWrqYaqfNOnHzpgWw4Awc= -github.com/grafana/aws-sdk-go-private v1.44.92 h1:pVbMmf7xFaRtIoScJ4YMiiqIRagIBtZ8+D916Xtc6BY= -github.com/grafana/aws-sdk-go-private v1.44.92/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo= +github.com/grafana/aws-sdk-go-private v1.44.93 h1:sbfrB66WcygTR5yKAhvBYi0kNj3OWXgX72iFZ8fiY/4= +github.com/grafana/aws-sdk-go-private v1.44.93/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo= github.com/grafana/codejen v0.0.3 h1:tAWxoTUuhgmEqxJPOLtJoxlPBbMULFwKFOcRsPRPXDw= github.com/grafana/codejen v0.0.3/go.mod h1:zmwwM/DRyQB7pfuBjTWII3CWtxcXh8LTwAYGfDfpR6s= github.com/grafana/cuetsy v0.1.1 h1:+1jaDDYCpvKlcOWJgBRbkc5+VZIClCEn5mbI+4PLZqM= @@ -1621,10 +1594,8 @@ github.com/jessevdk/go-flags v1.5.0 h1:1jKYvbxEjfUl0fmqTCOfonvskHHXMjBySTLW4y9LF github.com/jessevdk/go-flags v1.5.0/go.mod h1:Fw0T6WPc1dYxT4mKEZRfG5kJhaTDP9pj1c2EWnYs/m4= github.com/jhump/protoreflect v1.6.0 h1:h5jfMVslIg6l29nsMs0D8Wj17RDVdNYti0vDN/PZZoE= github.com/jhump/protoreflect v1.6.0/go.mod h1:eaTn3RZAmMBcV0fifFvlm6VHNz3wSkYyXYWUh7ymB74= -github.com/jmespath/go-jmespath v0.0.0-20160202185014-0b12d6b521d8/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k= github.com/jmespath/go-jmespath v0.0.0-20160803190731-bd40a432e4c7/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k= github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k= -github.com/jmespath/go-jmespath v0.3.0/go.mod h1:9QtRXoHjLGCJ5IBSaohpXITPlowMeeYCZ7fLUTSywik= github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg= github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo= github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8= diff --git a/pkg/tsdb/cloudwatch/cloudwatch.go b/pkg/tsdb/cloudwatch/cloudwatch.go index e78dfb8318ff..a378f3f06352 100644 --- a/pkg/tsdb/cloudwatch/cloudwatch.go +++ b/pkg/tsdb/cloudwatch/cloudwatch.go @@ -18,6 +18,7 @@ import ( "github.com/aws/aws-sdk-go/service/cloudwatchlogs/cloudwatchlogsiface" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/ec2/ec2iface" + "github.com/aws/aws-sdk-go/service/oam" "github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi" "github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi/resourcegroupstaggingapiiface" "github.com/grafana/grafana-aws-sdk/pkg/awsds" @@ -122,6 +123,7 @@ func (e *cloudWatchExecutor) getRequestContext(pluginCtx backend.PluginContext, return models.RequestContext{}, err } return models.RequestContext{ + OAMClientProvider: oam.New(sess), MetricsClientProvider: clients.NewMetricsClient(NewMetricsAPI(sess), e.cfg), Settings: instance.Settings, }, nil diff --git a/pkg/tsdb/cloudwatch/mocks/accounts_service.go b/pkg/tsdb/cloudwatch/mocks/accounts_service.go new file mode 100644 index 000000000000..01c2e7914125 --- /dev/null +++ b/pkg/tsdb/cloudwatch/mocks/accounts_service.go @@ -0,0 +1,16 @@ +package mocks + +import ( + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" + "github.com/stretchr/testify/mock" +) + +type AccountsServiceMock struct { + mock.Mock +} + +func (a *AccountsServiceMock) GetAccountsForCurrentUserOrRole() ([]*models.Account, error) { + args := a.Called() + + return args.Get(0).([]*models.Account), args.Error(1) +} diff --git a/pkg/tsdb/cloudwatch/mocks/oam_client.go b/pkg/tsdb/cloudwatch/mocks/oam_client.go new file mode 100644 index 000000000000..2d617cc143a0 --- /dev/null +++ b/pkg/tsdb/cloudwatch/mocks/oam_client.go @@ -0,0 +1,20 @@ +package mocks + +import ( + "github.com/aws/aws-sdk-go/service/oam" + "github.com/stretchr/testify/mock" +) + +type FakeOAMClient struct { + mock.Mock +} + +func (o *FakeOAMClient) ListSinks(input *oam.ListSinksInput) (*oam.ListSinksOutput, error) { + args := o.Called(input) + return args.Get(0).(*oam.ListSinksOutput), args.Error(1) +} + +func (o *FakeOAMClient) ListAttachedLinks(input *oam.ListAttachedLinksInput) (*oam.ListAttachedLinksOutput, error) { + args := o.Called(input) + return args.Get(0).(*oam.ListAttachedLinksOutput), args.Error(1) +} diff --git a/pkg/tsdb/cloudwatch/models/account.go b/pkg/tsdb/cloudwatch/models/account.go new file mode 100644 index 000000000000..73a6702938ec --- /dev/null +++ b/pkg/tsdb/cloudwatch/models/account.go @@ -0,0 +1,24 @@ +package models + +import ( + "github.com/aws/aws-sdk-go/service/oam" +) + +type Account struct { + Arn string `json:"arn"` + Label string `json:"label"` + IsMonitoringAccount bool `json:"isMonitoringAccount"` +} + +type OAMClientProvider interface { + ListSinks(*oam.ListSinksInput) (*oam.ListSinksOutput, error) + ListAttachedLinks(*oam.ListAttachedLinksInput) (*oam.ListAttachedLinksOutput, error) +} + +type AccountsProvider interface { + GetAccountsForCurrentUserOrRole() ([]*Account, error) +} + +type ClientsProvider interface { + OAMClientProvider +} diff --git a/pkg/tsdb/cloudwatch/models/http_helpers.go b/pkg/tsdb/cloudwatch/models/http_helpers.go new file mode 100644 index 000000000000..850eb2eaf594 --- /dev/null +++ b/pkg/tsdb/cloudwatch/models/http_helpers.go @@ -0,0 +1,22 @@ +package models + +import "fmt" + +type HttpError struct { + Message string + Error string + StatusCode int +} + +func NewHttpError(message string, statusCode int, err error) *HttpError { + httpError := &HttpError{ + Message: message, + StatusCode: statusCode, + } + if err != nil { + httpError.Error = err.Error() + httpError.Message = fmt.Sprintf("%s: %s", message, err) + } + + return httpError +} diff --git a/pkg/tsdb/cloudwatch/models/types.go b/pkg/tsdb/cloudwatch/models/types.go index df41487234dd..15dc2efafbb3 100644 --- a/pkg/tsdb/cloudwatch/models/types.go +++ b/pkg/tsdb/cloudwatch/models/types.go @@ -8,6 +8,7 @@ import ( type RequestContext struct { MetricsClientProvider MetricsClientProvider + OAMClientProvider OAMClientProvider Settings CloudWatchSettings } diff --git a/pkg/tsdb/cloudwatch/resource_handler.go b/pkg/tsdb/cloudwatch/resource_handler.go index d9c88baebd85..5fc19c09901f 100644 --- a/pkg/tsdb/cloudwatch/resource_handler.go +++ b/pkg/tsdb/cloudwatch/resource_handler.go @@ -23,7 +23,7 @@ func (e *cloudWatchExecutor) newResourceMux() *http.ServeMux { mux.HandleFunc("/metrics", routes.ResourceRequestMiddleware(routes.MetricsHandler, logger, e.getRequestContext)) mux.HandleFunc("/dimension-values", routes.ResourceRequestMiddleware(routes.DimensionValuesHandler, logger, e.getRequestContext)) mux.HandleFunc("/dimension-keys", routes.ResourceRequestMiddleware(routes.DimensionKeysHandler, logger, e.getRequestContext)) - mux.HandleFunc("/namespaces", routes.ResourceRequestMiddleware(routes.NamespacesHandler, logger, e.getRequestContext)) + mux.HandleFunc("/accounts", routes.ResourceRequestMiddleware(routes.AccountsHandler, logger, e.getRequestContext)) return mux } diff --git a/pkg/tsdb/cloudwatch/routes/accounts.go b/pkg/tsdb/cloudwatch/routes/accounts.go new file mode 100644 index 000000000000..f2159173c1c7 --- /dev/null +++ b/pkg/tsdb/cloudwatch/routes/accounts.go @@ -0,0 +1,65 @@ +package routes + +import ( + "encoding/json" + "errors" + "net/http" + + "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/services" +) + +func AccountsHandler(rw http.ResponseWriter, req *http.Request, clientFactory models.ClientsFactoryFunc, pluginCtx backend.PluginContext) { + if req.Method != "GET" { + respondWithError(rw, http.StatusMethodNotAllowed, "Invalid method", nil) + return + } + + region := req.URL.Query().Get("region") + if region == "" { + respondWithError(rw, http.StatusBadRequest, "region missing", nil) + return + } + + service, err := newAccountsService(pluginCtx, clientFactory, region) + if err != nil { + respondWithError(rw, http.StatusInternalServerError, "error in AccountsHandler", err) + return + } + + accounts, err := service.GetAccountsForCurrentUserOrRole() + if err != nil { + msg := "error getting accounts for current user or role" + switch { + case errors.Is(err, services.ErrAccessDeniedException): + respondWithError(rw, http.StatusForbidden, msg, err) + default: + respondWithError(rw, http.StatusInternalServerError, msg, err) + } + return + } + + accountsResponse, err := json.Marshal(accounts) + if err != nil { + respondWithError(rw, http.StatusInternalServerError, "error in AccountsHandler", err) + } + + rw.Header().Set("Content-Type", "application/json") + _, err = rw.Write(accountsResponse) + if err != nil { + respondWithError(rw, http.StatusInternalServerError, "error writing response in AccountsHandler", err) + } +} + +// newAccountService is an account service factory. +// +// Stubbable by tests. +var newAccountsService = func(pluginCtx backend.PluginContext, clientFactory models.ClientsFactoryFunc, region string) (models.AccountsProvider, error) { + oamClient, err := clientFactory(pluginCtx, region) + if err != nil { + return nil, err + } + + return services.NewAccountsService(oamClient), nil +} diff --git a/pkg/tsdb/cloudwatch/routes/accounts_test.go b/pkg/tsdb/cloudwatch/routes/accounts_test.go new file mode 100644 index 000000000000..07f27bf683cb --- /dev/null +++ b/pkg/tsdb/cloudwatch/routes/accounts_test.go @@ -0,0 +1,95 @@ +package routes + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/mocks" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/services" + "github.com/stretchr/testify/assert" +) + +func Test_accounts_route(t *testing.T) { + origNewAccountsService := newAccountsService + t.Cleanup(func() { + newAccountsService = origNewAccountsService + }) + var clientFactoryMock = func(pluginCtx backend.PluginContext, region string) (clients models.ClientsProvider, err error) { + return nil, nil + } + + t.Run("successfully returns array of accounts json", func(t *testing.T) { + mockAccountsService := mocks.AccountsServiceMock{} + mockAccountsService.On("GetAccountsForCurrentUserOrRole").Return([]*models.Account{{ + Arn: "some arn", + Label: "some label", + IsMonitoringAccount: true, + }}, nil) + newAccountsService = func(pluginCtx backend.PluginContext, clientFactory models.ClientsFactoryFunc, region string) (models.AccountsProvider, error) { + return &mockAccountsService, nil + } + + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/accounts?region=us-east-1", nil) + handler := http.HandlerFunc(RouteInjector(AccountsHandler, clientFactoryMock)) + handler.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code) + assert.JSONEq(t, `[{"arn":"some arn", "isMonitoringAccount":true, "label":"some label"}]`, rr.Body.String()) + }) + + t.Run("rejects POST method", func(t *testing.T) { + rr := httptest.NewRecorder() + req := httptest.NewRequest("POST", "/accounts?region=us-east-1", nil) + handler := http.HandlerFunc(RouteInjector(AccountsHandler, clientFactoryMock)) + handler.ServeHTTP(rr, req) + assert.Equal(t, http.StatusMethodNotAllowed, rr.Code) + }) + + t.Run("requires region query value", func(t *testing.T) { + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/accounts", nil) + handler := http.HandlerFunc(RouteInjector(AccountsHandler, clientFactoryMock)) + handler.ServeHTTP(rr, req) + assert.Equal(t, http.StatusBadRequest, rr.Code) + }) + + t.Run("returns 403 when accounts service returns ErrAccessDeniedException", func(t *testing.T) { + mockAccountsService := mocks.AccountsServiceMock{} + mockAccountsService.On("GetAccountsForCurrentUserOrRole").Return([]*models.Account(nil), + fmt.Errorf("%w: %s", services.ErrAccessDeniedException, "some AWS message")) + newAccountsService = func(pluginCtx backend.PluginContext, clientFactory models.ClientsFactoryFunc, region string) (models.AccountsProvider, error) { + return &mockAccountsService, nil + } + + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/accounts?region=us-east-1", nil) + handler := http.HandlerFunc(RouteInjector(AccountsHandler, clientFactoryMock)) + handler.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusForbidden, rr.Code) + assert.JSONEq(t, + `{"Message":"error getting accounts for current user or role: access denied. please check your IAM policy: some AWS message", + "Error":"access denied. please check your IAM policy: some AWS message","StatusCode":403}`, rr.Body.String()) + }) + + t.Run("returns 500 when accounts service returns unknown error", func(t *testing.T) { + mockAccountsService := mocks.AccountsServiceMock{} + mockAccountsService.On("GetAccountsForCurrentUserOrRole").Return([]*models.Account(nil), fmt.Errorf("some error")) + newAccountsService = func(pluginCtx backend.PluginContext, clientFactory models.ClientsFactoryFunc, region string) (models.AccountsProvider, error) { + return &mockAccountsService, nil + } + + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/accounts?region=us-east-1", nil) + handler := http.HandlerFunc(RouteInjector(AccountsHandler, clientFactoryMock)) + handler.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusInternalServerError, rr.Code) + assert.Equal(t, `{"Message":"error getting accounts for current user or role: some error","Error":"some error","StatusCode":500}`, rr.Body.String()) + }) +} diff --git a/pkg/tsdb/cloudwatch/services/accounts.go b/pkg/tsdb/cloudwatch/services/accounts.go new file mode 100644 index 000000000000..ae5b1e50e2e3 --- /dev/null +++ b/pkg/tsdb/cloudwatch/services/accounts.go @@ -0,0 +1,85 @@ +package services + +import ( + "errors" + "fmt" + + "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/service/oam" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" +) + +var ErrAccessDeniedException = errors.New("access denied. please check your IAM policy") + +type AccountsService struct { + models.OAMClientProvider +} + +func NewAccountsService(oamClient models.OAMClientProvider) models.AccountsProvider { + return &AccountsService{oamClient} +} + +func (a *AccountsService) GetAccountsForCurrentUserOrRole() ([]*models.Account, error) { + var nextToken *string + sinks := []*oam.ListSinksItem{} + for { + response, err := a.ListSinks(&oam.ListSinksInput{NextToken: nextToken}) + if err != nil { + var aerr awserr.Error + if errors.As(err, &aerr) { + switch aerr.Code() { + // unlike many other services, OAM doesn't define this error code. however, it's returned in case calling role/user has insufficient permissions + case "AccessDeniedException": + return nil, fmt.Errorf("%w: %s", ErrAccessDeniedException, aerr.Message()) + } + } + } + if err != nil { + return nil, fmt.Errorf("ListSinks error: %w", err) + } + + sinks = append(sinks, response.Items...) + + if response.NextToken == nil { + break + } + nextToken = response.NextToken + } + + if len(sinks) == 0 { + return nil, nil + } + + sinkIdentifier := sinks[0].Arn + result := []*models.Account{{ + Label: *sinks[0].Name, + Arn: *sinks[0].Arn, + IsMonitoringAccount: true, + }} + + nextToken = nil + for { + links, err := a.ListAttachedLinks(&oam.ListAttachedLinksInput{ + SinkIdentifier: sinkIdentifier, + NextToken: nextToken, + }) + if err != nil { + return nil, fmt.Errorf("ListAttachedLinks error: %w", err) + } + + for _, link := range links.Items { + result = append(result, &models.Account{ + Label: *link.Label, + Arn: *link.LinkArn, + IsMonitoringAccount: false, + }) + } + + if links.NextToken == nil { + break + } + nextToken = links.NextToken + } + + return result, nil +} diff --git a/pkg/tsdb/cloudwatch/services/accounts_test.go b/pkg/tsdb/cloudwatch/services/accounts_test.go new file mode 100644 index 000000000000..e8e9ce2bd544 --- /dev/null +++ b/pkg/tsdb/cloudwatch/services/accounts_test.go @@ -0,0 +1,165 @@ +package services + +import ( + "fmt" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/service/oam" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/mocks" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestHandleGetAccounts(t *testing.T) { + t.Run("Should return an error in case of insufficient permissions from ListSinks", func(t *testing.T) { + fakeOAMClient := &mocks.FakeOAMClient{} + fakeOAMClient.On("ListSinks", mock.Anything).Return(&oam.ListSinksOutput{}, awserr.New("AccessDeniedException", + "AWS message", nil)) + accounts := NewAccountsService(fakeOAMClient) + + resp, err := accounts.GetAccountsForCurrentUserOrRole() + + assert.Error(t, err) + assert.Nil(t, resp) + assert.Equal(t, err.Error(), "access denied. please check your IAM policy: AWS message") + assert.ErrorIs(t, err, ErrAccessDeniedException) + }) + + t.Run("Should return an error in case of any error from ListSinks", func(t *testing.T) { + fakeOAMClient := &mocks.FakeOAMClient{} + fakeOAMClient.On("ListSinks", mock.Anything).Return(&oam.ListSinksOutput{}, fmt.Errorf("some error")) + accounts := NewAccountsService(fakeOAMClient) + + resp, err := accounts.GetAccountsForCurrentUserOrRole() + + assert.Error(t, err) + assert.Nil(t, resp) + assert.Equal(t, err.Error(), "ListSinks error: some error") + }) + + t.Run("Should return empty array in case no monitoring account exists", func(t *testing.T) { + fakeOAMClient := &mocks.FakeOAMClient{} + fakeOAMClient.On("ListSinks", mock.Anything).Return(&oam.ListSinksOutput{}, nil) + accounts := NewAccountsService(fakeOAMClient) + + resp, err := accounts.GetAccountsForCurrentUserOrRole() + + assert.NoError(t, err) + assert.Empty(t, resp) + }) + + t.Run("Should return one monitoring account (the first) even though ListSinks returns multiple sinks", func(t *testing.T) { + fakeOAMClient := &mocks.FakeOAMClient{} + fakeOAMClient.On("ListSinks", mock.Anything).Return(&oam.ListSinksOutput{ + Items: []*oam.ListSinksItem{ + {Name: aws.String("Account 1"), Arn: aws.String("arn:aws:logs:us-east-1:123456789012:log-group:my-log-group1")}, + {Name: aws.String("Account 2"), Arn: aws.String("arn:aws:logs:us-east-1:123456789012:log-group:my-log-group2")}, + }, + NextToken: new(string), + }, nil).Once() + fakeOAMClient.On("ListSinks", mock.Anything).Return(&oam.ListSinksOutput{ + Items: []*oam.ListSinksItem{ + {Name: aws.String("Account 3"), Arn: aws.String("arn:aws:logs:us-east-1:123456789012:log-group:my-log-group3")}, + }, + NextToken: nil, + }, nil) + fakeOAMClient.On("ListAttachedLinks", mock.Anything).Return(&oam.ListAttachedLinksOutput{}, nil) + accounts := NewAccountsService(fakeOAMClient) + + resp, err := accounts.GetAccountsForCurrentUserOrRole() + + assert.NoError(t, err) + fakeOAMClient.AssertNumberOfCalls(t, "ListSinks", 2) + require.Len(t, resp, 1) + assert.True(t, resp[0].IsMonitoringAccount) + assert.Equal(t, "Account 1", resp[0].Label) + assert.Equal(t, "arn:aws:logs:us-east-1:123456789012:log-group:my-log-group1", resp[0].Arn) + }) + + t.Run("Should merge the first sink with attached links", func(t *testing.T) { + fakeOAMClient := &mocks.FakeOAMClient{} + fakeOAMClient.On("ListSinks", mock.Anything).Return(&oam.ListSinksOutput{ + Items: []*oam.ListSinksItem{ + {Name: aws.String("Account 1"), Arn: aws.String("arn:aws:logs:us-east-1:123456789012:log-group:my-log-group1")}, + {Name: aws.String("Account 2"), Arn: aws.String("arn:aws:logs:us-east-1:123456789012:log-group:my-log-group2")}, + }, + NextToken: new(string), + }, nil).Once() + fakeOAMClient.On("ListSinks", mock.Anything).Return(&oam.ListSinksOutput{ + Items: []*oam.ListSinksItem{ + {Name: aws.String("Account 3"), Arn: aws.String("arn:aws:logs:us-east-1:123456789012:log-group:my-log-group3")}, + }, + NextToken: nil, + }, nil) + fakeOAMClient.On("ListAttachedLinks", mock.Anything).Return(&oam.ListAttachedLinksOutput{ + Items: []*oam.ListAttachedLinksItem{ + {Label: aws.String("Account 10"), LinkArn: aws.String("arn:aws:logs:us-east-1:123456789012:log-group:my-log-group10")}, + {Label: aws.String("Account 11"), LinkArn: aws.String("arn:aws:logs:us-east-1:123456789012:log-group:my-log-group11")}, + }, + NextToken: new(string), + }, nil).Once() + fakeOAMClient.On("ListAttachedLinks", mock.Anything).Return(&oam.ListAttachedLinksOutput{ + Items: []*oam.ListAttachedLinksItem{ + {Label: aws.String("Account 12"), LinkArn: aws.String("arn:aws:logs:us-east-1:123456789012:log-group:my-log-group12")}, + }, + NextToken: nil, + }, nil) + accounts := NewAccountsService(fakeOAMClient) + + resp, err := accounts.GetAccountsForCurrentUserOrRole() + + assert.NoError(t, err) + fakeOAMClient.AssertNumberOfCalls(t, "ListSinks", 2) + fakeOAMClient.AssertNumberOfCalls(t, "ListAttachedLinks", 2) + expectedAccounts := []*models.Account{ + {Label: "Account 1", Arn: "arn:aws:logs:us-east-1:123456789012:log-group:my-log-group1", IsMonitoringAccount: true}, + {Label: "Account 10", Arn: "arn:aws:logs:us-east-1:123456789012:log-group:my-log-group10", IsMonitoringAccount: false}, + {Label: "Account 11", Arn: "arn:aws:logs:us-east-1:123456789012:log-group:my-log-group11", IsMonitoringAccount: false}, + {Label: "Account 12", Arn: "arn:aws:logs:us-east-1:123456789012:log-group:my-log-group12", IsMonitoringAccount: false}, + } + assert.Equal(t, expectedAccounts, resp) + }) + + t.Run("Should call ListAttachedLinks with arn of first sink", func(t *testing.T) { + fakeOAMClient := &mocks.FakeOAMClient{} + fakeOAMClient.On("ListSinks", mock.Anything).Return(&oam.ListSinksOutput{ + Items: []*oam.ListSinksItem{ + {Name: aws.String("Account 1"), Arn: aws.String("arn:aws:logs:us-east-1:123456789012:log-group:my-log-group1")}, + }, + NextToken: new(string), + }, nil).Once() + fakeOAMClient.On("ListSinks", mock.Anything).Return(&oam.ListSinksOutput{ + Items: []*oam.ListSinksItem{ + {Name: aws.String("Account 3"), Arn: aws.String("arn:aws:logs:us-east-1:123456789012:log-group:my-log-group3")}, + }, + NextToken: nil, + }, nil).Once() + fakeOAMClient.On("ListAttachedLinks", mock.Anything).Return(&oam.ListAttachedLinksOutput{}, nil) + accounts := NewAccountsService(fakeOAMClient) + + _, _ = accounts.GetAccountsForCurrentUserOrRole() + + fakeOAMClient.AssertCalled(t, "ListAttachedLinks", &oam.ListAttachedLinksInput{ + SinkIdentifier: aws.String("arn:aws:logs:us-east-1:123456789012:log-group:my-log-group1"), + }) + }) + + t.Run("Should return an error in case of any error from ListAttachedLinks", func(t *testing.T) { + fakeOAMClient := &mocks.FakeOAMClient{} + fakeOAMClient.On("ListSinks", mock.Anything).Return(&oam.ListSinksOutput{ + Items: []*oam.ListSinksItem{{Name: aws.String("Account 1"), Arn: aws.String("arn:aws:logs:us-east-1:123456789012:log-group:my-log-group1")}}, + }, nil) + fakeOAMClient.On("ListAttachedLinks", mock.Anything).Return(&oam.ListAttachedLinksOutput{}, fmt.Errorf("some error")).Once() + accounts := NewAccountsService(fakeOAMClient) + + resp, err := accounts.GetAccountsForCurrentUserOrRole() + + assert.Error(t, err) + assert.Nil(t, resp) + assert.Equal(t, err.Error(), "ListAttachedLinks error: some error") + }) +} From b33f728b33948a6e198dcca94b90fe8e066a5d10 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Wed, 12 Oct 2022 16:03:19 +0200 Subject: [PATCH 04/43] point to version that includes metric api changes (#574) --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index a18980529c49..bc6eecfc3aec 100644 --- a/go.mod +++ b/go.mod @@ -395,4 +395,4 @@ replace google.golang.org/grpc => google.golang.org/grpc v1.45.0 replace google.golang.org/genproto => google.golang.org/genproto v0.0.0-20220421151946-72621c1f0bd3 -replace github.com/aws/aws-sdk-go => github.com/grafana/aws-sdk-go-private v1.44.93 +replace github.com/aws/aws-sdk-go => github.com/grafana/aws-sdk-go-private v1.44.94 diff --git a/go.sum b/go.sum index 11245231c8d7..6903e947873f 100644 --- a/go.sum +++ b/go.sum @@ -1326,8 +1326,8 @@ github.com/gosimple/slug v1.12.0 h1:xzuhj7G7cGtd34NXnW/yF0l+AGNfWqwgh/IXgFy7dnc= github.com/gosimple/slug v1.12.0/go.mod h1:UiRaFH+GEilHstLUmcBgWcI42viBN7mAb818JrYOeFQ= github.com/gosimple/unidecode v1.0.1 h1:hZzFTMMqSswvf0LBJZCZgThIZrpDHFXux9KeGmn6T/o= github.com/gosimple/unidecode v1.0.1/go.mod h1:CP0Cr1Y1kogOtx0bJblKzsVWrqYaqfNOnHzpgWw4Awc= -github.com/grafana/aws-sdk-go-private v1.44.93 h1:sbfrB66WcygTR5yKAhvBYi0kNj3OWXgX72iFZ8fiY/4= -github.com/grafana/aws-sdk-go-private v1.44.93/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo= +github.com/grafana/aws-sdk-go-private v1.44.94 h1:TqxTqy31NUTzPvK0QIL/XAoxCe11soViiF/slzAa3pQ= +github.com/grafana/aws-sdk-go-private v1.44.94/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo= github.com/grafana/codejen v0.0.3 h1:tAWxoTUuhgmEqxJPOLtJoxlPBbMULFwKFOcRsPRPXDw= github.com/grafana/codejen v0.0.3/go.mod h1:zmwwM/DRyQB7pfuBjTWII3CWtxcXh8LTwAYGfDfpR6s= github.com/grafana/cuetsy v0.1.1 h1:+1jaDDYCpvKlcOWJgBRbkc5+VZIClCEn5mbI+4PLZqM= From e086fc5f34e08193e20e32d34e1ece9d7756aa01 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Mon, 17 Oct 2022 10:25:25 +0200 Subject: [PATCH 05/43] add accounts component (#575) --- .../app/plugins/datasource/cloudwatch/api.ts | 7 + .../cloudwatch/components/Account.test.tsx | 161 ++++++++++++++++++ .../cloudwatch/components/Account.tsx | 73 ++++++++ .../components/AnnotationQueryEditor.test.tsx | 7 + .../MetricStatEditor/MetricStatEditor.tsx | 16 +- .../plugins/datasource/cloudwatch/types.ts | 13 ++ 6 files changed, 272 insertions(+), 5 deletions(-) create mode 100644 public/app/plugins/datasource/cloudwatch/components/Account.test.tsx create mode 100644 public/app/plugins/datasource/cloudwatch/components/Account.tsx diff --git a/public/app/plugins/datasource/cloudwatch/api.ts b/public/app/plugins/datasource/cloudwatch/api.ts index a6342b114992..0289b1fba578 100644 --- a/public/app/plugins/datasource/cloudwatch/api.ts +++ b/public/app/plugins/datasource/cloudwatch/api.ts @@ -13,6 +13,7 @@ import { GetMetricsRequest, MetricResponse, MultiFilters, + Account, } from './types'; export interface SelectableResourceValue extends SelectableValue { @@ -33,6 +34,12 @@ export class CloudWatchAPI extends CloudWatchRequest { return getBackendSrv().get(`/api/datasources/${this.instanceSettings.id}/resources/${subtype}`, parameters); } + getAccounts(region: string): Promise { + return getBackendSrv().get(`/api/datasources/${this.instanceSettings.id}/resources/accounts`, { + region: this.templateSrv.replace(region), + }); + } + getRegions() { return this.memoizedGetRequest('regions').then((regions) => [ { label: 'default', value: 'default', text: 'default' }, diff --git a/public/app/plugins/datasource/cloudwatch/components/Account.test.tsx b/public/app/plugins/datasource/cloudwatch/components/Account.test.tsx new file mode 100644 index 000000000000..9def8c0f29af --- /dev/null +++ b/public/app/plugins/datasource/cloudwatch/components/Account.test.tsx @@ -0,0 +1,161 @@ +import { act, render, screen } from '@testing-library/react'; +import React from 'react'; + +import { config } from '@grafana/runtime'; + +import { setupMockedAPI } from '../__mocks__/API'; +import { MetricStat } from '../types'; + +import { Account } from './Account'; + +const accounts = [ + { + arn: 'arn:aws:iam::123456789012:root', + accountId: '123456789012', + label: 'test-account', + isMonitoringAccount: true, + }, + { + arn: 'arn:aws:iam::432156789012:root', + accountId: '432156789012', + label: 'test-account2', + isMonitoringAccount: false, + }, + { + arn: 'arn:aws:iam::987656789012:root', + accountId: '432156789012', + label: 'test-account3', + isMonitoringAccount: false, + }, +]; + +describe('Account', () => { + const metricStat: MetricStat = { + region: 'us-east-2', + namespace: '', + metricName: '', + dimensions: {}, + statistic: '', + matchExact: true, + accountArn: 'arn:aws:iam::123456789012:root', + }; + + const props = { + api: setupMockedAPI().api, + metricStat, + onChange: jest.fn(), + }; + + describe('account field', () => { + config.featureToggles.cloudwatchCrossAccountQuerying = true; + it('should be rendered when feature toggle is enabled', async () => { + const originalValue = config.featureToggles.cloudwatchCrossAccountQuerying; + config.featureToggles.cloudwatchCrossAccountQuerying = true; + const api = setupMockedAPI().api; + api.getAccounts = jest.fn().mockResolvedValue([ + { + arn: 'arn:aws:iam::123456789012:root', + accountId: '123456789012', + label: 'test-account', + isMonitoringAccount: true, + }, + ]); + await act(async () => { + render(); + }); + expect(await screen.getByLabelText('Account')).toBeInTheDocument(); + config.featureToggles.cloudwatchCrossAccountQuerying = originalValue; + }); + + it('should not be rendered when feature toggle is not enabled', async () => { + let api = setupMockedAPI().api; + api.getAccounts = jest.fn().mockResolvedValue(accounts); + const originalValue = config.featureToggles.cloudwatchCrossAccountQuerying; + config.featureToggles.cloudwatchCrossAccountQuerying = false; + await act(async () => { + const { container } = render(); + expect(container).toBeEmptyDOMElement(); + config.featureToggles.cloudwatchCrossAccountQuerying = originalValue; + }); + }); + + it('should not be rendered when no accounts are found', async () => { + await act(async () => { + const mock = setupMockedAPI(); + mock.api.getAccounts = jest.fn().mockResolvedValue([]); + const { container } = render(); + expect(container).toBeEmptyDOMElement(); + }); + }); + + it('should not be rendered and should unset accountArn when api returns an error', async () => { + const mock = setupMockedAPI(); + mock.api.getAccounts = jest.fn().mockRejectedValue(new Error('error')); + const onChange = jest.fn(); + await act(async () => { + const { container } = render(); + expect(container).toBeEmptyDOMElement(); + }); + expect(onChange).toHaveBeenCalledWith(undefined); + }); + + it('should not be rendered and should unset accountArn when api returns an empty list of accounts', async () => { + const mock = setupMockedAPI(); + mock.api.getAccounts = jest.fn().mockResolvedValue([]); + const onChange = jest.fn(); + await act(async () => { + const { container } = render( + + ); + expect(container).toBeEmptyDOMElement(); + }); + expect(onChange).toHaveBeenCalledWith(undefined); + }); + + it('should set accountArn to "all" if the current value is not in the loaded list of accounts', async () => { + const api = setupMockedAPI().api; + api.getAccounts = jest.fn().mockResolvedValue(accounts); + const onChange = jest.fn(); + await act(async () => { + render( + + ); + }); + expect(onChange).toHaveBeenCalledWith('all'); + }); + + it('should not unset accountArn if the current value is in the loaded list of accounts', async () => { + const api = setupMockedAPI().api; + api.getAccounts = jest.fn().mockResolvedValue(accounts); + const onChange = jest.fn(); + await act(async () => { + render( + + ); + }); + expect(onChange).not.toHaveBeenCalled(); + expect(await screen.getByText('test-account2')).toBeInTheDocument(); + }); + + it('should add "Monitoring account" text to the display label if its a monitoring account', async () => { + const api = setupMockedAPI().api; + api.getAccounts = jest.fn().mockResolvedValue(accounts); + await act(async () => { + render(); + }); + expect(await screen.getByText('test-account (Monitoring account)')).toBeInTheDocument(); + }); + }); +}); diff --git a/public/app/plugins/datasource/cloudwatch/components/Account.tsx b/public/app/plugins/datasource/cloudwatch/components/Account.tsx new file mode 100644 index 000000000000..7fbee430db3e --- /dev/null +++ b/public/app/plugins/datasource/cloudwatch/components/Account.tsx @@ -0,0 +1,73 @@ +import React, { useEffect } from 'react'; +import { useAsyncFn } from 'react-use'; + +import { SelectableValue } from '@grafana/data'; +import { config } from '@grafana/runtime'; +import { EditorField, Select } from '@grafana/ui'; + +import { CloudWatchAPI } from '../api'; +import { MetricStat } from '../types'; + +export interface Props { + onChange: (accountId?: string) => void; + query: MetricStat; + api: CloudWatchAPI; +} + +const allOption: SelectableValue = { + label: 'All', + value: 'all', + description: 'Target all linked accounts', +}; + +export function Account({ query, onChange, api }: Props) { + const fetchAccounts = () => + api + .getAccounts(query.region) + .then((accounts) => { + const options = accounts.map((a) => ({ + label: `${a.label}${a.isMonitoringAccount ? ' (Monitoring account)' : ''}`, + value: a.arn, + description: a.accountId, + })); + + if (!options.find((o) => o.value === query.accountArn)) { + onChange(options?.length ? 'all' : undefined); + } + return options.length ? [allOption, ...options] : options; + }) + .catch(() => { + query.accountArn && onChange(undefined); + return []; + }); + + const [state, doFetch] = useAsyncFn(fetchAccounts, [api, query.region]); + + useEffect(() => { + if (config.featureToggles.cloudwatchCrossAccountQuerying) { + doFetch(); + } + }, [api, query.region, doFetch]); + + if (!config.featureToggles.cloudwatchCrossAccountQuerying) { + return null; + } + + if (!state.value?.length) { + return null; + } + + return ( + + a.value === query.accountArn)} + value={value} options={state.value} - onChange={({ value: account }) => { - account && onChange(account); + onChange={({ value: accountArn }) => { + accountArn && + onChange({ + crossAccount: false, + account: accounts.find((a) => a.arn === accountArn), + }); }} /> diff --git a/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.tsx b/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.tsx index d4f113505aaa..7996a5bafdab 100644 --- a/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.tsx @@ -8,7 +8,7 @@ import { Dimensions } from '..'; import { CloudWatchDatasource } from '../../datasource'; import { useDimensionKeys, useMetrics, useNamespaces } from '../../hooks'; import { standardStatistics } from '../../standardStatistics'; -import { MetricStat } from '../../types'; +import { MetricStat, AccountInfo } from '../../types'; import { appendTemplateVariables, toOption } from '../../utils/utils'; import { Account } from '../Account'; @@ -118,7 +118,7 @@ export function MetricStatEditor({ {!disableExpressions && ( onMetricStatChange({ ...metricStat, accountArn })} + onChange={(accountInfo?: AccountInfo) => onMetricStatChange({ ...metricStat, accountInfo })} api={datasource.api} > )} diff --git a/public/app/plugins/datasource/cloudwatch/types.ts b/public/app/plugins/datasource/cloudwatch/types.ts index e235c0377c65..82c8bd862338 100644 --- a/public/app/plugins/datasource/cloudwatch/types.ts +++ b/public/app/plugins/datasource/cloudwatch/types.ts @@ -57,6 +57,11 @@ export interface CloudWatchMetricsQuery extends MetricStat, DataQuery { sql?: SQLExpression; } +export interface AccountInfo { + crossAccount: boolean; + account?: Account; +} + export interface MetricStat { region: string; namespace: string; @@ -64,7 +69,7 @@ export interface MetricStat { dimensions?: Dimensions; matchExact?: boolean; period?: string; - accountArn?: string; + accountInfo?: AccountInfo; statistic?: string; /** * @deprecated use statistic @@ -480,7 +485,7 @@ export interface GetMetricsRequest extends ResourceRequest { export interface Account { arn: string; - accountId: string; + id: string; label: string; isMonitoringAccount: boolean; } From 798bc586afecc83e72948beefaad2df25758ebc5 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Thu, 27 Oct 2022 10:52:00 +0200 Subject: [PATCH 13/43] add back namespaces handler --- pkg/tsdb/cloudwatch/resource_handler.go | 1 + pkg/tsdb/cloudwatch/routes/accounts_test.go | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/tsdb/cloudwatch/resource_handler.go b/pkg/tsdb/cloudwatch/resource_handler.go index 5fc19c09901f..aae9c9db1196 100644 --- a/pkg/tsdb/cloudwatch/resource_handler.go +++ b/pkg/tsdb/cloudwatch/resource_handler.go @@ -24,6 +24,7 @@ func (e *cloudWatchExecutor) newResourceMux() *http.ServeMux { mux.HandleFunc("/dimension-values", routes.ResourceRequestMiddleware(routes.DimensionValuesHandler, logger, e.getRequestContext)) mux.HandleFunc("/dimension-keys", routes.ResourceRequestMiddleware(routes.DimensionKeysHandler, logger, e.getRequestContext)) mux.HandleFunc("/accounts", routes.ResourceRequestMiddleware(routes.AccountsHandler, logger, e.getRequestContext)) + mux.HandleFunc("/namespaces", routes.ResourceRequestMiddleware(routes.NamespacesHandler, logger, e.getRequestContext)) return mux } diff --git a/pkg/tsdb/cloudwatch/routes/accounts_test.go b/pkg/tsdb/cloudwatch/routes/accounts_test.go index 6f2d7c317d4f..0247fa937d81 100644 --- a/pkg/tsdb/cloudwatch/routes/accounts_test.go +++ b/pkg/tsdb/cloudwatch/routes/accounts_test.go @@ -32,7 +32,7 @@ func Test_accounts_route(t *testing.T) { rr := httptest.NewRecorder() req := httptest.NewRequest("GET", "/accounts?region=us-east-1", nil) - handler := http.HandlerFunc(ResourceRequestMiddleware(AccountsHandler, nil)) + handler := http.HandlerFunc(ResourceRequestMiddleware(AccountsHandler, logger, nil)) handler.ServeHTTP(rr, req) assert.Equal(t, http.StatusOK, rr.Code) @@ -42,7 +42,7 @@ func Test_accounts_route(t *testing.T) { t.Run("rejects POST method", func(t *testing.T) { rr := httptest.NewRecorder() req := httptest.NewRequest("POST", "/accounts?region=us-east-1", nil) - handler := http.HandlerFunc(ResourceRequestMiddleware(AccountsHandler, nil)) + handler := http.HandlerFunc(ResourceRequestMiddleware(AccountsHandler, logger, nil)) handler.ServeHTTP(rr, req) assert.Equal(t, http.StatusMethodNotAllowed, rr.Code) }) @@ -50,7 +50,7 @@ func Test_accounts_route(t *testing.T) { t.Run("requires region query value", func(t *testing.T) { rr := httptest.NewRecorder() req := httptest.NewRequest("GET", "/accounts", nil) - handler := http.HandlerFunc(ResourceRequestMiddleware(AccountsHandler, nil)) + handler := http.HandlerFunc(ResourceRequestMiddleware(AccountsHandler, logger, nil)) handler.ServeHTTP(rr, req) assert.Equal(t, http.StatusBadRequest, rr.Code) }) @@ -65,7 +65,7 @@ func Test_accounts_route(t *testing.T) { rr := httptest.NewRecorder() req := httptest.NewRequest("GET", "/accounts?region=us-east-1", nil) - handler := http.HandlerFunc(ResourceRequestMiddleware(AccountsHandler, nil)) + handler := http.HandlerFunc(ResourceRequestMiddleware(AccountsHandler, logger, nil)) handler.ServeHTTP(rr, req) assert.Equal(t, http.StatusForbidden, rr.Code) @@ -83,7 +83,7 @@ func Test_accounts_route(t *testing.T) { rr := httptest.NewRecorder() req := httptest.NewRequest("GET", "/accounts?region=us-east-1", nil) - handler := http.HandlerFunc(ResourceRequestMiddleware(AccountsHandler, nil)) + handler := http.HandlerFunc(ResourceRequestMiddleware(AccountsHandler, logger, nil)) handler.ServeHTTP(rr, req) assert.Equal(t, http.StatusInternalServerError, rr.Code) From b90384694c8a281a4314fafea9925f9928c9056f Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Thu, 27 Oct 2022 14:47:10 +0200 Subject: [PATCH 14/43] Lattice: Parse account id and return it to frontend (#609) * parse account id and return to frontend * fix route test --- pkg/tsdb/cloudwatch/models/account.go | 1 + pkg/tsdb/cloudwatch/routes/accounts_test.go | 3 ++- pkg/tsdb/cloudwatch/services/accounts.go | 19 +++++++++++++++++-- pkg/tsdb/cloudwatch/services/accounts_test.go | 12 ++++++------ 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/pkg/tsdb/cloudwatch/models/account.go b/pkg/tsdb/cloudwatch/models/account.go index 73a6702938ec..9bff0ee71593 100644 --- a/pkg/tsdb/cloudwatch/models/account.go +++ b/pkg/tsdb/cloudwatch/models/account.go @@ -5,6 +5,7 @@ import ( ) type Account struct { + Id string `json:"id"` Arn string `json:"arn"` Label string `json:"label"` IsMonitoringAccount bool `json:"isMonitoringAccount"` diff --git a/pkg/tsdb/cloudwatch/routes/accounts_test.go b/pkg/tsdb/cloudwatch/routes/accounts_test.go index 0247fa937d81..e2ab92d87819 100644 --- a/pkg/tsdb/cloudwatch/routes/accounts_test.go +++ b/pkg/tsdb/cloudwatch/routes/accounts_test.go @@ -22,6 +22,7 @@ func Test_accounts_route(t *testing.T) { t.Run("successfully returns array of accounts json", func(t *testing.T) { mockAccountsService := mocks.AccountsServiceMock{} mockAccountsService.On("GetAccountsForCurrentUserOrRole").Return([]*models.Account{{ + Id: "123456789012", Arn: "some arn", Label: "some label", IsMonitoringAccount: true, @@ -36,7 +37,7 @@ func Test_accounts_route(t *testing.T) { handler.ServeHTTP(rr, req) assert.Equal(t, http.StatusOK, rr.Code) - assert.JSONEq(t, `[{"arn":"some arn", "isMonitoringAccount":true, "label":"some label"}]`, rr.Body.String()) + assert.JSONEq(t, `[{"id":"123456789012", "arn":"some arn", "isMonitoringAccount":true, "label":"some label"}]`, rr.Body.String()) }) t.Run("rejects POST method", func(t *testing.T) { diff --git a/pkg/tsdb/cloudwatch/services/accounts.go b/pkg/tsdb/cloudwatch/services/accounts.go index ae5b1e50e2e3..f7d56318549d 100644 --- a/pkg/tsdb/cloudwatch/services/accounts.go +++ b/pkg/tsdb/cloudwatch/services/accounts.go @@ -3,6 +3,7 @@ package services import ( "errors" "fmt" + "strings" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/oam" @@ -52,8 +53,9 @@ func (a *AccountsService) GetAccountsForCurrentUserOrRole() ([]*models.Account, sinkIdentifier := sinks[0].Arn result := []*models.Account{{ + Id: getAccountId(*sinkIdentifier), Label: *sinks[0].Name, - Arn: *sinks[0].Arn, + Arn: *sinkIdentifier, IsMonitoringAccount: true, }} @@ -68,9 +70,11 @@ func (a *AccountsService) GetAccountsForCurrentUserOrRole() ([]*models.Account, } for _, link := range links.Items { + arn := *link.LinkArn result = append(result, &models.Account{ + Id: getAccountId(arn), Label: *link.Label, - Arn: *link.LinkArn, + Arn: arn, IsMonitoringAccount: false, }) } @@ -83,3 +87,14 @@ func (a *AccountsService) GetAccountsForCurrentUserOrRole() ([]*models.Account, return result, nil } + +func getAccountId(arn string) string { + // format: arn:partition:service:region:account-id:resource-id + parts := strings.Split(arn, ":") + + if len(parts) >= 4 { + return parts[4] + } + + return "" +} diff --git a/pkg/tsdb/cloudwatch/services/accounts_test.go b/pkg/tsdb/cloudwatch/services/accounts_test.go index e8e9ce2bd544..e0bd1c571c21 100644 --- a/pkg/tsdb/cloudwatch/services/accounts_test.go +++ b/pkg/tsdb/cloudwatch/services/accounts_test.go @@ -97,8 +97,8 @@ func TestHandleGetAccounts(t *testing.T) { }, nil) fakeOAMClient.On("ListAttachedLinks", mock.Anything).Return(&oam.ListAttachedLinksOutput{ Items: []*oam.ListAttachedLinksItem{ - {Label: aws.String("Account 10"), LinkArn: aws.String("arn:aws:logs:us-east-1:123456789012:log-group:my-log-group10")}, - {Label: aws.String("Account 11"), LinkArn: aws.String("arn:aws:logs:us-east-1:123456789012:log-group:my-log-group11")}, + {Label: aws.String("Account 10"), LinkArn: aws.String("arn:aws:logs:us-east-1:123456789013:log-group:my-log-group10")}, + {Label: aws.String("Account 11"), LinkArn: aws.String("arn:aws:logs:us-east-1:123456789014:log-group:my-log-group11")}, }, NextToken: new(string), }, nil).Once() @@ -116,10 +116,10 @@ func TestHandleGetAccounts(t *testing.T) { fakeOAMClient.AssertNumberOfCalls(t, "ListSinks", 2) fakeOAMClient.AssertNumberOfCalls(t, "ListAttachedLinks", 2) expectedAccounts := []*models.Account{ - {Label: "Account 1", Arn: "arn:aws:logs:us-east-1:123456789012:log-group:my-log-group1", IsMonitoringAccount: true}, - {Label: "Account 10", Arn: "arn:aws:logs:us-east-1:123456789012:log-group:my-log-group10", IsMonitoringAccount: false}, - {Label: "Account 11", Arn: "arn:aws:logs:us-east-1:123456789012:log-group:my-log-group11", IsMonitoringAccount: false}, - {Label: "Account 12", Arn: "arn:aws:logs:us-east-1:123456789012:log-group:my-log-group12", IsMonitoringAccount: false}, + {Id: "123456789012", Label: "Account 1", Arn: "arn:aws:logs:us-east-1:123456789012:log-group:my-log-group1", IsMonitoringAccount: true}, + {Id: "123456789013", Label: "Account 10", Arn: "arn:aws:logs:us-east-1:123456789013:log-group:my-log-group10", IsMonitoringAccount: false}, + {Id: "123456789014", Label: "Account 11", Arn: "arn:aws:logs:us-east-1:123456789014:log-group:my-log-group11", IsMonitoringAccount: false}, + {Id: "123456789012", Label: "Account 12", Arn: "arn:aws:logs:us-east-1:123456789012:log-group:my-log-group12", IsMonitoringAccount: false}, } assert.Equal(t, expectedAccounts, resp) }) From d0fb359f5cae93ab76f6ca3355a078c9b2e68987 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Fri, 28 Oct 2022 12:48:56 +0200 Subject: [PATCH 15/43] only show badge when feature toggle is enabled (#615) --- .../MetricsQueryEditor/MetricsQueryHeader.tsx | 10 ++- .../components/PanelQueryEditor.test.tsx | 88 ++++++++++++++----- .../cloudwatch/components/QueryHeader.tsx | 6 +- 3 files changed, 76 insertions(+), 28 deletions(-) diff --git a/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/MetricsQueryHeader.tsx b/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/MetricsQueryHeader.tsx index 25b03abb279d..fe4f0457cea1 100644 --- a/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/MetricsQueryHeader.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/MetricsQueryEditor/MetricsQueryHeader.tsx @@ -2,6 +2,7 @@ import React, { useCallback, useState } from 'react'; import { SelectableValue } from '@grafana/data'; import { FlexItem, InlineSelect } from '@grafana/experimental'; +import { config } from '@grafana/runtime'; import { Badge, Button, ConfirmModal, RadioButtonGroup } from '@grafana/ui'; import { CloudWatchDatasource } from '../../datasource'; @@ -51,6 +52,11 @@ const MetricsQueryHeader: React.FC = ({ [setShowConfirm, onChange, sqlCodeEditorIsDirty, query, metricEditorMode, metricQueryType] ); + const shouldDisplayMonitoringBadge = + query.metricQueryType === MetricQueryType.Search && + isMonitoringAccount && + config.featureToggles.cloudwatchCrossAccountQuerying; + return ( <> = ({ /> - {query.metricQueryType === MetricQueryType.Search && isMonitoringAccount && ( - - )} + {shouldDisplayMonitoringBadge && } diff --git a/public/app/plugins/datasource/cloudwatch/components/PanelQueryEditor.test.tsx b/public/app/plugins/datasource/cloudwatch/components/PanelQueryEditor.test.tsx index e74de192e4e3..cad42325fbc2 100644 --- a/public/app/plugins/datasource/cloudwatch/components/PanelQueryEditor.test.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/PanelQueryEditor.test.tsx @@ -2,6 +2,7 @@ import { act, render, screen } from '@testing-library/react'; import React from 'react'; import { QueryEditorProps } from '@grafana/data'; +import { config } from '@grafana/runtime'; import { setupMockedDataSource } from '../__mocks__/CloudWatchDataSource'; import { @@ -142,39 +143,78 @@ describe('PanelQueryEditor should render right editor', () => { interface MonitoringBadgeScenario { name: string; query: CloudWatchQuery; + toggle: boolean; } - describe('monitoring badge should be displayed when a monitoring account is returned and', () => { - const cases: MonitoringBadgeScenario[] = [ - { name: 'it is logs query', query: validLogsQuery }, - { name: 'it is metric search builder query', query: validMetricSearchBuilderQuery }, - { name: 'it is metric search code query', query: validMetricSearchCodeQuery }, - ]; - test.each(cases)('$name', async ({ query }) => { - const datasourceMock = setupMockedDataSource(); + describe('monitoring badge', () => { + let originalValue: boolean | undefined; + let datasourceMock: ReturnType; + beforeEach(() => { + datasourceMock = setupMockedDataSource(); datasourceMock.datasource.api.isMonitoringAccount = jest.fn().mockResolvedValue(accounts); datasourceMock.datasource.api.getMetrics = jest.fn().mockResolvedValue([]); datasourceMock.datasource.api.getDimensionKeys = jest.fn().mockResolvedValue([]); - await act(async () => { - render(); + originalValue = config.featureToggles.cloudwatchCrossAccountQuerying; + }); + afterEach(() => { + config.featureToggles.cloudwatchCrossAccountQuerying = originalValue; + }); + + describe('should be displayed when a monitoring account is returned and', () => { + const cases: MonitoringBadgeScenario[] = [ + { name: 'it is logs query and feature is enabled', query: validLogsQuery, toggle: true }, + { + name: 'it is metric search builder query and feature is enabled', + query: validMetricSearchBuilderQuery, + toggle: true, + }, + { + name: 'it is metric search code query and feature is enabled', + query: validMetricSearchCodeQuery, + toggle: true, + }, + ]; + + test.each(cases)('$name', async ({ query, toggle }) => { + config.featureToggles.cloudwatchCrossAccountQuerying = toggle; + await act(async () => { + render(); + }); + expect(await screen.getByText('Monitoring account')).toBeInTheDocument(); }); - expect(await screen.getByText('Monitoring account')).toBeInTheDocument(); }); - }); - describe('should not be displayed when a monitoring account is returned and', () => { - const cases: MonitoringBadgeScenario[] = [ - { name: 'it is metric query builder query', query: validMetricQueryBuilderQuery }, - { name: 'it is metric query code query', query: validMetricQueryCodeQuery }, - ]; - test.each(cases)('$name', async ({ query }) => { - const datasourceMock = setupMockedDataSource(); - datasourceMock.datasource.api.isMonitoringAccount = jest.fn().mockResolvedValue(accounts); - datasourceMock.datasource.api.getMetrics = jest.fn().mockResolvedValue([]); - await act(async () => { - render(); + describe('should not be displayed when a monitoring account is returned and', () => { + const cases: MonitoringBadgeScenario[] = [ + { + name: 'it is metric query builder query and toggle is enabled', + query: validMetricQueryBuilderQuery, + toggle: true, + }, + { + name: 'it is metric query code query and toggle is not enabled', + query: validMetricQueryCodeQuery, + toggle: true, + }, + { name: 'it is logs query and feature is not enabled', query: validLogsQuery, toggle: false }, + { + name: 'it is metric search builder query and feature is not enabled', + query: validMetricSearchBuilderQuery, + toggle: false, + }, + { + name: 'it is metric search code query and feature is not enabled', + query: validMetricSearchCodeQuery, + toggle: false, + }, + ]; + test.each(cases)('$name', async ({ query, toggle }) => { + config.featureToggles.cloudwatchCrossAccountQuerying = toggle; + await act(async () => { + render(); + }); + expect(await screen.queryByText('Monitoring account')).toBeNull(); }); - expect(await screen.queryByText('Monitoring account')).toBeNull(); }); }); }); diff --git a/public/app/plugins/datasource/cloudwatch/components/QueryHeader.tsx b/public/app/plugins/datasource/cloudwatch/components/QueryHeader.tsx index d7aed95a7949..87d7c5a40cbb 100644 --- a/public/app/plugins/datasource/cloudwatch/components/QueryHeader.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/QueryHeader.tsx @@ -3,6 +3,7 @@ import React from 'react'; import { SelectableValue, ExploreMode } from '@grafana/data'; import { EditorHeader, InlineSelect, FlexItem } from '@grafana/experimental'; +import { config } from '@grafana/runtime'; import { Badge } from '@grafana/ui'; import { CloudWatchDatasource } from '../datasource'; @@ -48,6 +49,9 @@ const QueryHeader: React.FC = ({ query, sqlCodeEditorIsDirty, }); }; + const shouldDisplayMonitoringBadge = + queryMode === 'Logs' && isMonitoringAccount && config.featureToggles.cloudwatchCrossAccountQuerying; + return ( = ({ query, sqlCodeEditorIsDirty, - {queryMode === 'Logs' && isMonitoringAccount && ( + {shouldDisplayMonitoringBadge && ( <> From 33a10bd460d2d3d04d9ad4159e9ba4b708c1b669 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Fri, 28 Oct 2022 14:46:44 +0200 Subject: [PATCH 16/43] Lattice: Refactor resource response type and return account (#613) * refactor resource response type * remove not used file. * go lint * fix tests * remove commented code --- pkg/tsdb/cloudwatch/cloudwatch_test.go | 20 +++++----- pkg/tsdb/cloudwatch/mocks/accounts_service.go | 4 +- .../cloudwatch/mocks/list_metrics_service.go | 17 +++++---- pkg/tsdb/cloudwatch/models/account.go | 2 +- pkg/tsdb/cloudwatch/models/api.go | 8 ++-- pkg/tsdb/cloudwatch/models/resources/types.go | 5 +++ pkg/tsdb/cloudwatch/routes/accounts_test.go | 18 +++++---- pkg/tsdb/cloudwatch/routes/dimension_keys.go | 2 +- .../cloudwatch/routes/dimension_keys_test.go | 8 ++-- .../routes/dimension_values_test.go | 4 +- pkg/tsdb/cloudwatch/routes/metrics.go | 10 ++--- pkg/tsdb/cloudwatch/routes/metrics_test.go | 22 ++++------- pkg/tsdb/cloudwatch/routes/namespaces.go | 13 +++++-- pkg/tsdb/cloudwatch/routes/namespaces_test.go | 16 ++++---- pkg/tsdb/cloudwatch/services/accounts.go | 4 +- pkg/tsdb/cloudwatch/services/accounts_test.go | 16 ++++---- .../cloudwatch/services/hardcoded_metrics.go | 17 +++++---- .../services/hardcoded_metrics_test.go | 4 +- pkg/tsdb/cloudwatch/services/list_metrics.go | 38 ++++++++++--------- .../cloudwatch/services/list_metrics_test.go | 7 ++-- pkg/tsdb/cloudwatch/services/utils.go | 12 ++++++ .../plugins/datasource/cloudwatch/api.test.ts | 24 ++++++++---- .../app/plugins/datasource/cloudwatch/api.ts | 28 +++++++------- .../datasource/cloudwatch/datasource.test.ts | 8 +--- .../plugins/datasource/cloudwatch/types.ts | 5 +++ 25 files changed, 172 insertions(+), 140 deletions(-) create mode 100644 pkg/tsdb/cloudwatch/services/utils.go diff --git a/pkg/tsdb/cloudwatch/cloudwatch_test.go b/pkg/tsdb/cloudwatch/cloudwatch_test.go index e1d3c5f4402d..3b710fdc7428 100644 --- a/pkg/tsdb/cloudwatch/cloudwatch_test.go +++ b/pkg/tsdb/cloudwatch/cloudwatch_test.go @@ -589,10 +589,10 @@ func Test_CloudWatch_CallResource_Integration_Test(t *testing.T) { sent := sender.Response require.NotNil(t, sent) require.Equal(t, http.StatusOK, sent.Status) - res := []string{} + res := []models.ResourceResponse[string]{} err = json.Unmarshal(sent.Body, &res) require.Nil(t, err) - assert.Equal(t, []string{"Value1", "Value2", "Value7"}, res) + assert.Equal(t, []models.ResourceResponse[string]{{Value: "Value1"}, {Value: "Value2"}, {Value: "Value7"}}, res) }) t.Run("Should handle dimension key filter query and return keys from the api", func(t *testing.T) { @@ -625,10 +625,10 @@ func Test_CloudWatch_CallResource_Integration_Test(t *testing.T) { sent := sender.Response require.NotNil(t, sent) require.Equal(t, http.StatusOK, sent.Status) - res := []string{} + res := []models.ResourceResponse[string]{} err = json.Unmarshal(sent.Body, &res) require.Nil(t, err) - assert.Equal(t, []string{"Test_DimensionName1", "Test_DimensionName2", "Test_DimensionName4", "Test_DimensionName5"}, res) + assert.Equal(t, []models.ResourceResponse[string]{{Value: "Test_DimensionName1"}, {Value: "Test_DimensionName2"}, {Value: "Test_DimensionName4"}, {Value: "Test_DimensionName5"}}, res) }) t.Run("Should handle standard dimension key query and return hard coded keys", func(t *testing.T) { @@ -649,10 +649,10 @@ func Test_CloudWatch_CallResource_Integration_Test(t *testing.T) { sent := sender.Response require.NotNil(t, sent) require.Equal(t, http.StatusOK, sent.Status) - res := []string{} + res := []models.ResourceResponse[string]{} err = json.Unmarshal(sent.Body, &res) require.Nil(t, err) - assert.Equal(t, []string{"ClientId", "DomainName"}, res) + assert.Equal(t, []models.ResourceResponse[string]{{Value: "ClientId"}, {Value: "DomainName"}}, res) }) t.Run("Should handle custom namespace dimension key query and return hard coded keys", func(t *testing.T) { @@ -673,10 +673,10 @@ func Test_CloudWatch_CallResource_Integration_Test(t *testing.T) { sent := sender.Response require.NotNil(t, sent) require.Equal(t, http.StatusOK, sent.Status) - res := []string{} + res := []models.ResourceResponse[string]{} err = json.Unmarshal(sent.Body, &res) require.Nil(t, err) - assert.Equal(t, []string{"ClientId", "DomainName"}, res) + assert.Equal(t, []models.ResourceResponse[string]{{Value: "ClientId"}, {Value: "DomainName"}}, res) }) t.Run("Should handle custom namespace metrics query and return metrics from api", func(t *testing.T) { @@ -709,10 +709,10 @@ func Test_CloudWatch_CallResource_Integration_Test(t *testing.T) { sent := sender.Response require.NotNil(t, sent) require.Equal(t, http.StatusOK, sent.Status) - res := []resources.Metric{} + res := []models.ResourceResponse[models.Metric]{} err = json.Unmarshal(sent.Body, &res) require.Nil(t, err) - assert.Equal(t, []resources.Metric{{Name: "Test_MetricName1", Namespace: "AWS/EC2"}, {Name: "Test_MetricName2", Namespace: "AWS/EC2"}, {Name: "Test_MetricName3", Namespace: "AWS/ECS"}, {Name: "Test_MetricName10", Namespace: "AWS/ECS"}, {Name: "Test_MetricName4", Namespace: "AWS/ECS"}, {Name: "Test_MetricName5", Namespace: "AWS/Redshift"}}, res) + assert.Equal(t, []models.ResourceResponse[models.Metric]{{Value: models.Metric{Name: "Test_MetricName1", Namespace: "AWS/EC2"}}, {Value: models.Metric{Name: "Test_MetricName2", Namespace: "AWS/EC2"}}, {Value: models.Metric{Name: "Test_MetricName3", Namespace: "AWS/ECS"}}, {Value: models.Metric{Name: "Test_MetricName10", Namespace: "AWS/ECS"}}, {Value: models.Metric{Name: "Test_MetricName4", Namespace: "AWS/ECS"}}, {Value: models.Metric{Name: "Test_MetricName5", Namespace: "AWS/Redshift"}}}, res) }) } diff --git a/pkg/tsdb/cloudwatch/mocks/accounts_service.go b/pkg/tsdb/cloudwatch/mocks/accounts_service.go index 01c2e7914125..b3c9955c8c08 100644 --- a/pkg/tsdb/cloudwatch/mocks/accounts_service.go +++ b/pkg/tsdb/cloudwatch/mocks/accounts_service.go @@ -9,8 +9,8 @@ type AccountsServiceMock struct { mock.Mock } -func (a *AccountsServiceMock) GetAccountsForCurrentUserOrRole() ([]*models.Account, error) { +func (a *AccountsServiceMock) GetAccountsForCurrentUserOrRole() ([]models.ResourceResponse[*models.Account], error) { args := a.Called() - return args.Get(0).([]*models.Account), args.Error(1) + return args.Get(0).([]models.ResourceResponse[*models.Account]), args.Error(1) } diff --git a/pkg/tsdb/cloudwatch/mocks/list_metrics_service.go b/pkg/tsdb/cloudwatch/mocks/list_metrics_service.go index e809bc8518c8..fb1385343f71 100644 --- a/pkg/tsdb/cloudwatch/mocks/list_metrics_service.go +++ b/pkg/tsdb/cloudwatch/mocks/list_metrics_service.go @@ -1,6 +1,7 @@ package mocks import ( + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/resources" "github.com/stretchr/testify/mock" ) @@ -9,26 +10,26 @@ type ListMetricsServiceMock struct { mock.Mock } -func (a *ListMetricsServiceMock) GetDimensionKeysByDimensionFilter(r resources.DimensionKeysRequest) ([]string, error) { +func (a *ListMetricsServiceMock) GetDimensionKeysByDimensionFilter(r resources.DimensionKeysRequest) ([]models.ResourceResponse[string], error) { args := a.Called(r) - return args.Get(0).([]string), args.Error(1) + return args.Get(0).([]models.ResourceResponse[string]), args.Error(1) } -func (a *ListMetricsServiceMock) GetDimensionValuesByDimensionFilter(r resources.DimensionValuesRequest) ([]string, error) { +func (a *ListMetricsServiceMock) GetDimensionValuesByDimensionFilter(r resources.DimensionValuesRequest) ([]models.ResourceResponse[string], error) { args := a.Called(r) - return args.Get(0).([]string), args.Error(1) + return args.Get(0).([]models.ResourceResponse[string]), args.Error(1) } -func (a *ListMetricsServiceMock) GetDimensionKeysByNamespace(namespace string) ([]string, error) { +func (a *ListMetricsServiceMock) GetDimensionKeysByNamespace(namespace string) ([]models.ResourceResponse[string], error) { args := a.Called(namespace) - return args.Get(0).([]string), args.Error(1) + return args.Get(0).([]models.ResourceResponse[string]), args.Error(1) } -func (a *ListMetricsServiceMock) GetMetricsByNamespace(namespace string) ([]resources.Metric, error) { +func (a *ListMetricsServiceMock) GetMetricsByNamespace(namespace string) ([]models.ResourceResponse[models.Metric], error) { args := a.Called(namespace) - return args.Get(0).([]resources.Metric), args.Error(1) + return args.Get(0).([]models.ResourceResponse[models.Metric]), args.Error(1) } diff --git a/pkg/tsdb/cloudwatch/models/account.go b/pkg/tsdb/cloudwatch/models/account.go index 9bff0ee71593..5dc97fde17a6 100644 --- a/pkg/tsdb/cloudwatch/models/account.go +++ b/pkg/tsdb/cloudwatch/models/account.go @@ -17,7 +17,7 @@ type OAMClientProvider interface { } type AccountsProvider interface { - GetAccountsForCurrentUserOrRole() ([]*Account, error) + GetAccountsForCurrentUserOrRole() ([]ResourceResponse[*Account], error) } type ClientsProvider interface { diff --git a/pkg/tsdb/cloudwatch/models/api.go b/pkg/tsdb/cloudwatch/models/api.go index b86860e5efdc..b6655dc52c01 100644 --- a/pkg/tsdb/cloudwatch/models/api.go +++ b/pkg/tsdb/cloudwatch/models/api.go @@ -6,10 +6,10 @@ import ( ) type ListMetricsProvider interface { - GetDimensionKeysByDimensionFilter(resources.DimensionKeysRequest) ([]string, error) - GetDimensionKeysByNamespace(string) ([]string, error) - GetDimensionValuesByDimensionFilter(resources.DimensionValuesRequest) ([]string, error) - GetMetricsByNamespace(namespace string) ([]resources.Metric, error) + GetDimensionKeysByDimensionFilter(resources.DimensionKeysRequest) ([]ResourceResponse[string], error) + GetDimensionKeysByNamespace(string) ([]ResourceResponse[string], error) + GetDimensionValuesByDimensionFilter(resources.DimensionValuesRequest) ([]ResourceResponse[string], error) + GetMetricsByNamespace(namespace string) ([]ResourceResponse[Metric], error) } type MetricsClientProvider interface { diff --git a/pkg/tsdb/cloudwatch/models/resources/types.go b/pkg/tsdb/cloudwatch/models/resources/types.go index a218740a438b..328888ed50c3 100644 --- a/pkg/tsdb/cloudwatch/models/resources/types.go +++ b/pkg/tsdb/cloudwatch/models/resources/types.go @@ -5,6 +5,11 @@ type Dimension struct { Value string } +type ResourceResponse[T any] struct { + AccountId *string `json:"accountId,omitempty"` + Value T `json:"value"` +} + type Metric struct { Name string `json:"name"` Namespace string `json:"namespace"` diff --git a/pkg/tsdb/cloudwatch/routes/accounts_test.go b/pkg/tsdb/cloudwatch/routes/accounts_test.go index e2ab92d87819..c5bb4837caf2 100644 --- a/pkg/tsdb/cloudwatch/routes/accounts_test.go +++ b/pkg/tsdb/cloudwatch/routes/accounts_test.go @@ -21,11 +21,13 @@ func Test_accounts_route(t *testing.T) { t.Run("successfully returns array of accounts json", func(t *testing.T) { mockAccountsService := mocks.AccountsServiceMock{} - mockAccountsService.On("GetAccountsForCurrentUserOrRole").Return([]*models.Account{{ - Id: "123456789012", - Arn: "some arn", - Label: "some label", - IsMonitoringAccount: true, + mockAccountsService.On("GetAccountsForCurrentUserOrRole").Return([]models.ResourceResponse[*models.Account]{{ + Value: &models.Account{ + Id: "123456789012", + Arn: "some arn", + Label: "some label", + IsMonitoringAccount: true, + }, }}, nil) newAccountsService = func(pluginCtx backend.PluginContext, reqCtxFactory models.RequestContextFactoryFunc, region string) (models.AccountsProvider, error) { return &mockAccountsService, nil @@ -37,7 +39,7 @@ func Test_accounts_route(t *testing.T) { handler.ServeHTTP(rr, req) assert.Equal(t, http.StatusOK, rr.Code) - assert.JSONEq(t, `[{"id":"123456789012", "arn":"some arn", "isMonitoringAccount":true, "label":"some label"}]`, rr.Body.String()) + assert.JSONEq(t, `[{"value":{"id":"123456789012", "arn":"some arn", "isMonitoringAccount":true, "label":"some label"}}]`, rr.Body.String()) }) t.Run("rejects POST method", func(t *testing.T) { @@ -58,7 +60,7 @@ func Test_accounts_route(t *testing.T) { t.Run("returns 403 when accounts service returns ErrAccessDeniedException", func(t *testing.T) { mockAccountsService := mocks.AccountsServiceMock{} - mockAccountsService.On("GetAccountsForCurrentUserOrRole").Return([]*models.Account(nil), + mockAccountsService.On("GetAccountsForCurrentUserOrRole").Return([]models.ResourceResponse[*models.Account](nil), fmt.Errorf("%w: %s", services.ErrAccessDeniedException, "some AWS message")) newAccountsService = func(pluginCtx backend.PluginContext, reqCtxFactory models.RequestContextFactoryFunc, region string) (models.AccountsProvider, error) { return &mockAccountsService, nil @@ -77,7 +79,7 @@ func Test_accounts_route(t *testing.T) { t.Run("returns 500 when accounts service returns unknown error", func(t *testing.T) { mockAccountsService := mocks.AccountsServiceMock{} - mockAccountsService.On("GetAccountsForCurrentUserOrRole").Return([]*models.Account(nil), fmt.Errorf("some error")) + mockAccountsService.On("GetAccountsForCurrentUserOrRole").Return([]models.ResourceResponse[*models.Account](nil), fmt.Errorf("some error")) newAccountsService = func(pluginCtx backend.PluginContext, reqCtxFactory models.RequestContextFactoryFunc, region string) (models.AccountsProvider, error) { return &mockAccountsService, nil } diff --git a/pkg/tsdb/cloudwatch/routes/dimension_keys.go b/pkg/tsdb/cloudwatch/routes/dimension_keys.go index 302225dbed3c..1e5cf0bef49f 100644 --- a/pkg/tsdb/cloudwatch/routes/dimension_keys.go +++ b/pkg/tsdb/cloudwatch/routes/dimension_keys.go @@ -22,7 +22,7 @@ func DimensionKeysHandler(pluginCtx backend.PluginContext, reqCtxFactory models. return nil, models.NewHttpError("error in DimensionKeyHandler", http.StatusInternalServerError, err) } - var response []string + var response []models.ResourceResponse[string] switch dimensionKeysRequest.Type() { case resources.FilterDimensionKeysRequest: response, err = service.GetDimensionKeysByDimensionFilter(dimensionKeysRequest) diff --git a/pkg/tsdb/cloudwatch/routes/dimension_keys_test.go b/pkg/tsdb/cloudwatch/routes/dimension_keys_test.go index e48fa7dabb63..397e2168432e 100644 --- a/pkg/tsdb/cloudwatch/routes/dimension_keys_test.go +++ b/pkg/tsdb/cloudwatch/routes/dimension_keys_test.go @@ -24,7 +24,7 @@ var logger = &logtest.Fake{} func Test_DimensionKeys_Route(t *testing.T) { t.Run("calls FilterDimensionKeysRequest when a StandardDimensionKeysRequest is passed", func(t *testing.T) { mockListMetricsService := mocks.ListMetricsServiceMock{} - mockListMetricsService.On("GetDimensionKeysByDimensionFilter", mock.Anything).Return([]string{}, nil).Once() + mockListMetricsService.On("GetDimensionKeysByDimensionFilter", mock.Anything).Return([]models.ResourceResponse[string]{}, nil).Once() newListMetricsService = func(pluginCtx backend.PluginContext, reqCtxFactory models.RequestContextFactoryFunc, region string) (models.ListMetricsProvider, error) { return &mockListMetricsService, nil } @@ -47,10 +47,10 @@ func Test_DimensionKeys_Route(t *testing.T) { }) haveBeenCalled := false usedNamespace := "" - services.GetHardCodedDimensionKeysByNamespace = func(namespace string) ([]string, error) { + services.GetHardCodedDimensionKeysByNamespace = func(namespace string) ([]models.ResourceResponse[string], error) { haveBeenCalled = true usedNamespace = namespace - return []string{}, nil + return []models.ResourceResponse[string]{}, nil } rr := httptest.NewRecorder() req := httptest.NewRequest("GET", "/dimension-keys?region=us-east-2&namespace=AWS/EC2&metricName=CPUUtilization", nil) @@ -65,7 +65,7 @@ func Test_DimensionKeys_Route(t *testing.T) { t.Run("return 500 if GetDimensionKeysByDimensionFilter returns an error", func(t *testing.T) { mockListMetricsService := mocks.ListMetricsServiceMock{} - mockListMetricsService.On("GetDimensionKeysByDimensionFilter", mock.Anything).Return([]string{}, fmt.Errorf("some error")) + mockListMetricsService.On("GetDimensionKeysByDimensionFilter", mock.Anything).Return([]models.ResourceResponse[string]{}, fmt.Errorf("some error")) newListMetricsService = func(pluginCtx backend.PluginContext, reqCtxFactory models.RequestContextFactoryFunc, region string) (models.ListMetricsProvider, error) { return &mockListMetricsService, nil } diff --git a/pkg/tsdb/cloudwatch/routes/dimension_values_test.go b/pkg/tsdb/cloudwatch/routes/dimension_values_test.go index c2853a8c98b3..11fc4fb204d1 100644 --- a/pkg/tsdb/cloudwatch/routes/dimension_values_test.go +++ b/pkg/tsdb/cloudwatch/routes/dimension_values_test.go @@ -18,7 +18,7 @@ import ( func Test_DimensionValues_Route(t *testing.T) { t.Run("Calls GetDimensionValuesByDimensionFilter when a valid request is passed", func(t *testing.T) { mockListMetricsService := mocks.ListMetricsServiceMock{} - mockListMetricsService.On("GetDimensionValuesByDimensionFilter", mock.Anything).Return([]string{}, nil).Once() + mockListMetricsService.On("GetDimensionValuesByDimensionFilter", mock.Anything).Return([]models.ResourceResponse[string]{}, nil).Once() newListMetricsService = func(pluginCtx backend.PluginContext, reqCtxFactory models.RequestContextFactoryFunc, region string) (models.ListMetricsProvider, error) { return &mockListMetricsService, nil } @@ -37,7 +37,7 @@ func Test_DimensionValues_Route(t *testing.T) { t.Run("returns 500 if GetDimensionValuesByDimensionFilter returns an error", func(t *testing.T) { mockListMetricsService := mocks.ListMetricsServiceMock{} - mockListMetricsService.On("GetDimensionValuesByDimensionFilter", mock.Anything).Return([]string{}, fmt.Errorf("some error")) + mockListMetricsService.On("GetDimensionValuesByDimensionFilter", mock.Anything).Return([]models.ResourceResponse[string]{}, fmt.Errorf("some error")) newListMetricsService = func(pluginCtx backend.PluginContext, reqCtxFactory models.RequestContextFactoryFunc, region string) (models.ListMetricsProvider, error) { return &mockListMetricsService, nil } diff --git a/pkg/tsdb/cloudwatch/routes/metrics.go b/pkg/tsdb/cloudwatch/routes/metrics.go index db90f7b2f2c9..f65a90119260 100644 --- a/pkg/tsdb/cloudwatch/routes/metrics.go +++ b/pkg/tsdb/cloudwatch/routes/metrics.go @@ -22,20 +22,20 @@ func MetricsHandler(pluginCtx backend.PluginContext, reqCtxFactory models.Reques return nil, models.NewHttpError("error in MetricsHandler", http.StatusInternalServerError, err) } - var metrics []resources.Metric + var response []models.ResourceResponse[models.Metric] switch metricsRequest.Type() { case resources.AllMetricsRequestType: - metrics = services.GetAllHardCodedMetrics() + response = services.GetAllHardCodedMetrics() case resources.MetricsByNamespaceRequestType: - metrics, err = services.GetHardCodedMetricsByNamespace(metricsRequest.Namespace) + response, err = services.GetHardCodedMetricsByNamespace(metricsRequest.Namespace) case resources.CustomNamespaceRequestType: - metrics, err = service.GetMetricsByNamespace(metricsRequest.Namespace) + response, err = service.GetMetricsByNamespace(metricsRequest.Namespace) } if err != nil { return nil, models.NewHttpError("error in MetricsHandler", http.StatusInternalServerError, err) } - metricsResponse, err := json.Marshal(metrics) + metricsResponse, err := json.Marshal(response) if err != nil { return nil, models.NewHttpError("error in MetricsHandler", http.StatusInternalServerError, err) } diff --git a/pkg/tsdb/cloudwatch/routes/metrics_test.go b/pkg/tsdb/cloudwatch/routes/metrics_test.go index 061a846012c7..e518cabdedc5 100644 --- a/pkg/tsdb/cloudwatch/routes/metrics_test.go +++ b/pkg/tsdb/cloudwatch/routes/metrics_test.go @@ -1,27 +1,27 @@ package routes import ( - "encoding/json" "fmt" "net/http" "net/http/httptest" "testing" "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/resources" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/tsdb/cloudwatch/mocks" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/services" + "github.com/stretchr/testify/assert" ) func Test_Metrics_Route(t *testing.T) { t.Run("calls GetMetricsByNamespace when a CustomNamespaceRequestType is passed", func(t *testing.T) { mockListMetricsService := mocks.ListMetricsServiceMock{} - mockListMetricsService.On("GetMetricsByNamespace", mock.Anything).Return([]resources.Metric{}, nil) + mockListMetricsService.On("GetMetricsByNamespace", mock.Anything).Return([]resources.ResourceResponse[models.Metric]{}, nil) newListMetricsService = func(pluginCtx backend.PluginContext, reqCtxFactory models.RequestContextFactoryFunc, region string) (models.ListMetricsProvider, error) { return &mockListMetricsService, nil } @@ -38,17 +38,14 @@ func Test_Metrics_Route(t *testing.T) { services.GetAllHardCodedMetrics = origGetAllHardCodedMetrics }) haveBeenCalled := false - services.GetAllHardCodedMetrics = func() []resources.Metric { + services.GetAllHardCodedMetrics = func() []models.ResourceResponse[models.Metric] { haveBeenCalled = true - return []resources.Metric{} + return []models.ResourceResponse[models.Metric]{} } rr := httptest.NewRecorder() req := httptest.NewRequest("GET", "/metrics?region=us-east-2", nil) handler := http.HandlerFunc(ResourceRequestMiddleware(MetricsHandler, logger, nil)) handler.ServeHTTP(rr, req) - res := []resources.Metric{} - err := json.Unmarshal(rr.Body.Bytes(), &res) - require.Nil(t, err) assert.True(t, haveBeenCalled) }) @@ -59,25 +56,22 @@ func Test_Metrics_Route(t *testing.T) { }) haveBeenCalled := false usedNamespace := "" - services.GetHardCodedMetricsByNamespace = func(namespace string) ([]resources.Metric, error) { + services.GetHardCodedMetricsByNamespace = func(namespace string) ([]models.ResourceResponse[models.Metric], error) { haveBeenCalled = true usedNamespace = namespace - return []resources.Metric{}, nil + return []models.ResourceResponse[models.Metric]{}, nil } rr := httptest.NewRecorder() req := httptest.NewRequest("GET", "/metrics?region=us-east-2&namespace=AWS/DMS", nil) handler := http.HandlerFunc(ResourceRequestMiddleware(MetricsHandler, logger, nil)) handler.ServeHTTP(rr, req) - res := []resources.Metric{} - err := json.Unmarshal(rr.Body.Bytes(), &res) - require.Nil(t, err) assert.True(t, haveBeenCalled) assert.Equal(t, "AWS/DMS", usedNamespace) }) t.Run("returns 500 if GetMetricsByNamespace returns an error", func(t *testing.T) { mockListMetricsService := mocks.ListMetricsServiceMock{} - mockListMetricsService.On("GetMetricsByNamespace", mock.Anything).Return([]resources.Metric{}, fmt.Errorf("some error")) + mockListMetricsService.On("GetMetricsByNamespace", mock.Anything).Return([]resources.ResourceResponse[models.Metric]{}, fmt.Errorf("some error")) newListMetricsService = func(pluginCtx backend.PluginContext, reqCtxFactory models.RequestContextFactoryFunc, region string) (models.ListMetricsProvider, error) { return &mockListMetricsService, nil } diff --git a/pkg/tsdb/cloudwatch/routes/namespaces.go b/pkg/tsdb/cloudwatch/routes/namespaces.go index 040748c5407a..6503db1e68ee 100644 --- a/pkg/tsdb/cloudwatch/routes/namespaces.go +++ b/pkg/tsdb/cloudwatch/routes/namespaces.go @@ -18,14 +18,19 @@ func NamespacesHandler(pluginCtx backend.PluginContext, reqCtxFactory models.Req return nil, models.NewHttpError("error in NamespacesHandler", http.StatusInternalServerError, err) } - result := services.GetHardCodedNamespaces() + response := services.GetHardCodedNamespaces() customNamespace := reqCtx.Settings.Namespace if customNamespace != "" { - result = append(result, strings.Split(customNamespace, ",")...) + customNamespaces := strings.Split(customNamespace, ",") + for _, customNamespace := range customNamespaces { + response = append(response, models.ResourceResponse[string]{Value: customNamespace}) + } } - sort.Strings(result) + sort.Slice(response, func(i, j int) bool { + return response[i].Value < response[j].Value + }) - namespacesResponse, err := json.Marshal(result) + namespacesResponse, err := json.Marshal(response) if err != nil { return nil, models.NewHttpError("error in NamespacesHandler", http.StatusInternalServerError, err) } diff --git a/pkg/tsdb/cloudwatch/routes/namespaces_test.go b/pkg/tsdb/cloudwatch/routes/namespaces_test.go index 9eeec6f0a94d..1a00a8dbe85c 100644 --- a/pkg/tsdb/cloudwatch/routes/namespaces_test.go +++ b/pkg/tsdb/cloudwatch/routes/namespaces_test.go @@ -28,9 +28,9 @@ func Test_Namespaces_Route(t *testing.T) { services.GetHardCodedNamespaces = origGetHardCodedNamespaces }) haveBeenCalled := false - services.GetHardCodedNamespaces = func() []string { + services.GetHardCodedNamespaces = func() []models.ResourceResponse[string] { haveBeenCalled = true - return []string{} + return []models.ResourceResponse[string]{} } rr := httptest.NewRecorder() req := httptest.NewRequest("GET", "/namespaces", nil) @@ -44,15 +44,15 @@ func Test_Namespaces_Route(t *testing.T) { t.Cleanup(func() { services.GetHardCodedNamespaces = origGetHardCodedNamespaces }) - services.GetHardCodedNamespaces = func() []string { - return []string{"AWS/EC2", "AWS/ELB"} + services.GetHardCodedNamespaces = func() []models.ResourceResponse[string] { + return []models.ResourceResponse[string]{{Value: "AWS/EC2"}, {Value: "AWS/ELB"}} } rr := httptest.NewRecorder() req := httptest.NewRequest("GET", "/namespaces", nil) customNamespaces = "customNamespace1,customNamespace2" handler := http.HandlerFunc(ResourceRequestMiddleware(NamespacesHandler, logger, factoryFunc)) handler.ServeHTTP(rr, req) - assert.JSONEq(t, `["AWS/EC2", "AWS/ELB", "customNamespace1", "customNamespace2"]`, rr.Body.String()) + assert.JSONEq(t, `[{"value":"AWS/EC2"}, {"value":"AWS/ELB"}, {"value":"customNamespace1"}, {"value":"customNamespace2"}]`, rr.Body.String()) }) t.Run("sorts result", func(t *testing.T) { @@ -60,14 +60,14 @@ func Test_Namespaces_Route(t *testing.T) { t.Cleanup(func() { services.GetHardCodedNamespaces = origGetHardCodedNamespaces }) - services.GetHardCodedNamespaces = func() []string { - return []string{"AWS/XYZ", "AWS/ELB"} + services.GetHardCodedNamespaces = func() []models.ResourceResponse[string] { + return []models.ResourceResponse[string]{{Value: "AWS/XYZ"}, {Value: "AWS/ELB"}} } rr := httptest.NewRecorder() req := httptest.NewRequest("GET", "/namespaces", nil) customNamespaces = "DCustomNamespace1,ACustomNamespace2" handler := http.HandlerFunc(ResourceRequestMiddleware(NamespacesHandler, logger, factoryFunc)) handler.ServeHTTP(rr, req) - assert.JSONEq(t, `["ACustomNamespace2", "AWS/ELB", "AWS/XYZ", "DCustomNamespace1"]`, rr.Body.String()) + assert.JSONEq(t, `[{"value":"ACustomNamespace2"}, {"value":"AWS/ELB"}, {"value":"AWS/XYZ"}, {"value":"DCustomNamespace1"}]`, rr.Body.String()) }) } diff --git a/pkg/tsdb/cloudwatch/services/accounts.go b/pkg/tsdb/cloudwatch/services/accounts.go index f7d56318549d..dfc92ca01194 100644 --- a/pkg/tsdb/cloudwatch/services/accounts.go +++ b/pkg/tsdb/cloudwatch/services/accounts.go @@ -20,7 +20,7 @@ func NewAccountsService(oamClient models.OAMClientProvider) models.AccountsProvi return &AccountsService{oamClient} } -func (a *AccountsService) GetAccountsForCurrentUserOrRole() ([]*models.Account, error) { +func (a *AccountsService) GetAccountsForCurrentUserOrRole() ([]models.ResourceResponse[*models.Account], error) { var nextToken *string sinks := []*oam.ListSinksItem{} for { @@ -85,7 +85,7 @@ func (a *AccountsService) GetAccountsForCurrentUserOrRole() ([]*models.Account, nextToken = links.NextToken } - return result, nil + return valuesToListMetricRespone(result), nil } func getAccountId(arn string) string { diff --git a/pkg/tsdb/cloudwatch/services/accounts_test.go b/pkg/tsdb/cloudwatch/services/accounts_test.go index e0bd1c571c21..a83705a3b405 100644 --- a/pkg/tsdb/cloudwatch/services/accounts_test.go +++ b/pkg/tsdb/cloudwatch/services/accounts_test.go @@ -75,9 +75,9 @@ func TestHandleGetAccounts(t *testing.T) { assert.NoError(t, err) fakeOAMClient.AssertNumberOfCalls(t, "ListSinks", 2) require.Len(t, resp, 1) - assert.True(t, resp[0].IsMonitoringAccount) - assert.Equal(t, "Account 1", resp[0].Label) - assert.Equal(t, "arn:aws:logs:us-east-1:123456789012:log-group:my-log-group1", resp[0].Arn) + assert.True(t, resp[0].Value.IsMonitoringAccount) + assert.Equal(t, "Account 1", resp[0].Value.Label) + assert.Equal(t, "arn:aws:logs:us-east-1:123456789012:log-group:my-log-group1", resp[0].Value.Arn) }) t.Run("Should merge the first sink with attached links", func(t *testing.T) { @@ -115,11 +115,11 @@ func TestHandleGetAccounts(t *testing.T) { assert.NoError(t, err) fakeOAMClient.AssertNumberOfCalls(t, "ListSinks", 2) fakeOAMClient.AssertNumberOfCalls(t, "ListAttachedLinks", 2) - expectedAccounts := []*models.Account{ - {Id: "123456789012", Label: "Account 1", Arn: "arn:aws:logs:us-east-1:123456789012:log-group:my-log-group1", IsMonitoringAccount: true}, - {Id: "123456789013", Label: "Account 10", Arn: "arn:aws:logs:us-east-1:123456789013:log-group:my-log-group10", IsMonitoringAccount: false}, - {Id: "123456789014", Label: "Account 11", Arn: "arn:aws:logs:us-east-1:123456789014:log-group:my-log-group11", IsMonitoringAccount: false}, - {Id: "123456789012", Label: "Account 12", Arn: "arn:aws:logs:us-east-1:123456789012:log-group:my-log-group12", IsMonitoringAccount: false}, + expectedAccounts := []models.ResourceResponse[*models.Account]{ + {Value: &models.Account{Id: "123456789012", Label: "Account 1", Arn: "arn:aws:logs:us-east-1:123456789012:log-group:my-log-group1", IsMonitoringAccount: true}}, + {Value: &models.Account{Id: "123456789013", Label: "Account 10", Arn: "arn:aws:logs:us-east-1:123456789013:log-group:my-log-group10", IsMonitoringAccount: false}}, + {Value: &models.Account{Id: "123456789014", Label: "Account 11", Arn: "arn:aws:logs:us-east-1:123456789014:log-group:my-log-group11", IsMonitoringAccount: false}}, + {Value: &models.Account{Id: "123456789012", Label: "Account 12", Arn: "arn:aws:logs:us-east-1:123456789012:log-group:my-log-group12", IsMonitoringAccount: false}}, } assert.Equal(t, expectedAccounts, resp) }) diff --git a/pkg/tsdb/cloudwatch/services/hardcoded_metrics.go b/pkg/tsdb/cloudwatch/services/hardcoded_metrics.go index 2268089b566e..b6008bba9406 100644 --- a/pkg/tsdb/cloudwatch/services/hardcoded_metrics.go +++ b/pkg/tsdb/cloudwatch/services/hardcoded_metrics.go @@ -4,19 +4,20 @@ import ( "fmt" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/constants" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/resources" ) -var GetHardCodedDimensionKeysByNamespace = func(namespace string) ([]string, error) { +var GetHardCodedDimensionKeysByNamespace = func(namespace string) ([]resources.ResourceResponse[string], error) { var dimensionKeys []string exists := false if dimensionKeys, exists = constants.NamespaceDimensionKeysMap[namespace]; !exists { return nil, fmt.Errorf("unable to find dimensions for namespace '%q'", namespace) } - return dimensionKeys, nil + return valuesToListMetricRespone(dimensionKeys), nil } -var GetHardCodedMetricsByNamespace = func(namespace string) ([]resources.Metric, error) { +var GetHardCodedMetricsByNamespace = func(namespace string) ([]resources.ResourceResponse[models.Metric], error) { response := []resources.Metric{} exists := false var metrics []string @@ -28,10 +29,10 @@ var GetHardCodedMetricsByNamespace = func(namespace string) ([]resources.Metric, response = append(response, resources.Metric{Namespace: namespace, Name: metric}) } - return response, nil + return valuesToListMetricRespone(response), nil } -var GetAllHardCodedMetrics = func() []resources.Metric { +var GetAllHardCodedMetrics = func() []resources.ResourceResponse[models.Metric] { response := []resources.Metric{} for namespace, metrics := range constants.NamespaceMetricsMap { for _, metric := range metrics { @@ -39,14 +40,14 @@ var GetAllHardCodedMetrics = func() []resources.Metric { } } - return response + return valuesToListMetricRespone(response) } -var GetHardCodedNamespaces = func() []string { +var GetHardCodedNamespaces = func() []models.ResourceResponse[string] { var namespaces []string for key := range constants.NamespaceMetricsMap { namespaces = append(namespaces, key) } - return namespaces + return valuesToListMetricRespone(namespaces) } diff --git a/pkg/tsdb/cloudwatch/services/hardcoded_metrics_test.go b/pkg/tsdb/cloudwatch/services/hardcoded_metrics_test.go index 4895ef9dc617..efdbe9114333 100644 --- a/pkg/tsdb/cloudwatch/services/hardcoded_metrics_test.go +++ b/pkg/tsdb/cloudwatch/services/hardcoded_metrics_test.go @@ -19,7 +19,7 @@ func TestHardcodedMetrics_GetHardCodedDimensionKeysByNamespace(t *testing.T) { t.Run("Should return keys if namespace exist", func(t *testing.T) { resp, err := GetHardCodedDimensionKeysByNamespace("AWS/EC2") require.NoError(t, err) - assert.Equal(t, []string{"AutoScalingGroupName", "ImageId", "InstanceId", "InstanceType"}, resp) + assert.Equal(t, []models.ResourceResponse[string]{{Value: "AutoScalingGroupName"}, {Value: "ImageId"}, {Value: "InstanceId"}, {Value: "InstanceType"}}, resp) }) } @@ -34,6 +34,6 @@ func TestHardcodedMetrics_GetHardCodedMetricsByNamespace(t *testing.T) { t.Run("Should return metrics if namespace exist", func(t *testing.T) { resp, err := GetHardCodedMetricsByNamespace("AWS/IoTAnalytics") require.NoError(t, err) - assert.Equal(t, []resources.Metric{{Name: "ActionExecution", Namespace: "AWS/IoTAnalytics"}, {Name: "ActivityExecutionError", Namespace: "AWS/IoTAnalytics"}, {Name: "IncomingMessages", Namespace: "AWS/IoTAnalytics"}}, resp) + assert.Equal(t, []models.ResourceResponse[models.Metric]{{Value: models.Metric{Name: "ActionExecution", Namespace: "AWS/IoTAnalytics"}}, {Value: models.Metric{Name: "ActivityExecutionError", Namespace: "AWS/IoTAnalytics"}}, {Value: models.Metric{Name: "IncomingMessages", Namespace: "AWS/IoTAnalytics"}}}, resp) }) } diff --git a/pkg/tsdb/cloudwatch/services/list_metrics.go b/pkg/tsdb/cloudwatch/services/list_metrics.go index 8ab2d2bebadc..c61a710cfecf 100644 --- a/pkg/tsdb/cloudwatch/services/list_metrics.go +++ b/pkg/tsdb/cloudwatch/services/list_metrics.go @@ -18,7 +18,7 @@ func NewListMetricsService(metricsClient models.MetricsClientProvider) models.Li return &ListMetricsService{metricsClient} } -func (l *ListMetricsService) GetDimensionKeysByDimensionFilter(r resources.DimensionKeysRequest) ([]string, error) { +func (l *ListMetricsService) GetDimensionKeysByDimensionFilter(r resources.DimensionKeysRequest) ([]models.ResourceResponse[string], error) { input := &cloudwatch.ListMetricsInput{} if r.Namespace != "" { input.Namespace = aws.String(r.Namespace) @@ -33,7 +33,7 @@ func (l *ListMetricsService) GetDimensionKeysByDimensionFilter(r resources.Dimen return nil, fmt.Errorf("%v: %w", "unable to call AWS API", err) } - var dimensionKeys []string + var response []models.ResourceResponse[string] // remove duplicates dupCheck := make(map[string]struct{}) for _, metric := range metrics { @@ -56,14 +56,14 @@ func (l *ListMetricsService) GetDimensionKeysByDimensionFilter(r resources.Dimen } dupCheck[*dim.Name] = struct{}{} - dimensionKeys = append(dimensionKeys, *dim.Name) + response = append(response, models.ResourceResponse[string]{Value: *dim.Name}) } } - return dimensionKeys, nil + return response, nil } -func (l *ListMetricsService) GetDimensionValuesByDimensionFilter(r resources.DimensionValuesRequest) ([]string, error) { +func (l *ListMetricsService) GetDimensionValuesByDimensionFilter(r resources.DimensionValuesRequest) ([]models.ResourceResponse[string], error) { input := &cloudwatch.ListMetricsInput{ Namespace: aws.String(r.Namespace), MetricName: aws.String(r.MetricName), @@ -75,7 +75,7 @@ func (l *ListMetricsService) GetDimensionValuesByDimensionFilter(r resources.Dim return nil, fmt.Errorf("%v: %w", "unable to call AWS API", err) } - var dimensionValues []string + var response []models.ResourceResponse[string] dupCheck := make(map[string]bool) for _, metric := range metrics { for _, dim := range metric.Dimensions { @@ -85,51 +85,53 @@ func (l *ListMetricsService) GetDimensionValuesByDimensionFilter(r resources.Dim } dupCheck[*dim.Value] = true - dimensionValues = append(dimensionValues, *dim.Value) + response = append(response, models.ResourceResponse[string]{Value: *dim.Value}) } } } - sort.Strings(dimensionValues) - return dimensionValues, nil + sort.Slice(response, func(i, j int) bool { + return response[i].Value < response[j].Value + }) + return response, nil } -func (l *ListMetricsService) GetDimensionKeysByNamespace(namespace string) ([]string, error) { - metrics, err := l.ListMetricsWithPageLimit(&cloudwatch.ListMetricsInput{Namespace: aws.String(namespace)}) +func (l *ListMetricsService) GetDimensionKeysByNamespace(namespace string) ([]models.ResourceResponse[string], error) { + response, err := l.ListMetricsWithPageLimit(&cloudwatch.ListMetricsInput{Namespace: aws.String(namespace)}) if err != nil { - return []string{}, err + return []models.ResourceResponse[string]{}, err } - var dimensionKeys []string + var dimensionKeys []models.ResourceResponse[string] dupCheck := make(map[string]struct{}) - for _, metric := range metrics { + for _, metric := range response { for _, dim := range metric.Dimensions { if _, exists := dupCheck[*dim.Name]; exists { continue } dupCheck[*dim.Name] = struct{}{} - dimensionKeys = append(dimensionKeys, *dim.Name) + dimensionKeys = append(dimensionKeys, models.ResourceResponse[string]{Value: *dim.Name}) } } return dimensionKeys, nil } -func (l *ListMetricsService) GetMetricsByNamespace(namespace string) ([]resources.Metric, error) { +func (l *ListMetricsService) GetMetricsByNamespace(namespace string) ([]models.ResourceResponse[models.Metric], error) { metrics, err := l.ListMetricsWithPageLimit(&cloudwatch.ListMetricsInput{Namespace: aws.String(namespace)}) if err != nil { return nil, err } - response := []resources.Metric{} + response := []models.ResourceResponse[models.Metric]{} dupCheck := make(map[string]struct{}) for _, metric := range metrics { if _, exists := dupCheck[*metric.MetricName]; exists { continue } dupCheck[*metric.MetricName] = struct{}{} - response = append(response, resources.Metric{Name: *metric.MetricName, Namespace: *metric.Namespace}) + response = append(response, models.ResourceResponse[models.Metric]{Value: models.Metric{Name: *metric.MetricName, Namespace: *metric.Namespace}}) } return response, nil diff --git a/pkg/tsdb/cloudwatch/services/list_metrics_test.go b/pkg/tsdb/cloudwatch/services/list_metrics_test.go index 34548ffaa98f..298f99969df4 100644 --- a/pkg/tsdb/cloudwatch/services/list_metrics_test.go +++ b/pkg/tsdb/cloudwatch/services/list_metrics_test.go @@ -6,6 +6,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/cloudwatch" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/mocks" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/resources" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -57,7 +58,7 @@ func TestListMetricsService_GetDimensionKeysByDimensionFilter(t *testing.T) { }) require.NoError(t, err) - assert.Equal(t, []string{"InstanceType", "AutoScalingGroupName"}, resp) + assert.Equal(t, []models.ResourceResponse[string]{{Value: "InstanceType"}, {Value: "AutoScalingGroupName"}}, resp) }) } @@ -70,7 +71,7 @@ func TestListMetricsService_GetDimensionKeysByNamespace(t *testing.T) { resp, err := listMetricsService.GetDimensionKeysByNamespace("AWS/EC2") require.NoError(t, err) - assert.Equal(t, []string{"InstanceId", "InstanceType", "AutoScalingGroupName"}, resp) + assert.Equal(t, []models.ResourceResponse[string]{{Value: "InstanceId"}, {Value: "InstanceType"}, {Value: "AutoScalingGroupName"}}, resp) }) } @@ -91,6 +92,6 @@ func TestListMetricsService_GetDimensionValuesByDimensionFilter(t *testing.T) { }) require.NoError(t, err) - assert.Equal(t, []string{"i-1234567890abcdef0", "i-5234567890abcdef0", "i-64234567890abcdef0"}, resp) + assert.Equal(t, []models.ResourceResponse[string]{{Value: "i-1234567890abcdef0"}, {Value: "i-5234567890abcdef0"}, {Value: "i-64234567890abcdef0"}}, resp) }) } diff --git a/pkg/tsdb/cloudwatch/services/utils.go b/pkg/tsdb/cloudwatch/services/utils.go new file mode 100644 index 000000000000..b9decfc553c7 --- /dev/null +++ b/pkg/tsdb/cloudwatch/services/utils.go @@ -0,0 +1,12 @@ +package services + +import "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" + +func valuesToListMetricRespone[T any](values []T) []models.ResourceResponse[T] { + var response []models.ResourceResponse[T] + for _, value := range values { + response = append(response, models.ResourceResponse[T]{Value: value}) + } + + return response +} diff --git a/public/app/plugins/datasource/cloudwatch/api.test.ts b/public/app/plugins/datasource/cloudwatch/api.test.ts index 6199876b77e0..99fbbcc32f23 100644 --- a/public/app/plugins/datasource/cloudwatch/api.test.ts +++ b/public/app/plugins/datasource/cloudwatch/api.test.ts @@ -75,12 +75,16 @@ describe('api', () => { it('when getAllMetrics is called', async () => { const getMock = jest.fn().mockResolvedValue([ { - namespace: 'AWS/EC2', - name: 'CPUUtilization', + value: { + namespace: 'AWS/EC2', + name: 'CPUUtilization', + }, }, { - namespace: 'AWS/Redshift', - name: 'CPUPercentage', + value: { + namespace: 'AWS/Redshift', + name: 'CPUPercentage', + }, }, ]); const { api } = setupMockedAPI({ getMock }); @@ -94,12 +98,16 @@ describe('api', () => { it('when getMetrics', async () => { const getMock = jest.fn().mockResolvedValue([ { - namespace: 'AWS/EC2', - name: 'CPUUtilization', + value: { + namespace: 'AWS/EC2', + name: 'CPUUtilization', + }, }, { - namespace: 'AWS/EC2', - name: 'CPUPercentage', + value: { + namespace: 'AWS/EC2', + name: 'CPUPercentage', + }, }, ]); const { api } = setupMockedAPI({ getMock }); diff --git a/public/app/plugins/datasource/cloudwatch/api.ts b/public/app/plugins/datasource/cloudwatch/api.ts index b02c8e94215c..18d1cc4607c3 100644 --- a/public/app/plugins/datasource/cloudwatch/api.ts +++ b/public/app/plugins/datasource/cloudwatch/api.ts @@ -1,6 +1,6 @@ import { memoize } from 'lodash'; -import { DataSourceInstanceSettings, SelectableValue, toOption } from '@grafana/data'; +import { DataSourceInstanceSettings, SelectableValue } from '@grafana/data'; import { getBackendSrv } from '@grafana/runtime'; import { TemplateSrv } from 'app/features/templating/template_srv'; @@ -15,6 +15,7 @@ import { MultiFilters, Account, ResourceRequest, + ResourceResponse, } from './types'; export interface SelectableResourceValue extends SelectableValue { @@ -36,9 +37,9 @@ export class CloudWatchAPI extends CloudWatchRequest { } getAccounts({ region }: ResourceRequest): Promise { - return this.memoizedGetRequest('accounts', { + return this.memoizedGetRequest>>('accounts', { region: this.templateSrv.replace(region), - }); + }).then((accounts) => accounts.map((a) => a.value)); } isMonitoringAccount(region: string): Promise { @@ -55,8 +56,8 @@ export class CloudWatchAPI extends CloudWatchRequest { } getNamespaces() { - return this.memoizedGetRequest('namespaces').then((namespaces) => - namespaces.map((n) => ({ label: n, value: n })) + return this.memoizedGetRequest>>('namespaces').then((namespaces) => + namespaces.map((n) => ({ label: n.value, value: n.value })) ); } @@ -79,16 +80,16 @@ export class CloudWatchAPI extends CloudWatchRequest { return []; } - return this.memoizedGetRequest('metrics', { + return this.memoizedGetRequest>>('metrics', { region: this.templateSrv.replace(this.getActualRegion(region)), namespace: this.templateSrv.replace(namespace), - }).then((metrics) => metrics.map((m) => ({ label: m.name, value: m.name }))); + }).then((metrics) => metrics.map((m) => ({ label: m.value.name, value: m.value.name }))); } async getAllMetrics({ region }: GetMetricsRequest): Promise> { - return this.memoizedGetRequest('metrics', { + return this.memoizedGetRequest>>('metrics', { region: this.templateSrv.replace(this.getActualRegion(region)), - }).then((metrics) => metrics.map((m) => ({ metricName: m.name, namespace: m.namespace }))); + }).then((metrics) => metrics.map((m) => ({ metricName: m.value.name, namespace: m.value.namespace }))); } async getDimensionKeys({ @@ -97,12 +98,12 @@ export class CloudWatchAPI extends CloudWatchRequest { dimensionFilters = {}, metricName = '', }: GetDimensionKeysRequest): Promise>> { - return this.memoizedGetRequest('dimension-keys', { + return this.memoizedGetRequest>>('dimension-keys', { region: this.templateSrv.replace(this.getActualRegion(region)), namespace: this.templateSrv.replace(namespace), dimensionFilters: JSON.stringify(this.convertDimensionFormat(dimensionFilters, {})), metricName, - }).then((dimensionKeys) => dimensionKeys.map(toOption)); + }).then((r) => r.map((r) => ({ label: r.value, value: r.value }))); } async getDimensionValues({ @@ -116,14 +117,13 @@ export class CloudWatchAPI extends CloudWatchRequest { return []; } - const values = await this.memoizedGetRequest('dimension-values', { + const values = await this.memoizedGetRequest>>('dimension-values', { region: this.templateSrv.replace(this.getActualRegion(region)), namespace: this.templateSrv.replace(namespace), metricName: this.templateSrv.replace(metricName.trim()), dimensionKey: this.templateSrv.replace(dimensionKey), dimensionFilters: JSON.stringify(this.convertDimensionFormat(dimensionFilters, {})), - }).then((dimensionValues) => dimensionValues.map(toOption)); - + }).then((r) => r.map((r) => ({ label: r.value, value: r.value }))); return values; } diff --git a/public/app/plugins/datasource/cloudwatch/datasource.test.ts b/public/app/plugins/datasource/cloudwatch/datasource.test.ts index ef59ad566075..9cf9c7cb9e14 100644 --- a/public/app/plugins/datasource/cloudwatch/datasource.test.ts +++ b/public/app/plugins/datasource/cloudwatch/datasource.test.ts @@ -205,13 +205,9 @@ describe('datasource', () => { it('should map resource response to metric response', async () => { const datasource = setupMockedDataSource({ getMock: jest.fn().mockResolvedValue([ + { value: { namespace: 'AWS/EC2', name: 'CPUUtilization' } }, { - namespace: 'AWS/EC2', - name: 'CPUUtilization', - }, - { - namespace: 'AWS/Redshift', - name: 'CPUPercentage', + value: { namespace: 'AWS/Redshift', name: 'CPUPercentage' }, }, ]), }).datasource; diff --git a/public/app/plugins/datasource/cloudwatch/types.ts b/public/app/plugins/datasource/cloudwatch/types.ts index 82c8bd862338..456c7bc2953a 100644 --- a/public/app/plugins/datasource/cloudwatch/types.ts +++ b/public/app/plugins/datasource/cloudwatch/types.ts @@ -494,3 +494,8 @@ export interface MetricResponse { name: string; namespace: string; } + +export interface ResourceResponse { + account?: Account; + value: T; +} From 3411a5ddb3f096fc8072e4aa95083a285611941b Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Tue, 1 Nov 2022 15:30:30 +0100 Subject: [PATCH 17/43] Lattice: Use account as input when listing metric names and dimensions (#611) * use account in resource requests * add account to response * revert accountInfo to accountId * PR feedback * unit test account in list metrics response * remove not used asserts * don't assert on response that is not relevant to the test * removed dupe test * pr feedback --- pkg/tsdb/cloudwatch/clients/metrics.go | 15 +- pkg/tsdb/cloudwatch/clients/metrics_test.go | 29 +++ .../cloudwatch/mocks/cloudwatch_metric_api.go | 4 +- .../cloudwatch/mocks/list_metrics_service.go | 2 +- pkg/tsdb/cloudwatch/mocks/metrics_client.go | 5 +- pkg/tsdb/cloudwatch/models/api.go | 2 +- pkg/tsdb/cloudwatch/models/request/types.go | 12 ++ .../models/resources/resource_request.go | 14 +- pkg/tsdb/cloudwatch/models/types.go | 2 + pkg/tsdb/cloudwatch/routes/dimension_keys.go | 4 +- pkg/tsdb/cloudwatch/routes/metrics.go | 2 +- pkg/tsdb/cloudwatch/services/accounts.go | 6 +- .../cloudwatch/services/hardcoded_metrics.go | 12 +- pkg/tsdb/cloudwatch/services/list_metrics.go | 41 ++-- .../cloudwatch/services/list_metrics_test.go | 196 ++++++++++++++++-- pkg/tsdb/cloudwatch/services/utils.go | 2 + .../app/plugins/datasource/cloudwatch/api.ts | 15 +- .../cloudwatch/components/Account.test.tsx | 56 +---- .../cloudwatch/components/Account.tsx | 34 +-- .../components/Dimensions/FilterItem.tsx | 12 +- .../MetricStatEditor/MetricStatEditor.tsx | 7 +- .../SQLBuilderEditor/SQLBuilderSelectRow.tsx | 2 +- .../VariableQueryEditor.tsx | 2 +- .../plugins/datasource/cloudwatch/hooks.ts | 14 +- .../plugins/datasource/cloudwatch/types.ts | 8 +- 25 files changed, 348 insertions(+), 150 deletions(-) create mode 100644 pkg/tsdb/cloudwatch/models/request/types.go diff --git a/pkg/tsdb/cloudwatch/clients/metrics.go b/pkg/tsdb/cloudwatch/clients/metrics.go index d205c968cee6..1389f3046c63 100644 --- a/pkg/tsdb/cloudwatch/clients/metrics.go +++ b/pkg/tsdb/cloudwatch/clients/metrics.go @@ -6,6 +6,7 @@ import ( "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/request" ) type metricsClient struct { @@ -17,16 +18,22 @@ func NewMetricsClient(api models.CloudWatchMetricsAPIProvider, config *setting.C return &metricsClient{CloudWatchMetricsAPIProvider: api, config: config} } -func (l *metricsClient) ListMetricsWithPageLimit(params *cloudwatch.ListMetricsInput) ([]*cloudwatch.Metric, error) { - var cloudWatchMetrics []*cloudwatch.Metric +func (l *metricsClient) ListMetricsWithPageLimit(params *cloudwatch.ListMetricsInput) ([]*models.MetricOutput, error) { + var cloudWatchMetrics []*models.MetricOutput pageNum := 0 err := l.ListMetricsPages(params, func(page *cloudwatch.ListMetricsOutput, lastPage bool) bool { pageNum++ metrics.MAwsCloudWatchListMetrics.Inc() metrics, err := awsutil.ValuesAtPath(page, "Metrics") if err == nil { - for _, metric := range metrics { - cloudWatchMetrics = append(cloudWatchMetrics, metric.(*cloudwatch.Metric)) + for idx, metric := range metrics { + metric := &models.MetricOutput{Metric: metric.(*cloudwatch.Metric)} + if len(page.OwningAccounts) >= idx && params.IncludeLinkedAccounts != nil && *params.IncludeLinkedAccounts { + metric.Account = &request.Account{ + Id: *page.OwningAccounts[idx], + } + } + cloudWatchMetrics = append(cloudWatchMetrics, metric) } } return !lastPage && pageNum < l.config.AWSListMetricsPageLimit diff --git a/pkg/tsdb/cloudwatch/clients/metrics_test.go b/pkg/tsdb/cloudwatch/clients/metrics_test.go index 91636775ba5f..2e54b448f497 100644 --- a/pkg/tsdb/cloudwatch/clients/metrics_test.go +++ b/pkg/tsdb/cloudwatch/clients/metrics_test.go @@ -7,6 +7,8 @@ import ( "github.com/aws/aws-sdk-go/service/cloudwatch" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/mocks" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/request" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -46,4 +48,31 @@ func TestMetricsClient(t *testing.T) { assert.Equal(t, len(metrics), len(response)) }) + + t.Run("Should return account id in case IncludeLinkedAccounts is set to true", func(t *testing.T) { + fakeApi := &mocks.FakeMetricsAPI{Metrics: []*cloudwatch.Metric{ + {MetricName: aws.String("Test_MetricName1")}, + {MetricName: aws.String("Test_MetricName2")}, + {MetricName: aws.String("Test_MetricName3")}, + }, OwningAccounts: []*string{aws.String("1234567890"), aws.String("1234567890"), aws.String("1234567895")}} + client := NewMetricsClient(fakeApi, &setting.Cfg{AWSListMetricsPageLimit: 100}) + + response, err := client.ListMetricsWithPageLimit(&cloudwatch.ListMetricsInput{IncludeLinkedAccounts: aws.Bool(true)}) + require.NoError(t, err) + expected := []*models.MetricOutput{ + {Metric: &cloudwatch.Metric{MetricName: aws.String("Test_MetricName1")}, Account: &request.Account{Id: "1234567890"}}, + {Metric: &cloudwatch.Metric{MetricName: aws.String("Test_MetricName2")}, Account: &request.Account{Id: "1234567890"}}, + {Metric: &cloudwatch.Metric{MetricName: aws.String("Test_MetricName3")}, Account: &request.Account{Id: "1234567895"}}, + } + assert.Equal(t, expected, response) + }) + + t.Run("Should not return account id in case IncludeLinkedAccounts is set to false", func(t *testing.T) { + fakeApi := &mocks.FakeMetricsAPI{Metrics: []*cloudwatch.Metric{{MetricName: aws.String("Test_MetricName1")}}, OwningAccounts: []*string{aws.String("1234567890")}} + client := NewMetricsClient(fakeApi, &setting.Cfg{AWSListMetricsPageLimit: 100}) + + response, err := client.ListMetricsWithPageLimit(&cloudwatch.ListMetricsInput{IncludeLinkedAccounts: aws.Bool(false)}) + require.NoError(t, err) + assert.Nil(t, response[0].Account) + }) } diff --git a/pkg/tsdb/cloudwatch/mocks/cloudwatch_metric_api.go b/pkg/tsdb/cloudwatch/mocks/cloudwatch_metric_api.go index 8508f6e56976..ef89dfaff16a 100644 --- a/pkg/tsdb/cloudwatch/mocks/cloudwatch_metric_api.go +++ b/pkg/tsdb/cloudwatch/mocks/cloudwatch_metric_api.go @@ -12,6 +12,7 @@ type FakeMetricsAPI struct { cloudwatchiface.CloudWatchAPI Metrics []*cloudwatch.Metric + OwningAccounts []*string MetricsPerPage int } @@ -23,7 +24,8 @@ func (c *FakeMetricsAPI) ListMetricsPages(input *cloudwatch.ListMetricsInput, fn for i, metrics := range chunks { response := fn(&cloudwatch.ListMetricsOutput{ - Metrics: metrics, + Metrics: metrics, + OwningAccounts: c.OwningAccounts, }, i+1 == len(chunks)) if !response { break diff --git a/pkg/tsdb/cloudwatch/mocks/list_metrics_service.go b/pkg/tsdb/cloudwatch/mocks/list_metrics_service.go index fb1385343f71..7839b27a78cf 100644 --- a/pkg/tsdb/cloudwatch/mocks/list_metrics_service.go +++ b/pkg/tsdb/cloudwatch/mocks/list_metrics_service.go @@ -28,7 +28,7 @@ func (a *ListMetricsServiceMock) GetDimensionKeysByNamespace(namespace string) ( return args.Get(0).([]models.ResourceResponse[string]), args.Error(1) } -func (a *ListMetricsServiceMock) GetMetricsByNamespace(namespace string) ([]models.ResourceResponse[models.Metric], error) { +func (a *ListMetricsServiceMock) GetMetricsByNamespace(r *request.MetricsRequest) ([]models.ResourceResponse[models.Metric], error) { args := a.Called(namespace) return args.Get(0).([]models.ResourceResponse[models.Metric]), args.Error(1) diff --git a/pkg/tsdb/cloudwatch/mocks/metrics_client.go b/pkg/tsdb/cloudwatch/mocks/metrics_client.go index 3b0d818f7f49..7d142fa40e82 100644 --- a/pkg/tsdb/cloudwatch/mocks/metrics_client.go +++ b/pkg/tsdb/cloudwatch/mocks/metrics_client.go @@ -2,6 +2,7 @@ package mocks import ( "github.com/aws/aws-sdk-go/service/cloudwatch" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" "github.com/stretchr/testify/mock" ) @@ -9,7 +10,7 @@ type FakeMetricsClient struct { mock.Mock } -func (m *FakeMetricsClient) ListMetricsWithPageLimit(params *cloudwatch.ListMetricsInput) ([]*cloudwatch.Metric, error) { +func (m *FakeMetricsClient) ListMetricsWithPageLimit(params *cloudwatch.ListMetricsInput) ([]*models.MetricOutput, error) { args := m.Called(params) - return args.Get(0).([]*cloudwatch.Metric), args.Error(1) + return args.Get(0).([]*models.MetricOutput), args.Error(1) } diff --git a/pkg/tsdb/cloudwatch/models/api.go b/pkg/tsdb/cloudwatch/models/api.go index b6655dc52c01..d6b579eccea9 100644 --- a/pkg/tsdb/cloudwatch/models/api.go +++ b/pkg/tsdb/cloudwatch/models/api.go @@ -13,7 +13,7 @@ type ListMetricsProvider interface { } type MetricsClientProvider interface { - ListMetricsWithPageLimit(params *cloudwatch.ListMetricsInput) ([]*cloudwatch.Metric, error) + ListMetricsWithPageLimit(params *cloudwatch.ListMetricsInput) ([]*MetricOutput, error) } type CloudWatchMetricsAPIProvider interface { diff --git a/pkg/tsdb/cloudwatch/models/request/types.go b/pkg/tsdb/cloudwatch/models/request/types.go new file mode 100644 index 000000000000..5af65c28c40b --- /dev/null +++ b/pkg/tsdb/cloudwatch/models/request/types.go @@ -0,0 +1,12 @@ +package request + +type Dimension struct { + Name string + Value string +} + +type Account struct { + Id string `json:"id"` + Arn string `json:"arn,omitempty"` + Label string `json:"label,omitempty"` +} diff --git a/pkg/tsdb/cloudwatch/models/resources/resource_request.go b/pkg/tsdb/cloudwatch/models/resources/resource_request.go index e13c11cb558e..84bb2e2ab660 100644 --- a/pkg/tsdb/cloudwatch/models/resources/resource_request.go +++ b/pkg/tsdb/cloudwatch/models/resources/resource_request.go @@ -5,8 +5,15 @@ import ( "net/url" ) +const useLinkedAccountsId = "all" + type ResourceRequest struct { - Region string + Region string + AccountId *string +} + +func (r *ResourceRequest) ShouldTargetAllAccounts() bool { + return r.AccountId != nil && *r.AccountId == useLinkedAccountsId } func getResourceRequest(parameters url.Values) (*ResourceRequest, error) { @@ -14,6 +21,11 @@ func getResourceRequest(parameters url.Values) (*ResourceRequest, error) { Region: parameters.Get("region"), } + accountId := parameters.Get("accountId") + if accountId != "" { + request.AccountId = &accountId + } + if request.Region == "" { return nil, fmt.Errorf("region is required") } diff --git a/pkg/tsdb/cloudwatch/models/types.go b/pkg/tsdb/cloudwatch/models/types.go index 15dc2efafbb3..c62fd6ed20d6 100644 --- a/pkg/tsdb/cloudwatch/models/types.go +++ b/pkg/tsdb/cloudwatch/models/types.go @@ -3,7 +3,9 @@ package models import ( "net/url" + "github.com/aws/aws-sdk-go/service/cloudwatch" "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/request" ) type RequestContext struct { diff --git a/pkg/tsdb/cloudwatch/routes/dimension_keys.go b/pkg/tsdb/cloudwatch/routes/dimension_keys.go index 1e5cf0bef49f..155b2b23b2a1 100644 --- a/pkg/tsdb/cloudwatch/routes/dimension_keys.go +++ b/pkg/tsdb/cloudwatch/routes/dimension_keys.go @@ -22,12 +22,12 @@ func DimensionKeysHandler(pluginCtx backend.PluginContext, reqCtxFactory models. return nil, models.NewHttpError("error in DimensionKeyHandler", http.StatusInternalServerError, err) } - var response []models.ResourceResponse[string] + response := []models.ResourceResponse[string]{} switch dimensionKeysRequest.Type() { case resources.FilterDimensionKeysRequest: response, err = service.GetDimensionKeysByDimensionFilter(dimensionKeysRequest) default: - response, err = services.GetHardCodedDimensionKeysByNamespace(dimensionKeysRequest.Namespace) + response, err = services.GetHardCodedDimensionKeysByNamespace(dimensionKeysRequest) } if err != nil { return nil, models.NewHttpError("error in DimensionKeyHandler", http.StatusInternalServerError, err) diff --git a/pkg/tsdb/cloudwatch/routes/metrics.go b/pkg/tsdb/cloudwatch/routes/metrics.go index f65a90119260..8ba17f965522 100644 --- a/pkg/tsdb/cloudwatch/routes/metrics.go +++ b/pkg/tsdb/cloudwatch/routes/metrics.go @@ -29,7 +29,7 @@ func MetricsHandler(pluginCtx backend.PluginContext, reqCtxFactory models.Reques case resources.MetricsByNamespaceRequestType: response, err = services.GetHardCodedMetricsByNamespace(metricsRequest.Namespace) case resources.CustomNamespaceRequestType: - response, err = service.GetMetricsByNamespace(metricsRequest.Namespace) + response, err = service.GetMetricsByNamespace(metricsRequest) } if err != nil { return nil, models.NewHttpError("error in MetricsHandler", http.StatusInternalServerError, err) diff --git a/pkg/tsdb/cloudwatch/services/accounts.go b/pkg/tsdb/cloudwatch/services/accounts.go index dfc92ca01194..1ec2e03876df 100644 --- a/pkg/tsdb/cloudwatch/services/accounts.go +++ b/pkg/tsdb/cloudwatch/services/accounts.go @@ -52,7 +52,7 @@ func (a *AccountsService) GetAccountsForCurrentUserOrRole() ([]models.ResourceRe } sinkIdentifier := sinks[0].Arn - result := []*models.Account{{ + response := []*models.Account{{ Id: getAccountId(*sinkIdentifier), Label: *sinks[0].Name, Arn: *sinkIdentifier, @@ -71,7 +71,7 @@ func (a *AccountsService) GetAccountsForCurrentUserOrRole() ([]models.ResourceRe for _, link := range links.Items { arn := *link.LinkArn - result = append(result, &models.Account{ + response = append(response, &models.Account{ Id: getAccountId(arn), Label: *link.Label, Arn: arn, @@ -85,7 +85,7 @@ func (a *AccountsService) GetAccountsForCurrentUserOrRole() ([]models.ResourceRe nextToken = links.NextToken } - return valuesToListMetricRespone(result), nil + return valuesToListMetricRespone(response), nil } func getAccountId(arn string) string { diff --git a/pkg/tsdb/cloudwatch/services/hardcoded_metrics.go b/pkg/tsdb/cloudwatch/services/hardcoded_metrics.go index b6008bba9406..867af1d9ea1b 100644 --- a/pkg/tsdb/cloudwatch/services/hardcoded_metrics.go +++ b/pkg/tsdb/cloudwatch/services/hardcoded_metrics.go @@ -9,12 +9,12 @@ import ( ) var GetHardCodedDimensionKeysByNamespace = func(namespace string) ([]resources.ResourceResponse[string], error) { - var dimensionKeys []string + var response []string exists := false - if dimensionKeys, exists = constants.NamespaceDimensionKeysMap[namespace]; !exists { + if response, exists = constants.NamespaceDimensionKeysMap[namespace]; !exists { return nil, fmt.Errorf("unable to find dimensions for namespace '%q'", namespace) } - return valuesToListMetricRespone(dimensionKeys), nil + return valuesToListMetricRespone(response), nil } var GetHardCodedMetricsByNamespace = func(namespace string) ([]resources.ResourceResponse[models.Metric], error) { @@ -44,10 +44,10 @@ var GetAllHardCodedMetrics = func() []resources.ResourceResponse[models.Metric] } var GetHardCodedNamespaces = func() []models.ResourceResponse[string] { - var namespaces []string + response := []string{} for key := range constants.NamespaceMetricsMap { - namespaces = append(namespaces, key) + response = append(response, key) } - return valuesToListMetricRespone(namespaces) + return valuesToListMetricRespone(response) } diff --git a/pkg/tsdb/cloudwatch/services/list_metrics.go b/pkg/tsdb/cloudwatch/services/list_metrics.go index c61a710cfecf..92a713ca9c49 100644 --- a/pkg/tsdb/cloudwatch/services/list_metrics.go +++ b/pkg/tsdb/cloudwatch/services/list_metrics.go @@ -27,13 +27,14 @@ func (l *ListMetricsService) GetDimensionKeysByDimensionFilter(r resources.Dimen input.MetricName = aws.String(r.MetricName) } setDimensionFilter(input, r.DimensionFilter) + setAccount(input, r.ResourceRequest) metrics, err := l.ListMetricsWithPageLimit(input) if err != nil { return nil, fmt.Errorf("%v: %w", "unable to call AWS API", err) } - var response []models.ResourceResponse[string] + response := []models.ResourceResponse[string]{} // remove duplicates dupCheck := make(map[string]struct{}) for _, metric := range metrics { @@ -56,7 +57,7 @@ func (l *ListMetricsService) GetDimensionKeysByDimensionFilter(r resources.Dimen } dupCheck[*dim.Name] = struct{}{} - response = append(response, models.ResourceResponse[string]{Value: *dim.Name}) + response = append(response, models.ResourceResponse[string]{Account: metric.Account, Value: *dim.Name}) } } @@ -69,13 +70,14 @@ func (l *ListMetricsService) GetDimensionValuesByDimensionFilter(r resources.Dim MetricName: aws.String(r.MetricName), } setDimensionFilter(input, r.DimensionFilter) + setAccount(input, r.ResourceRequest) metrics, err := l.ListMetricsWithPageLimit(input) if err != nil { return nil, fmt.Errorf("%v: %w", "unable to call AWS API", err) } - var response []models.ResourceResponse[string] + response := []models.ResourceResponse[string]{} dupCheck := make(map[string]bool) for _, metric := range metrics { for _, dim := range metric.Dimensions { @@ -85,7 +87,7 @@ func (l *ListMetricsService) GetDimensionValuesByDimensionFilter(r resources.Dim } dupCheck[*dim.Value] = true - response = append(response, models.ResourceResponse[string]{Value: *dim.Value}) + response = append(response, models.ResourceResponse[string]{Account: metric.Account, Value: *dim.Value}) } } } @@ -96,30 +98,34 @@ func (l *ListMetricsService) GetDimensionValuesByDimensionFilter(r resources.Dim return response, nil } -func (l *ListMetricsService) GetDimensionKeysByNamespace(namespace string) ([]models.ResourceResponse[string], error) { - response, err := l.ListMetricsWithPageLimit(&cloudwatch.ListMetricsInput{Namespace: aws.String(namespace)}) +func (l *ListMetricsService) GetDimensionKeysByNamespace(r *request.DimensionKeysRequest) ([]models.ResourceResponse[string], error) { + input := &cloudwatch.ListMetricsInput{Namespace: aws.String(r.Namespace)} + setAccount(input, r.ResourceRequest) + metrics, err := l.ListMetricsWithPageLimit(input) if err != nil { return []models.ResourceResponse[string]{}, err } - var dimensionKeys []models.ResourceResponse[string] + response := []models.ResourceResponse[string]{} dupCheck := make(map[string]struct{}) - for _, metric := range response { + for _, metric := range metrics { for _, dim := range metric.Dimensions { if _, exists := dupCheck[*dim.Name]; exists { continue } dupCheck[*dim.Name] = struct{}{} - dimensionKeys = append(dimensionKeys, models.ResourceResponse[string]{Value: *dim.Name}) + response = append(response, models.ResourceResponse[string]{Account: metric.Account, Value: *dim.Name}) } } - return dimensionKeys, nil + return response, nil } -func (l *ListMetricsService) GetMetricsByNamespace(namespace string) ([]models.ResourceResponse[models.Metric], error) { - metrics, err := l.ListMetricsWithPageLimit(&cloudwatch.ListMetricsInput{Namespace: aws.String(namespace)}) +func (l *ListMetricsService) GetMetricsByNamespace(r *request.MetricsRequest) ([]models.ResourceResponse[models.Metric], error) { + input := &cloudwatch.ListMetricsInput{Namespace: aws.String(r.Namespace)} + setAccount(input, r.ResourceRequest) + metrics, err := l.ListMetricsWithPageLimit(input) if err != nil { return nil, err } @@ -131,7 +137,7 @@ func (l *ListMetricsService) GetMetricsByNamespace(namespace string) ([]models.R continue } dupCheck[*metric.MetricName] = struct{}{} - response = append(response, models.ResourceResponse[models.Metric]{Value: models.Metric{Name: *metric.MetricName, Namespace: *metric.Namespace}}) + response = append(response, models.ResourceResponse[models.Metric]{Account: metric.Account, Value: models.Metric{Name: *metric.MetricName, Namespace: *metric.Namespace}}) } return response, nil @@ -148,3 +154,12 @@ func setDimensionFilter(input *cloudwatch.ListMetricsInput, dimensionFilter []*r input.Dimensions = append(input.Dimensions, df) } } + +func setAccount(input *cloudwatch.ListMetricsInput, r *request.ResourceRequest) { + if r != nil && r.AccountId != nil { + input.IncludeLinkedAccounts = aws.Bool(true) + if !r.ShouldTargetAllAccounts() { + input.OwningAccount = r.AccountId + } + } +} diff --git a/pkg/tsdb/cloudwatch/services/list_metrics_test.go b/pkg/tsdb/cloudwatch/services/list_metrics_test.go index 298f99969df4..4f1c45baabe4 100644 --- a/pkg/tsdb/cloudwatch/services/list_metrics_test.go +++ b/pkg/tsdb/cloudwatch/services/list_metrics_test.go @@ -13,35 +13,49 @@ import ( "github.com/stretchr/testify/require" ) -var metricResponse = []*cloudwatch.Metric{ +const useLinkedAccountsId = "all" + +var metricResponse = []*models.MetricOutput{ { - MetricName: aws.String("CPUUtilization"), - Namespace: aws.String("AWS/EC2"), - Dimensions: []*cloudwatch.Dimension{ - {Name: aws.String("InstanceId"), Value: aws.String("i-1234567890abcdef0")}, - {Name: aws.String("InstanceType"), Value: aws.String("t2.micro")}, + Metric: &cloudwatch.Metric{ + MetricName: aws.String("CPUUtilization"), + Namespace: aws.String("AWS/EC2"), + Dimensions: []*cloudwatch.Dimension{ + {Name: aws.String("InstanceId"), Value: aws.String("i-1234567890abcdef0")}, + {Name: aws.String("InstanceType"), Value: aws.String("t2.micro")}, + }, }, }, { - MetricName: aws.String("CPUUtilization"), - Namespace: aws.String("AWS/EC2"), - Dimensions: []*cloudwatch.Dimension{ - {Name: aws.String("InstanceId"), Value: aws.String("i-5234567890abcdef0")}, - {Name: aws.String("InstanceType"), Value: aws.String("t2.micro")}, - {Name: aws.String("AutoScalingGroupName"), Value: aws.String("my-asg")}, + Metric: &cloudwatch.Metric{ + MetricName: aws.String("CPUUtilization"), + Namespace: aws.String("AWS/EC2"), + Dimensions: []*cloudwatch.Dimension{ + {Name: aws.String("InstanceId"), Value: aws.String("i-5234567890abcdef0")}, + {Name: aws.String("InstanceType"), Value: aws.String("t2.micro")}, + {Name: aws.String("AutoScalingGroupName"), Value: aws.String("my-asg")}, + }, }, }, { - MetricName: aws.String("CPUUtilization"), - Namespace: aws.String("AWS/EC2"), - Dimensions: []*cloudwatch.Dimension{ - {Name: aws.String("InstanceId"), Value: aws.String("i-64234567890abcdef0")}, - {Name: aws.String("InstanceType"), Value: aws.String("t3.micro")}, - {Name: aws.String("AutoScalingGroupName"), Value: aws.String("my-asg2")}, + Metric: &cloudwatch.Metric{ + MetricName: aws.String("CPUUtilization"), + Namespace: aws.String("AWS/EC2"), + Dimensions: []*cloudwatch.Dimension{ + {Name: aws.String("InstanceId"), Value: aws.String("i-64234567890abcdef0")}, + {Name: aws.String("InstanceType"), Value: aws.String("t3.micro")}, + {Name: aws.String("AutoScalingGroupName"), Value: aws.String("my-asg2")}, + }, }, }, } +type validateInputTestCase[T request.DimensionKeysRequest | request.DimensionValuesRequest] struct { + name string + input *T + listMetricsWithPageLimitInput *cloudwatch.ListMetricsInput +} + func TestListMetricsService_GetDimensionKeysByDimensionFilter(t *testing.T) { t.Run("Should filter out duplicates and keys matching dimension filter keys", func(t *testing.T) { fakeMetricsClient := &mocks.FakeMetricsClient{} @@ -52,14 +66,68 @@ func TestListMetricsService_GetDimensionKeysByDimensionFilter(t *testing.T) { ResourceRequest: &resources.ResourceRequest{Region: "us-east-1"}, Namespace: "AWS/EC2", MetricName: "CPUUtilization", - DimensionFilter: []*resources.Dimension{ - {Name: "InstanceId", Value: ""}, - }, + DimensionFilter: []*resources.Dimension{{Name: "InstanceId", Value: ""}}, }) require.NoError(t, err) assert.Equal(t, []models.ResourceResponse[string]{{Value: "InstanceType"}, {Value: "AutoScalingGroupName"}}, resp) }) + + testCases := []validateInputTestCase[request.DimensionKeysRequest]{ + { + name: "Should set account correctly on list metric input if it cross account is defined on the request", + input: &request.DimensionKeysRequest{ + ResourceRequest: &request.ResourceRequest{Region: "us-east-1", AccountId: stringPtr(useLinkedAccountsId)}, + Namespace: "AWS/EC2", + MetricName: "CPUUtilization", + DimensionFilter: []*request.Dimension{{Name: "InstanceId", Value: ""}}, + }, + listMetricsWithPageLimitInput: &cloudwatch.ListMetricsInput{ + MetricName: aws.String("CPUUtilization"), + Namespace: aws.String("AWS/EC2"), + Dimensions: []*cloudwatch.DimensionFilter{{Name: aws.String("InstanceId")}}, + IncludeLinkedAccounts: aws.Bool(true), + }, + }, + { + name: "Should set account correctly on list metric input if single account is defined on the request", + input: &request.DimensionKeysRequest{ + ResourceRequest: &request.ResourceRequest{Region: "us-east-1", AccountId: stringPtr("1234567890")}, + Namespace: "AWS/EC2", + MetricName: "CPUUtilization", + DimensionFilter: []*request.Dimension{{Name: "InstanceId", Value: ""}}, + }, + listMetricsWithPageLimitInput: &cloudwatch.ListMetricsInput{ + MetricName: aws.String("CPUUtilization"), + Namespace: aws.String("AWS/EC2"), + Dimensions: []*cloudwatch.DimensionFilter{{Name: aws.String("InstanceId")}}, + IncludeLinkedAccounts: aws.Bool(true), + OwningAccount: aws.String("1234567890"), + }, + }, + { + name: "Should not set namespace and metricName on list metric input if empty strings are set for these in the request", + input: &request.DimensionKeysRequest{ + ResourceRequest: &request.ResourceRequest{Region: "us-east-1"}, + Namespace: "", + MetricName: "", + DimensionFilter: []*request.Dimension{{Name: "InstanceId", Value: ""}}, + }, + listMetricsWithPageLimitInput: &cloudwatch.ListMetricsInput{Dimensions: []*cloudwatch.DimensionFilter{{Name: aws.String("InstanceId")}}}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + fakeMetricsClient := &mocks.FakeMetricsClient{} + fakeMetricsClient.On("ListMetricsWithPageLimit", mock.Anything).Return(metricResponse, nil) + listMetricsService := NewListMetricsService(fakeMetricsClient) + res, err := listMetricsService.GetDimensionKeysByDimensionFilter(tc.input) + require.NoError(t, err) + require.NotEmpty(t, res) + fakeMetricsClient.AssertCalled(t, "ListMetricsWithPageLimit", tc.listMetricsWithPageLimitInput) + }) + } } func TestListMetricsService_GetDimensionKeysByNamespace(t *testing.T) { @@ -68,11 +136,49 @@ func TestListMetricsService_GetDimensionKeysByNamespace(t *testing.T) { fakeMetricsClient.On("ListMetricsWithPageLimit", mock.Anything).Return(metricResponse, nil) listMetricsService := NewListMetricsService(fakeMetricsClient) - resp, err := listMetricsService.GetDimensionKeysByNamespace("AWS/EC2") + resp, err := listMetricsService.GetDimensionKeysByNamespace(&request.DimensionKeysRequest{Namespace: "AWS/EC2"}) require.NoError(t, err) assert.Equal(t, []models.ResourceResponse[string]{{Value: "InstanceId"}, {Value: "InstanceType"}, {Value: "AutoScalingGroupName"}}, resp) }) + + testCases := []validateInputTestCase[request.DimensionKeysRequest]{ + { + name: "Should set account correctly on list metric input if it cross account is defined on the request", + input: &request.DimensionKeysRequest{ + ResourceRequest: &request.ResourceRequest{Region: "us-east-1", AccountId: stringPtr(useLinkedAccountsId)}, + Namespace: "AWS/EC2", + }, + listMetricsWithPageLimitInput: &cloudwatch.ListMetricsInput{ + Namespace: aws.String("AWS/EC2"), + IncludeLinkedAccounts: aws.Bool(true), + }, + }, + { + name: "Should set account correctly on list metric input if single account is defined on the request", + input: &request.DimensionKeysRequest{ + ResourceRequest: &request.ResourceRequest{Region: "us-east-1", AccountId: stringPtr("1234567890")}, + Namespace: "AWS/EC2", + }, + listMetricsWithPageLimitInput: &cloudwatch.ListMetricsInput{ + Namespace: aws.String("AWS/EC2"), + IncludeLinkedAccounts: aws.Bool(true), + OwningAccount: aws.String("1234567890"), + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + fakeMetricsClient := &mocks.FakeMetricsClient{} + fakeMetricsClient.On("ListMetricsWithPageLimit", mock.Anything).Return(metricResponse, nil) + listMetricsService := NewListMetricsService(fakeMetricsClient) + res, err := listMetricsService.GetDimensionKeysByNamespace(tc.input) + require.NoError(t, err) + require.NotEmpty(t, res) + fakeMetricsClient.AssertCalled(t, "ListMetricsWithPageLimit", tc.listMetricsWithPageLimitInput) + }) + } } func TestListMetricsService_GetDimensionValuesByDimensionFilter(t *testing.T) { @@ -94,4 +200,50 @@ func TestListMetricsService_GetDimensionValuesByDimensionFilter(t *testing.T) { require.NoError(t, err) assert.Equal(t, []models.ResourceResponse[string]{{Value: "i-1234567890abcdef0"}, {Value: "i-5234567890abcdef0"}, {Value: "i-64234567890abcdef0"}}, resp) }) + + testCases := []validateInputTestCase[request.DimensionValuesRequest]{ + { + name: "Should set account correctly on list metric input if it cross account is defined on the request", + input: &request.DimensionValuesRequest{ + ResourceRequest: &request.ResourceRequest{Region: "us-east-1", AccountId: stringPtr(useLinkedAccountsId)}, + Namespace: "AWS/EC2", + MetricName: "CPUUtilization", + DimensionFilter: []*request.Dimension{{Name: "InstanceId", Value: ""}}, + }, + listMetricsWithPageLimitInput: &cloudwatch.ListMetricsInput{ + MetricName: aws.String("CPUUtilization"), + Namespace: aws.String("AWS/EC2"), + Dimensions: []*cloudwatch.DimensionFilter{{Name: aws.String("InstanceId")}}, + IncludeLinkedAccounts: aws.Bool(true), + }, + }, + { + name: "Should set account correctly on list metric input if single account is defined on the request", + input: &request.DimensionValuesRequest{ + ResourceRequest: &request.ResourceRequest{Region: "us-east-1", AccountId: stringPtr("1234567890")}, + Namespace: "AWS/EC2", + MetricName: "CPUUtilization", + DimensionFilter: []*request.Dimension{{Name: "InstanceId", Value: ""}}, + }, + listMetricsWithPageLimitInput: &cloudwatch.ListMetricsInput{ + MetricName: aws.String("CPUUtilization"), + Namespace: aws.String("AWS/EC2"), + Dimensions: []*cloudwatch.DimensionFilter{{Name: aws.String("InstanceId")}}, + IncludeLinkedAccounts: aws.Bool(true), + OwningAccount: aws.String("1234567890"), + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + fakeMetricsClient := &mocks.FakeMetricsClient{} + fakeMetricsClient.On("ListMetricsWithPageLimit", mock.Anything).Return(metricResponse, nil) + listMetricsService := NewListMetricsService(fakeMetricsClient) + res, err := listMetricsService.GetDimensionValuesByDimensionFilter(tc.input) + require.NoError(t, err) + require.Empty(t, res) + fakeMetricsClient.AssertCalled(t, "ListMetricsWithPageLimit", tc.listMetricsWithPageLimitInput) + }) + } } diff --git a/pkg/tsdb/cloudwatch/services/utils.go b/pkg/tsdb/cloudwatch/services/utils.go index b9decfc553c7..00636c1a7188 100644 --- a/pkg/tsdb/cloudwatch/services/utils.go +++ b/pkg/tsdb/cloudwatch/services/utils.go @@ -10,3 +10,5 @@ func valuesToListMetricRespone[T any](values []T) []models.ResourceResponse[T] { return response } + +func stringPtr(s string) *string { return &s } diff --git a/public/app/plugins/datasource/cloudwatch/api.ts b/public/app/plugins/datasource/cloudwatch/api.ts index 18d1cc4607c3..a65c7211f39e 100644 --- a/public/app/plugins/datasource/cloudwatch/api.ts +++ b/public/app/plugins/datasource/cloudwatch/api.ts @@ -75,7 +75,7 @@ export class CloudWatchAPI extends CloudWatchRequest { }); } - async getMetrics({ region, namespace }: GetMetricsRequest): Promise>> { + async getMetrics({ region, namespace, accountId }: GetMetricsRequest): Promise>> { if (!namespace) { return []; } @@ -83,12 +83,17 @@ export class CloudWatchAPI extends CloudWatchRequest { return this.memoizedGetRequest>>('metrics', { region: this.templateSrv.replace(this.getActualRegion(region)), namespace: this.templateSrv.replace(namespace), + accountId: this.templateSrv.replace(accountId), }).then((metrics) => metrics.map((m) => ({ label: m.value.name, value: m.value.name }))); } - async getAllMetrics({ region }: GetMetricsRequest): Promise> { + async getAllMetrics({ + region, + accountId, + }: GetMetricsRequest): Promise> { return this.memoizedGetRequest>>('metrics', { region: this.templateSrv.replace(this.getActualRegion(region)), + accountId: this.templateSrv.replace(accountId), }).then((metrics) => metrics.map((m) => ({ metricName: m.value.name, namespace: m.value.namespace }))); } @@ -97,12 +102,14 @@ export class CloudWatchAPI extends CloudWatchRequest { namespace = '', dimensionFilters = {}, metricName = '', + accountId, }: GetDimensionKeysRequest): Promise>> { return this.memoizedGetRequest>>('dimension-keys', { region: this.templateSrv.replace(this.getActualRegion(region)), namespace: this.templateSrv.replace(namespace), + accountId: this.templateSrv.replace(accountId), + metricName: this.templateSrv.replace(metricName), dimensionFilters: JSON.stringify(this.convertDimensionFormat(dimensionFilters, {})), - metricName, }).then((r) => r.map((r) => ({ label: r.value, value: r.value }))); } @@ -112,6 +119,7 @@ export class CloudWatchAPI extends CloudWatchRequest { namespace, dimensionFilters = {}, metricName = '', + accountId, }: GetDimensionValuesRequest) { if (!namespace || !metricName) { return []; @@ -123,6 +131,7 @@ export class CloudWatchAPI extends CloudWatchRequest { metricName: this.templateSrv.replace(metricName.trim()), dimensionKey: this.templateSrv.replace(dimensionKey), dimensionFilters: JSON.stringify(this.convertDimensionFormat(dimensionFilters, {})), + accountId: this.templateSrv.replace(accountId), }).then((r) => r.map((r) => ({ label: r.value, value: r.value }))); return values; } diff --git a/public/app/plugins/datasource/cloudwatch/components/Account.test.tsx b/public/app/plugins/datasource/cloudwatch/components/Account.test.tsx index 9cde87e30ec1..6f754054d9a8 100644 --- a/public/app/plugins/datasource/cloudwatch/components/Account.test.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/Account.test.tsx @@ -11,19 +11,19 @@ import { Account } from './Account'; export const accounts = [ { arn: 'arn:aws:iam::123456789012:root', - accountId: '123456789012', + id: '123456789012', label: 'test-account', isMonitoringAccount: true, }, { arn: 'arn:aws:iam::432156789012:root', - accountId: '432156789012', + id: '432156789012', label: 'test-account2', isMonitoringAccount: false, }, { - arn: 'arn:aws:iam::987656789012:root', - accountId: '432156789012', + arn: 'arn:aws:iam::432156789013:root', + id: '432156789013', label: 'test-account3', isMonitoringAccount: false, }, @@ -37,15 +37,7 @@ describe('Account', () => { dimensions: {}, statistic: '', matchExact: true, - accountInfo: { - crossAccount: false, - account: { - arn: 'arn:aws:iam::123456789012:root', - id: '123456789012', - label: 'test-account', - isMonitoringAccount: true, - }, - }, + accountId: '123456789012', }; const props = { @@ -117,15 +109,7 @@ describe('Account', () => { onChange={onChange} query={{ ...props.metricStat, - accountInfo: { - crossAccount: false, - account: { - arn: 'arn:aws:iam::58356789012:root', - id: '58356789012', - label: 'some label', - isMonitoringAccount: true, - }, - }, + accountId: '58356789012', }} api={mock.api} /> @@ -145,21 +129,13 @@ describe('Account', () => { onChange={onChange} query={{ ...props.metricStat, - accountInfo: { - crossAccount: false, - account: { - arn: 'arn:aws:iam::58356789012:root', - label: '', - id: '58356789012', - isMonitoringAccount: true, - }, - }, + accountId: '58356789012', }} api={api} /> ); }); - expect(onChange).toHaveBeenCalledWith({ crossAccount: true }); + expect(onChange).toHaveBeenCalledWith('all'); }); it('should render "all" if crossAccount is stored in the query model', async () => { @@ -172,9 +148,7 @@ describe('Account', () => { onChange={onChange} query={{ ...props.metricStat, - accountInfo: { - crossAccount: true, - }, + accountId: 'all', }} api={api} /> @@ -184,7 +158,7 @@ describe('Account', () => { expect(await screen.getByText('All')).toBeInTheDocument(); }); - it('should not unset accountArn if the current value is in the loaded list of accounts', async () => { + it('should not unset accountId if the current value is in the loaded list of accounts', async () => { const api = setupMockedAPI().api; api.getAccounts = jest.fn().mockResolvedValue(accounts); const onChange = jest.fn(); @@ -194,15 +168,7 @@ describe('Account', () => { onChange={onChange} query={{ ...props.metricStat, - accountInfo: { - crossAccount: false, - account: { - arn: 'arn:aws:iam::432156789012:root', - label: '', - id: '432156789012', - isMonitoringAccount: true, - }, - }, + accountId: '432156789012', }} api={api} /> diff --git a/public/app/plugins/datasource/cloudwatch/components/Account.tsx b/public/app/plugins/datasource/cloudwatch/components/Account.tsx index 06497213b439..0b3916893608 100644 --- a/public/app/plugins/datasource/cloudwatch/components/Account.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/Account.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useState } from 'react'; +import React, { useEffect } from 'react'; import { useAsyncFn } from 'react-use'; import { SelectableValue } from '@grafana/data'; @@ -7,10 +7,10 @@ import { config } from '@grafana/runtime'; import { Select } from '@grafana/ui'; import { CloudWatchAPI } from '../api'; -import { MetricStat, Account as AccountType, AccountInfo } from '../types'; +import { MetricStat } from '../types'; export interface Props { - onChange: (account?: AccountInfo) => void; + onChange: (accountId?: string) => void; query: MetricStat; api: CloudWatchAPI; } @@ -21,36 +21,29 @@ const allOption: SelectableValue = { description: 'Target all linked accounts', }; -const crossAccount: AccountInfo = { - crossAccount: true, -}; - export function Account({ query, onChange, api }: Props) { - const [accounts, setAccounts] = useState([]); - const fetchAccounts = () => api .getAccounts({ region: query.region }) .then((accounts) => { - if (!accounts.length && query.accountInfo) { + if (!accounts.length && query.accountId) { onChange(undefined); return []; } - setAccounts(accounts); const options = accounts.map((a) => ({ label: `${a.label}${a.isMonitoringAccount ? ' (Monitoring account)' : ''}`, - value: a.arn, + value: a.id, description: a.id, })); - if (!options.find((o) => o.value === query.accountInfo?.account?.arn)) { - onChange(crossAccount); + if (!options.find((o) => o.value === query?.accountId)) { + onChange(allOption.value); } return options.length ? [allOption, ...options] : options; }) .catch(() => { - query.accountInfo && onChange(undefined); + query.accountId && onChange(undefined); return []; }); @@ -70,8 +63,7 @@ export function Account({ query, onChange, api }: Props) { return null; } - const current = state.value?.find((a) => a.value === query.accountInfo?.account?.arn); - const value = current ?? allOption; + const value = state.value?.find((a) => a.value === query.accountId); return ( @@ -80,13 +72,7 @@ export function Account({ query, onChange, api }: Props) { aria-label="Account" value={value} options={state.value} - onChange={({ value: accountArn }) => { - accountArn && - onChange({ - crossAccount: false, - account: accounts.find((a) => a.arn === accountArn), - }); - }} + onChange={({ value: accountId }) => accountId && onChange(accountId)} /> ); diff --git a/public/app/plugins/datasource/cloudwatch/components/Dimensions/FilterItem.tsx b/public/app/plugins/datasource/cloudwatch/components/Dimensions/FilterItem.tsx index 6fa2a829cd39..4dea46fead0f 100644 --- a/public/app/plugins/datasource/cloudwatch/components/Dimensions/FilterItem.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/Dimensions/FilterItem.tsx @@ -34,7 +34,7 @@ const excludeCurrentKey = (dimensions: Dimensions, currentKey: string | undefine export const FilterItem: FunctionComponent = ({ filter, - metricStat: { region, namespace, metricName, dimensions }, + metricStat: { region, namespace, metricName, dimensions, accountId }, datasource, dimensionKeys, disableExpressions, @@ -58,6 +58,7 @@ export const FilterItem: FunctionComponent = ({ region, namespace, metricName, + accountId, }) .then((result: Array>) => { if (result.length && !disableExpressions && !result.some((o) => o.value === wildcardOption.value)) { @@ -67,7 +68,14 @@ export const FilterItem: FunctionComponent = ({ }); }; - const [state, loadOptions] = useAsyncFn(loadDimensionValues, [filter.key, dimensions]); + const [state, loadOptions] = useAsyncFn(loadDimensionValues, [ + filter.key, + dimensions, + region, + namespace, + metricName, + accountId, + ]); const theme = useTheme2(); const styles = getOperatorStyles(theme); diff --git a/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.tsx b/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.tsx index 7996a5bafdab..8f0a742b3e5e 100644 --- a/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/MetricStatEditor/MetricStatEditor.tsx @@ -8,7 +8,7 @@ import { Dimensions } from '..'; import { CloudWatchDatasource } from '../../datasource'; import { useDimensionKeys, useMetrics, useNamespaces } from '../../hooks'; import { standardStatistics } from '../../standardStatistics'; -import { MetricStat, AccountInfo } from '../../types'; +import { MetricStat } from '../../types'; import { appendTemplateVariables, toOption } from '../../utils/utils'; import { Account } from '../Account'; @@ -29,9 +29,8 @@ export function MetricStatEditor({ onChange, onRunQuery, }: React.PropsWithChildren) { - const { region, namespace } = metricStat; const namespaces = useNamespaces(datasource); - const metrics = useMetrics(datasource, region, namespace); + const metrics = useMetrics(datasource, metricStat); const dimensionKeys = useDimensionKeys(datasource, { ...metricStat, dimensionFilters: metricStat.dimensions }); const onMetricStatChange = (metricStat: MetricStat) => { @@ -118,7 +117,7 @@ export function MetricStatEditor({ {!disableExpressions && ( onMetricStatChange({ ...metricStat, accountInfo })} + onChange={(accountId?: string) => onMetricStatChange({ ...metricStat, accountId })} api={datasource.api} > )} diff --git a/public/app/plugins/datasource/cloudwatch/components/SQLBuilderEditor/SQLBuilderSelectRow.tsx b/public/app/plugins/datasource/cloudwatch/components/SQLBuilderEditor/SQLBuilderSelectRow.tsx index b80a3a4c71b5..ab1a8551dc2a 100644 --- a/public/app/plugins/datasource/cloudwatch/components/SQLBuilderEditor/SQLBuilderSelectRow.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/SQLBuilderEditor/SQLBuilderSelectRow.tsx @@ -48,7 +48,7 @@ const SQLBuilderSelectRow: React.FC = ({ datasource, q const withSchemaEnabled = isUsingWithSchema(sql.from); const namespaceOptions = useNamespaces(datasource); - const metricOptions = useMetrics(datasource, query.region, namespace); + const metricOptions = useMetrics(datasource, { region: query.region, namespace }); const existingFilters = useMemo(() => stringArrayToDimensions(schemaLabels ?? []), [schemaLabels]); const unusedDimensionKeys = useDimensionKeys(datasource, { region: query.region, diff --git a/public/app/plugins/datasource/cloudwatch/components/VariableQueryEditor/VariableQueryEditor.tsx b/public/app/plugins/datasource/cloudwatch/components/VariableQueryEditor/VariableQueryEditor.tsx index bed581412ddc..0e6248402c61 100644 --- a/public/app/plugins/datasource/cloudwatch/components/VariableQueryEditor/VariableQueryEditor.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/VariableQueryEditor/VariableQueryEditor.tsx @@ -34,7 +34,7 @@ export const VariableQueryEditor = ({ query, datasource, onChange }: Props) => { const { region, namespace, metricName, dimensionKey, dimensionFilters } = parsedQuery; const [regions, regionIsLoading] = useRegions(datasource); const namespaces = useNamespaces(datasource); - const metrics = useMetrics(datasource, region, namespace); + const metrics = useMetrics(datasource, { region, namespace }); const dimensionKeys = useDimensionKeys(datasource, { region, namespace, metricName }); const keysForDimensionFilter = useDimensionKeys(datasource, { region, namespace, metricName, dimensionFilters }); diff --git a/public/app/plugins/datasource/cloudwatch/hooks.ts b/public/app/plugins/datasource/cloudwatch/hooks.ts index cbab7f44864a..acc97d714832 100644 --- a/public/app/plugins/datasource/cloudwatch/hooks.ts +++ b/public/app/plugins/datasource/cloudwatch/hooks.ts @@ -5,7 +5,7 @@ import { SelectableValue, toOption } from '@grafana/data'; import { CloudWatchAPI } from './api'; import { CloudWatchDatasource } from './datasource'; -import { GetDimensionKeysRequest } from './types'; +import { GetDimensionKeysRequest, GetMetricsRequest } from './types'; import { appendTemplateVariables } from './utils/utils'; export const useRegions = (datasource: CloudWatchDatasource): [Array>, boolean] => { @@ -40,31 +40,31 @@ export const useNamespaces = (datasource: CloudWatchDatasource) => { return namespaces; }; -export const useMetrics = (datasource: CloudWatchDatasource, region: string, namespace: string | undefined) => { +export const useMetrics = (datasource: CloudWatchDatasource, { region, namespace, accountId }: GetMetricsRequest) => { const [metrics, setMetrics] = useState>>([]); useEffect(() => { - datasource.api.getMetrics({ namespace, region }).then((result: Array>) => { + datasource.api.getMetrics({ namespace, region, accountId }).then((result: Array>) => { setMetrics(appendTemplateVariables(datasource, result)); }); - }, [datasource, region, namespace]); + }, [datasource, region, namespace, accountId]); return metrics; }; export const useDimensionKeys = ( datasource: CloudWatchDatasource, - { namespace, region, dimensionFilters, metricName }: GetDimensionKeysRequest + { namespace, region, dimensionFilters, metricName, accountId }: GetDimensionKeysRequest ) => { const [dimensionKeys, setDimensionKeys] = useState>>([]); // doing deep comparison to avoid making new api calls to list metrics unless dimension filter object props changes useDeepCompareEffect(() => { datasource.api - .getDimensionKeys({ namespace, region, dimensionFilters, metricName }) + .getDimensionKeys({ namespace, region, dimensionFilters, metricName, accountId }) .then((result: Array>) => { setDimensionKeys(appendTemplateVariables(datasource, result)); }); - }, [datasource, region, namespace, metricName, dimensionFilters]); + }, [datasource, region, namespace, metricName, dimensionFilters, accountId]); return dimensionKeys; }; diff --git a/public/app/plugins/datasource/cloudwatch/types.ts b/public/app/plugins/datasource/cloudwatch/types.ts index 456c7bc2953a..0cb45cafb701 100644 --- a/public/app/plugins/datasource/cloudwatch/types.ts +++ b/public/app/plugins/datasource/cloudwatch/types.ts @@ -57,11 +57,6 @@ export interface CloudWatchMetricsQuery extends MetricStat, DataQuery { sql?: SQLExpression; } -export interface AccountInfo { - crossAccount: boolean; - account?: Account; -} - export interface MetricStat { region: string; namespace: string; @@ -69,7 +64,7 @@ export interface MetricStat { dimensions?: Dimensions; matchExact?: boolean; period?: string; - accountInfo?: AccountInfo; + accountId?: string; statistic?: string; /** * @deprecated use statistic @@ -464,6 +459,7 @@ export interface MetricResponse { export interface ResourceRequest { region: string; + accountId?: string; } export interface GetDimensionKeysRequest extends ResourceRequest { From 6869664b1933efc6973a25ee9e808affaba39a3a Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Wed, 2 Nov 2022 14:14:20 +0100 Subject: [PATCH 18/43] rename request package (#626) --- pkg/tsdb/cloudwatch/clients/metrics.go | 4 +- pkg/tsdb/cloudwatch/clients/metrics_test.go | 8 ++-- .../cloudwatch/mocks/list_metrics_service.go | 2 +- pkg/tsdb/cloudwatch/models/metric_types.go | 0 pkg/tsdb/cloudwatch/models/request/types.go | 12 ----- ...st.go => dimension_values_request_test.go} | 0 .../cloudwatch/models/resources/metrics.go | 0 .../models/resources/metrics_test.go | 0 pkg/tsdb/cloudwatch/models/resources/types.go | 8 +++- pkg/tsdb/cloudwatch/models/types.go | 2 +- pkg/tsdb/cloudwatch/services/list_metrics.go | 6 +-- .../cloudwatch/services/list_metrics_test.go | 48 +++++++++---------- 12 files changed, 42 insertions(+), 48 deletions(-) create mode 100644 pkg/tsdb/cloudwatch/models/metric_types.go delete mode 100644 pkg/tsdb/cloudwatch/models/request/types.go rename pkg/tsdb/cloudwatch/models/resources/{dimension_values_rquest_test.go => dimension_values_request_test.go} (100%) create mode 100644 pkg/tsdb/cloudwatch/models/resources/metrics.go create mode 100644 pkg/tsdb/cloudwatch/models/resources/metrics_test.go diff --git a/pkg/tsdb/cloudwatch/clients/metrics.go b/pkg/tsdb/cloudwatch/clients/metrics.go index 1389f3046c63..4ea8cb0409d2 100644 --- a/pkg/tsdb/cloudwatch/clients/metrics.go +++ b/pkg/tsdb/cloudwatch/clients/metrics.go @@ -6,7 +6,7 @@ import ( "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" - "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/request" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/resources" ) type metricsClient struct { @@ -29,7 +29,7 @@ func (l *metricsClient) ListMetricsWithPageLimit(params *cloudwatch.ListMetricsI for idx, metric := range metrics { metric := &models.MetricOutput{Metric: metric.(*cloudwatch.Metric)} if len(page.OwningAccounts) >= idx && params.IncludeLinkedAccounts != nil && *params.IncludeLinkedAccounts { - metric.Account = &request.Account{ + metric.Account = &resources.Account{ Id: *page.OwningAccounts[idx], } } diff --git a/pkg/tsdb/cloudwatch/clients/metrics_test.go b/pkg/tsdb/cloudwatch/clients/metrics_test.go index 2e54b448f497..5cd3a32fa691 100644 --- a/pkg/tsdb/cloudwatch/clients/metrics_test.go +++ b/pkg/tsdb/cloudwatch/clients/metrics_test.go @@ -8,7 +8,7 @@ import ( "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/mocks" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" - "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/request" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/resources" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -60,9 +60,9 @@ func TestMetricsClient(t *testing.T) { response, err := client.ListMetricsWithPageLimit(&cloudwatch.ListMetricsInput{IncludeLinkedAccounts: aws.Bool(true)}) require.NoError(t, err) expected := []*models.MetricOutput{ - {Metric: &cloudwatch.Metric{MetricName: aws.String("Test_MetricName1")}, Account: &request.Account{Id: "1234567890"}}, - {Metric: &cloudwatch.Metric{MetricName: aws.String("Test_MetricName2")}, Account: &request.Account{Id: "1234567890"}}, - {Metric: &cloudwatch.Metric{MetricName: aws.String("Test_MetricName3")}, Account: &request.Account{Id: "1234567895"}}, + {Metric: &cloudwatch.Metric{MetricName: aws.String("Test_MetricName1")}, Account: &resources.Account{Id: "1234567890"}}, + {Metric: &cloudwatch.Metric{MetricName: aws.String("Test_MetricName2")}, Account: &resources.Account{Id: "1234567890"}}, + {Metric: &cloudwatch.Metric{MetricName: aws.String("Test_MetricName3")}, Account: &resources.Account{Id: "1234567895"}}, } assert.Equal(t, expected, response) }) diff --git a/pkg/tsdb/cloudwatch/mocks/list_metrics_service.go b/pkg/tsdb/cloudwatch/mocks/list_metrics_service.go index 7839b27a78cf..1fb7652e1c3c 100644 --- a/pkg/tsdb/cloudwatch/mocks/list_metrics_service.go +++ b/pkg/tsdb/cloudwatch/mocks/list_metrics_service.go @@ -28,7 +28,7 @@ func (a *ListMetricsServiceMock) GetDimensionKeysByNamespace(namespace string) ( return args.Get(0).([]models.ResourceResponse[string]), args.Error(1) } -func (a *ListMetricsServiceMock) GetMetricsByNamespace(r *request.MetricsRequest) ([]models.ResourceResponse[models.Metric], error) { +func (a *ListMetricsServiceMock) GetMetricsByNamespace(r *resources.MetricsRequest) ([]models.ResourceResponse[models.Metric], error) { args := a.Called(namespace) return args.Get(0).([]models.ResourceResponse[models.Metric]), args.Error(1) diff --git a/pkg/tsdb/cloudwatch/models/metric_types.go b/pkg/tsdb/cloudwatch/models/metric_types.go new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/pkg/tsdb/cloudwatch/models/request/types.go b/pkg/tsdb/cloudwatch/models/request/types.go deleted file mode 100644 index 5af65c28c40b..000000000000 --- a/pkg/tsdb/cloudwatch/models/request/types.go +++ /dev/null @@ -1,12 +0,0 @@ -package request - -type Dimension struct { - Name string - Value string -} - -type Account struct { - Id string `json:"id"` - Arn string `json:"arn,omitempty"` - Label string `json:"label,omitempty"` -} diff --git a/pkg/tsdb/cloudwatch/models/resources/dimension_values_rquest_test.go b/pkg/tsdb/cloudwatch/models/resources/dimension_values_request_test.go similarity index 100% rename from pkg/tsdb/cloudwatch/models/resources/dimension_values_rquest_test.go rename to pkg/tsdb/cloudwatch/models/resources/dimension_values_request_test.go diff --git a/pkg/tsdb/cloudwatch/models/resources/metrics.go b/pkg/tsdb/cloudwatch/models/resources/metrics.go new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/pkg/tsdb/cloudwatch/models/resources/metrics_test.go b/pkg/tsdb/cloudwatch/models/resources/metrics_test.go new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/pkg/tsdb/cloudwatch/models/resources/types.go b/pkg/tsdb/cloudwatch/models/resources/types.go index 328888ed50c3..a5a00c1ce4fc 100644 --- a/pkg/tsdb/cloudwatch/models/resources/types.go +++ b/pkg/tsdb/cloudwatch/models/resources/types.go @@ -10,7 +10,13 @@ type ResourceResponse[T any] struct { Value T `json:"value"` } +type Account struct { + Id string `json:"id"` + Arn string `json:"arn,omitempty"` + Label string `json:"label,omitempty"` +} + type Metric struct { Name string `json:"name"` Namespace string `json:"namespace"` -} + diff --git a/pkg/tsdb/cloudwatch/models/types.go b/pkg/tsdb/cloudwatch/models/types.go index c62fd6ed20d6..0062cd8b101e 100644 --- a/pkg/tsdb/cloudwatch/models/types.go +++ b/pkg/tsdb/cloudwatch/models/types.go @@ -5,7 +5,7 @@ import ( "github.com/aws/aws-sdk-go/service/cloudwatch" "github.com/grafana/grafana-plugin-sdk-go/backend" - "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/request" + "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models/resources" ) type RequestContext struct { diff --git a/pkg/tsdb/cloudwatch/services/list_metrics.go b/pkg/tsdb/cloudwatch/services/list_metrics.go index 92a713ca9c49..10d93366eed0 100644 --- a/pkg/tsdb/cloudwatch/services/list_metrics.go +++ b/pkg/tsdb/cloudwatch/services/list_metrics.go @@ -98,7 +98,7 @@ func (l *ListMetricsService) GetDimensionValuesByDimensionFilter(r resources.Dim return response, nil } -func (l *ListMetricsService) GetDimensionKeysByNamespace(r *request.DimensionKeysRequest) ([]models.ResourceResponse[string], error) { +func (l *ListMetricsService) GetDimensionKeysByNamespace(r *resources.DimensionKeysRequest) ([]models.ResourceResponse[string], error) { input := &cloudwatch.ListMetricsInput{Namespace: aws.String(r.Namespace)} setAccount(input, r.ResourceRequest) metrics, err := l.ListMetricsWithPageLimit(input) @@ -122,7 +122,7 @@ func (l *ListMetricsService) GetDimensionKeysByNamespace(r *request.DimensionKey return response, nil } -func (l *ListMetricsService) GetMetricsByNamespace(r *request.MetricsRequest) ([]models.ResourceResponse[models.Metric], error) { +func (l *ListMetricsService) GetMetricsByNamespace(r *resources.MetricsRequest) ([]models.ResourceResponse[models.Metric], error) { input := &cloudwatch.ListMetricsInput{Namespace: aws.String(r.Namespace)} setAccount(input, r.ResourceRequest) metrics, err := l.ListMetricsWithPageLimit(input) @@ -155,7 +155,7 @@ func setDimensionFilter(input *cloudwatch.ListMetricsInput, dimensionFilter []*r } } -func setAccount(input *cloudwatch.ListMetricsInput, r *request.ResourceRequest) { +func setAccount(input *cloudwatch.ListMetricsInput, r *resources.ResourceRequest) { if r != nil && r.AccountId != nil { input.IncludeLinkedAccounts = aws.Bool(true) if !r.ShouldTargetAllAccounts() { diff --git a/pkg/tsdb/cloudwatch/services/list_metrics_test.go b/pkg/tsdb/cloudwatch/services/list_metrics_test.go index 4f1c45baabe4..1839287ff9be 100644 --- a/pkg/tsdb/cloudwatch/services/list_metrics_test.go +++ b/pkg/tsdb/cloudwatch/services/list_metrics_test.go @@ -50,7 +50,7 @@ var metricResponse = []*models.MetricOutput{ }, } -type validateInputTestCase[T request.DimensionKeysRequest | request.DimensionValuesRequest] struct { +type validateInputTestCase[T resources.DimensionKeysRequest | resources.DimensionValuesRequest] struct { name string input *T listMetricsWithPageLimitInput *cloudwatch.ListMetricsInput @@ -73,14 +73,14 @@ func TestListMetricsService_GetDimensionKeysByDimensionFilter(t *testing.T) { assert.Equal(t, []models.ResourceResponse[string]{{Value: "InstanceType"}, {Value: "AutoScalingGroupName"}}, resp) }) - testCases := []validateInputTestCase[request.DimensionKeysRequest]{ + testCases := []validateInputTestCase[resources.DimensionKeysRequest]{ { name: "Should set account correctly on list metric input if it cross account is defined on the request", - input: &request.DimensionKeysRequest{ - ResourceRequest: &request.ResourceRequest{Region: "us-east-1", AccountId: stringPtr(useLinkedAccountsId)}, + input: &resources.DimensionKeysRequest{ + ResourceRequest: &resources.ResourceRequest{Region: "us-east-1", AccountId: stringPtr(useLinkedAccountsId)}, Namespace: "AWS/EC2", MetricName: "CPUUtilization", - DimensionFilter: []*request.Dimension{{Name: "InstanceId", Value: ""}}, + DimensionFilter: []*resources.Dimension{{Name: "InstanceId", Value: ""}}, }, listMetricsWithPageLimitInput: &cloudwatch.ListMetricsInput{ MetricName: aws.String("CPUUtilization"), @@ -91,11 +91,11 @@ func TestListMetricsService_GetDimensionKeysByDimensionFilter(t *testing.T) { }, { name: "Should set account correctly on list metric input if single account is defined on the request", - input: &request.DimensionKeysRequest{ - ResourceRequest: &request.ResourceRequest{Region: "us-east-1", AccountId: stringPtr("1234567890")}, + input: &resources.DimensionKeysRequest{ + ResourceRequest: &resources.ResourceRequest{Region: "us-east-1", AccountId: stringPtr("1234567890")}, Namespace: "AWS/EC2", MetricName: "CPUUtilization", - DimensionFilter: []*request.Dimension{{Name: "InstanceId", Value: ""}}, + DimensionFilter: []*resources.Dimension{{Name: "InstanceId", Value: ""}}, }, listMetricsWithPageLimitInput: &cloudwatch.ListMetricsInput{ MetricName: aws.String("CPUUtilization"), @@ -107,11 +107,11 @@ func TestListMetricsService_GetDimensionKeysByDimensionFilter(t *testing.T) { }, { name: "Should not set namespace and metricName on list metric input if empty strings are set for these in the request", - input: &request.DimensionKeysRequest{ - ResourceRequest: &request.ResourceRequest{Region: "us-east-1"}, + input: &resources.DimensionKeysRequest{ + ResourceRequest: &resources.ResourceRequest{Region: "us-east-1"}, Namespace: "", MetricName: "", - DimensionFilter: []*request.Dimension{{Name: "InstanceId", Value: ""}}, + DimensionFilter: []*resources.Dimension{{Name: "InstanceId", Value: ""}}, }, listMetricsWithPageLimitInput: &cloudwatch.ListMetricsInput{Dimensions: []*cloudwatch.DimensionFilter{{Name: aws.String("InstanceId")}}}, }, @@ -136,17 +136,17 @@ func TestListMetricsService_GetDimensionKeysByNamespace(t *testing.T) { fakeMetricsClient.On("ListMetricsWithPageLimit", mock.Anything).Return(metricResponse, nil) listMetricsService := NewListMetricsService(fakeMetricsClient) - resp, err := listMetricsService.GetDimensionKeysByNamespace(&request.DimensionKeysRequest{Namespace: "AWS/EC2"}) + resp, err := listMetricsService.GetDimensionKeysByNamespace(&resources.DimensionKeysRequest{Namespace: "AWS/EC2"}) require.NoError(t, err) assert.Equal(t, []models.ResourceResponse[string]{{Value: "InstanceId"}, {Value: "InstanceType"}, {Value: "AutoScalingGroupName"}}, resp) }) - testCases := []validateInputTestCase[request.DimensionKeysRequest]{ + testCases := []validateInputTestCase[resources.DimensionKeysRequest]{ { name: "Should set account correctly on list metric input if it cross account is defined on the request", - input: &request.DimensionKeysRequest{ - ResourceRequest: &request.ResourceRequest{Region: "us-east-1", AccountId: stringPtr(useLinkedAccountsId)}, + input: &resources.DimensionKeysRequest{ + ResourceRequest: &resources.ResourceRequest{Region: "us-east-1", AccountId: stringPtr(useLinkedAccountsId)}, Namespace: "AWS/EC2", }, listMetricsWithPageLimitInput: &cloudwatch.ListMetricsInput{ @@ -156,8 +156,8 @@ func TestListMetricsService_GetDimensionKeysByNamespace(t *testing.T) { }, { name: "Should set account correctly on list metric input if single account is defined on the request", - input: &request.DimensionKeysRequest{ - ResourceRequest: &request.ResourceRequest{Region: "us-east-1", AccountId: stringPtr("1234567890")}, + input: &resources.DimensionKeysRequest{ + ResourceRequest: &resources.ResourceRequest{Region: "us-east-1", AccountId: stringPtr("1234567890")}, Namespace: "AWS/EC2", }, listMetricsWithPageLimitInput: &cloudwatch.ListMetricsInput{ @@ -201,14 +201,14 @@ func TestListMetricsService_GetDimensionValuesByDimensionFilter(t *testing.T) { assert.Equal(t, []models.ResourceResponse[string]{{Value: "i-1234567890abcdef0"}, {Value: "i-5234567890abcdef0"}, {Value: "i-64234567890abcdef0"}}, resp) }) - testCases := []validateInputTestCase[request.DimensionValuesRequest]{ + testCases := []validateInputTestCase[resources.DimensionValuesRequest]{ { name: "Should set account correctly on list metric input if it cross account is defined on the request", - input: &request.DimensionValuesRequest{ - ResourceRequest: &request.ResourceRequest{Region: "us-east-1", AccountId: stringPtr(useLinkedAccountsId)}, + input: &resources.DimensionValuesRequest{ + ResourceRequest: &resources.ResourceRequest{Region: "us-east-1", AccountId: stringPtr(useLinkedAccountsId)}, Namespace: "AWS/EC2", MetricName: "CPUUtilization", - DimensionFilter: []*request.Dimension{{Name: "InstanceId", Value: ""}}, + DimensionFilter: []*resources.Dimension{{Name: "InstanceId", Value: ""}}, }, listMetricsWithPageLimitInput: &cloudwatch.ListMetricsInput{ MetricName: aws.String("CPUUtilization"), @@ -219,11 +219,11 @@ func TestListMetricsService_GetDimensionValuesByDimensionFilter(t *testing.T) { }, { name: "Should set account correctly on list metric input if single account is defined on the request", - input: &request.DimensionValuesRequest{ - ResourceRequest: &request.ResourceRequest{Region: "us-east-1", AccountId: stringPtr("1234567890")}, + input: &resources.DimensionValuesRequest{ + ResourceRequest: &resources.ResourceRequest{Region: "us-east-1", AccountId: stringPtr("1234567890")}, Namespace: "AWS/EC2", MetricName: "CPUUtilization", - DimensionFilter: []*request.Dimension{{Name: "InstanceId", Value: ""}}, + DimensionFilter: []*resources.Dimension{{Name: "InstanceId", Value: ""}}, }, listMetricsWithPageLimitInput: &cloudwatch.ListMetricsInput{ MetricName: aws.String("CPUUtilization"), From ca512aeac4dcff4c4a0790e3eb679a17f4dbef2a Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Thu, 3 Nov 2022 08:52:56 +0100 Subject: [PATCH 19/43] Lattice: Move account component and add tooltip (#630) * move accounts component to the top of metric stat editor * add tooltip --- .../datasource/cloudwatch/components/Account.tsx | 6 +++++- .../MetricStatEditor/MetricStatEditor.tsx | 14 +++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/public/app/plugins/datasource/cloudwatch/components/Account.tsx b/public/app/plugins/datasource/cloudwatch/components/Account.tsx index 0b3916893608..7eecd461708e 100644 --- a/public/app/plugins/datasource/cloudwatch/components/Account.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/Account.tsx @@ -66,7 +66,11 @@ export function Account({ query, onChange, api }: Props) { const value = state.value?.find((a) => a.value === query.accountId); return ( - + - {!disableExpressions && ( - onMetricStatChange({ ...metricStat, accountId })} - api={datasource.api} - > - )} Date: Thu, 3 Nov 2022 10:59:19 +0100 Subject: [PATCH 20/43] CloudWatch: add account to GetMetricData queries (#627) * Add AccountId to metric stat query --- .../cloudwatch/metric_data_query_builder.go | 14 ++- .../metric_data_query_builder_test.go | 99 ++++++++++++++++++- .../cloudwatch/models/cloudwatch_query.go | 7 ++ .../models/cloudwatch_query_test.go | 27 +++++ 4 files changed, 142 insertions(+), 5 deletions(-) diff --git a/pkg/tsdb/cloudwatch/metric_data_query_builder.go b/pkg/tsdb/cloudwatch/metric_data_query_builder.go index 5baf0bb872b9..b921a88739a7 100644 --- a/pkg/tsdb/cloudwatch/metric_data_query_builder.go +++ b/pkg/tsdb/cloudwatch/metric_data_query_builder.go @@ -50,6 +50,7 @@ func (e *cloudWatchExecutor) buildMetricDataQuery(logger log.Logger, query *mode }) } mdq.MetricStat.Stat = aws.String(query.Statistic) + mdq.AccountId = query.AccountId } if mdq.Expression != nil { @@ -98,18 +99,27 @@ func buildSearchExpression(query *models.CloudWatchQuery, stat string) string { searchTerm = appendSearch(searchTerm, keyFilter) } + var account string + if query.AccountId != nil && *query.AccountId != "all" { + account = fmt.Sprintf(":aws.AccountId=%q", *query.AccountId) + } + if query.MatchExact { schema := fmt.Sprintf("%q", query.Namespace) if len(dimensionNames) > 0 { sort.Strings(dimensionNames) schema += fmt.Sprintf(",%s", join(dimensionNames, ",", `"`, `"`)) } - return fmt.Sprintf("REMOVE_EMPTY(SEARCH('{%s} %s', '%s', %s))", schema, searchTerm, stat, strconv.Itoa(query.Period)) + schema = fmt.Sprintf("{%s}", schema) + schemaSearchTermAndAccount := strings.TrimSpace(strings.Join([]string{schema, searchTerm, account}, " ")) + return fmt.Sprintf("REMOVE_EMPTY(SEARCH('%s', '%s', %s))", schemaSearchTermAndAccount, stat, strconv.Itoa(query.Period)) } sort.Strings(dimensionNamesWithoutKnownValues) searchTerm = appendSearch(searchTerm, join(dimensionNamesWithoutKnownValues, " ", `"`, `"`)) - return fmt.Sprintf(`REMOVE_EMPTY(SEARCH('Namespace="%s" %s', '%s', %s))`, query.Namespace, searchTerm, stat, strconv.Itoa(query.Period)) + namespace := fmt.Sprintf("Namespace=%q", query.Namespace) + namespaceSearchTermAndAccount := strings.TrimSpace(strings.Join([]string{namespace, searchTerm, account}, " ")) + return fmt.Sprintf(`REMOVE_EMPTY(SEARCH('%s', '%s', %s))`, namespaceSearchTermAndAccount, stat, strconv.Itoa(query.Period)) } func escapeDoubleQuotes(arr []string) []string { diff --git a/pkg/tsdb/cloudwatch/metric_data_query_builder_test.go b/pkg/tsdb/cloudwatch/metric_data_query_builder_test.go index b13c389504c7..84b827ec9254 100644 --- a/pkg/tsdb/cloudwatch/metric_data_query_builder_test.go +++ b/pkg/tsdb/cloudwatch/metric_data_query_builder_test.go @@ -3,11 +3,11 @@ package cloudwatch import ( "testing" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - + "github.com/aws/aws-sdk-go/aws" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/tsdb/cloudwatch/models" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestMetricDataQueryBuilder(t *testing.T) { @@ -24,6 +24,27 @@ func TestMetricDataQueryBuilder(t *testing.T) { assert.Equal(t, query.Namespace, *mdq.MetricStat.Metric.Namespace) }) + t.Run("should pass AccountId in metric stat query", func(t *testing.T) { + executor := newExecutor(nil, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures()) + query := getBaseQuery() + query.MetricEditorMode = models.MetricEditorModeBuilder + query.MetricQueryType = models.MetricQueryTypeSearch + query.AccountId = aws.String("some account id") + mdq, err := executor.buildMetricDataQuery(logger, query) + require.NoError(t, err) + assert.Equal(t, "some account id", *mdq.AccountId) + }) + + t.Run("should leave AccountId in metric stat query", func(t *testing.T) { + executor := newExecutor(nil, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures()) + query := getBaseQuery() + query.MetricEditorMode = models.MetricEditorModeBuilder + query.MetricQueryType = models.MetricQueryTypeSearch + mdq, err := executor.buildMetricDataQuery(logger, query) + require.NoError(t, err) + assert.Nil(t, mdq.AccountId) + }) + t.Run("should use custom built expression", func(t *testing.T) { executor := newExecutor(nil, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures()) query := getBaseQuery() @@ -112,6 +133,42 @@ func TestMetricDataQueryBuilder(t *testing.T) { assert.Nil(t, mdq.Label) }) } + + t.Run(`should not specify accountId when it is "all"`, func(t *testing.T) { + executor := newExecutor(nil, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures(featuremgmt.FlagCloudWatchDynamicLabels)) + query := &models.CloudWatchQuery{ + Namespace: "AWS/EC2", + MetricName: "CPUUtilization", + Statistic: "Average", + Period: 60, + MatchExact: false, + AccountId: aws.String("all"), + } + + mdq, err := executor.buildMetricDataQuery(logger, query) + + assert.NoError(t, err) + require.Nil(t, mdq.MetricStat) + assert.Equal(t, `REMOVE_EMPTY(SEARCH('Namespace="AWS/EC2" MetricName="CPUUtilization"', 'Average', 60))`, *mdq.Expression) + }) + + t.Run("should set accountId when it is specified", func(t *testing.T) { + executor := newExecutor(nil, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures(featuremgmt.FlagCloudWatchDynamicLabels)) + query := &models.CloudWatchQuery{ + Namespace: "AWS/EC2", + MetricName: "CPUUtilization", + Statistic: "Average", + Period: 60, + MatchExact: false, + AccountId: aws.String("12345"), + } + + mdq, err := executor.buildMetricDataQuery(logger, query) + + assert.NoError(t, err) + require.Nil(t, mdq.MetricStat) + assert.Equal(t, `REMOVE_EMPTY(SEARCH('Namespace="AWS/EC2" MetricName="CPUUtilization" :aws.AccountId="12345"', 'Average', 60))`, *mdq.Expression) + }) }) t.Run("Query should be matched exact", func(t *testing.T) { @@ -199,6 +256,24 @@ func TestMetricDataQueryBuilder(t *testing.T) { assert.Equal(t, `REMOVE_EMPTY(SEARCH('{"AWS/EC2","InstanceId","LoadBalancer"} MetricName="CPUUtilization" "LoadBalancer"=("lb1" OR "lb2" OR "lb3")', 'Average', 300))`, res) }) + t.Run("Query has multiple dimensions and an account Id", func(t *testing.T) { + query := &models.CloudWatchQuery{ + Namespace: "AWS/EC2", + MetricName: "CPUUtilization", + Dimensions: map[string][]string{ + "LoadBalancer": {"lb1", "lb2", "lb3"}, + "InstanceId": {"i-123", "*", "i-789"}, + }, + Period: 300, + Expression: "", + MatchExact: matchExact, + AccountId: aws.String("some account id"), + } + + res := buildSearchExpression(query, "Average") + assert.Equal(t, `REMOVE_EMPTY(SEARCH('{"AWS/EC2","InstanceId","LoadBalancer"} MetricName="CPUUtilization" "LoadBalancer"=("lb1" OR "lb2" OR "lb3") :aws.AccountId="some account id"', 'Average', 300))`, res) + }) + t.Run("Query has a dimension key with a space", func(t *testing.T) { query := &models.CloudWatchQuery{ Namespace: "AWS/Kafka", @@ -301,6 +376,24 @@ func TestMetricDataQueryBuilder(t *testing.T) { res := buildSearchExpression(query, "Average") assert.Equal(t, `REMOVE_EMPTY(SEARCH('Namespace="AWS/EC2" MetricName="CPUUtilization" "LoadBalancer"=("lb1" OR "lb2" OR "lb3") "InstanceId"', 'Average', 300))`, res) }) + + t.Run("query has multiple dimensions and an account Id", func(t *testing.T) { + query := &models.CloudWatchQuery{ + Namespace: "AWS/EC2", + MetricName: "CPUUtilization", + Dimensions: map[string][]string{ + "LoadBalancer": {"lb1", "lb2", "lb3"}, + "InstanceId": {"i-123", "*", "i-789"}, + }, + Period: 300, + Expression: "", + MatchExact: matchExact, + AccountId: aws.String("some account id"), + } + + res := buildSearchExpression(query, "Average") + assert.Equal(t, `REMOVE_EMPTY(SEARCH('Namespace="AWS/EC2" MetricName="CPUUtilization" "LoadBalancer"=("lb1" OR "lb2" OR "lb3") "InstanceId" :aws.AccountId="some account id"', 'Average', 300))`, res) + }) }) t.Run("Query has invalid characters in dimension values", func(t *testing.T) { diff --git a/pkg/tsdb/cloudwatch/models/cloudwatch_query.go b/pkg/tsdb/cloudwatch/models/cloudwatch_query.go index 248796e35b55..121b148668c6 100644 --- a/pkg/tsdb/cloudwatch/models/cloudwatch_query.go +++ b/pkg/tsdb/cloudwatch/models/cloudwatch_query.go @@ -60,6 +60,7 @@ type CloudWatchQuery struct { TimezoneUTCOffset string MetricQueryType MetricQueryType MetricEditorMode MetricEditorMode + AccountId *string } func (q *CloudWatchQuery) GetGMDAPIMode(logger log.Logger) GMDApiMode { @@ -95,6 +96,10 @@ func (q *CloudWatchQuery) IsInferredSearchExpression() bool { return false } + if q.AccountId != nil && *q.AccountId == "all" { + return true + } + if len(q.Dimensions) == 0 { return !q.MatchExact } @@ -214,6 +219,7 @@ type metricsDataQuery struct { QueryType string `json:"type"` Hide *bool `json:"hide"` Alias string `json:"alias"` + AccountId *string `json:"accountId"` } // ParseMetricDataQueries decodes the metric data queries json, validates, sets default values and returns an array of CloudWatchQueries. @@ -248,6 +254,7 @@ func ParseMetricDataQueries(dataQueries []backend.DataQuery, startTime time.Time SqlExpression: mdq.SqlExpression, TimezoneUTCOffset: mdq.TimezoneUTCOffset, Expression: mdq.Expression, + AccountId: mdq.AccountId, } if err := cwQuery.validateAndSetDefaults(refId, mdq, startTime, endTime); err != nil { diff --git a/pkg/tsdb/cloudwatch/models/cloudwatch_query_test.go b/pkg/tsdb/cloudwatch/models/cloudwatch_query_test.go index d2467b2ed466..aec81d3f3c3b 100644 --- a/pkg/tsdb/cloudwatch/models/cloudwatch_query_test.go +++ b/pkg/tsdb/cloudwatch/models/cloudwatch_query_test.go @@ -620,6 +620,18 @@ func Test_ParseMetricDataQueries_query_type_and_metric_editor_mode_and_GMD_query expectedMetricEditorMode: dummyTestEditorMode, expectedGMDApiMode: GMDApiModeMetricStat, }, + "no dimensions, matchExact is false": { + extraDataQueryJson: `"matchExact":false,`, + expectedMetricQueryType: MetricQueryTypeSearch, + expectedMetricEditorMode: MetricEditorModeBuilder, + expectedGMDApiMode: GMDApiModeInferredSearchExpression, + }, + "query metricQueryType": { + extraDataQueryJson: `"metricQueryType":1,`, + expectedMetricQueryType: MetricQueryTypeQuery, + expectedMetricEditorMode: MetricEditorModeBuilder, + expectedGMDApiMode: GMDApiModeSQLExpression, + }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { @@ -1105,3 +1117,18 @@ func Test_ParseMetricDataQueries_statistics_and_query_type_validation_and_MatchE assert.False(t, actual[0].MatchExact) }) } + +func Test_ParseMetricDataQueries_sets_account_id(t *testing.T) { + actual, err := ParseMetricDataQueries( + []backend.DataQuery{ + { + JSON: []byte(`{"accountId":"some account id", "statistic":"Average"}`), + }, + }, time.Now(), time.Now(), false) + assert.NoError(t, err) + + require.Len(t, actual, 1) + require.NotNil(t, actual[0]) + require.NotNil(t, actual[0].AccountId) + assert.Equal(t, "some account id", *actual[0].AccountId) +} From 70e23a8b97341adda7fe077f18ed07a8cdfd310e Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Thu, 3 Nov 2022 15:31:43 +0100 Subject: [PATCH 21/43] Lattice: Account variable support (#625) * add variable support in accounts component * add account variable query type * update variables * interpolate variable before its sent to backend * handle variable change in hooks * remove not used import * Update public/app/plugins/datasource/cloudwatch/components/Account.tsx Co-authored-by: Sarah Zinger * Update public/app/plugins/datasource/cloudwatch/hooks.ts Co-authored-by: Sarah Zinger * add one more unit test Co-authored-by: Sarah Zinger --- .../__mocks__/CloudWatchDataSource.ts | 15 ++- .../cloudwatch/components/Account.test.tsx | 59 ++++++--- .../cloudwatch/components/Account.tsx | 18 ++- .../VariableQueryEditor.tsx | 5 + .../datasource/cloudwatch/datasource.ts | 4 +- .../datasource/cloudwatch/hooks.test.ts | 91 ++++++++++++++ .../plugins/datasource/cloudwatch/hooks.ts | 42 ++++++- .../CloudWatchMetricsQueryRunner.test.ts | 20 +++ .../CloudWatchMetricsQueryRunner.ts | 42 +++---- .../query-runner/CloudWatchRequest.ts | 4 + .../plugins/datasource/cloudwatch/types.ts | 1 + .../datasource/cloudwatch/variables.test.ts | 24 ++++ .../datasource/cloudwatch/variables.ts | 115 ++++++++---------- 13 files changed, 323 insertions(+), 117 deletions(-) create mode 100644 public/app/plugins/datasource/cloudwatch/hooks.test.ts diff --git a/public/app/plugins/datasource/cloudwatch/__mocks__/CloudWatchDataSource.ts b/public/app/plugins/datasource/cloudwatch/__mocks__/CloudWatchDataSource.ts index 7cf43cee3463..192561b8e1b8 100644 --- a/public/app/plugins/datasource/cloudwatch/__mocks__/CloudWatchDataSource.ts +++ b/public/app/plugins/datasource/cloudwatch/__mocks__/CloudWatchDataSource.ts @@ -71,7 +71,7 @@ export function setupMockedDataSource({ if (variables) { templateService = setupMockedTemplateService(variables); if (mockGetVariableName) { - templateService.getVariableName = (name: string) => name; + templateService.getVariableName = (name: string) => name.replace('$', ''); } } @@ -242,3 +242,16 @@ export const periodIntervalVariable: CustomVariableModel = { hide: VariableHide.dontHide, type: 'custom', }; + +export const accountIdVariable: CustomVariableModel = { + ...initialCustomVariableModelState, + id: 'accountId', + name: 'accountId', + current: { + value: 'templatedaccountId', + text: 'templatedaccountId', + selected: true, + }, + options: [{ value: 'templatedRegion', text: 'templatedRegion', selected: true }], + multi: false, +}; diff --git a/public/app/plugins/datasource/cloudwatch/components/Account.test.tsx b/public/app/plugins/datasource/cloudwatch/components/Account.test.tsx index 6f754054d9a8..09f85ec382e1 100644 --- a/public/app/plugins/datasource/cloudwatch/components/Account.test.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/Account.test.tsx @@ -4,6 +4,7 @@ import React from 'react'; import { config } from '@grafana/runtime'; import { setupMockedAPI } from '../__mocks__/API'; +import { accountIdVariable, regionVariable } from '../__mocks__/CloudWatchDataSource'; import { MetricStat } from '../types'; import { Account } from './Account'; @@ -12,18 +13,18 @@ export const accounts = [ { arn: 'arn:aws:iam::123456789012:root', id: '123456789012', - label: 'test-account', + label: 'test-account1', isMonitoringAccount: true, }, { arn: 'arn:aws:iam::432156789012:root', - id: '432156789012', + id: '432156789013', label: 'test-account2', isMonitoringAccount: false, }, { arn: 'arn:aws:iam::432156789013:root', - id: '432156789013', + id: '432156789014', label: 'test-account3', isMonitoringAccount: false, }, @@ -73,17 +74,30 @@ describe('Account', () => { const originalValue = config.featureToggles.cloudwatchCrossAccountQuerying; config.featureToggles.cloudwatchCrossAccountQuerying = false; await act(async () => { - const { container } = render(); - expect(container).toBeEmptyDOMElement(); - config.featureToggles.cloudwatchCrossAccountQuerying = originalValue; + render(); + }); + expect(await screen.queryByLabelText('Account')).toBeNull(); + config.featureToggles.cloudwatchCrossAccountQuerying = originalValue; + }); + + it('should not be rendered when no accounts are found and accountId is defiend', async () => { + await act(async () => { + const mock = setupMockedAPI({ variables: [accountIdVariable] }); + mock.api.getAccounts = jest.fn().mockResolvedValue([]); + await act(async () => { + render(); + }); + expect(await screen.queryByLabelText('Account')).toBeNull(); }); }); - it('should not be rendered when no accounts are found', async () => { + it('should not be rendered when no accounts are found and accountId is not defiend', async () => { await act(async () => { - const mock = setupMockedAPI(); + const mock = setupMockedAPI({ variables: [accountIdVariable] }); mock.api.getAccounts = jest.fn().mockResolvedValue([]); - const { container } = render(); + const { container } = render( + + ); expect(container).toBeEmptyDOMElement(); }); }); @@ -168,23 +182,38 @@ describe('Account', () => { onChange={onChange} query={{ ...props.metricStat, - accountId: '432156789012', + accountId: accounts[1].id, }} api={api} /> ); }); expect(onChange).not.toHaveBeenCalled(); - expect(await screen.getByText('test-account2')).toBeInTheDocument(); + expect(await screen.getByText(accounts[1].label)).toBeInTheDocument(); }); - it('should add "Monitoring account" text to the display label if its a monitoring account', async () => { - const api = setupMockedAPI().api; + it('should display variable name in ui and not call onChange in case variable value exist in returned accounts array', async () => { + const api = setupMockedAPI({ variables: [regionVariable, accountIdVariable] }).api; + const onChange = jest.fn(); api.getAccounts = jest.fn().mockResolvedValue(accounts); await act(async () => { - render(); + render(); + }); + expect(await screen.getByText('$accountId')).toBeInTheDocument(); + expect(onChange).not.toHaveBeenCalled(); + }); + + it('should not display variable name in ui and call onChange in case variable value does not exist in returned accounts array', async () => { + const api = setupMockedAPI({ variables: [regionVariable, accountIdVariable] }).api; + const onChange = jest.fn(); + api.getAccounts = jest.fn().mockResolvedValue(accounts); + await act(async () => { + render( + + ); }); - expect(await screen.getByText('test-account (Monitoring account)')).toBeInTheDocument(); + expect(await screen.queryByText('$unknownVariable')).toBeNull(); + expect(onChange).toHaveBeenCalled(); }); }); }); diff --git a/public/app/plugins/datasource/cloudwatch/components/Account.tsx b/public/app/plugins/datasource/cloudwatch/components/Account.tsx index 7eecd461708e..8893684cb4ef 100644 --- a/public/app/plugins/datasource/cloudwatch/components/Account.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/Account.tsx @@ -1,7 +1,7 @@ import React, { useEffect } from 'react'; import { useAsyncFn } from 'react-use'; -import { SelectableValue } from '@grafana/data'; +import { SelectableValue, toOption } from '@grafana/data'; import { EditorField } from '@grafana/experimental'; import { config } from '@grafana/runtime'; import { Select } from '@grafana/ui'; @@ -15,13 +15,18 @@ export interface Props { api: CloudWatchAPI; } -const allOption: SelectableValue = { +export const allOption: SelectableValue = { label: 'All', value: 'all', description: 'Target all linked accounts', }; export function Account({ query, onChange, api }: Props) { + const variableOptions = api.getVariables().map(toOption); + const variableOptionGroup: SelectableValue = { + label: 'Template Variables', + options: variableOptions, + }; const fetchAccounts = () => api .getAccounts({ region: query.region }) @@ -32,15 +37,15 @@ export function Account({ query, onChange, api }: Props) { } const options = accounts.map((a) => ({ - label: `${a.label}${a.isMonitoringAccount ? ' (Monitoring account)' : ''}`, + label: a.label, value: a.id, description: a.id, })); - if (!options.find((o) => o.value === query?.accountId)) { + if (![...options, ...variableOptions].find((o) => o.value === query.accountId)) { onChange(allOption.value); } - return options.length ? [allOption, ...options] : options; + return options.length ? [allOption, ...options, variableOptionGroup] : []; }) .catch(() => { query.accountId && onChange(undefined); @@ -63,7 +68,7 @@ export function Account({ query, onChange, api }: Props) { return null; } - const value = state.value?.find((a) => a.value === query.accountId); + const value = [...state.value, ...variableOptions].find((o) => o.value === query.accountId); return ( } + value={searchFilter} + onChange={(event) => { + const searchPhrase = event.currentTarget.value; + setSearchFilter(searchPhrase); + debouncedSearch(searchPhrase); + }} + placeholder="search by log group name prefix" + /> + ); +}; + +export default Search; diff --git a/public/app/plugins/datasource/cloudwatch/api.ts b/public/app/plugins/datasource/cloudwatch/api.ts index a65c7211f39e..0a40b5e5321c 100644 --- a/public/app/plugins/datasource/cloudwatch/api.ts +++ b/public/app/plugins/datasource/cloudwatch/api.ts @@ -65,6 +65,7 @@ export class CloudWatchAPI extends CloudWatchRequest { return this.memoizedGetRequest('log-groups', { ...params, region: this.templateSrv.replace(this.getActualRegion(params.region)), + accountId: this.templateSrv.replace(params.accountId), }); } diff --git a/public/app/plugins/datasource/cloudwatch/components/Account.test.tsx b/public/app/plugins/datasource/cloudwatch/components/Account.test.tsx index fc83e04bfd83..e2803524fd1a 100644 --- a/public/app/plugins/datasource/cloudwatch/components/Account.test.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/Account.test.tsx @@ -1,219 +1,65 @@ -import { act, render, screen } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; import React from 'react'; - -import { config } from '@grafana/runtime'; - -import { setupMockedAPI } from '../__mocks__/API'; -import { accountIdVariable, regionVariable } from '../__mocks__/CloudWatchDataSource'; -import { MetricStat } from '../types'; +import selectEvent from 'react-select-event'; import { Account } from './Account'; -export const accounts = [ +export const AccountOptions = [ { - arn: 'arn:aws:iam::123456789012:root', - id: '123456789012', + value: '123456789', label: 'test-account1', - isMonitoringAccount: true, + description: '123456789', }, { - arn: 'arn:aws:iam::432156789012:root', - id: '432156789013', + value: '432156789013', label: 'test-account2', - isMonitoringAccount: false, + description: '432156789013', }, { - arn: 'arn:aws:iam::432156789013:root', - id: '432156789014', + value: '999999999999', label: 'test-account3', - isMonitoringAccount: false, + description: '999999999999', + }, + { + label: 'Template Variables', + options: [ + { + value: '$fakeVar', + label: '$fakeVar', + }, + ], }, ]; - describe('Account', () => { - const metricStat: MetricStat = { - region: 'us-east-2', - namespace: '', - metricName: '', - dimensions: {}, - statistic: '', - matchExact: true, - accountId: '123456789012', - }; - const props = { - api: setupMockedAPI().api, - metricStat, + accountOptions: AccountOptions, + region: 'us-east-2', onChange: jest.fn(), + accountId: '123456789012', }; - describe('account field', () => { - config.featureToggles.cloudWatchCrossAccountQuerying = true; - it('should be rendered when feature toggle is enabled', async () => { - const originalValue = config.featureToggles.cloudWatchCrossAccountQuerying; - config.featureToggles.cloudWatchCrossAccountQuerying = true; - const api = setupMockedAPI().api; - api.getAccounts = jest.fn().mockResolvedValue([ - { - arn: 'arn:aws:iam::123456789012:root', - accountId: '123456789012', - label: 'test-account', - isMonitoringAccount: true, - }, - ]); - await act(async () => { - render(); - }); - expect(await screen.getByLabelText('Account')).toBeInTheDocument(); - config.featureToggles.cloudWatchCrossAccountQuerying = originalValue; - }); - - it('should not be rendered when feature toggle is not enabled', async () => { - let api = setupMockedAPI().api; - api.getAccounts = jest.fn().mockResolvedValue(accounts); - const originalValue = config.featureToggles.cloudWatchCrossAccountQuerying; - config.featureToggles.cloudWatchCrossAccountQuerying = false; - await act(async () => { - render(); - }); - expect(await screen.queryByLabelText('Account')).toBeNull(); - config.featureToggles.cloudWatchCrossAccountQuerying = originalValue; - }); - - it('should not be rendered when no accounts are found and accountId is defiend', async () => { - await act(async () => { - const mock = setupMockedAPI({ variables: [accountIdVariable] }); - mock.api.getAccounts = jest.fn().mockResolvedValue([]); - await act(async () => { - render(); - }); - expect(await screen.queryByLabelText('Account')).toBeNull(); - }); - }); - - it('should not be rendered when no accounts are found and accountId is not defiend', async () => { - await act(async () => { - const mock = setupMockedAPI({ variables: [accountIdVariable] }); - mock.api.getAccounts = jest.fn().mockResolvedValue([]); - const { container } = render( - - ); - expect(container).toBeEmptyDOMElement(); - }); - }); - - it('should not be rendered and should unset accountArn when api returns an error', async () => { - const mock = setupMockedAPI(); - mock.api.getAccounts = jest.fn().mockRejectedValue(new Error('error')); - const onChange = jest.fn(); - await act(async () => { - const { container } = render(); - expect(container).toBeEmptyDOMElement(); - }); - expect(onChange).toHaveBeenCalledWith(undefined); - }); - - it('should not be rendered and should unset accountArn when api returns an empty list of accounts', async () => { - const mock = setupMockedAPI(); - mock.api.getAccounts = jest.fn().mockResolvedValue([]); - const onChange = jest.fn(); - await act(async () => { - const { container } = render( - - ); - expect(container).toBeEmptyDOMElement(); - }); - expect(onChange).toHaveBeenCalledWith(undefined); - }); - - it('should set accountArn to "all" if the current value is not in the loaded list of accounts', async () => { - const api = setupMockedAPI().api; - api.getAccounts = jest.fn().mockResolvedValue(accounts); - const onChange = jest.fn(); - await act(async () => { - render( - - ); - }); - expect(onChange).toHaveBeenCalledWith('all'); - }); - - it('should render "all" if crossAccount is stored in the query model', async () => { - const api = setupMockedAPI().api; - api.getAccounts = jest.fn().mockResolvedValue(accounts); - const onChange = jest.fn(); - await act(async () => { - render( - - ); - }); - expect(onChange).not.toHaveBeenCalledWith(); - expect(await screen.getByText('All')).toBeInTheDocument(); - }); + it('should not render if there are no accounts', async () => { + render(); + expect(screen.queryByLabelText('Account Selection')).not.toBeInTheDocument(); + }); - it('should not unset accountId if the current value is in the loaded list of accounts', async () => { - const api = setupMockedAPI().api; - api.getAccounts = jest.fn().mockResolvedValue(accounts); - const onChange = jest.fn(); - await act(async () => { - render( - - ); - }); - expect(onChange).not.toHaveBeenCalled(); - expect(await screen.getByText(accounts[1].label)).toBeInTheDocument(); - }); + it('should render a selectable field of accounts if there are accounts', async () => { + const onChange = jest.fn(); + render(); + expect(screen.getByLabelText('Account Selection')).toBeInTheDocument(); + await selectEvent.select(screen.getByLabelText('Account Selection'), 'test-account3', { container: document.body }); + expect(onChange).toBeCalledWith('999999999999'); + }); - it('should display variable name in ui and not call onChange in case variable value exist in returned accounts array', async () => { - const api = setupMockedAPI({ variables: [regionVariable, accountIdVariable] }).api; - const onChange = jest.fn(); - api.getAccounts = jest.fn().mockResolvedValue(accounts); - await act(async () => { - render(); - }); - expect(await screen.getByText('$accountId')).toBeInTheDocument(); - expect(onChange).not.toHaveBeenCalled(); - }); + it("should default to 'all' if there is no selection", () => { + render(); + expect(screen.getByLabelText('Account Selection')).toBeInTheDocument(); + expect(screen.getByText('All')).toBeInTheDocument(); + }); - it('should not display variable name in ui and call onChange in case variable value does not exist in returned accounts array', async () => { - const api = setupMockedAPI({ variables: [regionVariable, accountIdVariable] }).api; - const onChange = jest.fn(); - api.getAccounts = jest.fn().mockResolvedValue(accounts); - await act(async () => { - render( - - ); - }); - expect(await screen.queryByText('$unknownVariable')).toBeNull(); - expect(onChange).toHaveBeenCalled(); - }); + it('should select an uninterpolated template variable if it has been selected', () => { + render(); + expect(screen.getByLabelText('Account Selection')).toBeInTheDocument(); + expect(screen.getByText('$fakeVar')).toBeInTheDocument(); }); }); diff --git a/public/app/plugins/datasource/cloudwatch/components/Account.tsx b/public/app/plugins/datasource/cloudwatch/components/Account.tsx index 0c9e22bce395..57cde6ac83aa 100644 --- a/public/app/plugins/datasource/cloudwatch/components/Account.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/Account.tsx @@ -1,75 +1,40 @@ -import React, { useEffect } from 'react'; -import { useAsyncFn } from 'react-use'; +import React, { useMemo } from 'react'; -import { SelectableValue, toOption } from '@grafana/data'; +import { SelectableValue } from '@grafana/data'; import { EditorField } from '@grafana/experimental'; -import { config } from '@grafana/runtime'; import { Select } from '@grafana/ui'; -import { CloudWatchAPI } from '../api'; -import { MetricStat } from '../types'; - export interface Props { onChange: (accountId?: string) => void; - query: MetricStat; - api: CloudWatchAPI; + accountOptions: Array>; + accountId?: string; } -export const allOption: SelectableValue = { +export const ALL_ACCOUNTS_OPTION = { label: 'All', value: 'all', description: 'Target all linked accounts', }; -export function Account({ query, onChange, api }: Props) { - const variableOptions = api.getVariables().map(toOption); - const variableOptionGroup: SelectableValue = { - label: 'Template Variables', - options: variableOptions, - }; - const fetchAccounts = () => - api - .getAccounts({ region: query.region }) - .then((accounts) => { - if (!accounts.length && query.accountId) { - onChange(undefined); - return []; - } - - const options = accounts.map((a) => ({ - label: a.label, - value: a.id, - description: a.id, - })); - - if (![...options, ...variableOptions].find((o) => o.value === query.accountId)) { - onChange(allOption.value); +export function Account({ accountId, onChange, accountOptions }: Props) { + const selectedAccountExistsInOptions = useMemo( + () => + accountOptions.find((a) => { + if (a.options) { + const matchingTemplateVar = a.options.find((tempVar: SelectableValue) => { + return tempVar.value === accountId; + }); + return matchingTemplateVar; } - return options.length ? [allOption, ...options, variableOptionGroup] : []; - }) - .catch(() => { - query.accountId && onChange(undefined); - return []; - }); - - const [state, doFetch] = useAsyncFn(fetchAccounts, [api, query.region]); - - useEffect(() => { - if (config.featureToggles.cloudWatchCrossAccountQuerying) { - doFetch(); - } - }, [api, query.region, doFetch]); - - if (!config.featureToggles.cloudWatchCrossAccountQuerying) { - return null; - } + return a.value === accountId; + }), + [accountOptions, accountId] + ); - if (!state.value?.length) { + if (accountOptions.length === 0) { return null; } - const value = [...state.value, ...variableOptions].find((o) => o.value === query.accountId); - return (