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

Planned value does not match config value for cty.NumberIntVal #34589

Closed
bendbennett opened this issue Jan 30, 2024 · 6 comments · Fixed by #34756
Closed

Planned value does not match config value for cty.NumberIntVal #34589

bendbennett opened this issue Jan 30, 2024 · 6 comments · Fixed by #34756
Assignees
Labels
bug cty Use in conjunction with "upstream" when cty is the relevant upstream upstream

Comments

@bendbennett
Copy link

Terraform Version

Terraform v1.7.0
on darwin_arm64

Terraform Configuration Files

resource "example_resource" "example" {
  number_attribute = 9223372036854775808
}

Debug Output

https://gist.github.com/bendbennett/56bd4ca26d1026cdc1fcc450b012d573

Expected Behavior

Expected a successful terraform apply.

Actual Behavior

Error relating to invalid plan was generated:

│ Error: Provider produced invalid plan
│ 
│ Provider "registry.terraform.io/bendbennett/playground" planned an invalid value for example_resource.example.number_attribute: planned value cty.NumberIntVal(9.223372036854775807e+18) does not match config value cty.NumberIntVal(9.223372036854775808e+18).
│ 
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.

Steps to Reproduce

  1. terraform apply

Additional Context

This appears to be specific to the value of an integer that is supplied to a number attribute (i.e., 9223372036854775808) within a provider.

The structure of the provider is as follows:

var _ resource.Resource = (*exampleResource)(nil)
var _ resource.ResourceWithImportState = (*exampleResource)(nil)

type exampleResource struct {
	provider exampleProvider
}

func NewResource() resource.Resource {
	return &exampleResource{}
}

func (e *exampleResource) Metadata(_ context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) {
	resp.TypeName = req.ProviderTypeName + "_resource"
}

func (e *exampleResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) {
	resp.Schema = schema.Schema{
		Attributes: map[string]schema.Attribute{
			"number_attribute": schema.NumberAttribute{
				Optional: true,
				Computed: true,
			},
		},
	}
}

type exampleResourceData struct {
	NumberAttribute types.Number `tfsdk:"number_attribute"`
}

func (e *exampleResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
	var data exampleResourceData

	diags := req.Plan.Get(ctx, &data)
	resp.Diagnostics.Append(diags...)

	if resp.Diagnostics.HasError() {
		return
	}

	diags = resp.State.Set(ctx, &data)
	resp.Diagnostics.Append(diags...)
}

func (e *exampleResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) {
	var data exampleResourceData

	diags := req.State.Get(ctx, &data)
	resp.Diagnostics.Append(diags...)

	if resp.Diagnostics.HasError() {
		return
	}

	diags = resp.State.Set(ctx, &data)
	resp.Diagnostics.Append(diags...)
}

func (e *exampleResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) {
	var data exampleResourceData

	diags := req.Plan.Get(ctx, &data)
	resp.Diagnostics.Append(diags...)

	if resp.Diagnostics.HasError() {
		return
	}

	diags = resp.State.Set(ctx, &data)
	resp.Diagnostics.Append(diags...)
}

func (e *exampleResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
	var data exampleResourceData

	diags := req.State.Get(ctx, &data)
	resp.Diagnostics.Append(diags...)

	if resp.Diagnostics.HasError() {
		return
	}
}

func (e *exampleResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) {
	resource.ImportStatePassthroughID(ctx, path.Root("id"), req, resp)
}

Using the following configuration stores the correct value in state, but the CLI output reports an incorrect value:

resource "example_resource" "example" {
  number_attribute = 9223372036854775809
}
Terraform will perform the following actions:

  # example_resource.example will be created
  + resource "example_resource" "example" {
      + number_attribute = 9223372036854776000
    }
{
  "version": 4,
  "terraform_version": "1.7.0",
  "serial": 1,
  "lineage": "58c377a3-dd34-9d67-acab-e00359eed919",
  "outputs": {},
  "resources": [
    {
      "mode": "managed",
      "type": "example_resource",
      "name": "example",
      "provider": "provider[\"registry.terraform.io/bendbennett/playground\"]",
      "instances": [
        {
          "schema_version": 0,
          "attributes": {
            "number_attribute": 9223372036854775809
          },
          "sensitive_attributes": []
        }
      ]
    }
  ],
  "check_results": null
}

References

No response

@bendbennett bendbennett added bug new new issue not yet triaged labels Jan 30, 2024
@jbardin jbardin added upstream and removed new new issue not yet triaged labels Jan 30, 2024
@jbardin
Copy link
Member

jbardin commented Jan 30, 2024

Thanks @bendbennett!

This looks like a bug upstream in the cty msgpack encoding. Round-trip from cty->msgpack->cty returns the wrong value, so there is probably a step which is assuming an inadequate precision. The implementations are closely related, it would be good to check if the framework suffers from the same problem on its own.

@jbardin jbardin self-assigned this Jan 30, 2024
@jbardin jbardin added the cty Use in conjunction with "upstream" when cty is the relevant upstream label Jan 31, 2024
@jbardin
Copy link
Member

jbardin commented Jan 31, 2024

This was a great one @bendbennett! Martin summarized the situation really well in my PR here: zclconf/go-cty#176 (comment). A bug in the msgpack library, caused by system dependent behavior (this does still work on amd64!), combined with an unrelated bug in go-cty 😆

We can close this when we update go-cty.

@apparentlymart
Copy link
Contributor

FWIW (with cty maintainer hat on) I didn't rush to tag a new release for this one since it seemed like a relatively unlikely problem -- we've had this bug for a number of years without anyone noticing it -- and I only just did a cty patch release last week for a different bug.

If I'm misread the urgency on this please let me know, but otherwise I was gonna just sit on it for a while in case other PRs show up that can then all go out together.

@apparentlymart
Copy link
Contributor

Also FWIW, it looks like terraform-plugin-go does not have the same bug, because it just uses the default settings of the MessagePack encoder:

https://github.com/hashicorp/terraform-plugin-go/blob/a3f1922a21412f0d815893e337385a289730925b/tftypes/value.go#L581-L590

It does not enable the setting that caused the problem in cty, and it has equivalent logic in its own marshaling functions for testing if the number is representable as an int64 and using the MessagePack int encoding if so.

@bendbennett
Copy link
Author

Thanks for all the feedback, insight and fix on the underlying issue @jbardin and @apparentlymart. I don't think that it's particularly pressing that a new release of cty appears immediately as we only came across this issue in an edge case as we were adding further testing around provider-defined functions to terraform-provider-corner. Thank you also for confirming that terraform-plugin-go does not have the same bug, much appreciated.

Copy link
Contributor

github-actions bot commented Apr 1, 2024

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug cty Use in conjunction with "upstream" when cty is the relevant upstream upstream
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants