Skip to content

Commit

Permalink
fix: introduce 'overridePath' setting and fix Talos resolver
Browse files Browse the repository at this point in the history 
There was inconsistency in the way `/v2` was appended to registry
endpoint path between containerd (CRI) and Talos:

* Talos only appended `/v2` to empty paths
* containerd appended `/v2` if it's not the suffix already

Fix Talos to act same as containerd, and introduce a setting
`overridePath` which stops both Talos and `containerd` from appending
`/v2` (should be required with e.g. Harbor registry mirror).

Signed-off-by: Andrey Smirnov <andrey.smirnov@talos-systems.com>
  • Loading branch information
@smira
smira committed Dec 5, 2022
1 parent 0219d11 commit 5b2960e
Show file tree
Hide file tree
Showing 11 changed files with 292 additions and 78 deletions.
30 changes: 30 additions & 0 deletions hack/release.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,36 @@ talosctl machineconfig patch controlplane.yaml \
```
Additionally, `talosctl machineconfig gen` subcommand is introduced as an alias to `talosctl gen config`.
"""

[notes.registry-mirrors]
title = "Registry Mirrors"
description = """\
Talos had an inconsistency in the way registry mirror endpoints are handled when compared with `containerd` implementation:
```yaml
machine:
registries:
mirrors:
docker.io:
endpoints:
- "https://mirror-registry/v2/mirror.docker.io"
```
Talos would use endpoint `https://mirror-registry/v2/mirror.docker.io`, while `containerd` would use `https://mirror-registry/v2/mirror.docker.io/v2`.
This inconsistency is now fixed, and Talos uses same endpoint as `containerd`.
New `overridePath` configuration is introduced to skip appending `/v2` both on Talos and containerd side:
```yaml
machine:
registries:
mirrors:
docker.io:
endpoints:
- "https://mirror-registry/v2/mirror.docker.io"
overridePath: true
```
"""

[make_deps]
Expand Down
25 changes: 13 additions & 12 deletions internal/pkg/containers/cri/containerd/hosts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,44 +47,44 @@ func GenerateHosts(cfg config.Registries, basePath string) (*HostsConfig, error)
Directories: map[string]*HostsDirectory{},
}

configureTLS := func(host string, directoryName string, hostToml *HostToml, directory *HostsDirectory) {
tlsConfig, ok := cfg.Config()[host]
configureEndpoint := func(host string, directoryName string, hostToml *HostToml, directory *HostsDirectory) {
endpointConfig, ok := cfg.Config()[host]
if !ok {
return
}

if tlsConfig.TLS() != nil {
if tlsConfig.TLS().InsecureSkipVerify() {
if endpointConfig.TLS() != nil {
if endpointConfig.TLS().InsecureSkipVerify() {
hostToml.SkipVerify = true
}

if tlsConfig.TLS().CA() != nil {
if endpointConfig.TLS().CA() != nil {
relPath := fmt.Sprintf("%s-ca.crt", host)

directory.Files = append(directory.Files,
&HostsFile{
Name: relPath,
Contents: tlsConfig.TLS().CA(),
Contents: endpointConfig.TLS().CA(),
Mode: 0o600,
},
)

hostToml.CACert = filepath.Join(basePath, directoryName, relPath)
}

if tlsConfig.TLS().ClientIdentity() != nil {
if endpointConfig.TLS().ClientIdentity() != nil {
relPathCrt := fmt.Sprintf("%s-client.crt", host)
relPathKey := fmt.Sprintf("%s-client.key", host)

directory.Files = append(directory.Files,
&HostsFile{
Name: relPathCrt,
Contents: tlsConfig.TLS().ClientIdentity().Crt,
Contents: endpointConfig.TLS().ClientIdentity().Crt,
Mode: 0o600,
},
&HostsFile{
Name: relPathKey,
Contents: tlsConfig.TLS().ClientIdentity().Key,
Contents: endpointConfig.TLS().ClientIdentity().Key,
Mode: 0o600,
},
)
Expand Down Expand Up @@ -122,9 +122,10 @@ func GenerateHosts(cfg config.Registries, basePath string) (*HostsConfig, error)

hostsToml.HostConfigs[endpoint] = &HostToml{
Capabilities: []string{"pull", "resolve"}, // TODO: we should make it configurable eventually
OverridePath: endpoints.OverridePath(),
}

configureTLS(u.Host, directoryName, hostsToml.HostConfigs[endpoint], directory)
configureEndpoint(u.Host, directoryName, hostsToml.HostConfigs[endpoint], directory)

tomlBytes, err := toml.Marshal(hostsToml)
if err != nil {
Expand Down Expand Up @@ -192,13 +193,12 @@ func GenerateHosts(cfg config.Registries, basePath string) (*HostsConfig, error)
defaultHost = "https://" + defaultHost

hostsToml := HostsToml{
Server: defaultHost,
HostConfigs: map[string]*HostToml{
defaultHost: {},
},
}

configureTLS(hostname, directoryName, hostsToml.HostConfigs[defaultHost], directory)
configureEndpoint(hostname, directoryName, hostsToml.HostConfigs[defaultHost], directory)

marshaled, err := toml.Marshal(hostsToml)
if err != nil {
Expand Down Expand Up @@ -238,6 +238,7 @@ type HostsToml struct {
// HostToml is a single entry in `hosts.toml`.
type HostToml struct {
Capabilities []string `toml:"capabilities,omitempty"`
OverridePath bool `toml:"override_path,omitempty"`
CACert string `toml:"ca,omitempty"`
Client [][2]string `toml:"client,omitempty"`
SkipVerify bool `toml:"skip_verify,omitempty"`
Expand Down
90 changes: 80 additions & 10 deletions internal/pkg/containers/cri/containerd/hosts_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import (
"github.com/siderolabs/talos/pkg/machinery/config/types/v1alpha1"
)

func TestGenerateHosts(t *testing.T) {
cfgWithTLS := &mockConfig{
func TestGenerateHostsWithTLS(t *testing.T) {
cfg := &mockConfig{
mirrors: map[string]*v1alpha1.RegistryMirrorConfig{
"docker.io": {
MirrorEndpoints: []string{"https://registry-1.docker.io", "https://registry-2.docker.io"},
Expand Down Expand Up @@ -49,7 +49,7 @@ func TestGenerateHosts(t *testing.T) {
},
}

resultWithTLS, err := containerd.GenerateHosts(cfgWithTLS, "/etc/cri/conf.d/hosts")
result, err := containerd.GenerateHosts(cfg, "/etc/cri/conf.d/hosts")
require.NoError(t, err)

assert.Equal(t, &containerd.HostsConfig{
Expand Down Expand Up @@ -83,7 +83,7 @@ func TestGenerateHosts(t *testing.T) {
{
Name: "hosts.toml",
Mode: 0o600,
Contents: []byte("server = \"https://some.host:123\"\n\n[host]\n\n [host.\"https://some.host:123\"]\n ca = \"/etc/cri/conf.d/hosts/some.host_123_/some.host:123-ca.crt\"\n client = [[\"/etc/cri/conf.d/hosts/some.host_123_/some.host:123-client.crt\", \"/etc/cri/conf.d/hosts/some.host_123_/some.host:123-client.key\"]]\n skip_verify = true\n"), //nolint:lll
Contents: []byte("\n[host]\n\n [host.\"https://some.host:123\"]\n ca = \"/etc/cri/conf.d/hosts/some.host_123_/some.host:123-ca.crt\"\n client = [[\"/etc/cri/conf.d/hosts/some.host_123_/some.host:123-client.crt\", \"/etc/cri/conf.d/hosts/some.host_123_/some.host:123-client.key\"]]\n skip_verify = true\n"), //nolint:lll
},
},
},
Expand All @@ -92,14 +92,16 @@ func TestGenerateHosts(t *testing.T) {
{
Name: "hosts.toml",
Mode: 0o600,
Contents: []byte("server = \"https://registry-2.docker.io\"\n\n[host]\n\n [host.\"https://registry-2.docker.io\"]\n skip_verify = true\n"),
Contents: []byte("\n[host]\n\n [host.\"https://registry-2.docker.io\"]\n skip_verify = true\n"),
},
},
},
},
}, resultWithTLS)
}, result)
}

cfgWithoutTLS := &mockConfig{
func TestGenerateHostsWithoutTLS(t *testing.T) {
cfg := &mockConfig{
mirrors: map[string]*v1alpha1.RegistryMirrorConfig{
"docker.io": {
MirrorEndpoints: []string{"https://registry-1.docker.io", "https://registry-2.docker.io"},
Expand All @@ -117,7 +119,7 @@ func TestGenerateHosts(t *testing.T) {
},
}

resultWithoutTLS, err := containerd.GenerateHosts(cfgWithoutTLS, "/etc/cri/conf.d/hosts")
result, err := containerd.GenerateHosts(cfg, "/etc/cri/conf.d/hosts")
require.NoError(t, err)

assert.Equal(t, &containerd.HostsConfig{
Expand All @@ -136,10 +138,78 @@ func TestGenerateHosts(t *testing.T) {
{
Name: "hosts.toml",
Mode: 0o600,
Contents: []byte("server = \"https://some.host:123\"\n\n[host]\n\n [host.\"https://some.host:123\"]\n"),
Contents: []byte("\n[host]\n\n [host.\"https://some.host:123\"]\n"),
},
},
},
},
}, result)
}

func TestGenerateHostsWithHarbor(t *testing.T) {
cfg := &mockConfig{
mirrors: map[string]*v1alpha1.RegistryMirrorConfig{
"docker.io": {
MirrorEndpoints: []string{"https://harbor/v2/mirrors/proxy.docker.io"},
MirrorOverridePath: pointer.To(true),
},
"ghcr.io": {
MirrorEndpoints: []string{"https://harbor/v2/mirrors/proxy.ghcr.io"},
MirrorOverridePath: pointer.To(true),
},
},
config: map[string]*v1alpha1.RegistryConfig{
"harbor": {
RegistryAuth: &v1alpha1.RegistryAuthConfig{
RegistryUsername: "root",
RegistryPassword: "secret",
RegistryAuth: "auth",
RegistryIdentityToken: "token",
},
RegistryTLS: &v1alpha1.RegistryTLSConfig{
TLSInsecureSkipVerify: pointer.To(true),
},
},
},
}

result, err := containerd.GenerateHosts(cfg, "/etc/cri/conf.d/hosts")
require.NoError(t, err)

t.Logf(
"config %q",
string(result.Directories["harbor"].Files[0].Contents),
)

assert.Equal(t, &containerd.HostsConfig{
Directories: map[string]*containerd.HostsDirectory{
"docker.io": {
Files: []*containerd.HostsFile{
{
Name: "hosts.toml",
Mode: 0o600,
Contents: []byte("\n[host]\n\n [host.\"https://harbor/v2/mirrors/proxy.docker.io\"]\n capabilities = [\"pull\", \"resolve\"]\n override_path = true\n skip_verify = true\n"),
},
},
},
"ghcr.io": {
Files: []*containerd.HostsFile{
{
Name: "hosts.toml",
Mode: 0o600,
Contents: []byte("\n[host]\n\n [host.\"https://harbor/v2/mirrors/proxy.ghcr.io\"]\n capabilities = [\"pull\", \"resolve\"]\n override_path = true\n skip_verify = true\n"),
},
},
},
"harbor": {
Files: []*containerd.HostsFile{
{
Name: "hosts.toml",
Mode: 0o600,
Contents: []byte("\n[host]\n\n [host.\"https://harbor\"]\n skip_verify = true\n"),
},
},
},
},
}, resultWithoutTLS)
}, result)
}
41 changes: 22 additions & 19 deletions internal/pkg/containers/image/resolver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"net/http"
"net/url"
"path"
"strings"

"github.com/containerd/containerd/remotes"
Expand All @@ -33,7 +34,7 @@ func RegistryHosts(reg config.Registries) docker.RegistryHosts {
return func(host string) ([]docker.RegistryHost, error) {
var registries []docker.RegistryHost

endpoints, err := RegistryEndpoints(reg, host)
endpoints, overridePath, err := RegistryEndpoints(reg, host)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -61,7 +62,15 @@ func RegistryHosts(reg config.Registries) docker.RegistryHosts {
}

if u.Path == "" {
u.Path = "/v2"
if !overridePath {
u.Path = "/v2"
}
} else {
u.Path = path.Clean(u.Path)

if !strings.HasSuffix(u.Path, "/v2") && !overridePath {
u.Path += "/v2"
}
}

uu := u
Expand Down Expand Up @@ -89,30 +98,24 @@ func RegistryHosts(reg config.Registries) docker.RegistryHosts {
}

// RegistryEndpoints returns registry endpoints per host using reg.
func RegistryEndpoints(reg config.Registries, host string) ([]string, error) {
var endpoints []string

func RegistryEndpoints(reg config.Registries, host string) (endpoints []string, overridePath bool, err error) {
// direct hit by host
if hostConfig, ok := reg.Mirrors()[host]; ok {
endpoints = hostConfig.Endpoints()
return hostConfig.Endpoints(), hostConfig.OverridePath(), nil
}

if endpoints == nil {
if catchAllConfig, ok := reg.Mirrors()["*"]; ok {
endpoints = catchAllConfig.Endpoints()
}
// '*'
if catchAllConfig, ok := reg.Mirrors()["*"]; ok {
return catchAllConfig.Endpoints(), catchAllConfig.OverridePath(), nil
}

if len(endpoints) == 0 {
// still no endpoints, use default
defaultHost, err := docker.DefaultHost(host)
if err != nil {
return nil, fmt.Errorf("error getting default host for %q: %w", host, err)
}

endpoints = append(endpoints, "https://"+defaultHost)
// still no endpoints, use default
defaultHost, err := docker.DefaultHost(host)
if err != nil {
return nil, false, fmt.Errorf("error getting default host for %q: %w", host, err)
}

return endpoints, nil
return []string{"https://" + defaultHost}, false, nil
}

// PrepareAuth returns authentication info in the format expected by containerd.
Expand Down
Loading

0 comments on commit 5b2960e

Please sign in to comment.