error handling with working test

aws · Dec 21, 2020 · 0303236 · 0303236
1 parent 16d85f7
commit 0303236
Show file tree

Hide file tree

Showing 9 changed files with 385 additions and 118 deletions.
diff --git 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
diff --git 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 → config/testdata/empty_creds_config b/config/testdata/empty → config/testdata/empty_creds_config
diff --git a/config/testdata/load_config 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
@@ -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
@@ -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
@@ -0,0 +1,3 @@
+[partial-creds-1]
+aws_access_key_id = credsAccessKey
+
diff --git 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))