Terraform-plugin-go dynamic pseudo type json marshal/unmarshalling

Hi,

I’m trying to add a dynamic pseudo type and i’m having difficulty un/marshalling unstructured data. My API client takes a map[string]interface{} so I have no structs I can use the ValueCreator, ValueConverter interfaces for.

The kuberneters alpha provider can easily marshal/unmarshal unstructured json objects into cty.Value but I haven’t seen anything to easily do this with plugin-go -

Can we add something like this to the terraform-plugin-go tftypes:

Your question is very timely! We’ve been discussing this exact problem.

Fundamentally, the issue at play here is that the tftypes.Value type can contain unknown values, and you’ll have no way of knowing if the values are unknown until runtime, because users can inject unknown values at any depth in any field. When converting a tftypes.Value to an interface{}, then, it’s unclear what should happen with unknown values.

I hope to have something to share on this front in the very near future, ideally this week. Technically it’s solvable outside the tftypes package, and I think that’s where I’d like to start with a solution, just because the tftypes package is very expensive for us to break compatibility on, so I want a high degree of confidence in things that go into it. I’m hoping to ship a small module with code in it that will let you do this, however, which solves for my concern because the module can have a major version bump without impacting the rest of the ecosystem.

Thanks @paddy, yeah i explored most of the new framework and then came to a grinding halt with this :smiley: look forward to seeing the new module - happy to help test out if needed.

Hey @iwarapter, sorry for the delay here. Had to get a new public repo spun up and ready for primetime, which takes a little doing. :slight_smile: You can find the package I’m talking about here. Disclaimers about being pre-release and not-heavily-used apply here, but it should be illustrative and usable.

Thanks @paddy will give it a go!

@paddy I have terraform-plugin-go-contrib working great for a dynamic input to my data source - thanks.
The implementation of the data source’s logic returns an interface{} which I’d now like to set in the value passed as the response’s State field.
Are you planning on adding a ValueCreator implementation to this package so that the interface{} can be converted to a Value, or is there some other way of doing this?

:+1: the GoPrimitive is working nicely. I’d love to see it implement the ValueCreator as well!

I hadn’t planned on implementing the ValueCreator interface, for perhaps non-intuitive reasons. When implementing the ValueConverter interface, we know what the input values are going to be: they’re one of our internal representations for one of the Terraform types, which is a limited set. Because of that, the job of the package is straightforward: pick a canonical version of each of those types and set GoPrimitive.Value to them.

ValueCreator has a more difficult proposition, because the range of input values we can handle is theoretically infinite–GoPrimitive.Value can be set to any Go type, and the expectation is that we’ll turn it into a tftypes.Value. This is, obviously, not something we’ll ever be able to satisfy fully.

But maybe there’s a middle ground here where we can create a tftypes.Value from a certain subset of types, say the built-in Go types and some chosen standard library primitives, and throw an error on everything else. That may strike the right balance of pragmatism, though it seems like a shaky premise to build a provider on to me. I’d love an enhancement issue for this, and we can work through the nuance there in a way that will get tied to the code in context for future maintainers wondering why things are the way they are.

Hi @paddy I defiantly think this is a feature that is needed!

