Skip to content

Commit 83df4b6

Browse files
rainkwanrainkwan
authored
Implement validation logic for "identityschema.ListAttribute" (#1125)
* Checks the element type of the identityschema.ListAttribute to make sure it is an allowed primitive * Fixed linters
1 parent b25d4c2 commit 83df4b6

File tree

5 files changed

+247
-4
lines changed

5 files changed

+247
-4
lines changed

‎internal/fwschema/diagnostics.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,19 @@ func AttributeDefaultTypeMismatchDiag(attributePath path.Path, expectedType attr
6464
"The default value must match the type of the schema.",
6565
)
6666
}
67+
68+
// AttributeInvalidElementTypeDiag returns an error diagnostic to provider
69+
// developers about using non-primitive types in their Attribute
70+
// implementation. This is not allowed.
71+
func AttributeInvalidElementTypeDiag(attributePath path.Path, actualType attr.Type) diag.Diagnostic {
72+
// The diagnostic path is intentionally omitted as it is invalid in this
73+
// context. Diagnostic paths are intended to be mapped to actual data,
74+
// while this path information must be synthesized.
75+
return diag.NewErrorDiagnostic(
76+
"Invalid Attribute Implementation",
77+
"When validating the schema, an implementation issue was found. "+
78+
"This is always an issue with the provider and should be reported to the provider developers.\n\n"+
79+
fmt.Sprintf("%q contains an element of type %q that is not allowed for Lists in Resource Identity. ", attributePath, actualType)+
80+
"Lists in Resource Identity may only have primitive element types such as Bool, Int, Float, Number and String.",
81+
)
82+
}

‎internal/fwtype/static_collection_validation.go‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package fwtype
55

66
import (
77
"fmt"
8-
98
"github.com/hashicorp/terraform-plugin-framework/attr"
109
"github.com/hashicorp/terraform-plugin-framework/diag"
1110
"github.com/hashicorp/terraform-plugin-framework/path"
@@ -140,3 +139,19 @@ func ReturnCollectionWithDynamicTypeDiag() diag.Diagnostic {
140139
"If underlying dynamic values are required, replace the return definition with DynamicReturn instead.",
141140
)
142141
}
142+
143+
// IsAllowedPrimitiveType checks if the given attr.Type is an allowed primitive type for resource identity.
144+
func IsAllowedPrimitiveType(typ attr.Type) bool {
145+
switch typ.(type) {
146+
case basetypes.BoolTypable,
147+
basetypes.Float64Typable,
148+
basetypes.Float32Typable,
149+
basetypes.Int64Typable,
150+
basetypes.Int32Typable,
151+
basetypes.NumberTypable,
152+
basetypes.StringTypable:
153+
return true
154+
default:
155+
return false
156+
}
157+
}

‎internal/fwtype/static_collection_validation_test.go‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -944,3 +944,82 @@ func TestTypeContainsCollectionWithDynamic(t *testing.T) {
944944
})
945945
}
946946
}
947+
948+
func TestIsAllowedPrimitiveType(t *testing.T) {
949+
t.Parallel()
950+
951+
testCases := map[string]struct {
952+
attrTyp attr.Type
953+
expected bool
954+
}{
955+
"nil": {
956+
attrTyp: nil,
957+
expected: false,
958+
},
959+
"dynamic": {
960+
attrTyp: types.DynamicType,
961+
expected: false,
962+
},
963+
"bool": {
964+
attrTyp: types.BoolType,
965+
expected: true,
966+
},
967+
"int64": {
968+
attrTyp: types.Int64Type,
969+
expected: true,
970+
},
971+
"int32": {
972+
attrTyp: types.Int32Type,
973+
expected: true,
974+
},
975+
"float64": {
976+
attrTyp: types.Float64Type,
977+
expected: true,
978+
},
979+
"float32": {
980+
attrTyp: types.Float32Type,
981+
expected: true,
982+
},
983+
"number": {
984+
attrTyp: types.NumberType,
985+
expected: true,
986+
},
987+
"string": {
988+
attrTyp: types.StringType,
989+
expected: true,
990+
},
991+
"list-missing": {
992+
attrTyp: types.ListType{},
993+
expected: false,
994+
},
995+
"list-static": {
996+
attrTyp: types.ListType{
997+
ElemType: types.StringType,
998+
},
999+
expected: false,
1000+
},
1001+
"map": {
1002+
attrTyp: types.MapType{},
1003+
expected: false,
1004+
},
1005+
"object": {
1006+
attrTyp: types.ObjectType{},
1007+
expected: false,
1008+
},
1009+
"tuple": {
1010+
attrTyp: types.TupleType{},
1011+
expected: false,
1012+
},
1013+
}
1014+
for name, testCase := range testCases {
1015+
t.Run(name, func(t *testing.T) {
1016+
t.Parallel()
1017+
1018+
got := fwtype.IsAllowedPrimitiveType(testCase.attrTyp)
1019+
1020+
if diff := cmp.Diff(got, testCase.expected); diff != "" {
1021+
t.Errorf("unexpected difference: %s", diff)
1022+
}
1023+
})
1024+
}
1025+
}

