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

Support re-rolling unrolled recursive TF types #1468

Open
iwahbe opened this issue Oct 24, 2023 · 7 comments
Open

Support re-rolling unrolled recursive TF types #1468

iwahbe opened this issue Oct 24, 2023 · 7 comments
Labels
impact/performance Something is slower than expected kind/enhancement Improvements or new features size/M Estimated effort to complete (up to 5 days).

Comments

@iwahbe
Copy link
Member

iwahbe commented Oct 24, 2023

Hello!

  • Vote on this issue by adding a 👍 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

The terraform type system is structural, meaning that subtypes are not named. This prevents terraform from describing recursive types. Terraform providers fake recursion by manually unrolling recursive types to a given depth, generating a huge number of types in the process.

Pulumi needs to generate an in-code representation of these types leading to PRs like this:

Screenshot 2023-10-24 at 2 34 46 PM

Pulumi's type system is nominal (mirroring most programming languages), and we do support recursive types. Recursive typing is worse in Pulumi then it is in TF (for bridged providers) because of the translation, but Pulumi providers can actually offer a better experience than TF providers here.

Solution

The bridge should have native support for re-rolling recursive types back into their recursive descriptions.

A MVP implementation enables designating a resource type on ProviderInfo as "unrolled recursive" on specific fields. The bridge would then handle re-rolling that type.

An ideal solution would automatically detect deep nesting, and then re-roll without manually marking as recursive. This would require inferring that a type is recursive, and on what fields it recurses on, then calling into the MVP solution.

Backwards compatibility

The MVP is fully backwards compatible, since it is off by default. Automatic detection is technically breaking, but we can configure it so that it only breaks providers with an unusable number of resources (wafv2 style).

Affected area/feature

Prior Art

@iwahbe iwahbe added needs-triage Needs attention from the triage team kind/enhancement Improvements or new features labels Oct 24, 2023
@iwahbe
Copy link
Member Author

iwahbe commented Oct 24, 2023

This effectively blocks pulumi/pulumi-datadog#359, unless we want to ship a massive performance regression.

@iwahbe iwahbe added the size/M Estimated effort to complete (up to 5 days). label Oct 24, 2023
@mikhailshilkov mikhailshilkov added impact/performance Something is slower than expected and removed needs-triage Needs attention from the triage team labels Oct 26, 2023
@iwahbe
Copy link
Member Author

iwahbe commented Oct 31, 2023

Looks like this is also blocking pulumi/pulumi-akamai#314.

@t0yv0
Copy link
Member

t0yv0 commented Jan 22, 2024

Curious @iwahbe if you want to hand it over to me or @VenelinMartinov or still planning to finish out. Exciting feature.

@iwahbe
Copy link
Member Author

iwahbe commented Jan 26, 2024

Curious @iwahbe if you want to hand it over to me or @VenelinMartinov or still planning to finish out. Exciting feature.

I'm going to continue next iteration. I'd love some help finishing, it's a thornier technical issue then I expected.

@t0yv0
Copy link
Member

t0yv0 commented Sep 13, 2024

There's some related work I've been trying to wrap up. There are coupled problems.

  1. detect recursion emulation
  2. detect type replicas
  3. remove type replicas

This PR has reasonably good recursion detector, but it is written against PackageSpec instead of shim.Schema:

#2134

The next PR explores non-recursive type replica detection and removal at scale. It turns out scale makes things difficult-ish. The evolving approach is to have tooling that detects replicas and then has maintainer approve of them and pick names. Approval step is helpful to eliminate accidentally shared types that will evolve separately upstream. Naming is necessary to avoid super long tokens. Working on shim.Schema level instead of PackageSpec level seems advantageous for this use case.

pulumi/pulumi-aws#4449

This PR landed to give a mechanism for curtail type explosion at tfgen level:

#2409

@iwahbe
Copy link
Member Author

iwahbe commented Sep 26, 2024

pulumi/pulumi-akamai#366 is an instance of recursive types (and the subsequent compilation resource explosion).

@t0yv0
Copy link
Member

t0yv0 commented Sep 27, 2024

The PR is not quite ready but I learned a lot from prototyping, I'd like to pitch this sometime soon-ish on this to move forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/performance Something is slower than expected kind/enhancement Improvements or new features size/M Estimated effort to complete (up to 5 days).
Projects
None yet
Development

No branches or pull requests

3 participants