[sdk-gen] Don't JSON-encode provider config

[blampe]

Description

Most of our SDKs (node, python, dotnet, java) send JSON-encoded provider configuration for legacy reasons, but YAML doesn't do this (example).

This behavior originated a long time ago when our providers only supported string properties, but of course this is no longer the case. Fortunately providers today accept both JSON or strongly typed configuration -- they attempt vanilla plugin.UnmarshalProperties but will fall back to JSON deserialization if that fails. This explains why YAML is able to send rich configuration without issue, and why it's safe to remove the legacy JSON serialization from other SDKs.

The only exception is the newer go-provider, which doesn't handle non-primitive config from our SDKs (only from YAML pulumi/pulumi-go-provider#171) because it doesn't do any JSON unmarshalling. We could either:

1. add this legacy JSON unmarshalling to go-provider, and support it indefinitely in the same way tfbridge will need to support it forever (because JSON config will be persisted in state files), or
2. stop JSON encoding config from SDKs, so go-provider only needs to concern itself with rich configuration.

This PR opts for (2) and removes JSON serialization from provider configs. pulumi/pulumi-java#1308 makes a similar change for java.

Fixes #12773
Fixes pulumi/pulumi-go-provider#171
Supersedes pulumi/pulumi-go-provider#172
Related pulumi/pulumi-java#1308
Related pulumi/pulumi-terraform-bridge#1610
Refs #13435
Refs #7058
Refs pulumi/pulumi-yaml#455
Refs pulumi/pulumi-docker#456

Checklist

• I have run make tidy to update any new dependencies
• I have run make lint to verify my code passes the lint check
  • I have formatted my code using gofumpt

[pulumi-bot]

Changelog

[uncommitted] (2024-02-23)

Bug Fixes

• [pkg] Fix codegen to no longer JSON-encode provider config
  #15032

[Frassle]

Looks like this hasn't been needed since Pulumi 1.1, not sure why we didn't take it out before? Probably worth having an old timer like @justinvp or @pgavlin to sanity check this, there may be some subtle reason its been left in for so long.

[t0yv0]

This is very tempting. Obviously feel like we must depend on it somewhere but if we don't that's a great change. QQ, when passing "variables" in Configure, they're still JSON-encoded and this PR does not affect that right? For bridged providers we've weaned off reading variables but I think some natives still do. Perhaps that's handled in the engine not here in codegen?

message ConfigureRequest {
    map<string, string> variables = 1; // a map of configuration keys to values.
    google.protobuf.Struct args = 2;   // the input properties for the provider. Only filled in for newer providers.
    bool acceptSecrets  = 3;           // when true, operations should return secrets as strongly typed.
    bool acceptResources = 4;          // when true, operations should return resources as strongly typed values to the provider.
    bool sends_old_inputs = 5;         // when true, diff and update will be called with the old outputs and the old inputs.
    bool sends_old_inputs_to_delete = 6; // when true, delete will be called with the old outputs and the old inputs.
}

[Frassle]

Perhaps that's handled in the engine not here in codegen?

It is handled in the engine, was changed all the way back in pulumi 1.1.

[t0yv0]

Anything I can help with to unblock AWS tests?

[blampe]

Anything I can help with to unblock AWS tests?

@t0yv0 no need, I expect things will be green after pulumi/pulumi-awsx#1201 goes in. The only other issues were a small type error in the dotnet fix and pulumi/pulumi-awsx#1198.

[t0yv0]

This looks fantastic. Can we loop this through downstream bridged provider tests just in case to double-check that the bridge logic holds muster to parse these simpler config forms?

[iwahbe]

Why are we changing the target framework here?

[blampe]

Probably a rebase side effect. This was bumped in #15373 and was rolled back when I ran PULUMI_ACCEPT=true make test_codegen_dotnet. I'm also not sure why it added <PackageReference Include="Pulumi" Version="3.59.0" />.

[t0yv0]

@EronWright brought up another interesting question on the strange Import(json=true) annotation in pulumi/pulumi-kubernetes#3020

From what I can tell, Java SDK appears to generate this https://github.com/pulumi/pulumi-java/blob/main/pkg/codegen/java/gen.go#L449 to json-encode Provider configuration arguments specifically for explicit providers. This I think is an instance of this same problem.

[blampe]

@EronWright brought up another interesting question on the strange Import(json=true) annotation in pulumi/pulumi-kubernetes#3020

From what I can tell, Java SDK appears to generate this https://github.com/pulumi/pulumi-java/blob/main/pkg/codegen/java/gen.go#L449 to json-encode Provider configuration arguments specifically for explicit providers. This I think is an instance of this same problem.

@t0yv0 yeah, pulumi/pulumi-java#1308 covers the Java side. I'm almost inclined to suggest we start with Java (or a different language) if we don't have the appetite to change the behavior for everything at once.