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

fix panic when evaluating cyclic environment references #466

Merged
merged 3 commits into from
Feb 27, 2025

Conversation

nyobe
Copy link
Contributor

@nyobe nyobe commented Feb 27, 2025

We track the lifetime of each environments import evaluations to detect cyclic imports. Right now this is tracked while we're evaluating the imports decl.

However, #443 introduced a way to import environment values inline. Once we've started evaluating the environment values root, we currently stop tracking whether the environment is evaluating because we're done with all the imports.

The result of this is that a cyclic environment reference will look up the environment name in the imports map and won't detect that it is still evaluating. Since the cached value in the imports map hasn't been set yet (since the environment hasn't finished evaluating) we return nil as the value. This results in a panic when we try to actually access the value.

To fix this, I'm just hoisting the evaluation lifetime tracking to the evaluation root, rather than just the imports node, so it'll correctly detect cycles for inline references.

Fixes https://github.com/pulumi/pulumi-service/issues/26277

@nyobe nyobe requested a review from pgavlin February 27, 2025 03:09
@nyobe nyobe added the impact/no-changelog-required This issue doesn't require a CHANGELOG update label Feb 27, 2025
@nyobe nyobe requested a review from seanyeh February 27, 2025 03:10
@nyobe nyobe changed the title fix environment reference cycle panic fix panic when evaluating environment reference cycle Feb 27, 2025
@nyobe nyobe changed the title fix panic when evaluating environment reference cycle fix panic when evaluating cyclic environment references Feb 27, 2025
@nyobe nyobe enabled auto-merge (squash) February 27, 2025 17:59
@nyobe nyobe merged commit 36b8da2 into main Feb 27, 2025
4 checks passed
@nyobe nyobe deleted the claire/fix-ref-cycle-panic branch February 27, 2025 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/no-changelog-required This issue doesn't require a CHANGELOG update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants