Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Extend --kubernetes-namespace-whitelist to all operations #1471

Closed
squaremo opened this issue Oct 25, 2018 · 8 comments
Closed

Extend --kubernetes-namespace-whitelist to all operations #1471

squaremo opened this issue Oct 25, 2018 · 8 comments

Comments

@squaremo
Copy link
Member

At present, the experimental flag --kubernetes-namespace-whitelist controls which workloads appear in ListControllers and ListImages. This is useful for restricting the view of the cluster.

The usual expectation is that fluxd's operations can be entirely limited to the namespaces given; i.e., that the restriction appliees also to the resources that will be synced, workloads that are scanned for images, workloads that can be updated, and so on.

One tricky thing here is what to do with syncing resources that are not namespaced. I can think of a couple of designs:

  • Apply namespaces if they're on the whitelist; otherwise, ignore cluster-level resources unless fluxd has not been given a whitelist. A problem with this is: how do you then sync cluster-level things? A fluxd without a whitelist will try to sync everything ..

  • Apply namespaces if they're on the whitelist; otherwise, always attempt to apply cluster-level resources. This has the benefit of being simple, but you may get repeating errors if you have restricted fluxd with RBAC.

  • Have a flag determining whether fluxd will try to sync cluster-level resources, and succeed or fail based on RBAC. This at least means you can tell one fluxd to do the cluster-level things (and give it permission).

  • More hand-wavingly -- specify the whitelist in a way that is a bit closer to RBAC, so you can carve out exactly what you want to be able to operate on.

@2opremio
Copy link
Contributor

2opremio commented Dec 20, 2018

I would go for:

Have a flag determining whether fluxd will try to sync cluster-level resources, and succeed or fail based on RBAC. This at least means you can tell one fluxd to do the cluster-level things (and give it permission).

But I would do it gradually (at least in different PRs):

  1. Apply namespace filtering to everything which has one (i.e. always include global resources)
  2. Take RBACs into account to avoid errors
  3. Add a flag to filter global resources

However, as far as I recall (I haven't done k8s stuff in a few months, so correct me if I am wrong) there aren't so many global resources, so I wonder if flux.weave.works/ignore would be enough to avoid implementing 3 (and maybe 2). I am assuming namespace whitelisting is meant to be used as a filtering mechanism and not as a security policy. If the latter were true then the ignore label won't be enough.

@squaremo Thoughts?

@squaremo
Copy link
Member Author

@2opremio I like that approach -- make it consistent first, then more sensitive to RBAC.

(Side request: can we deprecate --kubernetes-namespace-whitelist, in favour of kubenernetes-allow-namespace, which is a more direct description ('allow') and leaves a bit more room for a family of flags (--kubernetes-allow-global, --kubernetes-block-namespace, ...).)

@rade
Copy link
Contributor

rade commented Jan 21, 2019

Excuse my ignorance, but why do we need a namespace whitelist/blacklist at all? Isn't RBAC alone sufficient for controlling which namespaces flux should/shouldn't touch?

@2opremio
Copy link
Contributor

2opremio commented Jan 22, 2019

@rade I am not an RBAC expert (yet!) but AFAIU, it should be sufficient as long as fluxd can list the RBAC Roles/Bindings affecting itself (which I am not sure is always true).

I was assuming that we wanted to maintain the flag due to:

  1. Historical reasons
  2. Users wanting to restrict the namespaces further than RBAC rules (ERRTOOMANYKNOBS?)

Regardless, starting with an explicit list of namespaces has been a good way to solve the problem incrementally (worrying about RBAC comes next, my plan is extracting that list from the RBAC rules).

@rade
Copy link
Contributor

rade commented Jan 22, 2019

as long as fluxd can list the RBAC Roles/Bindings affecting itself

Why would it have to be able to list them? Surely simply trying to do something, and correctly interpreting any resulting error due to RBAC, would suffice.

Users wanting to restrict the namespaces further than RBAC rules (ERRTOOMANYKNOBS?)

ERRTOOMANYKNOBS indeed. Furthermore, the extra knob has to keep that consistent with the RBAC knob.

@2opremio
Copy link
Contributor

2opremio commented Jan 22, 2019

Why would it have to be able to list them? Surely simply trying to do something, and correctly interpreting any resulting error due to RBAC, would suffice.

I am not sure that suffices.

I don't yet know all the architecture details of flux but I see a few problems related to narrowing down candidate resources for subsequent operations:

  1. AFAIU (please correct me if I am wrong) when releasing a new image version for all services (either manually or automatically), resource manifests are committed and pushed to git first and then synced with the cluster. I don't think we want to make a commit of something which won't be able to be reflected on the cluster.

  2. For some operations it's desirable to know that future actions will be allowed later on. For instance, when listing images/services (either in fluxctl or weave cloud) it's desirable to narrow down the list (or at least have a way to grey out) the ones you cannot later operate on. AFAIU, RBAC may allow flux to list a resource but necessarily modify them.

And there's probably other cases I am missing.

Now, RBAC has resource-granularity, so, a list of namespaces won't be enough but the sample issues apply.

@ipedrazas
Copy link

There are two things:

  • The API calls work at cluster level or per namespace
  • RBAC gives you the ability to define who can do what and where (a namespace: Role or cluster wide ClusterRole)
/api/v1/pods

Or

/api/v1/namespaces/flux/pods

This creates a problem when you have a flux to manage your cluster and you want another locked-down version to manage your application.

Having the ability to whitelist/blacklist namespaces is very useful because bny doing so, flux is saying no, I will not manage resources in a blacklisted namespace regardless of the RBAC configuration

@2opremio 2opremio removed their assignment Jan 15, 2020
@kingdonb kingdonb self-assigned this Feb 20, 2021
@kingdonb
Copy link
Member

Too much newness in this proposal, it would either be a breaking change or a new feature, neither of which can be accepted as Flux v1 is in maintenance.

Closing. Thanks

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants