nextflow
nextflow copied to clipboard
Change k8s client to use namespace and service account from nextflow config
Closes #2266
I used <class>.metaClass.static.<method>
to mock static methods. Looks like it works for Java classes as well, so it could eliminate the need to create little helper methods like ConfigDiscovery.path()
just for testing purposes.
I thought it was cleaner to separate the config discovery from the auth token discovery, rather than pass the namespace and service account down the call stack to discoverAuthToken()
. That required making ConfigDiscovery
a static helper class, which makes sense to me because ConfigDiscovery
doesn't really have any state like an object. However this testing framework really doesn't play well with static methods. I guess I could make ConfigDiscovery
a singleton instead, like OperatorImpl
.
it's already, what's done for the context, that than it's used in the fromConfig that's the same scope invoking also discoverAuthToken.
Looks straightforward extending it to support also namespace
and serviceAccount
ConfigDiscovery.fromConfig
loads the namespace and user from the current context, but K8sConfig.getClient
applies the namespace and serviceAccount from the nextflow config if they are provided. Basically, the nextflow k8s
config settings should be used first, and the current context used as a backup.
I think there is still a logic error here -- if the current context contains a user with a token, it will be used even if a service account is provided in the nextflow config. But I think the service account from the nextflow config should be used first.
ConfigDiscovery.fromConfig loads the namespace and user from the current context, but K8sConfig.getClient applies the namespace and serviceAccount from the nextflow config if they are provided. Basically, the nextflow k8s config settings should be used first, and the current context used as a backup.
Indeed, namespace
and serviceAccount
in the k8s
config should take precedence over the one in context.
Let's focus on this.
Here is my understanding of things:
- load user, cluster, and namespace from current context
- load certs and auth token from current user (if present)
- use namespace from nextflow config (if present)
- use service account from nextflow config (if present)
And the priority for credentials is as follows:
- service account from nextflow config -- discover auth token via kubectl
- current context user -- use certs from kube config
- current context user -- use auth token from kube config
If these priorities are correct then I need to adjust the logic slightly. Also need to make sure that default
namespace and default
service account are used by default.
It sounds reasonable
I think the logic is correct, however the use of discoverAuthToken
can fail when running it within a pod.
Is there any Kube API that can be used to find it?
Okay it's ready. The only caveat now is that if a user+token is defined in the KUBECONFIG, it will override k8s.serviceAccount
for the purposes of API requests. But I think most users don't do that anyway. I will make another PR later to deal with this minor issue.
So the overall logic is mostly unchanged, except that the namespace and serviceAccount are now used to discover the auth token.
Merge manually trying to not rely on any intermetied state for the namespace
and serviceAccount
attributes. See c3364d0f