pulumi / pulumi-terraform-bridge

A library allowing Terraform providers to be bridged into Pulumi.
Apache License 2.0
198 stars 43 forks source link

Flag for removing JSON config encoding from the bridge #2550

Open t0yv0 opened 1 week ago

t0yv0 commented 1 week ago

Testing removing JSON config encoding from product I discovered that it is not enough to make a codegen change, because the bridge re-encodes configuration data passed into Check using JSON config encoding even if the data was passed in the plain.

To enable successful rollout, we need to ensure that a bridge release is simultaneously:

  1. removing config encoding from all code paths except parsing olds in DiffConfig
  2. setting the flag for every affected code generator (Node, Python, Java, C#) to avoid JSON-encoding config properties

Alternatives to doing this:

iwahbe commented 1 week ago

What's the criteria of a successful rollout? Is the goal to allow users to see no diff on the rollout, or just to avoid breaking the user?

If the goal is simply to avoid breaking the user, could we:

  1. Prevent the bridge re-encoding the config in CheckConfig, so new provider state is saved without JSON encoding.
  2. Rollout (per language, independent from the bridge) the change to not JSON encode config (assuming this is correctly handled by the bridge now).
  3. When all languages have stopped JSON encoding their config, add a warning when the bridge detects JSON encoded config coming from anywhere but state (via DiffConfig or Configure).

I have no issue believing that ☝ is non-viable, but I don't understand why.

From your description, it seems like setting config_encoding to not re-encode in all cases is unsafe, can you explain why?

t0yv0 commented 1 week ago

Can consider. Let's try this out and see how it plays out. I would love to decouple the rollout constraints. This can be a good option Along with fix config_encoding to not re-encode unless it believes the data was JSON-decoded in the first place.

I have some uncertainty about detecting the encoding from the data and trying to make that transparent, does "true" encode true or "true"? Does [] encode an empty list or a string "[]"? skipMetadatApiCheck issues are slightly different but tangentially related here.