diff --git a/internal/backend/remote-state/s3/backend.go b/internal/backend/remote-state/s3/backend.go index 8e02aa97f2..86690a1cd0 100644 --- a/internal/backend/remote-state/s3/backend.go +++ b/internal/backend/remote-state/s3/backend.go @@ -567,7 +567,7 @@ var endpointsSchema = singleNestedAttribute{ }, validateString{ Validators: []stringValidator{ - validateStringURL, + validateStringLegacyURL, }, }, }, @@ -580,7 +580,7 @@ var endpointsSchema = singleNestedAttribute{ }, validateString{ Validators: []stringValidator{ - validateStringURL, + validateStringLegacyURL, }, }, }, @@ -593,7 +593,7 @@ var endpointsSchema = singleNestedAttribute{ }, validateString{ Validators: []stringValidator{ - validateStringURL, + validateStringLegacyURL, }, }, }, @@ -606,7 +606,7 @@ var endpointsSchema = singleNestedAttribute{ }, validateString{ Validators: []stringValidator{ - validateStringURL, + validateStringLegacyURL, }, }, }, @@ -769,7 +769,7 @@ func (b *Backend) PrepareConfig(obj cty.Value) (cty.Value, tfdiags.Diagnostics) endpointValidators := validateString{ Validators: []stringValidator{ - validateStringURL, + validateStringLegacyURL, }, } for _, k := range maps.Keys(endpointFields) { @@ -778,9 +778,15 @@ func (b *Backend) PrepareConfig(obj cty.Value) (cty.Value, tfdiags.Diagnostics) endpointValidators.ValidateAttr(val, attrPath, &diags) } } + if val := obj.GetAttr("ec2_metadata_service_endpoint"); !val.IsNull() { attrPath := cty.GetAttrPath("ec2_metadata_service_endpoint") - endpointValidators.ValidateAttr(val, attrPath, &diags) + ec2MetadataEndpointValidators := validateString{ + Validators: []stringValidator{ + validateStringValidURL, + }, + } + ec2MetadataEndpointValidators.ValidateAttr(val, attrPath, &diags) } validateAttributesConflict( diff --git a/internal/backend/remote-state/s3/backend_test.go b/internal/backend/remote-state/s3/backend_test.go index 840d1ae723..e680b10d42 100644 --- a/internal/backend/remote-state/s3/backend_test.go +++ b/internal/backend/remote-state/s3/backend_test.go @@ -273,12 +273,9 @@ func TestBackendConfig_DynamoDBEndpoint(t *testing.T) { "dynamodb": "dynamo.test", }, }, + expectedEndpoint: "dynamo.test/", expectedDiags: tfdiags.Diagnostics{ - attributeErrDiag( - "Invalid Value", - `The value must be a valid URL containing at least a scheme and hostname. Had "dynamo.test"`, - cty.GetAttrPath("endpoints").GetAttr("dynamodb"), - ), + legacyIncompleteURLDiag("dynamo.test", cty.GetAttrPath("endpoints").GetAttr("dynamodb")), }, }, "deprecated config URL": { @@ -294,13 +291,10 @@ func TestBackendConfig_DynamoDBEndpoint(t *testing.T) { config: map[string]any{ "dynamodb_endpoint": "dynamo.test", }, + expectedEndpoint: "dynamo.test/", expectedDiags: tfdiags.Diagnostics{ deprecatedAttrDiag(cty.GetAttrPath("dynamodb_endpoint"), cty.GetAttrPath("endpoints").GetAttr("dynamodb")), - attributeErrDiag( - "Invalid Value", - `The value must be a valid URL containing at least a scheme and hostname. Had "dynamo.test"`, - cty.GetAttrPath("dynamodb_endpoint"), - ), + legacyIncompleteURLDiag("dynamo.test", cty.GetAttrPath("dynamodb_endpoint")), }, }, "config conflict": { @@ -418,11 +412,7 @@ func TestBackendConfig_IAMEndpoint(t *testing.T) { }, }, expectedDiags: tfdiags.Diagnostics{ - attributeErrDiag( - "Invalid Value", - `The value must be a valid URL containing at least a scheme and hostname. Had "iam.test"`, - cty.GetAttrPath("endpoints").GetAttr("iam"), - ), + legacyIncompleteURLDiag("iam.test", cty.GetAttrPath("endpoints").GetAttr("iam")), }, }, "deprecated config URL": { @@ -439,11 +429,7 @@ func TestBackendConfig_IAMEndpoint(t *testing.T) { }, expectedDiags: tfdiags.Diagnostics{ deprecatedAttrDiag(cty.GetAttrPath("iam_endpoint"), cty.GetAttrPath("endpoints").GetAttr("iam")), - attributeErrDiag( - "Invalid Value", - `The value must be a valid URL containing at least a scheme and hostname. Had "iam.test"`, - cty.GetAttrPath("iam_endpoint"), - ), + legacyIncompleteURLDiag("iam.test", cty.GetAttrPath("iam_endpoint")), }, }, "config conflict": { @@ -542,12 +528,9 @@ func TestBackendConfig_S3Endpoint(t *testing.T) { "s3": "s3.test", }, }, + expectedEndpoint: "/s3.test", expectedDiags: tfdiags.Diagnostics{ - attributeErrDiag( - "Invalid Value", - `The value must be a valid URL containing at least a scheme and hostname. Had "s3.test"`, - cty.GetAttrPath("endpoints").GetAttr("s3"), - ), + legacyIncompleteURLDiag("s3.test", cty.GetAttrPath("endpoints").GetAttr("s3")), }, }, "deprecated config URL": { @@ -563,13 +546,10 @@ func TestBackendConfig_S3Endpoint(t *testing.T) { config: map[string]any{ "endpoint": "s3.test", }, + expectedEndpoint: "/s3.test", expectedDiags: tfdiags.Diagnostics{ deprecatedAttrDiag(cty.GetAttrPath("endpoint"), cty.GetAttrPath("endpoints").GetAttr("s3")), - attributeErrDiag( - "Invalid Value", - `The value must be a valid URL containing at least a scheme and hostname. Had "s3.test"`, - cty.GetAttrPath("endpoint"), - ), + legacyIncompleteURLDiag("s3.test", cty.GetAttrPath("endpoint")), }, }, "config conflict": { diff --git a/internal/backend/remote-state/s3/validate.go b/internal/backend/remote-state/s3/validate.go index 4fdda0ce51..603fe27240 100644 --- a/internal/backend/remote-state/s3/validate.go +++ b/internal/backend/remote-state/s3/validate.go @@ -268,7 +268,9 @@ func validateStringKMSKey(val string, path cty.Path, diags *tfdiags.Diagnostics) *diags = diags.Append(ds) } -func validateStringURL(val string, path cty.Path, diags *tfdiags.Diagnostics) { +// validateStringLegacyURL validates that a string can be parsed generally as a URL, but does +// not ensure that the URL is valid. +func validateStringLegacyURL(val string, path cty.Path, diags *tfdiags.Diagnostics) { u, err := url.Parse(val) if err != nil { *diags = diags.Append(attributeErrDiag( @@ -279,13 +281,44 @@ func validateStringURL(val string, path cty.Path, diags *tfdiags.Diagnostics) { return } if u.Scheme == "" || u.Host == "" { + *diags = diags.Append(legacyIncompleteURLDiag(val, path)) + return + } +} + +func legacyIncompleteURLDiag(val string, path cty.Path) tfdiags.Diagnostic { + return attributeWarningDiag( + "Complete URL Expected", + fmt.Sprintf(`The value should be a valid URL containing at least a scheme and hostname. Had %q. + +Using an incomplete URL, such as a hostname only, may work, but may have unexpected behavior.`, val), + path, + ) +} + +// validateStringValidURL validates that a URL is a valid URL, inclding a scheme and host +func validateStringValidURL(val string, path cty.Path, diags *tfdiags.Diagnostics) { + u, err := url.Parse(val) + if err != nil { *diags = diags.Append(attributeErrDiag( "Invalid Value", - fmt.Sprintf("The value must be a valid URL containing at least a scheme and hostname. Had %q", val), + fmt.Sprintf("The value %q cannot be parsed as a URL: %s", val, err), path, )) return } + if u.Scheme == "" || u.Host == "" { + *diags = diags.Append(invalidURLDiag(val, path)) + return + } +} + +func invalidURLDiag(val string, path cty.Path) tfdiags.Diagnostic { + return attributeErrDiag( + "Invalid Value", + fmt.Sprintf("The value must be a valid URL containing at least a scheme and hostname. Had %q", val), + path, + ) } // Using a val of `cty.ValueSet` would be better here, but we can't get an ElementIterator from a ValueSet diff --git a/internal/backend/remote-state/s3/validate_test.go b/internal/backend/remote-state/s3/validate_test.go index 4dad4f838c..516631b0ee 100644 --- a/internal/backend/remote-state/s3/validate_test.go +++ b/internal/backend/remote-state/s3/validate_test.go @@ -658,7 +658,7 @@ func TestValidateDurationBetween(t *testing.T) { } } -func TestValidateStringURL(t *testing.T) { +func TestValidateStringLegacyURL(t *testing.T) { t.Parallel() path := cty.GetAttrPath("field") @@ -694,44 +694,28 @@ func TestValidateStringURL(t *testing.T) { "no scheme no trailing slash": { val: "domain.test", expected: tfdiags.Diagnostics{ - attributeErrDiag( - "Invalid Value", - fmt.Sprintf("The value must be a valid URL containing at least a scheme and hostname. Had %q", "domain.test"), - path, - ), + legacyIncompleteURLDiag("domain.test", path), }, }, "no scheme no path": { val: "domain.test/", expected: tfdiags.Diagnostics{ - attributeErrDiag( - "Invalid Value", - fmt.Sprintf("The value must be a valid URL containing at least a scheme and hostname. Had %q", "domain.test/"), - path, - ), + legacyIncompleteURLDiag("domain.test/", path), }, }, "no scheme with path": { val: "domain.test/path", expected: tfdiags.Diagnostics{ - attributeErrDiag( - "Invalid Value", - fmt.Sprintf("The value must be a valid URL containing at least a scheme and hostname. Had %q", "domain.test/path"), - path, - ), + legacyIncompleteURLDiag("domain.test/path", path), }, }, "no scheme with port": { val: "domain.test:1234", expected: tfdiags.Diagnostics{ - attributeErrDiag( - "Invalid Value", - fmt.Sprintf("The value must be a valid URL containing at least a scheme and hostname. Had %q", "domain.test:1234"), - path, - ), + legacyIncompleteURLDiag("domain.test:1234", path), }, }, } @@ -742,14 +726,90 @@ func TestValidateStringURL(t *testing.T) { t.Parallel() var diags tfdiags.Diagnostics - validateStringURL(testcase.val, path, &diags) + validateStringLegacyURL(testcase.val, path, &diags) if diff := cmp.Diff(diags, testcase.expected, cmp.Comparer(diagnosticComparer)); diff != "" { t.Errorf("unexpected diagnostics difference: %s", diff) } }) } +} + +func TestValidateStringValidURL(t *testing.T) { + t.Parallel() + + path := cty.GetAttrPath("field") + + testcases := map[string]struct { + val string + expected tfdiags.Diagnostics + }{ + "no trailing slash": { + val: "https://domain.test", + }, + + "no path": { + val: "https://domain.test/", + }, + + "with path": { + val: "https://domain.test/path", + }, + + "with port no trailing slash": { + val: "https://domain.test:1234", + }, + + "with port no path": { + val: "https://domain.test:1234/", + }, + + "with port with path": { + val: "https://domain.test:1234/path", + }, + + "no scheme no trailing slash": { + val: "domain.test", + expected: tfdiags.Diagnostics{ + invalidURLDiag("domain.test", path), + }, + }, + + "no scheme no path": { + val: "domain.test/", + expected: tfdiags.Diagnostics{ + invalidURLDiag("domain.test/", path), + }, + }, + + "no scheme with path": { + val: "domain.test/path", + expected: tfdiags.Diagnostics{ + invalidURLDiag("domain.test/path", path), + }, + }, + + "no scheme with port": { + val: "domain.test:1234", + expected: tfdiags.Diagnostics{ + invalidURLDiag("domain.test:1234", path), + }, + }, + } + for name, testcase := range testcases { + testcase := testcase + t.Run(name, func(t *testing.T) { + t.Parallel() + + var diags tfdiags.Diagnostics + validateStringValidURL(testcase.val, path, &diags) + + if diff := cmp.Diff(diags, testcase.expected, cmp.Comparer(diagnosticComparer)); diff != "" { + t.Errorf("unexpected diagnostics difference: %s", diff) + } + }) + } } func Test_validateStringDoesNotContain(t *testing.T) {