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

Deprecate cross-namespace refs and watching multiple namespaces #329

Merged
merged 5 commits into from
Oct 5, 2022

Conversation

squaremo
Copy link
Contributor

Cross-namespace refs won't work with the provided configuration, which gives RBAC permissions for one namespace only. Granting permissions to other namespaces means they are also accessible to the programs run by stacks, which is a security hole.

Therefore: guard the use of api/pulumi/shared/SecretRef.Namespace, and watching >1 (or all) namespaces with WATCH_NAMESPACE, with an environment variable INSECURE_NO_NAMESPACE_ISOLATION. This must be set to 1 or true to enable cross-namespace operation.

This is a breaking change for anyone running the operator with either an empty value, or a comma-separated list (e.g., ns1,ns2 for WATCH_NAMESPACE; and, for anyone using cross-namespace secret references (i.e., with a value in .namespace under one of the fields listed below). The documented means of installing the operator sets WATCH_NAMESPACE to be the namespace in which the operator is deployed.

References in Stack which can be cross-namespace (* indicates there's a list or map):

  • .spec.envRefs[*].secret
  • .spec.secretRefs[*].secret
  • .spec.gitAuth.accessToken.secret
  • .spec.gitAuth.sshAuth.sshPrivateKey.secret
  • .spec.gitAuth.sshAuth.password.secret
  • .spec.gitAuth.basicAuth.userName.secret
  • .spec.gitAuth.basicAuth.password.secret

To fix the breakage:

  • if you are using a cross-namespace reference, consider using a secret in the same namespace as the Stack object. Kyverno and other such automation can help with syncing secrets across namespaces.
  • if you use WATCH_NAMESPACE with an empty value or several values, modify your configuration so that you're running the operator in each namespace where you have Stack objects. (This PR extends the provided Pulumi programs so they will do this for you.)
  • If it is not possible to use one of the fixes above, you can enable watching multiple namespaces and cross-namespace references by adding the env entry INSECURE_NO_NAMESPACE_ISOLATION to the operator deployment spec, in the same place as WATCH_NAMESPACES is defined. Be aware that this is really only suitable for "single tenant" clusters -- when you are happy for anyone with access to have access to everything.

@squaremo squaremo added security impact/security impact/breaking Fixing this issue will require a breaking change labels Sep 28, 2022
@squaremo squaremo changed the title Deprecate cross-namespace refs Deprecate cross-namespace refs and watching multiple namespaces Sep 28, 2022
@squaremo squaremo force-pushed the remove-multi-namespace branch from b4f2bb4 to 350060c Compare September 29, 2022 17:08
@squaremo squaremo requested a review from roothorp October 3, 2022 10:17
Copy link
Contributor

@lukehoban lukehoban left a comment

Choose a reason for hiding this comment

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

Namespace isolation changes LGTM. One question on the refactoring of reconciliation approach to retries/stalls.

pkg/controller/stack/stack_controller.go Outdated Show resolved Hide resolved
Cross-namespace refs won't work with the provided configuration, which
gives RBAC permissions for one namespace only. Granting permissions to
other namespaces means they are also accessible to the programs run by
stacks, which is a security hole.

Therefore: guard the use of `api/pulumi/shared/SecretRef.Namespace`, and
watching >1 (or all) namespaces with `WATCH_NAMESPACE`, with an
environment variable `INSECURE_NO_NAMESPACE_ISOLATION`. This must be set
to `1` or `true` to enable cross-namespace operation.

Signed-off-by: Michael Bridgen <[email protected]>
Signed-off-by: Michael Bridgen <[email protected]>
errors.Wrap is used extensively to provide extra context to errors. The
Go standard library now builds in wrapped errors, with `fmt.Errorf` and
the formatting instruction `%w`. This commit replaces
github.com/pkg/errors (which is archived) with uses of `fmt.Errorf`.

Signed-off-by: Michael Bridgen <[email protected]>
 - show that without INSECURE_NO_NAMESPACE_ISOLATION=true, the
   controller will fail a stack that uses a cross-ns ref
 - show that with the env entry, it succeeds

Signed-off-by: Michael Bridgen <[email protected]>
If you have a cross-namespace ref in your Stack, and namespace isolation
has not been waived, you will need to change the stack or reconfigure
the controller in order to make progress. This commit introduces a tiny
protocol (that could be used throughout, hint hint) so a helper func can
signal that an error should stall the object, rather than letting it be
retried; and, uses it in `resolveResourceRef` so that forbidden ref will
result in a stalled state.

Signed-off-by: Michael Bridgen <[email protected]>
@squaremo squaremo force-pushed the remove-multi-namespace branch from 350060c to b3666ea Compare October 4, 2022 15:45
@squaremo squaremo merged commit c81888c into master Oct 5, 2022
@squaremo squaremo deleted the remove-multi-namespace branch October 5, 2022 10:13
@yoyoraso
Copy link

Hi, I have a question related to the namespace isolation is it possible to deploy one pulumi operator in one namespace to watch and create stacks across the cluster in all namespaces ?

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 impact/security security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants