pulumi / pulumi-yaml

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

Introduce `fn::method` function for resource methods #431

Open aq17 opened 1 year ago

aq17 commented 1 year ago

Fixes #354 Allows resource methods to be called via fn::method, using the following syntax:

variables:
  kubeconfig:
    fn::method:
      resource: ${someClusterResource}
      function: getKubeconfig
      arguments:
        ...

Both the full function type token or just the function name may be used

iwahbe commented 1 year ago

It would be great if you could write up which syntax you have implemented, as there were several suggestions on the original issue. A test would likewise make the desired behavior clearer.

One thing comes to mind immediately. We have CallOpts of type InvokeOptionsDecl. I think this means we are allowing users to invoke a method on a resource of a different version/provider. I'm not sure this makes sense. @justinvp would know more if this should be allowed.

justinvp commented 1 year ago

One thing comes to mind immediately. We have CallOpts of type InvokeOptionsDecl. I think this means we are allowing users to invoke a method on a resource of a different version/provider. I'm not sure this makes sense. @justinvp would know more if this should be allowed.

Yeah, I wouldn't expect we'd allow users to pass these. It should be passed along from the resource itself.

aq17 commented 1 year ago

@AaronFriel update: pulumi up should be working with the syntax we discussed with @justinvp Todo:

AaronFriel commented 1 year ago

@aq17 unfortunately that will require extending the MockResourceMonitor:

https://github.com/pulumi/pulumi/blob/9d7ccc628b2fed47ac0da67b1526bb2fed265acc/sdk/go/pulumi/mocks.go#L113-L158

It looks like due to a fluke or naming mistake, that interface uses a method named Call and implementations typically use a field named CallF to mock the Invoke RPC. That'll make adding support for mocking the Call RPC a little unusual.

@justinvp what do you think?

justinvp commented 1 year ago

@justinvp what do you think?

This is https://github.com/pulumi/pulumi/issues/8328

(Yes, it is unfortunate that the mock name for invokes is Call rather than Invoke.) If we were to add support for mocking the gRPC Call, I think the simplest thing to do would be to make the gRPC Call call the mock Call. It might seem a little strange, but having the Invoke and Call gRPCs both call the mock Call method is similar to how the RegisterResource and ReadResource gRPCs both call the mock NewResource method. (And if we needed to, at some point, we could add more fields to MockCallArgs to indicate whether it's an Invoke or Call gRPC, etc.)

The alternative is doing more design work to have a separate method in the mock just for Call. Not sure what we'd name it, though.

t0yv0 commented 1 year ago

👋 any updates on this, bumping into this in the context of https://github.com/t0yv0/pulumi-12709 which special cases for single-value Resource returning methods, and YAML is part of the requirement. CC @lblackstone

It sounds like there's no current workaround to call the methods in YAML, it just is not possible. Considering options here, any potential workarounds or perhaps would it be feasible for me to pick up this PR and bring it to completion. I haven't contributed to this codebase yet but can get ramped up if needed. That's an option..

I can quickly look at the possibility if YAML can pass an Output<Resource> that is a component resource output, not method call, to another resource's provider: option. This would be asymmetric to other languages but could be acceptable.

iwahbe commented 1 month ago

Getting this in would make testing Call much easier.