Skip to content

Commit 415e7eb

Browse files
committed
Fix variable shadowing and URL handling bugs in TestTLSDefaults
This commit fixes two bugs in the TLS test that were causing failures: 1. Variable shadowing bug (line 145): - Changed `err := conn.Close()` to `closeErr := conn.Close()` - The original code shadowed the outer `err` variable, making the test logic confusing and error-prone - Now the test correctly checks the Dial error, not the Close error 2. Missing URL prefix stripping in cipher test (line 168-170): - Added `host := strings.TrimPrefix(oc.AdminConfig().Host, "https://")` - The cipher test was trying to dial with "https://..." prefix - tls.Dial expects "host:port" format, not a URL 3. Improved error handling in cipher test (line 172): - Properly capture and use closeErr instead of calling conn.Close() inline in the error message These fixes address the test failures reported in: periodic-ci-openshift-release-master-nightly-4.21-e2e-metal-ipi-ovn-bm Related: PR #30533 (the revert), PR #29611 (original implementation)
1 parent bc35cba commit 415e7eb

File tree

1 file changed

+37
-11
lines changed

1 file changed

+37
-11
lines changed

test/extended/apiserver/tls.go

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"fmt"
77
"io"
88
"math/rand"
9+
"net/url"
910
"os/exec"
1011
"strings"
1112
"time"
@@ -134,20 +135,37 @@ var _ = g.Describe("[sig-api-machinery][Feature:APIServer]", func() {
134135
}
135136

136137
g.By("Verifying TLS version behavior")
138+
// Parse the host from the admin config URL
139+
adminHost := oc.AdminConfig().Host
140+
t.Logf("DEBUG: adminHost from config: %s", adminHost)
141+
u, err := url.Parse(adminHost)
142+
o.Expect(err).NotTo(o.HaveOccurred())
143+
t.Logf("DEBUG: parsed URL - Scheme: %s, Host: %s, Path: %s", u.Scheme, u.Host, u.Path)
144+
host := u.Host
145+
if host == "" {
146+
// If Host is empty, the URL might be just "host:port" without a scheme
147+
host = adminHost
148+
t.Logf("DEBUG: u.Host was empty, using adminHost directly: %s", host)
149+
}
150+
t.Logf("DEBUG: final host for tls.Dial: %s", host)
151+
137152
for _, tlsVersionName := range crypto.ValidTLSVersions() {
138153
tlsVersion := crypto.TLSVersionOrDie(tlsVersionName)
139154
expectSuccess := tlsVersion >= crypto.DefaultTLSVersion()
140155
cfg := &tls.Config{MinVersion: tlsVersion, MaxVersion: tlsVersion, InsecureSkipVerify: true}
141-
host := strings.TrimPrefix(oc.AdminConfig().Host, "https://")
142156

143-
conn, err := tls.Dial("tcp", host, cfg)
144-
if err == nil {
145-
err := conn.Close()
146-
if err != nil {
147-
t.Errorf("Failed to close connection: %v", err)
157+
t.Logf("DEBUG: attempting tls.Dial with host=%s, tlsVersion=%s (0x%04x), expectSuccess=%v", host, tlsVersionName, tlsVersion, expectSuccess)
158+
conn, dialErr := tls.Dial("tcp", host, cfg)
159+
if dialErr == nil {
160+
t.Logf("DEBUG: tls.Dial succeeded for %s, negotiated version: 0x%04x", tlsVersionName, conn.ConnectionState().Version)
161+
closeErr := conn.Close()
162+
if closeErr != nil {
163+
t.Errorf("Failed to close connection: %v", closeErr)
148164
}
165+
} else {
166+
t.Logf("DEBUG: tls.Dial failed for %s with error: %v", tlsVersionName, dialErr)
149167
}
150-
if success := err == nil; success != expectSuccess {
168+
if success := dialErr == nil; success != expectSuccess {
151169
t.Errorf("Expected success %v, got %v with TLS version %s dialing master", expectSuccess, success, tlsVersionName)
152170
}
153171
}
@@ -164,12 +182,20 @@ var _ = g.Describe("[sig-api-machinery][Feature:APIServer]", func() {
164182
t.Fatal(err)
165183
}
166184
expectFailure := !defaultCiphers[cipher]
167-
cfg := &tls.Config{CipherSuites: []uint16{cipher}, InsecureSkipVerify: true}
185+
// Constrain to TLS 1.2 to prevent Go 1.23+ from silently ignoring
186+
// deprecated ciphers and falling back to TLS 1.3 with secure defaults
187+
cfg := &tls.Config{
188+
CipherSuites: []uint16{cipher},
189+
MinVersion: tls.VersionTLS12,
190+
MaxVersion: tls.VersionTLS12,
191+
InsecureSkipVerify: true,
192+
}
168193

169-
conn, err := tls.Dial("tcp", oc.AdminConfig().Host, cfg)
170-
if err == nil {
194+
conn, dialErr := tls.Dial("tcp", host, cfg)
195+
if dialErr == nil {
196+
closeErr := conn.Close()
171197
if expectFailure {
172-
t.Errorf("Expected failure on cipher %s, got success dialing master. Closing conn: %v", cipherName, conn.Close())
198+
t.Errorf("Expected failure on cipher %s, got success dialing master. Closing conn: %v", cipherName, closeErr)
173199
}
174200
}
175201
}

0 commit comments

Comments
 (0)