Use TCP so that we can support different ciphers easily for v1alpha3. #1658
Conversation
we can revisit the value of SSL health checks (which are less spammy) in the future, but i think the modularity requirements people currently have mean that we're better off supporting TCP for now (lest we potentially rule out certain APISErver configurations)
k8s-ci-robot
added
cncf-cla: yes
|
Hi @jayunit100. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jayunit100 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 |
k8s-ci-robot
added
the
size/XS
| @@ -255,7 +255,7 @@ func (s *Service) getAPIServerClassicELBSpec() (*infrav1.ClassicELB, error) { | |||
| }, | |||
| }, | |||
| HealthCheck: &infrav1.ClassicELBHealthCheck{ | |||
| Target: fmt.Sprintf("%v:%d", infrav1.ClassicELBProtocolSSL, 6443), | |||
There was a problem hiding this comment.
I’d rather update the listeners/health checks to use custom order/set of SSL ciphers. If not possible, we might want to compare our list with the ELB one cc @randomvariable
There was a problem hiding this comment.
As far as ELB goes, listeners != health checks in this regards. It is TCP passthrough for the listener. The healthcheck spec just ensures we do a TLS handshake.
There was a problem hiding this comment.
Verified that AWS only supports non-elliptic curve handshake tests for ELBv1.
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
/test pull-cluster-api-provider-aws-e2e |
|
Oops bug ! |
|
/test pull-cluster-api-provider-aws-e2e |
|
/close |
|
@detiber: Closed this PR. In response to this:
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. |
|
@jayunit100: The following test 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/test-infra repository. I understand the commands that are listed here. |
🐛 (:bug:, patch and bugfixes)
What this PR does / why we need it:
USes TCP rather then SSL, so that we can support any arbitrary APISErver Cipher. Otherwise, we need to alignt the ELB Ciphers with those in the APIServer, Scheduler, KCM, Kubelet, and so on.
What issue this fixes
#1657
(undos the work in #1658, which i think was valuable, but we can live without for the timebeing, correct me if im missing something)