Skip to content

Commit

Permalink
code review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
atoulme committed Jan 17, 2024
1 parent b36d129 commit 4673ff4
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 9 deletions.
7 changes: 6 additions & 1 deletion config/configtls/configtls.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package configtls // import "go.opentelemetry.io/collector/config/configtls"
import (
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"os"
"path/filepath"
Expand Down Expand Up @@ -197,6 +198,7 @@ func (c TLSSetting) loadTLSConfig() (*tls.Config, error) {

func convertCipherSuites(cipherSuites []string) ([]uint16, error) {
var result []uint16
var errs []error
for _, suite := range cipherSuites {
found := false
for _, supported := range tls.CipherSuites() {
Expand All @@ -207,9 +209,12 @@ func convertCipherSuites(cipherSuites []string) ([]uint16, error) {
}
}
if !found {
return nil, fmt.Errorf("invalid TLS cipher suite: %q", suite)
errs = append(errs, fmt.Errorf("invalid TLS cipher suite: %q", suite))
}
}
if len(errs) != 0 {
return nil, errors.Join(errs...)
}
return result, nil
}

Expand Down
21 changes: 13 additions & 8 deletions config/configtls/configtls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package configtls
import (
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"io"
"os"
Expand Down Expand Up @@ -633,37 +632,43 @@ func TestCipherSuites(t *testing.T) {
tests := []struct {
name string
tlsSetting TLSSetting
wantErr error
wantErr string
result []uint16
}{
{
name: "no suites set",
tlsSetting: TLSSetting{},
wantErr: nil,
result: nil,
},
{
name: "one cipher suite set",
tlsSetting: TLSSetting{
CipherSuites: []string{"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA"},
},
wantErr: nil,
result: []uint16{tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA},
result: []uint16{tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA},
},
{
name: "invalid cipher suite set",
tlsSetting: TLSSetting{
CipherSuites: []string{"FOO"},
},
wantErr: errors.New(`invalid TLS cipher suite: "FOO"`),
wantErr: `invalid TLS cipher suite: "FOO"`,
},
{
name: "multiple invalid cipher suites set",
tlsSetting: TLSSetting{
CipherSuites: []string{"FOO", "BAR"},
},
wantErr: `invalid TLS cipher suite: "FOO"
invalid TLS cipher suite: "BAR"`,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
config, err := test.tlsSetting.loadTLSConfig()
if test.wantErr != nil {
assert.Equal(t, test.wantErr, err)
if test.wantErr != "" {
assert.EqualError(t, err, test.wantErr)
} else {
assert.NoError(t, err)
assert.Equal(t, test.result, config.CipherSuites)
Expand Down

0 comments on commit 4673ff4

Please sign in to comment.