<!---
Thanks so much for your contribution! If this is your first time
contributing, please ensure that you have read the
[CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md)
documentation.
-->
# Description
<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. -->
Currently, when we detect that we've created a cycle in the dependency
graph, we early-exit. This works well enough for simple cycles, but when
early exiting is not sufficient (as when providing a resource output as
an argument to another resource's inputs), we will still fail to resolve
the full dependency graph. This PR introduces an exception-by-default,
as any cycles represent an unsafe/invalid dependency graph and should be
resolved manually. We also provide an escape hatch to fall back to
current behavior, in case users would prefer to retain the ability to
create unsafe dependency graphs (potentially introducing infinite hangs
when resolving those graphs).
Fixes https://github.com/pulumi/pulumi/issues/13551
## Checklist
- [x] I have run `make tidy` to update any new dependencies
- [x] I have run `make lint` to verify my code passes the lint check
- [ ] I have formatted my code using `gofumpt`
<!--- Please provide details if the checkbox below is to be left
unchecked. -->
- [x] I have added tests that prove my fix is effective or that my
feature works
<!---
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the
`changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the
Pulumi Cloud,
then the service should honor older versions of the CLI where this
change would not exist.
You must then bump the API version in
/pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi
Cloud API version
<!-- @Pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->
<!---
Thanks so much for your contribution! If this is your first time
contributing, please ensure that you have read the
[CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md)
documentation.
-->
# Description
<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. -->
Fixes https://github.com/pulumi/pulumi/issues/13997.
It would be a little nicer if every input type _actually_ implemented
the `Mapping` interface, but that trying to do that turned into a much
deeper fix than this one.
## Checklist
- [x] I have run `make tidy` to update any new dependencies
- [x] I have run `make lint` to verify my code passes the lint check
- [ ] I have formatted my code using `gofumpt`
<!--- Please provide details if the checkbox below is to be left
unchecked. -->
- [x] I have added tests that prove my fix is effective or that my
feature works
<!---
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the
`changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the
Pulumi Cloud,
then the service should honor older versions of the CLI where this
change would not exist.
You must then bump the API version in
/pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi
Cloud API version
<!-- @Pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->
A previous attempt to fix this in 5dd813ea47 didn't fully address the problem when there are more complex parent/child relationships with components.
This change fully addresses the problem by using the same cycle detection logic used elsewhere for local components. We now check for cycles up-front and exit early.
When a resource depends on a local component resource, rather than setting the component resource itself as a dependency, each of the component's descendants is added as a dependency. This can lead to hangs when cycles are introduced.
For example, consider the following parent/child hierarchy, where `ComponentA` is the parent of `ComponentB` and `ComponentB` is the parent of `CustomC`:
ComponentA
|
ComponentB
|
CustomC
If `ComponentB` specifies it has a dependency on `ComponentA`, the following takes place as part determining the full set of transitive dependencies:
1. `ComponentA` is a component resource so it isn't added as a dependency, its children are.
2. `ComponentA` has one child: `ComponentB`
3. `ComponentB` is a component resource so it isn't added as a dependency, its children are.
4. `ComponentB` has one child: `CustomC`, a custom resource.
5. Since `CustomC` is a custom resource, it is added to the set of dependencies.
6. We try to await its URN, but we'll never get it because `RegisterResource` hasn't yet been called for it. And we hang waiting.
To address this, skip looking at a component's children if it is the component from which the dependency is being added.
In the example, with the fix, at step 3 the dependency expansion will stop: we won't look at `ComponentB`'s children because we're adding the dependency from `ComponentB`.
PR feedback
It's currently possible in Pulumi to pass a ProviderResource
as a Provider option to a ComponentResource.
The ComponentResource will record the Provider for later,
and propagate it to its child resources.
This is currently the behavior in [all SDKs except Python][1].
[1]: https://github.com/pulumi/pulumi/issues/12161#issuecomment-1446864847
The reason this didn't work was because in Python's
providers merging logic, we use the incorrect package name
when saving the provider.
Instead of using the package name reported by the provider,
we were using the package name of the current resource
(the ComponentResource in this case).
To fix this, verify that the package name of the provider
matches the package name of the resource we're building.
If it doesn't, we will still save the provider
for child resources.
Resolves#12161
The MergeResourceOptions test was incorrectly invoking
the static method ResourceOptions.merge as an instance method.
This issue was reported by the type-checker.
* Inputty depends_on support for Python
* Fix circular import
* Add a test, fix bugs found by test
* Fix typo
* Minimize loss of dep information in presence of unks to match Node behavior
* Add CHANGELOG entr
* Update sdk/python/lib/pulumi/output.py
Co-authored-by: Justin Van Patten <jvp@justinvp.com>
* Update sdk/python/lib/test/test_resource.py
Co-authored-by: Justin Van Patten <jvp@justinvp.com>
* Fix _is_prompt
* Enhance tests, found out a gap against Node impl
* Add non-listy case
* Internal functional combinators
* Do not use all/deep await when merging
Co-authored-by: Justin Van Patten <jvp@justinvp.com>
When initializing `DependencyProviderResource`, pass the package to the base constructor instead of an empty string s.t. the provider is usable when its package is read.