Filter out forbidden and terminating namespaces #2158
Conversation
| readyNamespaces = append(readyNamespaces, namespace) | ||
| } | ||
| } | ||
| return readyNamespaces |
There was a problem hiding this comment.
Functional code for the win :)
| @@ -649,36 +649,49 @@ func filterAllowedNamespaces(userClientset combinedClientsetInterface, namespace | |||
| allowedNamespaces = append(allowedNamespaces, namespace) | |||
| } | |||
| } | |||
| namespaces.Items = allowedNamespaces | |||
| return namespaces, nil | |||
| return allowedNamespaces, nil | |||
There was a problem hiding this comment.
With this change, it might make sense to update the signature to accept namespaces []corev1.Namespace and just update callsites to call it with .Items on the list? Only reason is that then this can match the functional filter that you've created below (which is nicer, imo :) ). Whatever you think.
| namespaceList, err := filterAllowedNamespaces(a.clientset, namespaces) | ||
| if err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
Not sure why this filter has been moved out to here: isn't it a superfluous (and relatively expensive) check if the request was done with the user's own credentials? (if you decide to move it back, just remember to update the wording of the second test which changed accordingly, I think)
There was a problem hiding this comment.
It's not superfluous since it's covering the use case in which a user has permissions to list namespaces (which is harmless) but don't have permissions to access resources in all the namespaces (not an admin). This prevents people from using namespaces that they don't have access.
I'm thinking on the case, for example, I (andres@corp.com) have permissions to list namespaces but only have write access to my namespace foo. If bar exists, Kubeapps will autoselect the first namespace in alphabetical order (which is bar) so by default I will get a forbidden error if I try to do anything without changing the namespace.
Description of the change
This PR address two minor issues when listing namespaces:
Applicable issues