Add spec.api.bindAddress configuration #1038
Conversation
|
I'd prefer to use separate |
gakio12
changed the title
|
@gakio12 seems there's some unit test failures |
gakio12
force-pushed
the
main
branch
2 times, most recently
from
7f2b8a2
to
bff1d0e
Compare
|
Hey @gakio12 please rebase with the current code updates. Thanks! |
gakio12
force-pushed
the
main
branch
2 times, most recently
from
65d7ef4
to
4bbf01f
Compare
|
The PR is marked as stale since no activity has been recorded in 30 days |
|
@jnummelin Is there anything more I need to do on this PR? I want to make sure I am not missing anything. |
|
|
||
| func (a *APISpec) getAPIServerAddress(address string) string { | ||
| switch address { | ||
| case "0.0.0.0": |
There was a problem hiding this comment.
this is logically wrong, "0.0.0.0" means we want to listen on all interfaces available, while "localhost" means listening only on loopback iface. What it the logic behind that decision?
There was a problem hiding this comment.
The reason it is "0.0.0.0" here is because without specifying an interface (as the current version of k0s does), the kube-apiserver listens on 0.0.0.0. So the default is "0.0.0.0" to allow the functionality to remain the same as previous versions.
The reason we return "localhost" in getAPIServerAddress follows the same reason as the above, k0s uses a hardcoded "localhost" in the kubeconfig and other certs for the kube-apiserver in the current released version.
With this change, it keeps the default functionality of k0s, while allowing someone to change the bind address and still have the certs and kubeconfig be correct (a kubeconfig pointing at "localhost" when the kube-apiserver is listening on 192.168.1.1 doesn't work).
I'll add that logically, if a server is listening on 0.0.0.0, "localhost" would be an accurate response, since you can't connect to 0.0.0.0 directly. With that said, I can see how getAPIServerAddress can be confused with a getAPIServerBindAddress, whereas the former is the address used to connect to the APIServer, not the address it is binding to.
There was a problem hiding this comment.
since you can't connect to 0.0.0.0 directly
well, it turns out that one actually can do that as in many (most?) languages connecting to 0.0.0.0 actually means connecting to localhost. Now that this address is used for kube components which are all Golang, I think it should work if they connect to 0.0.0.0.
For reference: https://cs.opensource.google/go/go/+/refs/tags/go1.17.3:src/net/ipsock_posix.go;l=137-143
With this in mind, everything should work if we use the bind address as the address for other components to connect to the API, right?
There was a problem hiding this comment.
Huh, I was not aware of that. That is good to know! Since this string is used for health checks and a kubeconfig, an easy test is to take an existing cluster and change the kubeconfig to point to 0.0.0.0. The certificate will have to be signed with 0.0.0.0 as well, since "localhost" resolves to 127.0.0.1. Just doing that quick test I outlined, I got the expected certificate error.
I'll do a bit more testing on this and see if this logic can be removed while maintaining how k0s currently functions.
There was a problem hiding this comment.
@gakio12 hello! do you have any updates on the issue?
There was a problem hiding this comment.
@Soider Apologies for the delay on this, I am expecting to be able to work on this in the next few weeks. I have the changes in place locally, but I have lost my test environment the last two months, so was unable to test (apart from time constraints).
There was a problem hiding this comment.
My 2 cents here: the actual meaning of "0.0.0.0" depends on how it is used. When used to bind, it means all (or any) interfaces, but when used as a client, it means "this" host (or even more strongly, an "unspecified" address, and as pointed above, is interpreted to mean "localhost" or the loopback interface, at least in various cases (including the apiserver self client)
So IMHO, although the address does seem to work (albeit with a failure as it's not in the cert SAN), it seems better to me to keep the old behavior and use "localhost" in this case.
|
@gakio12 Just curious, are you still planning to work on this? I'd need this to make my setup work as well, so was simply wondering what your plans were to decide on whether to wait. (Among other things, this would then need an addition to k0sctl to allow setting the BindAddress per controller etc.) |
|
@ocraviotto Yes, I am planning to commit the changes proposed in the next few weeks now that I have gained some free time (and got my environment back). |
gakio12
force-pushed
the
main
branch
2 times, most recently
from
2187ee5
to
e917ae6
Compare
|
The PR is marked as stale since no activity has been recorded in 30 days |
|
The PR is marked as stale since no activity has been recorded in 30 days |
|
The PR is marked as stale since no activity has been recorded in 30 days |
|
The PR is marked as stale since no activity has been recorded in 30 days |
|
The PR is marked as stale since no activity has been recorded in 30 days |
|
The PR is marked as stale since no activity has been recorded in 30 days |
|
This has been open for quite a while. |
|
The PR is marked as stale since no activity has been recorded in 30 days |
|
The PR is marked as stale since no activity has been recorded in 30 days |
|
The PR is marked as stale since no activity has been recorded in 30 days |
|
This pull request has merge conflicts that need to be resolved. |
Should a new
bindAddressoption be created and change documentation for the existingaddressoption, or should the existingaddressoption's behavior be changed to match the existing documentation?The later I can see causing some issues with existing setups,
bindAddressis probably the safest route. Currently the default forbindAddressis "0.0.0.0", which is the default kube-api-server uses if the--bind-addressflag isn't used.Issue
Fixes #957
What this PR Includes
Adds the ability to define a
bindAddressfor the kube-api-server.