‎resource/identityschema/list_attribute.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package identityschema
55

66
import (
77
"context"
8+
"github.com/hashicorp/terraform-plugin-framework/internal/fwtype"
89

910
"github.com/hashicorp/terraform-plugin-go/tftypes"
1011

@@ -160,10 +161,13 @@ func (a ListAttribute) IsOptionalForImport() bool {
160161
// provider-defined implementation of the attribute to prevent unexpected
161162
// errors or panics. This logic runs during the GetResourceIdentitySchemas RPC and
162163
// should never include false positives.
163-
func (a ListAttribute) ValidateImplementation(ctx context.Context, req fwschema.ValidateImplementationRequest, resp *fwschema.ValidateImplementationResponse) {
164+
func (a ListAttribute) ValidateImplementation(_ context.Context, req fwschema.ValidateImplementationRequest, resp *fwschema.ValidateImplementationResponse) {
164165
if a.CustomType == nil && a.ElementType == nil {
165166
resp.Diagnostics.Append(fwschema.AttributeMissingElementTypeDiag(req.Path))
167+
return
166168
}
167169

168-
// TODO:ResourceIdentity: Write validation + tests that ensure the element type only contains primitive elements (bool, string, number)
170+
if a.CustomType == nil && !fwtype.IsAllowedPrimitiveType(a.ElementType) {
171+
resp.Diagnostics.Append(fwschema.AttributeInvalidElementTypeDiag(req.Path, a.ElementType))
172+
}
169173
}

‎resource/identityschema/list_attribute_test.go‎

Lines changed: 130 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@ package identityschema_test
66
import (
77
"context"
88
"fmt"
9+
"github.com/hashicorp/terraform-plugin-framework/diag"
910
"strings"
1011
"testing"
1112

1213
"github.com/google/go-cmp/cmp"
1314
"github.com/hashicorp/terraform-plugin-go/tftypes"
1415

1516
"github.com/hashicorp/terraform-plugin-framework/attr"
16-
"github.com/hashicorp/terraform-plugin-framework/diag"
1717
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
1818
"github.com/hashicorp/terraform-plugin-framework/internal/testing/testschema"
1919
"github.com/hashicorp/terraform-plugin-framework/internal/testing/testtypes"
@@ -480,6 +480,135 @@ func TestListAttributeValidateImplementation(t *testing.T) {
480480
},
481481
},
482482
},
483+
"elementtype-bool": {
484+
attribute: identityschema.ListAttribute{
485+
RequiredForImport: true,
486+
ElementType: types.BoolType,
487+
},
488+
request: fwschema.ValidateImplementationRequest{
489+
Name: "test",
490+
Path: path.Root("test"),
491+
},
492+
expected: &fwschema.ValidateImplementationResponse{},
493+
},
494+
"elementtype-int64": {
495+
attribute: identityschema.ListAttribute{
496+
RequiredForImport: true,
497+
ElementType: types.Int64Type,
498+
},
499+
request: fwschema.ValidateImplementationRequest{
500+
Name: "test",
501+
Path: path.Root("test"),
502+
},
503+
expected: &fwschema.ValidateImplementationResponse{},
504+
},
505+
"elementtype-int32": {
506+
attribute: identityschema.ListAttribute{
507+
RequiredForImport: true,
508+
ElementType: types.Int32Type,
509+
},
510+
request: fwschema.ValidateImplementationRequest{
511+
Name: "test",
512+
Path: path.Root("test"),
513+
},
514+
expected: &fwschema.ValidateImplementationResponse{},
515+
},
516+
"elementtype-float64": {
517+
attribute: identityschema.ListAttribute{
518+
RequiredForImport: true,
519+
ElementType: types.Float64Type,
520+
},
521+
request: fwschema.ValidateImplementationRequest{
522+
Name: "test",
523+
Path: path.Root("test"),
524+
},
525+
expected: &fwschema.ValidateImplementationResponse{},
526+
},
527+
"elementtype-float32": {
528+
attribute: identityschema.ListAttribute{
529+
RequiredForImport: true,
530+
ElementType: types.Float32Type,
531+
},
532+
request: fwschema.ValidateImplementationRequest{
533+
Name: "test",
534+
Path: path.Root("test"),
535+
},
536+
expected: &fwschema.ValidateImplementationResponse{},
537+
},
538+
"elementtype-number": {
539+
attribute: identityschema.ListAttribute{
540+
RequiredForImport: true,
541+
ElementType: types.NumberType,
542+
},
543+
request: fwschema.ValidateImplementationRequest{
544+
Name: "test",
545+
Path: path.Root("test"),
546+
},
547+
expected: &fwschema.ValidateImplementationResponse{},
548+
},
549+
"elementtype-notprimitive-dynamic": {
550+
attribute: identityschema.ListAttribute{
551+
RequiredForImport: true,
552+
ElementType: types.DynamicType,
553+
},
554+
request: fwschema.ValidateImplementationRequest{
555+
Name: "test",
556+
Path: path.Root("test"),
557+
},
558+
expected: &fwschema.ValidateImplementationResponse{
559+
Diagnostics: diag.Diagnostics{
560+
diag.NewErrorDiagnostic(
561+
"Invalid Attribute Implementation",
562+
"When validating the schema, an implementation issue was found. "+
563+
"This is always an issue with the provider and should be reported to the provider developers.\n\n"+
564+
"\"test\" contains an element of type \"basetypes.DynamicType\" that is not allowed for Lists in Resource Identity. "+
565+
"Lists in Resource Identity may only have primitive element types such as Bool, Int, Float, Number and String.",
566+
),
567+
},
568+
},
569+
},
570+
"elementtype-notprimitive-object": {
571+
attribute: identityschema.ListAttribute{
572+
RequiredForImport: true,
573+
ElementType: types.ObjectType{},
574+
},
575+
request: fwschema.ValidateImplementationRequest{
576+
Name: "test",
577+
Path: path.Root("test"),
578+
},
579+
expected: &fwschema.ValidateImplementationResponse{
580+
Diagnostics: diag.Diagnostics{
581+
diag.NewErrorDiagnostic(
582+
"Invalid Attribute Implementation",
583+
"When validating the schema, an implementation issue was found. "+
584+
"This is always an issue with the provider and should be reported to the provider developers.\n\n"+
585+
"\"test\" contains an element of type \"types.ObjectType[]\" that is not allowed for Lists in Resource Identity. "+
586+
"Lists in Resource Identity may only have primitive element types such as Bool, Int, Float, Number and String.",
587+
),
588+
},
589+
},
590+
},
591+
"elementtype-notprimitive-map": {
592+
attribute: identityschema.ListAttribute{
593+
RequiredForImport: true,
594+
ElementType: types.MapType{},
595+
},
596+
request: fwschema.ValidateImplementationRequest{
597+
Name: "test",
598+
Path: path.Root("test"),
599+
},
600+
expected: &fwschema.ValidateImplementationResponse{
601+
Diagnostics: diag.Diagnostics{
602+
diag.NewErrorDiagnostic(
603+
"Invalid Attribute Implementation",
604+
"When validating the schema, an implementation issue was found. "+
605+
"This is always an issue with the provider and should be reported to the provider developers.\n\n"+
606+
"\"test\" contains an element of type \"types.MapType[!!! MISSING TYPE !!!]\" that is not allowed for Lists in Resource Identity. "+
607+
"Lists in Resource Identity may only have primitive element types such as Bool, Int, Float, Number and String.",
608+
),
609+
},
610+
},
611+
},
483612
}
484613

485614
for name, testCase := range testCases {

0 commit comments

Comments
 (0)