Add description (only flag missing one).

Added more semantics here.

I don't think we'll have time to impl this

in v1.6

This feature could be added without breaking compatibility

I actually don't think we have to support this, do you?
If the user wants a TTL, they can remove the token automatically and add a new one that suits their needs.

Don't want to get too heavy on the flags here

2 reasons for this:

1. The "never expire" default is a bit on the sketchy side from a security point of view. Having an option from the start is a small way to start mitigating it.
2. Even having this parameter there with a help string that says "forever" will clue people in to what the default behavior is. This is a cheap way to document something that is somewhat security sensitive.

This doesn't make sense I think. We should just do the --token on the init side, I think this bloats the options here, since everything here's configurable with kubeadm token afterwards

--bootstrap-auth-token should be --tls-bootstrap-auth-token instead on kubeadm join and will work if there's a such secret with usage-bootstrap-authentication

So -- do we want to support having a different token for different usages? I can see going down to a single token but I want to make sure we don't preclude scenarios here.

I'll remove the specialized flags. We can look to add them back later.

I don't think this is needed. I've implemented this as a client.CoreV1().ConfigMaps(metav1.NamespacePublic).Get(bootstrapapi.ClusterInfoConfigMap) and I don't think this path should be configurable.
If the user needs to config this, go with https or file

I'm OK removing this for now. The only situation I can think of, at some point, may involve federation type stuff where I use one cluster to discover another cluster. But I'm having a hard time really wrapping my head around those scenarios.

Point out the signing makes is valid for --discovery-token and authentication makes it valid for --tls-bootstrap-token

this is a duration, point that out

nit: token-id.token-secret
So you mean this should be like this with both the full expiration time and the time that's left from Now()?

TOKEN                                   TTL    EXPIRATION                       USAGES
abcdef.0123456789abcdef      8h      01.01.2018T00:00:00.000   signing,authentication

or just

TOKEN                                   TTL    USAGES
abcdef.0123456789abcdef      8h     signing,authentication

I was thinking listing the expiration time in addition to the TTL. 2 reasons -- TTL will be rounded to be human readable and it doesn't hold up if someone is looking at the output from a script.

But up to you.

spelling (leoading)

I'm of the opinion that this should not support http or or https+insecure. It doesn't today.

Sure -- happy to remove it. We can always add it back later if we see a compelling case.

This token is used for both discovery and TLS bootstrap.

suggest: This token has both the usage-bootstrap-signing and usage-bootstrap-authentication flags set to true, which means it will be used for both discovery and TLS bootstrap.

Hmm, can we rename these to --tls-bootstrap-from-url or --tls-bootstrap-url instead?
I think that makes more sense since what's actually happening behind the scenes is just the TLS Bootstrap, nothing else...

This is discovery, not tls bootstrap. Discussion below.

Same goes for this arg: can it be --tls-bootstrap-from-file?

Sorry if I was unclear last review (I was in a hurry), but I do think we should keep what you had in terms of exposing both --discovery-token and --tls-bootstrap-token and using --token as a shorthand for setting both.

This way, the only --discovery-xxx flag we have is token, because that's actually the only one that's gonna do discovery with the cluster-info ConfigMap, JWS validation and all that. I see this as two phases:

1. Unsecure HTTP GET call to the cluster-info ConfigMap for getting the CA and master list and validate the content we recieved using the token -- this is discovery and is only available for the token method
2. TLS Bootstrap -- do the CSR dance and connect to the API Server with whatever creds you have (existing client cert or token), but this is most often a token regardless of method -- this phase is available for file, url and token.

I propose to define this in the doc and refer to 1. as discovery and 2. as just TLS bootstrap.
In the future, 2. may also be passed to the kubelet: kubernetes/kubeadm#173

So TL;DR; --discovery-token is used in 1. and --tls-bootstrap-token is used in 2. and --token sets both

I'm confused here.

Discovery == "getting and trusting the cluster-info file". We have 3 ways to do this: token, HTTPS endpoint (trusting the system CA bundle) and local file. This is driven by the --discovery-* flags.

I think you are wrong wrt 2. The TLS bootstrap dance only works with a token. It is what is used to authenticate to the API server using temporary credentials (the token) in order to submit a CSR. That would be the --tls-bootstrap-token flag. I'll make sure we have that.

