Lack of provider method/field in class besom.api.eks.Cluster

polkx commented 4 months ago

When I ask pulumi AI "aws eks hello world” I get: https://www.pulumi.com/ai/conversations/6ff221c6-9b0b-49be-8233-1afaf3e7fea7

import * as aws from "@pulumi/aws";
import * as eks from "@pulumi/eks";

// Create an EKS cluster with the default configuration.
const cluster = new eks.Cluster("my-cluster");

// Define the "hello world" Deployment.
const appLabels = { app: "hello-world" };
const deployment = new k8s.apps.v1.Deployment("hello-world-deployment", {
    metadata: { labels: appLabels },
    spec: {
        replicas: 2,
        selector: { matchLabels: appLabels },
        template: {
            metadata: { labels: appLabels },
            spec: {
                containers: [{
                    name: "hello-world",
                    image: "paulbouwer/hello-kubernetes:1.5",
                    ports: [{ containerPort: 8080 }],
                }],
            },
        },
    },
}, { provider: cluster.provider });

// Create a LoadBalancer Service to expose the "hello world" Deployment.
const service = new k8s.core.v1.Service("hello-world-service", {
    metadata: { labels: appLabels },
    spec: {
        type: "LoadBalancer",
        selector: appLabels,
        ports: [{ port: 80, targetPort: 8080 }],
    },
}, { provider: cluster.provider });

// Export the URL for the load-balanced service.
export const url = service.status.loadBalancer.ingress[0].hostname;

I want to use

{ provider: cluster.provider }

in Besom. Like below:

opts = opts(provider = cluster.provider)

But class besom.api.eks.Cluster don't have a provider method.

pawelprazak commented 4 months ago

Thank you for the report, I confirm we lack this method currently.

The workaround in this case is to explicitly create a provider like in this example.

@lbialy we need to find a good API design, because I'd like to avoid the problems other SDKs have with having both provider and providers. It is especially relevant here, because multiple providers are present in components, like eks.

From TS API:

    /**
     * An optional provider to use for this resource's CRUD operations. If no provider is supplied,
     * the default provider for the resource's package will be used. The default provider is pulled
     * from the parent's provider bag (see also ComponentResourceOptions.providers).
     *
     * If this is a [ComponentResourceOptions] do not provide both [provider] and [providers]
     */
    provider?: ProviderResource;

    /**
     * An optional set of providers to use for child resources. Either keyed by package name (e.g.
     * "aws"), or just provided as an array.  In the latter case, the package name will be retrieved
     * from the provider itself.
     *
     * Note: only a list should be used. Mapping keys are not respected.
     */
    providers?: Record<string, ProviderResource> | ProviderResource[];

Relevant documentation and code:

lbialy commented 4 months ago

First of all, is cluster.provider a field on the resource or a core method exposing provider used to deploy this particular resource? I assume the latter and that in other implementations it's just a method inherited from Resource, right? If so, it should be trivial to add it in core. All it has to do is to reach to the guts of context and fetch the provider from the state. The issue is - there's a probability that field provider exists somewhere so I guess we should do this as an extension method, which won't cause conflicts.

lbialy commented 4 months ago

@pawelprazak your opinion here?

pawelprazak commented 4 months ago

It's tricky IMO to provide sensible semantics here, because:

most resources will have only one provider
the one exception is that components can have multiple providers

This has some consequences that are less than ideal:

when creating a component, the user have to choose one "favorite" provider and return it as the Resource.provider and also populate all providers as well
when using a resource, user cannot know it the resource is actually a component or something else, so the user needs to guess what method to use

More context here: https://github.com/pulumi/pulumi/issues/8796

I believe Scala has strong enough type system to handle this gracefully, but I'm yet to find an elegant solution.

Alternatively, maybe the upstream implementation is good-enough and I'm overthinking this. A naive implementation would use those signatures:

def providers: NonEmptyString => Map[String, ProviderResource]
def provider: Optional[ProviderResource]

I'm curious what would StackResource return, I assume a None

lbialy commented 4 months ago

You're thinking in terms of inheritance. We are free to ditch inheritance whenever we want to! This. is. Scala!

We almost never upcast to Resource supertype so the easiest way to get this is to just use extension methods and the design is kinda trivial:

extension (cr: CustomResource) // this also handles ProviderResources but for them it's probably nonsensical
  def provider: Output[ProviderResource] = ???

extension (cb: ComponentResource)
  def providers: Output[Map[String, ProviderResource]] = ???

extension (cb: RemoteComponentResource)
  def providers: Output[Map[String, ProviderResource]] = ???

lbialy commented 1 month ago

fixed with #505 @polkx can you verify it works like you'd expect via just clean-all publish-local-all with 0.4.0-SNAPSHOT version of core?

VirtusLab / besom

Lack of provider method/field in class besom.api.eks.Cluster #397