-
Notifications
You must be signed in to change notification settings - Fork 41.6k
Validate kubelet serving cert CN via host rewrite and DialTLSContext #131450
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Monis Khan <mok@microsoft.com>
Signed-off-by: Monis Khan <mok@microsoft.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: enj The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…ontext Signed-off-by: Monis Khan <mok@microsoft.com>
…ialTLSContext Signed-off-by: Monis Khan <mok@microsoft.com>
…e and DialTLSContext Signed-off-by: Monis Khan <mok@microsoft.com>
… rewrite and DialTLSContext Signed-off-by: Monis Khan <mok@microsoft.com>
…ia host rewrite and DialTLSContext Signed-off-by: Monis Khan <mok@microsoft.com>
|
@enj: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
| // TODO add unit test | ||
| // 1. unique transport per call for same inputs | ||
| // 2. unwrapping still works at high log levels when the transport is actually wrapped | ||
| Proxy: http.ProxyFromEnvironment, |
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.
I don't love relying on this to get a unique transport without a test ... at least unit test this results in two distinct underlying http.Transport objects when set
edit: ha, just noticed you literally have a TODO to add a unit test for this
also, if you want to avoid trying to support HTTP_PROXY / HTTPS_PROXY in combination with this (since egress selector is available), you could set this to a no-op no-proxy function (just return nil always)
also, I think we only need to do this when config.TLSClientConfig.ValidateNodeName is set
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Hello @enj, This PR has not seen updates for about 1.5 months since the last activity in early May, and it currently needs a rebase along with multiple failing tests. I’d like to check what’s the current status or if there’s anything we can do to help move this forward. The code freeze for the 1.34 release starts at 02:00 UTC on Friday, 25th July 2025 (about 3 weeks from now). Please make sure this PR has both /lgtm and /approve labels, and that merge conflicts and test failures are resolved before the code freeze. Thanks! |
|
👋 Hello! If so, a gentle reminder that the code freeze will start at 02:00 UTC Friday 25th July 2025 . Please make sure any PRs have both lgtm and approved labels ASAP, and file an Exception if you think this PR needs additional time. Thanks! |
|
As code freeze has passed, I'm removing this from the 1.34 milestone. /milestone clear |
/kind feature
/triage accepted
/sig auth
/milestone v1.34