Ok, I see how you're thinking.
Let's see if we can come to a conclusion here... This is what I thought when I wrote that comment:

Discovery

Scenario A: We only have a token and an address to an API Server. What do we do?
We still need a CA cert, and in the future the addresses to the rest of the API Servers in the clusters.

We construct a KubeConfig file like this:

apiVersion: v1
kind: Config
clusters:
- cluster:
     server: https://BOOTSTRAP_SERVER:PORT
     insecureTLSVerify: false
  name: discovery
contexts:
- context:
    cluster: discovery
    user: ""
  name: discovery
current-context: discovery

and do

client.CoreV1().ConfigMaps(metav1.NamespacePublic).Get("cluster-info")

in order to get the KubeConfig with the CA cert and cluster addresses + the JWS token.
Then we validate the JWS token against our own token and the KubeConfig file we've got.

If everything succeeds, we'll end up with a KubeConfig file like this:

apiVersion: v1
kind: Config
clusters:
- cluster:
     server: https://SERVER:PORT{https://SERVER2:PORT2,...}
     certificate-authority-data: <cacertbytes>
  name: tlsbootstrap-ready
users:
- name: tlsbootstrap-ready
  user:
    token: <tls-bootstrap-token-id>:<tls-bootstrap-token-secret>
contexts:
- context:
    cluster: tlsbootstrap-ready
    user: tlsbootstrap-ready
  name: tlsbootstrap-ready
current-context: tlsbootstrap-ready

This is what we need for the next phase (TLS Bootstrap). I.e. this is the output of the discovery phase and the input to the TLS bootstrap phase in my world.

Scenario B: We only have a KubeConfig file with credentials that we can use to talk in a secure manner to the cluster. What do we do?

Well, since we have all the information needed for TLS Bootstrap already, we'll just skip this phase and go on to the next.

At least, this is what we do now in kubeadm. I don't know, maybe it makes sense to hit the cluster-info ConfigMap regardless in order to check for updated information (which can be more than the CA and the apiserver list in the future). If that was what you thought, then I support that. But FYI, that's far from what's implemented now.

TLS Bootstrap

This is just "the normal" TLS bootstrap flow, which in the future may/should be implemented by the kubelet itself for easier lifecycle management of those certs.

The only thing we need as an input here is a KubeConfig file that has:

• at least one address to an apiserver
• a CA cert so we can trust the apiserver in question
• an user that has access to the Certificates API. Using a token here is by far the most common method, but I don't think it matters how you're connecting. By default in kubeadm, the system:bootstrappers group is granted access to the Certs API. If the cluster admin decided to make client certs that has system:bootstrapper as its Organization, I guess this will just work as well as using a token.

