diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index 0a3a0be388..0153bb3a43 100644 --- a/cmd/dockerd/daemon.go +++ b/cmd/dockerd/daemon.go @@ -198,7 +198,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 diff --git a/daemon/reload_test.go b/daemon/reload_test.go index bf11b6bd56..c01d39c4e1 100644 --- a/daemon/reload_test.go +++ b/daemon/reload_test.go @@ -46,8 +46,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", @@ -56,6 +57,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", @@ -98,7 +102,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", @@ -106,6 +111,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{} @@ -191,8 +199,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", @@ -201,6 +210,9 @@ func TestDaemonReloadInsecureRegistries(t *testing.T) { "docker2.com", // this will be removed when reloading }, }) + if err != nil { + t.Fatal(err) + } daemon.configStore = &config.Config{} diff --git a/integration-cli/fixtures/plugin/plugin_linux.go b/integration-cli/fixtures/plugin/plugin_linux.go index 757694cd37..5da79fcb77 100644 --- a/integration-cli/fixtures/plugin/plugin_linux.go +++ b/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{}, diff --git a/registry/config.go b/registry/config.go index 70efb4330f..34d51048f0 100644 --- a/registry/config.go +++ b/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) - config.LoadMirrors(options.Mirrors) - config.LoadInsecureRegistries(options.InsecureRegistries) - - return config + return config, nil } // LoadAllowNondistributableArtifacts loads allow-nondistributable-artifacts registries into config. diff --git a/registry/config_test.go b/registry/config_test.go index 8cb7e5a543..0899e17f67 100644 --- a/registry/config_test.go +++ b/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) + } + } +} diff --git a/registry/registry_mock_test.go b/registry/registry_mock_test.go index 204a98344e..cf1cd19c1c 100644 --- a/registry/registry_mock_test.go +++ b/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, diff --git a/registry/registry_test.go b/registry/registry_test.go index d89c46c2c0..0b4f576971 100644 --- a/registry/registry_test.go +++ b/registry/registry_test.go @@ -539,7 +539,7 @@ func TestNewIndexInfo(t *testing.T) { } } - config := newServiceConfig(ServiceOptions{}) + config := emptyServiceConfig noMirrors := []string{} expectedIndexInfos := map[string]*registrytypes.IndexInfo{ IndexName: { @@ -570,7 +570,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: { @@ -618,7 +622,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", @@ -663,7 +670,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 { @@ -844,9 +855,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) } @@ -886,7 +900,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) } diff --git a/registry/service.go b/registry/service.go index d36b11f0e1..a991a8fc39 100644 --- a/registry/service.go +++ b/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 { - return &DefaultService{ - config: newServiceConfig(options), - } +func NewService(options ServiceOptions) (*DefaultService, error) { + config, err := newServiceConfig(options) + + return &DefaultService{config: config}, err } // ServiceConfig returns the public registry service configuration. diff --git a/registry/service_v1_test.go b/registry/service_v1_test.go index bd15dfffb8..6ea846eed7 100644 --- a/registry/service_v1_test.go +++ b/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