Skip to content

Commit 4fdb366

Browse files
mastersingh24mastersingh24
committed
Flatten comm.ServerConfig
ServerConfig had pointers for SecureOptions and KeepAliveOptions which really should just be values. FAB-15589 #done Change-Id: Ic1f8e38e0ee1f486254937eea8c7c324b8c5be49 Signed-off-by: Gari Singh <gari.r.singh@gmail.com>
1 parent 18111fd commit 4fdb366

38 files changed

+182
-186
lines changed

‎cmd/common/comm/client_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222

2323
func TestTLSClient(t *testing.T) {
2424
srv, err := comm.NewGRPCServer("127.0.0.1:", comm.ServerConfig{
25-
SecOpts: &comm.SecureOptions{
25+
SecOpts: comm.SecureOptions{
2626
UseTLS: true,
2727
Key: loadFileOrDie(filepath.Join("testdata", "server", "key.pem")),
2828
Certificate: loadFileOrDie(filepath.Join("testdata", "server", "cert.pem")),
@@ -57,7 +57,7 @@ func TestDialBadEndpoint(t *testing.T) {
5757

5858
func TestNonTLSClient(t *testing.T) {
5959
srv, err := comm.NewGRPCServer("127.0.0.1:", comm.ServerConfig{
60-
SecOpts: &comm.SecureOptions{},
60+
SecOpts: comm.SecureOptions{},
6161
})
6262
assert.NoError(t, err)
6363
go srv.Start()

‎cmd/common/comm/config.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,33 +28,33 @@ type Config struct {
2828
// ToSecureOptions converts this Config to SecureOptions.
2929
// The given function generates a self signed client TLS certificate if
3030
// the TLS certificate and key aren't present at the config
31-
func (conf Config) ToSecureOptions(newSelfSignedTLSCert genTLSCertFunc) (*comm.SecureOptions, error) {
31+
func (conf Config) ToSecureOptions(newSelfSignedTLSCert genTLSCertFunc) (comm.SecureOptions, error) {
3232
if conf.PeerCACertPath == "" {
33-
return &comm.SecureOptions{}, nil
33+
return comm.SecureOptions{}, nil
3434
}
3535
caBytes, err := loadFile(conf.PeerCACertPath)
3636
if err != nil {
37-
return nil, errors.WithStack(err)
37+
return comm.SecureOptions{}, errors.WithStack(err)
3838
}
3939
var keyBytes, certBytes []byte
4040
// If TLS key and certificate aren't given, generate a self signed one on the fly
4141
if conf.KeyPath == "" && conf.CertPath == "" {
4242
tlsCert, err := newSelfSignedTLSCert()
4343
if err != nil {
44-
return nil, err
44+
return comm.SecureOptions{}, err
4545
}
4646
keyBytes, certBytes = tlsCert.Key, tlsCert.Cert
4747
} else {
4848
keyBytes, err = loadFile(conf.KeyPath)
4949
if err != nil {
50-
return nil, errors.WithStack(err)
50+
return comm.SecureOptions{}, errors.WithStack(err)
5151
}
5252
certBytes, err = loadFile(conf.CertPath)
5353
if err != nil {
54-
return nil, errors.WithStack(err)
54+
return comm.SecureOptions{}, errors.WithStack(err)
5555
}
5656
}
57-
return &comm.SecureOptions{
57+
return comm.SecureOptions{
5858
Key: keyBytes,
5959
Certificate: certBytes,
6060
UseTLS: true,

‎core/chaincode/shim/shim.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ func StartInProc(env []string, args []string, cc Chaincode, recv <-chan *pb.Chai
114114
func newPeerClientConnection(address string) (*grpc.ClientConn, error) {
115115

116116
// set the keepalive options to match static settings for chaincode server
117-
kaOpts := &comm.KeepaliveOptions{
117+
kaOpts := comm.KeepaliveOptions{
118118
ClientInterval: time.Duration(1) * time.Minute,
119119
ClientTimeout: time.Duration(20) * time.Second,
120120
}
@@ -135,37 +135,37 @@ func newPeerClientConnection(address string) (*grpc.ClientConn, error) {
135135
return client.NewConnection(address, "")
136136
}
137137

138-
func secureOptions() (*comm.SecureOptions, error) {
138+
func secureOptions() (comm.SecureOptions, error) {
139139

140140
tlsEnabled, err := strconv.ParseBool(os.Getenv("CORE_PEER_TLS_ENABLED"))
141141
if err != nil {
142-
return nil, errors.WithMessage(
142+
return comm.SecureOptions{}, errors.WithMessage(
143143
err,
144144
"'CORE_PEER_TLS_ENABLED' must be set to 'true' or 'false'",
145145
)
146146
}
147147
if tlsEnabled {
148148
data, err := ioutil.ReadFile(os.Getenv("CORE_TLS_CLIENT_KEY_PATH"))
149149
if err != nil {
150-
return nil, errors.WithMessage(err, "failed to read private key file")
150+
return comm.SecureOptions{}, errors.WithMessage(err, "failed to read private key file")
151151
}
152152
key, err := base64.StdEncoding.DecodeString(string(data))
153153
if err != nil {
154-
return nil, errors.WithMessage(err, "failed to decode private key file")
154+
return comm.SecureOptions{}, errors.WithMessage(err, "failed to decode private key file")
155155
}
156156
data, err = ioutil.ReadFile(os.Getenv("CORE_TLS_CLIENT_CERT_PATH"))
157157
if err != nil {
158-
return nil, errors.WithMessage(err, "failed to read public key file")
158+
return comm.SecureOptions{}, errors.WithMessage(err, "failed to read public key file")
159159
}
160160
cert, err := base64.StdEncoding.DecodeString(string(data))
161161
if err != nil {
162-
return nil, errors.WithMessage(err, "failed to decode public key file")
162+
return comm.SecureOptions{}, errors.WithMessage(err, "failed to decode public key file")
163163
}
164164
root, err := ioutil.ReadFile(os.Getenv("CORE_PEER_TLS_ROOTCERT_FILE"))
165165
if err != nil {
166-
return nil, errors.WithMessage(err, "failed to read root cert file")
166+
return comm.SecureOptions{}, errors.WithMessage(err, "failed to read root cert file")
167167
}
168-
return &comm.SecureOptions{
168+
return comm.SecureOptions{
169169
UseTLS: true,
170170
Certificate: []byte(cert),
171171
Key: []byte(key),
@@ -174,7 +174,7 @@ func secureOptions() (*comm.SecureOptions, error) {
174174
},
175175
nil
176176
}
177-
return &comm.SecureOptions{}, nil
177+
return comm.SecureOptions{}, nil
178178
}
179179

180180
func chatWithPeer(chaincodename string, stream PeerChaincodeStream, cc Chaincode) error {

‎core/comm/client.go

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -43,18 +43,12 @@ func NewGRPCClient(config ClientConfig) (*GRPCClient, error) {
4343
}
4444

4545
// keepalive options
46-
var kap keepalive.ClientParameters
47-
if config.KaOpts != nil {
48-
kap = keepalive.ClientParameters{
49-
Time: config.KaOpts.ClientInterval,
50-
Timeout: config.KaOpts.ClientTimeout}
51-
} else {
52-
// use defaults
53-
kap = keepalive.ClientParameters{
54-
Time: DefaultKeepaliveOptions.ClientInterval,
55-
Timeout: DefaultKeepaliveOptions.ClientTimeout}
46+
47+
kap := keepalive.ClientParameters{
48+
Time: config.KaOpts.ClientInterval,
49+
Timeout: config.KaOpts.ClientTimeout,
50+
PermitWithoutStream: true,
5651
}
57-
kap.PermitWithoutStream = true
5852
// set keepalive
5953
client.dialOpts = append(client.dialOpts, grpc.WithKeepaliveParams(kap))
6054
// Unless asynchronous connect is set, make connection establishment blocking.
@@ -70,11 +64,12 @@ func NewGRPCClient(config ClientConfig) (*GRPCClient, error) {
7064
return client, nil
7165
}
7266

73-
func (client *GRPCClient) parseSecureOptions(opts *SecureOptions) error {
74-
75-
if opts == nil || !opts.UseTLS {
67+
func (client *GRPCClient) parseSecureOptions(opts SecureOptions) error {
68+
// if TLS is not enabled, return
69+
if !opts.UseTLS {
7670
return nil
7771
}
72+
7873
client.tlsConfig = &tls.Config{
7974
VerifyPeerCertificate: opts.VerifyCertificate,
8075
MinVersion: tls.VersionTLS12} // TLS 1.2 only

‎core/comm/client_test.go

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func TestNewGRPCClient_GoodConfig(t *testing.T) {
4747
assert.False(t, client.TLSEnabled())
4848
assert.False(t, client.MutualTLSRequired())
4949

50-
secOpts := &comm.SecureOptions{
50+
secOpts := comm.SecureOptions{
5151
UseTLS: false,
5252
}
5353
config.SecOpts = secOpts
@@ -57,7 +57,7 @@ func TestNewGRPCClient_GoodConfig(t *testing.T) {
5757
assert.False(t, client.TLSEnabled())
5858
assert.False(t, client.MutualTLSRequired())
5959

60-
secOpts = &comm.SecureOptions{
60+
secOpts = comm.SecureOptions{
6161
UseTLS: true,
6262
ServerRootCAs: [][]byte{testCerts.caPEM},
6363
RequireClientCert: false,
@@ -68,7 +68,7 @@ func TestNewGRPCClient_GoodConfig(t *testing.T) {
6868
assert.True(t, client.TLSEnabled())
6969
assert.False(t, client.MutualTLSRequired())
7070

71-
secOpts = &comm.SecureOptions{
71+
secOpts = comm.SecureOptions{
7272
Certificate: testCerts.certPEM,
7373
Key: testCerts.keyPEM,
7474
UseTLS: true,
@@ -89,7 +89,7 @@ func TestNewGRPCClient_BadConfig(t *testing.T) {
8989

9090
// bad root cert
9191
config := comm.ClientConfig{
92-
SecOpts: &comm.SecureOptions{
92+
SecOpts: comm.SecureOptions{
9393
UseTLS: true,
9494
ServerRootCAs: [][]byte{[]byte(badPEM)},
9595
},
@@ -99,7 +99,7 @@ func TestNewGRPCClient_BadConfig(t *testing.T) {
9999

100100
// missing key
101101
missing := "both Key and Certificate are required when using mutual TLS"
102-
config.SecOpts = &comm.SecureOptions{
102+
config.SecOpts = comm.SecureOptions{
103103
Certificate: []byte("cert"),
104104
UseTLS: true,
105105
RequireClientCert: true,
@@ -108,7 +108,7 @@ func TestNewGRPCClient_BadConfig(t *testing.T) {
108108
assert.Equal(t, missing, err.Error())
109109

110110
// missing cert
111-
config.SecOpts = &comm.SecureOptions{
111+
config.SecOpts = comm.SecureOptions{
112112
Key: []byte("key"),
113113
UseTLS: true,
114114
RequireClientCert: true,
@@ -118,7 +118,7 @@ func TestNewGRPCClient_BadConfig(t *testing.T) {
118118

119119
// bad key
120120
failed := "failed to load client certificate"
121-
config.SecOpts = &comm.SecureOptions{
121+
config.SecOpts = comm.SecureOptions{
122122
Certificate: testCerts.certPEM,
123123
Key: []byte(badPEM),
124124
UseTLS: true,
@@ -128,7 +128,7 @@ func TestNewGRPCClient_BadConfig(t *testing.T) {
128128
assert.Contains(t, err.Error(), failed)
129129

130130
// bad cert
131-
config.SecOpts = &comm.SecureOptions{
131+
config.SecOpts = comm.SecureOptions{
132132
Certificate: []byte(badPEM),
133133
Key: testCerts.keyPEM,
134134
UseTLS: true,
@@ -189,7 +189,7 @@ func TestNewConnection(t *testing.T) {
189189
{
190190
name: "client TLS / server no TLS",
191191
config: comm.ClientConfig{
192-
SecOpts: &comm.SecureOptions{
192+
SecOpts: comm.SecureOptions{
193193
Certificate: testCerts.certPEM,
194194
Key: testCerts.keyPEM,
195195
UseTLS: true,
@@ -204,7 +204,7 @@ func TestNewConnection(t *testing.T) {
204204
{
205205
name: "client TLS / server TLS match",
206206
config: comm.ClientConfig{
207-
SecOpts: &comm.SecureOptions{
207+
SecOpts: comm.SecureOptions{
208208
Certificate: testCerts.certPEM,
209209
Key: testCerts.keyPEM,
210210
UseTLS: true,
@@ -220,7 +220,7 @@ func TestNewConnection(t *testing.T) {
220220
{
221221
name: "client TLS / server TLS no server roots",
222222
config: comm.ClientConfig{
223-
SecOpts: &comm.SecureOptions{
223+
SecOpts: comm.SecureOptions{
224224
Certificate: testCerts.certPEM,
225225
Key: testCerts.keyPEM,
226226
UseTLS: true,
@@ -237,7 +237,7 @@ func TestNewConnection(t *testing.T) {
237237
{
238238
name: "client TLS / server TLS missing client cert",
239239
config: comm.ClientConfig{
240-
SecOpts: &comm.SecureOptions{
240+
SecOpts: comm.SecureOptions{
241241
Certificate: testCerts.certPEM,
242242
Key: testCerts.keyPEM,
243243
UseTLS: true,
@@ -255,7 +255,7 @@ func TestNewConnection(t *testing.T) {
255255
{
256256
name: "client TLS / server TLS client cert",
257257
config: comm.ClientConfig{
258-
SecOpts: &comm.SecureOptions{
258+
SecOpts: comm.SecureOptions{
259259
Certificate: testCerts.certPEM,
260260
Key: testCerts.keyPEM,
261261
UseTLS: true,
@@ -274,7 +274,7 @@ func TestNewConnection(t *testing.T) {
274274
{
275275
name: "server TLS pinning success",
276276
config: comm.ClientConfig{
277-
SecOpts: &comm.SecureOptions{
277+
SecOpts: comm.SecureOptions{
278278
VerifyCertificate: func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
279279
if bytes.Equal(rawCerts[0], testCerts.serverCert.Certificate[0]) {
280280
return nil
@@ -299,7 +299,7 @@ func TestNewConnection(t *testing.T) {
299299
{
300300
name: "server TLS pinning failure",
301301
config: comm.ClientConfig{
302-
SecOpts: &comm.SecureOptions{
302+
SecOpts: comm.SecureOptions{
303303
VerifyCertificate: func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
304304
return errors.New("TLS certificate mismatch")
305305
},
@@ -362,7 +362,7 @@ func TestSetServerRootCAs(t *testing.T) {
362362
testCerts := loadCerts(t)
363363

364364
config := comm.ClientConfig{
365-
SecOpts: &comm.SecureOptions{
365+
SecOpts: comm.SecureOptions{
366366
UseTLS: true,
367367
ServerRootCAs: [][]byte{testCerts.caPEM},
368368
},

‎core/comm/config.go

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ var (
2323
MaxRecvMsgSize = 100 * 1024 * 1024
2424
MaxSendMsgSize = 100 * 1024 * 1024
2525
// Default peer keepalive options
26-
DefaultKeepaliveOptions = &KeepaliveOptions{
26+
DefaultKeepaliveOptions = KeepaliveOptions{
2727
ClientInterval: time.Duration(1) * time.Minute, // 1 min
2828
ClientTimeout: time.Duration(20) * time.Second, // 20 sec - gRPC default
2929
ServerInterval: time.Duration(2) * time.Hour, // 2 hours - gRPC default
@@ -49,9 +49,9 @@ type ServerConfig struct {
4949
// for all new connections
5050
ConnectionTimeout time.Duration
5151
// SecOpts defines the security parameters
52-
SecOpts *SecureOptions
52+
SecOpts SecureOptions
5353
// KaOpts defines the keepalive parameters
54-
KaOpts *KeepaliveOptions
54+
KaOpts KeepaliveOptions
5555
// StreamInterceptors specifies a list of interceptors to apply to
5656
// streaming RPCs. They are executed in order.
5757
StreamInterceptors []grpc.StreamServerInterceptor
@@ -69,9 +69,9 @@ type ServerConfig struct {
6969
// ClientConfig defines the parameters for configuring a GRPCClient instance
7070
type ClientConfig struct {
7171
// SecOpts defines the security parameters
72-
SecOpts *SecureOptions
72+
SecOpts SecureOptions
7373
// KaOpts defines the keepalive parameters
74-
KaOpts *KeepaliveOptions
74+
KaOpts KeepaliveOptions
7575
// Timeout specifies how long the client will block when attempting to
7676
// establish a connection
7777
Timeout time.Duration
@@ -82,14 +82,6 @@ type ClientConfig struct {
8282
// Clone clones this ClientConfig
8383
func (cc ClientConfig) Clone() ClientConfig {
8484
shallowClone := cc
85-
if shallowClone.SecOpts != nil {
86-
secOptsClone := *cc.SecOpts
87-
shallowClone.SecOpts = &secOptsClone
88-
}
89-
if shallowClone.KaOpts != nil {
90-
kaOptsClone := *cc.KaOpts
91-
shallowClone.KaOpts = &kaOptsClone
92-
}
9385
return shallowClone
9486
}
9587

@@ -145,13 +137,8 @@ type Metrics struct {
145137
ClosedConnCounter metrics.Counter
146138
}
147139

148-
// ServerKeepaliveOptions returns gRPC keepalive options for server. If
149-
// opts is nil, the default keepalive options are returned
150-
func ServerKeepaliveOptions(ka *KeepaliveOptions) []grpc.ServerOption {
151-
// use default keepalive options if nil
152-
if ka == nil {
153-
ka = DefaultKeepaliveOptions
154-
}
140+
// ServerKeepaliveOptions returns gRPC keepalive options for server.
141+
func ServerKeepaliveOptions(ka KeepaliveOptions) []grpc.ServerOption {
155142
var serverOpts []grpc.ServerOption
156143
kap := keepalive.ServerParameters{
157144
Time: ka.ServerInterval,
@@ -167,14 +154,8 @@ func ServerKeepaliveOptions(ka *KeepaliveOptions) []grpc.ServerOption {
167154
return serverOpts
168155
}
169156

170-
// ClientKeepaliveOptions returns gRPC keepalive options for clients. If
171-
// opts is nil, the default keepalive options are returned
172-
func ClientKeepaliveOptions(ka *KeepaliveOptions) []grpc.DialOption {
173-
// use default keepalive options if nil
174-
if ka == nil {
175-
ka = DefaultKeepaliveOptions
176-
}
177-
157+
// ClientKeepaliveOptions returns gRPC keepalive options for clients.
158+
func ClientKeepaliveOptions(ka KeepaliveOptions) []grpc.DialOption {
178159
var dialOpts []grpc.DialOption
179160
kap := keepalive.ClientParameters{
180161
Time: ka.ClientInterval,

0 commit comments

Comments
 (0)