CloudWatch: Use context in aws DescribeRegionsWithContext (#76922)

* Use context in aws DescribeRegionsWithContext

In the current way, DescribeRegions is used which doesn't allow
cancelling the request if the context changes. Using
DescribeRegionsWithContext is the preferred way.

* Fix context variable

* Revert GetRegionsWithContext to GetRegions

GetRegions is not an AWS SDK method. Hence, GetRegions should be enough
as the name change is not needed for context implementation.
pull/76975/head
Shabeeb Khalid 2 years ago committed by GitHub
parent 8f96d23eee
commit 9dc6cac1f2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 2
      pkg/tsdb/cloudwatch/cloudwatch_integration_test.go
  2. 2
      pkg/tsdb/cloudwatch/metric_find_query.go
  3. 6
      pkg/tsdb/cloudwatch/metric_find_query_test.go
  4. 6
      pkg/tsdb/cloudwatch/mocks/regions.go
  5. 4
      pkg/tsdb/cloudwatch/models/api.go
  6. 2
      pkg/tsdb/cloudwatch/routes/regions.go
  7. 8
      pkg/tsdb/cloudwatch/services/regions.go
  8. 9
      pkg/tsdb/cloudwatch/services/regions_test.go
  9. 4
      pkg/tsdb/cloudwatch/test_utils.go

@ -39,7 +39,7 @@ func Test_CloudWatch_CallResource_Integration_Test(t *testing.T) {
return &logApi
}
ec2Mock := &mocks.EC2Mock{}
ec2Mock.On("DescribeRegions", mock.Anything, mock.Anything).Return(&ec2.DescribeRegionsOutput{}, nil)
ec2Mock.On("DescribeRegionsWithContext", mock.Anything, mock.Anything).Return(&ec2.DescribeRegionsOutput{}, nil)
NewEC2Client = func(provider client.ConfigProvider) models.EC2APIProvider {
return ec2Mock
}

@ -60,7 +60,7 @@ func (e *cloudWatchExecutor) handleGetRegions(ctx context.Context, pluginCtx bac
return nil, err
}
regions := constants.Regions()
ec2Regions, err := client.DescribeRegions(&ec2.DescribeRegionsInput{})
ec2Regions, err := client.DescribeRegionsWithContext(ctx, &ec2.DescribeRegionsInput{})
if err != nil {
// ignore error for backward compatibility
logger.Error("Failed to get regions", "error", err)

@ -39,7 +39,7 @@ func TestQuery_Regions(t *testing.T) {
}
t.Run("An extra region", func(t *testing.T) {
const regionName = "xtra-region"
ec2Mock.On("DescribeRegions", mock.Anything, mock.Anything).Return(&ec2.DescribeRegionsOutput{
ec2Mock.On("DescribeRegionsWithContext", mock.Anything, mock.Anything).Return(&ec2.DescribeRegionsOutput{
Regions: []*ec2.Region{
{
RegionName: utils.Pointer(regionName),
@ -109,7 +109,7 @@ func Test_handleGetRegions_regionCache(t *testing.T) {
})
t.Run("AWS only called once for multiple calls to handleGetRegions", func(t *testing.T) {
cli.On("DescribeRegions", mock.Anything, mock.Anything).Return(&ec2.DescribeRegionsOutput{}, nil)
cli.On("DescribeRegionsWithContext", mock.Anything, mock.Anything).Return(&ec2.DescribeRegionsOutput{}, nil)
executor := newExecutor(im, newTestConfig(), &fakeSessionCache{}, featuremgmt.WithFeatures())
_, err := executor.handleGetRegions(
context.Background(),
@ -121,7 +121,7 @@ func Test_handleGetRegions_regionCache(t *testing.T) {
backend.PluginContext{DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{}}, nil)
require.NoError(t, err)
cli.AssertNumberOfCalls(t, "DescribeRegions", 1)
cli.AssertNumberOfCalls(t, "DescribeRegionsWithContext", 1)
})
}
func TestQuery_InstanceAttributes(t *testing.T) {

@ -1,6 +1,8 @@
package mocks
import (
"context"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/request"
"github.com/aws/aws-sdk-go/service/ec2"
@ -12,7 +14,7 @@ type RegionsService struct {
mock.Mock
}
func (r *RegionsService) GetRegions() ([]resources.ResourceResponse[resources.Region], error) {
func (r *RegionsService) GetRegions(ctx context.Context) (in []resources.ResourceResponse[resources.Region], e error) {
args := r.Called()
return args.Get(0).(([]resources.ResourceResponse[resources.Region])), args.Error(1)
}
@ -21,7 +23,7 @@ type EC2Mock struct {
mock.Mock
}
func (e *EC2Mock) DescribeRegions(in *ec2.DescribeRegionsInput) (*ec2.DescribeRegionsOutput, error) {
func (e *EC2Mock) DescribeRegionsWithContext(ctx aws.Context, in *ec2.DescribeRegionsInput, opts ...request.Option) (*ec2.DescribeRegionsOutput, error) {
args := e.Called()
return args.Get(0).(*ec2.DescribeRegionsOutput), args.Error(1)
}

@ -46,7 +46,7 @@ type AccountsProvider interface {
}
type RegionsAPIProvider interface {
GetRegions() ([]resources.ResourceResponse[resources.Region], error)
GetRegions(ctx context.Context) ([]resources.ResourceResponse[resources.Region], error)
}
// Clients
@ -70,6 +70,6 @@ type OAMAPIProvider interface {
}
type EC2APIProvider interface {
DescribeRegions(in *ec2.DescribeRegionsInput) (*ec2.DescribeRegionsOutput, error)
DescribeRegionsWithContext(ctx context.Context, in *ec2.DescribeRegionsInput, opts ...request.Option) (*ec2.DescribeRegionsOutput, error)
DescribeInstancesPagesWithContext(ctx context.Context, in *ec2.DescribeInstancesInput, fn func(*ec2.DescribeInstancesOutput, bool) bool, opts ...request.Option) error
}

@ -25,7 +25,7 @@ func RegionsHandler(ctx context.Context, pluginCtx backend.PluginContext, reqCtx
return nil, models.NewHttpError("Error in Regions Handler when connecting to aws", http.StatusInternalServerError, err)
}
regions, err := service.GetRegions()
regions, err := service.GetRegions(ctx)
if err != nil {
return nil, models.NewHttpError("Error in Regions Handler while fetching regions", http.StatusInternalServerError, err)
}

@ -1,11 +1,11 @@
package services
import (
"context"
"sort"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/grafana/grafana/pkg/infra/log"
"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"
@ -31,12 +31,12 @@ func mergeEC2RegionsAndConstantRegions(regions map[string]struct{}, ec2Regions [
}
}
func (r *RegionsService) GetRegions() ([]resources.ResourceResponse[resources.Region], error) {
func (r *RegionsService) GetRegions(ctx context.Context) ([]resources.ResourceResponse[resources.Region], error) {
regions := constants.Regions()
result := make([]resources.ResourceResponse[resources.Region], 0)
ec2Regions, err := r.DescribeRegions(&ec2.DescribeRegionsInput{})
ec2Regions, err := r.DescribeRegionsWithContext(ctx, &ec2.DescribeRegionsInput{})
// we ignore this error and always send default regions
// we only fetch incase a user has enabled additional regions
// but we still log it in case the user is expecting to fetch regions specific to their account and are unable to

@ -1,6 +1,7 @@
package services
import (
"context"
"testing"
"github.com/aws/aws-sdk-go/service/ec2"
@ -23,8 +24,8 @@ func TestRegions(t *testing.T) {
},
}
ec2Mock := &mocks.EC2Mock{}
ec2Mock.On("DescribeRegions").Return(mockRegions, nil)
regions, err := NewRegionsService(ec2Mock, testLogger).GetRegions()
ec2Mock.On("DescribeRegionsWithContext").Return(mockRegions, nil)
regions, err := NewRegionsService(ec2Mock, testLogger).GetRegions(context.Background())
assert.NoError(t, err)
assert.Contains(t, regions, resources.ResourceResponse[resources.Region]{
Value: resources.Region{
@ -43,8 +44,8 @@ func TestRegions(t *testing.T) {
mockRegions := &ec2.DescribeRegionsOutput{
Regions: []*ec2.Region{},
}
ec2Mock.On("DescribeRegions").Return(mockRegions, assert.AnError)
regions, err := NewRegionsService(ec2Mock, testLogger).GetRegions()
ec2Mock.On("DescribeRegionsWithContext").Return(mockRegions, assert.AnError)
regions, err := NewRegionsService(ec2Mock, testLogger).GetRegions(context.Background())
assert.NoError(t, err)
assert.Contains(t, regions, resources.ResourceResponse[resources.Region]{
Value: resources.Region{

@ -125,7 +125,7 @@ type mockEC2Client struct {
mock.Mock
}
func (c *mockEC2Client) DescribeRegions(in *ec2.DescribeRegionsInput) (*ec2.DescribeRegionsOutput, error) {
func (c *mockEC2Client) DescribeRegionsWithContext(ctx aws.Context, in *ec2.DescribeRegionsInput, option ...request.Option) (*ec2.DescribeRegionsOutput, error) {
args := c.Called(in)
return args.Get(0).(*ec2.DescribeRegionsOutput), args.Error(1)
}
@ -143,7 +143,7 @@ type oldEC2Client struct {
reservations []*ec2.Reservation
}
func (c oldEC2Client) DescribeRegions(*ec2.DescribeRegionsInput) (*ec2.DescribeRegionsOutput, error) {
func (c oldEC2Client) DescribeRegionsWithContext(ctx aws.Context, in *ec2.DescribeRegionsInput, option ...request.Option) (*ec2.DescribeRegionsOutput, error) {
regions := []*ec2.Region{}
for _, region := range c.regions {
regions = append(regions, &ec2.Region{

Loading…
Cancel
Save