aherman
/
terraform
mirror of https://github.com/hashicorp/terraform
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #34017 from hashicorp/s3/b-relax-endpoint-validation

Browse Source 
backend/s3: Relax endpoint validation rule
pull/34045/head
Jared Baker 3 years ago committed by GitHub
parent 83a8003482 3de6dbc7a8
commit 4f26451cd3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 139 additions and 60 deletions
Show Stats Download Patch File Download Diff File

18
internal/backend/remote-state/s3/backend.go
Unescape View File

@ -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(

40
internal/backend/remote-state/s3/backend_test.go
Unescape View File

@ -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": {

37
internal/backend/remote-state/s3/validate.go
Unescape View File

@ -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

104
internal/backend/remote-state/s3/validate_test.go
Unescape View File

@ -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) {

Write Preview
Loading…
Cancel
Save