From 14f597921b4e948439857a039f422be0cd9d232e Mon Sep 17 00:00:00 2001 From: Jason Del Ponte <961963+jasdel@users.noreply.github.com> Date: Mon, 24 Jan 2022 08:54:34 -0800 Subject: [PATCH] config: Fix bug in SDK's merging of duration_seconds shared config (#1568) Fixes the SDK's handling of `duration_sections` in the shared credentials file or specified in multiple shared config and shared credentials files under the same profile. Fixes #1192 --- .../3989a7746d05464ba5ad8f91386e6e8a.json | 10 +++++ config/shared_config_test.go | 42 +++++++++++++++++-- config/testdata/shared_config | 7 ++++ config/testdata/shared_credentials | 4 ++ internal/ini/literal_tokens.go | 18 +------- internal/ini/literal_tokens_test.go | 33 +++++++++++++++ 6 files changed, 95 insertions(+), 19 deletions(-) create mode 100644 .changelog/3989a7746d05464ba5ad8f91386e6e8a.json diff --git a/.changelog/3989a7746d05464ba5ad8f91386e6e8a.json b/.changelog/3989a7746d05464ba5ad8f91386e6e8a.json new file mode 100644 index 00000000000..c57ce87750a --- /dev/null +++ b/.changelog/3989a7746d05464ba5ad8f91386e6e8a.json @@ -0,0 +1,10 @@ +{ + "id": "3989a774-6d05-464b-a5ad-8f91386e6e8a", + "type": "bugfix", + "collapse": true, + "description": "Fixes the SDK's handling of `duration_sections` in the shared credentials file or specified in multiple shared config and shared credentials files under the same profile. [#1568](https://github.com/aws/aws-sdk-go-v2/pull/1568). Thanks to [Amir Szekely](https://github.com/kichik) for help reproduce this bug.", + "modules": [ + "config", + "internal/ini" + ] +} \ No newline at end of file diff --git a/config/shared_config_test.go b/config/shared_config_test.go index f5195ecb4f5..abf3eab343d 100644 --- a/config/shared_config_test.go +++ b/config/shared_config_test.go @@ -534,6 +534,36 @@ func TestNewSharedConfig(t *testing.T) { Profile: "invaliddefaultsmode", Err: fmt.Errorf("failed to load defaults_mode from shared config, invalid value: invalid"), }, + "merged profiles across files": { + ConfigFilenames: []string{testConfigFilename}, + CredentialsFilenames: []string{testCredentialsFilename}, + Profile: "merged_profiles", + Expected: SharedConfig{ + Profile: "merged_profiles", + RoleARN: "creds_profile_arn", + RoleDurationSeconds: aws.Duration(1023 * time.Second), + }, + }, + "merged profiles across config files": { + ConfigFilenames: []string{testConfigFilename, testConfigFilename}, + CredentialsFilenames: []string{}, + Profile: "merged_profiles", + Expected: SharedConfig{ + Profile: "merged_profiles", + RoleARN: "config_profile_arn", + RoleDurationSeconds: aws.Duration(3601 * time.Second), + }, + }, + "merged profiles across credentials files": { + ConfigFilenames: []string{}, + CredentialsFilenames: []string{testCredentialsFilename, testCredentialsFilename}, + Profile: "merged_profiles", + Expected: SharedConfig{ + Profile: "merged_profiles", + RoleARN: "creds_profile_arn", + RoleDurationSeconds: aws.Duration(1023 * time.Second), + }, + }, } for name, c := range cases { @@ -593,6 +623,12 @@ func TestLoadSharedConfigFromSection(t *testing.T) { Profile: "profile partial_creds", Expected: SharedConfig{}, }, + "profile with role duration": { + Profile: "profile with_role_duration", + Expected: SharedConfig{ + RoleDurationSeconds: aws.Duration(3601 * time.Second), + }, + }, "profile with complete creds": { Profile: "profile complete_creds", Expected: SharedConfig{ @@ -689,12 +725,12 @@ func TestLoadSharedConfigFromSection(t *testing.T) { } return } - if err != nil { t.Fatalf("expect no error, got %v", err) } - if e, a := c.Expected, cfg; !reflect.DeepEqual(e, a) { - t.Errorf("expect %v, got %v", e, a) + + if diff := cmp.Diff(c.Expected, cfg); diff != "" { + t.Errorf("expect shared config match\n%s", diff) } }) } diff --git a/config/testdata/shared_config b/config/testdata/shared_config index ee2c7b9961c..b34736f08c4 100644 --- a/config/testdata/shared_config +++ b/config/testdata/shared_config @@ -13,6 +13,13 @@ aws_secret_access_key = SECRET [profile alt_profile_name] region = alt_profile_name_region +[profile with_role_duration] +duration_seconds = 3601 + +[profile merged_profiles] +duration_seconds = 3601 +role_arn = config_profile_arn + [profile short_profile_name_first] region = short_profile_name_first_short diff --git a/config/testdata/shared_credentials b/config/testdata/shared_credentials index 8b33504d77d..99ee1925645 100644 --- a/config/testdata/shared_credentials +++ b/config/testdata/shared_credentials @@ -18,3 +18,7 @@ region = eu-west-2 [DONOTNORMALIZE] region = eu-west-3 + +[merged_profiles] +duration_seconds = 1023 +role_arn = creds_profile_arn diff --git a/internal/ini/literal_tokens.go b/internal/ini/literal_tokens.go index bea83941ef1..eca42d1b293 100644 --- a/internal/ini/literal_tokens.go +++ b/internal/ini/literal_tokens.go @@ -216,22 +216,8 @@ func NewStringValue(str string) (Value, error) { // NewIntValue returns a Value type generated using an int64 input. func NewIntValue(i int64) (Value, error) { - return newValue(IntegerType, 10, []rune{rune(i)}) -} - -// Append will append values and change the type to a string -// type. -func (v *Value) Append(tok Token) { - r := tok.Raw() - if v.Type != QuotedStringType { - v.Type = StringType - r = tok.raw[1 : len(tok.raw)-1] - } - if tok.Type() != TokenLit { - v.raw = append(v.raw, tok.Raw()...) - } else { - v.raw = append(v.raw, r...) - } + v := strconv.FormatInt(i, 10) + return newValue(IntegerType, 10, []rune(v)) } func (v Value) String() string { diff --git a/internal/ini/literal_tokens_test.go b/internal/ini/literal_tokens_test.go index e8df2d07640..ec14d0c9298 100644 --- a/internal/ini/literal_tokens_test.go +++ b/internal/ini/literal_tokens_test.go @@ -196,3 +196,36 @@ func TestNewLiteralToken(t *testing.T) { }) } } + +func TestNewStringValue(t *testing.T) { + const expect = "abc123" + + actual, err := NewStringValue(expect) + if err != nil { + t.Fatalf("expect no error, %v", err) + } + + if e, a := StringType, actual.Type; e != a { + t.Errorf("expect %v type got %v", e, a) + } + if e, a := expect, actual.str; e != a { + t.Errorf("expect %v string got %v", e, a) + } +} + +func TestNewIntValue(t *testing.T) { + const expect int64 = 1234 + + actual, err := NewIntValue(expect) + if err != nil { + t.Fatalf("expect no error, %v", err) + } + + if e, a := IntegerType, actual.Type; e != a { + t.Errorf("expect %v type got %v", e, a) + } + if e, a := expect, actual.integer; e != a { + t.Errorf("expect %v integer got %v", e, a) + } + +}