From ec7e99339f69bd9fe8e11636ecb72f1812d461ea Mon Sep 17 00:00:00 2001 From: skotambkar Date: Mon, 21 Dec 2020 00:21:06 -0800 Subject: [PATCH 1/5] updates shared config loaders to be consistent with CLI and AWS SDKs --- aws/credentials.go | 2 +- config/env_config.go | 21 +- config/example_test.go | 2 + config/load_options.go | 37 +- config/provider.go | 22 + config/resolve_assume_role_test.go | 10 +- config/resolve_credentials_test.go | 37 +- config/resolve_processcreds_test.go | 4 +- config/resolve_test.go | 2 +- config/shared_config.go | 605 +++++++++++++++--- config/shared_config_test.go | 445 +++++++++---- ...ial_source_config => config_source_shared} | 20 +- ...ndows => config_source_shared_for_windows} | 8 +- config/testdata/credentials_source_shared | 7 + .../credentials_source_shared_for_windows | 7 + config/testdata/empty_creds_config | 1 + config/testdata/load_config | 19 + config/testdata/load_config_secondary | 3 + config/testdata/load_credentials | 16 + config/testdata/load_credentials_secondary | 3 + config/testdata/shared_config | 40 +- config/testdata/shared_config_other | 6 +- internal/ini/bench_test.go | 2 +- internal/ini/ini.go | 10 +- internal/ini/literal_tokens.go | 10 + internal/ini/testdata/valid/commented_profile | 1 + .../testdata/valid/commented_profile_expected | 3 +- internal/ini/testdata/valid/nested_fields | 2 +- internal/ini/visitor.go | 108 +++- internal/ini/walker_test.go | 7 +- 30 files changed, 1190 insertions(+), 270 deletions(-) rename config/testdata/{credential_source_config => config_source_shared} (65%) rename config/testdata/{credential_source_config_for_windows => config_source_shared_for_windows} (61%) create mode 100644 config/testdata/credentials_source_shared create mode 100644 config/testdata/credentials_source_shared_for_windows create mode 100644 config/testdata/empty_creds_config create mode 100644 config/testdata/load_config create mode 100644 config/testdata/load_config_secondary create mode 100644 config/testdata/load_credentials create mode 100644 config/testdata/load_credentials_secondary diff --git a/aws/credentials.go b/aws/credentials.go index 96d677f1ae6..ce3868a9f01 100644 --- a/aws/credentials.go +++ b/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. diff --git a/config/env_config.go b/config/env_config.go index c5e642cea67..cb589cf0ff9 100644 --- a/config/env_config.go +++ b/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) - } 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) { + files := make([]string, 0, 2) + 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 { diff --git a/config/example_test.go b/config/example_test.go index a81d29a6b93..4b0f39fb765 100644 --- a/config/example_test.go +++ b/config/example_test.go @@ -3,6 +3,8 @@ package config_test import ( "context" "fmt" + "github.com/awslabs/smithy-go/middleware" + smithyhttp "github.com/awslabs/smithy-go/transport/http" "log" "net/http" "path/filepath" diff --git a/config/load_options.go b/config/load_options.go index a6a9d5a1156..06f6fa3e746 100644 --- a/config/load_options.go +++ b/config/load_options.go @@ -56,9 +56,22 @@ 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'. + // If duplicate profiles are provided with a same, or across multiple shared config files, the next parsed + // profile will override only properties that conflict with the previously defined profile. 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. + // 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. + // If duplicate profiles are provided within a shared credentials and shared config files, the properties + // defined in shared credentials file take precedence. + SharedCredentialsFiles []string + // CustomCABundle is CA bundle PEM bytes reader CustomCABundle io.Reader @@ -186,6 +199,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 { diff --git a/config/provider.go b/config/provider.go index 611ef77ec59..cd282923517 100644 --- a/config/provider.go +++ b/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) diff --git a/config/resolve_assume_role_test.go b/config/resolve_assume_role_test.go index 84a777abd9d..74e662ab29b 100644 --- a/config/resolve_assume_role_test.go +++ b/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) { diff --git a/config/resolve_credentials_test.go b/config/resolve_credentials_test.go index 2a059eb0010..c051ac344a7 100644 --- a/config/resolve_credentials_test.go +++ b/config/resolve_credentials_test.go @@ -3,6 +3,8 @@ package config import ( "context" "fmt" + "github.com/aws/aws-sdk-go-v2/internal/awstesting" + "github.com/awslabs/smithy-go/middleware" "net/http" "net/http/httptest" "os" @@ -14,9 +16,7 @@ import ( "time" "github.com/aws/aws-sdk-go-v2/aws" - "github.com/aws/aws-sdk-go-v2/internal/awstesting" "github.com/aws/aws-sdk-go-v2/service/sts" - "github.com/aws/smithy-go/middleware" ) func swapECSContainerURI(path string) func() { @@ -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") diff --git a/config/resolve_processcreds_test.go b/config/resolve_processcreds_test.go index 7d46f7ad1f0..aca98038571 100644 --- a/config/resolve_processcreds_test.go +++ b/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() { diff --git a/config/resolve_test.go b/config/resolve_test.go index 984f66655ae..3d4b8e98e32 100644 --- a/config/resolve_test.go +++ b/config/resolve_test.go @@ -3,6 +3,7 @@ package config import ( "bytes" "context" + "github.com/awslabs/smithy-go/logging" "io/ioutil" "net/http" "testing" @@ -12,7 +13,6 @@ import ( "github.com/aws/aws-sdk-go-v2/credentials" "github.com/aws/aws-sdk-go-v2/internal/awstesting" "github.com/aws/aws-sdk-go-v2/internal/awstesting/unit" - "github.com/aws/smithy-go/logging" ) func TestResolveCustomCABundle(t *testing.T) { diff --git a/config/shared_config.go b/config/shared_config.go index 24b819e6723..d48affacdda 100644 --- a/config/shared_config.go +++ b/config/shared_config.go @@ -4,9 +4,11 @@ import ( "context" "errors" "fmt" + "github.com/awslabs/smithy-go/logging" "os" "path/filepath" "runtime" + "strings" "time" "github.com/aws/aws-sdk-go-v2/aws" @@ -77,10 +79,15 @@ func DefaultSharedConfigFilename() string { // DefaultSharedConfigFiles is a slice of the default shared config files that // the will be used in order to load the SharedConfig. var DefaultSharedConfigFiles = []string{ - DefaultSharedCredentialsFilename(), DefaultSharedConfigFilename(), } +// DefaultSharedCredentialsFiles is a slice of the default shared credentials files that +// the will be used in order to load the SharedConfig. +var DefaultSharedCredentialsFiles = []string{ + DefaultSharedCredentialsFilename(), +} + // SharedConfig represents the configuration fields of the SDK config files. type SharedConfig struct { Profile string @@ -157,7 +164,7 @@ func (c SharedConfig) getCredentialsProvider() (aws.Credentials, bool, error) { func loadSharedConfigIgnoreNotExist(ctx context.Context, configs configs) (Config, error) { cfg, err := loadSharedConfig(ctx, configs) if err != nil { - if _, ok := err.(SharedConfigNotExistErrors); ok { + if _, ok := err.(SharedConfigProfileNotExistError); ok { return SharedConfig{}, nil } return nil, err @@ -180,7 +187,8 @@ func loadSharedConfigIgnoreNotExist(ctx context.Context, configs configs) (Confi // * sharedConfigFilesProvider func loadSharedConfig(ctx context.Context, configs configs) (Config, error) { var profile string - var files []string + var configurationFiles []string + var credentialsFiles []string var ok bool var err error @@ -192,15 +200,56 @@ func loadSharedConfig(ctx context.Context, configs configs) (Config, error) { profile = defaultSharedConfigProfile } - files, ok, err = getSharedConfigFiles(ctx, configs) + configurationFiles, ok, err = getSharedConfigFiles(ctx, configs) if err != nil { return nil, err } if !ok { - files = DefaultSharedConfigFiles + configurationFiles = DefaultSharedConfigFiles } - return NewSharedConfig(profile, files) + credentialsFiles, ok, err = getSharedCredentialsFiles(ctx, configs) + if err != nil { + return nil, err + } + if !ok { + credentialsFiles = DefaultSharedCredentialsFiles + } + + // setup logger if log configuration warning is seti + var logger logging.Logger + logWarnings, found, err := getLogConfigurationWarnings(ctx, configs) + if err != nil { + return SharedConfig{}, err + } + if found && logWarnings { + logger, found, err = getLogger(ctx, configs) + if err != nil { + return SharedConfig{}, err + } + if !found { + logger = logging.NewStandardLogger(os.Stderr) + } + } + + return NewSharedConfig(ctx, profile, sharedConfigsLoadInfo{ + CredentialsFiles: credentialsFiles, + ConfigurationFiles: configurationFiles, + Logger: logger, + }) +} + +// sharedConfigsLoadInfo struct contains Values that can be used to load the config. +type sharedConfigsLoadInfo struct { + + // CredentialsFiles are the shared credentials files + CredentialsFiles []string + + // ConfigurationFiles are the shared config files + ConfigurationFiles []string + + // Logger is the logger used to log shared config behavior + Logger logging.Logger } // NewSharedConfig retrieves the configuration from the list of files @@ -210,78 +259,478 @@ func loadSharedConfig(ctx context.Context, configs configs) (Config, error) { // // For example, given two files A and B. Both define credentials. If the order // of the files are A then B, B's credential values will be used instead of A's. -func NewSharedConfig(profile string, filenames []string) (SharedConfig, error) { - if len(filenames) == 0 { - return SharedConfig{}, fmt.Errorf("no shared config files provided") +func NewSharedConfig(ctx context.Context, profile string, options sharedConfigsLoadInfo) (SharedConfig, error) { + if len(options.ConfigurationFiles) == 0 && len(options.CredentialsFiles) == 0 { + return SharedConfig{}, fmt.Errorf("no shared config or shared credentials options provided") } - files, err := loadSharedConfigIniFiles(filenames) + // load shared configuration sections from shared configuration INI options + configSections, err := loadIniFiles(options.ConfigurationFiles) if err != nil { return SharedConfig{}, err } + // check for profile prefix and drop duplicates or invalid profiles + err = processConfigSections(ctx, configSections, options.Logger) + if err != nil { + return SharedConfig{}, err + } + + // load shared credentials sections from shared credentials INI options + credentialsSections, err := loadIniFiles(options.CredentialsFiles) + if err != nil { + return SharedConfig{}, err + } + + // check for profile prefix and drop duplicates or invalid profiles + err = processCredentialsSections(ctx, credentialsSections, options.Logger) + if err != nil { + return SharedConfig{}, err + } + + err = mergeSections(configSections, credentialsSections) + if err != nil { + return SharedConfig{}, err + } + + // profile should be lower-cased to standardize + profile = strings.ToLower(profile) + cfg := SharedConfig{} profiles := map[string]struct{}{} - if err = cfg.setFromIniFiles(profiles, profile, files); err != nil { + if err = cfg.setFromIniSections(profiles, profile, configSections, options.Logger); err != nil { return SharedConfig{}, err } return cfg, nil } -type sharedConfigFile struct { +func processConfigSections(ctx context.Context, sections ini.Sections, logger logging.Logger) error { + + prefix := "profile " + for _, section := range sections.List() { + // drop profiles without prefix for config files + if !strings.HasPrefix(section, prefix) && !strings.EqualFold(section, "default") { + // drop this section, as invalid profile name + sections.DeleteSection(section) + + if logger != nil { + logger.Logf(logging.Debug, + "profile %v is ignored. A non-default profile without the \"profile \" prefix is invalid "+ + "for the shared configuration file.\n", + section, + ) + } + } + } + + // rename sections to remove `profile ` prefixing to match with credentials file. + // if default is already present, it will be dropped. + for _, section := range sections.List() { + if strings.HasPrefix(section, prefix) { + v, ok := sections.GetSection(section) + if !ok { + return fmt.Errorf("error processing profiles within the shared configuration files") + } + + // delete section with profile as prefix + sections.DeleteSection(section) + + // set the value to non-prefixed name in sections. + section = strings.TrimPrefix(section, prefix) + if sections.HasSection(section) { + oldSection, _ := sections.GetSection(section) + v.Logs = append(v.Logs, + fmt.Sprintf("A default profile prefixed with `profile` found in %s, "+ + "overrided non-prefixed default profile from %s", v.SourceFile, oldSection.SourceFile)) + } + + // assign non-prefixed name to section + v.Name = section + sections.SetSection(section, v) + } + } + return nil +} + +func processCredentialsSections(ctx context.Context, sections ini.Sections, logger logging.Logger) error { + + prefix := "profile" + for _, section := range sections.List() { + // drop profiles with prefix for credential files + if strings.HasPrefix(section, prefix) { + // drop this section, as invalid profile name + sections.DeleteSection(section) + + if logger != nil { + logger.Logf(logging.Debug, + "The %v is ignored. A profile with the \"profile \" prefix is invalid "+ + "for the shared credentials file.\n", + section, + ) + } + } + } + return nil +} + +type parsedINIFile struct { Filename string IniData ini.Sections } -func loadSharedConfigIniFiles(filenames []string) ([]sharedConfigFile, error) { - files := make([]sharedConfigFile, 0, len(filenames)) +func loadIniFiles(filenames []string) (ini.Sections, error) { + mergedSections := ini.NewSections() - var errs SharedConfigNotExistErrors for _, filename := range filenames { sections, err := ini.OpenFile(filename) var v *ini.UnableToReadFile if ok := errors.As(err, &v); ok { - errs = append(errs, - SharedConfigFileNotExistError{Filename: filename, Err: err}, - ) - // Skip files which can't be opened and read for whatever reason + // Skip files which can't be opened and read for whatever reason. + // We treat such files as empty, and do not fall back to other locations. continue } else if err != nil { - return nil, SharedConfigLoadError{Filename: filename, Err: err} + return ini.Sections{}, SharedConfigLoadError{Filename: filename, Err: err} } - files = append(files, sharedConfigFile{ - Filename: filename, IniData: sections, - }) + // mergeSections into mergedSections + err = mergeSections(mergedSections, sections) + if err != nil { + return ini.Sections{}, SharedConfigLoadError{Filename: filename, Err: err} + } } - if len(files) == 0 { - return nil, errs + return mergedSections, nil +} + +// mergeSections merges source section properties into destination section properties +func mergeSections(dst, src ini.Sections) error { + for _, sectionName := range src.List() { + srcSection, _ := src.GetSection(sectionName) + + if (!srcSection.Has(accessKeyIDKey) && srcSection.Has(secretAccessKey)) || + (srcSection.Has(accessKeyIDKey) && !srcSection.Has(secretAccessKey)) { + srcSection.Errors = append(srcSection.Errors, + fmt.Errorf("Partial Credentials found for profile %v", sectionName)) + } + + if !dst.HasSection(sectionName) { + dst.SetSection(sectionName, srcSection) + continue + } + + // merge with destination srcSection + dstSection, _ := dst.GetSection(sectionName) + + // errors should be overriden if any + dstSection.Errors = srcSection.Errors + + // Access key id update + if srcSection.Has(accessKeyIDKey) && srcSection.Has(secretAccessKey) { + accessKey := srcSection.String(accessKeyIDKey) + secretKey := srcSection.String(secretAccessKey) + + if dstSection.Has(accessKeyIDKey) { + dstSection.Logs = append(dstSection.Logs, + fmt.Sprintf("For profile: %v, overriding credentials value for aws access key id, "+ + "and aws secret access key, defined in %v, with values found in a duplicate profile "+ + "defined at file %v. \n", + sectionName, dstSection.SourceFile[accessKeyIDKey], + srcSection.SourceFile[accessKeyIDKey])) + } + + // update access key + v, err := ini.NewStringValue(accessKey) + if err != nil { + return fmt.Errorf("error merging access key, %w", err) + } + dstSection.UpdateValue(accessKeyIDKey, v) + + // update secret key + v, err = ini.NewStringValue(secretKey) + if err != nil { + return fmt.Errorf("error merging secret key, %w", err) + } + dstSection.UpdateValue(secretAccessKey, v) + + // update session token + if srcSection.Has(sessionTokenKey) { + sessionKey := srcSection.String(sessionTokenKey) + + val, e := ini.NewStringValue(sessionKey) + if e != nil { + return fmt.Errorf("error merging session key, %w", e) + } + + if dstSection.Has(sessionTokenKey) { + dstSection.Logs = append(dstSection.Logs, + fmt.Sprintf("For profile: %v, overriding %v value, defined in %v "+ + "with a %v value found in a duplicate profile defined at file %v. \n", + sectionName, sessionTokenKey, dstSection.SourceFile[sessionTokenKey], + sessionTokenKey, srcSection.SourceFile[sessionTokenKey])) + } + + dstSection.UpdateValue(sessionTokenKey, val) + dstSection.UpdateSourceFile(sessionTokenKey, srcSection.SourceFile[sessionTokenKey]) + } + + // update source file to reflect where the static creds came from + dstSection.UpdateSourceFile(accessKeyIDKey, srcSection.SourceFile[accessKeyIDKey]) + dstSection.UpdateSourceFile(secretAccessKey, srcSection.SourceFile[secretAccessKey]) + } + + if srcSection.Has(roleArnKey) { + key := srcSection.String(roleArnKey) + val, err := ini.NewStringValue(key) + if err != nil { + return fmt.Errorf("error merging roleArnKey, %w", err) + } + + if dstSection.Has(roleArnKey) { + dstSection.Logs = append(dstSection.Logs, + fmt.Sprintf("For profile: %v, overriding %v value, defined in %v "+ + "with a %v value found in a duplicate profile defined at file %v. \n", + sectionName, roleArnKey, dstSection.SourceFile[roleArnKey], + roleArnKey, srcSection.SourceFile[roleArnKey])) + } + + dstSection.UpdateValue(roleArnKey, val) + dstSection.UpdateSourceFile(roleArnKey, srcSection.SourceFile[roleArnKey]) + } + + if srcSection.Has(sourceProfileKey) { + key := srcSection.String(sourceProfileKey) + val, err := ini.NewStringValue(key) + if err != nil { + return fmt.Errorf("error merging sourceProfileKey, %w", err) + } + + if dstSection.Has(sourceProfileKey) { + dstSection.Logs = append(dstSection.Logs, + fmt.Sprintf("For profile: %v, overriding %v value, defined in %v "+ + "with a %v value found in a duplicate profile defined at file %v. \n", + sectionName, sourceProfileKey, dstSection.SourceFile[sourceProfileKey], + sourceProfileKey, srcSection.SourceFile[sourceProfileKey])) + } + + dstSection.UpdateValue(sourceProfileKey, val) + dstSection.UpdateSourceFile(sourceProfileKey, srcSection.SourceFile[sourceProfileKey]) + } + + if srcSection.Has(credentialSourceKey) { + key := srcSection.String(credentialSourceKey) + val, err := ini.NewStringValue(key) + if err != nil { + return fmt.Errorf("error merging credentialSourceKey, %w", err) + } + + if dstSection.Has(credentialSourceKey) { + dstSection.Logs = append(dstSection.Logs, + fmt.Sprintf("For profile: %v, overriding %v value, defined in %v "+ + "with a %v value found in a duplicate profile defined at file %v. \n", + sectionName, credentialSourceKey, dstSection.SourceFile[credentialSourceKey], + credentialSourceKey, srcSection.SourceFile[credentialSourceKey])) + } + + dstSection.UpdateValue(credentialSourceKey, val) + dstSection.UpdateSourceFile(credentialSourceKey, srcSection.SourceFile[credentialSourceKey]) + } + + if srcSection.Has(externalIDKey) { + key := srcSection.String(externalIDKey) + val, err := ini.NewStringValue(key) + if err != nil { + return fmt.Errorf("error merging externalIDKey, %w", err) + } + + if dstSection.Has(externalIDKey) { + dstSection.Logs = append(dstSection.Logs, + fmt.Sprintf("For profile: %v, overriding %v value, defined in %v "+ + "with a %v value found in a duplicate profile defined at file %v. \n", + sectionName, externalIDKey, dstSection.SourceFile[externalIDKey], + externalIDKey, srcSection.SourceFile[externalIDKey])) + } + + dstSection.UpdateValue(externalIDKey, val) + dstSection.UpdateSourceFile(externalIDKey, srcSection.SourceFile[externalIDKey]) + } + + if srcSection.Has(mfaSerialKey) { + key := srcSection.String(mfaSerialKey) + val, err := ini.NewStringValue(key) + if err != nil { + return fmt.Errorf("error merging mfaSerialKey, %w", err) + } + + if dstSection.Has(mfaSerialKey) { + dstSection.Logs = append(dstSection.Logs, + fmt.Sprintf("For profile: %v, overriding %v value, defined in %v "+ + "with a %v value found in a duplicate profile defined at file %v. \n", + sectionName, mfaSerialKey, dstSection.SourceFile[mfaSerialKey], + mfaSerialKey, srcSection.SourceFile[mfaSerialKey])) + } + + dstSection.UpdateValue(mfaSerialKey, val) + dstSection.UpdateSourceFile(mfaSerialKey, srcSection.SourceFile[mfaSerialKey]) + } + + if srcSection.Has(roleSessionNameKey) { + key := srcSection.String(roleSessionNameKey) + val, err := ini.NewStringValue(key) + if err != nil { + return fmt.Errorf("error merging roleSessionNameKey, %w", err) + } + + if dstSection.Has(roleSessionNameKey) { + dstSection.Logs = append(dstSection.Logs, + fmt.Sprintf("For profile: %v, overriding %v value, defined in %v "+ + "with a %v value found in a duplicate profile defined at file %v. \n", + sectionName, roleSessionNameKey, dstSection.SourceFile[roleSessionNameKey], + roleSessionNameKey, srcSection.SourceFile[roleSessionNameKey])) + } + + dstSection.UpdateValue(roleSessionNameKey, val) + dstSection.UpdateSourceFile(roleSessionNameKey, srcSection.SourceFile[roleSessionNameKey]) + } + + if srcSection.Has(regionKey) { + key := srcSection.String(regionKey) + val, err := ini.NewStringValue(key) + if err != nil { + return fmt.Errorf("error merging regionKey, %w", err) + } + + if dstSection.Has(regionKey) { + dstSection.Logs = append(dstSection.Logs, + fmt.Sprintf("For profile: %v, overriding %v value, defined in %v "+ + "with a %v value found in a duplicate profile defined at file %v. \n", + sectionName, regionKey, dstSection.SourceFile[regionKey], + regionKey, srcSection.SourceFile[regionKey])) + } + + dstSection.UpdateValue(regionKey, val) + dstSection.UpdateSourceFile(regionKey, srcSection.SourceFile[regionKey]) + } + + if srcSection.Has(enableEndpointDiscoveryKey) { + key := srcSection.String(enableEndpointDiscoveryKey) + val, err := ini.NewStringValue(key) + if err != nil { + return fmt.Errorf("error merging enableEndpointDiscoveryKey, %w", err) + } + + if dstSection.Has(enableEndpointDiscoveryKey) { + dstSection.Logs = append(dstSection.Logs, + fmt.Sprintf("For profile: %v, overriding %v value, defined in %v "+ + "with a %v value found in a duplicate profile defined at file %v. \n", + sectionName, enableEndpointDiscoveryKey, dstSection.SourceFile[enableEndpointDiscoveryKey], + enableEndpointDiscoveryKey, srcSection.SourceFile[enableEndpointDiscoveryKey])) + } + + dstSection.UpdateValue(enableEndpointDiscoveryKey, val) + dstSection.UpdateSourceFile(enableEndpointDiscoveryKey, srcSection.SourceFile[enableEndpointDiscoveryKey]) + } + + if srcSection.Has(credentialProcessKey) { + key := srcSection.String(credentialProcessKey) + val, err := ini.NewStringValue(key) + if err != nil { + return fmt.Errorf("error merging credentialProcessKey, %w", err) + } + + if dstSection.Has(credentialProcessKey) { + dstSection.Logs = append(dstSection.Logs, + fmt.Sprintf("For profile: %v, overriding %v value, defined in %v "+ + "with a %v value found in a duplicate profile defined at file %v. \n", + sectionName, credentialProcessKey, dstSection.SourceFile[credentialProcessKey], + credentialProcessKey, srcSection.SourceFile[credentialProcessKey])) + } + + dstSection.UpdateValue(credentialProcessKey, val) + dstSection.UpdateSourceFile(credentialProcessKey, srcSection.SourceFile[credentialProcessKey]) + } + + if srcSection.Has(webIdentityTokenFileKey) { + key := srcSection.String(webIdentityTokenFileKey) + val, err := ini.NewStringValue(key) + if err != nil { + return fmt.Errorf("error merging webIdentityTokenFileKey, %w", err) + } + + if dstSection.Has(webIdentityTokenFileKey) { + dstSection.Logs = append(dstSection.Logs, + fmt.Sprintf("For profile: %v, overriding %v value, defined in %v "+ + "with a %v value found in a duplicate profile defined at file %v. \n", + sectionName, webIdentityTokenFileKey, dstSection.SourceFile[webIdentityTokenFileKey], + webIdentityTokenFileKey, srcSection.SourceFile[webIdentityTokenFileKey])) + } + + dstSection.UpdateValue(webIdentityTokenFileKey, val) + dstSection.UpdateSourceFile(webIdentityTokenFileKey, srcSection.SourceFile[webIdentityTokenFileKey]) + } + + if srcSection.Has(s3UseARNRegionKey) { + key := srcSection.String(s3UseARNRegionKey) + val, err := ini.NewStringValue(key) + if err != nil { + return fmt.Errorf("error merging s3UseARNRegionKey, %w", err) + } + + if dstSection.Has(s3UseARNRegionKey) { + dstSection.Logs = append(dstSection.Logs, + fmt.Sprintf("For profile: %v, overriding %v value, defined in %v "+ + "with a %v value found in a duplicate profile defined at file %v. \n", + sectionName, s3UseARNRegionKey, dstSection.SourceFile[s3UseARNRegionKey], + s3UseARNRegionKey, srcSection.SourceFile[s3UseARNRegionKey])) + } + + dstSection.UpdateValue(s3UseARNRegionKey, val) + dstSection.UpdateSourceFile(s3UseARNRegionKey, srcSection.SourceFile[s3UseARNRegionKey]) + } + + // role duration seconds key update + if srcSection.Has(roleDurationSecondsKey) { + roleDurationSeconds := srcSection.Int(roleDurationSecondsKey) + v, err := ini.NewIntValue(roleDurationSeconds) + if err != nil { + return fmt.Errorf("error merging role duration seconds key, %w", err) + } + dstSection.UpdateValue(roleDurationSecondsKey, v) + + dstSection.UpdateSourceFile(roleDurationSecondsKey, srcSection.SourceFile[roleDurationSecondsKey]) + } + + // set srcSection on dst srcSection + dst = dst.SetSection(sectionName, dstSection) } - return files, nil + return nil } // Returns an error if all of the files fail to load. If at least one file is // successfully loaded and contains the profile, no error will be returned. -func (c *SharedConfig) setFromIniFiles(profiles map[string]struct{}, profile string, files []sharedConfigFile) error { +func (c *SharedConfig) setFromIniSections(profiles map[string]struct{}, profile string, + sections ini.Sections, logger logging.Logger) error { c.Profile = profile - // Trim files from the list that don't exist. - existErrs := SharedConfigNotExistErrors{} - for _, f := range files { - if err := c.setFromIniFile(profile, f); err != nil { - if _, ok := err.(SharedConfigProfileNotExistError); ok { - existErrs = append(existErrs, err) - continue - } - return err + section, ok := sections.GetSection(profile) + if !ok { + return SharedConfigProfileNotExistError{ + Profile: profile, + } + } + + // if logs are appended to the section, log them + if section.Logs != nil && logger != nil { + for _, log := range section.Logs { + logger.Logf(logging.Debug, log) } } - if len(existErrs) == len(files) { - return existErrs + // set config from the provided ini section + err := c.setFromIniSection(profile, section) + if err != nil { + return fmt.Errorf("error fetching config from profile, %v, %w", profile, err) } if _, ok := profiles[profile]; ok { @@ -298,8 +747,15 @@ func (c *SharedConfig) setFromIniFiles(profiles map[string]struct{}, profile str return err } } + + // if not top level profile and has credentials, return with credentials. + if len(profiles) != 0 && c.Credentials.HasKeys() { + return nil + } + profiles[profile] = struct{}{} + // validate no colliding credentials type are present if err := c.validateCredentialType(); err != nil { return err } @@ -311,10 +767,10 @@ func (c *SharedConfig) setFromIniFiles(profiles map[string]struct{}, profile str c.clearCredentialOptions() srcCfg := &SharedConfig{} - err := srcCfg.setFromIniFiles(profiles, c.SourceProfileName, files) + err := srcCfg.setFromIniSections(profiles, c.SourceProfileName, sections, logger) if err != nil { // SourceProfileName that doesn't exist is an error in configuration. - if _, ok := err.(SharedConfigNotExistErrors); ok { + if _, ok := err.(SharedConfigProfileNotExistError); ok { err = SharedConfigAssumeRoleError{ RoleARN: c.RoleARN, Profile: c.SourceProfileName, @@ -337,26 +793,34 @@ func (c *SharedConfig) setFromIniFiles(profiles map[string]struct{}, profile str return nil } -// setFromFile loads the configuration from the file using -// the profile provided. A SharedConfig pointer type value is used so that +// setFromIniSection loads the configuration from the profile section defined in +// the provided ini file. A SharedConfig pointer type value is used so that // multiple config file loadings can be chained. // // Only loads complete logically grouped values, and will not set fields in cfg // for incomplete grouped values in the config. Such as credentials. For example // if a config file only includes aws_access_key_id but no aws_secret_access_key // the aws_access_key_id will be ignored. -func (c *SharedConfig) setFromIniFile(profile string, file sharedConfigFile) error { - section, ok := file.IniData.GetSection(profile) - if !ok { - // Fallback to to alternate profile name: profile - section, ok = file.IniData.GetSection(fmt.Sprintf("profile %s", profile)) - if !ok { - return SharedConfigProfileNotExistError{ - Filename: file.Filename, - Profile: profile, - Err: nil, - } +func (c *SharedConfig) setFromIniSection(profile string, section ini.Section) error { + if len(section.Name) == 0 { + sources := make([]string, 0) + for _, v := range section.SourceFile { + sources = append(sources, v) + } + + return SharedConfigProfileNotExistError{ + Filename: sources, + Profile: profile, + Err: nil, + } + } + + if len(section.Errors) != 0 { + var errStatement string + for i, e := range section.Errors { + errStatement = fmt.Sprintf("%d, %v\n", i+1, e.Error()) } + return fmt.Errorf("Error using profile: \n %v", errStatement) } // Assume Role @@ -384,7 +848,7 @@ func (c *SharedConfig) setFromIniFile(profile string, file sharedConfigFile) err AccessKeyID: section.String(accessKeyIDKey), SecretAccessKey: section.String(secretAccessKey), SessionToken: section.String(sessionTokenKey), - Source: fmt.Sprintf("SharedConfigCredentials: %s", file.Filename), + Source: fmt.Sprintf("SharedConfigCredentials: %s", section.SourceFile[accessKeyIDKey]), } if creds.HasKeys() { @@ -459,18 +923,6 @@ func (c *SharedConfig) clearCredentialOptions() { c.Credentials = aws.Credentials{} } -// SharedConfigNotExistErrors provides an error type for failure to load shared -// config because resources do not exist. -type SharedConfigNotExistErrors []error - -func (es SharedConfigNotExistErrors) Error() string { - msg := "failed to load shared config\n" - for _, e := range es { - msg += "\t" + e.Error() - } - return msg -} - // SharedConfigLoadError is an error for the shared config file failed to load. type SharedConfigLoadError struct { Filename string @@ -486,27 +938,10 @@ func (e SharedConfigLoadError) Error() string { return fmt.Sprintf("failed to load shared config file, %s, %v", e.Filename, e.Err) } -// SharedConfigFileNotExistError is an error for the shared config when -// the filename does not exist. -type SharedConfigFileNotExistError struct { - Filename string - Profile string - Err error -} - -// Unwrap returns the underlying error that caused the failure. -func (e SharedConfigFileNotExistError) Unwrap() error { - return e.Err -} - -func (e SharedConfigFileNotExistError) Error() string { - return fmt.Sprintf("failed to open shared config file, %s, %v", e.Filename, e.Err) -} - // SharedConfigProfileNotExistError is an error for the shared config when // the profile was not find in the config file. type SharedConfigProfileNotExistError struct { - Filename string + Filename []string Profile string Err error } @@ -517,7 +952,7 @@ func (e SharedConfigProfileNotExistError) Unwrap() error { } func (e SharedConfigProfileNotExistError) Error() string { - return fmt.Sprintf("failed to get shared config profile, %s, in %s, %v", e.Profile, e.Filename, e.Err) + return fmt.Sprintf("failed to get shared config profile, %s", e.Profile) } // SharedConfigAssumeRoleError is an error for the shared config when the diff --git a/config/shared_config_test.go b/config/shared_config_test.go index b161c9d6a27..b8f7fc74746 100644 --- a/config/shared_config_test.go +++ b/config/shared_config_test.go @@ -1,17 +1,18 @@ package config import ( + "bytes" "context" "fmt" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/internal/ini" + "github.com/awslabs/smithy-go/logging" + "github.com/awslabs/smithy-go/ptr" "path/filepath" "reflect" "strconv" "strings" "testing" - - "github.com/aws/aws-sdk-go-v2/aws" - "github.com/aws/aws-sdk-go-v2/internal/ini" - "github.com/aws/smithy-go/ptr" ) var _ regionProvider = (*SharedConfig)(nil) @@ -22,18 +23,18 @@ var ( ) func TestNewSharedConfig(t *testing.T) { - cases := []struct { + cases := map[string]struct { Filenames []string Profile string Expected SharedConfig Err error }{ - 0: { + "file not exist": { Filenames: []string{"file_not_exist"}, Profile: "default", - Err: fmt.Errorf("failed to open shared config file, file_not_exist"), + Err: fmt.Errorf("failed to get shared config profile"), }, - 1: { + "default profile": { Filenames: []string{testConfigFilename}, Profile: "default", Expected: SharedConfig{ @@ -41,7 +42,7 @@ func TestNewSharedConfig(t *testing.T) { Region: "default_region", }, }, - 2: { + "multiple config files": { Filenames: []string{testConfigOtherFilename, testConfigFilename}, Profile: "config_file_load_order", Expected: SharedConfig{ @@ -54,7 +55,7 @@ func TestNewSharedConfig(t *testing.T) { }, }, }, - 3: { + "mutliple config files reverse order": { Filenames: []string{testConfigFilename, testConfigOtherFilename}, Profile: "config_file_load_order", Expected: SharedConfig{ @@ -67,7 +68,7 @@ func TestNewSharedConfig(t *testing.T) { }, }, }, - 4: { + "Assume role": { Filenames: []string{testConfigOtherFilename, testConfigFilename}, Profile: "assume_role", Expected: SharedConfig{ @@ -84,27 +85,19 @@ func TestNewSharedConfig(t *testing.T) { }, }, }, - 5: { + "Assume role with invalid source profile": { Filenames: []string{testConfigOtherFilename, testConfigFilename}, Profile: "assume_role_invalid_source_profile", Err: SharedConfigAssumeRoleError{ Profile: "profile_not_exists", RoleARN: "assume_role_invalid_source_profile_role_arn", - Err: SharedConfigNotExistErrors{ - SharedConfigProfileNotExistError{ - Profile: "profile_not_exists", - Filename: testConfigOtherFilename, - Err: nil, - }, - SharedConfigProfileNotExistError{ - Profile: "profile_not_exists", - Filename: testConfigFilename, - Err: nil, - }, + Err: SharedConfigProfileNotExistError{ + Profile: "profile_not_exists", + Err: nil, }, }, }, - 6: { + "Assume role with creds": { Filenames: []string{testConfigOtherFilename, testConfigFilename}, Profile: "assume_role_w_creds", Expected: SharedConfig{ @@ -123,7 +116,7 @@ func TestNewSharedConfig(t *testing.T) { }, }, }, - 7: { + "Assume role without creds": { Filenames: []string{testConfigOtherFilename, testConfigFilename}, Profile: "assume_role_wo_creds", Expected: SharedConfig{ @@ -136,7 +129,7 @@ func TestNewSharedConfig(t *testing.T) { RoleARN: "assume_role_wo_creds_role_arn", }, }, - 8: { + "Invalid INI file": { Filenames: []string{filepath.Join("testdata", "shared_config_invalid_ini")}, Profile: "profile_name", Err: SharedConfigLoadError{ @@ -144,7 +137,7 @@ func TestNewSharedConfig(t *testing.T) { Err: fmt.Errorf("invalid state"), }, }, - 9: { + "S3UseARNRegion property on profile": { Profile: "valid_arn_region", Filenames: []string{testConfigFilename}, Expected: SharedConfig{ @@ -152,7 +145,7 @@ func TestNewSharedConfig(t *testing.T) { S3UseARNRegion: ptr.Bool(true), }, }, - 10: { + "EndpointDiscovery property on profile": { Profile: "endpoint_discovery", Filenames: []string{testConfigFilename}, Expected: SharedConfig{ @@ -160,7 +153,7 @@ func TestNewSharedConfig(t *testing.T) { EnableEndpointDiscovery: ptr.Bool(true), }, }, - 11: { + "Assume role with credential source Ec2Metadata": { Filenames: []string{testConfigOtherFilename, testConfigFilename}, Profile: "assume_role_with_credential_source", Expected: SharedConfig{ @@ -169,7 +162,7 @@ func TestNewSharedConfig(t *testing.T) { CredentialSource: credSourceEc2Metadata, }, }, - 12: { + "Assume role chained with creds": { Filenames: []string{testConfigOtherFilename, testConfigFilename}, Profile: "multiple_assume_role", Expected: SharedConfig{ @@ -191,7 +184,7 @@ func TestNewSharedConfig(t *testing.T) { }, }, }, - 13: { + "Assume role chained with credential source": { Filenames: []string{testConfigOtherFilename, testConfigFilename}, Profile: "multiple_assume_role_with_credential_source", Expected: SharedConfig{ @@ -205,7 +198,7 @@ func TestNewSharedConfig(t *testing.T) { }, }, }, - 14: { + "Assume role chained with credential source reversed order": { Filenames: []string{testConfigOtherFilename, testConfigFilename}, Profile: "multiple_assume_role_with_credential_source2", Expected: SharedConfig{ @@ -226,9 +219,11 @@ func TestNewSharedConfig(t *testing.T) { }, } - for i, c := range cases { - t.Run(strconv.Itoa(i), func(t *testing.T) { - cfg, err := NewSharedConfig(c.Profile, c.Filenames) + for name, c := range cases { + t.Run(name, func(t *testing.T) { + cfg, err := NewSharedConfig(context.TODO(), c.Profile, sharedConfigsLoadInfo{ + ConfigurationFiles: c.Filenames, + }) if c.Err != nil { if e, a := c.Err.Error(), err.Error(); !strings.Contains(a, e) { t.Errorf("expect %q to be in %q", e, a) @@ -246,112 +241,124 @@ func TestNewSharedConfig(t *testing.T) { } } -func TestLoadSharedConfigFromFile(t *testing.T) { +func TestLoadSharedConfigFromSection(t *testing.T) { filename := testConfigFilename sections, err := ini.OpenFile(filename) + if err != nil { t.Fatalf("failed to load test config file, %s, %v", filename, err) } - iniFile := sharedConfigFile{IniData: sections, Filename: filename} - - cases := []struct { + cases := map[string]struct { Profile string Expected SharedConfig Err error }{ - 0: { + "Default as profile": { Profile: "default", Expected: SharedConfig{Region: "default_region"}, }, - 1: { - Profile: "alt_profile_name", + "prefixed profile": { + Profile: "profile alt_profile_name", Expected: SharedConfig{Region: "alt_profile_name_region"}, }, - 2: { - Profile: "short_profile_name_first", - Expected: SharedConfig{Region: "short_profile_name_first_short"}, + "prefixed profile 2": { + Profile: "profile short_profile_name_first", + Expected: SharedConfig{Region: "short_profile_name_first_alt"}, }, - 3: { - Profile: "partial_creds", + "profile with partial creds": { + Profile: "profile partial_creds", Expected: SharedConfig{}, }, - 4: { - Profile: "complete_creds", + "profile with complete creds": { + Profile: "profile complete_creds", Expected: SharedConfig{ Credentials: aws.Credentials{ AccessKeyID: "complete_creds_akid", SecretAccessKey: "complete_creds_secret", - Source: fmt.Sprintf("SharedConfigCredentials: %s", testConfigFilename), + Source: fmt.Sprintf("SharedConfigCredentials: %s", filename), }, }, }, - 5: { - Profile: "complete_creds_with_token", + "profile with complete creds and token": { + Profile: "profile complete_creds_with_token", Expected: SharedConfig{ Credentials: aws.Credentials{ AccessKeyID: "complete_creds_with_token_akid", SecretAccessKey: "complete_creds_with_token_secret", SessionToken: "complete_creds_with_token_token", - Source: fmt.Sprintf("SharedConfigCredentials: %s", testConfigFilename), + Source: fmt.Sprintf("SharedConfigCredentials: %s", filename), }, }, }, - 6: { - Profile: "full_profile", + "complete profile": { + Profile: "profile full_profile", Expected: SharedConfig{ Credentials: aws.Credentials{ AccessKeyID: "full_profile_akid", SecretAccessKey: "full_profile_secret", - Source: fmt.Sprintf("SharedConfigCredentials: %s", testConfigFilename), + Source: fmt.Sprintf("SharedConfigCredentials: %s", filename), }, Region: "full_profile_region", }, }, - 7: { - Profile: "partial_assume_role", + "profile with partial assume role": { + Profile: "profile partial_assume_role", Expected: SharedConfig{ RoleARN: "partial_assume_role_role_arn", }, }, - 8: { - Profile: "assume_role", + "profile using assume role": { + Profile: "profile assume_role", Expected: SharedConfig{ RoleARN: "assume_role_role_arn", SourceProfileName: "complete_creds", }, }, - 9: { - Profile: "assume_role_w_mfa", + "profile with assume role and MFA": { + Profile: "profile assume_role_w_mfa", Expected: SharedConfig{ RoleARN: "assume_role_role_arn", SourceProfileName: "complete_creds", MFASerial: "0123456789", }, }, - 10: { + "does not exist": { Profile: "does_not_exist", Err: SharedConfigProfileNotExistError{ - Filename: filepath.Join("testdata", "shared_config"), + Filename: []string{filename}, Profile: "does_not_exist", Err: nil, }, }, - { - Profile: "with_mixed_case_keys", + "profile with mixed casing": { + Profile: "profile with_mixed_case_keys", Expected: SharedConfig{ Credentials: aws.Credentials{ AccessKeyID: "accessKey", SecretAccessKey: "secret", - Source: fmt.Sprintf("SharedConfigCredentials: %s", testConfigFilename), + Source: fmt.Sprintf("SharedConfigCredentials: %s", filename), }, }, }, } - for i, c := range cases { - t.Run(strconv.Itoa(i), func(t *testing.T) { + for name, c := range cases { + t.Run(name, func(t *testing.T) { var cfg SharedConfig - err := cfg.setFromIniFile(c.Profile, iniFile) + + section, ok := sections.GetSection(c.Profile) + if !ok { + if c.Err == nil { + t.Fatalf("expected section to be present, was not") + } else { + if e, a := c.Err.Error(), "failed to get shared config profile"; !strings.Contains(e, a) { + t.Fatalf("expect %q to be in %q", a, e) + } + return + } + } + + err := cfg.setFromIniSection(c.Profile, section) if c.Err != nil { if e, a := c.Err.Error(), err.Error(); !strings.Contains(a, e) { t.Errorf("expect %q to be in %q", e, a) @@ -369,43 +376,7 @@ func TestLoadSharedConfigFromFile(t *testing.T) { } } -func TestLoadSharedConfigIniFiles(t *testing.T) { - cases := []struct { - Filenames []string - Expected []sharedConfigFile - }{ - { - Filenames: []string{"not_exist", testConfigFilename}, - Expected: []sharedConfigFile{ - {Filename: testConfigFilename}, - }, - }, - { - Filenames: []string{testConfigFilename, testConfigOtherFilename}, - Expected: []sharedConfigFile{ - {Filename: testConfigFilename}, - {Filename: testConfigOtherFilename}, - }, - }, - } - - for i, c := range cases { - t.Run(strconv.Itoa(i), func(t *testing.T) { - files, err := loadSharedConfigIniFiles(c.Filenames) - if err != nil { - t.Fatalf("expect no error, got %v", err) - } - if e, a := len(c.Expected), len(files); e != a { - t.Errorf("expect %v, got %v", e, a) - } - if e, a := c.Expected, files; !cmpFiles(e, a) { - t.Errorf("expect %v, got %v", e, a) - } - }) - } -} - -func cmpFiles(expects, actuals []sharedConfigFile) bool { +func cmpFiles(expects, actuals []parsedINIFile) bool { for i, expect := range expects { if expect.Filename != actuals[i].Filename { return false @@ -416,10 +387,12 @@ func cmpFiles(expects, actuals []sharedConfigFile) bool { func TestLoadSharedConfig(t *testing.T) { origProf := defaultSharedConfigProfile - origFiles := DefaultSharedConfigFiles + origConfigFiles := DefaultSharedConfigFiles + origCredentialFiles := DefaultSharedCredentialsFiles defer func() { defaultSharedConfigProfile = origProf - DefaultSharedConfigFiles = origFiles + DefaultSharedConfigFiles = origConfigFiles + DefaultSharedCredentialsFiles = origCredentialFiles }() cases := []struct { @@ -458,7 +431,7 @@ func TestLoadSharedConfig(t *testing.T) { filepath.Join("file_not_exist"), }, LoadFn: loadSharedConfig, - Err: "failed to open shared config file, file_not_exist", + Err: "failed to get shared config profile", }, { LoadOptionFn: WithSharedConfigProfile("profile_not_exist"), @@ -466,7 +439,7 @@ func TestLoadSharedConfig(t *testing.T) { filepath.Join("testdata", "shared_config"), }, LoadFn: loadSharedConfig, - Err: "failed to get shared config profile, profile_not_exist", + Err: "failed to get shared config profile", }, { LoadOptionFn: WithSharedConfigProfile("default"), @@ -496,7 +469,8 @@ func TestLoadSharedConfig(t *testing.T) { for i, c := range cases { t.Run(strconv.Itoa(i), func(t *testing.T) { defaultSharedConfigProfile = origProf - DefaultSharedConfigFiles = origFiles + DefaultSharedConfigFiles = origConfigFiles + DefaultSharedCredentialsFiles = origCredentialFiles if len(c.Profile) > 0 { defaultSharedConfigProfile = c.Profile @@ -505,6 +479,8 @@ func TestLoadSharedConfig(t *testing.T) { DefaultSharedConfigFiles = c.Files } + DefaultSharedCredentialsFiles = []string{} + var options LoadOptions c.LoadOptionFn(&options) @@ -527,3 +503,248 @@ func TestLoadSharedConfig(t *testing.T) { }) } } + +func TestSharedConfigLoading(t *testing.T) { + // initialize a logger + var loggerBuf bytes.Buffer + logger := logging.NewStandardLogger(&loggerBuf) + + cases := map[string]struct { + LoadOptionFns []func(*LoadOptions) error + LoadFn func(context.Context, configs) (Config, error) + Expect SharedConfig + ExpectLog string + Err string + }{ + "duplicate profiles in the configuration files": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("duplicate-profile"), + WithSharedConfigFiles([]string{"testdata/load_config"}), + WithSharedCredentialsFiles([]string{"testdata/empty_creds_config"}), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Expect: SharedConfig{ + Profile: "duplicate-profile", + Region: "us-west-2", + }, + ExpectLog: "For profile: profile duplicate-profile, overriding region value, with a region value found in a " + + "duplicate profile defined later in the same file testdata/load_config", + }, + + "profile prefix not used in the configuration files": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("no-such-profile"), + WithSharedConfigFiles([]string{"testdata/load_config"}), + WithSharedCredentialsFiles([]string{"testdata/empty_creds_config"}), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Expect: SharedConfig{}, + Err: "failed to get shared config profile", + }, + + "profile prefix overrides default": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigFiles([]string{"testdata/load_config"}), + WithSharedCredentialsFiles([]string{"testdata/empty_creds_config"}), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Expect: SharedConfig{ + Profile: "default", + Region: "ap-north-1", + }, + ExpectLog: "overrided non-prefixed default profile", + }, + + "duplicate profiles in credentials file": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("duplicate-profile"), + WithSharedConfigFiles([]string{"testdata/empty_creds_config"}), + WithSharedCredentialsFiles([]string{"testdata/load_credentials"}), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Expect: SharedConfig{ + Profile: "duplicate-profile", + Region: "us-west-2", + }, + ExpectLog: "overriding region value, with a region value found in a duplicate profile defined later in the same file", + Err: "", + }, + + "profile prefix used in credentials files": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("unused-profile"), + WithSharedConfigFiles([]string{"testdata/empty_creds_config"}), + WithSharedCredentialsFiles([]string{"testdata/load_credentials"}), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + ExpectLog: "profile unused-profile is ignored. A profile with the \"profile \" prefix is invalid for the shared credentials file", + Err: "failed to get shared config profile, unused-profile", + }, + "partial credentials in configuration files": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("partial-creds-1"), + WithSharedConfigFiles([]string{"testdata/load_config"}), + WithSharedCredentialsFiles([]string{"testdata/empty_creds_config"}), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Expect: SharedConfig{ + Profile: "partial-creds-1", + }, + Err: "Partial Credentials", + }, + "parital credentials in the credentials files": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("partial-creds-1"), + WithSharedConfigFiles([]string{"testdata/empty_creds_config"}), + WithSharedCredentialsFiles([]string{"testdata/load_credentials"}), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Expect: SharedConfig{ + Profile: "partial-creds-1", + }, + Err: "Partial Credentials found for profile partial-creds-1", + }, + "credentials override configuration profile": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("complete"), + WithSharedConfigFiles([]string{"testdata/load_config"}), + WithSharedCredentialsFiles([]string{"testdata/load_credentials"}), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Expect: SharedConfig{ + Profile: "complete", + Credentials: aws.Credentials{ + AccessKeyID: "credsAccessKey", + SecretAccessKey: "credsSecretKey", + Source: "SharedConfigCredentials: testdata/load_credentials", + }, + Region: "us-west-2", + }, + }, + "credentials profile has complete credentials": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("complete"), + WithSharedConfigFiles([]string{"testdata/empty_creds_config"}), + WithSharedCredentialsFiles([]string{"testdata/load_credentials"}), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Expect: SharedConfig{ + Profile: "complete", + Credentials: aws.Credentials{ + AccessKeyID: "credsAccessKey", + SecretAccessKey: "credsSecretKey", + Source: "SharedConfigCredentials: testdata/load_credentials", + }, + }, + }, + "credentials split between multiple credentials files": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("partial-creds-1"), + WithSharedConfigFiles([]string{"testdata/empty_creds_config"}), + WithSharedCredentialsFiles([]string{ + "testdata/load_credentials", + "testdata/load_credentials_secondary", + }), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Expect: SharedConfig{ + Profile: "partial-creds-1", + }, + Err: "Partial Credentials", + }, + "credentials split between multiple configuration files": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("partial-creds-1"), + WithSharedCredentialsFiles([]string{"testdata/empty_creds_config"}), + WithSharedConfigFiles([]string{ + "testdata/load_config", + "testdata/load_config_secondary", + }), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Expect: SharedConfig{ + Profile: "partial-creds-1", + Region: "us-west-2", + }, + ExpectLog: "", + Err: "Partial Credentials", + }, + "credentials split between credentials and config files": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("partial-creds-1"), + WithSharedConfigFiles([]string{ + "testdata/load_config", + }), + WithSharedCredentialsFiles([]string{ + "testdata/load_credentials", + }), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Expect: SharedConfig{ + Profile: "partial-creds-1", + }, + ExpectLog: "", + Err: "Partial Credentials", + }, + } + + for name, c := range cases { + t.Run(name, func(t *testing.T) { + defer loggerBuf.Reset() + + var options LoadOptions + + for _, fn := range c.LoadOptionFns { + fn(&options) + } + + cfg, err := c.LoadFn(context.Background(), configs{options}) + if len(c.Err) > 0 { + if err == nil { + t.Fatalf("expected error %v, got none", c.Err) + } + if e, a := c.Err, err.Error(); !strings.Contains(a, e) { + t.Fatalf("expect %q to be in %q", e, a) + } + return + } else if err != nil { + t.Fatalf("expect no error, got %v", err) + } + + if e, a := c.Expect, cfg; !reflect.DeepEqual(e, a) { + t.Errorf("expect %v got %v", e, a) + } + + if e, a := c.ExpectLog, loggerBuf.String(); !strings.Contains(a, e) { + t.Errorf("expect %v logged in %v", e, a) + } + if loggerBuf.Len() == 0 && len(c.ExpectLog) != 0 { + t.Errorf("expected log, got none") + } + }) + } +} diff --git a/config/testdata/credential_source_config b/config/testdata/config_source_shared similarity index 65% rename from config/testdata/credential_source_config rename to config/testdata/config_source_shared index f3fcfe8ae84..e94eb688a7a 100644 --- a/config/testdata/credential_source_config +++ b/config/testdata/config_source_shared @@ -1,33 +1,35 @@ -[env_var_credential_source] +[profile env_var_credential_source] role_arn = assume_role_w_creds_role_arn_env credential_source = Environment -[invalid_source_and_credential_source] +[profile invalid_source_and_credential_source] role_arn = assume_role_w_creds_role_arn_bad credential_source = Environment source_profile = env_var_credential_source -[ec2metadata] +[profile ec2metadata] role_arn = assume_role_w_creds_role_arn_ec2 credential_source = Ec2InstanceMetadata -[ecscontainer] +[profile ecscontainer] role_arn = assume_role_w_creds_role_arn_ecs credential_source = EcsContainer -[chained_assume_role] +[profile chained_assume_role] role_arn = assume_role_w_creds_role_arn_chain source_profile = ec2metadata -[cred_proc_no_arn_set] +[profile cred_proc_no_arn_set] credential_process = cat ./testdata/test_json.json -[cred_proc_arn_set] +[profile cred_proc_arn_set] role_arn = assume_role_w_creds_proc_role_arn credential_process = cat ./testdata/test_json.json -[chained_cred_proc] +[profile chained_cred_proc] role_arn = assume_role_w_creds_proc_source_prof source_profile = cred_proc_no_arn_set - +[profile credentials_overide] +role_arn = assume_role_w_creds_role_arn_ec2 +credential_source = Ec2InstanceMetadata diff --git a/config/testdata/credential_source_config_for_windows b/config/testdata/config_source_shared_for_windows similarity index 61% rename from config/testdata/credential_source_config_for_windows rename to config/testdata/config_source_shared_for_windows index 34073afb516..dc5d435dc99 100644 --- a/config/testdata/credential_source_config_for_windows +++ b/config/testdata/config_source_shared_for_windows @@ -1,10 +1,10 @@ -[cred_proc_no_arn_set] +[profile cred_proc_no_arn_set] credential_process = type .\testdata\test_json.json -[cred_proc_arn_set] +[profile cred_proc_arn_set] role_arn = assume_role_w_creds_proc_role_arn credential_process = type .\testdata\test_json.json -[chained_cred_proc] +[profile chained_cred_proc] role_arn = assume_role_w_creds_proc_source_prof -source_profile = cred_proc_no_arn_set \ No newline at end of file +source_profile = cred_proc_no_arn_set diff --git a/config/testdata/credentials_source_shared b/config/testdata/credentials_source_shared new file mode 100644 index 00000000000..17c09b04981 --- /dev/null +++ b/config/testdata/credentials_source_shared @@ -0,0 +1,7 @@ +[credentials_overide] +role_arn = assume_role_w_creds_role_arn_ec2 +credential_source = EcsContainer + +[only_credentials_source] +role_arn = assume_role_w_creds_role_arn_ecs +credential_source = EcsContainer diff --git a/config/testdata/credentials_source_shared_for_windows b/config/testdata/credentials_source_shared_for_windows new file mode 100644 index 00000000000..17c09b04981 --- /dev/null +++ b/config/testdata/credentials_source_shared_for_windows @@ -0,0 +1,7 @@ +[credentials_overide] +role_arn = assume_role_w_creds_role_arn_ec2 +credential_source = EcsContainer + +[only_credentials_source] +role_arn = assume_role_w_creds_role_arn_ecs +credential_source = EcsContainer diff --git a/config/testdata/empty_creds_config b/config/testdata/empty_creds_config new file mode 100644 index 00000000000..ab109a17201 --- /dev/null +++ b/config/testdata/empty_creds_config @@ -0,0 +1 @@ +[default] diff --git a/config/testdata/load_config b/config/testdata/load_config new file mode 100644 index 00000000000..bf1d305b7c1 --- /dev/null +++ b/config/testdata/load_config @@ -0,0 +1,19 @@ +[default] +region = ap-south-1 + +[profile default] +region = ap-north-1 + +[profile duplicate-profile] +region = us-east-1 + +[profile duplicate-profile] +region = us-west-2 + +[profile partial-creds-1] +aws_access_key_id = notForAccess + +[profile complete] +aws_access_key_id = configAccessKey +aws_secret_access_key = configSecretKey +region = us-west-2 diff --git a/config/testdata/load_config_secondary b/config/testdata/load_config_secondary new file mode 100644 index 00000000000..978e5b8f44a --- /dev/null +++ b/config/testdata/load_config_secondary @@ -0,0 +1,3 @@ +[profile partial-creds-1] +aws_secret_access_key = configSecretKey +region = us-west-2 diff --git a/config/testdata/load_credentials b/config/testdata/load_credentials new file mode 100644 index 00000000000..b742653f755 --- /dev/null +++ b/config/testdata/load_credentials @@ -0,0 +1,16 @@ +[duplicate-profile] +region = us-east-1 + +[duplicate-profile] +region = us-west-2 + +[profile unused-profile] +region = ap-north-1 + +[partial-creds-1] +aws_secret_access_key = notForSecretAccess + +[complete] +aws_access_key_id = credsAccessKey +aws_secret_access_key = credsSecretKey + diff --git a/config/testdata/load_credentials_secondary b/config/testdata/load_credentials_secondary new file mode 100644 index 00000000000..90505c7a957 --- /dev/null +++ b/config/testdata/load_credentials_secondary @@ -0,0 +1,3 @@ +[partial-creds-1] +aws_access_key_id = credsAccessKey + diff --git a/config/testdata/shared_config b/config/testdata/shared_config index cb9d2d03ae4..3896cfeaf99 100644 --- a/config/testdata/shared_config +++ b/config/testdata/shared_config @@ -13,51 +13,51 @@ aws_secret_access_key = SECRET [profile alt_profile_name] region = alt_profile_name_region -[short_profile_name_first] +[profile short_profile_name_first] region = short_profile_name_first_short [profile short_profile_name_first] region = short_profile_name_first_alt -[partial_creds] +[profile partial_creds] aws_access_key_id = partial_creds_akid -[complete_creds] +[profile complete_creds] aws_access_key_id = complete_creds_akid aws_secret_access_key = complete_creds_secret -[complete_creds_with_token] +[profile complete_creds_with_token] aws_access_key_id = complete_creds_with_token_akid aws_secret_access_key = complete_creds_with_token_secret aws_session_token = complete_creds_with_token_token -[full_profile] +[profile full_profile] aws_access_key_id = full_profile_akid aws_secret_access_key = full_profile_secret region = full_profile_region -[config_file_load_order] +[profile config_file_load_order] region = shared_config_region aws_access_key_id = shared_config_akid aws_secret_access_key = shared_config_secret -[partial_assume_role] +[profile partial_assume_role] role_arn = partial_assume_role_role_arn -[assume_role] +[profile assume_role] role_arn = assume_role_role_arn source_profile = complete_creds -[assume_role_w_mfa] +[profile assume_role_w_mfa] role_arn = assume_role_role_arn source_profile = complete_creds mfa_serial = 0123456789 -[assume_role_invalid_source_profile] +[profile assume_role_invalid_source_profile] role_arn = assume_role_invalid_source_profile_role_arn source_profile = profile_not_exists -[assume_role_w_creds] +[profile assume_role_w_creds] role_arn = assume_role_w_creds_role_arn source_profile = assume_role_w_creds external_id = 1234 @@ -65,7 +65,7 @@ role_session_name = assume_role_w_creds_session_name aws_access_key_id = assume_role_w_creds_akid aws_secret_access_key = assume_role_w_creds_secret -[assume_role_w_creds_ext_dur] +[profile assume_role_w_creds_ext_dur] role_arn = assume_role_w_creds_role_arn duration_seconds=1800 source_profile = assume_role_w_creds_ext_dur @@ -74,33 +74,33 @@ role_session_name = assume_role_w_creds_session_name aws_access_key_id = assume_role_w_creds_akid aws_secret_access_key = assume_role_w_creds_secret -[assume_role_wo_creds] +[profile assume_role_wo_creds] role_arn = assume_role_wo_creds_role_arn source_profile = assume_role_wo_creds -[valid_arn_region] +[profile valid_arn_region] s3_use_arn_region=true -[endpoint_discovery] +[profile endpoint_discovery] endpoint_discovery_enabled=true -[with_mixed_case_keys] +[profile with_mixed_case_keys] aWs_AcCeSs_kEy_ID = accessKey aWs_SecrEt_AccEsS_kEY = secret -[assume_role_with_credential_source] +[profile assume_role_with_credential_source] role_arn = assume_role_with_credential_source_role_arn credential_source = Ec2InstanceMetadata -[multiple_assume_role] +[profile multiple_assume_role] role_arn = multiple_assume_role_role_arn source_profile = assume_role -[multiple_assume_role_with_credential_source] +[profile multiple_assume_role_with_credential_source] role_arn = multiple_assume_role_with_credential_source_role_arn source_profile = assume_role_with_credential_source -[multiple_assume_role_with_credential_source2] +[profile multiple_assume_role_with_credential_source2] role_arn = multiple_assume_role_with_credential_source2_role_arn source_profile = multiple_assume_role_with_credential_source diff --git a/config/testdata/shared_config_other b/config/testdata/shared_config_other index 615831b1afa..7f518e91b02 100644 --- a/config/testdata/shared_config_other +++ b/config/testdata/shared_config_other @@ -1,17 +1,17 @@ [default] region = default_region -[partial_creds] +[profile partial_creds] aws_access_key_id = AKID [profile alt_profile_name] region = alt_profile_name_region -[creds_from_credentials] +[profile creds_from_credentials] aws_access_key_id = creds_from_config_akid aws_secret_access_key = creds_from_config_secret -[config_file_load_order] +[profile config_file_load_order] region = shared_config_other_region aws_access_key_id = shared_config_other_akid aws_secret_access_key = shared_config_other_secret diff --git a/internal/ini/bench_test.go b/internal/ini/bench_test.go index e4ebec22077..d97cc37182e 100644 --- a/internal/ini/bench_test.go +++ b/internal/ini/bench_test.go @@ -21,7 +21,7 @@ region = us-west-2 func BenchmarkINIParser(b *testing.B) { for i := 0; i < b.N; i++ { - ParseBytes([]byte(section)) + ParseBytes([]byte(section), "ini/bench_test") } } diff --git a/internal/ini/ini.go b/internal/ini/ini.go index 945c7db3b80..acb06ba982b 100644 --- a/internal/ini/ini.go +++ b/internal/ini/ini.go @@ -14,18 +14,18 @@ func OpenFile(path string) (Sections, error) { } defer f.Close() - return Parse(f) + return Parse(f, path) } // Parse will parse the given file using the shared config // visitor. -func Parse(f io.Reader) (Sections, error) { +func Parse(f io.Reader, path string) (Sections, error) { tree, err := ParseAST(f) if err != nil { return Sections{}, err } - v := NewDefaultVisitor() + v := NewDefaultVisitor(path) if err = Walk(tree, v); err != nil { return Sections{}, err } @@ -34,13 +34,13 @@ func Parse(f io.Reader) (Sections, error) { } // ParseBytes will parse the given bytes and return the parsed sections. -func ParseBytes(b []byte) (Sections, error) { +func ParseBytes(b []byte, path string) (Sections, error) { tree, err := ParseASTBytes(b) if err != nil { return Sections{}, err } - v := NewDefaultVisitor() + v := NewDefaultVisitor(path) if err = Walk(tree, v); err != nil { return Sections{}, err } diff --git a/internal/ini/literal_tokens.go b/internal/ini/literal_tokens.go index 24df543d38c..91b379986f5 100644 --- a/internal/ini/literal_tokens.go +++ b/internal/ini/literal_tokens.go @@ -193,6 +193,16 @@ func newValue(t ValueType, base int, raw []rune) (Value, error) { return v, err } +// NewStringValue returns a Value type generated using a string input. +func NewStringValue(str string) (Value, error) { + return newValue(StringType, 10, []rune(str)) +} + +// NewIntValue returns a Value type generated using an int64 input. +func NewIntValue(i int64) (Value, error) { + return newValue(IntegerType, 10, []rune{rune(i)}) +} + // Append will append values and change the type to a string // type. func (v *Value) Append(tok Token) { diff --git a/internal/ini/testdata/valid/commented_profile b/internal/ini/testdata/valid/commented_profile index 85e7792170a..f40a405042e 100644 --- a/internal/ini/testdata/valid/commented_profile +++ b/internal/ini/testdata/valid/commented_profile @@ -2,3 +2,4 @@ [ default ] region = "foo-region" # another comment output = json # comment again +bar = 123 ; comment with semi-colon diff --git a/internal/ini/testdata/valid/commented_profile_expected b/internal/ini/testdata/valid/commented_profile_expected index dbf5571cf6f..de7b26c6d7e 100644 --- a/internal/ini/testdata/valid/commented_profile_expected +++ b/internal/ini/testdata/valid/commented_profile_expected @@ -1,6 +1,7 @@ { "default": { "region": "foo-region", - "output": "json" + "output": "json", + "bar": "123" } } diff --git a/internal/ini/testdata/valid/nested_fields b/internal/ini/testdata/valid/nested_fields index b4b40f3139f..b8c7380f77c 100644 --- a/internal/ini/testdata/valid/nested_fields +++ b/internal/ini/testdata/valid/nested_fields @@ -9,4 +9,4 @@ aws_session_token = valid [baz] aws_access_key_id = valid aws_secret_access_key = valid -aws_session_token = valid \ No newline at end of file +aws_session_token = valid diff --git a/internal/ini/visitor.go b/internal/ini/visitor.go index 7ad98653638..4094f442f3c 100644 --- a/internal/ini/visitor.go +++ b/internal/ini/visitor.go @@ -19,16 +19,25 @@ type Visitor interface { // the Sections field which can be used to retrieve profile // configuration. type DefaultVisitor struct { - scope string + + // scope is the profile which is being visited + scope string + + // path is the file path which the visitor is visiting + path string + + // Sections defines list of the profile section Sections Sections } -// NewDefaultVisitor return a DefaultVisitor -func NewDefaultVisitor() *DefaultVisitor { +// NewDefaultVisitor returns a DefaultVisitor. It takes in a filepath +// which points to the file it is visiting. +func NewDefaultVisitor(filepath string) *DefaultVisitor { return &DefaultVisitor{ Sections: Sections{ container: map[string]Section{}, }, + path: filepath, } } @@ -38,6 +47,9 @@ func (v *DefaultVisitor) VisitExpr(expr AST) error { if t.values == nil { t.values = values{} } + if t.SourceFile == nil { + t.SourceFile = make(map[string]string, 0) + } switch expr.Kind { case ASTKindExprStatement: @@ -56,12 +68,26 @@ func (v *DefaultVisitor) VisitExpr(expr AST) error { } key := EqualExprKey(opExpr) - v, err := newValue(rhs.Root.ValueType, rhs.Root.base, rhs.Root.Raw()) + val, err := newValue(rhs.Root.ValueType, rhs.Root.base, rhs.Root.Raw()) if err != nil { return err } - t.values[strings.ToLower(key)] = v + // lower case key to standardize + k := strings.ToLower(key) + + // identify if the section already had this key, append log on section + if t.Has(k) { + t.Logs = append(t.Logs, + fmt.Sprintf("For profile: %v, overriding %v value, "+ + "with a %v value found in a duplicate profile defined later in the same file %v. \n", + t.Name, k, k, v.path)) + } + + // assign the value + t.values[k] = val + // update the source file path for region + t.SourceFile[k] = v.path default: return NewParseError(fmt.Sprintf("unsupported expression %v", expr)) } @@ -83,7 +109,25 @@ func (v *DefaultVisitor) VisitStatement(stmt AST) error { } name := string(child.Root.Raw()) - v.Sections.container[name] = Section{} + + // trim start and end space + name = strings.TrimSpace(name) + + // if has prefix "profile " + [ws+] + "profile-name", + // we standardize by removing the [ws+] between prefix and profile-name. + if strings.HasPrefix(name, "profile ") { + names := strings.SplitN(name, " ", 2) + name = names[0] + " " + strings.TrimLeft(names[1], " ") + } + + // lower casing name to handle duplicates correctly. + name = strings.ToLower(name) + // attach profile name on section + if !v.Sections.HasSection(name) { + v.Sections.container[name] = Section{ + Name: name, + } + } v.scope = name default: return NewParseError(fmt.Sprintf("unsupported statement: %s", stmt.Kind)) @@ -98,6 +142,13 @@ type Sections struct { container map[string]Section } +// NewSections returns empty ini Sections +func NewSections() Sections { + return Sections{ + container: make(map[string]Section, 0), + } +} + // GetSection will return section p. If section p does not exist, // false will be returned in the second parameter. func (t Sections) GetSection(p string) (Section, bool) { @@ -105,6 +156,24 @@ func (t Sections) GetSection(p string) (Section, bool) { return v, ok } +// HasSection denotes if Sections consist of a section with +// provided name. +func (t Sections) HasSection(p string) bool { + _, ok := t.container[p] + return ok +} + +// SetSection sets a section value for provided section name. +func (t Sections) SetSection(p string, v Section) Sections { + t.container[p] = v + return t +} + +// DeleteSection deletes a section entry/value for provided section name./ +func (t Sections) DeleteSection(p string) { + delete(t.container, p) +} + // values represents a map of union values. type values map[string]Value @@ -125,8 +194,33 @@ func (t Sections) List() []string { // Section contains a name and values. This represent // a sectioned entry in a configuration file. type Section struct { - Name string + // Name is the Section profile name + Name string + + // values are the values within parsed profile values values + + // Errors is the list of errors + Errors []error + + // Logs is the list of logs + Logs []string + + // SourceFile is the INI Source file from where this section + // was retrieved. They key is the property, value is the + // source file the property was retrieved from. + SourceFile map[string]string +} + +// UpdateSourceFile updates source file for a property to provided filepath. +func (t Section) UpdateSourceFile(property string, filepath string) { + t.SourceFile[property] = filepath +} + +// UpdateValue updates value for a provided key with provided value +func (t Section) UpdateValue(k string, v Value) error { + t.values[k] = v + return nil } // Has will return whether or not an entry exists in a given section diff --git a/internal/ini/walker_test.go b/internal/ini/walker_test.go index fe2f6d6fc8a..10b90637949 100644 --- a/internal/ini/walker_test.go +++ b/internal/ini/walker_test.go @@ -31,7 +31,7 @@ func TestValidDataFiles(t *testing.T) { t.Errorf("%s: unexpected parse error, %v", path, err) } - v := NewDefaultVisitor() + v := NewDefaultVisitor(path) err = Walk(tree, v) if err != nil { t.Errorf("%s: unexpected walk error, %v", path, err) @@ -52,6 +52,9 @@ func TestValidDataFiles(t *testing.T) { } for profile, tableIface := range e { + // standardize by lower casing + profile = strings.ToLower(profile) + p, ok := v.Sections.GetSection(profile) if !ok { t.Fatal("could not find profile " + profile) @@ -135,7 +138,7 @@ func TestInvalidDataFiles(t *testing.T) { return } - v := NewDefaultVisitor() + v := NewDefaultVisitor(c.path) err = Walk(tree, v) if err == nil && c.expectedWalkError { t.Errorf("%d: expected error, but received none", i+1) From 7f5f9fd42b010af3cfa56ef4c9282596a9f123ba Mon Sep 17 00:00:00 2001 From: skotambkar Date: Mon, 21 Dec 2020 21:32:19 -0800 Subject: [PATCH 2/5] adds more tests and feedback --- config/env_config.go | 4 +- config/load_options.go | 16 +- config/resolve_processcreds_test.go | 3 - config/shared_config.go | 123 +++++------ config/shared_config_test.go | 310 +++++++++++++++++++++++----- config/testdata/empty_creds_config | 1 - config/testdata/load_config | 25 +++ config/testdata/load_credentials | 19 ++ internal/ini/bench_test.go | 2 +- internal/ini/ini.go | 4 +- 10 files changed, 379 insertions(+), 128 deletions(-) diff --git a/config/env_config.go b/config/env_config.go index cb589cf0ff9..85040b45d30 100644 --- a/config/env_config.go +++ b/config/env_config.go @@ -255,7 +255,7 @@ func (c EnvConfig) getSharedConfigProfile(ctx context.Context) (string, bool, er // Will return the filenames in the order of: // * Shared Config func (c EnvConfig) getSharedConfigFiles(context.Context) ([]string, bool, error) { - files := make([]string, 0, 2) + var files []string if v := c.SharedConfigFile; len(v) > 0 { files = append(files, v) } @@ -271,7 +271,7 @@ func (c EnvConfig) getSharedConfigFiles(context.Context) ([]string, bool, error) // Will return the filenames in the order of: // * Shared Credentials func (c EnvConfig) getSharedCredentialsFiles(context.Context) ([]string, bool, error) { - files := make([]string, 0, 2) + var files []string if v := c.SharedCredentialsFile; len(v) > 0 { files = append(files, v) } diff --git a/config/load_options.go b/config/load_options.go index 06f6fa3e746..c296033271c 100644 --- a/config/load_options.go +++ b/config/load_options.go @@ -59,17 +59,25 @@ type LoadOptions struct { // 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'. - // If duplicate profiles are provided with a same, or across multiple shared config files, the next parsed - // profile will override only properties that conflict with the previously defined profile. + // 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. - // If duplicate profiles are provided within a shared credentials and shared config files, the properties - // defined in shared credentials file take precedence. + // 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 diff --git a/config/resolve_processcreds_test.go b/config/resolve_processcreds_test.go index aca98038571..d6056bc1e4d 100644 --- a/config/resolve_processcreds_test.go +++ b/config/resolve_processcreds_test.go @@ -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() diff --git a/config/shared_config.go b/config/shared_config.go index d48affacdda..fabebfb2cff 100644 --- a/config/shared_config.go +++ b/config/shared_config.go @@ -16,6 +16,9 @@ import ( ) const ( + // Prefix to use for filtering profiles + profilePrefix = `profile ` + // Static Credentials group accessKeyIDKey = `aws_access_key_id` // group required secretAccessKey = `aws_secret_access_key` // group required @@ -187,7 +190,7 @@ func loadSharedConfigIgnoreNotExist(ctx context.Context, configs configs) (Confi // * sharedConfigFilesProvider func loadSharedConfig(ctx context.Context, configs configs) (Config, error) { var profile string - var configurationFiles []string + var configFiles []string var credentialsFiles []string var ok bool var err error @@ -200,21 +203,15 @@ func loadSharedConfig(ctx context.Context, configs configs) (Config, error) { profile = defaultSharedConfigProfile } - configurationFiles, ok, err = getSharedConfigFiles(ctx, configs) + configFiles, ok, err = getSharedConfigFiles(ctx, configs) if err != nil { return nil, err } - if !ok { - configurationFiles = DefaultSharedConfigFiles - } credentialsFiles, ok, err = getSharedCredentialsFiles(ctx, configs) if err != nil { return nil, err } - if !ok { - credentialsFiles = DefaultSharedCredentialsFiles - } // setup logger if log configuration warning is seti var logger logging.Logger @@ -232,58 +229,75 @@ func loadSharedConfig(ctx context.Context, configs configs) (Config, error) { } } - return NewSharedConfig(ctx, profile, sharedConfigsLoadInfo{ - CredentialsFiles: credentialsFiles, - ConfigurationFiles: configurationFiles, - Logger: logger, - }) + return LoadSharedConfigProfile(ctx, profile, + func(o *LoadSharedConfigOptions) { + o.Logger = logger + o.ConfigFiles = configFiles + o.CredentialsFiles = credentialsFiles + }, + ) } -// sharedConfigsLoadInfo struct contains Values that can be used to load the config. -type sharedConfigsLoadInfo struct { +// LoadSharedConfigOptions struct contains optional values that can be used to load the config. +type LoadSharedConfigOptions struct { // CredentialsFiles are the shared credentials files CredentialsFiles []string - // ConfigurationFiles are the shared config files - ConfigurationFiles []string + // ConfigFiles are the shared config files + ConfigFiles []string // Logger is the logger used to log shared config behavior Logger logging.Logger } -// NewSharedConfig retrieves the configuration from the list of files +// LoadSharedConfigProfile retrieves the configuration from the list of files // using the profile provided. The order the files are listed will determine // precedence. Values in subsequent files will overwrite values defined in // earlier files. // // For example, given two files A and B. Both define credentials. If the order // of the files are A then B, B's credential values will be used instead of A's. -func NewSharedConfig(ctx context.Context, profile string, options sharedConfigsLoadInfo) (SharedConfig, error) { - if len(options.ConfigurationFiles) == 0 && len(options.CredentialsFiles) == 0 { - return SharedConfig{}, fmt.Errorf("no shared config or shared credentials options provided") +// +// If no config files, credentials files are provided, the LoadSharedConfigProfile +// will default to location `.aws/config` for config files and `.aws/credentials` +// for credentials files respectively as per +// https://docs.aws.amazon.com/credref/latest/refdocs/file-location.html#file-location +// +func LoadSharedConfigProfile(ctx context.Context, profile string, optFns ...func(*LoadSharedConfigOptions)) (SharedConfig, error) { + var option LoadSharedConfigOptions + for _, fn := range optFns { + fn(&option) + } + + if len(option.ConfigFiles) == 0 { + option.ConfigFiles = DefaultSharedConfigFiles + } + + if len(option.CredentialsFiles) == 0 { + option.CredentialsFiles = DefaultSharedCredentialsFiles } // load shared configuration sections from shared configuration INI options - configSections, err := loadIniFiles(options.ConfigurationFiles) + configSections, err := loadIniFiles(option.ConfigFiles) if err != nil { return SharedConfig{}, err } // check for profile prefix and drop duplicates or invalid profiles - err = processConfigSections(ctx, configSections, options.Logger) + err = processConfigSections(ctx, configSections, option.Logger) if err != nil { return SharedConfig{}, err } // load shared credentials sections from shared credentials INI options - credentialsSections, err := loadIniFiles(options.CredentialsFiles) + credentialsSections, err := loadIniFiles(option.CredentialsFiles) if err != nil { return SharedConfig{}, err } // check for profile prefix and drop duplicates or invalid profiles - err = processCredentialsSections(ctx, credentialsSections, options.Logger) + err = processCredentialsSections(ctx, credentialsSections, option.Logger) if err != nil { return SharedConfig{}, err } @@ -298,7 +312,7 @@ func NewSharedConfig(ctx context.Context, profile string, options sharedConfigsL cfg := SharedConfig{} profiles := map[string]struct{}{} - if err = cfg.setFromIniSections(profiles, profile, configSections, options.Logger); err != nil { + if err = cfg.setFromIniSections(profiles, profile, configSections, option.Logger); err != nil { return SharedConfig{}, err } @@ -306,18 +320,16 @@ func NewSharedConfig(ctx context.Context, profile string, options sharedConfigsL } func processConfigSections(ctx context.Context, sections ini.Sections, logger logging.Logger) error { - - prefix := "profile " for _, section := range sections.List() { // drop profiles without prefix for config files - if !strings.HasPrefix(section, prefix) && !strings.EqualFold(section, "default") { + if !strings.HasPrefix(section, profilePrefix) && !strings.EqualFold(section, "default") { // drop this section, as invalid profile name sections.DeleteSection(section) if logger != nil { logger.Logf(logging.Debug, - "profile %v is ignored. A non-default profile without the \"profile \" prefix is invalid "+ - "for the shared configuration file.\n", + "A profile defined with name `%v` is ignored. For use within a shared configuration file, " + + "a non-default profile must have `profile ` prefixed to the profile name.\n", section, ) } @@ -327,7 +339,7 @@ func processConfigSections(ctx context.Context, sections ini.Sections, logger lo // rename sections to remove `profile ` prefixing to match with credentials file. // if default is already present, it will be dropped. for _, section := range sections.List() { - if strings.HasPrefix(section, prefix) { + if strings.HasPrefix(section, profilePrefix) { v, ok := sections.GetSection(section) if !ok { return fmt.Errorf("error processing profiles within the shared configuration files") @@ -337,11 +349,11 @@ func processConfigSections(ctx context.Context, sections ini.Sections, logger lo sections.DeleteSection(section) // set the value to non-prefixed name in sections. - section = strings.TrimPrefix(section, prefix) + section = strings.TrimPrefix(section, profilePrefix) if sections.HasSection(section) { oldSection, _ := sections.GetSection(section) v.Logs = append(v.Logs, - fmt.Sprintf("A default profile prefixed with `profile` found in %s, "+ + fmt.Sprintf("A default profile prefixed with `profile ` found in %s, "+ "overrided non-prefixed default profile from %s", v.SourceFile, oldSection.SourceFile)) } @@ -354,17 +366,15 @@ func processConfigSections(ctx context.Context, sections ini.Sections, logger lo } func processCredentialsSections(ctx context.Context, sections ini.Sections, logger logging.Logger) error { - - prefix := "profile" for _, section := range sections.List() { // drop profiles with prefix for credential files - if strings.HasPrefix(section, prefix) { + if strings.HasPrefix(section, profilePrefix) { // drop this section, as invalid profile name sections.DeleteSection(section) if logger != nil { logger.Logf(logging.Debug, - "The %v is ignored. A profile with the \"profile \" prefix is invalid "+ + "The profile defined with name `%v` is ignored. A profile with the `profile ` prefix is invalid "+ "for the shared credentials file.\n", section, ) @@ -374,11 +384,6 @@ func processCredentialsSections(ctx context.Context, sections ini.Sections, logg return nil } -type parsedINIFile struct { - Filename string - IniData ini.Sections -} - func loadIniFiles(filenames []string) (ini.Sections, error) { mergedSections := ini.NewSections() @@ -411,7 +416,7 @@ func mergeSections(dst, src ini.Sections) error { if (!srcSection.Has(accessKeyIDKey) && srcSection.Has(secretAccessKey)) || (srcSection.Has(accessKeyIDKey) && !srcSection.Has(secretAccessKey)) { srcSection.Errors = append(srcSection.Errors, - fmt.Errorf("Partial Credentials found for profile %v", sectionName)) + fmt.Errorf("partial credentials found for profile %v", sectionName)) } if !dst.HasSection(sectionName) { @@ -593,6 +598,18 @@ func mergeSections(dst, src ini.Sections) error { dstSection.UpdateSourceFile(roleSessionNameKey, srcSection.SourceFile[roleSessionNameKey]) } + // role duration seconds key update + if srcSection.Has(roleDurationSecondsKey) { + roleDurationSeconds := srcSection.Int(roleDurationSecondsKey) + v, err := ini.NewIntValue(roleDurationSeconds) + if err != nil { + return fmt.Errorf("error merging role duration seconds key, %w", err) + } + dstSection.UpdateValue(roleDurationSecondsKey, v) + + dstSection.UpdateSourceFile(roleDurationSecondsKey, srcSection.SourceFile[roleDurationSecondsKey]) + } + if srcSection.Has(regionKey) { key := srcSection.String(regionKey) val, err := ini.NewStringValue(key) @@ -688,18 +705,6 @@ func mergeSections(dst, src ini.Sections) error { dstSection.UpdateSourceFile(s3UseARNRegionKey, srcSection.SourceFile[s3UseARNRegionKey]) } - // role duration seconds key update - if srcSection.Has(roleDurationSecondsKey) { - roleDurationSeconds := srcSection.Int(roleDurationSecondsKey) - v, err := ini.NewIntValue(roleDurationSeconds) - if err != nil { - return fmt.Errorf("error merging role duration seconds key, %w", err) - } - dstSection.UpdateValue(roleDurationSecondsKey, v) - - dstSection.UpdateSourceFile(roleDurationSecondsKey, srcSection.SourceFile[roleDurationSecondsKey]) - } - // set srcSection on dst srcSection dst = dst.SetSection(sectionName, dstSection) } @@ -808,11 +813,7 @@ func (c *SharedConfig) setFromIniSection(profile string, section ini.Section) er sources = append(sources, v) } - return SharedConfigProfileNotExistError{ - Filename: sources, - Profile: profile, - Err: nil, - } + return fmt.Errorf("parsing error : could not find profile section name after processing files: %v", sources) } if len(section.Errors) != 0 { diff --git a/config/shared_config_test.go b/config/shared_config_test.go index b8f7fc74746..c4e09df0f32 100644 --- a/config/shared_config_test.go +++ b/config/shared_config_test.go @@ -221,19 +221,22 @@ func TestNewSharedConfig(t *testing.T) { for name, c := range cases { t.Run(name, func(t *testing.T) { - cfg, err := NewSharedConfig(context.TODO(), c.Profile, sharedConfigsLoadInfo{ - ConfigurationFiles: c.Filenames, + cfg, err := LoadSharedConfigProfile(context.TODO(), c.Profile, func(o *LoadSharedConfigOptions) { + o.ConfigFiles = c.Filenames + o.CredentialsFiles = []string{filepath.Join("testdata", "empty_creds_config")} }) - if c.Err != nil { + if c.Err != nil && err != nil { if e, a := c.Err.Error(), err.Error(); !strings.Contains(a, e) { t.Errorf("expect %q to be in %q", e, a) } return } - if err != nil { t.Fatalf("expect no error, got %v", err) } + if c.Err != nil { + t.Errorf("expect error: %v, got none", c.Err) + } if e, a := c.Expected, cfg; !reflect.DeepEqual(e, a) { t.Errorf(" expect %v, got %v", e, a) } @@ -376,15 +379,6 @@ func TestLoadSharedConfigFromSection(t *testing.T) { } } -func cmpFiles(expects, actuals []parsedINIFile) bool { - for i, expect := range expects { - if expect.Filename != actuals[i].Filename { - return false - } - } - return true -} - func TestLoadSharedConfig(t *testing.T) { origProf := defaultSharedConfigProfile origConfigFiles := DefaultSharedConfigFiles @@ -519,8 +513,8 @@ func TestSharedConfigLoading(t *testing.T) { "duplicate profiles in the configuration files": { LoadOptionFns: []func(*LoadOptions) error{ WithSharedConfigProfile("duplicate-profile"), - WithSharedConfigFiles([]string{"testdata/load_config"}), - WithSharedCredentialsFiles([]string{"testdata/empty_creds_config"}), + WithSharedConfigFiles([]string{filepath.Join("testdata", "load_config")}), + WithSharedCredentialsFiles([]string{filepath.Join("testdata", "empty_creds_config")}), WithLogConfigurationWarnings(true), WithLogger(logger), }, @@ -536,8 +530,8 @@ func TestSharedConfigLoading(t *testing.T) { "profile prefix not used in the configuration files": { LoadOptionFns: []func(*LoadOptions) error{ WithSharedConfigProfile("no-such-profile"), - WithSharedConfigFiles([]string{"testdata/load_config"}), - WithSharedCredentialsFiles([]string{"testdata/empty_creds_config"}), + WithSharedConfigFiles([]string{filepath.Join("testdata", "load_config")}), + WithSharedCredentialsFiles([]string{filepath.Join("testdata", "empty_creds_config")}), WithLogConfigurationWarnings(true), WithLogger(logger), }, @@ -548,8 +542,8 @@ func TestSharedConfigLoading(t *testing.T) { "profile prefix overrides default": { LoadOptionFns: []func(*LoadOptions) error{ - WithSharedConfigFiles([]string{"testdata/load_config"}), - WithSharedCredentialsFiles([]string{"testdata/empty_creds_config"}), + WithSharedConfigFiles([]string{filepath.Join("testdata", "load_config")}), + WithSharedCredentialsFiles([]string{filepath.Join("testdata", "empty_creds_config")}), WithLogConfigurationWarnings(true), WithLogger(logger), }, @@ -564,8 +558,8 @@ func TestSharedConfigLoading(t *testing.T) { "duplicate profiles in credentials file": { LoadOptionFns: []func(*LoadOptions) error{ WithSharedConfigProfile("duplicate-profile"), - WithSharedConfigFiles([]string{"testdata/empty_creds_config"}), - WithSharedCredentialsFiles([]string{"testdata/load_credentials"}), + WithSharedConfigFiles([]string{filepath.Join("testdata", "empty_creds_config")}), + WithSharedCredentialsFiles([]string{filepath.Join("testdata", "load_credentials")}), WithLogConfigurationWarnings(true), WithLogger(logger), }, @@ -581,20 +575,20 @@ func TestSharedConfigLoading(t *testing.T) { "profile prefix used in credentials files": { LoadOptionFns: []func(*LoadOptions) error{ WithSharedConfigProfile("unused-profile"), - WithSharedConfigFiles([]string{"testdata/empty_creds_config"}), - WithSharedCredentialsFiles([]string{"testdata/load_credentials"}), + WithSharedConfigFiles([]string{filepath.Join("testdata", "empty_creds_config")}), + WithSharedCredentialsFiles([]string{filepath.Join("testdata", "load_credentials")}), WithLogConfigurationWarnings(true), WithLogger(logger), }, LoadFn: loadSharedConfig, - ExpectLog: "profile unused-profile is ignored. A profile with the \"profile \" prefix is invalid for the shared credentials file", + ExpectLog: "profile defined with name `profile unused-profile` is ignored.", Err: "failed to get shared config profile, unused-profile", }, "partial credentials in configuration files": { LoadOptionFns: []func(*LoadOptions) error{ WithSharedConfigProfile("partial-creds-1"), - WithSharedConfigFiles([]string{"testdata/load_config"}), - WithSharedCredentialsFiles([]string{"testdata/empty_creds_config"}), + WithSharedConfigFiles([]string{filepath.Join("testdata", "load_config")}), + WithSharedCredentialsFiles([]string{filepath.Join("testdata", "empty_creds_config")}), WithLogConfigurationWarnings(true), WithLogger(logger), }, @@ -602,13 +596,13 @@ func TestSharedConfigLoading(t *testing.T) { Expect: SharedConfig{ Profile: "partial-creds-1", }, - Err: "Partial Credentials", + Err: "partial credentials", }, "parital credentials in the credentials files": { LoadOptionFns: []func(*LoadOptions) error{ WithSharedConfigProfile("partial-creds-1"), - WithSharedConfigFiles([]string{"testdata/empty_creds_config"}), - WithSharedCredentialsFiles([]string{"testdata/load_credentials"}), + WithSharedConfigFiles([]string{filepath.Join("testdata", "empty_creds_config")}), + WithSharedCredentialsFiles([]string{filepath.Join("testdata", "load_credentials")}), WithLogConfigurationWarnings(true), WithLogger(logger), }, @@ -616,13 +610,13 @@ func TestSharedConfigLoading(t *testing.T) { Expect: SharedConfig{ Profile: "partial-creds-1", }, - Err: "Partial Credentials found for profile partial-creds-1", + Err: "partial credentials found for profile partial-creds-1", }, "credentials override configuration profile": { LoadOptionFns: []func(*LoadOptions) error{ WithSharedConfigProfile("complete"), - WithSharedConfigFiles([]string{"testdata/load_config"}), - WithSharedCredentialsFiles([]string{"testdata/load_credentials"}), + WithSharedConfigFiles([]string{filepath.Join("testdata", "load_config")}), + WithSharedCredentialsFiles([]string{filepath.Join("testdata", "load_credentials")}), WithLogConfigurationWarnings(true), WithLogger(logger), }, @@ -632,7 +626,8 @@ func TestSharedConfigLoading(t *testing.T) { Credentials: aws.Credentials{ AccessKeyID: "credsAccessKey", SecretAccessKey: "credsSecretKey", - Source: "SharedConfigCredentials: testdata/load_credentials", + Source: fmt.Sprintf("SharedConfigCredentials: %v", + filepath.Join("testdata", "load_credentials")), }, Region: "us-west-2", }, @@ -640,8 +635,8 @@ func TestSharedConfigLoading(t *testing.T) { "credentials profile has complete credentials": { LoadOptionFns: []func(*LoadOptions) error{ WithSharedConfigProfile("complete"), - WithSharedConfigFiles([]string{"testdata/empty_creds_config"}), - WithSharedCredentialsFiles([]string{"testdata/load_credentials"}), + WithSharedConfigFiles([]string{filepath.Join("testdata", "empty_creds_config")}), + WithSharedCredentialsFiles([]string{filepath.Join("testdata", "load_credentials")}), WithLogConfigurationWarnings(true), WithLogger(logger), }, @@ -658,10 +653,10 @@ func TestSharedConfigLoading(t *testing.T) { "credentials split between multiple credentials files": { LoadOptionFns: []func(*LoadOptions) error{ WithSharedConfigProfile("partial-creds-1"), - WithSharedConfigFiles([]string{"testdata/empty_creds_config"}), + WithSharedConfigFiles([]string{filepath.Join("testdata", "empty_creds_config")}), WithSharedCredentialsFiles([]string{ - "testdata/load_credentials", - "testdata/load_credentials_secondary", + filepath.Join("testdata", "load_credentials"), + filepath.Join("testdata", "load_credentials_secondary"), }), WithLogConfigurationWarnings(true), WithLogger(logger), @@ -670,15 +665,15 @@ func TestSharedConfigLoading(t *testing.T) { Expect: SharedConfig{ Profile: "partial-creds-1", }, - Err: "Partial Credentials", + Err: "partial credentials", }, "credentials split between multiple configuration files": { LoadOptionFns: []func(*LoadOptions) error{ WithSharedConfigProfile("partial-creds-1"), - WithSharedCredentialsFiles([]string{"testdata/empty_creds_config"}), + WithSharedCredentialsFiles([]string{filepath.Join("testdata", "empty_creds_config")}), WithSharedConfigFiles([]string{ - "testdata/load_config", - "testdata/load_config_secondary", + filepath.Join("testdata", "load_config"), + filepath.Join("testdata", "load_config_secondary"), }), WithLogConfigurationWarnings(true), WithLogger(logger), @@ -689,26 +684,231 @@ func TestSharedConfigLoading(t *testing.T) { Region: "us-west-2", }, ExpectLog: "", - Err: "Partial Credentials", + Err: "partial credentials", }, "credentials split between credentials and config files": { LoadOptionFns: []func(*LoadOptions) error{ WithSharedConfigProfile("partial-creds-1"), WithSharedConfigFiles([]string{ - "testdata/load_config", + filepath.Join("testdata", "load_config"), }), WithSharedCredentialsFiles([]string{ - "testdata/load_credentials", + filepath.Join("testdata", "load_credentials"), }), WithLogConfigurationWarnings(true), WithLogger(logger), }, - LoadFn: loadSharedConfig, - Expect: SharedConfig{ + LoadFn: loadSharedConfig, + Expect: SharedConfig{ Profile: "partial-creds-1", }, ExpectLog: "", - Err: "Partial Credentials", + Err: "partial credentials", + }, + "replaced profile with prefixed profile in config files": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("replaced-profile"), + WithSharedConfigFiles([]string{ + filepath.Join("testdata", "load_config"), + }), + WithSharedCredentialsFiles([]string{ + filepath.Join("testdata", "empty_creds_config"), + }), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Expect: SharedConfig{ + Profile: "replaced-profile", + Region: "eu-west-1", + }, + ExpectLog: "profile defined with name `replaced-profile` is ignored.", + }, + "replaced profile with prefixed profile in credentials files": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("replaced-profile"), + WithSharedCredentialsFiles([]string{ + filepath.Join("testdata", "load_credentials"), + }), + WithSharedConfigFiles([]string{ + filepath.Join("testdata", "empty_creds_config"), + }), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Expect: SharedConfig{ + Profile: "replaced-profile", + Region: "us-west-2", + }, + ExpectLog: "profile defined with name `profile replaced-profile` is ignored.", + }, + "ignored profiles w/o prefixed profile across credentials and config files": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("replaced-profile"), + WithSharedCredentialsFiles([]string{ + filepath.Join("testdata", "load_credentials"), + }), + WithSharedConfigFiles([]string{ + filepath.Join("testdata", "load_config"), + }), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Expect: SharedConfig{ + Profile: "replaced-profile", + Region: "us-west-2", + }, + ExpectLog: "profile defined with name `profile replaced-profile` is ignored.", + }, + "1. profile with name as `profile` in config file": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("profile"), + WithSharedCredentialsFiles([]string{ + filepath.Join("testdata", "empty_creds_config"), + }), + WithSharedConfigFiles([]string{ + filepath.Join("testdata", "load_config"), + }), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Err: "failed to get shared config profile, profile", + ExpectLog: "profile defined with name `profile` is ignored", + }, + "2. profile with name as `profile ` in config file": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("profile "), + WithSharedCredentialsFiles([]string{ + filepath.Join("testdata", "empty_creds_config"), + }), + WithSharedConfigFiles([]string{ + filepath.Join("testdata", "load_config"), + }), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Err: "failed to get shared config profile, profile", + ExpectLog: "profile defined with name `profile` is ignored", + }, + "3. profile with name as `profile\t` in config file": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("profile"), + WithSharedCredentialsFiles([]string{ + filepath.Join("testdata", "empty_creds_config"), + }), + WithSharedConfigFiles([]string{ + filepath.Join("testdata", "load_config"), + }), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Err: "failed to get shared config profile, profile", + ExpectLog: "profile defined with name `profile` is ignored", + }, + "profile with tabs as delimiter for profile prefix in config file": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("with-tab"), + WithSharedCredentialsFiles([]string{ + filepath.Join("testdata", "empty_creds_config"), + }), + WithSharedConfigFiles([]string{ + filepath.Join("testdata", "load_config"), + }), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Expect: SharedConfig{ + Profile: "with-tab", + Region: "cn-north-1", + }, + }, + "profile with tabs as delimiter for profile prefix in credentials file": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("with-tab"), + WithSharedCredentialsFiles([]string{ + filepath.Join("testdata", "load_credentials"), + }), + WithSharedConfigFiles([]string{ + filepath.Join("testdata", "empty_creds_config"), + }), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Err: "failed to get shared config profile, with-tab", + ExpectLog: "profile defined with name `profile with-tab` is ignored", + }, + "profile with name as `profile profile` in credentials file": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("profile"), + WithSharedCredentialsFiles([]string{ + filepath.Join("testdata", "load_credentials"), + }), + WithSharedConfigFiles([]string{ + filepath.Join("testdata", "empty_creds_config"), + }), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Err: "failed to get shared config profile, profile", + ExpectLog: "profile defined with name `profile profile` is ignored", + }, + "profile with name profile-bar in credentials file": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("profile-bar"), + WithSharedCredentialsFiles([]string{ + filepath.Join("testdata", "load_credentials"), + }), + WithSharedConfigFiles([]string{ + filepath.Join("testdata", "empty_creds_config"), + }), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Expect: SharedConfig{ + Profile: "profile-bar", + Region: "us-west-2", + }, + }, + "profile with name profile-bar in config file": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("profile-bar"), + WithSharedCredentialsFiles([]string{ + filepath.Join("testdata", "empty_creds_config"), + }), + WithSharedConfigFiles([]string{ + filepath.Join("testdata", "load_config"), + }), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Err: "failed to get shared config profile, profile-bar", + ExpectLog: "profile defined with name `profile-bar` is ignored", + }, + "profile ignored in credentials and config file": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedConfigProfile("ignored-profile"), + WithSharedCredentialsFiles([]string{ + filepath.Join("testdata", "load_credentials"), + }), + WithSharedConfigFiles([]string{ + filepath.Join("testdata", "load_config"), + }), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Err: "failed to get shared config profile, ignored-profile", + ExpectLog: "profile defined with name `ignored-profile` is ignored.", }, } @@ -723,6 +923,14 @@ func TestSharedConfigLoading(t *testing.T) { } cfg, err := c.LoadFn(context.Background(), configs{options}) + + if e, a := c.ExpectLog, loggerBuf.String(); !strings.Contains(a, e) { + t.Errorf("expect %v logged in %v", e, a) + } + if loggerBuf.Len() == 0 && len(c.ExpectLog) != 0 { + t.Errorf("expected log, got none") + } + if len(c.Err) > 0 { if err == nil { t.Fatalf("expected error %v, got none", c.Err) @@ -739,12 +947,6 @@ func TestSharedConfigLoading(t *testing.T) { t.Errorf("expect %v got %v", e, a) } - if e, a := c.ExpectLog, loggerBuf.String(); !strings.Contains(a, e) { - t.Errorf("expect %v logged in %v", e, a) - } - if loggerBuf.Len() == 0 && len(c.ExpectLog) != 0 { - t.Errorf("expected log, got none") - } }) } } diff --git a/config/testdata/empty_creds_config b/config/testdata/empty_creds_config index ab109a17201..e69de29bb2d 100644 --- a/config/testdata/empty_creds_config +++ b/config/testdata/empty_creds_config @@ -1 +0,0 @@ -[default] diff --git a/config/testdata/load_config b/config/testdata/load_config index bf1d305b7c1..204a24b7099 100644 --- a/config/testdata/load_config +++ b/config/testdata/load_config @@ -1,6 +1,9 @@ [default] region = ap-south-1 +[profile with-tab] +region = cn-north-1 + [profile default] region = ap-north-1 @@ -17,3 +20,25 @@ aws_access_key_id = notForAccess aws_access_key_id = configAccessKey aws_secret_access_key = configSecretKey region = us-west-2 + +[replaced-profile] +region = eu-northeast-1 + +[profile replaced-profile] +region = eu-west-1 + +[profile] +region = us-west-2 + +[profile ] +region = ap-east-2 + +[profile\t] +region = ap-south-1 + +[profile-bar] +region = us-east-1 + +[ignored-profile] +region = ap-east-1 + diff --git a/config/testdata/load_credentials b/config/testdata/load_credentials index b742653f755..c5a7a5106d9 100644 --- a/config/testdata/load_credentials +++ b/config/testdata/load_credentials @@ -1,6 +1,9 @@ [duplicate-profile] region = us-east-1 +[profile with-tab] +region = cn-south-2 + [duplicate-profile] region = us-west-2 @@ -14,3 +17,19 @@ aws_secret_access_key = notForSecretAccess aws_access_key_id = credsAccessKey aws_secret_access_key = credsSecretKey +[replaced-profile] +region = us-west-2 + +[profile replaced-profile] +region = us-east-1 + +[profile profile] +region = us-east-1 + +[profile-bar] +region = us-west-2 + +[profile ignored-profile] +region = ap-east-1 + + diff --git a/internal/ini/bench_test.go b/internal/ini/bench_test.go index d97cc37182e..e4ebec22077 100644 --- a/internal/ini/bench_test.go +++ b/internal/ini/bench_test.go @@ -21,7 +21,7 @@ region = us-west-2 func BenchmarkINIParser(b *testing.B) { for i := 0; i < b.N; i++ { - ParseBytes([]byte(section), "ini/bench_test") + ParseBytes([]byte(section)) } } diff --git a/internal/ini/ini.go b/internal/ini/ini.go index acb06ba982b..4a80eb9a99d 100644 --- a/internal/ini/ini.go +++ b/internal/ini/ini.go @@ -34,13 +34,13 @@ func Parse(f io.Reader, path string) (Sections, error) { } // ParseBytes will parse the given bytes and return the parsed sections. -func ParseBytes(b []byte, path string) (Sections, error) { +func ParseBytes(b []byte) (Sections, error) { tree, err := ParseASTBytes(b) if err != nil { return Sections{}, err } - v := NewDefaultVisitor(path) + v := NewDefaultVisitor("") if err = Walk(tree, v); err != nil { return Sections{}, err } From ac08327185a7ab6c0fc1a6df4f1c10080c94f7ed Mon Sep 17 00:00:00 2001 From: skotambkar Date: Mon, 21 Dec 2020 23:12:58 -0800 Subject: [PATCH 3/5] fix windows test failure --- config/shared_config_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/shared_config_test.go b/config/shared_config_test.go index c4e09df0f32..c64c40dd3a8 100644 --- a/config/shared_config_test.go +++ b/config/shared_config_test.go @@ -524,7 +524,7 @@ func TestSharedConfigLoading(t *testing.T) { Region: "us-west-2", }, ExpectLog: "For profile: profile duplicate-profile, overriding region value, with a region value found in a " + - "duplicate profile defined later in the same file testdata/load_config", + "duplicate profile defined later in the same file", }, "profile prefix not used in the configuration files": { @@ -646,7 +646,7 @@ func TestSharedConfigLoading(t *testing.T) { Credentials: aws.Credentials{ AccessKeyID: "credsAccessKey", SecretAccessKey: "credsSecretKey", - Source: "SharedConfigCredentials: testdata/load_credentials", + Source: fmt.Sprintf("SharedConfigCredentials: %v", filepath.Join("testdata", "load_credentials")), }, }, }, From 910a7500eae48af7c9c059a1212f2f6c8a431aa7 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Tue, 22 Dec 2020 12:59:04 -0800 Subject: [PATCH 4/5] set config or creds file defaults only if unset --- config/shared_config.go | 16 +++++++++------- config/shared_config_test.go | 10 ++++++++++ config/testdata/load_config | 2 +- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/config/shared_config.go b/config/shared_config.go index fabebfb2cff..5b55570d677 100644 --- a/config/shared_config.go +++ b/config/shared_config.go @@ -259,9 +259,11 @@ type LoadSharedConfigOptions struct { // For example, given two files A and B. Both define credentials. If the order // of the files are A then B, B's credential values will be used instead of A's. // -// If no config files, credentials files are provided, the LoadSharedConfigProfile -// will default to location `.aws/config` for config files and `.aws/credentials` -// for credentials files respectively as per +// If config files are not set, SDK will default to using a file at location `.aws/config` if present. +// If credentials files are not set, SDK will default to using a file at location `.aws/credentials` if present. +// No default files are set, if files set to an empty slice. +// +// You can read more about shared config and credentials file location at // https://docs.aws.amazon.com/credref/latest/refdocs/file-location.html#file-location // func LoadSharedConfigProfile(ctx context.Context, profile string, optFns ...func(*LoadSharedConfigOptions)) (SharedConfig, error) { @@ -270,11 +272,11 @@ func LoadSharedConfigProfile(ctx context.Context, profile string, optFns ...func fn(&option) } - if len(option.ConfigFiles) == 0 { + if option.ConfigFiles == nil { option.ConfigFiles = DefaultSharedConfigFiles } - if len(option.CredentialsFiles) == 0 { + if option.CredentialsFiles == nil { option.CredentialsFiles = DefaultSharedCredentialsFiles } @@ -328,8 +330,8 @@ func processConfigSections(ctx context.Context, sections ini.Sections, logger lo if logger != nil { logger.Logf(logging.Debug, - "A profile defined with name `%v` is ignored. For use within a shared configuration file, " + - "a non-default profile must have `profile ` prefixed to the profile name.\n", + "A profile defined with name `%v` is ignored. For use within a shared configuration file, "+ + "a non-default profile must have `profile ` prefixed to the profile name.\n", section, ) } diff --git a/config/shared_config_test.go b/config/shared_config_test.go index c64c40dd3a8..b798e7d5723 100644 --- a/config/shared_config_test.go +++ b/config/shared_config_test.go @@ -910,6 +910,16 @@ func TestSharedConfigLoading(t *testing.T) { Err: "failed to get shared config profile, ignored-profile", ExpectLog: "profile defined with name `ignored-profile` is ignored.", }, + "config and creds files explicitly set to empty slice": { + LoadOptionFns: []func(*LoadOptions) error{ + WithSharedCredentialsFiles([]string{}), + WithSharedConfigFiles([]string{}), + WithLogConfigurationWarnings(true), + WithLogger(logger), + }, + LoadFn: loadSharedConfig, + Err: "failed to get shared config profile, default", + }, } for name, c := range cases { diff --git a/config/testdata/load_config b/config/testdata/load_config index 204a24b7099..f5b4391438f 100644 --- a/config/testdata/load_config +++ b/config/testdata/load_config @@ -33,7 +33,7 @@ region = us-west-2 [profile ] region = ap-east-2 -[profile\t] +[profile ] region = ap-south-1 [profile-bar] From 8ba8642c53c63a00e9cb86c2a8d9b06c17a13571 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Tue, 22 Dec 2020 13:43:56 -0800 Subject: [PATCH 5/5] rebase branch with master --- config/example_test.go | 2 -- config/resolve_credentials_test.go | 4 ++-- config/resolve_test.go | 2 +- config/shared_config.go | 2 +- config/shared_config_test.go | 9 +++++---- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/config/example_test.go b/config/example_test.go index 4b0f39fb765..a81d29a6b93 100644 --- a/config/example_test.go +++ b/config/example_test.go @@ -3,8 +3,6 @@ package config_test import ( "context" "fmt" - "github.com/awslabs/smithy-go/middleware" - smithyhttp "github.com/awslabs/smithy-go/transport/http" "log" "net/http" "path/filepath" diff --git a/config/resolve_credentials_test.go b/config/resolve_credentials_test.go index c051ac344a7..c26703ad411 100644 --- a/config/resolve_credentials_test.go +++ b/config/resolve_credentials_test.go @@ -3,8 +3,6 @@ package config import ( "context" "fmt" - "github.com/aws/aws-sdk-go-v2/internal/awstesting" - "github.com/awslabs/smithy-go/middleware" "net/http" "net/http/httptest" "os" @@ -16,7 +14,9 @@ import ( "time" "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/internal/awstesting" "github.com/aws/aws-sdk-go-v2/service/sts" + "github.com/aws/smithy-go/middleware" ) func swapECSContainerURI(path string) func() { diff --git a/config/resolve_test.go b/config/resolve_test.go index 3d4b8e98e32..984f66655ae 100644 --- a/config/resolve_test.go +++ b/config/resolve_test.go @@ -3,7 +3,6 @@ package config import ( "bytes" "context" - "github.com/awslabs/smithy-go/logging" "io/ioutil" "net/http" "testing" @@ -13,6 +12,7 @@ import ( "github.com/aws/aws-sdk-go-v2/credentials" "github.com/aws/aws-sdk-go-v2/internal/awstesting" "github.com/aws/aws-sdk-go-v2/internal/awstesting/unit" + "github.com/aws/smithy-go/logging" ) func TestResolveCustomCABundle(t *testing.T) { diff --git a/config/shared_config.go b/config/shared_config.go index 5b55570d677..e6130714b14 100644 --- a/config/shared_config.go +++ b/config/shared_config.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "github.com/awslabs/smithy-go/logging" "os" "path/filepath" "runtime" @@ -13,6 +12,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/internal/ini" + "github.com/aws/smithy-go/logging" ) const ( diff --git a/config/shared_config_test.go b/config/shared_config_test.go index b798e7d5723..cf41e70a99e 100644 --- a/config/shared_config_test.go +++ b/config/shared_config_test.go @@ -4,15 +4,16 @@ import ( "bytes" "context" "fmt" - "github.com/aws/aws-sdk-go-v2/aws" - "github.com/aws/aws-sdk-go-v2/internal/ini" - "github.com/awslabs/smithy-go/logging" - "github.com/awslabs/smithy-go/ptr" "path/filepath" "reflect" "strconv" "strings" "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/internal/ini" + "github.com/aws/smithy-go/logging" + "github.com/aws/smithy-go/ptr" ) var _ regionProvider = (*SharedConfig)(nil)