pulumi / pulumi-linode

Linode resource provider for Pulumi
Apache License 2.0
27 stars 7 forks source link

Several issues with incorrect types in pulumi-linode #140

Closed naegelin closed 3 years ago

naegelin commented 3 years ago

When creating new instances of nodebalancers, nodebalancer configurations, or other linode objects the id returned is often represented as a string by the Output but any dependent Inputs expect this to be an integer.

Steps to reproduce

I followed the example for creating a linode nodebalancer per the docs (literally cut and paste the example) . The example is this snippet from https://www.pulumi.com/docs/reference/pkg/linode/nodebalancerconfig/:

foobar = linode.NodeBalancer("foobar",
    label="mynodebalancer",
    region="us-east",
    client_conn_throttle=20)

foofig = linode.NodeBalancerConfig("foofig",
    nodebalancer_id=foobar.id,
    port=8088,
    protocol="http",
    check="http",
    check_path="/foo",
    check_attempts=3,
    check_timeout=30,
    stickiness="http_cookie",
    algorithm="source")

The error is:
error: linode:index/nodeBalancerConfig:NodeBalancerConfig resource 'foofig' has a problem: Attribute must be a whole number, got 143111. Examine values at 'NodeBalancerConfig.NodebalancerId'.

Expected: Compatible types across various linode objects

Actual: Integers returned as strings for id fields in numerous places.

To fix the problem at the moment one must use Output.apply to cast from string to int which is not ideal and makes the code messy.

Example workaround:

foobar = linode.NodeBalancer("foobar",
    label="mynodebalancer",
    region="us-east",
    client_conn_throttle=20)

foofig = linode.NodeBalancerConfig("foofig",
    nodebalancer_id=foobar.id.apply(lambda x: int(x)),  #using apply to fix type issue
    port=8088,
    protocol="http",
    check="http",
    check_path="/foo",
    check_attempts=3,
    check_timeout=30,
    stickiness="http_cookie",
    algorithm="source")
displague commented 3 years ago

.id attributes in Terraform are always strings. The Pulumi provider is inheriting this. On Linode, the resource ids happen to be integers for these resources and generally, a secondary alias attribute (nodebalancer.nodebalancer_id) is not provided based on the intention to match the API (where the id field is returned).

I wonder if the Pulumi tools that generate the provider, docs, and examples could use type hints on the destination field and type-cast accordingly in the examples (nodebalancer_config.nodebalancer_id is an int, so nodebalancer.id must be cast to int from string when provided).

naegelin commented 3 years ago

While I see your point and updating the documents to reflect correct syntax is nice, I would argue that this negatively impacts the developer experience. In the documented example above, nodebalancer_id is a hard requirement in creating a NodeBalancerConfig so having to type cast between two related properties seems unnatural .

infin8x commented 3 years ago

Duplicate of pulumi/pulumi-terraform-bridge#352

We're tracking the larger fix for this issue in the pulumi-terraform-bridge issue mentioned above. For future discoverers of this issue, you can unblock yourself using the workaround:

To fix the problem at the moment one must use Output.apply to cast from string to int which is not ideal and makes the code messy.