If we should hit the cluster-info endpoint regardless of (token|file|https), then there should be --discovery-(file|token|https). If we should just do a "fast-forward" and use the kubeconfig taken from the file or https and give it to TLS bootstrap directly (how it's implemented currently), this should be --tls-bootstrap-(https|file)

Anyway I think it's very important to define the inputs/outputs here. WDYT about this?

Wrt Discovery -- This is all outlined in the first section of the document. This is about distributing information about the cluster itself including the CA bundle for that server. Over time we can use this to find a set of API servers but that is for the future. This has nothing to do with auth at all and it is bad news if you put TLS client certificates in cluster-info. Earlier in the doc I say "It is expected that the kubeconfig file will have a single unnamed Cluster entry. Other information (especially authentication secrets) must be omitted."

But, once you find a cluster, how do you get a unique client certificate signed by a CA that the server trusts? That is the TLS bootstrap flow. And the option we are using here os to also use the token for that. If you have a client certificate that you've already gotten via other means we should support that. But that really isn't "bootstrap" in any way.

The client TLS stuff is a bit out of scope for this doc but I'd be cool with a set of flags like this:

• --client-tls-bootstrap-token Configure client TLS by using a token and the Cert API
• --client-tls-certificate and --client-tls-key Instead of doing TLS bootstrapping with a token, read the client certificate and key from these files. Install them in the appropriate directories. Verify that they can be used to talk to the server. Note that the flag names mirror the kubectl flags somewhat.

If either --client-tls-certificate or --client-tls-key are set then it is assumed that users do not want to use the cert API to bootstrap TLS. If both are not set or they cannot successfully be used to communicate with the server then an error is thrown.

This has nothing to do with auth at all and it is bad news if you put TLS client certificates in cluster-info

Sorry, that was not what I meant. I meant that you could make a TLS-Bootstrap-only client cert (instead of a token) in your KubeConfig file that will be used for TLS bootstrapping i.e. in the second phase of what kubeadm does. I just said that I think this is possible as an answer to:

I think you are wrong wrt 2. The TLS bootstrap dance only works with a token.

--client-tls-certificate and --client-tls-key Instead of doing TLS bootstrapping with a token, read the client certificate and key from these files. Install them in the appropriate directories. Verify that they can be used to talk to the server. Note that the flag names mirror the kubectl flags somewhat

I agree those flags would be really cool.

I think kubeadm token create [token] [flags] is much more UX-friendly. If [token] isn't specified, it will be autogenerated. Sounds good?

sure

Everything here is UTC time I suppose?

Since TTL is "time from now" UTC doesn't come in to play. This will be converted into an absolute UTC time as it is written in to the token secret.

specify the id or the secret

specify the token id or the full token

Initially, we're only opening get configmap in kube-public to avoid getting opening ourselves to the world too much

There is nothing else in kube-public. The whole idea is that it will be totally public. But, sure.

In kubeadm, this defaults to true

This is for secret itself, not the UX for kubeadm. If a usage is missing from the token secret, users should assume that usage isn't allowed. I'll make it more clear in the text.

Defaults to true in kubeadm

You haven't defined ClusterInfo. I think you just mean the KubeConfig file with the list of addresses to API Servers with the cluster-info ConfigMap exposed and the CA cert(s)

It is defined in the section called "Cluster location information" but isn't as clear as it could be. I'll add more text there and call out the term.

controller-manager

Hmm, I wonder whether we actually should add a well-known description field here?
Don't wanna bloat the spec, but as we grow the responsibilities of these tokens; we might for example want to allocate a token for the scheduler and the controller-manager (kubernetes/kubeadm#172) respectively, and then it would probably be helpful with a human-friendly message that states "this token is for the scheduler" and if the user goes and removes it, the scheduler will lose access to the apiserver.

In any case, when you have 15 different tokens with different TTLs and usages and a new person comes and looks at it, he/she will have no idea where they are used and what they are used for. Adding a generic description field to this spec and to kubeadm token list will clarify things a lot. It also aligns with the rest of k8s where the metadata field is constantly present. WDYT?

I'm in favor of generic free form description fields. I'm happy to add it. It is a bit of an overload -- this could go in an annotation or it could go in the of the secret.

I just think it would be really helpful for kubeadm token list to have a DESCRIPTION column.
I'm open to using a metadata field on the Secret as well, but it feels like we're better of being consistent with the rest of the data

I don't want to bloat RBAC into this spec either, but would it make sense to add a --cluster-role to this command?
This could essentially be a shorthand for

kubectl create clusterrolebinding bootstrap-token-(token-id) --cluster-role (cluster-role) --user system:bootstrap:(token-id)

that would be applied in the same go if this arg is specified. This way the user could easily attach an extra role...

Well, feel free to ignore this, it was just a crazy idea that sprung to mind :)

I don't think we want to encourage people to use these tokens outside of just bootstrapping. We can always add it later if we find another bootstrap-ish use for it, but, for now, I'd like to keep these scoped.

I'm ok with that

I think it might make sense to output the Secret name in the first column here actually.
Otherwise you'll have a very hard time knowing which token matches which Secret without iterating all of them

I'd like to avoid that. Most consumers will be unaware that the tokens are implemented as kube-system secrets under the covers. It will be confusing and break the mental model for users at this point.

Instead, we could warn loudly if we have duplicates and list all the secret names. Perhaps with instructions on how to use kubectl to delete them all and recreate the secret.

It will be confusing and break the mental model for users at this point.

Fair point

I'm ok with that

I just think it would be really helpful for kubeadm token list to have a DESCRIPTION column.
I'm open to using a metadata field on the Secret as well, but it feels like we're better of being consistent with the rest of the data