Skip to content
This repository was archived by the owner on Mar 9, 2022. It is now read-only.

Commit dffd0df

Browse files
JulienBalestraJulienBalestra
committed
streaming: tls conf validation to func with tests
Signed-off-by: JulienBalestra <[email protected]>
1 parent 859003a commit dffd0df

3 files changed

Lines changed: 203 additions & 19 deletions

File tree

‎docs/config.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ The explanation and default value of each configuration item are as follows:
2828
# It generates a self-sign certificate unless the following x509_key_pair_streaming are both set.
2929
enable_tls_streaming = false
3030

31-
# "plugins.cri.x509_key_pair_streaming" constains a x509 valid key pair to stream with tls.
31+
# "plugins.cri.x509_key_pair_streaming" contains a x509 valid key pair to stream with tls.
3232
[plugins.cri.x509_key_pair_streaming]
3333
# tls_cert_file is the filepath to the certificate paired with the "tls_key_file"
3434
tls_cert_file = ""

‎pkg/server/streaming.go‎

Lines changed: 49 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,36 @@ import (
3434
ctrdutil "github.com/containerd/cri/pkg/containerd/util"
3535
)
3636

37+
type streamListenerMode int
38+
39+
const (
40+
x509KeyPairTLS streamListenerMode = iota
41+
selfSignTLS
42+
withoutTLS
43+
)
44+
45+
func getStreamListenerMode(c *criService) (streamListenerMode, error) {
46+
if c.config.EnableTLSStreaming {
47+
if c.config.X509KeyPairStreaming.TLSCertFile != "" && c.config.X509KeyPairStreaming.TLSKeyFile != "" {
48+
return x509KeyPairTLS, nil
49+
}
50+
if c.config.X509KeyPairStreaming.TLSCertFile != "" && c.config.X509KeyPairStreaming.TLSKeyFile == "" {
51+
return -1, errors.New("must set X509KeyPairStreaming.TLSKeyFile")
52+
}
53+
if c.config.X509KeyPairStreaming.TLSCertFile == "" && c.config.X509KeyPairStreaming.TLSKeyFile != "" {
54+
return -1, errors.New("must set X509KeyPairStreaming.TLSCertFile")
55+
}
56+
return selfSignTLS, nil
57+
}
58+
if c.config.X509KeyPairStreaming.TLSCertFile != "" {
59+
return -1, errors.New("X509KeyPairStreaming.TLSCertFile is set but EnableTLSStreaming is not set")
60+
}
61+
if c.config.X509KeyPairStreaming.TLSKeyFile != "" {
62+
return -1, errors.New("X509KeyPairStreaming.TLSKeyFile is set but EnableTLSStreaming is not set")
63+
}
64+
return withoutTLS, nil
65+
}
66+
3767
func newStreamServer(c *criService, addr, port string) (streaming.Server, error) {
3868
if addr == "" {
3969
a, err := k8snet.ChooseBindAddress(nil)
@@ -45,13 +75,12 @@ func newStreamServer(c *criService, addr, port string) (streaming.Server, error)
4575
config := streaming.DefaultConfig
4676
config.Addr = net.JoinHostPort(addr, port)
4777
run := newStreamRuntime(c)
48-
if !c.config.EnableTLSStreaming {
49-
if c.config.X509KeyPairStreaming.TLSCertFile != "" || c.config.X509KeyPairStreaming.TLSKeyFile != "" {
50-
return nil, errors.Errorf("X509KeyPairStreaming.TLSCertFile and/or X509KeyPairStreaming.TLSKeyFile are set but EnableTLSStreaming is not set")
51-
}
52-
return streaming.NewServer(config, run)
78+
tlsMode, err := getStreamListenerMode(c)
79+
if err != nil {
80+
return nil, errors.Wrapf(err, "invalid stream server configuration")
5381
}
54-
if c.config.X509KeyPairStreaming.TLSCertFile != "" && c.config.X509KeyPairStreaming.TLSKeyFile != "" {
82+
switch tlsMode {
83+
case x509KeyPairTLS:
5584
tlsCert, err := tls.LoadX509KeyPair(c.config.X509KeyPairStreaming.TLSCertFile, c.config.X509KeyPairStreaming.TLSKeyFile)
5685
if err != nil {
5786
return nil, errors.Wrap(err, "failed to load x509 key pair for stream server")
@@ -60,19 +89,21 @@ func newStreamServer(c *criService, addr, port string) (streaming.Server, error)
6089
Certificates: []tls.Certificate{tlsCert},
6190
}
6291
return streaming.NewServer(config, run)
63-
} else if c.config.X509KeyPairStreaming.TLSCertFile != "" || c.config.X509KeyPairStreaming.TLSKeyFile != "" {
64-
return nil, errors.Errorf("must set both X509KeyPairStreaming.TLSCertFile and X509KeyPairStreaming.TLSKeyFile")
65-
}
66-
// generating self-sign certs
67-
tlsCert, err := newTLSCert()
68-
if err != nil {
69-
return nil, errors.Wrap(err, "failed to generate tls certificate for stream server")
70-
}
71-
config.TLSConfig = &tls.Config{
72-
Certificates: []tls.Certificate{tlsCert},
73-
InsecureSkipVerify: true,
92+
case selfSignTLS:
93+
tlsCert, err := newTLSCert()
94+
if err != nil {
95+
return nil, errors.Wrap(err, "failed to generate tls certificate for stream server")
96+
}
97+
config.TLSConfig = &tls.Config{
98+
Certificates: []tls.Certificate{tlsCert},
99+
InsecureSkipVerify: true,
100+
}
101+
return streaming.NewServer(config, run)
102+
case withoutTLS:
103+
return streaming.NewServer(config, run)
104+
default:
105+
return nil, errors.New("invalid configuration for the stream listener")
74106
}
75-
return streaming.NewServer(config, run)
76107
}
77108

78109
type streamRuntime struct {

‎pkg/server/streaming_test.go‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
/*
2+
Copyright 2017 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package server
18+
19+
import (
20+
"testing"
21+
22+
"github.com/containerd/cri/pkg/config"
23+
"github.com/stretchr/testify/assert"
24+
)
25+
26+
func TestValidateStreamServer(t *testing.T) {
27+
for desc, test := range map[string]struct {
28+
*criService
29+
tlsMode streamListenerMode
30+
expectErr bool
31+
}{
32+
"should pass with default withoutTLS": {
33+
criService: &criService{
34+
config: config.Config{
35+
PluginConfig: config.DefaultConfig(),
36+
},
37+
},
38+
tlsMode: withoutTLS,
39+
expectErr: false,
40+
},
41+
"should pass with x509KeyPairTLS": {
42+
criService: &criService{
43+
config: config.Config{
44+
PluginConfig: config.PluginConfig{
45+
EnableTLSStreaming: true,
46+
X509KeyPairStreaming: config.X509KeyPairStreaming{
47+
TLSKeyFile: "non-empty",
48+
TLSCertFile: "non-empty",
49+
},
50+
},
51+
},
52+
},
53+
tlsMode: x509KeyPairTLS,
54+
expectErr: false,
55+
},
56+
"should pass with selfSign": {
57+
criService: &criService{
58+
config: config.Config{
59+
PluginConfig: config.PluginConfig{
60+
EnableTLSStreaming: true,
61+
},
62+
},
63+
},
64+
tlsMode: selfSignTLS,
65+
expectErr: false,
66+
},
67+
"should return error with X509 keypair but not EnableTLSStreaming": {
68+
criService: &criService{
69+
config: config.Config{
70+
PluginConfig: config.PluginConfig{
71+
EnableTLSStreaming: false,
72+
X509KeyPairStreaming: config.X509KeyPairStreaming{
73+
TLSKeyFile: "non-empty",
74+
TLSCertFile: "non-empty",
75+
},
76+
},
77+
},
78+
},
79+
tlsMode: -1,
80+
expectErr: true,
81+
},
82+
"should return error with X509 TLSCertFile empty": {
83+
criService: &criService{
84+
config: config.Config{
85+
PluginConfig: config.PluginConfig{
86+
EnableTLSStreaming: true,
87+
X509KeyPairStreaming: config.X509KeyPairStreaming{
88+
TLSKeyFile: "non-empty",
89+
TLSCertFile: "",
90+
},
91+
},
92+
},
93+
},
94+
tlsMode: -1,
95+
expectErr: true,
96+
},
97+
"should return error with X509 TLSKeyFile empty": {
98+
criService: &criService{
99+
config: config.Config{
100+
PluginConfig: config.PluginConfig{
101+
EnableTLSStreaming: true,
102+
X509KeyPairStreaming: config.X509KeyPairStreaming{
103+
TLSKeyFile: "",
104+
TLSCertFile: "non-empty",
105+
},
106+
},
107+
},
108+
},
109+
tlsMode: -1,
110+
expectErr: true,
111+
},
112+
"should return error without EnableTLSStreaming and only TLSCertFile set": {
113+
criService: &criService{
114+
config: config.Config{
115+
PluginConfig: config.PluginConfig{
116+
EnableTLSStreaming: false,
117+
X509KeyPairStreaming: config.X509KeyPairStreaming{
118+
TLSKeyFile: "",
119+
TLSCertFile: "non-empty",
120+
},
121+
},
122+
},
123+
},
124+
tlsMode: -1,
125+
expectErr: true,
126+
},
127+
"should return error without EnableTLSStreaming and only TLSKeyFile set": {
128+
criService: &criService{
129+
config: config.Config{
130+
PluginConfig: config.PluginConfig{
131+
EnableTLSStreaming: false,
132+
X509KeyPairStreaming: config.X509KeyPairStreaming{
133+
TLSKeyFile: "non-empty",
134+
TLSCertFile: "",
135+
},
136+
},
137+
},
138+
},
139+
tlsMode: -1,
140+
expectErr: true,
141+
},
142+
} {
143+
t.Run(desc, func(t *testing.T) {
144+
tlsMode, err := getStreamListenerMode(test.criService)
145+
if test.expectErr {
146+
assert.Error(t, err)
147+
return
148+
}
149+
assert.NoError(t, err)
150+
assert.Equal(t, test.tlsMode, tlsMode)
151+
})
152+
}
153+
}

0 commit comments

Comments
 (0)