pulumi / pulumi

Pulumi - Infrastructure as Code in any programming language 🚀
https://www.pulumi.com
Apache License 2.0
21.34k stars 1.1k forks source link

Adding IsOverlay entries to register a dangling token results in breaking changes #17274

Open t0yv0 opened 6 days ago

t0yv0 commented 6 days ago

What happened?

As part of customer-requested https://github.com/pulumi/pulumi-aws/issues/2565 we have been investigating dangling references to non-existing types in Pulumi AWS Package Schema. A common problem is with Node-only overlays where object types or enumerations are implemented manually but only made available in Node JS. These types currently rely on dangling references.

We have two options to move forward:

  1. remove the overlays in favor of fully schematized types; we need to work this out for every instance but this is the general preference. However these changes are breaking in some of the language projections and therefore we cannot move forward until AWS major v7.

  2. in the meantime, it seems like it should be possible to fully explain overlay types in the Pulumi package schema without breaking the existing SDKs.

Spending some time with (2) it appears that IsOverlay option doesn't work as expected and generates breaking changes.

Example

Tried filling out a dangling token in AWS:

    typeOverlays["aws:index/aRN:ARN"] = schema.ComplexTypeSpec{
        ObjectTypeSpec: schema.ObjectTypeSpec{
            Type:      "object",
            IsOverlay: true,
        },
    }

The schema diff introduced a new entry, which is good:

+        "aws:index/aRN:ARN": {
+            "type": "object",
+            "isOverlay": true
+        },

However Node SDK no longer compiles with diffs like this:

-    target?: pulumi.Input<ARN>;
+    target?: pulumi.Input<inputs.ARN>;

Python SDK is suspect as well:

-                 policy_arn: pulumi.Input[str],
+                 policy_arn: pulumi.Input['_root_inputs.ARNArgs'],

Go SDK:

-   PolicyArn *string `pulumi:"policyArn"`
+   PolicyArn *aws.ARN `pulumi:"policyArn"`

Output of pulumi about

N/A

Additional context

Trying this on https://github.com/pulumi/pulumi-aws/releases/tag/v6.52.0

Contributing

Vote on this issue by adding a 👍 reaction. To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

justinvp commented 6 days ago

Seems like more of a enhancement to make IsOverlay compatible with Node.js dangling references.

t0yv0 commented 6 days ago

Err, possibly. Question is though what does the IsOverlay support for types do at present in the schema? If I add an entry under Types with IsOverlay=true that indicates something is an overlay type, and the schema indicates that languages supporting overlays are just the Node language, how would I ever be able to use the current behavior by referencing this type token somewhere?

In node when it generates:

 target?: pulumi.Input<inputs.ARN>;

Possibly if the overlay actually populated inputs.ARN this could be made to work. The AWS problem is perhaps that that the historic ARN type lives somewhere else instead. We could maybe alias it there.

Changes in languages other than node don't have great workarounds as they necessitate overlays in all languages where we want to keep them in node.

justinvp commented 6 days ago

Question is though what does the IsOverlay support for types do at present in the schema?

IsOverlay was added mainly for K8s overlay documentation in https://github.com/pulumi/pulumi/pull/8338. Even there, the overlay docs didn't always perfectly match the actual overlays for each language.

t0yv0 commented 6 days ago

Excellent ref, thanks! Indeed sounds like initial scope is documentation-only. I'll follow up on 2565 if we really need anything here or we can get by with plan A of just removing these overlays.