From bcdec31c0c150a569940c79787c8a7d914d09c37 Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Tue, 21 Jun 2022 14:06:52 +0100 Subject: [PATCH 1/5] Adding Map validation for KeysAre and ValuesAre (#13) --- mapvalidator/keys_are.go | 61 ++++++++++++++ mapvalidator/keys_are_test.go | 119 +++++++++++++++++++++++++++ mapvalidator/type_validation.go | 28 +++++++ mapvalidator/type_validation_test.go | 82 ++++++++++++++++++ mapvalidator/values_are.go | 60 ++++++++++++++ mapvalidator/values_are_test.go | 113 +++++++++++++++++++++++++ 6 files changed, 463 insertions(+) create mode 100644 mapvalidator/keys_are.go create mode 100644 mapvalidator/keys_are_test.go create mode 100644 mapvalidator/type_validation.go create mode 100644 mapvalidator/type_validation_test.go create mode 100644 mapvalidator/values_are.go create mode 100644 mapvalidator/values_are_test.go diff --git a/mapvalidator/keys_are.go b/mapvalidator/keys_are.go new file mode 100644 index 00000000..454f662c --- /dev/null +++ b/mapvalidator/keys_are.go @@ -0,0 +1,61 @@ +package mapvalidator + +import ( + "context" + "fmt" + "strings" + + "github.com/hashicorp/terraform-plugin-framework/tfsdk" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +var _ tfsdk.AttributeValidator = keysAreValidator{} + +// keysAreValidator validates that each map key validates against each of the value validators. +type keysAreValidator struct { + keyValidators []tfsdk.AttributeValidator +} + +// Description describes the validation in plain text formatting. +func (v keysAreValidator) Description(ctx context.Context) string { + var descriptions []string + for _, validator := range v.keyValidators { + descriptions = append(descriptions, validator.Description(ctx)) + } + + return fmt.Sprintf("key must satisfy all validations: %s", strings.Join(descriptions, " + ")) +} + +// MarkdownDescription describes the validation in Markdown formatting. +func (v keysAreValidator) MarkdownDescription(ctx context.Context) string { + return v.Description(ctx) +} + +// Validate performs the validation. +func (v keysAreValidator) Validate(ctx context.Context, req tfsdk.ValidateAttributeRequest, resp *tfsdk.ValidateAttributeResponse) { + elems, ok := validateMap(ctx, req, resp) + if !ok { + return + } + + for k := range elems { + request := tfsdk.ValidateAttributeRequest{ + AttributePath: req.AttributePath.WithElementKeyString(k), + AttributeConfig: types.String{Value: k}, + Config: req.Config, + } + + for _, validator := range v.keyValidators { + validator.Validate(ctx, request, resp) + if resp.Diagnostics.HasError() { + return + } + } + } +} + +func KeysAre(keyValidators ...tfsdk.AttributeValidator) tfsdk.AttributeValidator { + return keysAreValidator{ + keyValidators: keyValidators, + } +} diff --git a/mapvalidator/keys_are_test.go b/mapvalidator/keys_are_test.go new file mode 100644 index 00000000..3fad272f --- /dev/null +++ b/mapvalidator/keys_are_test.go @@ -0,0 +1,119 @@ +package mapvalidator + +import ( + "context" + "testing" + + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-go/tftypes" + + "github.com/hashicorp/terraform-plugin-framework-validators/int64validator" + "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" +) + +func TestKeysAreValidator(t *testing.T) { + t.Parallel() + + type testCase struct { + val attr.Value + keysAreValidators []tfsdk.AttributeValidator + expectError bool + } + tests := map[string]testCase{ + "not Map": { + val: types.List{ + ElemType: types.StringType, + }, + expectError: true, + }, + "Map unknown": { + val: types.Map{ + Unknown: true, + ElemType: types.StringType, + }, + expectError: false, + }, + "Map null": { + val: types.Map{ + Null: true, + ElemType: types.StringType, + }, + expectError: false, + }, + "Map key invalid": { + val: types.Map{ + ElemType: types.StringType, + Elems: map[string]attr.Value{ + "one": types.String{Value: "first"}, + "two": types.String{Value: "second"}, + }, + }, + keysAreValidators: []tfsdk.AttributeValidator{ + stringvalidator.LengthAtLeast(4), + }, + expectError: true, + }, + "Map key invalid for second validator": { + val: types.Map{ + ElemType: types.StringType, + Elems: map[string]attr.Value{ + "one": types.String{Value: "first"}, + "two": types.String{Value: "second"}, + }, + }, + keysAreValidators: []tfsdk.AttributeValidator{ + stringvalidator.LengthAtLeast(2), + stringvalidator.LengthAtLeast(6), + }, + expectError: true, + }, + "Map keys wrong type for validator": { + val: types.Map{ + ElemType: types.StringType, + Elems: map[string]attr.Value{ + "one": types.String{Value: "first"}, + "two": types.String{Value: "second"}, + }, + }, + keysAreValidators: []tfsdk.AttributeValidator{ + int64validator.AtLeast(6), + }, + expectError: true, + }, + "Map keys valid": { + val: types.Map{ + ElemType: types.StringType, + Elems: map[string]attr.Value{ + "one": types.String{Value: "first"}, + "two": types.String{Value: "second"}, + }, + }, + keysAreValidators: []tfsdk.AttributeValidator{ + stringvalidator.LengthAtLeast(3), + }, + expectError: false, + }, + } + + for name, test := range tests { + name, test := name, test + t.Run(name, func(t *testing.T) { + request := tfsdk.ValidateAttributeRequest{ + AttributePath: tftypes.NewAttributePath().WithAttributeName("test"), + AttributeConfig: test.val, + } + response := tfsdk.ValidateAttributeResponse{} + KeysAre(test.keysAreValidators...).Validate(context.TODO(), request, &response) + + if !response.Diagnostics.HasError() && test.expectError { + t.Fatal("expected error, got no error") + } + + if response.Diagnostics.HasError() && !test.expectError { + t.Fatalf("got unexpected error: %s", response.Diagnostics) + } + }) + } +} diff --git a/mapvalidator/type_validation.go b/mapvalidator/type_validation.go new file mode 100644 index 00000000..e6448f0c --- /dev/null +++ b/mapvalidator/type_validation.go @@ -0,0 +1,28 @@ +package mapvalidator + +import ( + "context" + + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +// validateMap ensures that the request contains a Map value. +func validateMap(ctx context.Context, request tfsdk.ValidateAttributeRequest, response *tfsdk.ValidateAttributeResponse) (map[string]attr.Value, bool) { + var m types.Map + + diags := tfsdk.ValueAs(ctx, request.AttributeConfig, &m) + + if diags.HasError() { + response.Diagnostics = append(response.Diagnostics, diags...) + + return nil, false + } + + if m.Unknown || m.Null { + return nil, false + } + + return m.Elems, true +} diff --git a/mapvalidator/type_validation_test.go b/mapvalidator/type_validation_test.go new file mode 100644 index 00000000..ead99a52 --- /dev/null +++ b/mapvalidator/type_validation_test.go @@ -0,0 +1,82 @@ +package mapvalidator + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-go/tftypes" +) + +func TestValidateMap(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + request tfsdk.ValidateAttributeRequest + expectedMap map[string]attr.Value + expectedOk bool + }{ + "invalid-type": { + request: tfsdk.ValidateAttributeRequest{ + AttributeConfig: types.Bool{Value: true}, + AttributePath: tftypes.NewAttributePath().WithAttributeName("test"), + }, + expectedMap: nil, + expectedOk: false, + }, + "map-null": { + request: tfsdk.ValidateAttributeRequest{ + AttributeConfig: types.Map{Null: true}, + AttributePath: tftypes.NewAttributePath().WithAttributeName("test"), + }, + expectedMap: nil, + expectedOk: false, + }, + "map-unknown": { + request: tfsdk.ValidateAttributeRequest{ + AttributeConfig: types.Map{Unknown: true}, + AttributePath: tftypes.NewAttributePath().WithAttributeName("test"), + }, + expectedMap: nil, + expectedOk: false, + }, + "map-value": { + request: tfsdk.ValidateAttributeRequest{ + AttributeConfig: types.Map{ + ElemType: types.StringType, + Elems: map[string]attr.Value{ + "one": types.String{Value: "first"}, + "two": types.String{Value: "second"}, + }, + }, + AttributePath: tftypes.NewAttributePath().WithAttributeName("test"), + }, + expectedMap: map[string]attr.Value{ + "one": types.String{Value: "first"}, + "two": types.String{Value: "second"}, + }, + expectedOk: true, + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + gotMapElems, gotOk := validateMap(context.Background(), testCase.request, &tfsdk.ValidateAttributeResponse{}) + + if diff := cmp.Diff(gotMapElems, testCase.expectedMap); diff != "" { + t.Errorf("unexpected float64 difference: %s", diff) + } + + if diff := cmp.Diff(gotOk, testCase.expectedOk); diff != "" { + t.Errorf("unexpected ok difference: %s", diff) + } + }) + } +} diff --git a/mapvalidator/values_are.go b/mapvalidator/values_are.go new file mode 100644 index 00000000..3ea82c09 --- /dev/null +++ b/mapvalidator/values_are.go @@ -0,0 +1,60 @@ +package mapvalidator + +import ( + "context" + "fmt" + "strings" + + "github.com/hashicorp/terraform-plugin-framework/tfsdk" +) + +var _ tfsdk.AttributeValidator = valuesAreValidator{} + +// valuesAreValidator validates that each map value validates against each of the value validators. +type valuesAreValidator struct { + valueValidators []tfsdk.AttributeValidator +} + +// Description describes the validation in plain text formatting. +func (v valuesAreValidator) Description(ctx context.Context) string { + var descriptions []string + for _, validator := range v.valueValidators { + descriptions = append(descriptions, validator.Description(ctx)) + } + + return fmt.Sprintf("value must satisfy all validations: %s", strings.Join(descriptions, " + ")) +} + +// MarkdownDescription describes the validation in Markdown formatting. +func (v valuesAreValidator) MarkdownDescription(ctx context.Context) string { + return v.Description(ctx) +} + +// Validate performs the validation. +func (v valuesAreValidator) Validate(ctx context.Context, req tfsdk.ValidateAttributeRequest, resp *tfsdk.ValidateAttributeResponse) { + elems, ok := validateMap(ctx, req, resp) + if !ok { + return + } + + for k, elem := range elems { + request := tfsdk.ValidateAttributeRequest{ + AttributePath: req.AttributePath.WithElementKeyString(k), + AttributeConfig: elem, + Config: req.Config, + } + + for _, validator := range v.valueValidators { + validator.Validate(ctx, request, resp) + if resp.Diagnostics.HasError() { + return + } + } + } +} + +func ValuesAre(valueValidators ...tfsdk.AttributeValidator) tfsdk.AttributeValidator { + return valuesAreValidator{ + valueValidators: valueValidators, + } +} diff --git a/mapvalidator/values_are_test.go b/mapvalidator/values_are_test.go new file mode 100644 index 00000000..1f7ffe56 --- /dev/null +++ b/mapvalidator/values_are_test.go @@ -0,0 +1,113 @@ +package mapvalidator + +import ( + "context" + "testing" + + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-go/tftypes" + + "github.com/hashicorp/terraform-plugin-framework-validators/int64validator" + "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" +) + +func TestValuesAreValidator(t *testing.T) { + t.Parallel() + + type testCase struct { + val attr.Value + valuesAreValidators []tfsdk.AttributeValidator + expectError bool + } + tests := map[string]testCase{ + "Map unknown": { + val: types.Map{ + Unknown: true, + ElemType: types.StringType, + }, + expectError: false, + }, + "Map null": { + val: types.Map{ + Null: true, + ElemType: types.StringType, + }, + expectError: false, + }, + "Map value invalid": { + val: types.Map{ + ElemType: types.StringType, + Elems: map[string]attr.Value{ + "number_one": types.String{Value: "first"}, + "number_two": types.String{Value: "second"}, + }, + }, + valuesAreValidators: []tfsdk.AttributeValidator{ + stringvalidator.LengthAtLeast(6), + }, + expectError: true, + }, + "Maps value invalid for second validator": { + val: types.Map{ + ElemType: types.StringType, + Elems: map[string]attr.Value{ + "number_one": types.String{Value: "first"}, + "number_two": types.String{Value: "second"}, + }, + }, + valuesAreValidators: []tfsdk.AttributeValidator{ + stringvalidator.LengthAtLeast(2), + stringvalidator.LengthAtLeast(6), + }, + expectError: true, + }, + "Map values wrong type for validator": { + val: types.Map{ + ElemType: types.StringType, + Elems: map[string]attr.Value{ + "number_one": types.String{Value: "first"}, + "number_two": types.String{Value: "second"}, + }, + }, + valuesAreValidators: []tfsdk.AttributeValidator{ + int64validator.AtLeast(6), + }, + expectError: true, + }, + "Map values valid": { + val: types.Map{ + ElemType: types.StringType, + Elems: map[string]attr.Value{ + "one": types.String{Value: "first"}, + "two": types.String{Value: "second"}, + }, + }, + valuesAreValidators: []tfsdk.AttributeValidator{ + stringvalidator.LengthAtLeast(5), + }, + expectError: false, + }, + } + + for name, test := range tests { + name, test := name, test + t.Run(name, func(t *testing.T) { + request := tfsdk.ValidateAttributeRequest{ + AttributePath: tftypes.NewAttributePath().WithAttributeName("test"), + AttributeConfig: test.val, + } + response := tfsdk.ValidateAttributeResponse{} + ValuesAre(test.valuesAreValidators...).Validate(context.TODO(), request, &response) + + if !response.Diagnostics.HasError() && test.expectError { + t.Fatal("expected error, got no error") + } + + if response.Diagnostics.HasError() && !test.expectError { + t.Fatalf("got unexpected error: %s", response.Diagnostics) + } + }) + } +} From 7970d3e959d2f23c2daa44092226608da56c2d58 Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Wed, 22 Jun 2022 15:12:37 +0100 Subject: [PATCH 2/5] Updates following code review (#13) --- .changelog/13.txt | 3 +++ mapvalidator/doc.go | 2 ++ mapvalidator/type_validation_test.go | 2 +- mapvalidator/values_are.go | 10 +++++++--- 4 files changed, 13 insertions(+), 4 deletions(-) create mode 100644 .changelog/13.txt create mode 100644 mapvalidator/doc.go diff --git a/.changelog/13.txt b/.changelog/13.txt new file mode 100644 index 00000000..005db2a5 --- /dev/null +++ b/.changelog/13.txt @@ -0,0 +1,3 @@ +```release-note:feature +Introduced `mapvalidator` package with `ValuesAre()` validation function +``` \ No newline at end of file diff --git a/mapvalidator/doc.go b/mapvalidator/doc.go new file mode 100644 index 00000000..31871729 --- /dev/null +++ b/mapvalidator/doc.go @@ -0,0 +1,2 @@ +// Package mapvalidator provides validators for types.Map attributes. +package mapvalidator diff --git a/mapvalidator/type_validation_test.go b/mapvalidator/type_validation_test.go index ead99a52..1101d812 100644 --- a/mapvalidator/type_validation_test.go +++ b/mapvalidator/type_validation_test.go @@ -71,7 +71,7 @@ func TestValidateMap(t *testing.T) { gotMapElems, gotOk := validateMap(context.Background(), testCase.request, &tfsdk.ValidateAttributeResponse{}) if diff := cmp.Diff(gotMapElems, testCase.expectedMap); diff != "" { - t.Errorf("unexpected float64 difference: %s", diff) + t.Errorf("unexpected map difference: %s", diff) } if diff := cmp.Diff(gotOk, testCase.expectedOk); diff != "" { diff --git a/mapvalidator/values_are.go b/mapvalidator/values_are.go index 3ea82c09..828a7898 100644 --- a/mapvalidator/values_are.go +++ b/mapvalidator/values_are.go @@ -46,13 +46,17 @@ func (v valuesAreValidator) Validate(ctx context.Context, req tfsdk.ValidateAttr for _, validator := range v.valueValidators { validator.Validate(ctx, request, resp) - if resp.Diagnostics.HasError() { - return - } } } } +// ValuesAre returns an AttributeValidator which ensures that any configured +// attribute value: +// +// - Is a Map. +// - Contains Map elements, each of which validate against each value validator. +// +// Null (unconfigured) and unknown (known after apply) values are skipped. func ValuesAre(valueValidators ...tfsdk.AttributeValidator) tfsdk.AttributeValidator { return valuesAreValidator{ valueValidators: valueValidators, From e5550b3ad4e72a920d7a5b3e1d9cc3fca6f2b792 Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Wed, 22 Jun 2022 15:14:22 +0100 Subject: [PATCH 3/5] Update CHANGELOG.md (#13) --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4386bf05..ecf42c83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +# 0.3.0 (unreleased) + +FEATURES: +* Introduced `mapvalidator` package with `ValuesAre()` validation function ([#13](https://github.com/hashicorp/terraform-plugin-framework-validators/issues/13)) + # 0.2.0 (June 7, 2022) BREAKING CHANGES: From 5001b0dee714b1b5f045af3ca39180f1078b3d0e Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Thu, 23 Jun 2022 08:51:40 +0100 Subject: [PATCH 4/5] Rename .changelog file to match PR number and remove updates to CHANGELOG.md (#13) --- .changelog/{13.txt => 38.txt} | 0 CHANGELOG.md | 5 ----- 2 files changed, 5 deletions(-) rename .changelog/{13.txt => 38.txt} (100%) diff --git a/.changelog/13.txt b/.changelog/38.txt similarity index 100% rename from .changelog/13.txt rename to .changelog/38.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index ecf42c83..4386bf05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,3 @@ -# 0.3.0 (unreleased) - -FEATURES: -* Introduced `mapvalidator` package with `ValuesAre()` validation function ([#13](https://github.com/hashicorp/terraform-plugin-framework-validators/issues/13)) - # 0.2.0 (June 7, 2022) BREAKING CHANGES: From 88157712f50593f9f1c3714cb6f3b9ea057fd885 Mon Sep 17 00:00:00 2001 From: Benjamin Bennett Date: Thu, 23 Jun 2022 09:13:52 +0100 Subject: [PATCH 5/5] Updating docs to clarify validation of Map keys (#13) --- mapvalidator/keys_are.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mapvalidator/keys_are.go b/mapvalidator/keys_are.go index 454f662c..109ee1fa 100644 --- a/mapvalidator/keys_are.go +++ b/mapvalidator/keys_are.go @@ -32,6 +32,9 @@ func (v keysAreValidator) MarkdownDescription(ctx context.Context) string { } // Validate performs the validation. +// Note that the AttributePath specified in the ValidateAttributeRequest refers to the value in the Map with key `k`, +// whereas the AttributeConfig refers to the key itself (i.e., `k`). This is intentional as the validation being +// performed is for the keys of the Map. func (v keysAreValidator) Validate(ctx context.Context, req tfsdk.ValidateAttributeRequest, resp *tfsdk.ValidateAttributeResponse) { elems, ok := validateMap(ctx, req, resp) if !ok {