Skip to content

Commit

Permalink
adds more tests and feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@skotambkar
skotambkar committed Dec 22, 2020
1 parent 193ffa1 commit 7937434
Show file tree
Hide file tree
Showing 10 changed files with 379 additions and 128 deletions.
4 changes: 2 additions & 2 deletions config/env_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down
16 changes: 12 additions & 4 deletions config/load_options.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions config/resolve_processcreds_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()

Expand All @@ -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()

Expand Down
123 changes: 62 additions & 61 deletions config/shared_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
}
Expand All @@ -298,26 +312,24 @@ 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
}

return cfg, nil
}

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,
)
}
Expand All @@ -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")
Expand All @@ -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))
}

Expand All @@ -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,
)
Expand All @@ -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()

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit 7937434

Please sign in to comment.