diff --git a/internal/fwschema/diagnostics.go b/internal/fwschema/diagnostics.go index c79531326..efb53c00f 100644 --- a/internal/fwschema/diagnostics.go +++ b/internal/fwschema/diagnostics.go @@ -64,3 +64,19 @@ func AttributeDefaultTypeMismatchDiag(attributePath path.Path, expectedType attr "The default value must match the type of the schema.", ) } + +// AttributeInvalidElementTypeDiag returns an error diagnostic to provider +// developers about using non-primitive types in their Attribute +// implementation. This is not allowed. +func AttributeInvalidElementTypeDiag(attributePath path.Path, actualType attr.Type) diag.Diagnostic { + // The diagnostic path is intentionally omitted as it is invalid in this + // context. Diagnostic paths are intended to be mapped to actual data, + // while this path information must be synthesized. + return diag.NewErrorDiagnostic( + "Invalid Attribute Implementation", + "When validating the schema, an implementation issue was found. "+ + "This is always an issue with the provider and should be reported to the provider developers.\n\n"+ + fmt.Sprintf("%q contains an element of type %q that is not allowed for Lists in Resource Identity. ", attributePath, actualType)+ + "Lists in Resource Identity may only have primitive element types such as Bool, Int, Float, Number and String.", + ) +} diff --git a/internal/fwtype/static_collection_validation.go b/internal/fwtype/static_collection_validation.go index 2a91e2fbb..50a4b9f2d 100644 --- a/internal/fwtype/static_collection_validation.go +++ b/internal/fwtype/static_collection_validation.go @@ -5,7 +5,6 @@ package fwtype import ( "fmt" - "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/path" @@ -140,3 +139,19 @@ func ReturnCollectionWithDynamicTypeDiag() diag.Diagnostic { "If underlying dynamic values are required, replace the return definition with DynamicReturn instead.", ) } + +// IsAllowedPrimitiveType checks if the given attr.Type is an allowed primitive type for resource identity. +func IsAllowedPrimitiveType(typ attr.Type) bool { + switch typ.(type) { + case basetypes.BoolTypable, + basetypes.Float64Typable, + basetypes.Float32Typable, + basetypes.Int64Typable, + basetypes.Int32Typable, + basetypes.NumberTypable, + basetypes.StringTypable: + return true + default: + return false + } +} diff --git a/internal/fwtype/static_collection_validation_test.go b/internal/fwtype/static_collection_validation_test.go index 8000bc91a..058ee3aa9 100644 --- a/internal/fwtype/static_collection_validation_test.go +++ b/internal/fwtype/static_collection_validation_test.go @@ -944,3 +944,82 @@ func TestTypeContainsCollectionWithDynamic(t *testing.T) { }) } } + +func TestIsAllowedPrimitiveType(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + attrTyp attr.Type + expected bool + }{ + "nil": { + attrTyp: nil, + expected: false, + }, + "dynamic": { + attrTyp: types.DynamicType, + expected: false, + }, + "bool": { + attrTyp: types.BoolType, + expected: true, + }, + "int64": { + attrTyp: types.Int64Type, + expected: true, + }, + "int32": { + attrTyp: types.Int32Type, + expected: true, + }, + "float64": { + attrTyp: types.Float64Type, + expected: true, + }, + "float32": { + attrTyp: types.Float32Type, + expected: true, + }, + "number": { + attrTyp: types.NumberType, + expected: true, + }, + "string": { + attrTyp: types.StringType, + expected: true, + }, + "list-missing": { + attrTyp: types.ListType{}, + expected: false, + }, + "list-static": { + attrTyp: types.ListType{ + ElemType: types.StringType, + }, + expected: false, + }, + "map": { + attrTyp: types.MapType{}, + expected: false, + }, + "object": { + attrTyp: types.ObjectType{}, + expected: false, + }, + "tuple": { + attrTyp: types.TupleType{}, + expected: false, + }, + } + for name, testCase := range testCases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + got := fwtype.IsAllowedPrimitiveType(testCase.attrTyp) + + if diff := cmp.Diff(got, testCase.expected); diff != "" { + t.Errorf("unexpected difference: %s", diff) + } + }) + } +} diff --git a/resource/identityschema/list_attribute.go b/resource/identityschema/list_attribute.go index ef396c248..185a3bba8 100644 --- a/resource/identityschema/list_attribute.go +++ b/resource/identityschema/list_attribute.go @@ -5,6 +5,7 @@ package identityschema import ( "context" + "github.com/hashicorp/terraform-plugin-framework/internal/fwtype" "github.com/hashicorp/terraform-plugin-go/tftypes" @@ -160,10 +161,13 @@ func (a ListAttribute) IsOptionalForImport() bool { // provider-defined implementation of the attribute to prevent unexpected // errors or panics. This logic runs during the GetResourceIdentitySchemas RPC and // should never include false positives. -func (a ListAttribute) ValidateImplementation(ctx context.Context, req fwschema.ValidateImplementationRequest, resp *fwschema.ValidateImplementationResponse) { +func (a ListAttribute) ValidateImplementation(_ context.Context, req fwschema.ValidateImplementationRequest, resp *fwschema.ValidateImplementationResponse) { if a.CustomType == nil && a.ElementType == nil { resp.Diagnostics.Append(fwschema.AttributeMissingElementTypeDiag(req.Path)) + return } - // TODO:ResourceIdentity: Write validation + tests that ensure the element type only contains primitive elements (bool, string, number) + if a.CustomType == nil && !fwtype.IsAllowedPrimitiveType(a.ElementType) { + resp.Diagnostics.Append(fwschema.AttributeInvalidElementTypeDiag(req.Path, a.ElementType)) + } } diff --git a/resource/identityschema/list_attribute_test.go b/resource/identityschema/list_attribute_test.go index 45fc40c17..4affa8886 100644 --- a/resource/identityschema/list_attribute_test.go +++ b/resource/identityschema/list_attribute_test.go @@ -6,6 +6,7 @@ package identityschema_test import ( "context" "fmt" + "github.com/hashicorp/terraform-plugin-framework/diag" "strings" "testing" @@ -13,7 +14,6 @@ import ( "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/hashicorp/terraform-plugin-framework/attr" - "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/internal/fwschema" "github.com/hashicorp/terraform-plugin-framework/internal/testing/testschema" "github.com/hashicorp/terraform-plugin-framework/internal/testing/testtypes" @@ -480,6 +480,135 @@ func TestListAttributeValidateImplementation(t *testing.T) { }, }, }, + "elementtype-bool": { + attribute: identityschema.ListAttribute{ + RequiredForImport: true, + ElementType: types.BoolType, + }, + request: fwschema.ValidateImplementationRequest{ + Name: "test", + Path: path.Root("test"), + }, + expected: &fwschema.ValidateImplementationResponse{}, + }, + "elementtype-int64": { + attribute: identityschema.ListAttribute{ + RequiredForImport: true, + ElementType: types.Int64Type, + }, + request: fwschema.ValidateImplementationRequest{ + Name: "test", + Path: path.Root("test"), + }, + expected: &fwschema.ValidateImplementationResponse{}, + }, + "elementtype-int32": { + attribute: identityschema.ListAttribute{ + RequiredForImport: true, + ElementType: types.Int32Type, + }, + request: fwschema.ValidateImplementationRequest{ + Name: "test", + Path: path.Root("test"), + }, + expected: &fwschema.ValidateImplementationResponse{}, + }, + "elementtype-float64": { + attribute: identityschema.ListAttribute{ + RequiredForImport: true, + ElementType: types.Float64Type, + }, + request: fwschema.ValidateImplementationRequest{ + Name: "test", + Path: path.Root("test"), + }, + expected: &fwschema.ValidateImplementationResponse{}, + }, + "elementtype-float32": { + attribute: identityschema.ListAttribute{ + RequiredForImport: true, + ElementType: types.Float32Type, + }, + request: fwschema.ValidateImplementationRequest{ + Name: "test", + Path: path.Root("test"), + }, + expected: &fwschema.ValidateImplementationResponse{}, + }, + "elementtype-number": { + attribute: identityschema.ListAttribute{ + RequiredForImport: true, + ElementType: types.NumberType, + }, + request: fwschema.ValidateImplementationRequest{ + Name: "test", + Path: path.Root("test"), + }, + expected: &fwschema.ValidateImplementationResponse{}, + }, + "elementtype-notprimitive-dynamic": { + attribute: identityschema.ListAttribute{ + RequiredForImport: true, + ElementType: types.DynamicType, + }, + request: fwschema.ValidateImplementationRequest{ + Name: "test", + Path: path.Root("test"), + }, + expected: &fwschema.ValidateImplementationResponse{ + Diagnostics: diag.Diagnostics{ + diag.NewErrorDiagnostic( + "Invalid Attribute Implementation", + "When validating the schema, an implementation issue was found. "+ + "This is always an issue with the provider and should be reported to the provider developers.\n\n"+ + "\"test\" contains an element of type \"basetypes.DynamicType\" that is not allowed for Lists in Resource Identity. "+ + "Lists in Resource Identity may only have primitive element types such as Bool, Int, Float, Number and String.", + ), + }, + }, + }, + "elementtype-notprimitive-object": { + attribute: identityschema.ListAttribute{ + RequiredForImport: true, + ElementType: types.ObjectType{}, + }, + request: fwschema.ValidateImplementationRequest{ + Name: "test", + Path: path.Root("test"), + }, + expected: &fwschema.ValidateImplementationResponse{ + Diagnostics: diag.Diagnostics{ + diag.NewErrorDiagnostic( + "Invalid Attribute Implementation", + "When validating the schema, an implementation issue was found. "+ + "This is always an issue with the provider and should be reported to the provider developers.\n\n"+ + "\"test\" contains an element of type \"types.ObjectType[]\" that is not allowed for Lists in Resource Identity. "+ + "Lists in Resource Identity may only have primitive element types such as Bool, Int, Float, Number and String.", + ), + }, + }, + }, + "elementtype-notprimitive-map": { + attribute: identityschema.ListAttribute{ + RequiredForImport: true, + ElementType: types.MapType{}, + }, + request: fwschema.ValidateImplementationRequest{ + Name: "test", + Path: path.Root("test"), + }, + expected: &fwschema.ValidateImplementationResponse{ + Diagnostics: diag.Diagnostics{ + diag.NewErrorDiagnostic( + "Invalid Attribute Implementation", + "When validating the schema, an implementation issue was found. "+ + "This is always an issue with the provider and should be reported to the provider developers.\n\n"+ + "\"test\" contains an element of type \"types.MapType[!!! MISSING TYPE !!!]\" that is not allowed for Lists in Resource Identity. "+ + "Lists in Resource Identity may only have primitive element types such as Bool, Int, Float, Number and String.", + ), + }, + }, + }, } for name, testCase := range testCases {