Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to Unmarshal RawState to a type with OptionalAttributes #82

Open
bschaeffer opened this issue Apr 29, 2021 · 1 comment
Open
Labels
bug Something isn't working

Comments

@bschaeffer
Copy link

bschaeffer commented Apr 29, 2021

terraform-plugin-go version

v0.3.0

Relevant provider source code

package main

import (
	"github.com/hashicorp/terraform-plugin-go/tfprotov5"
	"github.com/hashicorp/terraform-plugin-go/tftypes"
)

var (
	roleType = tftypes.Object{
		AttributeTypes: map[string]tftypes.Type{
			"id":   tftypes.Number,
			"name": tftypes.String,
		},
		OptionalAttributes: map[string]struct{}{
			"name": struct{}{},
		},
	}

	objType = tftypes.Object{
		AttributeTypes: map[string]tftypes.Type{
			"id":   tftypes.Number,
			"name": tftypes.String,
			"role": tftypes.Map{AttributeType: roleType},
		},
	}
)

func main() {
	value := tftypes.NewValue(objType, map[string]tftypes.Value{
		"id":   tftypes.NewValue(tftypes.Number, 1),
		"name": tftypes.NewValue(tftypes.String, "foo"),
		"role": tftypes.NewValue(tftypes.Map{AttributeType: roleType}, map[string]tftypes.Value{
			"bar": tftypes.NewValue(roleType, map[string]tftypes.Value{
				"id":   tftypes.NewValue(tftypes.Number, 1),
				"name": tftypes.NewValue(tftypes.String, "baz"),
			}),
		}),
	})

	_, err := tfprotov5.NewDynamicValue(value.Type(), value)
	if err != nil {
		panic(err) // This should not error
	}

	rawState := &tfprotov5.RawState{JSON: []byte(`{"id":1,"name":"foo","role":{"bar":{"id":1,"name":"baz"}}}`)}
	_, _ = rawState.Unmarshal(objType)
}

Terraform Configuration Files

Unrelated

Expected Behavior

The above script should not panic. Basically, I feel like should be able to use the same tftypes.Type to generate a tftypes.Value that I pass into RawState.Unmarshal when I expect the state to match.

Actual Behavior

panic: ElementKeyString("bar"): can't use 
  tftypes.Object["id":tftypes.Number, "name":tftypes.String]
as 
  tftypes.Object["id":tftypes.Number, "name":tftypes.String?]

Steps to Reproduce

Run the supplied script

References

@bschaeffer bschaeffer added the bug Something isn't working label Apr 29, 2021
@bschaeffer bschaeffer changed the title Unable to Unmarshal RawState when using a type OptionalAttributes Unable to Unmarshal RawState when to a type with OptionalAttributes Apr 29, 2021
@bschaeffer bschaeffer changed the title Unable to Unmarshal RawState when to a type with OptionalAttributes Unable to Unmarshal RawState to a type with OptionalAttributes May 3, 2021
@paddycarver
Copy link
Contributor

This is something we're working on. The problem here, basically, is that Terraform has a method of comparing types that terraform-plugin-go doesn't accurately reproduce: whether a type can be used as another type. Usually this means a concrete type filling a DynamicPseudoType, but I think (@terraform-core can correct me if I'm wrong) it also should let an Object without optional attributes be used as an Object with optional attributes.

Optional attributes are really more schema information than they are information about values, I guess. So you're still going to need to set them to null in your provider code, etc. It's just that practitioners won't need to explicitly set them in their configs, unlike normal object attributes.

We're resolving this by refactoring how we handle type comparisons a bit. Rather than a single overloaded Is method, we're working on splitting out type comparisons into three discrete methods with explicit semantics:

  • Is is for shallow-checking the "kind" of type we're talking about. All objects will return true for other objects, even if they don't have the same attributes.
  • Equal is for deep-checking the type. It needs to be a 1:1 match in every way. Attributes, optional attributes, elements, everything needs to match.
  • UsableAs is for the type of comparison I describe above. Anything is UsableAs DynamicPseudoType, an object without optional attributes is UsableAs an object with optional attributes, etc.

My hope is that by making these distinctions and updating tftypes to use the appropriate distinction in the appropriate place, we'll resolve this issue.

Note that we're currently leaning towards you won't be able to create Values with a Type that has optional attributes, because that doesn't make much sense; optional attributes are really schema information, honestly, and so shouldn't really appear outside of that. Depending on how unfortunate a developer experience this is, we may change that decision in the future, but it's currently where the thinking is.

@bflad, feel free to shout if I missed anything.

(Sorry everyone for the duplicate notifications, I commented on the wrong issue :( )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants