From 050523e20310d3b106af68c2bdd5d7a23d3b4f40 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 19 Apr 2024 06:38:53 +0000 Subject: [PATCH 1/9] build(deps): bump github.com/hashicorp/terraform-plugin-framework Bumps the terraform-devex group with 1 update: [github.com/hashicorp/terraform-plugin-framework](https://github.com/hashicorp/terraform-plugin-framework). Updates `github.com/hashicorp/terraform-plugin-framework` from 1.7.0 to 1.8.0 - [Release notes](https://github.com/hashicorp/terraform-plugin-framework/releases) - [Changelog](https://github.com/hashicorp/terraform-plugin-framework/blob/main/CHANGELOG.md) - [Commits](https://github.com/hashicorp/terraform-plugin-framework/compare/v1.7.0...v1.8.0) --- updated-dependencies: - dependency-name: github.com/hashicorp/terraform-plugin-framework dependency-type: direct:production update-type: version-update:semver-minor dependency-group: terraform-devex ... Signed-off-by: dependabot[bot] --- go.mod | 10 +++++----- go.sum | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/go.mod b/go.mod index 01e5f61f761b..46196ce368eb 100644 --- a/go.mod +++ b/go.mod @@ -188,12 +188,12 @@ require ( github.com/hashicorp/go-uuid v1.0.3 github.com/hashicorp/go-version v1.6.0 github.com/hashicorp/hcl/v2 v2.20.0 - github.com/hashicorp/terraform-plugin-framework v1.7.0 + github.com/hashicorp/terraform-plugin-framework v1.8.0 github.com/hashicorp/terraform-plugin-framework-jsontypes v0.1.0 github.com/hashicorp/terraform-plugin-framework-timeouts v0.4.1 github.com/hashicorp/terraform-plugin-framework-timetypes v0.3.0 github.com/hashicorp/terraform-plugin-framework-validators v0.12.0 - github.com/hashicorp/terraform-plugin-go v0.22.1 + github.com/hashicorp/terraform-plugin-go v0.22.2 github.com/hashicorp/terraform-plugin-log v0.9.0 github.com/hashicorp/terraform-plugin-mux v0.15.0 github.com/hashicorp/terraform-plugin-sdk/v2 v2.33.0 @@ -242,7 +242,7 @@ require ( github.com/go-logr/logr v1.4.1 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-test/deep v1.1.0 // indirect - github.com/golang/protobuf v1.5.3 // indirect + github.com/golang/protobuf v1.5.4 // indirect github.com/google/uuid v1.6.0 // indirect github.com/hashicorp/errwrap v1.1.0 // indirect github.com/hashicorp/go-checkpoint v0.5.0 // indirect @@ -280,8 +280,8 @@ require ( golang.org/x/net v0.24.0 // indirect golang.org/x/sys v0.19.0 // indirect google.golang.org/appengine v1.6.8 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20240123012728-ef4313101c80 // indirect - google.golang.org/grpc v1.62.1 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20240227224415-6ceb2ff114de // indirect + google.golang.org/grpc v1.63.2 // indirect google.golang.org/protobuf v1.33.0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/go.sum b/go.sum index 38a8dc0247e4..6769e5ce74ae 100644 --- a/go.sum +++ b/go.sum @@ -424,8 +424,8 @@ github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da/go.mod h1:cIg4er github.com/golang/protobuf v1.1.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= -github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg= -github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= +github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= +github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= @@ -476,8 +476,8 @@ github.com/hashicorp/terraform-exec v0.20.0 h1:DIZnPsqzPGuUnq6cH8jWcPunBfY+C+M8J github.com/hashicorp/terraform-exec v0.20.0/go.mod h1:ckKGkJWbsNqFKV1itgMnE0hY9IYf1HoiekpuN0eWoDw= github.com/hashicorp/terraform-json v0.21.0 h1:9NQxbLNqPbEMze+S6+YluEdXgJmhQykRyRNd+zTI05U= github.com/hashicorp/terraform-json v0.21.0/go.mod h1:qdeBs11ovMzo5puhrRibdD6d2Dq6TyE/28JiU4tIQxk= -github.com/hashicorp/terraform-plugin-framework v1.7.0 h1:wOULbVmfONnJo9iq7/q+iBOBJul5vRovaYJIu2cY/Pw= -github.com/hashicorp/terraform-plugin-framework v1.7.0/go.mod h1:jY9Id+3KbZ17OMpulgnWLSfwxNVYSoYBQFTgsx044CI= +github.com/hashicorp/terraform-plugin-framework v1.8.0 h1:P07qy8RKLcoBkCrY2RHJer5AEvJnDuXomBgou6fD8kI= +github.com/hashicorp/terraform-plugin-framework v1.8.0/go.mod h1:/CpTukO88PcL/62noU7cuyaSJ4Rsim+A/pa+3rUVufY= github.com/hashicorp/terraform-plugin-framework-jsontypes v0.1.0 h1:b8vZYB/SkXJT4YPbT3trzE6oJ7dPyMy68+9dEDKsJjE= github.com/hashicorp/terraform-plugin-framework-jsontypes v0.1.0/go.mod h1:tP9BC3icoXBz72evMS5UTFvi98CiKhPdXF6yLs1wS8A= github.com/hashicorp/terraform-plugin-framework-timeouts v0.4.1 h1:gm5b1kHgFFhaKFhm4h2TgvMUlNzFAtUqlcOWnWPm+9E= @@ -486,8 +486,8 @@ github.com/hashicorp/terraform-plugin-framework-timetypes v0.3.0 h1:egR4InfakWkg github.com/hashicorp/terraform-plugin-framework-timetypes v0.3.0/go.mod h1:9vjvl36aY1p6KltaA5QCvGC5hdE/9t4YuhGftw6WOgE= github.com/hashicorp/terraform-plugin-framework-validators v0.12.0 h1:HOjBuMbOEzl7snOdOoUfE2Jgeto6JOjLVQ39Ls2nksc= github.com/hashicorp/terraform-plugin-framework-validators v0.12.0/go.mod h1:jfHGE/gzjxYz6XoUwi/aYiiKrJDeutQNUtGQXkaHklg= -github.com/hashicorp/terraform-plugin-go v0.22.1 h1:iTS7WHNVrn7uhe3cojtvWWn83cm2Z6ryIUDTRO0EV7w= -github.com/hashicorp/terraform-plugin-go v0.22.1/go.mod h1:qrjnqRghvQ6KnDbB12XeZ4FluclYwptntoWCr9QaXTI= +github.com/hashicorp/terraform-plugin-go v0.22.2 h1:5o8uveu6eZUf5J7xGPV0eY0TPXg3qpmwX9sce03Bxnc= +github.com/hashicorp/terraform-plugin-go v0.22.2/go.mod h1:drq8Snexp9HsbFZddvyLHN6LuWHHndSQg+gV+FPkcIM= github.com/hashicorp/terraform-plugin-mux v0.15.0 h1:+/+lDx0WUsIOpkAmdwBIoFU8UP9o2eZASoOnLsWbKME= github.com/hashicorp/terraform-plugin-mux v0.15.0/go.mod h1:9ezplb1Dyq394zQ+ldB0nvy/qbNAz3mMoHHseMTMaKo= github.com/hashicorp/terraform-plugin-sdk/v2 v2.33.0 h1:qHprzXy/As0rxedphECBEQAh3R4yp6pKksKHcqZx5G8= @@ -671,10 +671,10 @@ golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8T google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= google.golang.org/appengine v1.6.8 h1:IhEN5q69dyKagZPYMSdIjS2HqprW324FRQZJcGqPAsM= google.golang.org/appengine v1.6.8/go.mod h1:1jJ3jBArFh5pcgW8gCtRJnepW8FzD1V44FJffLiz/Ds= -google.golang.org/genproto/googleapis/rpc v0.0.0-20240123012728-ef4313101c80 h1:AjyfHzEPEFp/NpvfN5g+KDla3EMojjhRVZc1i7cj+oM= -google.golang.org/genproto/googleapis/rpc v0.0.0-20240123012728-ef4313101c80/go.mod h1:PAREbraiVEVGVdTZsVWjSbbTtSyGbAgIIvni8a8CD5s= -google.golang.org/grpc v1.62.1 h1:B4n+nfKzOICUXMgyrNd19h/I9oH0L1pizfk1d4zSgTk= -google.golang.org/grpc v1.62.1/go.mod h1:IWTG0VlJLCh1SkC58F7np9ka9mx/WNkjl4PGJaiq+QE= +google.golang.org/genproto/googleapis/rpc v0.0.0-20240227224415-6ceb2ff114de h1:cZGRis4/ot9uVm639a+rHCUaG0JJHEsdyzSQTMX+suY= +google.golang.org/genproto/googleapis/rpc v0.0.0-20240227224415-6ceb2ff114de/go.mod h1:H4O17MA/PE9BsGx3w+a+W2VOLLD1Qf7oJneAoU6WktY= +google.golang.org/grpc v1.63.2 h1:MUeiw1B2maTVZthpU5xvASfTh3LDbxHd6IJ6QQVU+xM= +google.golang.org/grpc v1.63.2/go.mod h1:WAX/8DgncnokcFUldAxq7GeB5DXHDbMF+lLvDomNkRA= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= google.golang.org/protobuf v1.33.0 h1:uNO2rsAINq/JlFpSdYEKIZ0uKD/R9cpdv0T+yoGwGmI= From 71455b4e331e9fb8673ad0b93decddebf36491a7 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Fri, 19 Apr 2024 16:02:23 -0400 Subject: [PATCH 2/9] internal/framework/types: covert string enum to value validation --- internal/framework/types/string_enum.go | 86 ++++++++------------ internal/framework/types/string_enum_test.go | 42 ++++------ 2 files changed, 53 insertions(+), 75 deletions(-) diff --git a/internal/framework/types/string_enum.go b/internal/framework/types/string_enum.go index a30e32184c67..163de73ba190 100644 --- a/internal/framework/types/string_enum.go +++ b/internal/framework/types/string_enum.go @@ -7,14 +7,11 @@ import ( "context" "fmt" - "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/attr/xattr" "github.com/hashicorp/terraform-plugin-framework/diag" - "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource/schema/defaults" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault" - "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-framework/types/basetypes" "github.com/hashicorp/terraform-plugin-go/tftypes" @@ -29,60 +26,20 @@ func (dummyValueser) Values() []dummyValueser { } var ( - _ xattr.TypeWithValidate = (*stringEnumType[dummyValueser])(nil) - _ basetypes.StringTypable = (*stringEnumType[dummyValueser])(nil) - _ basetypes.StringValuable = (*StringEnum[dummyValueser])(nil) + _ basetypes.StringTypable = (*stringEnumType[dummyValueser])(nil) ) -type customStringTypeWithValidator struct { - basetypes.StringType - validator validator.String -} - -func (t customStringTypeWithValidator) Validate(ctx context.Context, in tftypes.Value, path path.Path) diag.Diagnostics { - var diags diag.Diagnostics - - if in.IsNull() || !in.IsKnown() { - return diags - } - - var value string - err := in.As(&value) - if err != nil { - diags.AddAttributeError( - path, - "Invalid Terraform Value", - "An unexpected error occurred while attempting to convert a Terraform value to a string. "+ - "This generally is an issue with the provider schema implementation. "+ - "Please contact the provider developers.\n\n"+ - "Path: "+path.String()+"\n"+ - "Error: "+err.Error(), - ) - return diags - } - - request := validator.StringRequest{ - ConfigValue: types.StringValue(value), - Path: path, - } - response := validator.StringResponse{} - t.validator.ValidateString(ctx, request, &response) - diags.Append(response.Diagnostics...) - - return diags -} - type stringEnumTypeWithAttributeDefault[T enum.Valueser[T]] interface { basetypes.StringTypable AttributeDefault(T) defaults.String } type stringEnumType[T enum.Valueser[T]] struct { - customStringTypeWithValidator + basetypes.StringType } func StringEnumType[T enum.Valueser[T]]() stringEnumTypeWithAttributeDefault[T] { - return stringEnumType[T]{customStringTypeWithValidator: customStringTypeWithValidator{validator: stringvalidator.OneOf(tfslices.AppendUnique(enum.Values[T](), "")...)}} + return stringEnumType[T]{} } func (t stringEnumType[T]) Equal(o attr.Type) bool { @@ -144,6 +101,15 @@ func (t stringEnumType[T]) AttributeDefault(defaultVal T) defaults.String { return stringdefault.StaticString(string(defaultVal)) } +var ( + _ basetypes.StringValuable = (*StringEnum[dummyValueser])(nil) + _ xattr.ValidateableAttribute = (*StringEnum[dummyValueser])(nil) +) + +type StringEnum[T enum.Valueser[T]] struct { + basetypes.StringValue +} + func StringEnumNull[T enum.Valueser[T]]() StringEnum[T] { return StringEnum[T]{StringValue: basetypes.NewStringNull()} } @@ -156,10 +122,6 @@ func StringEnumValue[T enum.Valueser[T]](value T) StringEnum[T] { return StringEnum[T]{StringValue: basetypes.NewStringValue(string(value))} } -type StringEnum[T enum.Valueser[T]] struct { - basetypes.StringValue -} - func (v StringEnum[T]) Equal(o attr.Value) bool { other, ok := o.(StringEnum[T]) @@ -184,3 +146,27 @@ func (v StringEnum[T]) ValueEnum() T { func (v StringEnum[T]) StringEnumValue(value string) StringEnum[T] { return StringEnum[T]{StringValue: basetypes.NewStringValue(value)} } + +func (v StringEnum[T]) ValidateAttribute(ctx context.Context, req xattr.ValidateAttributeRequest, resp *xattr.ValidateAttributeResponse) { + if v.IsNull() || v.IsUnknown() { + return + } + + vs := v.ValueString() + validValues := tfslices.AppendUnique(v.ValueEnum().Values(), "") + + for _, enumVal := range validValues { + if vs == string(enumVal) { + return + } + } + + resp.Diagnostics.AddAttributeError( + req.Path, + "Invalid String Enum Value", + "The provided value does not match any valid values.\n\n"+ + "Path: "+req.Path.String()+"\n"+ + "Given Value: "+vs+"\n"+ + "Valid Values: "+fmt.Sprintf("%s", validValues), + ) +} diff --git a/internal/framework/types/string_enum_test.go b/internal/framework/types/string_enum_test.go index 105a8f0ab8cd..88dd2fcba020 100644 --- a/internal/framework/types/string_enum_test.go +++ b/internal/framework/types/string_enum_test.go @@ -11,7 +11,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/attr/xattr" - "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-go/tftypes" fwtypes "github.com/hashicorp/terraform-provider-aws/internal/framework/types" ) @@ -56,32 +55,27 @@ func TestStringEnumTypeValueFromTerraform(t *testing.T) { } } -func TestStringEnumTypeValidate(t *testing.T) { +func TestStringEnumValidateAttribute(t *testing.T) { t.Parallel() - type testCase struct { - val tftypes.Value + tests := map[string]struct { + val fwtypes.StringEnum[awstypes.AclPermission] expectError bool - } - tests := map[string]testCase{ - "not a string": { - val: tftypes.NewValue(tftypes.Bool, true), - expectError: true, + }{ + "null value": { + val: fwtypes.StringEnumNull[awstypes.AclPermission](), }, - "unknown string": { - val: tftypes.NewValue(tftypes.String, tftypes.UnknownValue), + "unknown value": { + val: fwtypes.StringEnumUnknown[awstypes.AclPermission](), }, - "null string": { - val: tftypes.NewValue(tftypes.String, nil), + "zero value enum": { + val: fwtypes.StringEnumValue(awstypes.AclPermission("")), }, "valid enum": { - val: tftypes.NewValue(tftypes.String, string(awstypes.AclPermissionWrite)), - }, - "valid zero value": { - val: tftypes.NewValue(tftypes.String, ""), + val: fwtypes.StringEnumValue(awstypes.AclPermissionRead), }, "invalid enum": { - val: tftypes.NewValue(tftypes.String, "LIST"), + val: fwtypes.StringEnumValue(awstypes.AclPermission("invalid")), expectError: true, }, } @@ -93,14 +87,12 @@ func TestStringEnumTypeValidate(t *testing.T) { ctx := context.Background() - diags := fwtypes.StringEnumType[awstypes.AclPermission]().(xattr.TypeWithValidate).Validate(ctx, test.val, path.Root("test")) - - if !diags.HasError() && test.expectError { - t.Fatal("expected error, got no error") - } + req := xattr.ValidateAttributeRequest{} + resp := xattr.ValidateAttributeResponse{} - if diags.HasError() && !test.expectError { - t.Fatalf("got unexpected error: %#v", diags) + test.val.ValidateAttribute(ctx, req, &resp) + if resp.Diagnostics.HasError() != test.expectError { + t.Errorf("resp.Diagnostics.HasError() = %t, want = %t", resp.Diagnostics.HasError(), test.expectError) } }) } From d0595d91ed12d3415474a6e4e0a4ee37582faf08 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Mon, 22 Apr 2024 10:29:01 -0400 Subject: [PATCH 3/9] internal/framework/types: convert arn to value validation --- internal/framework/types/arn.go | 54 +++++++++++----------------- internal/framework/types/arn_test.go | 43 ++++++++++------------ 2 files changed, 40 insertions(+), 57 deletions(-) diff --git a/internal/framework/types/arn.go b/internal/framework/types/arn.go index 942f09b00dcc..383ccf7791c2 100644 --- a/internal/framework/types/arn.go +++ b/internal/framework/types/arn.go @@ -11,7 +11,6 @@ import ( "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/attr/xattr" "github.com/hashicorp/terraform-plugin-framework/diag" - "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-framework/types/basetypes" "github.com/hashicorp/terraform-plugin-go/tftypes" @@ -19,9 +18,7 @@ import ( ) var ( - _ xattr.TypeWithValidate = (*arnType)(nil) - _ basetypes.StringTypable = (*arnType)(nil) - _ basetypes.StringValuable = (*ARN)(nil) + _ basetypes.StringTypable = (*arnType)(nil) ) type arnType struct { @@ -90,35 +87,10 @@ func (arnType) ValueType(context.Context) attr.Value { return ARN{} } -func (t arnType) Validate(ctx context.Context, in tftypes.Value, path path.Path) diag.Diagnostics { - var diags diag.Diagnostics - - if !in.IsKnown() || in.IsNull() { - return diags - } - - var value string - err := in.As(&value) - if err != nil { - diags.AddAttributeError( - path, - "ARN Type Validation Error", - ProviderErrorDetailPrefix+fmt.Sprintf("Cannot convert value to string: %s", err), - ) - return diags - } - - if !arn.IsARN(value) { - diags.AddAttributeError( - path, - "ARN Type Validation Error", - fmt.Sprintf("Value %q cannot be parsed as an ARN.", value), - ) - return diags - } - - return diags -} +var ( + _ basetypes.StringValuable = (*ARN)(nil) + _ xattr.ValidateableAttribute = (*ARN)(nil) +) func ARNNull() ARN { return ARN{StringValue: basetypes.NewStringNull()} @@ -170,3 +142,19 @@ func (ARN) Type(context.Context) attr.Type { func (v ARN) ValueARN() arn.ARN { return v.value } + +func (v ARN) ValidateAttribute(ctx context.Context, req xattr.ValidateAttributeRequest, resp *xattr.ValidateAttributeResponse) { + if v.IsNull() || v.IsUnknown() { + return + } + + if !arn.IsARN(v.ValueString()) { + resp.Diagnostics.AddAttributeError( + req.Path, + "Invalid ARN Value", + "The provided value cannot be parsed as an ARN.\n\n"+ + "Path: "+req.Path.String()+"\n"+ + "Value: "+v.ValueString(), + ) + } +} diff --git a/internal/framework/types/arn_test.go b/internal/framework/types/arn_test.go index 80c9eac13c51..3b009e1b121d 100644 --- a/internal/framework/types/arn_test.go +++ b/internal/framework/types/arn_test.go @@ -9,7 +9,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/hashicorp/terraform-plugin-framework/attr" - "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/attr/xattr" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-go/tftypes" fwtypes "github.com/hashicorp/terraform-provider-aws/internal/framework/types" @@ -59,31 +59,28 @@ func TestARNTypeValueFromTerraform(t *testing.T) { } } -func TestARNTypeValidate(t *testing.T) { +func TestARNValidateAttribute(t *testing.T) { t.Parallel() type testCase struct { - val tftypes.Value + val fwtypes.ARN expectError bool } tests := map[string]testCase{ - "not a string": { - val: tftypes.NewValue(tftypes.Bool, true), - expectError: true, - }, - "unknown string": { - val: tftypes.NewValue(tftypes.String, tftypes.UnknownValue), - }, - "null string": { - val: tftypes.NewValue(tftypes.String, nil), + "null value": { + val: fwtypes.ARNNull(), }, - "valid string": { - val: tftypes.NewValue(tftypes.String, "arn:aws:rds:us-east-1:123456789012:db:test"), // lintignore:AWSAT003,AWSAT005 + "unknown value": { + val: fwtypes.ARNUnknown(), }, - "invalid string": { - val: tftypes.NewValue(tftypes.String, "not ok"), - expectError: true, + "valid arn": { + val: fwtypes.ARNValueMust("arn:aws:rds:us-east-1:123456789012:db:test"), // lintignore:AWSAT003,AWSAT005 }, + // TODO: merging of ARNValue/ARNValueMust needs to happen first + // "invalid arn": { + // val: fwtypes.ARNValueMust("not ok"), // lintignore:AWSAT003,AWSAT005 + // expectError: true, + // }, } for name, test := range tests { @@ -93,14 +90,12 @@ func TestARNTypeValidate(t *testing.T) { ctx := context.Background() - diags := fwtypes.ARNType.Validate(ctx, test.val, path.Root("test")) - - if !diags.HasError() && test.expectError { - t.Fatal("expected error, got no error") - } + req := xattr.ValidateAttributeRequest{} + resp := xattr.ValidateAttributeResponse{} - if diags.HasError() && !test.expectError { - t.Fatalf("got unexpected error: %#v", diags) + test.val.ValidateAttribute(ctx, req, &resp) + if resp.Diagnostics.HasError() != test.expectError { + t.Errorf("resp.Diagnostics.HasError() = %t, want = %t", resp.Diagnostics.HasError(), test.expectError) } }) } From 9516ce98b4864d7336d266b0ab2d0707fe239db2 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Mon, 22 Apr 2024 11:16:52 -0400 Subject: [PATCH 4/9] internal/framework/types: remove ARNValueMust function Initialization of a known `ARN` type now can only be done via the `ARNValue` function. The `ARNValueMust` function has been removed, and validation previously applied during initialization has moved into the `ValidateAttribute` method. This change has minimal impact to most existing usages of the initialization functions. In the cases where the diagnostics response from `ARNValue` was being explicitly handled, the `arn.Parse` function has been inserted instead, providing the same effect the init-time validation previously supplied. --- internal/framework/flex/auto_expand_test.go | 4 +-- internal/framework/flex/auto_flatten_test.go | 4 +-- internal/framework/flex/string_test.go | 4 +-- internal/framework/types/arn.go | 30 +++++++++---------- internal/framework/types/arn_test.go | 13 ++++---- ...ault_auto_scaling_configuration_version.go | 2 +- internal/service/bedrock/custom_model.go | 2 +- .../bedrockagent/agent_action_group.go | 2 +- .../service/cloudfrontkeyvaluestore/key.go | 5 ++-- internal/service/dynamodb/resource_policy.go | 5 ++-- internal/service/kinesis/resource_policy.go | 5 ++-- .../redshift/data_share_authorization.go | 2 +- .../data_share_consumer_association.go | 4 +-- .../application_layer_automatic_response.go | 5 ++-- .../ssoadmin/application_access_scope.go | 2 +- 15 files changed, 45 insertions(+), 44 deletions(-) diff --git a/internal/framework/flex/auto_expand_test.go b/internal/framework/flex/auto_expand_test.go index 3d59ff20f6a5..94805690e5a7 100644 --- a/internal/framework/flex/auto_expand_test.go +++ b/internal/framework/flex/auto_expand_test.go @@ -259,13 +259,13 @@ func TestExpand(t *testing.T) { }, { TestName: "single ARN Source and single string Target", - Source: &TestFlexTF17{Field1: fwtypes.ARNValueMust(testARN)}, + Source: &TestFlexTF17{Field1: fwtypes.ARNValue(testARN)}, Target: &TestFlexAWS01{}, WantTarget: &TestFlexAWS01{Field1: testARN}, }, { TestName: "single ARN Source and single *string Target", - Source: &TestFlexTF17{Field1: fwtypes.ARNValueMust(testARN)}, + Source: &TestFlexTF17{Field1: fwtypes.ARNValue(testARN)}, Target: &TestFlexAWS02{}, WantTarget: &TestFlexAWS02{Field1: aws.String(testARN)}, }, diff --git a/internal/framework/flex/auto_flatten_test.go b/internal/framework/flex/auto_flatten_test.go index b7adfc96fae4..819ab7aca157 100644 --- a/internal/framework/flex/auto_flatten_test.go +++ b/internal/framework/flex/auto_flatten_test.go @@ -376,13 +376,13 @@ func TestFlatten(t *testing.T) { TestName: "single string Source and single ARN Target", Source: &TestFlexAWS01{Field1: testARN}, Target: &TestFlexTF17{}, - WantTarget: &TestFlexTF17{Field1: fwtypes.ARNValueMust(testARN)}, + WantTarget: &TestFlexTF17{Field1: fwtypes.ARNValue(testARN)}, }, { TestName: "single *string Source and single ARN Target", Source: &TestFlexAWS02{Field1: aws.String(testARN)}, Target: &TestFlexTF17{}, - WantTarget: &TestFlexTF17{Field1: fwtypes.ARNValueMust(testARN)}, + WantTarget: &TestFlexTF17{Field1: fwtypes.ARNValue(testARN)}, }, { TestName: "single nil *string Source and single ARN Target", diff --git a/internal/framework/flex/string_test.go b/internal/framework/flex/string_test.go index 9c027e937fa2..2176437cf86e 100644 --- a/internal/framework/flex/string_test.go +++ b/internal/framework/flex/string_test.go @@ -208,7 +208,7 @@ func TestARNStringFromFramework(t *testing.T) { } tests := map[string]testCase{ "valid ARN": { - input: fwtypes.ARNValueMust("arn:aws:iam::123456789012:user/David"), + input: fwtypes.ARNValue("arn:aws:iam::123456789012:user/David"), expected: aws.String("arn:aws:iam::123456789012:user/David"), }, "null ARN": { @@ -245,7 +245,7 @@ func TestStringToFrameworkARN(t *testing.T) { tests := map[string]testCase{ "valid ARN": { input: aws.String("arn:aws:iam::123456789012:user/David"), - expected: fwtypes.ARNValueMust("arn:aws:iam::123456789012:user/David"), + expected: fwtypes.ARNValue("arn:aws:iam::123456789012:user/David"), }, "null ARN": { input: nil, diff --git a/internal/framework/types/arn.go b/internal/framework/types/arn.go index 383ccf7791c2..f9b48c706876 100644 --- a/internal/framework/types/arn.go +++ b/internal/framework/types/arn.go @@ -14,7 +14,6 @@ import ( "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-framework/types/basetypes" "github.com/hashicorp/terraform-plugin-go/tftypes" - "github.com/hashicorp/terraform-provider-aws/internal/errs/fwdiag" ) var ( @@ -58,7 +57,7 @@ func (t arnType) ValueFromString(_ context.Context, in types.String) (basetypes. return ARNUnknown(), diags // Must not return validation errors. } - return ARNValueMust(valueString), diags + return ARNValue(valueString), diags } func (t arnType) ValueFromTerraform(ctx context.Context, in tftypes.Value) (attr.Value, error) { @@ -100,23 +99,22 @@ func ARNUnknown() ARN { return ARN{StringValue: basetypes.NewStringUnknown()} } -func ARNValue(value string) (ARN, diag.Diagnostics) { - var diags diag.Diagnostics - - v, err := arn.Parse(value) - if err != nil { - diags.Append(diag.NewErrorDiagnostic("Invalid ARN", err.Error())) - return ARNUnknown(), diags - } +// ARNValue initializes a new ARN type with the provided value +// +// This function does not return diagnostics, and therefore invalid ARN values +// are not handled during construction. Invalid values will be detected by the +// ValidateAttribute method, called by the ValidateResourceConfig RPC during +// operations like `terraform validate`, `plan`, or `apply`. +func ARNValue(value string) ARN { + // swallow any ARN parsing errors here and just pass along the + // zero value arn.ARN. Invalid values will be handled downstream + // by the ValidateAttribute method. + v, _ := arn.Parse(value) return ARN{ StringValue: basetypes.NewStringValue(value), value: v, - }, diags -} - -func ARNValueMust(value string) ARN { - return fwdiag.Must(ARNValue(value)) + } } type ARN struct { @@ -138,7 +136,7 @@ func (ARN) Type(context.Context) attr.Type { return ARNType } -// ValueARN returns the known arn.ARN value. If ARN is null or unknown, returns {}. +// ValueARN returns the known arn.ARN value. If ARN is null, unknown, or invalid returns ARN{}. func (v ARN) ValueARN() arn.ARN { return v.value } diff --git a/internal/framework/types/arn_test.go b/internal/framework/types/arn_test.go index 3b009e1b121d..148981e46f40 100644 --- a/internal/framework/types/arn_test.go +++ b/internal/framework/types/arn_test.go @@ -32,7 +32,7 @@ func TestARNTypeValueFromTerraform(t *testing.T) { }, "valid ARN": { val: tftypes.NewValue(tftypes.String, "arn:aws:rds:us-east-1:123456789012:db:test"), // lintignore:AWSAT003,AWSAT005 - expected: fwtypes.ARNValueMust("arn:aws:rds:us-east-1:123456789012:db:test"), // lintignore:AWSAT003,AWSAT005 + expected: fwtypes.ARNValue("arn:aws:rds:us-east-1:123456789012:db:test"), // lintignore:AWSAT003,AWSAT005 }, "invalid ARN": { val: tftypes.NewValue(tftypes.String, "not ok"), @@ -74,13 +74,12 @@ func TestARNValidateAttribute(t *testing.T) { val: fwtypes.ARNUnknown(), }, "valid arn": { - val: fwtypes.ARNValueMust("arn:aws:rds:us-east-1:123456789012:db:test"), // lintignore:AWSAT003,AWSAT005 + val: fwtypes.ARNValue("arn:aws:rds:us-east-1:123456789012:db:test"), // lintignore:AWSAT003,AWSAT005 + }, + "invalid arn": { + val: fwtypes.ARNValue("not ok"), // lintignore:AWSAT003,AWSAT005 + expectError: true, }, - // TODO: merging of ARNValue/ARNValueMust needs to happen first - // "invalid arn": { - // val: fwtypes.ARNValueMust("not ok"), // lintignore:AWSAT003,AWSAT005 - // expectError: true, - // }, } for name, test := range tests { diff --git a/internal/service/apprunner/default_auto_scaling_configuration_version.go b/internal/service/apprunner/default_auto_scaling_configuration_version.go index d6b4258cc6f0..7e296b2275e3 100644 --- a/internal/service/apprunner/default_auto_scaling_configuration_version.go +++ b/internal/service/apprunner/default_auto_scaling_configuration_version.go @@ -98,7 +98,7 @@ func (r *defaultAutoScalingConfigurationVersionResource) Read(ctx context.Contex return } - data.AutoScalingConfigurationARN = fwtypes.ARNValueMust(aws.ToString(output.AutoScalingConfigurationArn)) + data.AutoScalingConfigurationARN = fwtypes.ARNValue(aws.ToString(output.AutoScalingConfigurationArn)) response.Diagnostics.Append(response.State.Set(ctx, &data)...) } diff --git a/internal/service/bedrock/custom_model.go b/internal/service/bedrock/custom_model.go index 456bce8a23c4..88557aa20fc9 100644 --- a/internal/service/bedrock/custom_model.go +++ b/internal/service/bedrock/custom_model.go @@ -379,7 +379,7 @@ func (r *customModelResource) Read(ctx context.Context, request resource.ReadReq if len(strings.SplitN(old.Resource, ":", 2)) == 1 { // Old ARN doesn't contain the model version and parameter count. new.Resource = strings.SplitN(new.Resource, ":", 2)[0] - data.BaseModelIdentifier = fwtypes.ARNValueMust(new.String()) + data.BaseModelIdentifier = fwtypes.ARNValue(new.String()) } } } diff --git a/internal/service/bedrockagent/agent_action_group.go b/internal/service/bedrockagent/agent_action_group.go index e73eddc96210..854bbe26d508 100644 --- a/internal/service/bedrockagent/agent_action_group.go +++ b/internal/service/bedrockagent/agent_action_group.go @@ -439,7 +439,7 @@ func flattenActionGroupExecutor(ctx context.Context, apiObject awstypes.ActionGr switch v := apiObject.(type) { case *awstypes.ActionGroupExecutorMemberLambda: - actionGroupExecutorData.Lambda = fwtypes.ARNValueMust(v.Value) + actionGroupExecutorData.Lambda = fwtypes.ARNValue(v.Value) } return fwtypes.NewListNestedObjectValueOfPtrMust(ctx, &actionGroupExecutorData) diff --git a/internal/service/cloudfrontkeyvaluestore/key.go b/internal/service/cloudfrontkeyvaluestore/key.go index 64d25ccb551b..efc24e8ac07d 100644 --- a/internal/service/cloudfrontkeyvaluestore/key.go +++ b/internal/service/cloudfrontkeyvaluestore/key.go @@ -8,6 +8,7 @@ import ( "fmt" "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/aws/arn" "github.com/aws/aws-sdk-go-v2/service/cloudfrontkeyvaluestore" awstypes "github.com/aws/aws-sdk-go-v2/service/cloudfrontkeyvaluestore/types" "github.com/hashicorp/terraform-plugin-framework/resource" @@ -328,12 +329,12 @@ func (data *keyResourceModel) InitFromID() error { return err } - v, err := fwdiag.AsError(fwtypes.ARNValue(parts[0])) + _, err = arn.Parse(parts[0]) if err != nil { return err } - data.KvsARN = v + data.KvsARN = fwtypes.ARNValue(parts[0]) data.Key = types.StringValue(parts[1]) return nil diff --git a/internal/service/dynamodb/resource_policy.go b/internal/service/dynamodb/resource_policy.go index 4c1e1cd0f584..29377c97a326 100644 --- a/internal/service/dynamodb/resource_policy.go +++ b/internal/service/dynamodb/resource_policy.go @@ -8,6 +8,7 @@ import ( "fmt" "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/aws/arn" "github.com/aws/aws-sdk-go-v2/service/dynamodb" awstypes "github.com/aws/aws-sdk-go-v2/service/dynamodb/types" "github.com/hashicorp/terraform-plugin-framework/resource" @@ -239,12 +240,12 @@ type resourcePolicyResourceModel struct { } func (data *resourcePolicyResourceModel) InitFromID() error { - v, err := fwdiag.AsError(fwtypes.ARNValue(data.ID.ValueString())) + _, err := arn.Parse(data.ID.ValueString()) if err != nil { return err } - data.ResourceARN = v + data.ResourceARN = fwtypes.ARNValue(data.ID.ValueString()) return nil } diff --git a/internal/service/kinesis/resource_policy.go b/internal/service/kinesis/resource_policy.go index 006d0388d7e1..47b200933832 100644 --- a/internal/service/kinesis/resource_policy.go +++ b/internal/service/kinesis/resource_policy.go @@ -8,6 +8,7 @@ import ( "fmt" "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/aws/arn" "github.com/aws/aws-sdk-go-v2/service/kinesis" awstypes "github.com/aws/aws-sdk-go-v2/service/kinesis/types" "github.com/hashicorp/terraform-plugin-framework/resource" @@ -218,12 +219,12 @@ type resourcePolicyResourceModel struct { } func (data *resourcePolicyResourceModel) InitFromID() error { - v, err := fwdiag.AsError(fwtypes.ARNValue(data.ID.ValueString())) + _, err := arn.Parse(data.ID.ValueString()) if err != nil { return err } - data.ResourceARN = v + data.ResourceARN = fwtypes.ARNValue(data.ID.ValueString()) return nil } diff --git a/internal/service/redshift/data_share_authorization.go b/internal/service/redshift/data_share_authorization.go index 246b730ff4ef..cf5ffa6664f6 100644 --- a/internal/service/redshift/data_share_authorization.go +++ b/internal/service/redshift/data_share_authorization.go @@ -161,7 +161,7 @@ func (r *resourceDataShareAuthorization) Read(ctx context.Context, req resource. return } // split ID and write constituent parts to state to support import - state.DataShareARN = fwtypes.ARNValueMust(parts[0]) + state.DataShareARN = fwtypes.ARNValue(parts[0]) state.ConsumerIdentifier = types.StringValue(parts[1]) out, err := findDataShareAuthorizationByID(ctx, conn, state.ID.ValueString()) diff --git a/internal/service/redshift/data_share_consumer_association.go b/internal/service/redshift/data_share_consumer_association.go index fbe5e8e30b6e..a5d9dc4db74d 100644 --- a/internal/service/redshift/data_share_consumer_association.go +++ b/internal/service/redshift/data_share_consumer_association.go @@ -191,12 +191,12 @@ func (r *resourceDataShareConsumerAssociation) Read(ctx context.Context, req res return } // split ID and write constituent parts to state to support import - state.DataShareARN = fwtypes.ARNValueMust(parts[0]) + state.DataShareARN = fwtypes.ARNValue(parts[0]) if parts[1] != "" { state.AssociateEntireAccount = types.BoolValue(parts[1] == "true") } if parts[2] != "" { - state.ConsumerARN = fwtypes.ARNValueMust(parts[2]) + state.ConsumerARN = fwtypes.ARNValue(parts[2]) } if parts[3] != "" { state.ConsumerRegion = types.StringValue(parts[3]) diff --git a/internal/service/shield/application_layer_automatic_response.go b/internal/service/shield/application_layer_automatic_response.go index 84e0c0760b01..c9e04f81bbac 100644 --- a/internal/service/shield/application_layer_automatic_response.go +++ b/internal/service/shield/application_layer_automatic_response.go @@ -9,6 +9,7 @@ import ( "time" "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/aws/arn" "github.com/aws/aws-sdk-go-v2/service/shield" awstypes "github.com/aws/aws-sdk-go-v2/service/shield/types" "github.com/hashicorp/terraform-plugin-framework-timeouts/resource/timeouts" @@ -331,12 +332,12 @@ type applicationLayerAutomaticResponseResourceModel struct { } func (data *applicationLayerAutomaticResponseResourceModel) InitFromID() error { - v, err := fwdiag.AsError(fwtypes.ARNValue(data.ID.ValueString())) + _, err := arn.Parse(data.ID.ValueString()) if err != nil { return err } - data.ResourceARN = v + data.ResourceARN = fwtypes.ARNValue(data.ID.ValueString()) return nil } diff --git a/internal/service/ssoadmin/application_access_scope.go b/internal/service/ssoadmin/application_access_scope.go index 57fc09e0d0d6..e0b2a3f233c0 100644 --- a/internal/service/ssoadmin/application_access_scope.go +++ b/internal/service/ssoadmin/application_access_scope.go @@ -160,7 +160,7 @@ func (r *resourceApplicationAccessScope) Read(ctx context.Context, req resource. return } - state.ApplicationARN = fwtypes.ARNValueMust(parts[0]) + state.ApplicationARN = fwtypes.ARNValue(parts[0]) state.AuthorizedTargets = flex.FlattenFrameworkStringValueList(ctx, out.AuthorizedTargets) state.Scope = flex.StringToFramework(ctx, out.Scope) From 15c8fd73ffa9827c018108523ab7f1e522ac09b7 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Mon, 22 Apr 2024 13:50:31 -0400 Subject: [PATCH 5/9] internal/framework/types: convert cidr block to value validation --- internal/framework/types/cidr_block.go | 54 ++++++++------------- internal/framework/types/cidr_block_test.go | 46 ++++++++---------- 2 files changed, 41 insertions(+), 59 deletions(-) diff --git a/internal/framework/types/cidr_block.go b/internal/framework/types/cidr_block.go index c8c4ede09dfb..976dc6557d22 100644 --- a/internal/framework/types/cidr_block.go +++ b/internal/framework/types/cidr_block.go @@ -10,7 +10,6 @@ import ( "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/attr/xattr" "github.com/hashicorp/terraform-plugin-framework/diag" - "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-framework/types/basetypes" "github.com/hashicorp/terraform-plugin-go/tftypes" @@ -18,9 +17,7 @@ import ( ) var ( - _ xattr.TypeWithValidate = (*cidrBlockType)(nil) - _ basetypes.StringTypable = (*cidrBlockType)(nil) - _ basetypes.StringValuable = (*CIDRBlock)(nil) + _ basetypes.StringTypable = (*cidrBlockType)(nil) ) type cidrBlockType struct { @@ -89,35 +86,10 @@ func (cidrBlockType) ValueType(context.Context) attr.Value { return CIDRBlock{} } -func (t cidrBlockType) Validate(ctx context.Context, in tftypes.Value, path path.Path) diag.Diagnostics { - var diags diag.Diagnostics - - if !in.IsKnown() || in.IsNull() { - return diags - } - - var value string - err := in.As(&value) - if err != nil { - diags.AddAttributeError( - path, - "CIDRBlock Type Validation Error", - ProviderErrorDetailPrefix+fmt.Sprintf("Cannot convert value to string: %s", err), - ) - return diags - } - - if err := itypes.ValidateCIDRBlock(value); err != nil { - diags.AddAttributeError( - path, - "CIDRBlock Type Validation Error", - err.Error(), - ) - return diags - } - - return diags -} +var ( + _ basetypes.StringValuable = (*CIDRBlock)(nil) + _ xattr.ValidateableAttribute = (*CIDRBlock)(nil) +) func CIDRBlockNull() CIDRBlock { return CIDRBlock{StringValue: basetypes.NewStringNull()} @@ -148,3 +120,19 @@ func (v CIDRBlock) Equal(o attr.Value) bool { func (CIDRBlock) Type(context.Context) attr.Type { return CIDRBlockType } + +func (v CIDRBlock) ValidateAttribute(ctx context.Context, req xattr.ValidateAttributeRequest, resp *xattr.ValidateAttributeResponse) { + if v.IsNull() || v.IsUnknown() { + return + } + + if err := itypes.ValidateCIDRBlock(v.ValueString()); err != nil { + resp.Diagnostics.AddAttributeError( + req.Path, + "Invalid CIDR Block Value", + "The provided value failed validation.\n\n"+ + "Path: "+req.Path.String()+"\n"+ + "Error: "+err.Error(), + ) + } +} diff --git a/internal/framework/types/cidr_block_test.go b/internal/framework/types/cidr_block_test.go index a14af0119595..a0b44610f7db 100644 --- a/internal/framework/types/cidr_block_test.go +++ b/internal/framework/types/cidr_block_test.go @@ -9,7 +9,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/hashicorp/terraform-plugin-framework/attr" - "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/attr/xattr" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-go/tftypes" fwtypes "github.com/hashicorp/terraform-provider-aws/internal/framework/types" @@ -59,36 +59,32 @@ func TestCIDRBlockTypeValueFromTerraform(t *testing.T) { } } -func TestCIDRBlockTypeValidate(t *testing.T) { +func TestCIDRBlockValidateAttrbute(t *testing.T) { t.Parallel() type testCase struct { - val tftypes.Value + val fwtypes.CIDRBlock expectError bool } tests := map[string]testCase{ - "not a string": { - val: tftypes.NewValue(tftypes.Bool, true), - expectError: true, - }, - "unknown string": { - val: tftypes.NewValue(tftypes.String, tftypes.UnknownValue), + "unknown": { + val: fwtypes.CIDRBlockUnknown(), }, - "null string": { - val: tftypes.NewValue(tftypes.String, nil), + "null": { + val: fwtypes.CIDRBlockNull(), }, - "valid IPv4 string": { - val: tftypes.NewValue(tftypes.String, "10.2.2.0/24"), + "valid IPv4": { + val: fwtypes.CIDRBlockValue("10.2.2.0/24"), }, - "invalid IPv4 string": { - val: tftypes.NewValue(tftypes.String, "10.2.2.2/24"), + "invalid IPv4": { + val: fwtypes.CIDRBlockValue("10.2.2.2/24"), expectError: true, }, - "valid IPv6 string": { - val: tftypes.NewValue(tftypes.String, "2000::/15"), + "valid IPv6": { + val: fwtypes.CIDRBlockValue("2000::/15"), }, - "invalid IPv6 string": { - val: tftypes.NewValue(tftypes.String, "2001::/15"), + "invalid IPv6": { + val: fwtypes.CIDRBlockValue("2001::/15"), expectError: true, }, } @@ -100,14 +96,12 @@ func TestCIDRBlockTypeValidate(t *testing.T) { ctx := context.Background() - diags := fwtypes.CIDRBlockType.Validate(ctx, test.val, path.Root("test")) - - if !diags.HasError() && test.expectError { - t.Fatal("expected error, got no error") - } + req := xattr.ValidateAttributeRequest{} + resp := xattr.ValidateAttributeResponse{} - if diags.HasError() && !test.expectError { - t.Fatalf("got unexpected error: %#v", diags) + test.val.ValidateAttribute(ctx, req, &resp) + if resp.Diagnostics.HasError() != test.expectError { + t.Errorf("resp.Diagnostics.HasError() = %t, want = %t", resp.Diagnostics.HasError(), test.expectError) } }) } From b9f0afe7be852aa2f13554e196cb23797a76a988 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Mon, 22 Apr 2024 14:23:35 -0400 Subject: [PATCH 6/9] internal/framework/types: convert duration to value validation --- internal/framework/types/duration.go | 72 +++++++++++------------ internal/framework/types/duration_test.go | 42 ++++++------- 2 files changed, 53 insertions(+), 61 deletions(-) diff --git a/internal/framework/types/duration.go b/internal/framework/types/duration.go index 4dcf236d77ff..2fcc47aa9e8e 100644 --- a/internal/framework/types/duration.go +++ b/internal/framework/types/duration.go @@ -11,17 +11,13 @@ import ( "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/attr/xattr" "github.com/hashicorp/terraform-plugin-framework/diag" - "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-framework/types/basetypes" "github.com/hashicorp/terraform-plugin-go/tftypes" - "github.com/hashicorp/terraform-provider-aws/internal/errs" ) var ( - _ xattr.TypeWithValidate = (*durationType)(nil) - _ basetypes.StringTypable = (*durationType)(nil) - _ basetypes.StringValuable = (*Duration)(nil) + _ basetypes.StringTypable = (*durationType)(nil) ) type durationType struct { @@ -61,7 +57,7 @@ func (t durationType) ValueFromString(_ context.Context, in types.String) (baset return DurationUnknown(), diags // Must not return validation errors } - return DurationValueMust(valueString), diags + return DurationValue(valueString), diags } func (t durationType) ValueFromTerraform(ctx context.Context, in tftypes.Value) (attr.Value, error) { @@ -90,35 +86,10 @@ func (durationType) ValueType(context.Context) attr.Value { return Duration{} } -func (t durationType) Validate(ctx context.Context, in tftypes.Value, path path.Path) diag.Diagnostics { - var diags diag.Diagnostics - - if !in.IsKnown() || in.IsNull() { - return diags - } - - var value string - err := in.As(&value) - if err != nil { - diags.AddAttributeError( - path, - "Duration Type Validation Error", - ProviderErrorDetailPrefix+fmt.Sprintf("Cannot convert value to string: %s", err), - ) - return diags - } - - if _, err = time.ParseDuration(value); err != nil { - diags.AddAttributeError( - path, - "Duration Type Validation Error", - fmt.Sprintf("Value %q cannot be parsed as a Duration.", value), - ) - return diags - } - - return diags -} +var ( + _ basetypes.StringValuable = (*Duration)(nil) + _ xattr.ValidateableAttribute = (*Duration)(nil) +) func DurationNull() Duration { return Duration{StringValue: basetypes.NewStringNull()} @@ -128,10 +99,21 @@ func DurationUnknown() Duration { return Duration{StringValue: basetypes.NewStringUnknown()} } -func DurationValueMust(value string) Duration { +// DurationValue initializes a new Duration type with the provided value +// +// This function does not return diagnostics, and therefore invalid duration values +// are not handled during construction. Invalid values will be detected by the +// ValidateAttribute method, called by the ValidateResourceConfig RPC during +// operations like `terraform validate`, `plan`, or `apply`. +func DurationValue(value string) Duration { + // swallow any Duration parsing errors here and just pass along the + // zero value time.Duration. Invalid values will be handled downstream + // by the ValidateAttribute method. + v, _ := time.ParseDuration(value) + return Duration{ StringValue: basetypes.NewStringValue(value), - value: errs.Must(time.ParseDuration(value)), + value: v, } } @@ -158,3 +140,19 @@ func (Duration) Type(context.Context) attr.Type { func (v Duration) ValueDuration() time.Duration { return v.value } + +func (v Duration) ValidateAttribute(ctx context.Context, req xattr.ValidateAttributeRequest, resp *xattr.ValidateAttributeResponse) { + if v.IsNull() || v.IsUnknown() { + return + } + + if _, err := time.ParseDuration(v.ValueString()); err != nil { + resp.Diagnostics.AddAttributeError( + req.Path, + "Invalid Duration Value", + "The provided value cannot be parsed as a Duration.\n\n"+ + "Path: "+req.Path.String()+"\n"+ + "Error: "+err.Error(), + ) + } +} diff --git a/internal/framework/types/duration_test.go b/internal/framework/types/duration_test.go index 00407a748889..7fa3d636f254 100644 --- a/internal/framework/types/duration_test.go +++ b/internal/framework/types/duration_test.go @@ -9,7 +9,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/hashicorp/terraform-plugin-framework/attr" - "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/attr/xattr" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-go/tftypes" fwtypes "github.com/hashicorp/terraform-provider-aws/internal/framework/types" @@ -32,7 +32,7 @@ func TestDurationTypeValueFromTerraform(t *testing.T) { }, "valid duration": { val: tftypes.NewValue(tftypes.String, "2h"), - expected: fwtypes.DurationValueMust("2h"), + expected: fwtypes.DurationValue("2h"), }, "invalid duration": { val: tftypes.NewValue(tftypes.String, "not ok"), @@ -59,29 +59,25 @@ func TestDurationTypeValueFromTerraform(t *testing.T) { } } -func TestDurationTypeValidate(t *testing.T) { +func TestDurationValidateAttribute(t *testing.T) { t.Parallel() type testCase struct { - val tftypes.Value + val fwtypes.Duration expectError bool } tests := map[string]testCase{ - "not a string": { - val: tftypes.NewValue(tftypes.Bool, true), - expectError: true, - }, - "unknown string": { - val: tftypes.NewValue(tftypes.String, tftypes.UnknownValue), + "unknown": { + val: fwtypes.DurationUnknown(), }, - "null string": { - val: tftypes.NewValue(tftypes.String, nil), + "null": { + val: fwtypes.DurationNull(), }, - "valid string": { - val: tftypes.NewValue(tftypes.String, "2h"), + "valid": { + val: fwtypes.DurationValue("2h"), }, - "invalid string": { - val: tftypes.NewValue(tftypes.String, "not ok"), + "invalid": { + val: fwtypes.DurationValue("not ok"), expectError: true, }, } @@ -93,14 +89,12 @@ func TestDurationTypeValidate(t *testing.T) { ctx := context.Background() - diags := fwtypes.DurationType.Validate(ctx, test.val, path.Root("test")) - - if !diags.HasError() && test.expectError { - t.Fatal("expected error, got no error") - } + req := xattr.ValidateAttributeRequest{} + resp := xattr.ValidateAttributeResponse{} - if diags.HasError() && !test.expectError { - t.Fatalf("got unexpected error: %#v", diags) + test.val.ValidateAttribute(ctx, req, &resp) + if resp.Diagnostics.HasError() != test.expectError { + t.Errorf("resp.Diagnostics.HasError() = %t, want = %t", resp.Diagnostics.HasError(), test.expectError) } }) } @@ -114,7 +108,7 @@ func TestDurationToStringValue(t *testing.T) { expected types.String }{ "value": { - duration: fwtypes.DurationValueMust("2h"), + duration: fwtypes.DurationValue("2h"), expected: types.StringValue("2h"), }, "null": { From ab51eba2982cf39619b94df707eab353b1456697 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Mon, 22 Apr 2024 14:41:04 -0400 Subject: [PATCH 7/9] internal/framework/types: convert iam policy to value validation --- internal/framework/types/iam_policy.go | 62 ++++++++------------- internal/framework/types/iam_policy_test.go | 39 ++++++------- 2 files changed, 38 insertions(+), 63 deletions(-) diff --git a/internal/framework/types/iam_policy.go b/internal/framework/types/iam_policy.go index e9349ee6f382..f0ae09faf957 100644 --- a/internal/framework/types/iam_policy.go +++ b/internal/framework/types/iam_policy.go @@ -13,17 +13,13 @@ import ( "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/attr/xattr" "github.com/hashicorp/terraform-plugin-framework/diag" - "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-framework/types/basetypes" "github.com/hashicorp/terraform-plugin-go/tftypes" ) var ( - _ xattr.TypeWithValidate = (*iamPolicyType)(nil) - _ basetypes.StringTypable = (*iamPolicyType)(nil) - _ basetypes.StringValuable = (*IAMPolicy)(nil) - _ basetypes.StringValuableWithSemanticEquals = (*IAMPolicy)(nil) + _ basetypes.StringTypable = (*iamPolicyType)(nil) ) type iamPolicyType struct { @@ -87,41 +83,11 @@ func (t iamPolicyType) ValueType(context.Context) attr.Value { return IAMPolicy{} } -func (t iamPolicyType) Validate(ctx context.Context, in tftypes.Value, path path.Path) diag.Diagnostics { - var diags diag.Diagnostics - - if !in.IsKnown() || in.IsNull() { - return diags - } - - var value string - err := in.As(&value) - if err != nil { - diags.AddAttributeError( - path, - "Invalid Terraform Value", - "An unexpected error occurred while attempting to convert a Terraform value to a string. "+ - "This generally is an issue with the provider schema implementation. "+ - "Please contact the provider developers.\n\n"+ - "Path: "+path.String()+"\n"+ - "Error: "+err.Error(), - ) - return diags - } - - if !json.Valid([]byte(value)) { - diags.AddAttributeError( - path, - "Invalid JSON String Value", - "A string value was provided that is not valid JSON string format (RFC 7159).\n\n"+ - "Path: "+path.String()+"\n"+ - "Given Value: "+value+"\n", - ) - return diags - } - - return diags -} +var ( + _ basetypes.StringValuable = (*IAMPolicy)(nil) + _ basetypes.StringValuableWithSemanticEquals = (*IAMPolicy)(nil) + _ xattr.ValidateableAttribute = (*IAMPolicy)(nil) +) func IAMPolicyNull() IAMPolicy { return IAMPolicy{StringValue: basetypes.NewStringNull()} @@ -190,3 +156,19 @@ func policyStringsEquivalent(s1, s2 string) bool { return equivalent } + +func (v IAMPolicy) ValidateAttribute(ctx context.Context, req xattr.ValidateAttributeRequest, resp *xattr.ValidateAttributeResponse) { + if v.IsNull() || v.IsUnknown() { + return + } + + if !json.Valid([]byte(v.ValueString())) { + resp.Diagnostics.AddAttributeError( + req.Path, + "Invalid IAM Policy Value", + "The provided value is not valid JSON string format (RFC 7159).\n\n"+ + "Path: "+req.Path.String()+"\n"+ + "Value: "+v.ValueString(), + ) + } +} diff --git a/internal/framework/types/iam_policy_test.go b/internal/framework/types/iam_policy_test.go index 7ff90f24b937..66c38a1ffc36 100644 --- a/internal/framework/types/iam_policy_test.go +++ b/internal/framework/types/iam_policy_test.go @@ -7,34 +7,29 @@ import ( "context" "testing" - "github.com/hashicorp/terraform-plugin-framework/path" - "github.com/hashicorp/terraform-plugin-go/tftypes" + "github.com/hashicorp/terraform-plugin-framework/attr/xattr" fwtypes "github.com/hashicorp/terraform-provider-aws/internal/framework/types" ) -func TestIAMPolicyTypeValidate(t *testing.T) { +func TestIAMPolicyValidateAttribute(t *testing.T) { t.Parallel() type testCase struct { - val tftypes.Value + val fwtypes.IAMPolicy expectError bool } tests := map[string]testCase{ - "not a string": { - val: tftypes.NewValue(tftypes.Bool, true), - expectError: true, - }, - "unknown string": { - val: tftypes.NewValue(tftypes.String, tftypes.UnknownValue), + "unknown": { + val: fwtypes.IAMPolicyUnknown(), }, - "null string": { - val: tftypes.NewValue(tftypes.String, nil), + "null": { + val: fwtypes.IAMPolicyNull(), }, - "valid string": { - val: tftypes.NewValue(tftypes.String, `{"Key1": "Value", "Key2": [1, 2, 3]}`), + "valid": { + val: fwtypes.IAMPolicyValue(`{"Key1": "Value", "Key2": [1, 2, 3]}`), }, - "invalid string": { - val: tftypes.NewValue(tftypes.String, "not ok"), + "invalid": { + val: fwtypes.IAMPolicyValue("not ok"), expectError: true, }, } @@ -46,14 +41,12 @@ func TestIAMPolicyTypeValidate(t *testing.T) { ctx := context.Background() - diags := fwtypes.IAMPolicyType.Validate(ctx, test.val, path.Root("test")) - - if !diags.HasError() && test.expectError { - t.Fatal("expected error, got no error") - } + req := xattr.ValidateAttributeRequest{} + resp := xattr.ValidateAttributeResponse{} - if diags.HasError() && !test.expectError { - t.Fatalf("got unexpected error: %#v", diags) + test.val.ValidateAttribute(ctx, req, &resp) + if resp.Diagnostics.HasError() != test.expectError { + t.Errorf("resp.Diagnostics.HasError() = %t, want = %t", resp.Diagnostics.HasError(), test.expectError) } }) } From e1be78d77ec63d828f2060c4f76c312889dece78 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Mon, 22 Apr 2024 15:04:38 -0400 Subject: [PATCH 8/9] internal/framework/types: convert once a week window to value validation --- .../framework/types/once_a_week_window.go | 70 +++++++++---------- .../types/once_a_week_window_test.go | 43 +++++------- 2 files changed, 50 insertions(+), 63 deletions(-) diff --git a/internal/framework/types/once_a_week_window.go b/internal/framework/types/once_a_week_window.go index 81d282de1f5c..838669439383 100644 --- a/internal/framework/types/once_a_week_window.go +++ b/internal/framework/types/once_a_week_window.go @@ -12,17 +12,23 @@ import ( "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/attr/xattr" "github.com/hashicorp/terraform-plugin-framework/diag" - "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-framework/types/basetypes" "github.com/hashicorp/terraform-plugin-go/tftypes" ) +const ( + // Valid time format is "ddd:hh24:mi". + validTimeFormat = "(sun|mon|tue|wed|thu|fri|sat):([0-1][0-9]|2[0-3]):([0-5][0-9])" + validTimeFormatConsolidated = "^(" + validTimeFormat + "-" + validTimeFormat + "|)$" +) + var ( - _ xattr.TypeWithValidate = (*onceAWeekWindowType)(nil) - _ basetypes.StringTypable = (*onceAWeekWindowType)(nil) - _ basetypes.StringValuable = (*OnceAWeekWindow)(nil) - _ basetypes.StringValuableWithSemanticEquals = (*OnceAWeekWindow)(nil) + validTimeFormatConsolidatedRegex = regexache.MustCompile(validTimeFormatConsolidated) +) + +var ( + _ basetypes.StringTypable = (*onceAWeekWindowType)(nil) ) type onceAWeekWindowType struct { @@ -82,39 +88,11 @@ func (onceAWeekWindowType) ValueType(context.Context) attr.Value { return OnceAWeekWindow{} } -func (t onceAWeekWindowType) Validate(ctx context.Context, in tftypes.Value, path path.Path) diag.Diagnostics { - var diags diag.Diagnostics - - if !in.IsKnown() || in.IsNull() { - return diags - } - - var value string - err := in.As(&value) - if err != nil { - diags.AddAttributeError( - path, - "OnceAWeekWindowType Validation Error", - ProviderErrorDetailPrefix+fmt.Sprintf("Cannot convert value to string: %s", err), - ) - return diags - } - - // Valid time format is "ddd:hh24:mi". - validTimeFormat := "(sun|mon|tue|wed|thu|fri|sat):([0-1][0-9]|2[0-3]):([0-5][0-9])" - validTimeFormatConsolidated := "^(" + validTimeFormat + "-" + validTimeFormat + "|)$" - - if v := strings.ToLower(value); !regexache.MustCompile(validTimeFormatConsolidated).MatchString(v) { - diags.AddAttributeError( - path, - "OnceAWeekWindowType Validation Error", - fmt.Sprintf("Value %q must satisfy the format of \"ddd:hh24:mi-ddd:hh24:mi\".", value), - ) - return diags - } - - return diags -} +var ( + _ basetypes.StringValuable = (*OnceAWeekWindow)(nil) + _ basetypes.StringValuableWithSemanticEquals = (*OnceAWeekWindow)(nil) + _ xattr.ValidateableAttribute = (*OnceAWeekWindow)(nil) +) type OnceAWeekWindow struct { basetypes.StringValue @@ -168,3 +146,19 @@ func (v OnceAWeekWindow) StringSemanticEquals(ctx context.Context, newValuable b // Case insensitive comparison. return strings.EqualFold(old.ValueString(), new.ValueString()), diags } + +func (v OnceAWeekWindow) ValidateAttribute(ctx context.Context, req xattr.ValidateAttributeRequest, resp *xattr.ValidateAttributeResponse) { + if v.IsNull() || v.IsUnknown() { + return + } + + if vs := strings.ToLower(v.ValueString()); !validTimeFormatConsolidatedRegex.MatchString(vs) { + resp.Diagnostics.AddAttributeError( + req.Path, + "Invalid Once A Week Window Value", + "The provided value does not satisfy the format \"ddd:hh24:mi-ddd:hh24:mi\".\n\n"+ + "Path: "+req.Path.String()+"\n"+ + "Value: "+vs, + ) + } +} diff --git a/internal/framework/types/once_a_week_window_test.go b/internal/framework/types/once_a_week_window_test.go index 498e32e43f6d..4ca270fd55ac 100644 --- a/internal/framework/types/once_a_week_window_test.go +++ b/internal/framework/types/once_a_week_window_test.go @@ -7,37 +7,32 @@ import ( "context" "testing" - "github.com/hashicorp/terraform-plugin-framework/path" - "github.com/hashicorp/terraform-plugin-go/tftypes" + "github.com/hashicorp/terraform-plugin-framework/attr/xattr" fwtypes "github.com/hashicorp/terraform-provider-aws/internal/framework/types" ) -func TestOnceAWeekWindowTypeValidate(t *testing.T) { +func TestOnceAWeekWindowValidateAttribute(t *testing.T) { t.Parallel() type testCase struct { - val tftypes.Value + val fwtypes.OnceAWeekWindow expectError bool } tests := map[string]testCase{ - "not a string": { - val: tftypes.NewValue(tftypes.Bool, true), - expectError: true, - }, - "unknown string": { - val: tftypes.NewValue(tftypes.String, tftypes.UnknownValue), + "unknown": { + val: fwtypes.OnceAWeekWindowUnknown(), }, - "null string": { - val: tftypes.NewValue(tftypes.String, nil), + "null": { + val: fwtypes.OnceAWeekWindowNull(), }, - "valid string lowercase": { - val: tftypes.NewValue(tftypes.String, "thu:07:44-thu:09:44"), + "valid lowercase": { + val: fwtypes.OnceAWeekWindowValue("thu:07:44-thu:09:44"), }, - "valid string uppercase": { - val: tftypes.NewValue(tftypes.String, "THU:07:44-THU:09:44"), + "valid uppercase": { + val: fwtypes.OnceAWeekWindowValue("THU:07:44-THU:09:44"), }, - "invalid string": { - val: tftypes.NewValue(tftypes.String, "thu:25:44-zat:09:88"), + "invalid": { + val: fwtypes.OnceAWeekWindowValue("thu:25:44-zat:09:88"), expectError: true, }, } @@ -49,14 +44,12 @@ func TestOnceAWeekWindowTypeValidate(t *testing.T) { ctx := context.Background() - diags := fwtypes.OnceAWeekWindowType.Validate(ctx, test.val, path.Root("test")) - - if !diags.HasError() && test.expectError { - t.Fatal("expected error, got no error") - } + req := xattr.ValidateAttributeRequest{} + resp := xattr.ValidateAttributeResponse{} - if diags.HasError() && !test.expectError { - t.Fatalf("got unexpected error: %#v", diags) + test.val.ValidateAttribute(ctx, req, &resp) + if resp.Diagnostics.HasError() != test.expectError { + t.Errorf("resp.Diagnostics.HasError() = %t, want = %t", resp.Diagnostics.HasError(), test.expectError) } }) } From fa504f707727647c013e4d020bd0683287287874 Mon Sep 17 00:00:00 2001 From: Jared Baker Date: Mon, 22 Apr 2024 15:18:04 -0400 Subject: [PATCH 9/9] internal/framework/types: convert regexp to value validation --- internal/framework/types/regexp.go | 74 ++++++++++++------------- internal/framework/types/regexp_test.go | 42 ++++++-------- 2 files changed, 55 insertions(+), 61 deletions(-) diff --git a/internal/framework/types/regexp.go b/internal/framework/types/regexp.go index 530e6ccf1d30..9e8650b5be39 100644 --- a/internal/framework/types/regexp.go +++ b/internal/framework/types/regexp.go @@ -8,20 +8,16 @@ import ( "fmt" "regexp" - "github.com/YakDriver/regexache" "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/attr/xattr" "github.com/hashicorp/terraform-plugin-framework/diag" - "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-framework/types/basetypes" "github.com/hashicorp/terraform-plugin-go/tftypes" ) var ( - _ xattr.TypeWithValidate = (*regexpType)(nil) - _ basetypes.StringTypable = (*regexpType)(nil) - _ basetypes.StringValuable = (*Regexp)(nil) + _ basetypes.StringTypable = (*regexpType)(nil) ) type regexpType struct { @@ -61,7 +57,7 @@ func (t regexpType) ValueFromString(_ context.Context, in types.String) (basetyp return RegexpUnknown(), diags // Must not return validation errors. } - return RegexpValueMust(valueString), diags + return RegexpValue(valueString), diags } func (t regexpType) ValueFromTerraform(ctx context.Context, in tftypes.Value) (attr.Value, error) { @@ -90,35 +86,10 @@ func (regexpType) ValueType(context.Context) attr.Value { return Regexp{} } -func (t regexpType) Validate(_ context.Context, in tftypes.Value, path path.Path) diag.Diagnostics { - var diags diag.Diagnostics - - if !in.IsKnown() || in.IsNull() { - return diags - } - - var value string - err := in.As(&value) - if err != nil { - diags.AddAttributeError( - path, - "Regexp Type Validation Error", - ProviderErrorDetailPrefix+fmt.Sprintf("Cannot convert value to string: %s", err), - ) - return diags - } - - if _, err := regexp.Compile(value); err != nil { - diags.AddAttributeError( - path, - "Regexp Type Validation Error", - fmt.Sprintf("Value %q cannot be parsed as a regular expression: %s", value, err), - ) - return diags - } - - return diags -} +var ( + _ basetypes.StringValuable = (*Regexp)(nil) + _ xattr.ValidateableAttribute = (*Regexp)(nil) +) func RegexpNull() Regexp { return Regexp{StringValue: basetypes.NewStringNull()} @@ -128,10 +99,21 @@ func RegexpUnknown() Regexp { return Regexp{StringValue: basetypes.NewStringUnknown()} } -func RegexpValueMust(value string) Regexp { +// RegexpValue initializes a new Regexp type with the provided value +// +// This function does not return diagnostics, and therefore invalid regular expression values +// are not handled during construction. Invalid values will be detected by the +// ValidateAttribute method, called by the ValidateResourceConfig RPC during +// operations like `terraform validate`, `plan`, or `apply`. +func RegexpValue(value string) Regexp { + // swallow any regex parsing errors here and just pass along the + // zero value regexp.Regexp. Invalid values will be handled downstream + // by the ValidateAttribute method. + v, _ := regexp.Compile(value) + return Regexp{ StringValue: basetypes.NewStringValue(value), - value: regexache.MustCompile(value), + value: v, } } @@ -157,3 +139,21 @@ func (Regexp) Type(context.Context) attr.Type { func (v Regexp) ValueRegexp() *regexp.Regexp { return v.value } + +func (v Regexp) ValidateAttribute(ctx context.Context, req xattr.ValidateAttributeRequest, resp *xattr.ValidateAttributeResponse) { + if v.IsNull() || v.IsUnknown() { + return + } + + vs := v.ValueString() + if _, err := regexp.Compile(vs); err != nil { + resp.Diagnostics.AddAttributeError( + req.Path, + "Invalid Regexp Value", + "The provided value cannot be parsed as a regular expression.\n\n"+ + "Path: "+req.Path.String()+"\n"+ + "Value: "+vs+"\n"+ + "Error: "+err.Error(), + ) + } +} diff --git a/internal/framework/types/regexp_test.go b/internal/framework/types/regexp_test.go index c708ae552377..51e62b25960e 100644 --- a/internal/framework/types/regexp_test.go +++ b/internal/framework/types/regexp_test.go @@ -7,7 +7,7 @@ import ( "context" "testing" - "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/attr/xattr" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-go/tftypes" fwtypes "github.com/hashicorp/terraform-provider-aws/internal/framework/types" @@ -30,7 +30,7 @@ func TestRegexpTypeValueFromTerraform(t *testing.T) { }, "valid Regexp": { val: tftypes.NewValue(tftypes.String, `\w+`), - expected: fwtypes.RegexpValueMust(`\w+`), + expected: fwtypes.RegexpValue(`\w+`), }, "invalid Regexp": { val: tftypes.NewValue(tftypes.String, `(`), @@ -57,29 +57,25 @@ func TestRegexpTypeValueFromTerraform(t *testing.T) { } } -func TestRegexpTypeValidate(t *testing.T) { +func TestRegexpValidateAttribute(t *testing.T) { t.Parallel() type testCase struct { - val tftypes.Value + val fwtypes.Regexp expectError bool } tests := map[string]testCase{ - "not a string": { - val: tftypes.NewValue(tftypes.Bool, true), - expectError: true, - }, - "unknown string": { - val: tftypes.NewValue(tftypes.String, tftypes.UnknownValue), + "unknown": { + val: fwtypes.RegexpUnknown(), }, - "null string": { - val: tftypes.NewValue(tftypes.String, nil), + "null": { + val: fwtypes.RegexpNull(), }, - "valid string": { - val: tftypes.NewValue(tftypes.String, `\w+`), + "valid": { + val: fwtypes.RegexpValue(`\w+`), }, - "invalid string": { - val: tftypes.NewValue(tftypes.String, `(`), + "invalid": { + val: fwtypes.RegexpValue(`(`), expectError: true, }, } @@ -91,14 +87,12 @@ func TestRegexpTypeValidate(t *testing.T) { ctx := context.Background() - diags := fwtypes.RegexpType.Validate(ctx, test.val, path.Root("test")) - - if !diags.HasError() && test.expectError { - t.Fatal("expected error, got no error") - } + req := xattr.ValidateAttributeRequest{} + resp := xattr.ValidateAttributeResponse{} - if diags.HasError() && !test.expectError { - t.Fatalf("got unexpected error: %#v", diags) + test.val.ValidateAttribute(ctx, req, &resp) + if resp.Diagnostics.HasError() != test.expectError { + t.Errorf("resp.Diagnostics.HasError() = %t, want = %t", resp.Diagnostics.HasError(), test.expectError) } }) } @@ -112,7 +106,7 @@ func TestRegexpToStringValue(t *testing.T) { expected types.String }{ "value": { - regexp: fwtypes.RegexpValueMust(`\w+`), + regexp: fwtypes.RegexpValue(`\w+`), expected: types.StringValue(`\w+`), }, "null": {