Skip to content

Commit 8678316

Browse files
rainkwanrainkwan
committed
Checks the element type of the identityschema.ListAttribute to make sure it is an allowed primitive
1 parent b25d4c2 commit 8678316

File tree

5 files changed

+242
-4
lines changed

5 files changed

+242
-4
lines changed

‎internal/fwschema/diagnostics.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,18 @@ 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) 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 Attribute type that is not allowed for Resource Identity. ", attributePath),
80+
)
81+
}

‎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.BoolType,
147+
basetypes.Float64Type,
148+
basetypes.Float32Type,
149+
basetypes.Int64Type,
150+
basetypes.Int32Type,
151+
basetypes.NumberType,
152+
basetypes.StringType:
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: 5 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,12 @@ 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))
166167
}
167168

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

‎resource/identityschema/list_attribute_test.go‎

Lines changed: 127 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,132 @@ 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+
fmt.Sprintf("\"test\" contains an Attribute type that is not allowed for Resource Identity. "),
565+
),
566+
},
567+
},
568+
},
569+
"elementtype-notprimitive-object": {
570+
attribute: identityschema.ListAttribute{
571+
RequiredForImport: true,
572+
ElementType: types.ObjectType{},
573+
},
574+
request: fwschema.ValidateImplementationRequest{
575+
Name: "test",
576+
Path: path.Root("test"),
577+
},
578+
expected: &fwschema.ValidateImplementationResponse{
579+
Diagnostics: diag.Diagnostics{
580+
diag.NewErrorDiagnostic(
581+
"Invalid Attribute Implementation",
582+
"When validating the schema, an implementation issue was found. "+
583+
"This is always an issue with the provider and should be reported to the provider developers.\n\n"+
584+
fmt.Sprintf("\"test\" contains an Attribute type that is not allowed for Resource Identity. "),
585+
),
586+
},
587+
},
588+
},
589+
"elementtype-notprimitive-map": {
590+
attribute: identityschema.ListAttribute{
591+
RequiredForImport: true,
592+
ElementType: types.MapType{},
593+
},
594+
request: fwschema.ValidateImplementationRequest{
595+
Name: "test",
596+
Path: path.Root("test"),
597+
},
598+
expected: &fwschema.ValidateImplementationResponse{
599+
Diagnostics: diag.Diagnostics{
600+
diag.NewErrorDiagnostic(
601+
"Invalid Attribute Implementation",
602+
"When validating the schema, an implementation issue was found. "+
603+
"This is always an issue with the provider and should be reported to the provider developers.\n\n"+
604+
fmt.Sprintf("\"test\" contains an Attribute type that is not allowed for Resource Identity. "),
605+
),
606+
},
607+
},
608+
},
483609
}
484610

485611
for name, testCase := range testCases {

0 commit comments

Comments
 (0)