Skip to content

Commit

Permalink
Merge pull request #34300 from hashicorp/b-rollback-ini-parsing-error
Browse files Browse the repository at this point in the history
Rolls back AWS SDK modules with shared config parsing bug
  • Loading branch information
gdavison authored Nov 7, 2023
2 parents 48e1a98 + dab76b0 commit 5e7fddb
Show file tree
Hide file tree
Showing 6 changed files with 171 additions and 6 deletions.
7 changes: 7 additions & 0 deletions .changelog/34300.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:bug
provider: Fixes parsing error in AWS shared config files with extra whitespace
```

```release-note:bug
provider: Fixes poor performance when parsing AWS shared config files
```
12 changes: 10 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ require (
github.com/YakDriver/regexache v0.23.0
github.com/aws/aws-sdk-go v1.47.4
github.com/aws/aws-sdk-go-v2 v1.22.1
github.com/aws/aws-sdk-go-v2/config v1.22.1
github.com/aws/aws-sdk-go-v2/config v1.20.0
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.14.2
github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.13.2
github.com/aws/aws-sdk-go-v2/service/accessanalyzer v1.23.0
Expand Down Expand Up @@ -129,7 +129,7 @@ require (
github.com/aws/aws-sdk-go-v2/credentials v1.15.1 // indirect
github.com/aws/aws-sdk-go-v2/internal/configsources v1.2.1 // indirect
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.5.1 // indirect
github.com/aws/aws-sdk-go-v2/internal/ini v1.5.0 // indirect
github.com/aws/aws-sdk-go-v2/internal/ini v1.4.0 // indirect
github.com/aws/aws-sdk-go-v2/internal/v4a v1.2.1 // indirect
github.com/aws/aws-sdk-go-v2/service/dynamodb v1.23.0 // indirect
github.com/aws/aws-sdk-go-v2/service/iam v1.22.7 // indirect
Expand Down Expand Up @@ -199,3 +199,11 @@ require (
)

replace github.com/hashicorp/terraform-plugin-log => github.com/gdavison/terraform-plugin-log v0.0.0-20230928191232-6c653d8ef8fb

exclude ( // Contains INI parsing regression
github.com/aws/aws-sdk-go-v2/config v1.21.0
github.com/aws/aws-sdk-go-v2/config v1.22.0
github.com/aws/aws-sdk-go-v2/config v1.22.1
github.com/aws/aws-sdk-go-v2/internal/ini v1.5.0
github.com/aws/aws-sdk-go-v2/internal/ini v1.5.1
)
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ github.com/aws/aws-sdk-go-v2 v1.22.1 h1:sjnni/AuoTXxHitsIdT0FwmqUuNUuHtufcVDErVF
github.com/aws/aws-sdk-go-v2 v1.22.1/go.mod h1:Kd0OJtkW3Q0M0lUWGszapWjEvrXDzRW+D21JNsroB+c=
github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.5.0 h1:hHgLiIrTRtddC0AKcJr5s7i/hLgcpTt+q/FKxf1Zayk=
github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.5.0/go.mod h1:w4I/v3NOWgD+qvs1NPEwhd++1h3XPHFaVxasfY6HlYQ=
github.com/aws/aws-sdk-go-v2/config v1.22.1 h1:UrRYnF7mXCGuKmZWlczOXeH0WUbQpi/gseQIPtrhme8=
github.com/aws/aws-sdk-go-v2/config v1.22.1/go.mod h1:2eWgw5lps8fKI7LZVTrRTYP6HE6k/uEFUuTSHfXwqP0=
github.com/aws/aws-sdk-go-v2/config v1.20.0 h1:q2+/mqFhY0J9m3Tb5RGFE3R4sdaUkIe4k2EuDfE3c08=
github.com/aws/aws-sdk-go-v2/config v1.20.0/go.mod h1:7+1riCZXyT+sAGvneR5j+Zl1GyfbBUNQurpQTE6FP6k=
github.com/aws/aws-sdk-go-v2/credentials v1.15.1 h1:hmf6lAm9hk7uLCfapZn/jL05lm6Uwdbn1B0fgjyuf4M=
github.com/aws/aws-sdk-go-v2/credentials v1.15.1/go.mod h1:QTcHga3ZbQOneJuxmGBOCxiClxmp+TlvmjFexAnJ790=
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.14.2 h1:gIeH4+o1MN/caGBWjoGQTUTIu94xD6fI5B2+TcwBf70=
Expand All @@ -43,8 +43,8 @@ github.com/aws/aws-sdk-go-v2/internal/configsources v1.2.1/go.mod h1:V5CY8wNurvP
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.4.37/go.mod h1:Qe+2KtKml+FEsQF/DHmDV+xjtche/hwoF75EG4UlHW8=
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.5.1 h1:ZpaV/j48RlPc4AmOZuPv22pJliXjXq8/reL63YzyFnw=
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.5.1/go.mod h1:R8aXraabD2e3qv1csxM14/X9WF4wFMIY0kH4YEtYD5M=
github.com/aws/aws-sdk-go-v2/internal/ini v1.5.0 h1:DqOQvIfmGkXZUVJnl9VRk0AnxyS59tCtX9k1Pyss4Ak=
github.com/aws/aws-sdk-go-v2/internal/ini v1.5.0/go.mod h1:VV/Kbw9Mg1GWJOT9WK+oTL3cWZiXtapnNvDSRqTZLsg=
github.com/aws/aws-sdk-go-v2/internal/ini v1.4.0 h1:21tlTXq3ev10yLMAjXZzpkZbrl49h3ElSjmxD57tD/E=
github.com/aws/aws-sdk-go-v2/internal/ini v1.4.0/go.mod h1:d9YrBHJhyzDCv5UsEVRizHlFV6Q0sLemFq6uxuqWfUw=
github.com/aws/aws-sdk-go-v2/internal/v4a v1.2.1 h1:vzYLDkwTw4CY0vUk84MeSufRf8XIsC/GsoIFXD60sTg=
github.com/aws/aws-sdk-go-v2/internal/v4a v1.2.1/go.mod h1:ToBFBnjeGR2ruMx8IWp/y7vSK3Irj5/oPwifruiqoOM=
github.com/aws/aws-sdk-go-v2/service/accessanalyzer v1.23.0 h1:po7m50dyaYEThawBGQigDwXcIPdZ2TbL43mBwgAG1K4=
Expand Down
28 changes: 28 additions & 0 deletions internal/errs/sdkdiag/compare.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0

package sdkdiag

import (
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
)

// Comparer is a Comparer function for use with cmp.Diff to compare two diag.Diagnostic values
func Comparer(l, r diag.Diagnostic) bool {
if l.Severity != r.Severity {
return false
}
if l.Summary != r.Summary {
return false
}
if l.Detail != r.Detail {
return false
}

lp := l.AttributePath
rp := r.AttributePath
if len(lp) != len(rp) {
return false
}
return lp.Equals(rp)
}
12 changes: 12 additions & 0 deletions internal/errs/sdkdiag/diags.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package sdkdiag
import (
"errors"
"fmt"
"strings"

"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices"
Expand Down Expand Up @@ -48,3 +49,14 @@ func DiagnosticString(d diag.Diagnostic) string {
}
return fmt.Sprintf("%s\n\n%s", d.Summary, d.Detail)
}

// DiagnosticsString formats a Diagnostics
func DiagnosticsString(diags diag.Diagnostics) string {
var buf strings.Builder

for _, d := range diags {
fmt.Fprintln(&buf, DiagnosticString(d))
}

return buf.String()
}
110 changes: 110 additions & 0 deletions internal/provider/provider_config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0

package provider

import (
"context"
"os"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/hashicorp/aws-sdk-go-base/v2/servicemocks"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
terraformsdk "github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
"github.com/hashicorp/terraform-provider-aws/internal/errs"
"github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag"
)

// TestSharedConfigFileParsing prevents regression in shared config file parsing
// * https://github.com/aws/aws-sdk-go-v2/issues/2349: indented keys
func TestSharedConfigFileParsing(t *testing.T) { //nolint:paralleltest
testcases := map[string]struct {
Config map[string]any
SharedConfigurationFile string
Check func(t *testing.T, meta *conns.AWSClient)
}{
"leading whitespace": {
// Do not "fix" indentation!
SharedConfigurationFile: `
[default]
region = us-west-2
`, //lintignore:AWSAT003
Check: func(t *testing.T, meta *conns.AWSClient) {
//lintignore:AWSAT003
if a, e := meta.Region, "us-west-2"; a != e {
t.Errorf("expected region %q, got %q", e, a)
}
},
},
}

for name, tc := range testcases { //nolint:paralleltest
tc := tc

t.Run(name, func(t *testing.T) {
ctx := context.TODO()

oldenv := servicemocks.InitSessionTestEnv()
defer servicemocks.PopEnv(oldenv)

config := map[string]any{
"access_key": servicemocks.MockStaticAccessKey,
"secret_key": servicemocks.MockStaticSecretKey,
"skip_credentials_validation": true,
"skip_requesting_account_id": true,
}

if tc.SharedConfigurationFile != "" {
file, err := os.CreateTemp("", "aws-sdk-go-base-shared-configuration-file")

if err != nil {
t.Fatalf("unexpected error creating temporary shared configuration file: %s", err)
}

defer os.Remove(file.Name())

err = os.WriteFile(file.Name(), []byte(tc.SharedConfigurationFile), 0600)

if err != nil {
t.Fatalf("unexpected error writing shared configuration file: %s", err)
}

config["shared_config_files"] = []any{file.Name()}
}

rc := terraformsdk.NewResourceConfigRaw(config)

p, err := New(ctx)
if err != nil {
t.Fatal(err)
}

var diags diag.Diagnostics
diags = append(diags, p.Validate(rc)...)
if diags.HasError() {
t.Fatalf("validating: %s", sdkdiag.DiagnosticsString(diags))
}

diags = append(diags, p.Configure(ctx, rc)...)

// The provider always returns a warning if there is no account ID
var expected diag.Diagnostics
expected = append(expected,
errs.NewWarningDiagnostic(
"AWS account ID not found for provider",
"See https://www.terraform.io/docs/providers/aws/index.html#skip_requesting_account_id for implications.",
),
)

if diff := cmp.Diff(diags, expected, cmp.Comparer(sdkdiag.Comparer)); diff != "" {
t.Errorf("unexpected diagnostics difference: %s", diff)
}

meta := p.Meta().(*conns.AWSClient)

tc.Check(t, meta)
})
}
}

0 comments on commit 5e7fddb

Please sign in to comment.