diff --git a/CHANGELOG.md b/CHANGELOG.md index db60be4e..d38f0fc7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- [#601](https://github.com/spegel-org/spegel/pull/601) Fix Containerd host mirror ordering. + ### Security ## v0.0.25 diff --git a/go.mod b/go.mod index 5b40aead..b572158c 100644 --- a/go.mod +++ b/go.mod @@ -25,7 +25,6 @@ require ( github.com/stretchr/testify v1.9.0 go.etcd.io/bbolt v1.3.11 golang.org/x/sync v0.8.0 - golang.org/x/time v0.7.0 k8s.io/client-go v0.31.1 k8s.io/cri-api v0.31.1 k8s.io/klog/v2 v2.130.1 @@ -199,6 +198,7 @@ require ( golang.org/x/sys v0.22.0 // indirect golang.org/x/term v0.22.0 // indirect golang.org/x/text v0.16.0 // indirect + golang.org/x/time v0.7.0 // indirect golang.org/x/tools v0.23.0 // indirect gonum.org/v1/gonum v0.15.0 // indirect google.golang.org/genproto v0.0.0-20231211222908-989df2bf70f3 // indirect diff --git a/pkg/oci/containerd.go b/pkg/oci/containerd.go index 76885fed..c0aff67a 100644 --- a/pkg/oci/containerd.go +++ b/pkg/oci/containerd.go @@ -1,6 +1,7 @@ package oci import ( + "bytes" "context" "encoding/json" "errors" @@ -11,6 +12,7 @@ import ( "path" "path/filepath" "strings" + "text/template" semver "github.com/Masterminds/semver/v3" "github.com/containerd/containerd" @@ -22,6 +24,7 @@ import ( "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pelletier/go-toml/v2" + tomlu "github.com/pelletier/go-toml/v2/unstable" "github.com/spf13/afero" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1" @@ -330,20 +333,6 @@ func createFilters(registries []url.URL) (string, string) { return listFilter, eventFilter } -type hostFile struct { - HostConfigs map[string]hostConfig `toml:"host"` - Server string `toml:"server"` -} - -type hostConfig struct { - CACert interface{} `toml:"ca"` - Client interface{} `toml:"client"` - OverridePath *bool `toml:"override_path"` - SkipVerify *bool `toml:"skip_verify"` - Header map[string]interface{} `toml:"header"` - Capabilities []string `toml:"capabilities"` -} - // Refer to containerd registry configuration documentation for more information about required configuration. // https://github.com/containerd/containerd/blob/main/docs/cri/config.md#registry-configuration // https://github.com/containerd/containerd/blob/main/docs/hosts.md#registry-configuration---examples @@ -372,14 +361,11 @@ func AddMirrorConfiguration(ctx context.Context, fs afero.Fs, configPath string, capabilities = append(capabilities, "resolve") } for _, registryURL := range registryURLs { - hf, appending, err := getHostFile(fs, configPath, appendToBackup, registryURL) + existingHosts, err := existingHosts(fs, configPath, registryURL) if err != nil { return err } - for _, u := range mirrorURLs { - hf.HostConfigs[u.String()] = hostConfig{Capabilities: capabilities} - } - b, err := toml.Marshal(&hf) + templatedHosts, err := templateHosts(registryURL, mirrorURLs, capabilities, existingHosts) if err != nil { return err } @@ -388,11 +374,11 @@ func AddMirrorConfiguration(ctx context.Context, fs afero.Fs, configPath string, if err != nil { return err } - err = afero.WriteFile(fs, fp, b, 0o644) + err = afero.WriteFile(fs, fp, []byte(templatedHosts), 0o644) if err != nil { return err } - if appending { + if existingHosts != "" { log.Info("appending to existing Containerd mirror configuration", "registry", registryURL.String(), "path", fp) } else { log.Info("added Containerd mirror configuration", "registry", registryURL.String(), "path", fp) @@ -470,29 +456,96 @@ func clearConfig(fs afero.Fs, configPath string) error { return nil } -func getHostFile(fs afero.Fs, configPath string, appendToBackup bool, registryURL url.URL) (hostFile, bool, error) { - if appendToBackup { - fp := path.Join(configPath, backupDir, registryURL.Host, "hosts.toml") - b, err := afero.ReadFile(fs, fp) - if err != nil && !errors.Is(err, afero.ErrFileNotFound) { - return hostFile{}, false, err - } - if err == nil { - hf := hostFile{} - err := toml.Unmarshal(b, &hf) - if err != nil { - return hostFile{}, false, err - } - return hf, true, nil - } - } +func templateHosts(registryURL url.URL, mirrorURLs []url.URL, capabilities []string, existingHosts string) (string, error) { server := registryURL.String() if registryURL.String() == "https://docker.io" { server = "https://registry-1.docker.io" } - hf := hostFile{ - Server: server, - HostConfigs: map[string]hostConfig{}, + capabilitiesStr := strings.Join(capabilities, "', '") + capabilitiesStr = fmt.Sprintf("['%s']", capabilitiesStr) + hc := struct { + Server string + Capabilities string + MirrorURLs []url.URL + }{ + Server: server, + Capabilities: capabilitiesStr, + MirrorURLs: mirrorURLs, + } + tmpl, err := template.New("").Parse(`server = '{{ .Server }}' +{{ range .MirrorURLs }} +[host.'{{ .String }}'] +capabilities = {{ $.Capabilities }} +{{ end }}`) + if err != nil { + return "", err + } + buf := bytes.NewBuffer(nil) + err = tmpl.Execute(buf, hc) + if err != nil { + return "", err + } + output := strings.TrimSpace(buf.String()) + if existingHosts != "" { + output = output + "\n\n" + existingHosts + } + return output, nil +} + +type hostFile struct { + Hosts map[string]interface{} `toml:"host"` +} + +func existingHosts(fs afero.Fs, configPath string, registryURL url.URL) (string, error) { + fp := path.Join(configPath, backupDir, registryURL.Host, "hosts.toml") + b, err := afero.ReadFile(fs, fp) + if errors.Is(err, afero.ErrFileNotFound) { + return "", nil + } + if err != nil { + return "", err + } + + var hf hostFile + err = toml.Unmarshal(b, &hf) + if err != nil { + return "", err + } + if len(hf.Hosts) == 0 { + return "", nil + } + + hosts := []string{} + parser := tomlu.Parser{} + parser.Reset(b) + for parser.NextExpression() { + err := parser.Error() + if err != nil { + return "", err + } + e := parser.Expression() + if e.Kind != tomlu.Table { + continue + } + ki := e.Key() + if ki.Next() && string(ki.Node().Data) == "host" && ki.Next() && ki.IsLast() { + hosts = append(hosts, string(ki.Node().Data)) + } + } + + ehs := []string{} + for _, h := range hosts { + data := hostFile{ + Hosts: map[string]interface{}{ + h: hf.Hosts[h], + }, + } + b, err := toml.Marshal(data) + if err != nil { + return "", err + } + eh := strings.TrimPrefix(string(b), "[host]\n") + ehs = append(ehs, eh) } - return hf, false, nil + return strings.TrimSpace(strings.Join(ehs, "\n")), nil } diff --git a/pkg/oci/containerd_test.go b/pkg/oci/containerd_test.go index e1b60974..bff1fcd7 100644 --- a/pkg/oci/containerd_test.go +++ b/pkg/oci/containerd_test.go @@ -5,6 +5,7 @@ import ( "fmt" iofs "io/fs" "net/url" + "path/filepath" "testing" eventtypes "github.com/containerd/containerd/api/events" @@ -211,17 +212,18 @@ func TestMirrorConfiguration(t *testing.T) { name: "multiple mirros", resolveTags: true, registries: stringListToUrlList(t, []string{"http://foo.bar:5000"}), - mirrors: stringListToUrlList(t, []string{"http://127.0.0.1:5000", "http://127.0.0.1:5001"}), + mirrors: stringListToUrlList(t, []string{"http://127.0.0.1:5000", "http://127.0.0.2:5000", "http://127.0.0.1:5001"}), expectedFiles: map[string]string{ "/etc/containerd/certs.d/foo.bar:5000/hosts.toml": `server = 'http://foo.bar:5000' -[host] [host.'http://127.0.0.1:5000'] capabilities = ['pull', 'resolve'] -[host.'http://127.0.0.1:5001'] +[host.'http://127.0.0.2:5000'] capabilities = ['pull', 'resolve'] -`, + +[host.'http://127.0.0.1:5001'] +capabilities = ['pull', 'resolve']`, }, }, { @@ -232,16 +234,12 @@ capabilities = ['pull', 'resolve'] expectedFiles: map[string]string{ "/etc/containerd/certs.d/docker.io/hosts.toml": `server = 'https://registry-1.docker.io' -[host] [host.'http://127.0.0.1:5000'] -capabilities = ['pull'] -`, +capabilities = ['pull']`, "/etc/containerd/certs.d/foo.bar:5000/hosts.toml": `server = 'http://foo.bar:5000' -[host] [host.'http://127.0.0.1:5000'] -capabilities = ['pull'] -`, +capabilities = ['pull']`, }, }, { @@ -253,16 +251,12 @@ capabilities = ['pull'] expectedFiles: map[string]string{ "/etc/containerd/certs.d/docker.io/hosts.toml": `server = 'https://registry-1.docker.io' -[host] [host.'http://127.0.0.1:5000'] -capabilities = ['pull', 'resolve'] -`, +capabilities = ['pull', 'resolve']`, "/etc/containerd/certs.d/foo.bar:5000/hosts.toml": `server = 'http://foo.bar:5000' -[host] [host.'http://127.0.0.1:5000'] -capabilities = ['pull', 'resolve'] -`, +capabilities = ['pull', 'resolve']`, }, }, { @@ -274,16 +268,12 @@ capabilities = ['pull', 'resolve'] expectedFiles: map[string]string{ "/etc/containerd/certs.d/docker.io/hosts.toml": `server = 'https://registry-1.docker.io' -[host] [host.'http://127.0.0.1:5000'] -capabilities = ['pull', 'resolve'] -`, +capabilities = ['pull', 'resolve']`, "/etc/containerd/certs.d/foo.bar:5000/hosts.toml": `server = 'http://foo.bar:5000' -[host] [host.'http://127.0.0.1:5000'] -capabilities = ['pull', 'resolve'] -`, +capabilities = ['pull', 'resolve']`, }, }, { @@ -293,24 +283,20 @@ capabilities = ['pull', 'resolve'] mirrors: stringListToUrlList(t, []string{"http://127.0.0.1:5000"}), createConfigPathDir: true, existingFiles: map[string]string{ - "/etc/containerd/certs.d/docker.io/hosts.toml": "Hello World", - "/etc/containerd/certs.d/ghcr.io/hosts.toml": "Foo Bar", + "/etc/containerd/certs.d/docker.io/hosts.toml": "hello = 'world'", + "/etc/containerd/certs.d/ghcr.io/hosts.toml": "foo = 'bar'", }, expectedFiles: map[string]string{ - "/etc/containerd/certs.d/_backup/docker.io/hosts.toml": "Hello World", - "/etc/containerd/certs.d/_backup/ghcr.io/hosts.toml": "Foo Bar", + "/etc/containerd/certs.d/_backup/docker.io/hosts.toml": "hello = 'world'", + "/etc/containerd/certs.d/_backup/ghcr.io/hosts.toml": "foo = 'bar'", "/etc/containerd/certs.d/docker.io/hosts.toml": `server = 'https://registry-1.docker.io' -[host] [host.'http://127.0.0.1:5000'] -capabilities = ['pull', 'resolve'] -`, +capabilities = ['pull', 'resolve']`, "/etc/containerd/certs.d/foo.bar:5000/hosts.toml": `server = 'http://foo.bar:5000' -[host] [host.'http://127.0.0.1:5000'] -capabilities = ['pull', 'resolve'] -`, +capabilities = ['pull', 'resolve']`, }, }, { @@ -320,26 +306,22 @@ capabilities = ['pull', 'resolve'] mirrors: stringListToUrlList(t, []string{"http://127.0.0.1:5000"}), createConfigPathDir: true, existingFiles: map[string]string{ - "/etc/containerd/certs.d/_backup/docker.io/hosts.toml": "Hello World", - "/etc/containerd/certs.d/_backup/ghcr.io/hosts.toml": "Foo Bar", + "/etc/containerd/certs.d/_backup/docker.io/hosts.toml": "hello = 'world'", + "/etc/containerd/certs.d/_backup/ghcr.io/hosts.toml": "foo = 'bar'", "/etc/containerd/certs.d/test.txt": "test", "/etc/containerd/certs.d/foo": "bar", }, expectedFiles: map[string]string{ - "/etc/containerd/certs.d/_backup/docker.io/hosts.toml": "Hello World", - "/etc/containerd/certs.d/_backup/ghcr.io/hosts.toml": "Foo Bar", + "/etc/containerd/certs.d/_backup/docker.io/hosts.toml": "hello = 'world'", + "/etc/containerd/certs.d/_backup/ghcr.io/hosts.toml": "foo = 'bar'", "/etc/containerd/certs.d/docker.io/hosts.toml": `server = 'https://registry-1.docker.io' -[host] [host.'http://127.0.0.1:5000'] -capabilities = ['pull', 'resolve'] -`, +capabilities = ['pull', 'resolve']`, "/etc/containerd/certs.d/foo.bar:5000/hosts.toml": `server = 'http://foo.bar:5000' -[host] [host.'http://127.0.0.1:5000'] -capabilities = ['pull', 'resolve'] -`, +capabilities = ['pull', 'resolve']`, }, }, { @@ -352,48 +334,52 @@ capabilities = ['pull', 'resolve'] existingFiles: map[string]string{ "/etc/containerd/certs.d/docker.io/hosts.toml": `server = 'https://registry-1.docker.io' -[host] [host.'http://example.com:30020'] capabilities = ['pull', 'resolve'] client = ['/etc/certs/xxx/client.cert', '/etc/certs/xxx/client.key'] [host.'http://example.com:30021'] -capabilities = ['pull', 'resolve'] client = ['/etc/certs/xxx/client.cert', '/etc/certs/xxx/client.key'] -`, +capabilities = ['pull', 'resolve'] + +[host.'http://bar.com:30020'] +capabilities = ['pull', 'resolve'] +client = ['/etc/certs/xxx/client.cert', '/etc/certs/xxx/client.key']`, }, expectedFiles: map[string]string{ "/etc/containerd/certs.d/_backup/docker.io/hosts.toml": `server = 'https://registry-1.docker.io' -[host] [host.'http://example.com:30020'] capabilities = ['pull', 'resolve'] client = ['/etc/certs/xxx/client.cert', '/etc/certs/xxx/client.key'] [host.'http://example.com:30021'] -capabilities = ['pull', 'resolve'] client = ['/etc/certs/xxx/client.cert', '/etc/certs/xxx/client.key'] -`, +capabilities = ['pull', 'resolve'] + +[host.'http://bar.com:30020'] +capabilities = ['pull', 'resolve'] +client = ['/etc/certs/xxx/client.cert', '/etc/certs/xxx/client.key']`, "/etc/containerd/certs.d/docker.io/hosts.toml": `server = 'https://registry-1.docker.io' -[host] [host.'http://127.0.0.1:5000'] capabilities = ['pull', 'resolve'] [host.'http://example.com:30020'] -client = ['/etc/certs/xxx/client.cert', '/etc/certs/xxx/client.key'] capabilities = ['pull', 'resolve'] +client = ['/etc/certs/xxx/client.cert', '/etc/certs/xxx/client.key'] [host.'http://example.com:30021'] +capabilities = ['pull', 'resolve'] client = ['/etc/certs/xxx/client.cert', '/etc/certs/xxx/client.key'] + +[host.'http://bar.com:30020'] capabilities = ['pull', 'resolve'] -`, +client = ['/etc/certs/xxx/client.cert', '/etc/certs/xxx/client.key']`, "/etc/containerd/certs.d/foo.bar:5000/hosts.toml": `server = 'http://foo.bar:5000' -[host] [host.'http://127.0.0.1:5000'] -capabilities = ['pull', 'resolve'] -`, +capabilities = ['pull', 'resolve']`, }, }, } @@ -456,6 +442,92 @@ func TestMirrorConfigurationInvalidMirrorURL(t *testing.T) { require.EqualError(t, err, "invalid registry url user has to be empty: https://foo@docker.io") } +func TestExistingHosts(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + u, err := url.Parse("https://ghcr.io") + require.NoError(t, err) + + eh, err := existingHosts(fs, "", *u) + require.NoError(t, err) + require.Empty(t, eh) + + tomlHosts := `server = "https://registry-1.docker.io" +[host."https://mirror.registry"] + capabilities = ["pull"] + ca = "/etc/certs/mirror.pem" + skip_verify = false + [host."https://mirror.registry".header] + x-custom-2 = ["value1", "value2"] + +[host] + +[host."https://mirror-bak.registry/us"] + capabilities = ["pull"] + skip_verify = true + +[host."http://mirror.registry"] + capabilities = ["pull"] + +[host."https://test-3.registry"] + client = ["/etc/certs/client-1.pem", "/etc/certs/client-2.pem"] + +[host."https://test-2.registry".header] + x-custom-2 = ["foo"] + +[host."https://test-1.registry"] + capabilities = ["pull", "resolve", "push"] + ca = ["/etc/certs/test-1-ca.pem", "/etc/certs/special.pem"] + client = [["/etc/certs/client.cert", "/etc/certs/client.key"],["/etc/certs/client.pem", ""]] + +[host."https://test-2.registry"] + client = "/etc/certs/client.pem" + +[host."https://non-compliant-mirror.registry/v2/upstream"] + capabilities = ["pull"] + override_path = true` + + err = afero.WriteFile(fs, filepath.Join(backupDir, u.Host, "hosts.toml"), []byte(tomlHosts), 0o644) + require.NoError(t, err) + eh, err = existingHosts(fs, "", *u) + require.NoError(t, err) + expected := `[host.'https://mirror.registry'] +ca = '/etc/certs/mirror.pem' +capabilities = ['pull'] +skip_verify = false + +[host.'https://mirror.registry'.header] +x-custom-2 = ['value1', 'value2'] + +[host.'https://mirror-bak.registry/us'] +capabilities = ['pull'] +skip_verify = true + +[host.'http://mirror.registry'] +capabilities = ['pull'] + +[host.'https://test-3.registry'] +client = ['/etc/certs/client-1.pem', '/etc/certs/client-2.pem'] + +[host.'https://test-1.registry'] +ca = ['/etc/certs/test-1-ca.pem', '/etc/certs/special.pem'] +capabilities = ['pull', 'resolve', 'push'] +client = [['/etc/certs/client.cert', '/etc/certs/client.key'], ['/etc/certs/client.pem', '']] + +[host.'https://test-2.registry'] +client = '/etc/certs/client.pem' + +[host.'https://test-2.registry'.header] +x-custom-2 = ['foo'] + +[host.'https://non-compliant-mirror.registry/v2/upstream'] +capabilities = ['pull'] +override_path = true` + require.Equal(t, expected, eh) + +} + func stringListToUrlList(t *testing.T, list []string) []url.URL { t.Helper() urls := []url.URL{}