Enable additional golangci-lint linters and fix reports

.golangci.yml

@@ -6,14 +6,21 @@ linters:
   disable-all: true
   enable:
     - deadcode
+    - dogsled
     - errcheck
+    - goconst
     - gofmt
+    - gomnd
     - gosimple
     - ineffassign
+    - interfacer
     - misspell
+    - scopelint
     - staticcheck
     - structcheck
     - unconvert
+    - unparam
     - unused
+    - typecheck
     - varcheck
     - vet

awsauth.go

@@ -27,6 +27,10 @@ const (
 	errMsgNoValidCredentialSources = `No valid credential sources found for AWS Provider.
 	Please see https://terraform.io/docs/providers/aws/index.html for more information on
 	providing credentials for the AWS Provider`
+
+	// Default amount of time for EC2/ECS metadata client operations.
+	// Keep this value low to prevent long delays in non-EC2/ECS environments.
+	DefaultMetadataClientTimeout = 100 * time.Millisecond
 )
 
 var (
@@ -236,9 +240,7 @@ func GetCredentials(c *Config) (*awsCredentials.Credentials, error) {
 
 	// Build isolated HTTP client to avoid issues with globally-shared settings
 	client := cleanhttp.DefaultClient()
-
-	// Keep the default timeout (100ms) low as we don't want to wait in non-EC2 environments
-	client.Timeout = 100 * time.Millisecond
+	client.Timeout = DefaultMetadataClientTimeout
 
 	const userTimeoutEnvVar = "AWS_METADATA_TIMEOUT"
 	userTimeout := os.Getenv(userTimeoutEnvVar)

awsauth_test.go

@@ -120,6 +120,8 @@ func TestGetAccountIDAndPartition(t *testing.T) {
 	}
 
 	for _, testCase := range testCases {
+		testCase := testCase
+
 		t.Run(testCase.Description, func(t *testing.T) {
 			resetEnv := unsetEnv(t)
 			defer resetEnv()
@@ -223,6 +225,8 @@ func TestGetAccountIDAndPartitionFromIAMGetUser(t *testing.T) {
 	}
 
 	for _, testCase := range testCases {
+		testCase := testCase
+
 		t.Run(testCase.Description, func(t *testing.T) {
 			closeIam, iamSess, err := GetMockedAwsApiSession("IAM", testCase.MockEndpoints)
 			defer closeIam()
@@ -281,6 +285,8 @@ func TestGetAccountIDAndPartitionFromIAMListRoles(t *testing.T) {
 	}
 
 	for _, testCase := range testCases {
+		testCase := testCase
+
 		t.Run(testCase.Description, func(t *testing.T) {
 			closeIam, iamSess, err := GetMockedAwsApiSession("IAM", testCase.MockEndpoints)
 			defer closeIam()
@@ -339,6 +345,8 @@ func TestGetAccountIDAndPartitionFromSTSGetCallerIdentity(t *testing.T) {
 	}
 
 	for _, testCase := range testCases {
+		testCase := testCase
+
 		t.Run(testCase.Description, func(t *testing.T) {
 			closeSts, stsSess, err := GetMockedAwsApiSession("STS", testCase.MockEndpoints)
 			defer closeSts()
@@ -399,6 +407,8 @@ func TestAWSParseAccountIDAndPartitionFromARN(t *testing.T) {
 	}
 
 	for _, testCase := range testCases {
+		testCase := testCase
+
 		t.Run(testCase.InputARN, func(t *testing.T) {
 			accountID, partition, err := parseAccountIDAndPartitionFromARN(testCase.InputARN)
 			if err != nil && testCase.ErrCount == 0 {
@@ -434,7 +444,7 @@ func TestAWSGetCredentials_shouldErrorWhenBlank(t *testing.T) {
 }
 
 func TestAWSGetCredentials_shouldBeStatic(t *testing.T) {
-	simple := []struct {
+	testCases := []struct {
 		Key, Secret, Token string
 	}{
 		{
@@ -447,7 +457,9 @@ func TestAWSGetCredentials_shouldBeStatic(t *testing.T) {
 		},
 	}
 
-	for _, c := range simple {
+	for _, testCase := range testCases {
+		c := testCase
+
 		cfg := Config{
 			AccessKey: c.Key,
 			SecretKey: c.Secret,
@@ -526,7 +538,7 @@ func TestAWSGetCredentials_shouldIgnoreIAM(t *testing.T) {
 	// capture the test server's close method, to call after the test returns
 	ts := awsMetadataApiMock(append(ec2metadata_securityCredentialsEndpoints, ec2metadata_instanceIdEndpoint, ec2metadata_iamInfoEndpoint))
 	defer ts()
-	simple := []struct {
+	testCases := []struct {
 		Key, Secret, Token string
 	}{
 		{
@@ -539,7 +551,9 @@ func TestAWSGetCredentials_shouldIgnoreIAM(t *testing.T) {
 		},
 	}
 
-	for _, c := range simple {
+	for _, testCase := range testCases {
+		c := testCase
+
 		cfg := Config{
 			AccessKey: c.Key,
 			SecretKey: c.Secret,
@@ -574,7 +588,7 @@ func TestAWSGetCredentials_shouldErrorWithInvalidEndpoint(t *testing.T) {
 	resetEnv := unsetEnv(t)
 	defer resetEnv()
 	// capture the test server's close method, to call after the test returns
-	ts := invalidAwsEnv(t)
+	ts := invalidAwsEnv()
 	defer ts()
 
 	_, err := GetCredentials(&Config{})
@@ -591,7 +605,7 @@ func TestAWSGetCredentials_shouldIgnoreInvalidEndpoint(t *testing.T) {
 	resetEnv := unsetEnv(t)
 	defer resetEnv()
 	// capture the test server's close method, to call after the test returns
-	ts := invalidAwsEnv(t)
+	ts := invalidAwsEnv()
 	defer ts()
 
 	creds, err := GetCredentials(&Config{AccessKey: "accessKey", SecretKey: "secretKey"})
@@ -732,7 +746,7 @@ func TestAWSGetCredentials_shouldBeENV(t *testing.T) {
 
 // invalidAwsEnv establishes a httptest server to simulate behaviour
 // when endpoint doesn't respond as expected
-func invalidAwsEnv(t *testing.T) func() {
+func invalidAwsEnv() func() {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(400)
 	}))

awserr_test.go

@@ -103,6 +103,8 @@ func TestIsAwsErr(t *testing.T) {
 	}
 
 	for _, testCase := range testCases {
+		testCase := testCase
+
 		t.Run(testCase.Name, func(t *testing.T) {
 			got := IsAWSErr(testCase.Err, testCase.Code, testCase.Message)
 
@@ -329,6 +331,8 @@ func TestIsAwsErrExtended(t *testing.T) {
 	}
 
 	for _, testCase := range testCases {
+		testCase := testCase
+
 		t.Run(testCase.Name, func(t *testing.T) {
 			got := IsAWSErrExtended(testCase.Err, testCase.Code, testCase.Message, testCase.ExtendedMessage)
 

mock.go

@@ -19,7 +19,7 @@ func MockAwsApiServer(svcName string, endpoints []*MockEndpoint) *httptest.Serve
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		buf := new(bytes.Buffer)
 		if _, err := buf.ReadFrom(r.Body); err != nil {
-			w.WriteHeader(500)
+			w.WriteHeader(http.StatusInternalServerError)
 			fmt.Fprintf(w, "Error reading from HTTP Request Body: %s", err)
 			return
 		}
@@ -43,7 +43,7 @@ func MockAwsApiServer(svcName string, endpoints []*MockEndpoint) *httptest.Serve
 			}
 		}
 
-		w.WriteHeader(400)
+		w.WriteHeader(http.StatusBadRequest)
 	}))
 
 	return ts
@@ -80,7 +80,7 @@ func awsMetadataApiMock(responses []*MetadataResponse) func() {
 				return
 			}
 		}
-		w.WriteHeader(400)
+		w.WriteHeader(http.StatusBadRequest)
 	}))
 
 	os.Setenv("AWS_METADATA_URL", ts.URL+"/latest")

session.go

@@ -15,6 +15,15 @@ import (
 	"github.com/hashicorp/go-cleanhttp"
 )
 
+const (
+	// Maximum network retries.
+	// We depend on the AWS Go SDK DefaultRetryer exponential backoff.
+	// Ensure that if the AWS Config MaxRetries is set high (which it is by
+	// default), that we only retry for a few seconds with typically
+	// unrecoverable network errors, such as DNS lookup failures.
+	MaxNetworkRetryCount = 9
+)
+
 // GetSessionOptions attempts to return valid AWS Go SDK session authentication
 // options based on pre-existing credential provider, configured profile, or
 // fallback to automatically a determined session via the AWS Go SDK.
@@ -82,9 +91,7 @@ func GetSession(c *Config) (*session.Session, error) {
 	// NOTE: This logic can be fooled by other request errors raising the retry count
 	//       before any networking error occurs
 	sess.Handlers.Retry.PushBack(func(r *request.Request) {
-		// We currently depend on the DefaultRetryer exponential backoff here.
-		// ~10 retries gives a fair backoff of a few seconds.
-		if r.RetryCount < 9 {
+		if r.RetryCount < MaxNetworkRetryCount {
 			return
 		}
 		// RequestError: send request failed

session_test.go

@@ -10,7 +10,7 @@ func TestGetSessionOptions(t *testing.T) {
 	oldEnv := initSessionTestEnv()
 	defer PopEnv(oldEnv)
 
-	tt := []struct {
+	testCases := []struct {
 		desc        string
 		config      *Config
 		expectError bool
@@ -29,7 +29,9 @@ func TestGetSessionOptions(t *testing.T) {
 		},
 	}
 
-	for _, tc := range tt {
+	for _, testCase := range testCases {
+		tc := testCase
+
 		t.Run(tc.desc, func(t *testing.T) {
 			opts, err := GetSessionOptions(tc.config)
 			if err != nil && tc.expectError == false {
@@ -91,7 +93,7 @@ func TestGetSessionWithAccountIDAndPartition(t *testing.T) {
 	})
 	defer ts.Close()
 
-	tt := []struct {
+	testCases := []struct {
 		desc              string
 		config            *Config
 		expectedAcctID    string
@@ -138,7 +140,9 @@ func TestGetSessionWithAccountIDAndPartition(t *testing.T) {
 			"", "", "No valid credential sources found for AWS Provider."},
 	}
 
-	for _, tc := range tt {
+	for _, testCase := range testCases {
+		tc := testCase
+
 		t.Run(tc.desc, func(t *testing.T) {
 			sess, acctID, part, err := GetSessionWithAccountIDAndPartition(tc.config)
 			if err != nil {

validation_test.go

@@ -50,6 +50,8 @@ func TestValidateAccountID(t *testing.T) {
 	}
 
 	for _, testCase := range testCases {
+		testCase := testCase
+
 		t.Run(testCase.Description, func(t *testing.T) {
 			err := ValidateAccountID(testCase.AccountID, testCase.AllowedAccountIDs, testCase.ForbiddenAccountIDs)
 			if err != nil && !testCase.ExpectError {
@@ -86,6 +88,8 @@ func TestValidateRegion(t *testing.T) {
 	}
 
 	for _, testCase := range testCases {
+		testCase := testCase
+
 		t.Run(testCase.Region, func(t *testing.T) {
 			err := ValidateRegion(testCase.Region)
 			if err != nil && !testCase.ExpectError {