From 03032361dd35ee5a38d133f6dfe1a7efe771df0e Mon Sep 17 00:00:00 2001 From: skotambkar Date: Sun, 20 Dec 2020 23:51:11 -0800 Subject: [PATCH] error handling with working test --- config/resolve_processcreds_test.go | 2 +- config/shared_config.go | 196 ++++++-------- config/shared_config_test.go | 250 +++++++++++++++++- config/testdata/{empty => empty_creds_config} | 0 config/testdata/load_config | 19 ++ config/testdata/load_config_secondary | 3 + config/testdata/load_credentials | 16 ++ config/testdata/load_credentials_secondary | 3 + internal/ini/visitor.go | 14 +- 9 files changed, 385 insertions(+), 118 deletions(-) rename config/testdata/{empty => empty_creds_config} (100%) 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/config/resolve_processcreds_test.go b/config/resolve_processcreds_test.go index 512f62a23b8..aca98038571 100644 --- a/config/resolve_processcreds_test.go +++ b/config/resolve_processcreds_test.go @@ -16,7 +16,7 @@ func setupEnvForProcesscredsConfigFile() { } os.Setenv("AWS_CONFIG_FILE", filepath.Join("testdata", filename)) - os.Setenv("AWS_SHARED_CREDENTIALS_FILE", filepath.Join("testdata", "empty")) + os.Setenv("AWS_SHARED_CREDENTIALS_FILE", filepath.Join("testdata", "empty_creds_config")) } func setupEnvForProcesscredsCredentialsFile() { diff --git a/config/shared_config.go b/config/shared_config.go index 3ee325d9df1..f7b20fc93a6 100644 --- a/config/shared_config.go +++ b/config/shared_config.go @@ -205,7 +205,7 @@ func loadSharedConfig(ctx context.Context, configs configs) (Config, error) { return nil, err } if !ok { - configurationFiles = append(configurationFiles, DefaultSharedConfigFiles...) + configurationFiles = DefaultSharedConfigFiles } credentialsFiles, ok, err = getSharedCredentialsFiles(ctx, configs) @@ -213,7 +213,7 @@ func loadSharedConfig(ctx context.Context, configs configs) (Config, error) { return nil, err } if !ok { - credentialsFiles = append(credentialsFiles, DefaultSharedCredentialsFiles...) + credentialsFiles = DefaultSharedCredentialsFiles } // setup logger if log configuration warning is seti @@ -338,6 +338,14 @@ func processConfigSections(ctx context.Context, sections ini.Sections, logger lo // 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) } @@ -356,7 +364,7 @@ func processCredentialsSections(ctx context.Context, sections ini.Sections, logg if logger != nil { logger.Logf(logging.Debug, - "profile %v is ignored. A profile with the \"profile \" prefix is invalid"+ + "The %v is ignored. A profile with the \"profile \" prefix is invalid "+ "for the shared credentials file.\n", section, ) @@ -398,36 +406,37 @@ func loadIniFiles(filenames []string) (ini.Sections, error) { // mergeSections merges source section properties into destination section properties func mergeSections(dst, src ini.Sections) error { for _, sectionName := range src.List() { - section, _ := src.GetSection(sectionName) + 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, section) + dst.SetSection(sectionName, srcSection) continue } - // merge with destination section + // merge with destination srcSection dstSection, _ := dst.GetSection(sectionName) - // errors should be merged if any - dstSection.Errors = section.Errors - if !(section.Has(accessKeyIDKey) && section.Has(secretAccessKey)) { - section.Errors = append(section.Errors, - fmt.Errorf("Partial Credentials found for profile %v", sectionName)) - } + // errors should be overriden if any + dstSection.Errors = srcSection.Errors // Access key id update - if section.Has(accessKeyIDKey) && section.Has(secretAccessKey) { - accessKey := section.String(accessKeyIDKey) - secretKey := section.String(secretAccessKey) + if srcSection.Has(accessKeyIDKey) && srcSection.Has(secretAccessKey) { + accessKey := srcSection.String(accessKeyIDKey) + secretKey := srcSection.String(secretAccessKey) - logKeys := []string{accessKeyIDKey, secretAccessKey} - for _, lk := range logKeys { - if dstSection.Has(lk) { - 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, lk, dstSection.SourceFile[lk], - lk, section.SourceFile[lk])) - } + 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 @@ -445,8 +454,8 @@ func mergeSections(dst, src ini.Sections) error { dstSection.UpdateValue(secretAccessKey, v) // update session token - if section.Has(sessionTokenKey) { - sessionKey := section.String(sessionTokenKey) + if srcSection.Has(sessionTokenKey) { + sessionKey := srcSection.String(sessionTokenKey) val, e := ini.NewStringValue(sessionKey) if e != nil { @@ -458,20 +467,20 @@ func mergeSections(dst, src ini.Sections) error { 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, section.SourceFile[sessionTokenKey])) + sessionTokenKey, srcSection.SourceFile[sessionTokenKey])) } dstSection.UpdateValue(sessionTokenKey, val) - dstSection.UpdateSourceFile(sessionTokenKey, section.SourceFile[sessionTokenKey]) + dstSection.UpdateSourceFile(sessionTokenKey, srcSection.SourceFile[sessionTokenKey]) } // update source file to reflect where the static creds came from - dstSection.UpdateSourceFile(accessKeyIDKey, section.SourceFile[accessKeyIDKey]) - dstSection.UpdateSourceFile(secretAccessKey, section.SourceFile[secretAccessKey]) + dstSection.UpdateSourceFile(accessKeyIDKey, srcSection.SourceFile[accessKeyIDKey]) + dstSection.UpdateSourceFile(secretAccessKey, srcSection.SourceFile[secretAccessKey]) } - if section.Has(roleArnKey) { - key := section.String(roleArnKey) + if srcSection.Has(roleArnKey) { + key := srcSection.String(roleArnKey) val, err := ini.NewStringValue(key) if err != nil { return fmt.Errorf("error merging roleArnKey, %w", err) @@ -482,15 +491,15 @@ func mergeSections(dst, src ini.Sections) error { 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, section.SourceFile[roleArnKey])) + roleArnKey, srcSection.SourceFile[roleArnKey])) } dstSection.UpdateValue(roleArnKey, val) - dstSection.UpdateSourceFile(roleArnKey, section.SourceFile[roleArnKey]) + dstSection.UpdateSourceFile(roleArnKey, srcSection.SourceFile[roleArnKey]) } - if section.Has(sourceProfileKey) { - key := section.String(sourceProfileKey) + if srcSection.Has(sourceProfileKey) { + key := srcSection.String(sourceProfileKey) val, err := ini.NewStringValue(key) if err != nil { return fmt.Errorf("error merging sourceProfileKey, %w", err) @@ -501,15 +510,15 @@ func mergeSections(dst, src ini.Sections) error { 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, section.SourceFile[sourceProfileKey])) + sourceProfileKey, srcSection.SourceFile[sourceProfileKey])) } dstSection.UpdateValue(sourceProfileKey, val) - dstSection.UpdateSourceFile(sourceProfileKey, section.SourceFile[sourceProfileKey]) + dstSection.UpdateSourceFile(sourceProfileKey, srcSection.SourceFile[sourceProfileKey]) } - if section.Has(credentialSourceKey) { - key := section.String(credentialSourceKey) + if srcSection.Has(credentialSourceKey) { + key := srcSection.String(credentialSourceKey) val, err := ini.NewStringValue(key) if err != nil { return fmt.Errorf("error merging credentialSourceKey, %w", err) @@ -520,15 +529,15 @@ func mergeSections(dst, src ini.Sections) error { 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, section.SourceFile[credentialSourceKey])) + credentialSourceKey, srcSection.SourceFile[credentialSourceKey])) } dstSection.UpdateValue(credentialSourceKey, val) - dstSection.UpdateSourceFile(credentialSourceKey, section.SourceFile[credentialSourceKey]) + dstSection.UpdateSourceFile(credentialSourceKey, srcSection.SourceFile[credentialSourceKey]) } - if section.Has(externalIDKey) { - key := section.String(externalIDKey) + if srcSection.Has(externalIDKey) { + key := srcSection.String(externalIDKey) val, err := ini.NewStringValue(key) if err != nil { return fmt.Errorf("error merging externalIDKey, %w", err) @@ -539,15 +548,15 @@ func mergeSections(dst, src ini.Sections) error { 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, section.SourceFile[externalIDKey])) + externalIDKey, srcSection.SourceFile[externalIDKey])) } dstSection.UpdateValue(externalIDKey, val) - dstSection.UpdateSourceFile(externalIDKey, section.SourceFile[externalIDKey]) + dstSection.UpdateSourceFile(externalIDKey, srcSection.SourceFile[externalIDKey]) } - if section.Has(mfaSerialKey) { - key := section.String(mfaSerialKey) + if srcSection.Has(mfaSerialKey) { + key := srcSection.String(mfaSerialKey) val, err := ini.NewStringValue(key) if err != nil { return fmt.Errorf("error merging mfaSerialKey, %w", err) @@ -558,15 +567,15 @@ func mergeSections(dst, src ini.Sections) error { 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, section.SourceFile[mfaSerialKey])) + mfaSerialKey, srcSection.SourceFile[mfaSerialKey])) } dstSection.UpdateValue(mfaSerialKey, val) - dstSection.UpdateSourceFile(mfaSerialKey, section.SourceFile[mfaSerialKey]) + dstSection.UpdateSourceFile(mfaSerialKey, srcSection.SourceFile[mfaSerialKey]) } - if section.Has(roleSessionNameKey) { - key := section.String(roleSessionNameKey) + if srcSection.Has(roleSessionNameKey) { + key := srcSection.String(roleSessionNameKey) val, err := ini.NewStringValue(key) if err != nil { return fmt.Errorf("error merging roleSessionNameKey, %w", err) @@ -577,15 +586,15 @@ func mergeSections(dst, src ini.Sections) error { 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, section.SourceFile[roleSessionNameKey])) + roleSessionNameKey, srcSection.SourceFile[roleSessionNameKey])) } dstSection.UpdateValue(roleSessionNameKey, val) - dstSection.UpdateSourceFile(roleSessionNameKey, section.SourceFile[roleSessionNameKey]) + dstSection.UpdateSourceFile(roleSessionNameKey, srcSection.SourceFile[roleSessionNameKey]) } - if section.Has(regionKey) { - key := section.String(regionKey) + if srcSection.Has(regionKey) { + key := srcSection.String(regionKey) val, err := ini.NewStringValue(key) if err != nil { return fmt.Errorf("error merging regionKey, %w", err) @@ -596,15 +605,15 @@ func mergeSections(dst, src ini.Sections) error { 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, section.SourceFile[regionKey])) + regionKey, srcSection.SourceFile[regionKey])) } dstSection.UpdateValue(regionKey, val) - dstSection.UpdateSourceFile(regionKey, section.SourceFile[regionKey]) + dstSection.UpdateSourceFile(regionKey, srcSection.SourceFile[regionKey]) } - if section.Has(enableEndpointDiscoveryKey) { - key := section.String(enableEndpointDiscoveryKey) + if srcSection.Has(enableEndpointDiscoveryKey) { + key := srcSection.String(enableEndpointDiscoveryKey) val, err := ini.NewStringValue(key) if err != nil { return fmt.Errorf("error merging enableEndpointDiscoveryKey, %w", err) @@ -615,15 +624,15 @@ func mergeSections(dst, src ini.Sections) error { 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, section.SourceFile[enableEndpointDiscoveryKey])) + enableEndpointDiscoveryKey, srcSection.SourceFile[enableEndpointDiscoveryKey])) } dstSection.UpdateValue(enableEndpointDiscoveryKey, val) - dstSection.UpdateSourceFile(enableEndpointDiscoveryKey, section.SourceFile[enableEndpointDiscoveryKey]) + dstSection.UpdateSourceFile(enableEndpointDiscoveryKey, srcSection.SourceFile[enableEndpointDiscoveryKey]) } - if section.Has(credentialProcessKey) { - key := section.String(credentialProcessKey) + if srcSection.Has(credentialProcessKey) { + key := srcSection.String(credentialProcessKey) val, err := ini.NewStringValue(key) if err != nil { return fmt.Errorf("error merging credentialProcessKey, %w", err) @@ -634,15 +643,15 @@ func mergeSections(dst, src ini.Sections) error { 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, section.SourceFile[credentialProcessKey])) + credentialProcessKey, srcSection.SourceFile[credentialProcessKey])) } dstSection.UpdateValue(credentialProcessKey, val) - dstSection.UpdateSourceFile(credentialProcessKey, section.SourceFile[credentialProcessKey]) + dstSection.UpdateSourceFile(credentialProcessKey, srcSection.SourceFile[credentialProcessKey]) } - if section.Has(webIdentityTokenFileKey) { - key := section.String(webIdentityTokenFileKey) + if srcSection.Has(webIdentityTokenFileKey) { + key := srcSection.String(webIdentityTokenFileKey) val, err := ini.NewStringValue(key) if err != nil { return fmt.Errorf("error merging webIdentityTokenFileKey, %w", err) @@ -653,15 +662,15 @@ func mergeSections(dst, src ini.Sections) error { 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, section.SourceFile[webIdentityTokenFileKey])) + webIdentityTokenFileKey, srcSection.SourceFile[webIdentityTokenFileKey])) } dstSection.UpdateValue(webIdentityTokenFileKey, val) - dstSection.UpdateSourceFile(webIdentityTokenFileKey, section.SourceFile[webIdentityTokenFileKey]) + dstSection.UpdateSourceFile(webIdentityTokenFileKey, srcSection.SourceFile[webIdentityTokenFileKey]) } - if section.Has(s3UseARNRegionKey) { - key := section.String(s3UseARNRegionKey) + if srcSection.Has(s3UseARNRegionKey) { + key := srcSection.String(s3UseARNRegionKey) val, err := ini.NewStringValue(key) if err != nil { return fmt.Errorf("error merging s3UseARNRegionKey, %w", err) @@ -672,26 +681,26 @@ func mergeSections(dst, src ini.Sections) error { 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, section.SourceFile[s3UseARNRegionKey])) + s3UseARNRegionKey, srcSection.SourceFile[s3UseARNRegionKey])) } dstSection.UpdateValue(s3UseARNRegionKey, val) - dstSection.UpdateSourceFile(s3UseARNRegionKey, section.SourceFile[s3UseARNRegionKey]) + dstSection.UpdateSourceFile(s3UseARNRegionKey, srcSection.SourceFile[s3UseARNRegionKey]) } // role duration seconds key update - if section.Has(roleDurationSecondsKey) { - roleDurationSeconds := section.Int(roleDurationSecondsKey) + 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, section.SourceFile[roleDurationSecondsKey]) + dstSection.UpdateSourceFile(roleDurationSecondsKey, srcSection.SourceFile[roleDurationSecondsKey]) } - // set section on dst section + // set srcSection on dst srcSection dst = dst.SetSection(sectionName, dstSection) } @@ -914,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 @@ -941,23 +938,6 @@ 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 { @@ -972,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 7f678abb23c..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" ) var _ regionProvider = (*SharedConfig)(nil) @@ -502,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/empty b/config/testdata/empty_creds_config similarity index 100% rename from config/testdata/empty rename to config/testdata/empty_creds_config 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/internal/ini/visitor.go b/internal/ini/visitor.go index a80621deb7d..4094f442f3c 100644 --- a/internal/ini/visitor.go +++ b/internal/ini/visitor.go @@ -23,11 +23,11 @@ type DefaultVisitor struct { // scope is the profile which is being visited scope string - // Sections defines list of the profile section - Sections Sections - // path is the file path which the visitor is visiting path string + + // Sections defines list of the profile section + Sections Sections } // NewDefaultVisitor returns a DefaultVisitor. It takes in a filepath @@ -123,11 +123,11 @@ func (v *DefaultVisitor) VisitStatement(stmt AST) error { // lower casing name to handle duplicates correctly. name = strings.ToLower(name) // attach profile name on section - v.Sections.container[name] = Section{ - Name: name, - SourceFile: make(map[string]string, 0), + 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))