pulumi / pulumi-yaml

YAML language provider for Pulumi
Apache License 2.0
39 stars 12 forks source link

Default value is not respected #458

Open komalali opened 1 year ago

komalali commented 1 year ago

The pulumiservice provider has a Webhook resource which has a format property with a (string) enum type and a default (string) value. This default value is not respected and I get a diff during refresh. The diff shows an update (add) in the format property - this property should be the one where the default gets used when it's not provided.

@AaronFriel recommended:

In the mean time, you could modify the Check() method for this resource to set the value to "raw" if it's empty.

However, this doesn't work because Check isn't called before Read during a refresh operation (only Configure and Read are run).

komalali commented 1 year ago

I was able to work around this by modifying Read to specifically ignore this situation.

https://github.com/pulumi/pulumi-pulumiservice/pull/138/commits/1f9c25fbab67ab0053f52dd0fa63398aa7810404#diff-44afb8cfa041012398fd65edf0a58a678a8d4241a367ed65f8a35ca51706672fR374-R382

AaronFriel commented 1 year ago

@komalali did you try the Check() method of defaulting? I think it's a viable workaround.

Prior to calling RegisterResource, which triggers check/diff/create/update (collectively: write) operations, Pulumi YAML is not setting the default value for the field.

Setting the value in Check() if it's a null or empty string is in effect moving the defaulting logic from the SDK codegen (or for YAML: the language plugin) to be enforced by the provider plugin. The inputs that Check() returns are used for all subsequent write operations, so the POST/PUT to the Pulumi Cloud API would contain this defaulted value, and the state for the resource would also contain the defaulted value.

A subsequent Read() will later return whatever the Pulumi Cloud API returns on GET. As a result, refresh will not see a diff between the defaulted value in state and from the remote read. Assuming the Pulumi Cloud API honors the PUT/POST in the write operation; but were it not, that would be a provider bug.

komalali commented 1 year ago

I realized where I had made the mistake and I was able to just set the default in Check as recommended by @AaronFriel https://github.com/pulumi/pulumi-pulumiservice/blob/main/provider/pkg/provider/webhook.go#L153-L159

mikhailshilkov commented 1 year ago

@AaronFriel I understand that changing Check is a workaround that helped @komalali. Do we still have the problem for most other providers (bridged/native)? If I remember correctly, default values are applied in language SDKs, so it's explicable that YAML may miss on those.

Anyway, I'd love to see this issue updated with what steps are needed further (or closed, if everything is great). Thank you!

komalali commented 1 year ago

Indeed, the general problem that yaml isn't applying defaults (potentially just for top-level enums?) still exists and using Check was a workaround to unblock me.

komalali commented 10 months ago

I ran into this again, but it was not an enum, just a boolean. My guess is yaml does not respect any defaults.

thomas11 commented 4 months ago

This is still the case. I traced the gRPC calls for a Connection where the Port property was unspecified and see that RegisterResource responds without a port. This causes remote pulumi-command programs to panic because a port is required for SSH connections.