Merge pull request #34495 from ripcurld0/registry_mirror_json

Exit if service config is loaded unsuccessfully on startup

cmd/dockerd/daemon.go

@@ -199,7 +199,11 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
 		cli.api.Accept(addr, ls...)
 	}
 
-	registryService := registry.NewService(cli.Config.ServiceOptions)
+	registryService, err := registry.NewService(cli.Config.ServiceOptions)
+	if err != nil {
+		return err
+	}
+
 	containerdRemote, err := libcontainerd.New(cli.getLibcontainerdRoot(), cli.getPlatformRemoteOptions()...)
 	if err != nil {
 		return err

daemon/reload_test.go

@@ -47,8 +47,9 @@ func TestDaemonReloadAllowNondistributableArtifacts(t *testing.T) {
 		configStore: &config.Config{},
 	}
 
+	var err error
 	// Initialize daemon with some registries.
-	daemon.RegistryService = registry.NewService(registry.ServiceOptions{
+	daemon.RegistryService, err = registry.NewService(registry.ServiceOptions{
 		AllowNondistributableArtifacts: []string{
 			"127.0.0.0/8",
 			"10.10.1.11:5000",
@@ -57,6 +58,9 @@ func TestDaemonReloadAllowNondistributableArtifacts(t *testing.T) {
 			"docker2.com", // This will be removed during reload.
 		},
 	})
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	registries := []string{
 		"127.0.0.0/8",
@@ -95,7 +99,8 @@ func TestDaemonReloadAllowNondistributableArtifacts(t *testing.T) {
 
 func TestDaemonReloadMirrors(t *testing.T) {
 	daemon := &Daemon{}
-	daemon.RegistryService = registry.NewService(registry.ServiceOptions{
+	var err error
+	daemon.RegistryService, err = registry.NewService(registry.ServiceOptions{
 		InsecureRegistries: []string{},
 		Mirrors: []string{
 			"https://mirror.test1.com",
@@ -103,6 +108,9 @@ func TestDaemonReloadMirrors(t *testing.T) {
 			"https://mirror.test3.com", // this will be removed when reloading
 		},
 	})
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	daemon.configStore = &config.Config{}
 
@@ -188,8 +196,9 @@ func TestDaemonReloadMirrors(t *testing.T) {
 
 func TestDaemonReloadInsecureRegistries(t *testing.T) {
 	daemon := &Daemon{}
+	var err error
 	// initialize daemon with existing insecure registries: "127.0.0.0/8", "10.10.1.11:5000", "10.10.1.22:5000"
-	daemon.RegistryService = registry.NewService(registry.ServiceOptions{
+	daemon.RegistryService, err = registry.NewService(registry.ServiceOptions{
 		InsecureRegistries: []string{
 			"127.0.0.0/8",
 			"10.10.1.11:5000",
@@ -198,6 +207,9 @@ func TestDaemonReloadInsecureRegistries(t *testing.T) {
 			"docker2.com", // this will be removed when reloading
 		},
 	})
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	daemon.configStore = &config.Config{}
 

integration-cli/fixtures/plugin/plugin_linux.go

@@ -71,9 +71,14 @@ func CreateInRegistry(ctx context.Context, repo string, auth *types.AuthConfig,
 	}
 	defer tar.Close()
 
+	regService, err := registry.NewService(registry.ServiceOptions{V2Only: true})
+	if err != nil {
+		return err
+	}
+
 	managerConfig := plugin.ManagerConfig{
 		Store:           plugin.NewStore(),
-		RegistryService: registry.NewService(registry.ServiceOptions{V2Only: true}),
+		RegistryService: regService,
 		Root:            filepath.Join(tmpDir, "root"),
 		ExecRoot:        "/run/docker", // manager init fails if not set
 		Executor:        dummyExecutor{},

registry/config.go

@@ -60,7 +60,7 @@ var (
 	// not have the correct form
 	ErrInvalidRepositoryName = errors.New("Invalid repository name (ex: \"registry.domain.tld/myrepos\")")
 
-	emptyServiceConfig = newServiceConfig(ServiceOptions{})
+	emptyServiceConfig, _ = newServiceConfig(ServiceOptions{})
 )
 
 var (
@@ -71,7 +71,7 @@ var (
 var lookupIP = net.LookupIP
 
 // newServiceConfig returns a new instance of ServiceConfig
-func newServiceConfig(options ServiceOptions) *serviceConfig {
+func newServiceConfig(options ServiceOptions) (*serviceConfig, error) {
 	config := &serviceConfig{
 		ServiceConfig: registrytypes.ServiceConfig{
 			InsecureRegistryCIDRs: make([]*registrytypes.NetIPNet, 0),
@@ -81,12 +81,17 @@ func newServiceConfig(options ServiceOptions) *serviceConfig {
 		},
 		V2Only: options.V2Only,
 	}
+	if err := config.LoadAllowNondistributableArtifacts(options.AllowNondistributableArtifacts); err != nil {
+		return nil, err
+	}
+	if err := config.LoadMirrors(options.Mirrors); err != nil {
+		return nil, err
+	}
+	if err := config.LoadInsecureRegistries(options.InsecureRegistries); err != nil {
+		return nil, err
+	}
 
-	config.LoadAllowNondistributableArtifacts(options.AllowNondistributableArtifacts)
+	return config, nil
-	config.LoadMirrors(options.Mirrors)
-	config.LoadInsecureRegistries(options.InsecureRegistries)
-
-	return config
 }
 
 // LoadAllowNondistributableArtifacts loads allow-nondistributable-artifacts registries into config.

registry/config_test.go

@@ -5,6 +5,8 @@ import (
 	"sort"
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestLoadAllowNondistributableArtifacts(t *testing.T) {
@@ -90,7 +92,7 @@ func TestLoadAllowNondistributableArtifacts(t *testing.T) {
 		},
 	}
 	for _, testCase := range testCases {
-		config := newServiceConfig(ServiceOptions{})
+		config := emptyServiceConfig
 		err := config.LoadAllowNondistributableArtifacts(testCase.registries)
 		if testCase.err == "" {
 			if err != nil {
@@ -233,7 +235,7 @@ func TestLoadInsecureRegistries(t *testing.T) {
 		},
 	}
 	for _, testCase := range testCases {
-		config := newServiceConfig(ServiceOptions{})
+		config := emptyServiceConfig
 		err := config.LoadInsecureRegistries(testCase.registries)
 		if testCase.err == "" {
 			if err != nil {
@@ -258,3 +260,60 @@ func TestLoadInsecureRegistries(t *testing.T) {
 		}
 	}
 }
+
+func TestNewServiceConfig(t *testing.T) {
+	testCases := []struct {
+		opts   ServiceOptions
+		errStr string
+	}{
+		{
+			ServiceOptions{},
+			"",
+		},
+		{
+			ServiceOptions{
+				Mirrors: []string{"example.com:5000"},
+			},
+			`invalid mirror: unsupported scheme "example.com" in "example.com:5000"`,
+		},
+		{
+			ServiceOptions{
+				Mirrors: []string{"http://example.com:5000"},
+			},
+			"",
+		},
+		{
+			ServiceOptions{
+				InsecureRegistries: []string{"[fe80::]/64"},
+			},
+			`insecure registry [fe80::]/64 is not valid: invalid host "[fe80::]/64"`,
+		},
+		{
+			ServiceOptions{
+				InsecureRegistries: []string{"102.10.8.1/24"},
+			},
+			"",
+		},
+		{
+			ServiceOptions{
+				AllowNondistributableArtifacts: []string{"[fe80::]/64"},
+			},
+			`allow-nondistributable-artifacts registry [fe80::]/64 is not valid: invalid host "[fe80::]/64"`,
+		},
+		{
+			ServiceOptions{
+				AllowNondistributableArtifacts: []string{"102.10.8.1/24"},
+			},
+			"",
+		},
+	}
+
+	for _, testCase := range testCases {
+		_, err := newServiceConfig(testCase.opts)
+		if testCase.errStr != "" {
+			assert.EqualError(t, err, testCase.errStr)
+		} else {
+			assert.Nil(t, err)
+		}
+	}
+}

registry/registry_mock_test.go

@@ -175,7 +175,7 @@ func makePublicIndex() *registrytypes.IndexInfo {
 	return index
 }
 
-func makeServiceConfig(mirrors []string, insecureRegistries []string) *serviceConfig {
+func makeServiceConfig(mirrors []string, insecureRegistries []string) (*serviceConfig, error) {
 	options := ServiceOptions{
 		Mirrors:            mirrors,
 		InsecureRegistries: insecureRegistries,

registry/registry_test.go

@@ -540,7 +540,7 @@ func TestNewIndexInfo(t *testing.T) {
 		}
 	}
 
-	config := newServiceConfig(ServiceOptions{})
+	config := emptyServiceConfig
 	noMirrors := []string{}
 	expectedIndexInfos := map[string]*registrytypes.IndexInfo{
 		IndexName: {
@@ -571,7 +571,11 @@ func TestNewIndexInfo(t *testing.T) {
 	testIndexInfo(config, expectedIndexInfos)
 
 	publicMirrors := []string{"http://mirror1.local", "http://mirror2.local"}
-	config = makeServiceConfig(publicMirrors, []string{"example.com"})
+	var err error
+	config, err = makeServiceConfig(publicMirrors, []string{"example.com"})
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	expectedIndexInfos = map[string]*registrytypes.IndexInfo{
 		IndexName: {
@@ -619,7 +623,10 @@ func TestNewIndexInfo(t *testing.T) {
 	}
 	testIndexInfo(config, expectedIndexInfos)
 
-	config = makeServiceConfig(nil, []string{"42.42.0.0/16"})
+	config, err = makeServiceConfig(nil, []string{"42.42.0.0/16"})
+	if err != nil {
+		t.Fatal(err)
+	}
 	expectedIndexInfos = map[string]*registrytypes.IndexInfo{
 		"example.com": {
 			Name:     "example.com",
@@ -664,7 +671,11 @@ func TestMirrorEndpointLookup(t *testing.T) {
 		}
 		return false
 	}
-	s := DefaultService{config: makeServiceConfig([]string{"https://my.mirror"}, nil)}
+	cfg, err := makeServiceConfig([]string{"https://my.mirror"}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	s := DefaultService{config: cfg}
 
 	imageName, err := reference.WithName(IndexName + "/test/image")
 	if err != nil {
@@ -841,9 +852,12 @@ func TestAllowNondistributableArtifacts(t *testing.T) {
 		{"invalid.domain.com:5000", []string{"invalid.domain.com:5000"}, true},
 	}
 	for _, tt := range tests {
-		config := newServiceConfig(ServiceOptions{
+		config, err := newServiceConfig(ServiceOptions{
 			AllowNondistributableArtifacts: tt.registries,
 		})
+		if err != nil {
+			t.Error(err)
+		}
 		if v := allowNondistributableArtifacts(config, tt.addr); v != tt.expected {
 			t.Errorf("allowNondistributableArtifacts failed for %q %v, expected %v got %v", tt.addr, tt.registries, tt.expected, v)
 		}
@@ -883,7 +897,10 @@ func TestIsSecureIndex(t *testing.T) {
 		{"invalid.domain.com:5000", []string{"invalid.domain.com:5000"}, false},
 	}
 	for _, tt := range tests {
-		config := makeServiceConfig(nil, tt.insecureRegistries)
+		config, err := makeServiceConfig(nil, tt.insecureRegistries)
+		if err != nil {
+			t.Error(err)
+		}
 		if sec := isSecureIndex(config, tt.addr); sec != tt.expected {
 			t.Errorf("isSecureIndex failed for %q %v, expected %v got %v", tt.addr, tt.insecureRegistries, tt.expected, sec)
 		}

registry/service.go

@@ -45,10 +45,10 @@ type DefaultService struct {
 
 // NewService returns a new instance of DefaultService ready to be
 // installed into an engine.
-func NewService(options ServiceOptions) *DefaultService {
+func NewService(options ServiceOptions) (*DefaultService, error) {
-	return &DefaultService{
+	config, err := newServiceConfig(options)
-		config: newServiceConfig(options),
+
-	}
+	return &DefaultService{config: config}, err
 }
 
 // ServiceConfig returns the public registry service configuration.

registry/service_v1_test.go

@@ -3,7 +3,10 @@ package registry
 import "testing"
 
 func TestLookupV1Endpoints(t *testing.T) {
-	s := NewService(ServiceOptions{})
+	s, err := NewService(ServiceOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	cases := []struct {
 		hostname    string