Skip to content

Commit db78445

Browse files
committed
refactor(grpc): simplify CipherSuites to a comma-separated string
Change CipherSuites from []string to string, eliminating the split-then-join round-trip between ApplyTLSFlags and validateCipherSuites. The string is passed directly to pkgtls.ParseCipherSuites without intermediate conversions. Signed-off-by: Jia Zhu <jiazhu@redhat.com> Signed-off-by: zhujian <jiazhu@redhat.com>
1 parent bb0e27b commit db78445

2 files changed

Lines changed: 15 additions & 26 deletions

File tree

pkg/server/grpc/options.go

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"fmt"
66
"math"
77
"os"
8-
"strings"
98
"time"
109

1110
"github.com/spf13/pflag"
@@ -21,7 +20,7 @@ type GRPCServerOptions struct {
2120
ClientCAFile string `json:"client_ca_file" yaml:"client_ca_file"`
2221
TLSMinVersion uint16 `json:"tls_min_version" yaml:"tls_min_version"`
2322
TLSMaxVersion uint16 `json:"tls_max_version" yaml:"tls_max_version"`
24-
CipherSuites []string `json:"cipher_suites" yaml:"cipher_suites"`
23+
CipherSuites string `json:"cipher_suites" yaml:"cipher_suites"`
2524
ServerBindPort string `json:"server_bind_port" yaml:"server_bind_port"`
2625
MaxConcurrentStreams uint32 `json:"max_concurrent_streams" yaml:"max_concurrent_streams"`
2726
MaxReceiveMessageSize int `json:"max_receive_message_size" yaml:"max_receive_message_size"`
@@ -118,7 +117,7 @@ func (o *GRPCServerOptions) Validate() error {
118117
return fmt.Errorf("cert_watch_interval (%v) must be greater than 30 seconds", o.CertWatchInterval)
119118
}
120119

121-
return o.validateCipherSuites()
120+
return o.parseCipherSuiteIDs()
122121
}
123122

124123
// ApplyTLSFlags overrides TLS settings loaded from the config file with values
@@ -133,22 +132,18 @@ func (o *GRPCServerOptions) ApplyTLSFlags(minVersion, cipherSuites string) error
133132
o.TLSMinVersion = ver
134133
}
135134
if cipherSuites != "" {
136-
o.CipherSuites = strings.Split(cipherSuites, ",")
137-
for i := range o.CipherSuites {
138-
o.CipherSuites[i] = strings.TrimSpace(o.CipherSuites[i])
139-
}
135+
o.CipherSuites = cipherSuites
140136
}
141137
return o.Validate()
142138
}
143139

144-
// validateCipherSuites parses CipherSuites IANA names into uint16 IDs
140+
// parseCipherSuiteIDs converts the CipherSuites IANA names into uint16 IDs
145141
// using the shared pkg/tls parsing utilities.
146-
func (o *GRPCServerOptions) validateCipherSuites() error {
147-
if len(o.CipherSuites) == 0 {
142+
func (o *GRPCServerOptions) parseCipherSuiteIDs() error {
143+
if o.CipherSuites == "" {
148144
return nil
149145
}
150-
cipherString := strings.Join(o.CipherSuites, ",")
151-
ids, unsupported := pkgtls.ParseCipherSuites(cipherString)
146+
ids, unsupported := pkgtls.ParseCipherSuites(o.CipherSuites)
152147
if len(unsupported) > 0 {
153148
return fmt.Errorf("unrecognized cipher suite: %s", unsupported[0])
154149
}

pkg/server/grpc/options_test.go

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ func TestApplyTLSFlags(t *testing.T) {
248248
expectErr bool
249249
errorContains string
250250
expectedMinVer uint16
251-
expectedCiphers []string
251+
expectedCipherCount int
252252
}{
253253
{
254254
name: "valid min version override",
@@ -264,7 +264,7 @@ func TestApplyTLSFlags(t *testing.T) {
264264
name: "valid cipher suite override",
265265
cipherSuites: "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
266266
expectedMinVer: tls.VersionTLS12,
267-
expectedCiphers: []string{"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256"},
267+
expectedCipherCount: 2,
268268
},
269269
{
270270
name: "invalid min version",
@@ -283,7 +283,7 @@ func TestApplyTLSFlags(t *testing.T) {
283283
minVersion: "VersionTLS12",
284284
cipherSuites: "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
285285
expectedMinVer: tls.VersionTLS12,
286-
expectedCiphers: []string{"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"},
286+
expectedCipherCount: 1,
287287
},
288288
{
289289
name: "empty strings are no-ops",
@@ -296,7 +296,7 @@ func TestApplyTLSFlags(t *testing.T) {
296296
minVersion: "VersionTLS12",
297297
cipherSuites: "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
298298
expectedMinVer: tls.VersionTLS12,
299-
expectedCiphers: []string{"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384"},
299+
expectedCipherCount: 1,
300300
},
301301
}
302302

@@ -324,12 +324,9 @@ func TestApplyTLSFlags(t *testing.T) {
324324
t.Errorf("expected TLSMinVersion %d, got %d", tt.expectedMinVer, opts.TLSMinVersion)
325325
}
326326

327-
if len(tt.expectedCiphers) > 0 {
328-
if len(opts.CipherSuites) != len(tt.expectedCiphers) {
329-
t.Errorf("expected %d cipher suites, got %d", len(tt.expectedCiphers), len(opts.CipherSuites))
330-
}
331-
if len(opts.cipherSuiteIDs) != len(tt.expectedCiphers) {
332-
t.Errorf("expected %d parsed cipher IDs, got %d", len(tt.expectedCiphers), len(opts.cipherSuiteIDs))
327+
if tt.expectedCipherCount > 0 {
328+
if len(opts.cipherSuiteIDs) != tt.expectedCipherCount {
329+
t.Errorf("expected %d parsed cipher IDs, got %d", tt.expectedCipherCount, len(opts.cipherSuiteIDs))
333330
}
334331
}
335332
})
@@ -340,7 +337,7 @@ func TestApplyTLSFlags_OverridesConfigFile(t *testing.T) {
340337
opts := NewGRPCServerOptions()
341338
// Simulate config file values
342339
opts.TLSMinVersion = tls.VersionTLS12
343-
opts.CipherSuites = []string{"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"}
340+
opts.CipherSuites = "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"
344341

345342
// Flags override
346343
err := opts.ApplyTLSFlags("VersionTLS13",
@@ -352,9 +349,6 @@ func TestApplyTLSFlags_OverridesConfigFile(t *testing.T) {
352349
if opts.TLSMinVersion != tls.VersionTLS13 {
353350
t.Errorf("expected TLSMinVersion TLS 1.3, got %d", opts.TLSMinVersion)
354351
}
355-
if len(opts.CipherSuites) != 2 {
356-
t.Errorf("expected 2 cipher suites, got %d", len(opts.CipherSuites))
357-
}
358352
if len(opts.cipherSuiteIDs) != 2 {
359353
t.Errorf("expected 2 parsed cipher IDs, got %d", len(opts.cipherSuiteIDs))
360354
}

0 commit comments

Comments
 (0)