refactor shared config Loading (#990)

* updates shared config loaders to be consistent with CLI and AWS SDKs

* adds more tests and feedback

* fix windows test failure

* set config or creds file defaults only if unset

* rebase branch with master

aws/credentials.go

@@ -110,7 +110,7 @@ func (v Credentials) HasKeys() bool {
 //
 // A credentials provider implementation can be wrapped with a CredentialCache
 // to cache the credential value retrieved. Without the cache the SDK will
-// attempt to retrieve the credentials for ever request.
+// attempt to retrieve the credentials for every request.
 type CredentialsProvider interface {
 	// Retrieve returns nil if it successfully retrieved the value.
 	// Error is returned if the value were not obtainable, or empty.

config/env_config.go

@@ -250,16 +250,12 @@ func (c EnvConfig) getSharedConfigProfile(ctx context.Context) (string, bool, er
 	return c.SharedConfigProfile, true, nil
 }
 
-// GetSharedConfigFiles returns a slice of filenames set in the environment.
+// getSharedConfigFiles returns a slice of filenames set in the environment.
 //
 // Will return the filenames in the order of:
-// * Shared Credentials
 // * Shared Config
 func (c EnvConfig) getSharedConfigFiles(context.Context) ([]string, bool, error) {
-	files := make([]string, 0, 2)
-	if v := c.SharedCredentialsFile; len(v) > 0 {
-		files = append(files, v)
-	}
+	var files []string
 	if v := c.SharedConfigFile; len(v) > 0 {
 		files = append(files, v)
 	}
@@ -270,6 +266,21 @@ func (c EnvConfig) getSharedConfigFiles(context.Context) ([]string, bool, error)
 	return files, true, nil
 }
 
+// getSharedCredentialsFiles returns a slice of filenames set in the environment.
+//
+// Will return the filenames in the order of:
+// * Shared Credentials
+func (c EnvConfig) getSharedCredentialsFiles(context.Context) ([]string, bool, error) {
+	var files []string
+	if v := c.SharedCredentialsFile; len(v) > 0 {
+		files = append(files, v)
+	}
+	if len(files) == 0 {
+		return nil, false, nil
+	}
+	return files, true, nil
+}
+
 // GetCustomCABundle returns the custom CA bundle's PEM bytes if the file was
 func (c EnvConfig) getCustomCABundle(context.Context) (io.Reader, bool, error) {
 	if len(c.CustomCABundle) == 0 {

config/load_options.go

@@ -56,9 +56,30 @@ type LoadOptions struct {
 	// SharedConfigProfile is the profile to be used when loading the SharedConfig
 	SharedConfigProfile string
 
-	// SharedConfigFiles is the slice of custom shared config files to use when loading the SharedConfig
+	// SharedConfigFiles is the slice of custom shared config files to use when loading the SharedConfig.
+	// A non-default profile used within config file must have name defined with prefix 'profile '.
+	// eg [profile xyz] indicates a profile with name 'xyz'.
+	// To read more on the format of the config file, please refer the documentation at
+	// https://docs.aws.amazon.com/credref/latest/refdocs/file-format.html#file-format-config
+	//
+	// If duplicate profiles are provided within the same, or across multiple shared config files, the next parsed
+	// profile will override only the properties that conflict with the previously defined profile.
+	// Note that if duplicate profiles are provided within the SharedCredentialsFiles and SharedConfigFiles,
+	// the properties defined in shared credentials file take precedence.
 	SharedConfigFiles []string
 
+	// SharedCredentialsFile is the slice of custom shared credentials files to use when loading the SharedConfig.
+	// The profile name used within credentials file must not prefix 'profile '.
+	// eg [xyz] indicates a profile with name 'xyz'. Profile declared as [profile xyz] will be ignored.
+	// To read more on the format of the credentials file, please refer the documentation at
+	// https://docs.aws.amazon.com/credref/latest/refdocs/file-format.html#file-format-creds
+	//
+	// If duplicate profiles are provided with a same, or across multiple shared credentials files, the next parsed
+	// profile will override only properties that conflict with the previously defined profile.
+	// Note that if duplicate profiles are provided within the SharedCredentialsFiles and SharedConfigFiles,
+	// the properties defined in shared credentials file take precedence.
+	SharedCredentialsFiles []string
+
 	// CustomCABundle is CA bundle PEM bytes reader
 	CustomCABundle io.Reader
 
@@ -186,6 +207,28 @@ func WithSharedConfigFiles(v []string) LoadOptionsFunc {
 	}
 }
 
+// getSharedCredentialsFiles returns SharedCredentialsFiles set on config's LoadOptions
+func (o LoadOptions) getSharedCredentialsFiles(ctx context.Context) ([]string, bool, error) {
+	if o.SharedCredentialsFiles == nil {
+		return nil, false, nil
+	}
+
+	return o.SharedCredentialsFiles, true, nil
+}
+
+// WithSharedCredentialsFiles is a helper function to construct functional options
+// that sets slice of SharedCredentialsFiles on config's LoadOptions.
+// Setting the shared credentials files to an nil string slice, will result in the
+// shared credentials files value being ignored.
+// If multiple WithSharedCredentialsFiles calls are made, the last call overrides
+// the previous call values.
+func WithSharedCredentialsFiles(v []string) LoadOptionsFunc {
+	return func(o *LoadOptions) error {
+		o.SharedCredentialsFiles = v
+		return nil
+	}
+}
+
 // getCustomCABundle returns CustomCABundle from LoadOptions
 func (o LoadOptions) getCustomCABundle(ctx context.Context) (io.Reader, bool, error) {
 	if o.CustomCABundle == nil {

config/provider.go

@@ -57,6 +57,28 @@ func getSharedConfigFiles(ctx context.Context, configs configs) (value []string,
 	return
 }
 
+// sharedCredentialsFilesProvider provides access to the shared credentials filesnames
+// external configuration value.
+type sharedCredentialsFilesProvider interface {
+	getSharedCredentialsFiles(ctx context.Context) ([]string, bool, error)
+}
+
+// getSharedCredentialsFiles searches the configs for a sharedCredentialsFilesProvider
+// and returns the value if found. Returns an error if a provider fails before a
+// value is found.
+func getSharedCredentialsFiles(ctx context.Context, configs configs) (value []string, found bool, err error) {
+	for _, cfg := range configs {
+		if p, ok := cfg.(sharedCredentialsFilesProvider); ok {
+			value, found, err = p.getSharedCredentialsFiles(ctx)
+			if err != nil || found {
+				break
+			}
+		}
+	}
+
+	return
+}
+
 // customCABundleProvider provides access to the custom CA bundle PEM bytes.
 type customCABundleProvider interface {
 	getCustomCABundle(ctx context.Context) (io.Reader, bool, error)

config/resolve_assume_role_test.go

@@ -20,7 +20,7 @@ func TestAssumeRole(t *testing.T) {
 	defer awstesting.PopEnv(restoreEnv)
 
 	os.Setenv("AWS_REGION", "us-east-1")
-	os.Setenv("AWS_SHARED_CREDENTIALS_FILE", testConfigFilename)
+	os.Setenv("AWS_CONFIG_FILE", testConfigFilename)
 	os.Setenv("AWS_PROFILE", "assume_role_w_creds")
 
 	client := mockHTTPClient(func(r *http.Request) (*http.Response, error) {
@@ -59,7 +59,7 @@ func TestAssumeRole_WithMFA(t *testing.T) {
 	defer awstesting.PopEnv(restoreEnv)
 
 	os.Setenv("AWS_REGION", "us-east-1")
-	os.Setenv("AWS_SHARED_CREDENTIALS_FILE", testConfigFilename)
+	os.Setenv("AWS_CONFIG_FILE", testConfigFilename)
 	os.Setenv("AWS_PROFILE", "assume_role_w_creds")
 
 	client := mockHTTPClient(func(r *http.Request) (*http.Response, error) {
@@ -125,7 +125,7 @@ func TestAssumeRole_WithMFA_NoTokenProvider(t *testing.T) {
 	defer awstesting.PopEnv(restoreEnv)
 
 	os.Setenv("AWS_REGION", "us-east-1")
-	os.Setenv("AWS_SHARED_CREDENTIALS_FILE", testConfigFilename)
+	os.Setenv("AWS_CONFIG_FILE", testConfigFilename)
 	os.Setenv("AWS_PROFILE", "assume_role_w_creds")
 
 	_, err := LoadDefaultConfig(context.Background(), WithSharedConfigProfile("assume_role_w_mfa"))
@@ -140,7 +140,7 @@ func TestAssumeRole_InvalidSourceProfile(t *testing.T) {
 	restoreEnv := initConfigTestEnv()
 	defer awstesting.PopEnv(restoreEnv)
 
-	os.Setenv("AWS_SHARED_CREDENTIALS_FILE", testConfigFilename)
+	os.Setenv("AWS_CONFIG_FILE", testConfigFilename)
 	os.Setenv("AWS_PROFILE", "assume_role_invalid_source_profile")
 
 	_, err := LoadDefaultConfig(context.Background())
@@ -159,7 +159,7 @@ func TestAssumeRole_ExtendedDuration(t *testing.T) {
 	defer awstesting.PopEnv(restoreEnv)
 
 	os.Setenv("AWS_REGION", "us-east-1")
-	os.Setenv("AWS_SHARED_CREDENTIALS_FILE", testConfigFilename)
+	os.Setenv("AWS_CONFIG_FILE", testConfigFilename)
 	os.Setenv("AWS_PROFILE", "assume_role_w_creds_ext_dur")
 
 	client := mockHTTPClient(func(r *http.Request) (*http.Response, error) {

config/resolve_credentials_test.go

@@ -88,8 +88,11 @@ func setupCredentialsEndpoints(t *testing.T) (aws.EndpointResolver, func()) {
 }
 
 func TestSharedConfigCredentialSource(t *testing.T) {
-	var configFileForWindows = filepath.Join("testdata", "credential_source_config_for_windows")
-	var configFile = filepath.Join("testdata", "credential_source_config")
+	var configFileForWindows = filepath.Join("testdata", "config_source_shared_for_windows")
+	var configFile = filepath.Join("testdata", "config_source_shared")
+
+	var credFileForWindows = filepath.Join("testdata", "credentials_source_shared_for_windows")
+	var credFile = filepath.Join("testdata", "credentials_source_shared")
 
 	cases := map[string]struct {
 		name              string
@@ -104,7 +107,7 @@ func TestSharedConfigCredentialSource(t *testing.T) {
 	}{
 		"credential source and source profile": {
 			envProfile:    "invalid_source_and_credential_source",
-			expectedError: "nly source profile or credential source can be specified",
+			expectedError: "only source profile or credential source can be specified",
 			init: func() {
 				os.Setenv("AWS_ACCESS_KEY", "access_key")
 				os.Setenv("AWS_SECRET_KEY", "secret_key")
@@ -174,6 +177,28 @@ func TestSharedConfigCredentialSource(t *testing.T) {
 				"assume_role_w_creds_proc_source_prof",
 			},
 		},
+		"credential source overrides config source": {
+			envProfile:        "credentials_overide",
+			expectedAccessKey: "AKID",
+			expectedSecretKey: "SECRET",
+			expectedChain: []string{
+				"assume_role_w_creds_role_arn_ec2",
+			},
+			init: func() {
+				os.Setenv("AWS_CONTAINER_CREDENTIALS_RELATIVE_URI", "/ECS")
+			},
+		},
+		"only credential source": {
+			envProfile:        "only_credentials_source",
+			expectedAccessKey: "AKID",
+			expectedSecretKey: "SECRET",
+			expectedChain: []string{
+				"assume_role_w_creds_role_arn_ecs",
+			},
+			init: func() {
+				os.Setenv("AWS_CONTAINER_CREDENTIALS_RELATIVE_URI", "/ECS")
+			},
+		},
 	}
 
 	for name, c := range cases {
@@ -183,8 +208,10 @@ func TestSharedConfigCredentialSource(t *testing.T) {
 
 			if c.dependentOnOS && runtime.GOOS == "windows" {
 				os.Setenv("AWS_CONFIG_FILE", configFileForWindows)
+				os.Setenv("AWS_SHARED_CREDENTIALS_FILE", credFileForWindows)
 			} else {
 				os.Setenv("AWS_CONFIG_FILE", configFile)
+				os.Setenv("AWS_SHARED_CREDENTIALS_FILE", credFile)
 			}
 
 			os.Setenv("AWS_REGION", "us-east-1")

config/resolve_processcreds_test.go

@@ -2,12 +2,11 @@ package config
 
 import (
 	"context"
+	"github.com/aws/aws-sdk-go-v2/internal/awstesting"
 	"os"
 	"path/filepath"
 	"runtime"
 	"testing"
-
-	"github.com/aws/aws-sdk-go-v2/internal/awstesting"
 )
 
 func setupEnvForProcesscredsConfigFile() {
@@ -17,6 +16,7 @@ func setupEnvForProcesscredsConfigFile() {
 	}
 
 	os.Setenv("AWS_CONFIG_FILE", filepath.Join("testdata", filename))
+	os.Setenv("AWS_SHARED_CREDENTIALS_FILE", filepath.Join("testdata", "empty_creds_config"))
 }
 
 func setupEnvForProcesscredsCredentialsFile() {
@@ -33,7 +33,6 @@ func TestProcessCredentialsProvider_FromConfig(t *testing.T) {
 	defer awstesting.PopEnv(restoreEnv)
 
 	setupEnvForProcesscredsConfigFile()
-	os.Setenv("AWS_SDK_LOAD_CONFIG", "1")
 
 	config, err := LoadDefaultConfig(context.Background(), WithRegion("region"))
 	if err != nil {
@@ -63,7 +62,6 @@ func TestProcessCredentialsProvider_FromConfigWithProfile(t *testing.T) {
 	restoreEnv := awstesting.StashEnv()
 	defer awstesting.PopEnv(restoreEnv)
 
-	os.Setenv("AWS_SDK_LOAD_CONFIG", "1")
 	os.Setenv("AWS_PROFILE", "not_expire")
 	setupEnvForProcesscredsConfigFile()
 
@@ -87,7 +85,6 @@ func TestProcessCredentialsProvider_FromConfigWithStaticCreds(t *testing.T) {
 	restoreEnv := awstesting.StashEnv()
 	defer awstesting.PopEnv(restoreEnv)
 
-	os.Setenv("AWS_SDK_LOAD_CONFIG", "1")
 	os.Setenv("AWS_PROFILE", "not_alone")
 	setupEnvForProcesscredsConfigFile()