Browse code

Exit if service config is loaded unsuccessfully on startup

Signed-off-by: Boaz Shuster <ripcurld.github@gmail.com>

Boaz Shuster authored on 2017/09/01 23:35:04
Showing 9 changed files
... ...
@@ -198,7 +198,11 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
198 198
 		cli.api.Accept(addr, ls...)
199 199
 	}
200 200
 
201
-	registryService := registry.NewService(cli.Config.ServiceOptions)
201
+	registryService, err := registry.NewService(cli.Config.ServiceOptions)
202
+	if err != nil {
203
+		return err
204
+	}
205
+
202 206
 	containerdRemote, err := libcontainerd.New(cli.getLibcontainerdRoot(), cli.getPlatformRemoteOptions()...)
203 207
 	if err != nil {
204 208
 		return err
... ...
@@ -46,8 +46,9 @@ func TestDaemonReloadAllowNondistributableArtifacts(t *testing.T) {
46 46
 		configStore: &config.Config{},
47 47
 	}
48 48
 
49
+	var err error
49 50
 	// Initialize daemon with some registries.
50
-	daemon.RegistryService = registry.NewService(registry.ServiceOptions{
51
+	daemon.RegistryService, err = registry.NewService(registry.ServiceOptions{
51 52
 		AllowNondistributableArtifacts: []string{
52 53
 			"127.0.0.0/8",
53 54
 			"10.10.1.11:5000",
... ...
@@ -56,6 +57,9 @@ func TestDaemonReloadAllowNondistributableArtifacts(t *testing.T) {
56 56
 			"docker2.com", // This will be removed during reload.
57 57
 		},
58 58
 	})
59
+	if err != nil {
60
+		t.Fatal(err)
61
+	}
59 62
 
60 63
 	registries := []string{
61 64
 		"127.0.0.0/8",
... ...
@@ -98,7 +102,8 @@ func TestDaemonReloadAllowNondistributableArtifacts(t *testing.T) {
98 98
 
99 99
 func TestDaemonReloadMirrors(t *testing.T) {
100 100
 	daemon := &Daemon{}
101
-	daemon.RegistryService = registry.NewService(registry.ServiceOptions{
101
+	var err error
102
+	daemon.RegistryService, err = registry.NewService(registry.ServiceOptions{
102 103
 		InsecureRegistries: []string{},
103 104
 		Mirrors: []string{
104 105
 			"https://mirror.test1.com",
... ...
@@ -106,6 +111,9 @@ func TestDaemonReloadMirrors(t *testing.T) {
106 106
 			"https://mirror.test3.com", // this will be removed when reloading
107 107
 		},
108 108
 	})
109
+	if err != nil {
110
+		t.Fatal(err)
111
+	}
109 112
 
110 113
 	daemon.configStore = &config.Config{}
111 114
 
... ...
@@ -191,8 +199,9 @@ func TestDaemonReloadMirrors(t *testing.T) {
191 191
 
192 192
 func TestDaemonReloadInsecureRegistries(t *testing.T) {
193 193
 	daemon := &Daemon{}
194
+	var err error
194 195
 	// initialize daemon with existing insecure registries: "127.0.0.0/8", "10.10.1.11:5000", "10.10.1.22:5000"
195
-	daemon.RegistryService = registry.NewService(registry.ServiceOptions{
196
+	daemon.RegistryService, err = registry.NewService(registry.ServiceOptions{
196 197
 		InsecureRegistries: []string{
197 198
 			"127.0.0.0/8",
198 199
 			"10.10.1.11:5000",
... ...
@@ -201,6 +210,9 @@ func TestDaemonReloadInsecureRegistries(t *testing.T) {
201 201
 			"docker2.com", // this will be removed when reloading
202 202
 		},
203 203
 	})
204
+	if err != nil {
205
+		t.Fatal(err)
206
+	}
204 207
 
205 208
 	daemon.configStore = &config.Config{}
206 209
 
... ...
@@ -71,9 +71,14 @@ func CreateInRegistry(ctx context.Context, repo string, auth *types.AuthConfig,
71 71
 	}
72 72
 	defer tar.Close()
73 73
 
74
+	regService, err := registry.NewService(registry.ServiceOptions{V2Only: true})
75
+	if err != nil {
76
+		return err
77
+	}
78
+
74 79
 	managerConfig := plugin.ManagerConfig{
75 80
 		Store:           plugin.NewStore(),
76
-		RegistryService: registry.NewService(registry.ServiceOptions{V2Only: true}),
81
+		RegistryService: regService,
77 82
 		Root:            filepath.Join(tmpDir, "root"),
78 83
 		ExecRoot:        "/run/docker", // manager init fails if not set
79 84
 		Executor:        dummyExecutor{},
... ...
@@ -60,7 +60,7 @@ var (
60 60
 	// not have the correct form
61 61
 	ErrInvalidRepositoryName = errors.New("Invalid repository name (ex: \"registry.domain.tld/myrepos\")")
62 62
 
63
-	emptyServiceConfig = newServiceConfig(ServiceOptions{})
63
+	emptyServiceConfig, _ = newServiceConfig(ServiceOptions{})
64 64
 )
65 65
 
66 66
 var (
... ...
@@ -71,7 +71,7 @@ var (
71 71
 var lookupIP = net.LookupIP
72 72
 
73 73
 // newServiceConfig returns a new instance of ServiceConfig
74
-func newServiceConfig(options ServiceOptions) *serviceConfig {
74
+func newServiceConfig(options ServiceOptions) (*serviceConfig, error) {
75 75
 	config := &serviceConfig{
76 76
 		ServiceConfig: registrytypes.ServiceConfig{
77 77
 			InsecureRegistryCIDRs: make([]*registrytypes.NetIPNet, 0),
... ...
@@ -81,12 +81,17 @@ func newServiceConfig(options ServiceOptions) *serviceConfig {
81 81
 		},
82 82
 		V2Only: options.V2Only,
83 83
 	}
84
+	if err := config.LoadAllowNondistributableArtifacts(options.AllowNondistributableArtifacts); err != nil {
85
+		return nil, err
86
+	}
87
+	if err := config.LoadMirrors(options.Mirrors); err != nil {
88
+		return nil, err
89
+	}
90
+	if err := config.LoadInsecureRegistries(options.InsecureRegistries); err != nil {
91
+		return nil, err
92
+	}
84 93
 
85
-	config.LoadAllowNondistributableArtifacts(options.AllowNondistributableArtifacts)
86
-	config.LoadMirrors(options.Mirrors)
87
-	config.LoadInsecureRegistries(options.InsecureRegistries)
88
-
89
-	return config
94
+	return config, nil
90 95
 }
91 96
 
92 97
 // LoadAllowNondistributableArtifacts loads allow-nondistributable-artifacts registries into config.
... ...
@@ -5,6 +5,8 @@ import (
5 5
 	"sort"
6 6
 	"strings"
7 7
 	"testing"
8
+
9
+	"github.com/stretchr/testify/assert"
8 10
 )
9 11
 
10 12
 func TestLoadAllowNondistributableArtifacts(t *testing.T) {
... ...
@@ -90,7 +92,7 @@ func TestLoadAllowNondistributableArtifacts(t *testing.T) {
90 90
 		},
91 91
 	}
92 92
 	for _, testCase := range testCases {
93
-		config := newServiceConfig(ServiceOptions{})
93
+		config := emptyServiceConfig
94 94
 		err := config.LoadAllowNondistributableArtifacts(testCase.registries)
95 95
 		if testCase.err == "" {
96 96
 			if err != nil {
... ...
@@ -233,7 +235,7 @@ func TestLoadInsecureRegistries(t *testing.T) {
233 233
 		},
234 234
 	}
235 235
 	for _, testCase := range testCases {
236
-		config := newServiceConfig(ServiceOptions{})
236
+		config := emptyServiceConfig
237 237
 		err := config.LoadInsecureRegistries(testCase.registries)
238 238
 		if testCase.err == "" {
239 239
 			if err != nil {
... ...
@@ -258,3 +260,60 @@ func TestLoadInsecureRegistries(t *testing.T) {
258 258
 		}
259 259
 	}
260 260
 }
261
+
262
+func TestNewServiceConfig(t *testing.T) {
263
+	testCases := []struct {
264
+		opts   ServiceOptions
265
+		errStr string
266
+	}{
267
+		{
268
+			ServiceOptions{},
269
+			"",
270
+		},
271
+		{
272
+			ServiceOptions{
273
+				Mirrors: []string{"example.com:5000"},
274
+			},
275
+			`invalid mirror: unsupported scheme "example.com" in "example.com:5000"`,
276
+		},
277
+		{
278
+			ServiceOptions{
279
+				Mirrors: []string{"http://example.com:5000"},
280
+			},
281
+			"",
282
+		},
283
+		{
284
+			ServiceOptions{
285
+				InsecureRegistries: []string{"[fe80::]/64"},
286
+			},
287
+			`insecure registry [fe80::]/64 is not valid: invalid host "[fe80::]/64"`,
288
+		},
289
+		{
290
+			ServiceOptions{
291
+				InsecureRegistries: []string{"102.10.8.1/24"},
292
+			},
293
+			"",
294
+		},
295
+		{
296
+			ServiceOptions{
297
+				AllowNondistributableArtifacts: []string{"[fe80::]/64"},
298
+			},
299
+			`allow-nondistributable-artifacts registry [fe80::]/64 is not valid: invalid host "[fe80::]/64"`,
300
+		},
301
+		{
302
+			ServiceOptions{
303
+				AllowNondistributableArtifacts: []string{"102.10.8.1/24"},
304
+			},
305
+			"",
306
+		},
307
+	}
308
+
309
+	for _, testCase := range testCases {
310
+		_, err := newServiceConfig(testCase.opts)
311
+		if testCase.errStr != "" {
312
+			assert.EqualError(t, err, testCase.errStr)
313
+		} else {
314
+			assert.Nil(t, err)
315
+		}
316
+	}
317
+}
... ...
@@ -175,7 +175,7 @@ func makePublicIndex() *registrytypes.IndexInfo {
175 175
 	return index
176 176
 }
177 177
 
178
-func makeServiceConfig(mirrors []string, insecureRegistries []string) *serviceConfig {
178
+func makeServiceConfig(mirrors []string, insecureRegistries []string) (*serviceConfig, error) {
179 179
 	options := ServiceOptions{
180 180
 		Mirrors:            mirrors,
181 181
 		InsecureRegistries: insecureRegistries,
... ...
@@ -539,7 +539,7 @@ func TestNewIndexInfo(t *testing.T) {
539 539
 		}
540 540
 	}
541 541
 
542
-	config := newServiceConfig(ServiceOptions{})
542
+	config := emptyServiceConfig
543 543
 	noMirrors := []string{}
544 544
 	expectedIndexInfos := map[string]*registrytypes.IndexInfo{
545 545
 		IndexName: {
... ...
@@ -570,7 +570,11 @@ func TestNewIndexInfo(t *testing.T) {
570 570
 	testIndexInfo(config, expectedIndexInfos)
571 571
 
572 572
 	publicMirrors := []string{"http://mirror1.local", "http://mirror2.local"}
573
-	config = makeServiceConfig(publicMirrors, []string{"example.com"})
573
+	var err error
574
+	config, err = makeServiceConfig(publicMirrors, []string{"example.com"})
575
+	if err != nil {
576
+		t.Fatal(err)
577
+	}
574 578
 
575 579
 	expectedIndexInfos = map[string]*registrytypes.IndexInfo{
576 580
 		IndexName: {
... ...
@@ -618,7 +622,10 @@ func TestNewIndexInfo(t *testing.T) {
618 618
 	}
619 619
 	testIndexInfo(config, expectedIndexInfos)
620 620
 
621
-	config = makeServiceConfig(nil, []string{"42.42.0.0/16"})
621
+	config, err = makeServiceConfig(nil, []string{"42.42.0.0/16"})
622
+	if err != nil {
623
+		t.Fatal(err)
624
+	}
622 625
 	expectedIndexInfos = map[string]*registrytypes.IndexInfo{
623 626
 		"example.com": {
624 627
 			Name:     "example.com",
... ...
@@ -663,7 +670,11 @@ func TestMirrorEndpointLookup(t *testing.T) {
663 663
 		}
664 664
 		return false
665 665
 	}
666
-	s := DefaultService{config: makeServiceConfig([]string{"https://my.mirror"}, nil)}
666
+	cfg, err := makeServiceConfig([]string{"https://my.mirror"}, nil)
667
+	if err != nil {
668
+		t.Fatal(err)
669
+	}
670
+	s := DefaultService{config: cfg}
667 671
 
668 672
 	imageName, err := reference.WithName(IndexName + "/test/image")
669 673
 	if err != nil {
... ...
@@ -844,9 +855,12 @@ func TestAllowNondistributableArtifacts(t *testing.T) {
844 844
 		{"invalid.domain.com:5000", []string{"invalid.domain.com:5000"}, true},
845 845
 	}
846 846
 	for _, tt := range tests {
847
-		config := newServiceConfig(ServiceOptions{
847
+		config, err := newServiceConfig(ServiceOptions{
848 848
 			AllowNondistributableArtifacts: tt.registries,
849 849
 		})
850
+		if err != nil {
851
+			t.Error(err)
852
+		}
850 853
 		if v := allowNondistributableArtifacts(config, tt.addr); v != tt.expected {
851 854
 			t.Errorf("allowNondistributableArtifacts failed for %q %v, expected %v got %v", tt.addr, tt.registries, tt.expected, v)
852 855
 		}
... ...
@@ -886,7 +900,10 @@ func TestIsSecureIndex(t *testing.T) {
886 886
 		{"invalid.domain.com:5000", []string{"invalid.domain.com:5000"}, false},
887 887
 	}
888 888
 	for _, tt := range tests {
889
-		config := makeServiceConfig(nil, tt.insecureRegistries)
889
+		config, err := makeServiceConfig(nil, tt.insecureRegistries)
890
+		if err != nil {
891
+			t.Error(err)
892
+		}
890 893
 		if sec := isSecureIndex(config, tt.addr); sec != tt.expected {
891 894
 			t.Errorf("isSecureIndex failed for %q %v, expected %v got %v", tt.addr, tt.insecureRegistries, tt.expected, sec)
892 895
 		}
... ...
@@ -45,10 +45,10 @@ type DefaultService struct {
45 45
 
46 46
 // NewService returns a new instance of DefaultService ready to be
47 47
 // installed into an engine.
48
-func NewService(options ServiceOptions) *DefaultService {
49
-	return &DefaultService{
50
-		config: newServiceConfig(options),
51
-	}
48
+func NewService(options ServiceOptions) (*DefaultService, error) {
49
+	config, err := newServiceConfig(options)
50
+
51
+	return &DefaultService{config: config}, err
52 52
 }
53 53
 
54 54
 // ServiceConfig returns the public registry service configuration.
... ...
@@ -3,7 +3,10 @@ package registry
3 3
 import "testing"
4 4
 
5 5
 func TestLookupV1Endpoints(t *testing.T) {
6
-	s := NewService(ServiceOptions{})
6
+	s, err := NewService(ServiceOptions{})
7
+	if err != nil {
8
+		t.Fatal(err)
9
+	}
7 10
 
8 11
 	cases := []struct {
9 12
 		hostname    string