Support custom object types #1674
Conversation
This is necessary to support custom object types in the PF.
iwahbe
force-pushed
the
iwahbe/support-custom-object-types
branch
from
6a1bb31
to
43f2eed
Compare
| if t, err := r.obj.ApplyTerraform5AttributePathStep(tftypes.AttributeName(key)); err == nil { | ||
| typ, ok := t.(attr.Type) |
There was a problem hiding this comment.
As I understand it r.obj.ApplyTerraform5AttributePathStep(tftypes.AttributeName(key)) represents a .key access in the same space value space (attr.Type) as r.obj.
By working within attr.Type (instead of down-casting to types.ObjectType, we get support for custom implementations of basetypes.ObjectTypeable.
|
This fix covers tfgen, but I wonder if we have some similar incompleteness at runtime. Did you get a chance to provision this at runtime to see if that works out as expected? Much appreciated. |
I didn't take the time to provision this and check that it works as expected, but I believe that it will. Unlike pulumi-terraform-bridge/pf/internal/convert/object.go Lines 25 to 26 in 43f2eed We should be immune to this kind of error here. I have checked and we don't have any usage of |
|
|
||
| // --- custom object machinery --- | ||
| // | ||
| // copied from terraform-provider-aws/internal/framework/types/objectof.go |
There was a problem hiding this comment.
Is it reasonable to use the TF AWS implementation of custom object as a general test support machinery here? unless I'm misunderstanding something, this pattern is specific to AWS custom objects, isn't it?
I wonder if it would be possible (and not awfully impractical) to pull this in as a dependency instead, rather than copying?
| return objectTypeOf[T]{basetypes.ObjectType{AttrTypes: AttributeTypesMust[T](ctx)}} | ||
| } | ||
|
|
||
| func (t objectTypeOf[T]) Equal(o attr.Type) bool { |
There was a problem hiding this comment.
super nitty readability nit: we call objectTypeOf[T] -> t and we also call *testing.T -> t. these are both reasonable conventions; they just clash here.
Maybe this machinery code can live in a different file, or we can got with typ as a variable name for the former.
| @@ -57,7 +58,7 @@ func (*typeSchema) Sensitive() bool { return false } | |||
|
|
|||
| func (s *typeSchema) Elem() interface{} { | |||
| switch tt := s.t.(type) { | |||
| case types.ObjectType: | |||
| case basetypes.ObjectTypable: | |||
There was a problem hiding this comment.
Does this get rid of ObjectType entirely, in favor of the more flexible basetypes implementation?
There was a problem hiding this comment.
ah, rereading your comment: yes, it is meant to do that, and we should follow up with other types in the same manner.
Fixes #1673 and unblocks pulumi/pulumi-aws#3378.