-
Notifications
You must be signed in to change notification settings - Fork 1.5k
tls: remove serving capability from deprecated apiserver cert, also fix Complete ClientCA bundle for apiserver #1640
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
Conversation
As we're fixing the kubelet TLS files, we've noticed places that are over-signing and trusting the wrong thing. This updates the next case we can use to ratchet the fixes into the MCO.
pkg/asset/tls/apiserver.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've ratcheted through changes in kas-o, mco, kubelet, kas-o, and now we can fix this. It's never used to serve anymore
pkg/asset/tls/apiserver.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This prevents kas-o from rotating client cert/key pairs.
pkg/asset/tls/apiserver.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sjenning this is the one it is legal to depend upon.
|
This removes usages from a deprecated cert ? |
Yeah, tightening things up so they can't grow back while we're working out full removal. |
|
Not very clear to me, but it's been open for a while, and CI is green, so: /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, wking The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold |
|
Just wanna take a quick look. Will hold cancel asap |
| // Generate generates the cert/key pair based on its dependencies. | ||
| func (a *APIServerCertKey) Generate(dependencies asset.Parents) error { | ||
| kubeCA := &KubeCA{} | ||
| installConfig := &installconfig.InstallConfig{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should also drop the dependency from https://github.com/openshift/installer/pull/1640/files#diff-c7316d3d0bee102140cf53dc3ae5edf9R27
|
#1640 (comment) is non-blocking. /hold cancel |
The kubelet terminates mTLS to determine the identity of the caller. This fixes the combined client bundle that can provide a stepping stone to getting the kubelet restricting properly.
@sjenning as we discussed yesterday
/assign @wking