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

Put all resources in specified provider namespace #506

Merged
merged 2 commits into from
Apr 2, 2019

Conversation

lblackstone
Copy link
Member

Fixes: #415

Reviewer notes / points of discussion:

  1. This change currently requires the provider namespace to be created explicitly. This is up for discussion.
  2. The current behavior is to always override the resource namespace if it is set on the provider, even if a different namespace is specified on the resource.

Copy link
Contributor

@hausdorff hausdorff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to go ahead and approve this, but we should test it pretty extensively before shipping.

If I'm not mistaken, this is very close to a solution for #217, right? To solve that, we basically have to be able to pass along the namespace as a part of the Create/Update RPC calls, and then we just need to consult the same NamespacedKind here to decide to override the namespace, right?

@lblackstone
Copy link
Member Author

@hausdorff Right, it should be pretty straightforward to fix 217 from this change as well.

@lblackstone lblackstone force-pushed the lblackstone/provider-namespace branch from 17f0788 to 3bb5e12 Compare April 2, 2019 20:23
@hausdorff hausdorff added the impact/breaking Fixing this issue will require a breaking change label Apr 2, 2019
@lblackstone lblackstone merged commit 5204b3b into master Apr 2, 2019
@pulumi-bot pulumi-bot deleted the lblackstone/provider-namespace branch April 2, 2019 22:21
lblackstone added a commit that referenced this pull request Apr 5, 2019
lblackstone added a commit that referenced this pull request Apr 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/breaking Fixing this issue will require a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants