From 415638cddadfc0cf930b62568596a0acdd4cab35 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Thu, 23 Jul 2020 08:17:20 +0200 Subject: [PATCH] Chore: Use interfaces where possible (#26392) Signed-off-by: Arve Knudsen --- pkg/components/imguploader/s3uploader.go | 5 +++-- pkg/infra/metrics/graphitebridge/graphite.go | 2 +- pkg/services/alerting/alerting_usage.go | 5 ++--- pkg/services/alerting/alerting_usage_test.go | 16 ++++++++++------ pkg/services/alerting/extractor.go | 3 ++- .../testdata/settings/invalid_format.json | 0 pkg/tsdb/cloudwatch/cloudwatch.go | 4 +++- pkg/tsdb/cloudwatch/credentials.go | 4 ++-- scripts/go/configs/.golangci.toml | 1 - 9 files changed, 23 insertions(+), 17 deletions(-) delete mode 100644 pkg/services/alerting/testdata/settings/invalid_format.json diff --git a/pkg/components/imguploader/s3uploader.go b/pkg/components/imguploader/s3uploader.go index 9103465a21a..d49e9018f91 100644 --- a/pkg/components/imguploader/s3uploader.go +++ b/pkg/components/imguploader/s3uploader.go @@ -7,6 +7,7 @@ import ( "time" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/client" "github.com/aws/aws-sdk-go/aws/credentials" "github.com/aws/aws-sdk-go/aws/credentials/ec2rolecreds" "github.com/aws/aws-sdk-go/aws/credentials/endpointcreds" @@ -99,7 +100,7 @@ func (u *S3Uploader) Upload(ctx context.Context, imageDiskPath string) (string, return result.Location, nil } -func webIdentityProvider(sess *session.Session) credentials.Provider { +func webIdentityProvider(sess client.ConfigProvider) credentials.Provider { svc := sts.New(sess) roleARN := os.Getenv("AWS_ROLE_ARN") @@ -128,6 +129,6 @@ func ecsCredProvider(sess *session.Session, uri string) credentials.Provider { func(p *endpointcreds.Provider) { p.ExpiryWindow = 5 * time.Minute }) } -func ec2RoleProvider(sess *session.Session) credentials.Provider { +func ec2RoleProvider(sess client.ConfigProvider) credentials.Provider { return &ec2rolecreds.EC2RoleProvider{Client: ec2metadata.New(sess), ExpiryWindow: 5 * time.Minute} } diff --git a/pkg/infra/metrics/graphitebridge/graphite.go b/pkg/infra/metrics/graphitebridge/graphite.go index cc09eba1bcf..1504ef3e146 100644 --- a/pkg/infra/metrics/graphitebridge/graphite.go +++ b/pkg/infra/metrics/graphitebridge/graphite.go @@ -295,7 +295,7 @@ func writeMetric(buf *bufio.Writer, m model.Metric, mf *dto.MetricFamily) error return addExtensionConventionForRollups(buf, mf, m) } -func addExtensionConventionForRollups(buf *bufio.Writer, mf *dto.MetricFamily, m model.Metric) error { +func addExtensionConventionForRollups(buf io.Writer, mf *dto.MetricFamily, m model.Metric) error { // Adding `.count` `.sum` suffix makes it possible to configure // different rollup strategies based on metric type diff --git a/pkg/services/alerting/alerting_usage.go b/pkg/services/alerting/alerting_usage.go index cdca4f982b9..102eeeb6e5e 100644 --- a/pkg/services/alerting/alerting_usage.go +++ b/pkg/services/alerting/alerting_usage.go @@ -3,7 +3,6 @@ package alerting import ( "encoding/json" - "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/models" ) @@ -75,7 +74,7 @@ func (ae *AlertEngine) mapRulesToUsageStats(rules []*models.Alert) (DatasourceAl return result, nil } -func (ae *AlertEngine) parseAlertRuleModel(settings *simplejson.Json) ([]int64, error) { +func (ae *AlertEngine) parseAlertRuleModel(settings json.Marshaler) ([]int64, error) { datasourceIDs := []int64{} model := alertJSONModel{} @@ -85,7 +84,7 @@ func (ae *AlertEngine) parseAlertRuleModel(settings *simplejson.Json) ([]int64, bytes, err := settings.MarshalJSON() if err != nil { - return datasourceIDs, err + return nil, err } err = json.Unmarshal(bytes, &model) diff --git a/pkg/services/alerting/alerting_usage_test.go b/pkg/services/alerting/alerting_usage_test.go index 5816d1aa3ec..3c554fef7ea 100644 --- a/pkg/services/alerting/alerting_usage_test.go +++ b/pkg/services/alerting/alerting_usage_test.go @@ -1,6 +1,7 @@ package alerting import ( + "encoding/json" "io/ioutil" "testing" @@ -95,8 +96,7 @@ func TestParsingAlertRuleSettings(t *testing.T) { shouldErr: require.NoError, }, { - name: "can parse blank content", - file: "testdata/settings/invalid_format.json", + name: "can handle nil content", expected: []int64{}, shouldErr: require.NoError, }, @@ -108,12 +108,16 @@ func TestParsingAlertRuleSettings(t *testing.T) { for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - content, err := ioutil.ReadFile(tc.file) - require.NoError(t, err, "expected to be able to read file") + var settings json.Marshaler + if tc.file != "" { + content, err := ioutil.ReadFile(tc.file) + require.NoError(t, err, "expected to be able to read file") - j, _ := simplejson.NewJson(content) + settings, err = simplejson.NewJson(content) + require.NoError(t, err) + } - result, err := ae.parseAlertRuleModel(j) + result, err := ae.parseAlertRuleModel(settings) tc.shouldErr(t, err) diff := cmp.Diff(tc.expected, result) diff --git a/pkg/services/alerting/extractor.go b/pkg/services/alerting/extractor.go index 497b1feaefe..d20d61a702b 100644 --- a/pkg/services/alerting/extractor.go +++ b/pkg/services/alerting/extractor.go @@ -1,6 +1,7 @@ package alerting import ( + "encoding/json" "errors" "fmt" "time" @@ -64,7 +65,7 @@ func findPanelQueryByRefID(panel *simplejson.Json, refID string) *simplejson.Jso return nil } -func copyJSON(in *simplejson.Json) (*simplejson.Json, error) { +func copyJSON(in json.Marshaler) (*simplejson.Json, error) { rawJSON, err := in.MarshalJSON() if err != nil { return nil, err diff --git a/pkg/services/alerting/testdata/settings/invalid_format.json b/pkg/services/alerting/testdata/settings/invalid_format.json deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/pkg/tsdb/cloudwatch/cloudwatch.go b/pkg/tsdb/cloudwatch/cloudwatch.go index 2ce069c0fe5..8ed16980dee 100644 --- a/pkg/tsdb/cloudwatch/cloudwatch.go +++ b/pkg/tsdb/cloudwatch/cloudwatch.go @@ -12,6 +12,7 @@ import ( "github.com/aws/aws-sdk-go/aws/request" "github.com/aws/aws-sdk-go/service/cloudwatch" "github.com/aws/aws-sdk-go/service/cloudwatchlogs" + "github.com/aws/aws-sdk-go/service/cloudwatchlogs/cloudwatchlogsiface" "github.com/aws/aws-sdk-go/service/ec2/ec2iface" "github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi/resourcegroupstaggingapiiface" "github.com/grafana/grafana/pkg/components/simplejson" @@ -115,7 +116,8 @@ func (e *cloudWatchExecutor) getCWLogsClient(region string) (*cloudwatchlogs.Clo return newLogsClient, nil } -func (e *cloudWatchExecutor) alertQuery(ctx context.Context, logsClient *cloudwatchlogs.CloudWatchLogs, queryContext *tsdb.TsdbQuery) (*cloudwatchlogs.GetQueryResultsOutput, error) { +func (e *cloudWatchExecutor) alertQuery(ctx context.Context, logsClient cloudwatchlogsiface.CloudWatchLogsAPI, + queryContext *tsdb.TsdbQuery) (*cloudwatchlogs.GetQueryResultsOutput, error) { const maxAttempts = 8 const pollPeriod = 1000 * time.Millisecond diff --git a/pkg/tsdb/cloudwatch/credentials.go b/pkg/tsdb/cloudwatch/credentials.go index af2d5dd51ca..2f6cb5a17fd 100644 --- a/pkg/tsdb/cloudwatch/credentials.go +++ b/pkg/tsdb/cloudwatch/credentials.go @@ -142,7 +142,7 @@ func getCredentials(dsInfo *datasourceInfo) (*credentials.Credentials, error) { return creds, nil } -func webIdentityProvider(sess *session.Session) credentials.Provider { +func webIdentityProvider(sess client.ConfigProvider) credentials.Provider { svc := newSTSService(sess) roleARN := os.Getenv("AWS_ROLE_ARN") @@ -171,7 +171,7 @@ func ecsCredProvider(sess *session.Session, uri string) credentials.Provider { func(p *endpointcreds.Provider) { p.ExpiryWindow = 5 * time.Minute }) } -func ec2RoleProvider(sess *session.Session) credentials.Provider { +func ec2RoleProvider(sess client.ConfigProvider) credentials.Provider { return &ec2rolecreds.EC2RoleProvider{Client: newEC2Metadata(sess), ExpiryWindow: 5 * time.Minute} } diff --git a/scripts/go/configs/.golangci.toml b/scripts/go/configs/.golangci.toml index 52ba674cbca..cec1cc9ce16 100644 --- a/scripts/go/configs/.golangci.toml +++ b/scripts/go/configs/.golangci.toml @@ -26,7 +26,6 @@ enable = [ "gosimple", "govet", "ineffassign", - # "interfacer", "misspell", "nakedret", "rowserrcheck",