Introduce AssetOrArchive to support the SDK's Asset type which can be both

[thomas11]

Resolves #237.

This PR addresses an issue with the design of the assets and archives support in Pulumi core. Quoting #237:

When defining a property of type resource.Asset, the generated schema will contain a $ref of pulumi.json#/Asset. However, in the SDKs, an "asset" is represented by pulumi.AssetOrArchive because an archive is currently always a valid asset.

Therefore, when deserializing the PropertyValue for the provider to use, it's valid for the user to have specified one of the Archive types which then isn't deserializable into the resource.Asset field within the provider.

To work around this, we define a new type types.AssetOrArchive here in pulumi-go-provider. In the schema, it's represented as a standard pulumi.json#/Asset for interoperability. An incoming core Asset is translated into a types.AssetOrArchive, in Decode, with either Asset or Archive populated. Vice versa, Encode pulls the asset/archive out of AssetOrArchive.

Notes

• The decode and encode implementations are bespoke and there might be opportunities to move it to a new, more generic extension point.

[iwahbe]

The serde stuff looks good to me. This is the kind of place that I'd really appreciate integration tests to lock in good behavior.

[danielrbradley]

This looks pretty close. I'm happy the implementation looks correct, just a few last bits of polish.

[danielrbradley]

Is a panic the best course of action here? I know we can't have both in one right now, but it's not beyond the realms of possibility that platform created some new type which is both an asset archive for some reason. I'm guessing we don't have any kind of spec saying is can't have both sigs, just it's not currently possible?

Perhaps we should just continue here as it doesn't break our model to have both given how our union type is moddled?

[iwahbe]

I believe that this is when we are serializing. We need to serialize AssetOrArchive into either an asset or an archive, and we can't do that. We can't compress struct { Asset, Archive } into an Asset or Archive without losing information. It is always a provider bug and right now panicing is a reasonable option. It would be better to return an error, but it's not clear how.

[iwahbe]

Platform will not create a new type that has our sigs (we assume that sigs won't conflict by accident).

[danielrbradley]

It was the other way round - when we're parsing - we're adding a panic if we recieve something which contains both asset or archive information. My suggestion here would be to just check if it's an asset first, then check if it's an archive. The code below would already do that if the panic removed, if I read it correctly, though it prefers archive over asset.

[iwahbe]

We should panic on ambiguity, as it is always a provider or framework bug. The user will never see the ambiguity and the engine can only pass ambiguous information if it copies the sigs we use, which it should not.

[danielrbradley]

I agree if it's user or framework error, but this data is coming from the engine – for which we don't have a super-good specification of what they might send, so I'd consider being defensive over making contract assertions. But perhaps it's reasonable here to fail hard because it would be a weird change engine side that hopefully they wouldn't make because it's likely not backward compatible (not that we define backward compatibility in our protocols either 😞).

[iwahbe]

There is no remotely backwards compatible change (beyond copying the sigs we created so they wouldn't be copied, on purpose or by accident) that would allow this to be an engine bug. It will always be a provider author or framework bug, and it should panic to allow the responsible party to fix it, instead of doing the wrong thing for end users.

[danielrbradley]

Think this is good to go.

Discussions are just asides at this point.

[iwahbe]

This looks good to me. There are a couple simple changes to make before merging, but it mostly looks good to go.