From 7937434e18622578a9cf67d196ce915c3bfb52d4 Mon Sep 17 00:00:00 2001 From: skotambkar Date: Mon, 21 Dec 2020 21:32:19 -0800 Subject: [PATCH] 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 8fc7055f896..e96373feb16 100644 --- a/config/load_options.go +++ b/config/load_options.go @@ -60,17 +60,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 }