Skip to content

Commit

Permalink
Fix configurations with no server provided
Browse files Browse the repository at this point in the history
When a server is specified at the top level, there is a bug
that prevents the keys from being checked properly.
When no server is provided, the server attempts to parse
with an empty host, leaving partial values and a defaulted
skip verify configuration.

Signed-off-by: Derek McGowan <derek@mcg.dev>
  • Loading branch information
dmcgowan committed May 20, 2020
1 parent 06b0cd4 commit 84619ee
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 31 deletions.
53 changes: 26 additions & 27 deletions remotes/docker/config/hosts.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ type hostFileConfig struct {
// TODO: Make this an array (two key types, one for pairs (multiple files), one for single file?)
Client toml.Primitive `toml:"client"`

SkipVerify bool `toml:"skip_verify"`
SkipVerify *bool `toml:"skip_verify"`

// API (default: "docker")
// API Version (default: "v2")
Expand Down Expand Up @@ -322,33 +322,32 @@ func parseHostsFile(ctx context.Context, baseDir string, b []byte) ([]hostConfig
for i, server := range orderedHosts {
hostConfig := c.HostConfigs[server]

if !strings.HasPrefix(server, "http") {
server = "https://" + server
}
u, err := url.Parse(server)
if err != nil {
return nil, errors.Errorf("unable to parse server %v", server)
}
hosts[i].scheme = u.Scheme
hosts[i].host = u.Host

// TODO: Handle path based on registry protocol
// Define a registry protocol type
// OCI v1 - Always use given path as is
// Docker v2 - Always ensure ends with /v2/
if len(u.Path) > 0 {
u.Path = path.Clean(u.Path)
if !strings.HasSuffix(u.Path, "/v2") {
u.Path = u.Path + "/v2"
if server != "" {
if !strings.HasPrefix(server, "http") {
server = "https://" + server
}
} else {
u.Path = "/v2"
}
hosts[i].path = u.Path

if hosts[i].scheme == "https" {
hosts[i].skipVerify = &hostConfig.SkipVerify
u, err := url.Parse(server)
if err != nil {
return nil, errors.Errorf("unable to parse server %v", server)
}
hosts[i].scheme = u.Scheme
hosts[i].host = u.Host

// TODO: Handle path based on registry protocol
// Define a registry protocol type
// OCI v1 - Always use given path as is
// Docker v2 - Always ensure ends with /v2/
if len(u.Path) > 0 {
u.Path = path.Clean(u.Path)
if !strings.HasSuffix(u.Path, "/v2") {
u.Path = u.Path + "/v2"
}
} else {
u.Path = "/v2"
}
hosts[i].path = u.Path
}
hosts[i].skipVerify = hostConfig.SkipVerify

if len(hostConfig.Capabilities) > 0 {
for _, c := range hostConfig.Capabilities {
Expand All @@ -368,7 +367,7 @@ func parseHostsFile(ctx context.Context, baseDir string, b []byte) ([]hostConfig
}

baseKey := []string{}
if server != "" {
if server != "" && server != c.Server {
baseKey = append(baseKey, "host", server)
}
caKey := append(baseKey, "ca")
Expand Down
6 changes: 2 additions & 4 deletions remotes/docker/config/hosts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ ca = "/etc/path/default"
[host."https://mirror.registry"]
capabilities = ["pull"]
ca = "/etc/certs/mirror.pem"
skip_verify = false
[host."https://mirror-bak.registry/us"]
capabilities = ["pull"]
Expand Down Expand Up @@ -132,7 +133,6 @@ ca = "/etc/path/default"
{filepath.FromSlash("/etc/certs/client.cert"), filepath.FromSlash("/etc/certs/client.key")},
{filepath.FromSlash("/etc/certs/client.pem"), ""},
},
skipVerify: &fb,
},
{
scheme: "https",
Expand All @@ -142,7 +142,6 @@ ca = "/etc/path/default"
clientPairs: [][2]string{
{filepath.FromSlash("/etc/certs/client.pem")},
},
skipVerify: &fb,
},
{
scheme: "https",
Expand All @@ -153,14 +152,13 @@ ca = "/etc/path/default"
{filepath.FromSlash("/etc/certs/client-1.pem")},
{filepath.FromSlash("/etc/certs/client-2.pem")},
},
skipVerify: &fb,
},
{
scheme: "https",
host: "test-default.registry",
path: "/v2",
capabilities: allCaps,
skipVerify: &fb,
caCerts: []string{filepath.FromSlash("/etc/path/default")},
},
}
hosts, err := parseHostsFile(ctx, "", []byte(testtoml))
Expand Down

0 comments on commit 84619ee

Please sign in to comment.