From 3c3e216b3fc06079bd1add5302ea2967349a6291 Mon Sep 17 00:00:00 2001 From: Chris Marget Date: Tue, 19 Sep 2023 14:14:32 -0400 Subject: [PATCH 1/5] introduce `attributeConflictValidator` --- apstra/apstra_validator/attribute_conflict.go | 239 ++++++++++ .../attribute_conflict_test.go | 416 ++++++++++++++++++ 2 files changed, 655 insertions(+) create mode 100644 apstra/apstra_validator/attribute_conflict.go create mode 100644 apstra/apstra_validator/attribute_conflict_test.go diff --git a/apstra/apstra_validator/attribute_conflict.go b/apstra/apstra_validator/attribute_conflict.go new file mode 100644 index 00000000..6e24e259 --- /dev/null +++ b/apstra/apstra_validator/attribute_conflict.go @@ -0,0 +1,239 @@ +package apstravalidator + +import ( + "context" + "encoding/base64" + "fmt" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types/basetypes" + "strings" +) + +type CollectionValidator interface { + //validator.List + //validator.Map + validator.Set +} + +var _ CollectionValidator = attributeConflictValidator{} + +// attributeConflictValidator ensures that no two elements of a list, map, or +// set of objects use the same value across all attributes enumerated in +// keyAttrs. +// +// For example, if keyAttrs contains just {"name"}, then having two objects +// with `name: "foo"` will produce a validation error. +// +// If keyAttrs contains {"protocol", "port"} then having two objects with +// `protocol: "TCP"` and `port: 80` will produce a validation error. +// +// If keyAttrs is empty, then values across all attributes are evaluated. +type attributeConflictValidator struct { + keyAttrs []string +} + +func (o attributeConflictValidator) Description(_ context.Context) string { + if len(o.keyAttrs) == 0 { + return "Ensure that no two collection (list/map/set) members share values for all attributes" + } + + return fmt.Sprintf( + "Ensure that no two collection (list/map/set) members share values for these attributes: [%s]", + strings.Join(o.keyAttrs, " "), + ) +} + +func (o attributeConflictValidator) MarkdownDescription(ctx context.Context) string { + return o.Description(ctx) +} + +//func (o attributeConflictValidator) ValidateList(ctx context.Context, req validator.ListRequest, resp *validator.ListResponse) { +// if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() { +// return +// } +// +// uniqueStrings := make(map[string]bool) +// +// for i, element := range req.ConfigValue.Elements() { +// objectPath := req.Path.AtListIndex(i) +// +// objectValuable, ok := element.(basetypes.ObjectValuable) +// if !ok { +// resp.Diagnostics.AddAttributeError( +// req.Path, +// "Invalid Validator for Element Value", +// "While performing schema-based validation, an unexpected error occurred. "+ +// "The attribute declares a Object values validator, however its values do not implement the types.ObjectValuable interface for custom Object types. "+ +// "This is likely an issue with terraform-plugin-framework and should be reported to the provider developers.\n\n"+ +// fmt.Sprintf("Path: %s\n", req.Path.String())+ +// fmt.Sprintf("Element Type: %T\n", req.ConfigValue.ElementType(ctx))+ +// fmt.Sprintf("Element Value Type: %T\n", element), +// ) +// +// return +// } +// +// objectValue, diags := objectValuable.ToObjectValue(ctx) +// resp.Diagnostics.Append(diags...) +// if diags.HasError() { +// return +// } +// +// for attrName, attrValue := range objectValue.Attributes() { +// if attrName != o.keyAttrs || attrValue.IsUnknown() { +// continue +// } +// +// if uniqueStrings[attrValue.String()] { +// resp.Diagnostics.AddAttributeError( +// objectPath, +// fmt.Sprintf("%s collision", o.keyAttrs), +// fmt.Sprintf("Two objects cannot use the same %s", o.keyAttrs), +// ) +// } else { +// uniqueStrings[attrValue.String()] = true +// } +// break // the correct attribute has been found; move on to the next +// } +// } +//} +// +//func (o attributeConflictValidator) ValidateMap(ctx context.Context, req validator.MapRequest, resp *validator.MapResponse) { +// if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() { +// return +// } +// +// uniqueStrings := make(map[string]bool) +// +// for mapKey, element := range req.ConfigValue.Elements() { +// objectPath := req.Path.AtMapKey(mapKey) +// +// objectValuable, ok := element.(basetypes.ObjectValuable) +// if !ok { +// resp.Diagnostics.AddAttributeError( +// req.Path, +// "Invalid Validator for Element Value", +// "While performing schema-based validation, an unexpected error occurred. "+ +// "The attribute declares a Object values validator, however its values do not implement the types.ObjectValuable interface for custom Object types. "+ +// "This is likely an issue with terraform-plugin-framework and should be reported to the provider developers.\n\n"+ +// fmt.Sprintf("Path: %s\n", req.Path.String())+ +// fmt.Sprintf("Element Type: %T\n", req.ConfigValue.ElementType(ctx))+ +// fmt.Sprintf("Element Value Type: %T\n", element), +// ) +// +// return +// } +// +// objectValue, diags := objectValuable.ToObjectValue(ctx) +// resp.Diagnostics.Append(diags...) +// if diags.HasError() { +// return +// } +// +// for attrName, attrValue := range objectValue.Attributes() { +// if attrName != o.keyAttrs || attrValue.IsUnknown() { +// continue +// } +// +// if uniqueStrings[attrValue.String()] { +// resp.Diagnostics.AddAttributeError( +// objectPath, +// fmt.Sprintf("%s collision", o.keyAttrs), +// fmt.Sprintf("Two objects cannot use the same %s", o.keyAttrs), +// ) +// } else { +// uniqueStrings[attrValue.String()] = true +// } +// break // the correct attribute has been found; move on to the next +// } +// } +//} + +func (o attributeConflictValidator) ValidateSet(ctx context.Context, req validator.SetRequest, resp *validator.SetResponse) { + if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() { + return + } + + // map of key attribute names + keyAttributeNames := make(map[string]bool, len(o.keyAttrs)) + for _, key := range o.keyAttrs { + keyAttributeNames[key] = true + } + + // map of found value combinations delimited by ':' + // base64(key1):base64(key2):...:base64(keyN) + foundKeyValueCombinations := make(map[string]bool) // found value combinations + + for _, element := range req.ConfigValue.Elements() { // loop over set members + objectPath := req.Path.AtSetValue(element) + + objectValuable, ok := element.(basetypes.ObjectValuable) + if !ok { + resp.Diagnostics.AddAttributeError( + req.Path, + "Invalid Validator for Element Value", + "While performing schema-based validation, an unexpected error occurred. "+ + "The attribute declares a Object values validator, however its values do not implement the types.ObjectValuable interface for custom Object types. "+ + "This is likely an issue with terraform-plugin-framework and should be reported to the provider developers.\n\n"+ + fmt.Sprintf("Path: %s\n", req.Path.String())+ + fmt.Sprintf("Element Type: %T\n", req.ConfigValue.ElementType(ctx))+ + fmt.Sprintf("Element Value Type: %T\n", element), + ) + + return + } + + objectValue, diags := objectValuable.ToObjectValue(ctx) + resp.Diagnostics.Append(diags...) + if diags.HasError() { + return + } + + if len(o.keyAttrs) == 0 { + // the caller didn't specify any "key" attributes, so we'll use all of them + for k := range objectValue.Attributes() { + o.keyAttrs = append(o.keyAttrs, k) + keyAttributeNames[k] = true + } + } + + keyValuesMap := make(map[string]string, len(keyAttributeNames)) + for attrName, attrValue := range objectValue.Attributes() { // loop over set member attributes + if !keyAttributeNames[attrName] { + continue // attribute is not interesting + } + + if attrValue.IsUnknown() { + return // cannot validate when attribute is unknown + } + + keyValuesMap[attrName] = base64.StdEncoding.EncodeToString([]byte(attrValue.String())) + if len(keyValuesMap) < len(keyAttributeNames) { + continue // keep going until we fill keyValuesMap + } + + sb := strings.Builder{} + sb.WriteString(keyValuesMap[o.keyAttrs[0]]) // keyAttrs always has at least 1 entry + for i := range o.keyAttrs[1:] { + sb.WriteString(":" + keyValuesMap[o.keyAttrs[i]]) + } + + if foundKeyValueCombinations[sb.String()] { // seen this value before? + resp.Diagnostics.AddAttributeError( + objectPath, + fmt.Sprintf("%s collision", o.keyAttrs), + fmt.Sprintf("Two objects cannot use the same %s", o.keyAttrs), + ) + } else { + foundKeyValueCombinations[sb.String()] = true // log the name for future collision checks + } + break // all of the the required attribute have been found; move on to the next set member + } + } +} + +func UniquteValueCombinationsAt(attrNames ...string) CollectionValidator { + return attributeConflictValidator{ + keyAttrs: attrNames, + } +} diff --git a/apstra/apstra_validator/attribute_conflict_test.go b/apstra/apstra_validator/attribute_conflict_test.go new file mode 100644 index 00000000..6b57a39a --- /dev/null +++ b/apstra/apstra_validator/attribute_conflict_test.go @@ -0,0 +1,416 @@ +package apstravalidator + +import ( + "context" + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" + "log" + "testing" +) + +func TestAttributeConflictValidator_ValidateSet(t *testing.T) { + ctx := context.Background() + + type testCase struct { + keyAttrNames []string + attrTypes map[string]attr.Type + attrValues []attr.Value + expectError bool + } + + testCases := map[string]testCase{ + "one_key_no_collision": { + keyAttrNames: []string{"key1"}, + attrTypes: map[string]attr.Type{ + "key1": types.StringType, + }, + attrValues: []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("foo"), + }, + ), + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("bar"), + }, + ), + }, + expectError: false, + }, + "one_key_collision": { + keyAttrNames: []string{"key1"}, + attrTypes: map[string]attr.Type{ + "key1": types.StringType, + }, + attrValues: []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("foo"), + }, + ), + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("foo"), + }, + ), + }, + expectError: true, + }, + "one_key_plus_extras_no_collision": { + keyAttrNames: []string{"key1"}, + attrTypes: map[string]attr.Type{ + "key1": types.StringType, + "extra1": types.StringType, + "extra2": types.StringType, + }, + attrValues: []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + "extra1": types.StringType, + "extra2": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("foo"), + "extra1": types.StringValue("foo"), + "extra2": types.StringValue("foo"), + }, + ), + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + "extra1": types.StringType, + "extra2": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("bar"), + "extra1": types.StringValue("bar"), + "extra2": types.StringValue("bar"), + }, + ), + }, + expectError: false, + }, + "one_key_plus_extras_collision": { + keyAttrNames: []string{"key1"}, + attrTypes: map[string]attr.Type{ + "key1": types.StringType, + "extra1": types.StringType, + "extra2": types.StringType, + }, + attrValues: []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + "extra1": types.StringType, + "extra2": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("foo"), + "extra1": types.StringValue("bar"), + "extra2": types.StringValue("baz"), + }, + ), + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + "extra1": types.StringType, + "extra2": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("foo"), + "extra1": types.StringValue("bar"), + "extra2": types.StringValue("baz"), + }, + ), + }, + expectError: true, + }, + "three_keys_no_collision": { + keyAttrNames: []string{"key1", "key2", "key3"}, + attrTypes: map[string]attr.Type{ + "key1": types.StringType, + "key2": types.StringType, + "key3": types.StringType, + }, + attrValues: []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + "key2": types.StringType, + "key3": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("foo"), + "key2": types.StringValue("bar"), + "key3": types.StringValue("baz"), + }, + ), + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + "key2": types.StringType, + "key3": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("bar"), + "key2": types.StringValue("baz"), + "key3": types.StringValue("foo"), + }, + ), + }, + expectError: false, + }, + "three_keys_with_extras_no_collision": { + keyAttrNames: []string{"key1", "key2", "key3"}, + attrTypes: map[string]attr.Type{ + "key1": types.StringType, + "key2": types.StringType, + "key3": types.StringType, + "extra1": types.StringType, + "extra2": types.StringType, + "extra3": types.StringType, + }, + attrValues: []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + "key2": types.StringType, + "key3": types.StringType, + "extra1": types.StringType, + "extra2": types.StringType, + "extra3": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("foo"), + "key2": types.StringValue("bar"), + "key3": types.StringValue("baz"), + "extra1": types.StringValue("foo"), + "extra2": types.StringValue("bar"), + "extra3": types.StringValue("baz"), + }, + ), + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + "key2": types.StringType, + "key3": types.StringType, + "extra1": types.StringType, + "extra2": types.StringType, + "extra3": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("bar"), + "key2": types.StringValue("baz"), + "key3": types.StringValue("foo"), + "extra1": types.StringValue("foo"), + "extra2": types.StringValue("bar"), + "extra3": types.StringValue("baz"), + }, + ), + }, + expectError: false, + }, + "three_keys_collision": { + keyAttrNames: []string{"key1", "key2", "key3"}, + attrTypes: map[string]attr.Type{ + "key1": types.StringType, + "key2": types.StringType, + "key3": types.StringType, + }, + attrValues: []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + "key2": types.StringType, + "key3": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("foo"), + "key2": types.StringValue("bar"), + "key3": types.StringValue("baz"), + }, + ), + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + "key2": types.StringType, + "key3": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("foo"), + "key2": types.StringValue("bar"), + "key3": types.StringValue("baz"), + }, + ), + }, + expectError: true, + }, + "three_keys_with_extras_collision": { + keyAttrNames: []string{"key1", "key2", "key3"}, + attrTypes: map[string]attr.Type{ + "key1": types.StringType, + "key2": types.StringType, + "key3": types.StringType, + "extra1": types.StringType, + "extra2": types.StringType, + "extra3": types.StringType, + }, + attrValues: []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + "key2": types.StringType, + "key3": types.StringType, + "extra1": types.StringType, + "extra2": types.StringType, + "extra3": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("foo"), + "key2": types.StringValue("bar"), + "key3": types.StringValue("baz"), + "extra1": types.StringValue("foo"), + "extra2": types.StringValue("bar"), + "extra3": types.StringValue("baz"), + }, + ), + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + "key2": types.StringType, + "key3": types.StringType, + "extra1": types.StringType, + "extra2": types.StringType, + "extra3": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("foo"), + "key2": types.StringValue("bar"), + "key3": types.StringValue("baz"), + "extra1": types.StringValue("foo"), + "extra2": types.StringValue("bar"), + "extra3": types.StringValue("baz"), + }, + ), + }, + expectError: true, + }, + "all_keys_no_collision": { + keyAttrNames: []string{}, + attrTypes: map[string]attr.Type{ + "key1": types.StringType, + "key2": types.StringType, + "key3": types.StringType, + }, + attrValues: []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + "key2": types.StringType, + "key3": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("foo"), + "key2": types.StringValue("bar"), + "key3": types.StringValue("baz"), + }, + ), + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + "key2": types.StringType, + "key3": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("bar"), + "key2": types.StringValue("baz"), + "key3": types.StringValue("foo"), + }, + ), + }, + expectError: false, + }, + "all_keys_collision": { + keyAttrNames: []string{}, + attrTypes: map[string]attr.Type{ + "key1": types.StringType, + "key2": types.StringType, + "key3": types.StringType, + }, + attrValues: []attr.Value{ + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + "key2": types.StringType, + "key3": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("foo"), + "key2": types.StringValue("bar"), + "key3": types.StringValue("baz"), + }, + ), + types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + "key2": types.StringType, + "key3": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("foo"), + "key2": types.StringValue("bar"), + "key3": types.StringValue("baz"), + }, + ), + }, + expectError: true, + }, + } + + for tName, tCase := range testCases { + tName, tCase := tName, tCase + t.Run(tName, func(t *testing.T) { + t.Parallel() + request := validator.SetRequest{ + Path: path.Root("test"), + PathExpression: path.MatchRoot("test"), + ConfigValue: types.SetValueMust(types.ObjectType{AttrTypes: tCase.attrTypes}, tCase.attrValues), + } + response := validator.SetResponse{} + validator := UniquteValueCombinationsAt(tCase.keyAttrNames...) + validator.ValidateSet(ctx, request, &response) + + if !response.Diagnostics.HasError() && tCase.expectError { + t.Fatal("expected error, got no error") + } + + if response.Diagnostics.HasError() && !tCase.expectError { + t.Fatalf("got unexpected error: %s", response.Diagnostics) + } + + if response.Diagnostics.HasError() { + for _, error := range response.Diagnostics.Errors() { + log.Println(validator.Description(ctx)) + log.Println(error.Summary()) + log.Println(error.Detail()) + } + } + }) + } +} From 9490ea1ea11b0848e2f0bc5ebbf9b1cac575623b Mon Sep 17 00:00:00 2001 From: Chris Marget Date: Tue, 19 Sep 2023 17:46:07 -0400 Subject: [PATCH 2/5] centralize logic into dedicated function; add list/map validation --- apstra/apstra_validator/attribute_conflict.go | 294 ++++++++---------- .../attribute_conflict_test.go | 153 ++++++--- 2 files changed, 241 insertions(+), 206 deletions(-) diff --git a/apstra/apstra_validator/attribute_conflict.go b/apstra/apstra_validator/attribute_conflict.go index 6e24e259..cf27bfe1 100644 --- a/apstra/apstra_validator/attribute_conflict.go +++ b/apstra/apstra_validator/attribute_conflict.go @@ -4,14 +4,17 @@ import ( "context" "encoding/base64" "fmt" + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-framework/types/basetypes" "strings" ) type CollectionValidator interface { - //validator.List - //validator.Map + validator.List + validator.Map validator.Set } @@ -47,193 +50,150 @@ func (o attributeConflictValidator) MarkdownDescription(ctx context.Context) str return o.Description(ctx) } -//func (o attributeConflictValidator) ValidateList(ctx context.Context, req validator.ListRequest, resp *validator.ListResponse) { -// if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() { -// return -// } -// -// uniqueStrings := make(map[string]bool) -// -// for i, element := range req.ConfigValue.Elements() { -// objectPath := req.Path.AtListIndex(i) -// -// objectValuable, ok := element.(basetypes.ObjectValuable) -// if !ok { -// resp.Diagnostics.AddAttributeError( -// req.Path, -// "Invalid Validator for Element Value", -// "While performing schema-based validation, an unexpected error occurred. "+ -// "The attribute declares a Object values validator, however its values do not implement the types.ObjectValuable interface for custom Object types. "+ -// "This is likely an issue with terraform-plugin-framework and should be reported to the provider developers.\n\n"+ -// fmt.Sprintf("Path: %s\n", req.Path.String())+ -// fmt.Sprintf("Element Type: %T\n", req.ConfigValue.ElementType(ctx))+ -// fmt.Sprintf("Element Value Type: %T\n", element), -// ) -// -// return -// } -// -// objectValue, diags := objectValuable.ToObjectValue(ctx) -// resp.Diagnostics.Append(diags...) -// if diags.HasError() { -// return -// } -// -// for attrName, attrValue := range objectValue.Attributes() { -// if attrName != o.keyAttrs || attrValue.IsUnknown() { -// continue -// } -// -// if uniqueStrings[attrValue.String()] { -// resp.Diagnostics.AddAttributeError( -// objectPath, -// fmt.Sprintf("%s collision", o.keyAttrs), -// fmt.Sprintf("Two objects cannot use the same %s", o.keyAttrs), -// ) -// } else { -// uniqueStrings[attrValue.String()] = true -// } -// break // the correct attribute has been found; move on to the next -// } -// } -//} -// -//func (o attributeConflictValidator) ValidateMap(ctx context.Context, req validator.MapRequest, resp *validator.MapResponse) { -// if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() { -// return -// } -// -// uniqueStrings := make(map[string]bool) -// -// for mapKey, element := range req.ConfigValue.Elements() { -// objectPath := req.Path.AtMapKey(mapKey) -// -// objectValuable, ok := element.(basetypes.ObjectValuable) -// if !ok { -// resp.Diagnostics.AddAttributeError( -// req.Path, -// "Invalid Validator for Element Value", -// "While performing schema-based validation, an unexpected error occurred. "+ -// "The attribute declares a Object values validator, however its values do not implement the types.ObjectValuable interface for custom Object types. "+ -// "This is likely an issue with terraform-plugin-framework and should be reported to the provider developers.\n\n"+ -// fmt.Sprintf("Path: %s\n", req.Path.String())+ -// fmt.Sprintf("Element Type: %T\n", req.ConfigValue.ElementType(ctx))+ -// fmt.Sprintf("Element Value Type: %T\n", element), -// ) -// -// return -// } -// -// objectValue, diags := objectValuable.ToObjectValue(ctx) -// resp.Diagnostics.Append(diags...) -// if diags.HasError() { -// return -// } -// -// for attrName, attrValue := range objectValue.Attributes() { -// if attrName != o.keyAttrs || attrValue.IsUnknown() { -// continue -// } -// -// if uniqueStrings[attrValue.String()] { -// resp.Diagnostics.AddAttributeError( -// objectPath, -// fmt.Sprintf("%s collision", o.keyAttrs), -// fmt.Sprintf("Two objects cannot use the same %s", o.keyAttrs), -// ) -// } else { -// uniqueStrings[attrValue.String()] = true -// } -// break // the correct attribute has been found; move on to the next -// } -// } -//} +func (o attributeConflictValidator) ValidateList(ctx context.Context, req validator.ListRequest, resp *validator.ListResponse) { + if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() { + return + } -func (o attributeConflictValidator) ValidateSet(ctx context.Context, req validator.SetRequest, resp *validator.SetResponse) { + foundKeyValueCombinations := make(map[string]bool) + for i, element := range req.ConfigValue.Elements() { // loop over set members + validateRequest := attributeConflictValidateElementRequest{ + elementValue: element, + elementPath: req.Path.AtListIndex(i), + foundKeyValueCombinations: foundKeyValueCombinations, + path: req.Path, + } + validateResponse := attributeConflictValidateElementResponse{} + o.validateElement(ctx, validateRequest, &validateResponse) + resp.Diagnostics.Append(validateResponse.Diagnostics...) + } +} + +func (o attributeConflictValidator) ValidateMap(ctx context.Context, req validator.MapRequest, resp *validator.MapResponse) { if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() { return } - // map of key attribute names - keyAttributeNames := make(map[string]bool, len(o.keyAttrs)) - for _, key := range o.keyAttrs { - keyAttributeNames[key] = true + foundKeyValueCombinations := make(map[string]bool) + for mapKey, element := range req.ConfigValue.Elements() { // loop over set members + validateRequest := attributeConflictValidateElementRequest{ + elementValue: element, + elementPath: req.Path.AtMapKey(mapKey), + foundKeyValueCombinations: foundKeyValueCombinations, + path: req.Path, + } + validateResponse := attributeConflictValidateElementResponse{} + o.validateElement(ctx, validateRequest, &validateResponse) + resp.Diagnostics.Append(validateResponse.Diagnostics...) } +} - // map of found value combinations delimited by ':' - // base64(key1):base64(key2):...:base64(keyN) - foundKeyValueCombinations := make(map[string]bool) // found value combinations +func (o attributeConflictValidator) ValidateSet(ctx context.Context, req validator.SetRequest, resp *validator.SetResponse) { + if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() { + return + } + foundKeyValueCombinations := make(map[string]bool) for _, element := range req.ConfigValue.Elements() { // loop over set members - objectPath := req.Path.AtSetValue(element) + validateRequest := attributeConflictValidateElementRequest{ + elementValue: element, + elementPath: req.Path.AtSetValue(element), + foundKeyValueCombinations: foundKeyValueCombinations, + path: req.Path, + } + validateResponse := attributeConflictValidateElementResponse{} + o.validateElement(ctx, validateRequest, &validateResponse) + resp.Diagnostics.Append(validateResponse.Diagnostics...) + } +} - objectValuable, ok := element.(basetypes.ObjectValuable) - if !ok { - resp.Diagnostics.AddAttributeError( - req.Path, - "Invalid Validator for Element Value", - "While performing schema-based validation, an unexpected error occurred. "+ - "The attribute declares a Object values validator, however its values do not implement the types.ObjectValuable interface for custom Object types. "+ - "This is likely an issue with terraform-plugin-framework and should be reported to the provider developers.\n\n"+ - fmt.Sprintf("Path: %s\n", req.Path.String())+ - fmt.Sprintf("Element Type: %T\n", req.ConfigValue.ElementType(ctx))+ - fmt.Sprintf("Element Value Type: %T\n", element), - ) +func UniqueValueCombinationsAt(attrNames ...string) CollectionValidator { + return attributeConflictValidator{ + keyAttrs: attrNames, + } +} - return - } +type attributeConflictValidateElementRequest struct { + elementValue attr.Value + elementPath path.Path + foundKeyValueCombinations map[string]bool + path path.Path +} - objectValue, diags := objectValuable.ToObjectValue(ctx) - resp.Diagnostics.Append(diags...) - if diags.HasError() { - return - } +type attributeConflictValidateElementResponse struct { + Diagnostics diag.Diagnostics +} - if len(o.keyAttrs) == 0 { - // the caller didn't specify any "key" attributes, so we'll use all of them - for k := range objectValue.Attributes() { - o.keyAttrs = append(o.keyAttrs, k) - keyAttributeNames[k] = true - } +func (o *attributeConflictValidator) validateElement(ctx context.Context, req attributeConflictValidateElementRequest, resp *attributeConflictValidateElementResponse) { + objectValuable, ok := req.elementValue.(basetypes.ObjectValuable) + if !ok { + resp.Diagnostics.AddAttributeError( + req.path, + "Invalid Validator for Element Value", + "While performing schema-based validation, an unexpected error occurred. "+ + "The attribute declares a Object values validator, however its values do not implement the types.ObjectValuable interface for custom Object types. "+ + "This is likely an issue with terraform-plugin-framework and should be reported to the provider developers.\n\n"+ + fmt.Sprintf("Path: %s\n", req.path.String())+ + fmt.Sprintf("Element Type: %T\n", req.elementValue.Type(ctx))+ + fmt.Sprintf("Element Value Type: %T\n", req.elementValue), + ) + + return + } + + objectValue, d := objectValuable.ToObjectValue(ctx) + resp.Diagnostics.Append(d...) + if resp.Diagnostics.HasError() { + return + } + + // if the caller didn't specify any "key" attributes we use all of them + if len(o.keyAttrs) == 0 { + for k := range objectValue.Attributes() { + o.keyAttrs = append(o.keyAttrs, k) } + } - keyValuesMap := make(map[string]string, len(keyAttributeNames)) - for attrName, attrValue := range objectValue.Attributes() { // loop over set member attributes - if !keyAttributeNames[attrName] { - continue // attribute is not interesting - } + // map of key attribute names used to quickly recognize whether an attribute is interesting + keyAttributeNames := make(map[string]bool, len(o.keyAttrs)) + for _, key := range o.keyAttrs { + keyAttributeNames[key] = true + } - if attrValue.IsUnknown() { - return // cannot validate when attribute is unknown - } + keyValuesMap := make(map[string]string, len(keyAttributeNames)) + for attrName, attrValue := range objectValue.Attributes() { // loop over set member attributes + if !keyAttributeNames[attrName] { + continue // attribute is not interesting + } - keyValuesMap[attrName] = base64.StdEncoding.EncodeToString([]byte(attrValue.String())) - if len(keyValuesMap) < len(keyAttributeNames) { - continue // keep going until we fill keyValuesMap - } + if attrValue.IsUnknown() { + return // cannot validate when attribute is unknown + } - sb := strings.Builder{} - sb.WriteString(keyValuesMap[o.keyAttrs[0]]) // keyAttrs always has at least 1 entry - for i := range o.keyAttrs[1:] { - sb.WriteString(":" + keyValuesMap[o.keyAttrs[i]]) - } + keyValuesMap[attrName] = base64.StdEncoding.EncodeToString([]byte(attrValue.String())) + if len(keyValuesMap) < len(keyAttributeNames) { + continue // keep going until we fill keyValuesMap + } - if foundKeyValueCombinations[sb.String()] { // seen this value before? - resp.Diagnostics.AddAttributeError( - objectPath, - fmt.Sprintf("%s collision", o.keyAttrs), - fmt.Sprintf("Two objects cannot use the same %s", o.keyAttrs), - ) + sb := strings.Builder{} + for i := range o.keyAttrs { + if i == 0 { + sb.WriteString(keyValuesMap[o.keyAttrs[i]]) } else { - foundKeyValueCombinations[sb.String()] = true // log the name for future collision checks + sb.WriteString(":" + keyValuesMap[o.keyAttrs[i]]) } - break // all of the the required attribute have been found; move on to the next set member } - } -} -func UniquteValueCombinationsAt(attrNames ...string) CollectionValidator { - return attributeConflictValidator{ - keyAttrs: attrNames, + if req.foundKeyValueCombinations[sb.String()] { // seen this value before? + resp.Diagnostics.AddAttributeError( + req.elementPath, + fmt.Sprintf("%s collision", o.keyAttrs), + fmt.Sprintf("Two objects cannot use the same %s", o.keyAttrs), + ) + } else { + req.foundKeyValueCombinations[sb.String()] = true // log the name for future collision checks + } + break // all of the the required attribute have been found; move on to the next set member } + } diff --git a/apstra/apstra_validator/attribute_conflict_test.go b/apstra/apstra_validator/attribute_conflict_test.go index 6b57a39a..02354860 100644 --- a/apstra/apstra_validator/attribute_conflict_test.go +++ b/apstra/apstra_validator/attribute_conflict_test.go @@ -10,24 +10,34 @@ import ( "testing" ) -func TestAttributeConflictValidator_ValidateSet(t *testing.T) { +func TestAttributeConflictValidator(t *testing.T) { ctx := context.Background() type testCase struct { keyAttrNames []string attrTypes map[string]attr.Type - attrValues []attr.Value + attrValues map[string]attr.Value expectError bool } + attrValueSlice := func(in map[string]attr.Value) []attr.Value { + result := make([]attr.Value, len(in)) + var i int + for _, attrValue := range in { + result[i] = attrValue + i++ + } + return result + } + testCases := map[string]testCase{ "one_key_no_collision": { keyAttrNames: []string{"key1"}, attrTypes: map[string]attr.Type{ "key1": types.StringType, }, - attrValues: []attr.Value{ - types.ObjectValueMust( + attrValues: map[string]attr.Value{ + "one": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, }, @@ -35,7 +45,7 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { "key1": types.StringValue("foo"), }, ), - types.ObjectValueMust( + "two": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, }, @@ -51,8 +61,8 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { attrTypes: map[string]attr.Type{ "key1": types.StringType, }, - attrValues: []attr.Value{ - types.ObjectValueMust( + attrValues: map[string]attr.Value{ + "one": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, }, @@ -60,7 +70,7 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { "key1": types.StringValue("foo"), }, ), - types.ObjectValueMust( + "two": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, }, @@ -78,8 +88,8 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { "extra1": types.StringType, "extra2": types.StringType, }, - attrValues: []attr.Value{ - types.ObjectValueMust( + attrValues: map[string]attr.Value{ + "one": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, "extra1": types.StringType, @@ -91,7 +101,7 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { "extra2": types.StringValue("foo"), }, ), - types.ObjectValueMust( + "two": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, "extra1": types.StringType, @@ -113,8 +123,8 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { "extra1": types.StringType, "extra2": types.StringType, }, - attrValues: []attr.Value{ - types.ObjectValueMust( + attrValues: map[string]attr.Value{ + "one": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, "extra1": types.StringType, @@ -126,7 +136,7 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { "extra2": types.StringValue("baz"), }, ), - types.ObjectValueMust( + "two": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, "extra1": types.StringType, @@ -148,8 +158,8 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { "key2": types.StringType, "key3": types.StringType, }, - attrValues: []attr.Value{ - types.ObjectValueMust( + attrValues: map[string]attr.Value{ + "one": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, "key2": types.StringType, @@ -161,7 +171,7 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { "key3": types.StringValue("baz"), }, ), - types.ObjectValueMust( + "two": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, "key2": types.StringType, @@ -186,8 +196,8 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { "extra2": types.StringType, "extra3": types.StringType, }, - attrValues: []attr.Value{ - types.ObjectValueMust( + attrValues: map[string]attr.Value{ + "one": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, "key2": types.StringType, @@ -205,7 +215,7 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { "extra3": types.StringValue("baz"), }, ), - types.ObjectValueMust( + "two": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, "key2": types.StringType, @@ -233,8 +243,8 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { "key2": types.StringType, "key3": types.StringType, }, - attrValues: []attr.Value{ - types.ObjectValueMust( + attrValues: map[string]attr.Value{ + "one": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, "key2": types.StringType, @@ -246,7 +256,7 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { "key3": types.StringValue("baz"), }, ), - types.ObjectValueMust( + "two": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, "key2": types.StringType, @@ -271,8 +281,8 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { "extra2": types.StringType, "extra3": types.StringType, }, - attrValues: []attr.Value{ - types.ObjectValueMust( + attrValues: map[string]attr.Value{ + "one": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, "key2": types.StringType, @@ -290,7 +300,7 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { "extra3": types.StringValue("baz"), }, ), - types.ObjectValueMust( + "two": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, "key2": types.StringType, @@ -318,8 +328,8 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { "key2": types.StringType, "key3": types.StringType, }, - attrValues: []attr.Value{ - types.ObjectValueMust( + attrValues: map[string]attr.Value{ + "one": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, "key2": types.StringType, @@ -331,7 +341,7 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { "key3": types.StringValue("baz"), }, ), - types.ObjectValueMust( + "two": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, "key2": types.StringType, @@ -353,8 +363,8 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { "key2": types.StringType, "key3": types.StringType, }, - attrValues: []attr.Value{ - types.ObjectValueMust( + attrValues: map[string]attr.Value{ + "one": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, "key2": types.StringType, @@ -366,7 +376,7 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { "key3": types.StringValue("baz"), }, ), - types.ObjectValueMust( + "two": types.ObjectValueMust( map[string]attr.Type{ "key1": types.StringType, "key2": types.StringType, @@ -383,6 +393,71 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { }, } + // test list validation + for tName, tCase := range testCases { + tName, tCase := tName, tCase + t.Run(tName, func(t *testing.T) { + t.Parallel() + request := validator.ListRequest{ + Path: path.Root("test"), + PathExpression: path.MatchRoot("test"), + ConfigValue: types.ListValueMust(types.ObjectType{AttrTypes: tCase.attrTypes}, attrValueSlice(tCase.attrValues)), + } + response := validator.ListResponse{} + v := UniqueValueCombinationsAt(tCase.keyAttrNames...) + v.ValidateList(ctx, request, &response) + + if !response.Diagnostics.HasError() && tCase.expectError { + t.Fatal("expected error, got no error") + } + + if response.Diagnostics.HasError() && !tCase.expectError { + t.Fatalf("got unexpected error: %s", response.Diagnostics) + } + + if response.Diagnostics.HasError() { + for _, diags := range response.Diagnostics.Errors() { + log.Println(v.Description(ctx)) + log.Println(diags.Summary()) + log.Println(diags.Detail()) + } + } + }) + } + + // test map validation + for tName, tCase := range testCases { + tName, tCase := tName, tCase + t.Run(tName, func(t *testing.T) { + t.Parallel() + request := validator.MapRequest{ + Path: path.Root("test"), + PathExpression: path.MatchRoot("test"), + ConfigValue: types.MapValueMust(types.ObjectType{AttrTypes: tCase.attrTypes}, tCase.attrValues), + } + response := validator.MapResponse{} + v := UniqueValueCombinationsAt(tCase.keyAttrNames...) + v.ValidateMap(ctx, request, &response) + + if !response.Diagnostics.HasError() && tCase.expectError { + t.Fatal("expected error, got no error") + } + + if response.Diagnostics.HasError() && !tCase.expectError { + t.Fatalf("got unexpected error: %s", response.Diagnostics) + } + + if response.Diagnostics.HasError() { + for _, diags := range response.Diagnostics.Errors() { + log.Println(v.Description(ctx)) + log.Println(diags.Summary()) + log.Println(diags.Detail()) + } + } + }) + } + + // test set validation for tName, tCase := range testCases { tName, tCase := tName, tCase t.Run(tName, func(t *testing.T) { @@ -390,11 +465,11 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { request := validator.SetRequest{ Path: path.Root("test"), PathExpression: path.MatchRoot("test"), - ConfigValue: types.SetValueMust(types.ObjectType{AttrTypes: tCase.attrTypes}, tCase.attrValues), + ConfigValue: types.SetValueMust(types.ObjectType{AttrTypes: tCase.attrTypes}, attrValueSlice(tCase.attrValues)), } response := validator.SetResponse{} - validator := UniquteValueCombinationsAt(tCase.keyAttrNames...) - validator.ValidateSet(ctx, request, &response) + v := UniqueValueCombinationsAt(tCase.keyAttrNames...) + v.ValidateSet(ctx, request, &response) if !response.Diagnostics.HasError() && tCase.expectError { t.Fatal("expected error, got no error") @@ -405,10 +480,10 @@ func TestAttributeConflictValidator_ValidateSet(t *testing.T) { } if response.Diagnostics.HasError() { - for _, error := range response.Diagnostics.Errors() { - log.Println(validator.Description(ctx)) - log.Println(error.Summary()) - log.Println(error.Detail()) + for _, diags := range response.Diagnostics.Errors() { + log.Println(v.Description(ctx)) + log.Println(diags.Summary()) + log.Println(diags.Detail()) } } }) From d5ea44f4761eaec4c9cbbfca9b067bbd30f221b9 Mon Sep 17 00:00:00 2001 From: Chris Marget Date: Tue, 19 Sep 2023 19:37:06 -0400 Subject: [PATCH 3/5] remove stray whitespace --- apstra/apstra_validator/attribute_conflict.go | 1 - 1 file changed, 1 deletion(-) diff --git a/apstra/apstra_validator/attribute_conflict.go b/apstra/apstra_validator/attribute_conflict.go index cf27bfe1..36f6ddb0 100644 --- a/apstra/apstra_validator/attribute_conflict.go +++ b/apstra/apstra_validator/attribute_conflict.go @@ -195,5 +195,4 @@ func (o *attributeConflictValidator) validateElement(ctx context.Context, req at } break // all of the the required attribute have been found; move on to the next set member } - } From fcb5d9c586e3e8a6d784e03315622288904d3754 Mon Sep 17 00:00:00 2001 From: Chris Marget Date: Wed, 20 Sep 2023 13:59:53 -0400 Subject: [PATCH 4/5] add support for case-insensitive collision detection --- apstra/apstra_validator/attribute_conflict.go | 46 +++++++++-- .../attribute_conflict_test.go | 82 +++++++++++++++++-- 2 files changed, 112 insertions(+), 16 deletions(-) diff --git a/apstra/apstra_validator/attribute_conflict.go b/apstra/apstra_validator/attribute_conflict.go index 36f6ddb0..40df890f 100644 --- a/apstra/apstra_validator/attribute_conflict.go +++ b/apstra/apstra_validator/attribute_conflict.go @@ -32,7 +32,8 @@ var _ CollectionValidator = attributeConflictValidator{} // // If keyAttrs is empty, then values across all attributes are evaluated. type attributeConflictValidator struct { - keyAttrs []string + keyAttrs []string + caseInsensitive bool } func (o attributeConflictValidator) Description(_ context.Context) string { @@ -107,12 +108,6 @@ func (o attributeConflictValidator) ValidateSet(ctx context.Context, req validat } } -func UniqueValueCombinationsAt(attrNames ...string) CollectionValidator { - return attributeConflictValidator{ - keyAttrs: attrNames, - } -} - type attributeConflictValidateElementRequest struct { elementValue attr.Value elementPath path.Path @@ -170,7 +165,14 @@ func (o *attributeConflictValidator) validateElement(ctx context.Context, req at return // cannot validate when attribute is unknown } - keyValuesMap[attrName] = base64.StdEncoding.EncodeToString([]byte(attrValue.String())) + var valueToCompare string // a configured value we're checking for unique-ness + if o.caseInsensitive { + valueToCompare = strings.ToLower(attrValue.String()) + } else { + valueToCompare = attrValue.String() + } + + keyValuesMap[attrName] = base64.StdEncoding.EncodeToString([]byte(valueToCompare)) if len(keyValuesMap) < len(keyAttributeNames) { continue // keep going until we fill keyValuesMap } @@ -185,10 +187,23 @@ func (o *attributeConflictValidator) validateElement(ctx context.Context, req at } if req.foundKeyValueCombinations[sb.String()] { // seen this value before? + var detailedError string + switch len(o.keyAttrs) { + case 0: + detailedError = fmt.Sprintf("Two objects cannot use the same value "+ + "combination across all attributes (case sensitive: %t", o.caseInsensitive) + case 1: + detailedError = fmt.Sprintf("Two objects cannot use the same value for "+ + "'%s' (case sensitive: %t)", o.keyAttrs[0], o.caseInsensitive) + default: + detailedError = fmt.Sprintf("Two objects cannot use the same value "+ + "combination for these attributes: ['%s'] (case sensitive: %t)", + strings.Join(o.keyAttrs, "', '"), o.caseInsensitive) + } resp.Diagnostics.AddAttributeError( req.elementPath, fmt.Sprintf("%s collision", o.keyAttrs), - fmt.Sprintf("Two objects cannot use the same %s", o.keyAttrs), + detailedError, ) } else { req.foundKeyValueCombinations[sb.String()] = true // log the name for future collision checks @@ -196,3 +211,16 @@ func (o *attributeConflictValidator) validateElement(ctx context.Context, req at break // all of the the required attribute have been found; move on to the next set member } } + +func UniqueValueCombinationsAt(attrNames ...string) CollectionValidator { + return attributeConflictValidator{ + keyAttrs: attrNames, + } +} + +func UniqueInsensitiveValueCombinationsAt(attrNames ...string) CollectionValidator { + return attributeConflictValidator{ + keyAttrs: attrNames, + caseInsensitive: true, + } +} diff --git a/apstra/apstra_validator/attribute_conflict_test.go b/apstra/apstra_validator/attribute_conflict_test.go index 02354860..18fb1ee2 100644 --- a/apstra/apstra_validator/attribute_conflict_test.go +++ b/apstra/apstra_validator/attribute_conflict_test.go @@ -14,10 +14,11 @@ func TestAttributeConflictValidator(t *testing.T) { ctx := context.Background() type testCase struct { - keyAttrNames []string - attrTypes map[string]attr.Type - attrValues map[string]attr.Value - expectError bool + keyAttrNames []string + attrTypes map[string]attr.Type + attrValues map[string]attr.Value + expectError bool + caseInsensitive bool } attrValueSlice := func(in map[string]attr.Value) []attr.Value { @@ -391,6 +392,58 @@ func TestAttributeConflictValidator(t *testing.T) { }, expectError: true, }, + "one_key_no_collision_case_sensitive": { + keyAttrNames: []string{"key1"}, + attrTypes: map[string]attr.Type{ + "key1": types.StringType, + }, + attrValues: map[string]attr.Value{ + "one": types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("foo"), + }, + ), + "two": types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("FOO"), + }, + ), + }, + expectError: false, + caseInsensitive: false, + }, + "one_key_collision_case_insensitive": { + keyAttrNames: []string{"key1"}, + attrTypes: map[string]attr.Type{ + "key1": types.StringType, + }, + attrValues: map[string]attr.Value{ + "one": types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("foo"), + }, + ), + "two": types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("FOO"), + }, + ), + }, + expectError: true, + caseInsensitive: true, + }, } // test list validation @@ -404,7 +457,12 @@ func TestAttributeConflictValidator(t *testing.T) { ConfigValue: types.ListValueMust(types.ObjectType{AttrTypes: tCase.attrTypes}, attrValueSlice(tCase.attrValues)), } response := validator.ListResponse{} - v := UniqueValueCombinationsAt(tCase.keyAttrNames...) + var v CollectionValidator + if tCase.caseInsensitive { + v = UniqueInsensitiveValueCombinationsAt(tCase.keyAttrNames...) + } else { + v = UniqueValueCombinationsAt(tCase.keyAttrNames...) + } v.ValidateList(ctx, request, &response) if !response.Diagnostics.HasError() && tCase.expectError { @@ -436,7 +494,12 @@ func TestAttributeConflictValidator(t *testing.T) { ConfigValue: types.MapValueMust(types.ObjectType{AttrTypes: tCase.attrTypes}, tCase.attrValues), } response := validator.MapResponse{} - v := UniqueValueCombinationsAt(tCase.keyAttrNames...) + var v CollectionValidator + if tCase.caseInsensitive { + v = UniqueInsensitiveValueCombinationsAt(tCase.keyAttrNames...) + } else { + v = UniqueValueCombinationsAt(tCase.keyAttrNames...) + } v.ValidateMap(ctx, request, &response) if !response.Diagnostics.HasError() && tCase.expectError { @@ -468,7 +531,12 @@ func TestAttributeConflictValidator(t *testing.T) { ConfigValue: types.SetValueMust(types.ObjectType{AttrTypes: tCase.attrTypes}, attrValueSlice(tCase.attrValues)), } response := validator.SetResponse{} - v := UniqueValueCombinationsAt(tCase.keyAttrNames...) + var v CollectionValidator + if tCase.caseInsensitive { + v = UniqueInsensitiveValueCombinationsAt(tCase.keyAttrNames...) + } else { + v = UniqueValueCombinationsAt(tCase.keyAttrNames...) + } v.ValidateSet(ctx, request, &response) if !response.Diagnostics.HasError() && tCase.expectError { From 6c51e898c4abf00bd65ee75322c84fa3e524504e Mon Sep 17 00:00:00 2001 From: Chris Marget Date: Fri, 22 Sep 2023 12:07:41 -0400 Subject: [PATCH 5/5] move conflict checking out of the loop --- apstra/apstra_validator/attribute_conflict.go | 71 +++++++++++-------- .../attribute_conflict_test.go | 17 +++++ 2 files changed, 58 insertions(+), 30 deletions(-) diff --git a/apstra/apstra_validator/attribute_conflict.go b/apstra/apstra_validator/attribute_conflict.go index 40df890f..84b143bf 100644 --- a/apstra/apstra_validator/attribute_conflict.go +++ b/apstra/apstra_validator/attribute_conflict.go @@ -173,42 +173,53 @@ func (o *attributeConflictValidator) validateElement(ctx context.Context, req at } keyValuesMap[attrName] = base64.StdEncoding.EncodeToString([]byte(valueToCompare)) - if len(keyValuesMap) < len(keyAttributeNames) { - continue // keep going until we fill keyValuesMap + if len(keyValuesMap) == len(keyAttributeNames) { + break // keyValuesMap is full, no need to look at remaining attributes } + } - sb := strings.Builder{} - for i := range o.keyAttrs { - if i == 0 { - sb.WriteString(keyValuesMap[o.keyAttrs[i]]) - } else { - sb.WriteString(":" + keyValuesMap[o.keyAttrs[i]]) - } + // did we find all of the required "key attributes" ? + if len(keyValuesMap) < len(keyAttributeNames) { + // collect object's attribute names so we can complain about them + var attrNames []string + for attrName := range objectValue.Attributes() { + attrNames = append(attrNames, attrName) } - if req.foundKeyValueCombinations[sb.String()] { // seen this value before? - var detailedError string - switch len(o.keyAttrs) { - case 0: - detailedError = fmt.Sprintf("Two objects cannot use the same value "+ - "combination across all attributes (case sensitive: %t", o.caseInsensitive) - case 1: - detailedError = fmt.Sprintf("Two objects cannot use the same value for "+ - "'%s' (case sensitive: %t)", o.keyAttrs[0], o.caseInsensitive) - default: - detailedError = fmt.Sprintf("Two objects cannot use the same value "+ - "combination for these attributes: ['%s'] (case sensitive: %t)", - strings.Join(o.keyAttrs, "', '"), o.caseInsensitive) - } - resp.Diagnostics.AddAttributeError( - req.elementPath, - fmt.Sprintf("%s collision", o.keyAttrs), - detailedError, - ) + resp.Diagnostics.AddAttributeError( + req.path, + "Invalid Validator for Element Value", + "While performing schema-based validation, an unexpected error occurred. "+ + "The attribute declares an Object values validator which has been asked "+ + "to validate attributes not present in the object. "+ + "This issue should be reported to the provider developers.\n\n"+ + fmt.Sprintf("Path: %s\n", req.path.String())+ + fmt.Sprintf("Element Attributes: '%s'\n", strings.Join(attrNames, "', '"))+ + fmt.Sprintf("Element Attributes to validate: '%s'\n", strings.Join(o.keyAttrs, "', '")), + ) + + return + } + + sb := strings.Builder{} + for i := range o.keyAttrs { + if i == 0 { + sb.WriteString(keyValuesMap[o.keyAttrs[i]]) } else { - req.foundKeyValueCombinations[sb.String()] = true // log the name for future collision checks + sb.WriteString(":" + keyValuesMap[o.keyAttrs[i]]) } - break // all of the the required attribute have been found; move on to the next set member + } + + if req.foundKeyValueCombinations[sb.String()] { // seen this value before? + resp.Diagnostics.AddAttributeError( + req.elementPath, + fmt.Sprintf("%s collision", o.keyAttrs), + fmt.Sprintf("Two objects cannot use the same value "+ + "combination for these attributes: ['%s'] (case sensitive: %t)", + strings.Join(o.keyAttrs, "', '"), o.caseInsensitive), + ) + } else { + req.foundKeyValueCombinations[sb.String()] = true // log the name for future collision checks } } diff --git a/apstra/apstra_validator/attribute_conflict_test.go b/apstra/apstra_validator/attribute_conflict_test.go index 18fb1ee2..71dba9fd 100644 --- a/apstra/apstra_validator/attribute_conflict_test.go +++ b/apstra/apstra_validator/attribute_conflict_test.go @@ -444,6 +444,23 @@ func TestAttributeConflictValidator(t *testing.T) { expectError: true, caseInsensitive: true, }, + "missing_key": { + keyAttrNames: []string{"key1", "key2"}, + attrTypes: map[string]attr.Type{ + "key1": types.StringType, + }, + attrValues: map[string]attr.Value{ + "one": types.ObjectValueMust( + map[string]attr.Type{ + "key1": types.StringType, + }, + map[string]attr.Value{ + "key1": types.StringValue("foo"), + }, + ), + }, + expectError: true, + }, } // test list validation