Add support for attr.ValueAs

[paddycarver]

Module version

v0.2.0

Use-cases

As described in #69.

Proposal

As described in #69.

[paddycarver]

This is unexpectedly complicated by the need of the reflect.Into method to have the attr.Type available for the attr.Value it's injecting into.

I'm elbow-deep in trying to add a Type(context.Context) method to attr.Value, but it has some ramifications for the extensibility of these types that I'm not loving (the attr.Value implementations can no longer be produced by multiple types, they also need to be wrapped, and they can't just inherit the Equal implementation, that needs to be wrapped, too. It forces a stricter 1:1 mapping of attr.Type : attr.Value where before it could be 1:N).

We needed the attr.Type to reflect into an attr.Value because we needed to know how to create the attr.Value, to replace it (as not all attr.Value implementations can be modified in-place). If, however, we add this Type method to attr.Value, we could avoid needing to pass it into reflect.Into explicitly. This would still lead to the inheritance mess we talked about in the last paragraph, but at least it would add the benefit that people could reflect into attr.Values other than the ones defined in their schema, as long as they were compatible at the tftypes.Value level.

I'm currently 🤔 about whether making attr.Type and attr.Value harder to create is worth it. Another option is to make provider developers pass the attr.Type of the attr.Value as an argument to attr.ValueAs, but that seems rather annoying for provider developers.

[paddycarver]

Another option is to introduce AttributeConfig, AttributePlan, and AttributeState types that bundle the attr.Type and attr.Value, like Config, Plan, and State do, with a method like As that does the reflection. This offloads the complexity onto the framework, but makes the functionality less generally useful--it's now limited only to situations in which the framework provides those types, instead of applicable to all attr.Values, which means there are still likely to be places where it's necessary to type-assert an attr.Value, which I think we're trying to avoid.

[paddycarver]

If, however, we add this Type method to attr.Value, we could avoid needing to pass it into reflect.Into explicitly. This would still lead to the inheritance mess we talked about in the last paragraph, but at least it would add the benefit that people could reflect into attr.Values other than the ones defined in their schema, as long as they were compatible at the tftypes.Value level.

This is not true, on investigation. Reflection goes from a tftypes.Value to an arbitrary Go type, which may be an attr.Value. So we would need to construct an attr.Value to pass into reflect.Into, which may not be a big deal, but it doesn't really gain us much. This could help us allow conversion into attr.Value types different than those created by the attr.Type in the schema, but I think it's not required to, and may be full of edge cases. Consider:

type MyValueType struct{}

func (m MyValueType) ValueFromTerraform(_ context.Context, in tftypes.Value) (attr.Value, error) {
  return MyValue{val: in, createdBy: MyValueType{}}, nil
}

type MyValue struct {
  val tftypes.Value
  createdBy attr.Type
}

func (m MyValue) Type(_ context.Context) attr.Type {
  return m.createdBy
}

func Create() {
  type req struct {
    foo MyValue `tfsdk:"foo"`
  }
  var config req
  err := r.Config.Get(ctx, &config)
}

This would not work, as config's createdBy property wouldn't be populated, since it wasn't produced by ValueFromTerraform. Specifying it manually would be possible for the provider, but that's annoying when it's a property in an otherwise-empty struct.

I'm going to keep investigating adding attr.Value.Type(), to see if I can solve the problem at hand, but there should probably be more 🤔 into if we can allow the use of different attr.Values than the attr.Value created by the attr.Type in the schema. I don't think so, but it would certainly be nice.

[github-actions]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.