Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
polkx opened this issue Feb 21, 2024 · 6 comments
Closed

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

polkx opened this issue Feb 21, 2024 · 6 comments
Assignees
Labels
area/api User visible API needs-design Needs from design work: architecture, API, DX/UX
Milestone

Comments

@polkx
Copy link
Collaborator

polkx commented Feb 21, 2024

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 pawelprazak added area/api User visible API needs-design Needs from design work: architecture, API, DX/UX labels Feb 21, 2024
@pawelprazak
Copy link
Collaborator

pawelprazak commented Feb 21, 2024

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
Copy link
Collaborator

lbialy commented Feb 21, 2024

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
Copy link
Collaborator

lbialy commented Feb 22, 2024

@pawelprazak your opinion here?

@pawelprazak
Copy link
Collaborator

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: pulumi/pulumi#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
Copy link
Collaborator

lbialy commented Feb 22, 2024

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 lbialy self-assigned this Apr 21, 2024
@lbialy lbialy added this to the 0.4.0 milestone Apr 21, 2024
lbialy added a commit that referenced this issue May 25, 2024
lbialy added a commit that referenced this issue May 25, 2024
lbialy added a commit that referenced this issue May 25, 2024
@lbialy
Copy link
Collaborator

lbialy commented May 25, 2024

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?

@lbialy lbialy closed this as completed May 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api User visible API needs-design Needs from design work: architecture, API, DX/UX
Projects
None yet
Development

No branches or pull requests

3 participants