From 286e2203594990b810fb54c83ce056a5873a5d14 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Wed, 3 Jul 2024 18:33:53 -0700 Subject: [PATCH] Add more linters --- .golangci.yml | 26 +++++++++++++++++++++++--- Makefile | 2 +- cmd/sync/aws.go | 32 ++++++++++++++++---------------- cmd/sync/aws_test.go | 4 ++++ cmd/sync/azure.go | 22 +++++++++++----------- cmd/sync/azure_test.go | 5 +++++ cmd/sync/config.go | 14 +++++++------- cmd/sync/config_test.go | 13 ++++++++----- cmd/sync/main.go | 6 +++--- cmd/sync/parameters_test.go | 2 ++ cmd/sync/provider_test.go | 2 ++ cmd/sync/validation_test.go | 1 + go.mod | 2 +- 13 files changed, 84 insertions(+), 47 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 8474d339..5047ecba 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -14,7 +14,6 @@ linters-settings: - name: error-strings - name: errorf - name: exported - - name: if-return - name: increment-decrement - name: indent-error-flow - name: package-comments @@ -29,33 +28,53 @@ linters-settings: - name: var-declaration - name: var-naming govet: - check-shadowing: true enable-all: true - linters: enable: + - asasalint - asciicheck - bidichk + - contextcheck - dupword + - durationcheck - errcheck + - errchkjson + - errname - errorlint + - exportloopref + - fatcontext + - forcetypeassert + - gocheckcompilerdirectives + - gochecksumtype + - gocritic + - godot - gofmt - gofumpt - goimports - gosec - gosimple + - gosmopolitan - govet - ineffassign + - intrange - makezero - misspell + - musttag - nilerr - noctx + - nolintlint + - paralleltest - perfsprint + - prealloc - predeclared - reassign - revive - staticcheck + - stylecheck - tagalign + - tenv + - testpackage + - thelper - tparallel - typecheck - unconvert @@ -64,6 +83,7 @@ linters: - usestdlibvars - wastedassign - whitespace + - wrapcheck disable-all: true issues: max-issues-per-linter: 0 diff --git a/Makefile b/Makefile index 2dd48b02..d61bc3d8 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,7 @@ .PHONY: test test: - go test ./... + go test -v -shuffle=on -race ./... .PHONY: lint lint: diff --git a/cmd/sync/aws.go b/cmd/sync/aws.go index 9f62107e..42e9ad4a 100644 --- a/cmd/sync/aws.go +++ b/cmd/sync/aws.go @@ -17,14 +17,14 @@ import ( yaml "gopkg.in/yaml.v2" ) -// AWSClient allows you to get the list of IP addresses of instances of an Auto Scaling group. It implements the CloudProvider interface +// AWSClient allows you to get the list of IP addresses of instances of an Auto Scaling group. It implements the CloudProvider interface. type AWSClient struct { svcEC2 *ec2.Client svcAutoscaling *autoscaling.Client config *awsConfig } -// NewAWSClient creates and configures an AWSClient +// NewAWSClient creates and configures an AWSClient. func NewAWSClient(data []byte) (*AWSClient, error) { awsClient := &AWSClient{} cfg, err := parseAWSConfig(data) @@ -37,7 +37,7 @@ func NewAWSClient(data []byte) (*AWSClient, error) { conf, loadErr := config.LoadDefaultConfig(context.TODO()) if loadErr != nil { - return nil, loadErr + return nil, fmt.Errorf("unable to load default AWS config: %w", loadErr) } client := imds.NewFromConfig(conf, func(o *imds.Options) { @@ -61,10 +61,10 @@ func NewAWSClient(data []byte) (*AWSClient, error) { return awsClient, nil } -// GetUpstreams returns the Upstreams list +// GetUpstreams returns the Upstreams list. func (client *AWSClient) GetUpstreams() []Upstream { - var upstreams []Upstream - for i := 0; i < len(client.config.Upstreams); i++ { + upstreams := make([]Upstream, 0, len(client.config.Upstreams)) + for i := range len(client.config.Upstreams) { u := Upstream{ Name: client.config.Upstreams[i].Name, Port: client.config.Upstreams[i].Port, @@ -81,13 +81,13 @@ func (client *AWSClient) GetUpstreams() []Upstream { return upstreams } -// configure configures the AWSClient with necessary parameters +// configure configures the AWSClient with necessary parameters. func (client *AWSClient) configure() error { httpClient := &http.Client{Timeout: connTimeoutInSecs * time.Second} cfg, err := config.LoadDefaultConfig(context.TODO()) if err != nil { - return err + return fmt.Errorf("unable to load default AWS config: %w", err) } client.svcEC2 = ec2.NewFromConfig(cfg, func(o *ec2.Options) { @@ -103,12 +103,12 @@ func (client *AWSClient) configure() error { return nil } -// parseAWSConfig parses and validates AWSClient config +// parseAWSConfig parses and validates AWSClient config. func parseAWSConfig(data []byte) (*awsConfig, error) { cfg := &awsConfig{} err := yaml.Unmarshal(data, cfg) if err != nil { - return nil, err + return nil, fmt.Errorf("error unmarshalling AWS config: %w", err) } err = validateAWSConfig(cfg) @@ -119,7 +119,7 @@ func parseAWSConfig(data []byte) (*awsConfig, error) { return cfg, nil } -// CheckIfScalingGroupExists checks if the Auto Scaling group exists +// CheckIfScalingGroupExists checks if the Auto Scaling group exists. func (client *AWSClient) CheckIfScalingGroupExists(name string) (bool, error) { params := &ec2.DescribeInstancesInput{ Filters: []types.Filter{ @@ -140,7 +140,7 @@ func (client *AWSClient) CheckIfScalingGroupExists(name string) (bool, error) { return len(response.Reservations) > 0, nil } -// GetPrivateIPsForScalingGroup returns the list of IP addresses of instances of the Auto Scaling group +// GetPrivateIPsForScalingGroup returns the list of IP addresses of instances of the Auto Scaling group. func (client *AWSClient) GetPrivateIPsForScalingGroup(name string) ([]string, error) { var onlyInService bool for _, u := range client.GetUpstreams() { @@ -162,7 +162,7 @@ func (client *AWSClient) GetPrivateIPsForScalingGroup(name string) ([]string, er response, err := client.svcEC2.DescribeInstances(context.Background(), params) if err != nil { - return nil, err + return nil, fmt.Errorf("couldn't describe instances: %w", err) } if len(response.Reservations) == 0 { @@ -193,14 +193,14 @@ func (client *AWSClient) GetPrivateIPsForScalingGroup(name string) ([]string, er return result, nil } -// getInstancesInService returns the list of instances that have LifecycleState == InService +// getInstancesInService returns the list of instances that have LifecycleState == InService. func (client *AWSClient) getInstancesInService(insIDtoIP map[string]string) ([]string, error) { const maxItems = 50 var result []string keys := reflect.ValueOf(insIDtoIP).MapKeys() instanceIDs := make([]string, len(keys)) - for i := 0; i < len(keys); i++ { + for i := range len(keys) { instanceIDs[i] = keys[i].String() } @@ -211,7 +211,7 @@ func (client *AWSClient) getInstancesInService(insIDtoIP map[string]string) ([]s } response, err := client.svcAutoscaling.DescribeAutoScalingInstances(context.Background(), params) if err != nil { - return nil, err + return nil, fmt.Errorf("couldn't describe AutoScaling instances: %w", err) } for _, ins := range response.AutoScalingInstances { diff --git a/cmd/sync/aws_test.go b/cmd/sync/aws_test.go index 1d0bdf3f..969c7055 100644 --- a/cmd/sync/aws_test.go +++ b/cmd/sync/aws_test.go @@ -74,6 +74,7 @@ func getInvalidAWSConfigInput() []*testInputAWS { } func TestValidateAWSConfigNotValid(t *testing.T) { + t.Parallel() input := getInvalidAWSConfigInput() for _, item := range input { @@ -85,6 +86,7 @@ func TestValidateAWSConfigNotValid(t *testing.T) { } func TestValidateAWSConfigValid(t *testing.T) { + t.Parallel() cfg := getValidAWSConfig() err := validateAWSConfig(cfg) @@ -94,6 +96,7 @@ func TestValidateAWSConfigValid(t *testing.T) { } func TestGetUpstreamsAWS(t *testing.T) { + t.Parallel() cfg := getValidAWSConfig() upstreams := []awsUpstream{ { @@ -165,6 +168,7 @@ func areEqualUpstreamsAWS(u1 awsUpstream, u2 Upstream) bool { } func TestPrepareBatches(t *testing.T) { + t.Parallel() const maxItems = 3 ids := []string{"i-394ujfs", "i-dfdinf", "i-fsfsf", "i-8hr83hfwif", "i-nsnsnan"} instanceIDs := make([]string, len(ids)) diff --git a/cmd/sync/azure.go b/cmd/sync/azure.go index b656b582..a65b9c8e 100644 --- a/cmd/sync/azure.go +++ b/cmd/sync/azure.go @@ -11,14 +11,14 @@ import ( yaml "gopkg.in/yaml.v2" ) -// AzureClient allows you to get the list of IP addresses of VirtualMachines of a VirtualMachine Scale Set. It implements the CloudProvider interface +// AzureClient allows you to get the list of IP addresses of VirtualMachines of a VirtualMachine Scale Set. It implements the CloudProvider interface. type AzureClient struct { config *azureConfig vMSSClient compute.VirtualMachineScaleSetsClient iFaceClient network.InterfacesClient } -// NewAzureClient creates an AzureClient +// NewAzureClient creates an AzureClient. func NewAzureClient(data []byte) (*AzureClient, error) { azureClient := &AzureClient{} cfg, err := parseAzureConfig(data) @@ -36,12 +36,12 @@ func NewAzureClient(data []byte) (*AzureClient, error) { return azureClient, nil } -// parseAzureConfig parses and validates AzureClient config +// parseAzureConfig parses and validates AzureClient config. func parseAzureConfig(data []byte) (*azureConfig, error) { cfg := &azureConfig{} err := yaml.Unmarshal(data, cfg) if err != nil { - return nil, err + return nil, fmt.Errorf("couldn't unmarshal Azure config: %w", err) } err = validateAzureConfig(cfg) @@ -52,7 +52,7 @@ func parseAzureConfig(data []byte) (*azureConfig, error) { return cfg, nil } -// GetPrivateIPsForScalingGroup returns the list of IP addresses of instances of the Virtual Machine Scale Set +// GetPrivateIPsForScalingGroup returns the list of IP addresses of instances of the Virtual Machine Scale Set. func (client *AzureClient) GetPrivateIPsForScalingGroup(name string) ([]string, error) { var ips []string @@ -60,7 +60,7 @@ func (client *AzureClient) GetPrivateIPsForScalingGroup(name string) ([]string, for iFaces, err := client.iFaceClient.ListVirtualMachineScaleSetNetworkInterfaces(ctx, client.config.ResourceGroupName, name); iFaces.NotDone() || err != nil; err = iFaces.NextWithContext(ctx) { if err != nil { - return nil, err + return nil, fmt.Errorf("couldn't get the list of network interfaces: %w", err) } for _, iFace := range iFaces.Values() { @@ -102,7 +102,7 @@ func getPrimaryIPFromInterfaceIPConfiguration(ipConfig network.InterfaceIPConfig return *ipConfig.InterfaceIPConfigurationPropertiesFormat.PrivateIPAddress } -// CheckIfScalingGroupExists checks if the Virtual Machine Scale Set exists +// CheckIfScalingGroupExists checks if the Virtual Machine Scale Set exists. func (client *AzureClient) CheckIfScalingGroupExists(name string) (bool, error) { ctx := context.TODO() vmss, err := client.vMSSClient.Get(ctx, client.config.ResourceGroupName, name, "userData") @@ -116,7 +116,7 @@ func (client *AzureClient) CheckIfScalingGroupExists(name string) (bool, error) func (client *AzureClient) configure() error { authorizer, err := auth.NewAuthorizerFromEnvironment() if err != nil { - return err + return fmt.Errorf("couldn't create authorizer: %w", err) } client.vMSSClient = compute.NewVirtualMachineScaleSetsClient(client.config.SubscriptionID) @@ -127,10 +127,10 @@ func (client *AzureClient) configure() error { return nil } -// GetUpstreams returns the Upstreams list +// GetUpstreams returns the Upstreams list. func (client *AzureClient) GetUpstreams() []Upstream { - var upstreams []Upstream - for i := 0; i < len(client.config.Upstreams); i++ { + upstreams := make([]Upstream, 0, len(client.config.Upstreams)) + for i := range len(client.config.Upstreams) { u := Upstream{ Name: client.config.Upstreams[i].Name, Port: client.config.Upstreams[i].Port, diff --git a/cmd/sync/azure_test.go b/cmd/sync/azure_test.go index 0ae3b891..95f7a89e 100644 --- a/cmd/sync/azure_test.go +++ b/cmd/sync/azure_test.go @@ -80,6 +80,7 @@ func getInvalidAzureConfigInput() []*testInputAzure { } func TestValidateAzureConfigNotValid(t *testing.T) { + t.Parallel() input := getInvalidAzureConfigInput() for _, item := range input { @@ -91,6 +92,7 @@ func TestValidateAzureConfigNotValid(t *testing.T) { } func TestValidateAzureConfigValid(t *testing.T) { + t.Parallel() cfg := getValidAzureConfig() err := validateAzureConfig(cfg) @@ -100,6 +102,7 @@ func TestValidateAzureConfigValid(t *testing.T) { } func TestGetPrimaryIPFromInterfaceIPConfiguration(t *testing.T) { + t.Parallel() primary := true address := "127.0.0.1" ipConfig := network.InterfaceIPConfiguration{ @@ -115,6 +118,7 @@ func TestGetPrimaryIPFromInterfaceIPConfiguration(t *testing.T) { } func TestGetPrimaryIPFromInterfaceIPConfigurationFail(t *testing.T) { + t.Parallel() primaryFalse := false primaryTrue := true tests := []struct { @@ -158,6 +162,7 @@ func TestGetPrimaryIPFromInterfaceIPConfigurationFail(t *testing.T) { } func TestGetUpstreamsAzure(t *testing.T) { + t.Parallel() cfg := getValidAzureConfig() upstreams := []azureUpstream{ { diff --git a/cmd/sync/config.go b/cmd/sync/config.go index 7a888f3d..d6935e31 100644 --- a/cmd/sync/config.go +++ b/cmd/sync/config.go @@ -8,18 +8,18 @@ import ( yaml "gopkg.in/yaml.v2" ) -// commonConfig stores the configuration parameters common to all providers +// commonConfig stores the configuration parameters common to all providers. type commonConfig struct { - APIEndpoint string `yaml:"api_endpoint"` - CloudProvider string `yaml:"cloud_provider"` - SyncIntervalInSeconds time.Duration `yaml:"sync_interval_in_seconds"` + APIEndpoint string `yaml:"api_endpoint"` + CloudProvider string `yaml:"cloud_provider"` + SyncInterval time.Duration `yaml:"sync_interval_in_seconds"` } func parseCommonConfig(data []byte) (*commonConfig, error) { cfg := &commonConfig{} err := yaml.Unmarshal(data, cfg) if err != nil { - return nil, err + return nil, fmt.Errorf("couldn't unmarshal common config: %w", err) } err = validateCommonConfig(cfg) @@ -35,7 +35,7 @@ func validateCommonConfig(cfg *commonConfig) error { return fmt.Errorf(errorMsgFormat, "api_endpoint") } - if cfg.SyncIntervalInSeconds == 0 { + if cfg.SyncInterval == 0 { return errors.New(intervalErrorMsg) } @@ -50,7 +50,7 @@ func validateCommonConfig(cfg *commonConfig) error { return nil } -// Upstream is the cloud agnostic representation of an Upstream (eg, common fields for every cloud provider) +// Upstream is the cloud agnostic representation of an Upstream (eg, common fields for every cloud provider). type Upstream struct { MaxConns *int MaxFails *int diff --git a/cmd/sync/config_test.go b/cmd/sync/config_test.go index f961a7ed..5d792c50 100644 --- a/cmd/sync/config_test.go +++ b/cmd/sync/config_test.go @@ -15,8 +15,8 @@ type testInputCommon struct { func getValidCommonConfig() *commonConfig { return &commonConfig{ - APIEndpoint: "http://127.0.0.1:8080/api", - SyncIntervalInSeconds: 1, + APIEndpoint: "http://127.0.0.1:8080/api", + SyncInterval: 1, } } @@ -27,14 +27,15 @@ func getInvalidCommonConfigInput() []*testInputCommon { invalidAPIEndpointCfg.APIEndpoint = "" input = append(input, &testInputCommon{invalidAPIEndpointCfg, "invalid api_endpoint"}) - invalidSyncIntervalInSecondsCfg := getValidCommonConfig() - invalidSyncIntervalInSecondsCfg.SyncIntervalInSeconds = 0 - input = append(input, &testInputCommon{invalidSyncIntervalInSecondsCfg, "invalid sync_interval_in_seconds"}) + invalidSyncIntervalCfg := getValidCommonConfig() + invalidSyncIntervalCfg.SyncInterval = 0 + input = append(input, &testInputCommon{invalidSyncIntervalCfg, "invalid sync_interval_in_seconds"}) return input } func TestValidateCommonConfigNotValid(t *testing.T) { + t.Parallel() input := getInvalidCommonConfigInput() for _, item := range input { @@ -46,6 +47,7 @@ func TestValidateCommonConfigNotValid(t *testing.T) { } func TestValidateCommonConfigValid(t *testing.T) { + t.Parallel() cfg := getValidCommonConfig() err := validateCommonConfig(cfg) @@ -55,6 +57,7 @@ func TestValidateCommonConfigValid(t *testing.T) { } func TestParseCommonConfig(t *testing.T) { + t.Parallel() _, err := parseCommonConfig(validYaml) if err != nil { t.Errorf("parseCommonConfig() failed for the valid config yaml: %v", string(validYaml)) diff --git a/cmd/sync/main.go b/cmd/sync/main.go index 12346274..ac61f3ee 100644 --- a/cmd/sync/main.go +++ b/cmd/sync/main.go @@ -163,7 +163,7 @@ func main() { } select { - case <-time.After(commonConfig.SyncIntervalInSeconds * time.Second): + case <-time.After(commonConfig.SyncInterval * time.Second): //nolint:durationcheck case <-sigterm: log.Println("Terminating...") return @@ -172,7 +172,7 @@ func main() { } func getUpstreamServerAddresses(server []nginx.UpstreamServer) []string { - var upstreamServerAddr []string + upstreamServerAddr := make([]string, 0, len(server)) for _, s := range server { upstreamServerAddr = append(upstreamServerAddr, s.Server) } @@ -180,7 +180,7 @@ func getUpstreamServerAddresses(server []nginx.UpstreamServer) []string { } func getStreamUpstreamServerAddresses(server []nginx.StreamUpstreamServer) []string { - var streamUpstreamServerAddr []string + streamUpstreamServerAddr := make([]string, 0, len(server)) for _, s := range server { streamUpstreamServerAddr = append(streamUpstreamServerAddr, s.Server) } diff --git a/cmd/sync/parameters_test.go b/cmd/sync/parameters_test.go index 6a543e99..c68466b6 100644 --- a/cmd/sync/parameters_test.go +++ b/cmd/sync/parameters_test.go @@ -5,6 +5,7 @@ import ( ) func TestGetFailTimeoutOrDefault(t *testing.T) { + t.Parallel() tests := []struct { input string expected string @@ -28,6 +29,7 @@ func TestGetFailTimeoutOrDefault(t *testing.T) { } func TestGetSlowStartOrDefault(t *testing.T) { + t.Parallel() tests := []struct { input string expected string diff --git a/cmd/sync/provider_test.go b/cmd/sync/provider_test.go index 53a44450..a09bf1b1 100644 --- a/cmd/sync/provider_test.go +++ b/cmd/sync/provider_test.go @@ -3,6 +3,7 @@ package main import "testing" func TestValidateCloudProviderValid(t *testing.T) { + t.Parallel() provider := "AWS" valid := validateCloudProvider(provider) if !valid { @@ -11,6 +12,7 @@ func TestValidateCloudProviderValid(t *testing.T) { } func TestValidateCloudProviderInvalid(t *testing.T) { + t.Parallel() provider := "invalid" valid := validateCloudProvider(provider) if valid { diff --git a/cmd/sync/validation_test.go b/cmd/sync/validation_test.go index dd9d113d..d14b24ab 100644 --- a/cmd/sync/validation_test.go +++ b/cmd/sync/validation_test.go @@ -3,6 +3,7 @@ package main import "testing" func TestIsValidTime(t *testing.T) { + t.Parallel() testsWithValidInput := []string{"1", "1m10s", "11 11", "5m 30s", "1s", "100m", "5w", "15m", "11M", "3h", "100y", "600"} invalidInput := []string{"ss", "rM", "m0m", "s1s", "-5s", "1L"} diff --git a/go.mod b/go.mod index d6c4131f..edb46346 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/nginxinc/nginx-asg-sync -go 1.21.3 +go 1.22.5 require ( github.com/Azure/azure-sdk-for-go v68.0.0+incompatible