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

Introduce an import heuristic to drop entries that fail validators #2314

Open
1 task
t0yv0 opened this issue Aug 14, 2024 · 2 comments
Open
1 task

Introduce an import heuristic to drop entries that fail validators #2314

t0yv0 opened this issue Aug 14, 2024 · 2 comments
Assignees
Labels
area/import kind/enhancement Improvements or new features

Comments

@t0yv0
Copy link
Member

t0yv0 commented Aug 14, 2024

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

Import for bridged providers may return data from Read() that does not pass validations. This causes Pulumi to report warnings.

Since the issue does not manifest as badly in TF as in Pulumi, the underlying Read() implementations are unlikely to be fixed upstream at scale. The suggestion therefore is to introduce heuristic-like code in the bridge that tries to autocorrect Read() results and drop entries that fail to pass validators. A similar tactic was applied in #1948

Initial implementation in #2337

Todo:

  • Account for nested properties.

Affected area/feature

Affects import. See also:

@t0yv0 t0yv0 added kind/enhancement Improvements or new features needs-triage Needs attention from the triage team labels Aug 14, 2024
@t0yv0 t0yv0 added area/import and removed needs-triage Needs attention from the triage team labels Aug 14, 2024
corymhall added a commit that referenced this issue Aug 22, 2024
> This is a reroll of #2277 which was reverted in #2299
> I've added additional handing for secret values

During an import operation property values are read from the cloud and
turned into inputs which then becomes part of the generated code. For
some properties (Computed & Optional) the cloud can return values that
fail validation. Since these are computed & optional the provider will
allow the user to not specify a value. In these cases the import would
succeed, but if the user took the generated code as is and attempted to
run pulumi up it would fail due to the validation errors.

Rather than generate invalid inputs we will instead run Validate during
import
and then processes any resulting validation errors. If any properties
fail validation (and they match our other cases) then we remove those
inputs so that the generated program code will not contain the invalid
inputs

NOTE: we want to (for now at least) scope this as narrowly as possible
because we do not want to
create a scenario where the resulting inputs create a different error
(e.g. removing an invalid required property). In these cases we will
just allow the invalid configuration
through and the user will have to decide what to do.

fixes pulumi/pulumi-aws#4054, re #2314
@t0yv0
Copy link
Member Author

t0yv0 commented Sep 9, 2024

@corymhall I think #2337 has this mostly done but didn't tackle nested properties, that sounds right? Leaving it in the backlog for now until we have more evidence like linode 373 that this needs fixing.

@corymhall
Copy link
Contributor

Yep that is correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/import kind/enhancement Improvements or new features
Projects
None yet
Development

No branches or pull requests

2 participants