Skip to content

Commit

Permalink
Merge pull request #35726 from hashicorp/td-endpoint-validation
Browse files Browse the repository at this point in the history
Adds warnings for conflicting service endpoints
  • Loading branch information
gdavison authored Feb 12, 2024
2 parents 13e14ac + af57066 commit 1202377
Show file tree
Hide file tree
Showing 41 changed files with 522 additions and 63 deletions.
20 changes: 18 additions & 2 deletions internal/generate/serviceendpointtests/file.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ import (
smithyhttp "github.com/aws/smithy-go/transport/http"
"github.com/google/go-cmp/cmp"
"github.com/hashicorp/aws-sdk-go-base/v2/servicemocks"
{{- if gt (len .Aliases) 0 }}
"github.com/hashicorp/go-cty/cty"
{{- end }}
"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"
Expand Down Expand Up @@ -129,7 +132,7 @@ func TestEndpointConfiguration(t *testing.T) { //nolint:paralleltest // uses t.S
withPackageNameEndpointInConfig,
withAliasName{{ $i }}EndpointInConfig,
},
expected: expectPackageNameConfigEndpoint(),
expected: conflictsWith(expectPackageNameConfigEndpoint()),
},
{{ end }}

Expand Down Expand Up @@ -205,7 +208,7 @@ func TestEndpointConfiguration(t *testing.T) { //nolint:paralleltest // uses t.S
withAliasName{{ $i }}EndpointInConfig,
withAliasName{{ $j }}EndpointInConfig,
},
expected: expectAliasName{{ $i }}ConfigEndpoint(),
expected: conflictsWith(expectAliasName{{ $i }}ConfigEndpoint()),
},
{{ end }}

Expand Down Expand Up @@ -592,6 +595,19 @@ func withAliasName{{ $i }}EndpointInConfig(setup *caseSetup) {
}
{{ end }}

{{ if gt (len .Aliases) 0 }}
func conflictsWith(e caseExpectations) caseExpectations {
e.diags = append(e.diags, provider.ConflictingEndpointsWarningDiag(
cty.GetAttrPath("endpoints").IndexInt(0),
packageName,
{{ range $i, $alias := .Aliases -}}
aliasName{{ $i }},
{{ end }}
))
return e
}
{{ end }}

func withAwsEnvVar(setup *caseSetup) {
setup.environmentVariables[awsEnvVar] = awsServiceEnvvarEndpoint
}
Expand Down
51 changes: 50 additions & 1 deletion internal/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ import (
"errors"
"fmt"
"os"
"strings"
"time"

"github.com/YakDriver/regexache"
"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/feature/ec2/imds"
awsbase "github.com/hashicorp/aws-sdk-go-base/v2"
"github.com/hashicorp/go-cty/cty"
"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
Expand Down Expand Up @@ -865,18 +867,50 @@ func expandIgnoreTags(ctx context.Context, tfMap map[string]interface{}) *tftags
func expandEndpoints(_ context.Context, tfList []interface{}) (map[string]string, diag.Diagnostics) {
var diags diag.Diagnostics

endpointsPath := cty.GetAttrPath("endpoints")

if l := len(tfList); l > 1 {
diags = append(diags, errs.NewAttributeWarningDiagnostic(
endpointsPath,
"Invalid Attribute Value",
fmt.Sprintf("Attribute %q should have at most 1 element, got %d."+
"\n\nThis will be an error in a future release.",
errs.PathString(endpointsPath), l),
))
}

endpoints := make(map[string]string)

for _, tfMapRaw := range tfList {
for i, tfMapRaw := range tfList {
tfMap, ok := tfMapRaw.(map[string]interface{})

if !ok {
continue
}

elementPath := endpointsPath.IndexInt(i)

for _, endpoint := range names.Endpoints() {
pkg := endpoint.ProviderPackage

if len(endpoint.Aliases) > 0 {
count := 0
attrs := []string{pkg}
if tfMap[pkg] != "" {
count++
}
for _, alias := range endpoint.Aliases {
attrs = append(attrs, alias)
if tfMap[alias] != "" {
count++
}
}

if count > 1 {
diags = append(diags, ConflictingEndpointsWarningDiag(elementPath, attrs...))
}
}

if endpoints[pkg] == "" {
if v := tfMap[pkg].(string); v != "" {
endpoints[pkg] = v
Expand Down Expand Up @@ -941,3 +975,18 @@ func DeprecatedEnvVarDiag(envvar, replacement string) diag.Diagnostic {
fmt.Sprintf(`The environment variable "%s" is deprecated. Use environment variable "%s" instead.`, envvar, replacement),
)
}

func ConflictingEndpointsWarningDiag(elementPath cty.Path, attrs ...string) diag.Diagnostic {
attrPaths := make([]string, len(attrs))
for i, attr := range attrs {
path := elementPath.GetAttr(attr)
attrPaths[i] = `"` + errs.PathString(path) + `"`
}
return errs.NewAttributeWarningDiagnostic(
elementPath,
"Invalid Attribute Combination",
fmt.Sprintf("Only one of the following attributes should be set: %s"+
"\n\nThis will be an error in a future release.",
strings.Join(attrPaths, ", ")),
)
}
11 changes: 8 additions & 3 deletions internal/provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/hashicorp/go-cty/cty"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag"
"github.com/hashicorp/terraform-provider-aws/names"
Expand Down Expand Up @@ -64,6 +65,7 @@ func TestEndpointMultipleKeys(t *testing.T) { //nolint:paralleltest
endpoints map[string]string
expectedService string
expectedEndpoint string
expectedDiags diag.Diagnostics
}{
{
endpoints: map[string]string{
Expand All @@ -86,6 +88,11 @@ func TestEndpointMultipleKeys(t *testing.T) { //nolint:paralleltest
},
expectedService: names.Transcribe,
expectedEndpoint: "https://transcribe.fake.test",
expectedDiags: diag.Diagnostics{ConflictingEndpointsWarningDiag(
cty.GetAttrPath("endpoints").IndexInt(0),
"transcribe",
"transcribeservice",
)},
},
}

Expand All @@ -101,10 +108,8 @@ func TestEndpointMultipleKeys(t *testing.T) { //nolint:paralleltest
endpoints[k] = v
}

var expectedDiags diag.Diagnostics

results, diags := expandEndpoints(ctx, []interface{}{endpoints})
if diff := cmp.Diff(diags, expectedDiags, cmp.Comparer(sdkdiag.Comparer)); diff != "" {
if diff := cmp.Diff(diags, testcase.expectedDiags, cmp.Comparer(sdkdiag.Comparer)); diff != "" {
t.Errorf("unexpected diagnostics difference: %s", diff)
}

Expand Down
17 changes: 14 additions & 3 deletions internal/service/amp/service_endpoints_gen_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 11 additions & 1 deletion internal/service/appautoscaling/service_endpoints_gen_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 11 additions & 1 deletion internal/service/appintegrations/service_endpoints_gen_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 11 additions & 1 deletion internal/service/ce/service_endpoints_gen_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 11 additions & 1 deletion internal/service/cloudcontrol/service_endpoints_gen_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 11 additions & 1 deletion internal/service/cloudhsmv2/service_endpoints_gen_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 1202377

Please sign in to comment.