Plumbing the proxy dialer to the webhook admission plugin; Fix the tls config

[caesarxuchao]

• Fixing Plumb the proxy dialer to the webhook admission plugin #49987. Plumb the Dial function to the transport.Config
• Fixing Webhook admission plugin should set ServerName in its tls config #52366. Let the webhook admission plugin sets the TLSConfg.ServerName.

I tested it in my gke setup. I don't have time to implement an e2e test before 1.8 release. I think it's ok to add the test later, because i) the change only affects the alpha webhook admission feature, and ii) the webhook feature is unusable without the fix. That said, it's up to my reviewer to decide.

Filed #52368 for the missing e2e test.

( The second commit is #52372, which is just a cleanup of client configuration in e2e tests. It removed a function that marshalled the client config to json and then unmarshalled it. It is a prerequisite of this PR, because this PR added the Dial function to the config which is not json marshallable.)

Fixed the webhook admission plugin so that it works even if the apiserver and the nodes are in two networks (e.g., in GKE).
Fixed the webhook admission plugin so that webhook author could use the DNS name of the service as the CommonName when generating the server cert for the webhook.

Action required:
Anyone who generated server cert for admission webhooks need to regenerate the cert. Previously, when generating server cert for the admission webhook, the CN value doesn't matter. Now you must set it to the DNS name of the webhook service, i.e., `<service.Name>.<service.Namespace>.svc`.

[caesarxuchao]

Still a WIP, because I don't know what's the proper way to test it.
@mandarjog when you said you could help testing it, you meant you could test it after the code is rolled out in gke, right?

[caesarxuchao]

/release-note-none
/sig api-machinery

[cheftako]

This seems somewhat GKE specific. Can we add apimachinery e2e test for at least one cloud provider which uses the dialer (GKE) and one which does not (GCE)?

[caesarxuchao]

The fix is not working yet.

$ kubectl get endpoints
webhook      10.32.0.5:8000     15m

API server log:

I0912 04:42:13.119439       5 ssh.go:400] [3971e1b2076ea996: 10.32.0.5:8000] Dialing...
W0912 04:42:13.119461       5 ssh.go:424] SSH tunnel not found for address "10.32.0.5", picking random node
I0912 04:42:13.121601       5 ssh.go:402] [3971e1b2076ea996: 10.32.0.5:8000] Dialed in 2.163495ms.
W0912 04:42:13.122779       5 admission.go:181] Failed calling webhook pod-image.k8s.io: failed calling admission webhook "pod-image.k8s.io": Post http://10.32.0.5:8000/: malformed HTTP response "\x15\x03\x01\x00\x02\x02\x16"
E0912 04:42:13.122808       5 admission.go:182] failed calling admission webhook "pod-image.k8s.io": Post http://10.32.0.5:8000/: malformed HTTP response "\x15\x03\x01\x00\x02\x02\x16"

http: TLS handshake error from 10.128.0.3:55796: tls: oversized record received with length 21536

[caesarxuchao]

Reproduced in the unit test.

[caesarxuchao]

Got it working in my local gke setup. I'll try to get together an gke e2e test, but no promise at the moment.

[k8s-ci-robot]

@caesarxuchao: Your pull request title starts with "[WIP]", so the do-not-merge/work-in-progress label will be added.

This label will ensure that your pull request will not be merged. Remove the prefix from your pull request title to trigger the removal of the label and allow for your pull request to be merged.

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/test-infra repository.

[cheftako]

Need to look at the old certs customers may have had.

[cheftako]

1. Is this backward compatible for old clients?
2. Not a big deal but do we really need to do "." + "svc" vs ".svc"

[caesarxuchao]

For 1, It's not backward compatible. Added "Action Required" in the release note.

Will fix 2.

[lavalamp]

I was kinda hoping to combine the service resolver with this proxy transport. But maybe it's not important. hm.

[caesarxuchao]

Do you want them to be combined because both of them are needed iff an admission plugin wants to contact a service provided by the user? I can put them into a single struct if you think it's necessary.

[lavalamp]

/approve

[k8s-github-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: caesarxuchao, cheftako, lavalamp

Associated issue: 49987

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

• cmd/kube-apiserver/OWNERS [caesarxuchao,lavalamp]
• pkg/kubeapiserver/OWNERS [lavalamp]
• plugin/pkg/admission/OWNERS [lavalamp]
• staging/src/k8s.io/client-go/OWNERS [caesarxuchao,lavalamp]
• test/OWNERS [lavalamp]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

[caesarxuchao]

Rebased and squashed. Adding back the label.

[caesarxuchao]

/retest

[caesarxuchao]

/retest

[caesarxuchao]

/retest

[caesarxuchao]

/retest

[k8s-ci-robot]

@caesarxuchao: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-bazel	5ac3afe	link	/test pull-kubernetes-bazel
pull-kubernetes-e2e-gce-bazel	856a1db	link	/test pull-kubernetes-e2e-gce-bazel

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/test-infra repository. I understand the commands that are listed here.

[k8s-github-robot]

Automatic merge from submit-queue (batch tested with PRs 51824, 50476, 52451, 52009, 52237)

[prabushyam]

@caesarxuchao: Could we cherry pick this into 1.7 ?