-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Fixes panic on round tripper when TLS under a proxy #8896
Conversation
[test] |
I don't think this will make things any worse 😉 There needs to be a corresponding upstream pull to Kube and then this needs to turn into an UPSTREAM commit |
de72ffa
to
dcbf9e1
Compare
@ncdc updated to UPSTREAM PR kubernetes/kubernetes#25690. |
dcbf9e1
to
5c8e213
Compare
Evaluated for origin test up to 5c8e213 |
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/3861/) |
LGTM, would like a test. Did you sweep the generic proxy handler to make sure a similar fix is not needed? |
@liggitt @ncdc Unfortunately I don't think this can be properly tested. I'd need a certificate from a trusted authority here, so that I could set |
I actually meant expecting an error rather than a panic |
@liggitt Of course. You have to set |
[merge] |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/5947/) (Image: devenv-rhel7_4238) |
Evaluated for origin merge up to 5c8e213 |
@ncdc PTAL. I'll open upstream if it makes sense.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1336337
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1323666