Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Salt minion ID longer than 52 characters ends up with bad kubeconfig being generated #3431

Open
alexandre-allard opened this issue Jun 29, 2021 · 2 comments
Labels
complexity:easy Something that requires less than a day to fix kind:bug Something isn't working

Comments

@alexandre-allard
Copy link
Contributor

alexandre-allard commented Jun 29, 2021

Component: salt

What happened:
@JBWatenbergScality created a VM with MetalK8s 2.10 (but should be reproducible on all versions) with a very long name metalk8s-single-module-federation-bootstrap.novalocal (53 characters).
The hostname is by default used as the Salt minion ID.
This ID is then used when generating the salt-master kubeconfig, using it as the CN by appending the salt-master- prefix to it and in this specific case, making it 65 characters long.
Since this is not considered as a valid CN, kube-apiserver then rejects any authentication from the salt-master, hence the bootstrap fails throwing some Python traceback with HTTP 401 error codes.

What was expected:
Either the bootstrap go to the end, either a clear error message stating that the salt-minion ID is too long early during the bootstrap process.

Steps to reproduce:
Create a VM with a name of 53+ characters or just set the salt-minion ID prior to launch the bootstrap:

mkdir /etc/salt
echo "my-super-long-minion-id-that-puts-metalk8s-bootstrap-in-fire" > /etc/salt/minion_id

Resolution proposal (optional):
We can either add a pre-check to ensure the salt-minion ID is shorter than 53 characters and bail out early if not, or we could truncate the name to fit in the 64 allowed characters (to be checked if it does not create any other issue).

@alexandre-allard alexandre-allard added kind:bug Something isn't working complexity:easy Something that requires less than a day to fix labels Jun 29, 2021
@NicolasT
Copy link
Contributor

NicolasT commented Jun 29, 2021

Strictly speaking, the minion ID is not too long. The CN we generate is. Hence, given said CN is not used for host identification, can't we truncate it, somewhat under the assumption variable parts of a hostname/FQDN come 'early' rather than at its end? Basically

In [1]: "salt-master-my-super-long-minion-id-that-puts-metalk8s-bootstrap-in-fire"[:64]                                                                                                                                                                                                                                       
Out[1]: 'salt-master-my-super-long-minion-id-that-puts-metalk8s-bootstrap'

@alexandre-allard
Copy link
Contributor Author

alexandre-allard commented Jun 29, 2021

Totally agree.
We can't be sure for the variable part of the name (we could check it when adding a node to the cluster)... but if you have a cluster with the firsts 52 chars shared by some machines, you likely need to review your naming convention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity:easy Something that requires less than a day to fix kind:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants