Get individual namespaces when given whitelist #1298
Conversation
squaremo
force-pushed
the
dont-require-list-ns
branch
from
abaa1b6 to
c5bfdd1
Compare
|
/cc @justinbarrick @mwhittington21 |
Asking for the full list of namespaces requires a cluster-scoped permission of listing namespaces; however, a common scenario for using the whitelist is that you want to restrict permissions. If we simply Get the whitelisted namespaces, ignoring those we're forbidden to see (or that don't exist, as before), we don't need the cluster-scoped permission and can just be given permissions per namespace. The trade is that we do an API request per whitelisted namespace. I expect there to be relatively few, though, so I don't think this is a huge deal.
squaremo
force-pushed
the
dont-require-list-ns
branch
from
c5bfdd1 to
9b7aee1
Compare
|
I presume this would be part of a fix for the question I raised recently: |
This changes how whitelisting is implemented, so you need to give flux slightly fewer permissions. I'll do a follow-up that documents exactly what you do need, but with this PR, it should be a little as:
|
|
Awesome! |
cluster/kubernetes/kubernetes.go
Outdated
| for _, name := range c.nsWhitelist { | ||
| if ns, err := c.client.CoreV1().Namespaces().Get(name, meta_v1.GetOptions{}); err == nil { | ||
| nsList = append(nsList, *ns) | ||
| } else if !(apierrors.IsNotFound(err) || apierrors.IsUnauthorized(err) || apierrors.IsForbidden(err)) { |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
squaremo
force-pushed
the
dont-require-list-ns
branch
from
d0a846e to
8600d06
Compare
|
If you wanted to avoid spamming the logs you could cache in memory which namespaces you have logged errors for and rate limit it. |
cluster/kubernetes/kubernetes.go
Outdated
| nsList = append(nsList, *ns) | ||
| case apierrors.IsUnauthorized(err) || apierrors.IsForbidden(err) || apierrors.IsNotFound(err): | ||
| if !c.nsWhitelistLogged[name] { | ||
| c.logger.Log("warning", "whitelisted namespace unauthorized, forbidden, or not found", "namespace", name) |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
If we log a warning every time a whitelisted is missing, there may be an awful lot of repeated warnings. Instead, keep track of which namespaces have been seen to be missing (resetting when they are seen again), and log only when the namespace was not known to be missing.
squaremo
force-pushed
the
dont-require-list-ns
branch
from
2e9ad51 to
2302abf
Compare
Asking for the full list of namespaces requires a cluster-scoped permission of listing namespaces; however, a common scenario for using the whitelist is that you want to restrict permissions.
If we simply Get the whitelisted namespaces, ignoring those we're forbidden to see (or that don't exist, as before), we don't need the cluster-scoped permission and can just be given permissions per namespace.
The trade is that we do an API request per whitelisted namespace. I expect there to be relatively few, though, so I don't think this is a huge deal.