-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Unrevert tls tests with fixes #30536
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: main
Are you sure you want to change the base?
Conversation
…)" This reverts commit 10dab7a.
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
/payload-job periodic-ci-openshift-release-master-nightly-4.21-e2e-metal-ipi-ovn-bm |
|
@wangke19: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/be3adcd0-ca92-11f0-97ec-df7a13906fc6-0 |
|
/payload-job periodic-ci-openshift-release-master-nightly-4.21-e2e-metal-ipi-ovn-bm |
|
@wangke19: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/7f1cfd90-caf5-11f0-9972-e7706940b560-0 |
|
/payload-job periodic-ci-openshift-release-master-nightly-4.21-e2e-agent-ha-dualstack-conformance |
|
@wangke19: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/92ceccb0-caf5-11f0-9c4d-d12e7d9d0fa5-0 |
|
/payload-job periodic-ci-openshift-release-master-nightly-4.21-e2e-metal-ipi-ovn-upgrade |
|
@wangke19: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/ab7846b0-caf5-11f0-9605-34cb18c668c9-0 |
|
/payload-job periodic-ci-openshift-release-master-nightly-4.21-e2e-agent-ha-dualstack-conformance |
|
@wangke19: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/6ba60730-cb3a-11f0-8d31-f7a85637c458-0 |
|
Scheduling required tests: |
102a071 to
20e06a3
Compare
|
/payload-job periodic-ci-openshift-release-master-nightly-4.21-e2e-metal-ipi-ovn-bm |
|
@wangke19: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/1ab2d0e0-cb64-11f0-8005-06e842255afc-0 |
|
/payload-job periodic-ci-openshift-release-master-nightly-4.21-e2e-agent-ha-dualstack-conformance |
|
@wangke19: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/25e247c0-cb64-11f0-9b6a-fc6039487e5f-0 |
|
/payload-job periodic-ci-openshift-release-master-nightly-4.21-e2e-metal-ipi-ovn-upgrade |
|
@wangke19: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/2ffb6ca0-cb64-11f0-87e6-5149162538b8-0 |
|
Scheduling required tests: |
20e06a3 to
415e7eb
Compare
|
/payload-job periodic-ci-openshift-release-master-nightly-4.21-e2e-metal-ipi-ovn-bm |
|
@wangke19: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/d89f6d30-cc67-11f0-9540-2d1926f59e52-0 |
|
/payload-job periodic-ci-openshift-release-master-nightly-4.21-e2e-rosa-sts-ovn |
|
@wangke19: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/4a609180-cf63-11f0-95ea-e884c70dd3df-0 |
|
CI job periodic-ci-openshift-release-master-nightly-4.21-e2e-rosa-sts-ovn depends on openshift/release#72041 |
|
Regarding bug 1: Where is this /lgtm |
|
The failures of CI job periodic-ci-openshift-release-master-nightly-4.21-e2e-rosa-sts-ovn has been fixed in openshift/release#72041. |
Update the comment explaining why cipher suite tests are constrained to TLS 1.2 to be more technically accurate. The previous comment suggested this was about "Go 1.23+ behavior", but the real issue is fundamental to how TLS 1.3 works: - The intermediate profile allows both TLS 1.2 and TLS 1.3 - Clients negotiate TLS 1.3 when MaxVersion is unspecified and server supports it - TLS 1.3 spec predefines cipher suites and doesn't support configuration - Therefore, specifying any cipher suite has no effect with TLS 1.3 - Forcing TLS 1.2 allows actual testing of cipher suite restrictions This makes the reasoning clearer for future maintainers.
|
/payload-job periodic-ci-openshift-release-master-nightly-4.21-e2e-rosa-sts-ovn |
|
@wangke19: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/9cfb89b0-d010-11f0-9fbd-4a66c14e5851-0 |
|
Scheduling required tests: |
|
/test e2e-metal-ipi-ovn-ipv6 |
|
/test e2e-aws-ovn-serial-1of2 |
|
/test e2e-aws-ovn-serial-2of2 |
|
/test e2e-metal-ipi-ovn-ipv6 |
|
Job Failure Risk Analysis for sha: 4402603
|
1 similar comment
|
Job Failure Risk Analysis for sha: 4402603
|
|
/retest |
|
/retest-required |
1 similar comment
|
/retest-required |
|
Job Failure Risk Analysis for sha: 4402603
|
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jacobsee, wangke19 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 |
|
/retest-required |
|
@wangke19: all tests passed! Full PR test history. Your PR dashboard. 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. |
### What Was Fixed:
This PR unrereverts the TLS 1.3 / Modern profile tests that were previously reverted in PR #30533 due to test failures. The original implementation was in PR #29611 for OCPBUGS-64799.
After investigation, the test failures were caused by two distinct bugs in the TLS tests:
Bug 1: TestTLSDefaults used direct connection instead of port-forwarding
Problem: The test attempted to connect directly to the external API server hostname from the kubeconfig (e.g.,
api.cluster5.ocpci.eng.rdu2.redhat.com). When running as a pod in the cluster (CI environment), the pod's internal DNS cannot resolve this external hostname, resulting in:Fix: Updated
TestTLSDefaultsto use the sameforwardPortAndExecute()approach asTestTLSMinimumVersions, which creates anoc port-forwardtunnel to the apiserver service. This approach:TestTLSMinimumVersionspatternBug 2: TLS 1.3 doesn't support cipher suite configuration
Problem: The intermediate TLS profile allows both TLS 1.2 and TLS 1.3. When the client doesn't specify
MaxVersion, it negotiates TLS 1.3 if the server supports it. TLS 1.3 does not support configuring cipher suites (they're predetermined by the spec), so specifying any cipher suite (RC4 or modern ciphers) has no effect. This caused the cipher suite validation test to incorrectly succeed when connecting with deprecated ciphers that should have been rejected.Example observed behavior:
TLS_ECDHE_ECDSA_WITH_RC4_128_SHA(0xC007)TLS_AES_128_GCM_SHA256(0x1301)Fix: Constrain the cipher test to TLS 1.2 only to ensure cipher suite restrictions are actually tested:
Additional fixes:
err := conn.Close()shadowed the outererrfromtls.Dial(), making the test check the wrong errordialErrandcloseErrfor clarityIPv6 Support
Removed the IPv4-only restriction to enable tests on IPv6 clusters:
The tests use port-forwarding to
localhost, which resolves appropriately in both IPv4 (127.0.0.1) and IPv6 (::1) environments.Testing
Tested against live OpenShift cluster with both single-stack IPv4 and dual-stack IPv6 configurations:
Related Issues
References