terraform-cdk icon indicating copy to clipboard operation
terraform-cdk copied to clipboard

Number attributes not synthed as references

Open ex-nihil opened this issue 2 years ago • 9 comments

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

cdktf & Language Versions

com.hashicorp:cdktf:0.9.0 "aws@~> 3.0" OpenJDK Runtime Environment (build 1.8.0_312-b10)

Affected Resource(s)

https://www.terraform.io/docs/providers/aws/r/memorydb_cluster https://www.terraform.io/docs/providers/aws/r/db_instance

Debug Output

Expected Behavior

db.port should synthesize to Terraform reference instead of number:

"value": "${aws_memorydb_cluster.cluster_9AC9C0F1.cluster_endpoint.0.port}"

This workaround can achieve the above:

db.getStringAttribute("port")

Actual Behavior

db.port synthesizes to:

"value": "-8.109562212591454E298"

Steps to Reproduce

I assume this happens for any provider when trying to synth number types.

Important Factoids

This seems to be a regression as I found #679 that was supposedly fixed in 0.6.0.

References

  • #0000

ex-nihil avatar Jan 31 '22 21:01 ex-nihil

I just did a simple test in typescript and everything was working correctly. I wouldn't expect any issues in Java, but perhaps some more details on the code involved would shed some light.

It looks like you are using the 0.9.0 version of the cdktf library; are you also using version 0.9.0 of the cli? Are you just passing the port directly from the cluster to the db instance, or is there something else happening?

jsteinich avatar Feb 02 '22 03:02 jsteinich

Could you provide a snippet of your code as an example so that we can reproduce it?

DanielMSchmidt avatar Feb 08 '22 17:02 DanielMSchmidt

Not really sure if this is relevant or even related to this issue but when searching this was the closest we could find.

com.hashicorp:cdktf:0.12.0 "akamai/akamai@~> 2.1.1" OpenJDK 64-Bit Server VM Corretto-17.0.3.6.1 (build 17.0.3+6-LTS)

We actually had this issue with the Java version. We are trying to create a Akamai cpCode and use its generated id in a later stage. The cpCodeId is a string of pattern "cpc_123456" and we need to extract the number part as an actual numeric value.

We can use the Fn.trimprefix function to get the string representation of the cpCodeId (so "cpc_123456" becomes "123456") but when trying to parse it to a number we always get the same value -8.109562212591454E298. That actually happens with any number we try to parse.

var shouldBeNumber1 = Fn.parseint("123456", 10); // becomes -8.109562212591454E298
 
var shouldBeNumber2 = Fn.tonumber("123456");     // becomes -8.109562212591454E298

var shouldBeNumber3 = Fn.parseint(Fn.trimprefix(relevant-TfToken, "cpc_"), 10); // becomes -8.109562212591454E298

var shouldBeNumber4 = Fn.tonumber(Fn.trimprefix(relevant-TfToken, "cpc_"));     // becomes -8.109562212591454E298

teodoradra avatar Sep 13 '22 10:09 teodoradra

@teodoradra This is expected. Terraform functions return a token value (this extremely bug negative number). They are translated to ${parseint("123456", 10)} in the terraform JSON and then run at execution time.

You can read more about Tokens here.

DanielMSchmidt avatar Sep 13 '22 10:09 DanielMSchmidt

@DanielMSchmidt thank you for your quick response!

But the thing that we keep seeing it that, after planning and running the changes we still have the exact same value if we try to convert the string to a number. If we leave it as a string (which we anyway trim using trimprefix and that works just fine) we can see the actual value in Terraform Cloud as it should be (meaning, the actual value not the negative float), so we are encountering a problem only when trying convert the string to number (using Fn.tonumber or Fn.parseint). And on our provider side we see the exact same thing (negative number) after running and applying the plan. This is what we get on the Terraform Cloud side when we try to convert the string that we trimmed (using functions as well) to a number:

id : -8.109562212591383e+298

and this is what we get when we just trim it and don't convert to a number:

id : "1376863"

So what is unclear to me is why when I use Fn.trimprefix(code.getId(), "cpc_") I get the right value, and if I use Fn.parseInt("1376863",10) I get that negative number on the Terraform Cloud side. If I got it wrong, please let me know, I will revisit this problem that we have. Thank you!

teodoradra avatar Sep 13 '22 11:09 teodoradra

Hello, I am a teammate of Teodora and there are some things that are still a bit unclear to me.

It seems to me that the problem is that we have to provide a String representation of the rules but that is a complex object that we are creating as POJOs and then serialising (with either jackson or gson, I tried both).

I created this simplified code snippet to illustrate the situation

  public record TestingThing(Number id, String someOtherProperty) {}
  
  final var testingThing = new TestingThing(
        Fn.tonumber(Fn.trimprefix("cpc_123456", "cpc_")),
        Fn.trimprefix("cpc_123456", "cpc_"));

  PropertyBuilder()
  	...
        .count(Fn.tonumber(Fn.trimprefix("cpc_123456", "cpc_"))) // expects a Number
        .rules(new Gson().toJson(testingThing)) // expects a String
        ...
        .build();

After runnig synth on that this is the output.

	{
	  ...
          "count": "${tonumber(trimprefix(\"cpc_123456\", \"cpc_\"))}",
	  "rules": "{\"id\":-8.109562212591386E298,\"someOtherProperty\":\"${trimprefix(\"cpc_123456\", \"cpc_\")}\"}"
	  ...
	}

And that value of id is never converted to the proper terraform token. The most weird thing to me is that the trimprefix worked as expected, even after serialising the object and running the synth, but the tonumber show two different behaviours.

marceloaguiarr avatar Sep 14 '22 14:09 marceloaguiarr

It would be great if you could build a small example so we can debug this further. I am not sure what exactly happens here on the not-cdktf side that could influence the result, but it could also be a cdktf problem directly. I see you put the tokens into a class and serialize it out of that. If e.g. a number to string conversion appears outside of the cdktf context it can be that the token value itself is destroyed, e.g. by an accidental rounding

DanielMSchmidt avatar Sep 14 '22 16:09 DanielMSchmidt

Hello. I have created this repo to show the error happening. Hope that makes things more clear.

All you have to do is clone, then run the test class. It should print the synthesized string with the error. As you said, this really looks like a rounding error when the serialisation of the java class into json happens.

marceloaguiarr avatar Sep 15 '22 13:09 marceloaguiarr

Hey folks, sorry for taking so long, but I have found the problem.

During synth your program transforms the number to a sting with Gson. After that CDKTF runs through all the numbers it knows of and translates them to terraform expressions if applicable. But your number is not a number anymore, it's a string now so we dont catch it. To solve this issue right now you can use Fn.jsonencode(testingResource) and it works.

The bigger question here is, can we make your current example work. I think we possibly can try to find things that look like numbers in all strings and see if they are known to us. For that we would need to check if a tokenized number can collide with another tokenized number if serialized to string. If we are sure this can not happen we can then search through every string for something that looks like a serialized number and replace this with the TF expression needed. The serialization format could vary between languages though, JS uses e+ instead of E like Java for example. It could also be a custom serialization engine being used. So I guess it's hard to get right all the time, but getting it right most of the time and never getting it wrong would be sufficient I think

DanielMSchmidt avatar Sep 16 '22 10:09 DanielMSchmidt