I had a quick play at implementing this - I have been able to get something that works (it converts all the test cases for GoPrimitive back to their tftypes.Value.

However for this to work I needed a small patch of the plugin-go module to add a Type() method to return the Value type (i just vendored and added the project to demo it) - terraform-plugin-go-contrib/value.go at feature/add-value-creator-for-go-primitives · iwarapter/terraform-plugin-go-contrib · GitHub

However it’s doesn’t properly work with NewValue as it actually returns the ready to go tftypes.Value but NewValue then tries to type check it against the actual go primitives -

if creator, ok := val.(ValueCreator); ok {
	var err error
	val, err = creator.ToTerraform5Value()
	if err != nil {
		panic("error creating tftypes.Value: " + err.Error())
	}
}

This only works because I’m storing the type data when the value is originally unmarshalled and assumes its not changing.

Quick Update: I have tested this with one of my providers and its working as expected (for some basic dynamic types - will try find some more challenging scenarios

Anyways food for thought.

It’s actually pretty easy to break if the dynamic configuration changes structure from the original GoPrimitive type. It was an interesting experiment at least :slight_smile:.

I suppose the real question now is why are we not just using cty.Value since we know this functionality will work.

This is a useful exercise and something I’m interested in! Thanks for doing this. :slight_smile:

This is something I keep going back and forth on. I had a long conversation about it recently, actually. I think my hesitation here is that I haven’t run into a situation where it’s needed, yet, it just makes things easier in certain situations. And I’m trying not to make things that most people don’t want to do too easy, because I don’t want to confuse people. I’m still mulling this one over a bit, and think we’ll probably come down on the side of making the type retrievable from a value, but I’m trying to be deliberate about that kind of stuff. I worry that if we expose a tftypes.Value's Type, people will switch on them or do raw comparisons instead of using Is, and that has some pitfalls if you’re not thoughtful about it.

Technically you shouldn’t need to do that for this; you can inspect each level of the type using reflect or a type switch, and constructing the tftypes.Type dynamically.

You appear to be returning a tftypes.Value as the interface, but according to the documentation for tftypes.ValueCreator (emphasis added):

So rather than returning a new tftypes.Value, your implementation should be returning one of the supported primitives. E.g., objects should return a map[string]tftypes.Value, strings should return a string, etc.

I’m interested in hearing more about this. What does “break” mean, here? Can you be clearer about what you mean by “changes structure”?

That one I have an answer to! Basically, cty isn’t designed for this use case. And I don’t mean that it can’t work in this use case, it just means we’re trying to do something other than what cty was designed for.

cty was built to let Go programs define a typed configuration language. That’s what the packages are built and designed for, and that makes sense for Terraform to use. Terraform is, after all, trying to define a configuration language.

The SDK, however, is not trying to design a configuration language. It’s trying to surface a configuration language that has already been designed. It doesn’t want consumers creating their own types, it doesn’t need arbitrary expressions, it just wants to give users access to values expressed in an existing type system.

And that’s what tftypes is for. It’s not a type system building library, it’s a representation of Terraform’s type system, specifically. And it’s designed and tailored towards making the things people want to do with Terraform’s type system obvious and easy, making things that people probably shouldn’t be doing impossible, and making things that most people won’t want to be doing a little harder than the things most people want to do, to help guide them towards the things they probably want to be doing.

I’m very open to the feedback we missed the mark on that so far. I think that’s likely, and something we plan to continue to iterate on as we learn more. But I think the strength of tftypes is that we get to iterate to something that makes sense for users of the SDK, which we don’t get to do with cty because cty isn’t designed specifically for the SDK, and so its designs need to work for Terraform and every other system out there using it.

Does that all make sense? I know it’s frustrating to watch us iron out bugs and circle towards a more stable implementation when a stable implementation already exists, but we feel confident that this will yield a more stable and more approachable provider development experience in the long run.

Hi @paddy thanks for the detailed response.

When I did my quick test the reason I exposed the Type it was to try to maintain some integrity when you have for example a Set or List because their go types is just going to be a slice and you probably need to have some context on what you are expecting (which I’ll come back to later). Since I knew what the original type was (and any element/attribute subtypes) I can work through it.

In this case I had defined the minimum config to allow the API to create the resource

{
	"path" = "/foo/bar"
	"subjectAttributeName" = "foo"
}

But the response included other fields, which my implementation didn’t handle as I was lazily iterating through expecting for find pre-defined types for my data, essentially it doesn’t handle config diffs which is an issue with the quick and dirty implementation.

Yes, this makes sense - I watched your response on the Office Hours.

I have written an implementation using reflect which satisfies all of the test cases in contrib except for determining when to use set vs list which I think is going be a (provider developer) context based decision. I’ll push this to my fork in a little bit for reference.

Back to the use case for my providers, it actually occurred to me that I have an API which will tell me the structure for of what to expect so I’ll be able to dynamically generate my values.

Hmm interesting edge case - what would you say should be expected behaviour if your presented with an object with a nil:

map[string]interface{}{
	"description": nil,
	"path": "/foo/bar",
},

I’m tempted to say remove they key with the nil value?

Ooof, sorry for the delayed response, been a busy couple of weeks. :frowning:

We just recently merged a PR to plugin-go to expose Type information from Values. That should be in a release soon.

If I’m understanding this right, you’re struggling with some “diff didn’t match apply” problems. I think you’ll need to know the shape of the API response and populate the fields that aren’t in the config with UnknownValues (which is how Kubernetes is handling it) or just not set anything in state the user didn’t set in their config.

I think it depends, and it’s kinda why I was hesitant to include something like this in the main plugin-go repo; I think different implementations will use different semantics to mean different things. My no-context guess is that maybe a Value containing null would be the right choice, but I don’t honestly know without digging into the specifics.