Skip to content

[release-4.18] OCPBUGS-55787: Fix containerd host.toml generation to work with mirrors using an org_name #2879

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -274,13 +274,25 @@ See install options in the [EC2LaunchV2 documentation](https://docs.aws.amazon.c

In order to run Windows workloads on Nodes, the image `mcr.microsoft.com/oss/kubernetes/pause:3.9` must be mirrored.
See [Image configuration resources](https://docs.openshift.com/container-platform/latest/openshift_images/image-configuration.html) for general information on image mirroring.
Using ImageDigestMirrorSets and ImageTagMirrorSets to mirror container images results in different behavior than Linux Nodes.
Please account for the following differences when reading the above documentation.

Windows images mirrored through ImageDigestMirrorSet and ImageTagMirrorSet have specific naming requirements.
Mirroring on Linux nodes acts on the image level, while on Windows nodes we are only able to mirror on the registry level.
For example, if the cluster has an ImageTagMirrorSet (ITMS) specifying that `quay.io/remote-org/image` should use the mirror `quay.io/my-org/image`
any container that uses `quay.io/remote-org/image:tag` will instead use the image `quay.io/my-org/image:tag`.

This behavior differs for Windows Nodes, which will take the ITMS and use it to configure registry wide mirrors. Following the previous example,
by specifying a mirror of `quay.io/remote-org/image` to `quay.io/my-org/image`, Windows Nodes now use that mirror for all images from `quay.io/remote-org`.
In practice this means that `quay.io/remote-org/image:tag` will use the image `quay.io/my-org/image:tag` as expected, but another container using `quay.io/remote-org/different-image:tag`
will also try to use the mirror `quay.io/remote-org/different-image:tag`. This can cause unintended behavior if it is not accounted for.
For this reason it is recommended to specify container images using a digest, and to use ImageDigestMirrorSets instead of ImageTagMirrorSets.
This can prevent the wrong container image from being used, by ensuring the image the container specifies and the image being pulled have the same digest.

Additionally, Windows images mirrored through ImageDigestMirrorSet and ImageTagMirrorSet have specific naming requirements.
The mirrored image's suffix (final portion of namespaces and the image name) must match that of the source image.
For example, when mirroring the image `mcr.microsoft.com/oss/kubernetes/pause:3.9`, the mirror must have the format
`$mirrorRegistry/[$optionalNamespaces/]oss/kubernetes/pause:3.9` where `$optionalNamespaces` can be any number of
leading namespaces. Some valid values could be: `$mirrorRegistry/oss/kubernetes/pause:3.9`,
`$mirrorRegistry/custom/oss/kubernetes/pause:3.9`, `$mirrorRegistry/x/y/z/oss/kubernetes/pause:3.9`.
`$mirrorRegistry/[$org/]oss/kubernetes/pause:3.9` where `$org` can be any org name, or excluded completely.
Some valid values could be: `$mirrorRegistry/oss/kubernetes/pause:3.9`, `$mirrorRegistry/custom/oss/kubernetes/pause:3.9`, `$mirrorRegistry/x/y/z/oss/kubernetes/pause:3.9`.

## Limitations

Expand Down
68 changes: 51 additions & 17 deletions pkg/registries/registries.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/base64"
"encoding/json"
"fmt"
"net/url"
"sort"
"strings"

Expand Down Expand Up @@ -41,7 +42,7 @@ func newMirror(sourceImageLocation, mirrorImageLocation string, resolveTags bool
mirrorHost = extractMirrorURL(sourceImageLocation, mirrorImageLocation)
} else {
// special case if source and mirror are the same. Do not drop the host repo name to avoid an empty host entry
mirrorHost = extractHostname(mirrorImageLocation)
mirrorHost = extractRegistryHostname(mirrorImageLocation)
}
return mirror{host: mirrorHost, resolveTags: resolveTags}
}
Expand Down Expand Up @@ -82,14 +83,33 @@ func newMirrorSet(srcImage string, mirrorLocations []config.ImageMirror, resolve
for _, m := range mirrorLocations {
truncatedMirrors = append(truncatedMirrors, newMirror(srcImage, string(m), resolveTags))
}
return mirrorSet{source: extractHostname(srcImage), mirrors: truncatedMirrors, mirrorSourcePolicy: mirrorSourcePolicy}
return mirrorSet{source: extractRegistryHostname(srcImage), mirrors: truncatedMirrors, mirrorSourcePolicy: mirrorSourcePolicy}
}

// extractHostname extracts just the initial host repo from a full image location
// e.g. mcr.microsoft.com would be extracted from mcr.microsoft.com/oss/kubernetes/pause:3.9
func extractHostname(fullImage string) string {
parts := strings.Split(fullImage, imagePathSeparator)
return parts[0]
// extractRegistryHostname extracts just the initial host repo from a full image location, as containerd does not allow
// registries to exist on a subpath, given an input image `mcr.microsoft.com/oss/kubernetes/pause:3.9`,
// mcr.microsoft.com would be the determined registry hostname.
func extractRegistryHostname(fullImage string) string {
// url.Parse will only work if URL has a scheme (https://)
if parsedURL, err := url.Parse(fullImage); err == nil && parsedURL.Hostname() != "" {
if parsedURL.Port() != "" {
return parsedURL.Hostname() + ":" + parsedURL.Port()
}
return parsedURL.Hostname()
}
// For URLs without a scheme, just return everything before the first `/`
return strings.Split(fullImage, imagePathSeparator)[0]
}

// extractRegistryOrgPath returns only the org path when given a reference to a registry.
// The input for this function should not include an image name.
func extractRegistryOrgPath(registry string) string {
hostname := extractRegistryHostname(registry)
hostnameSplit := strings.SplitN(registry, hostname, 2)
if len(hostnameSplit) != 2 {
return ""
}
return strings.TrimPrefix(hostnameSplit[1], "/")
}

// getMergedMirrorSets extracts and merges the contents of the given mirror sets.
Expand Down Expand Up @@ -209,14 +229,24 @@ func (ms *mirrorSet) generateConfig(secretsConfig credentialprovider.DockerConfi
// set the fallback server to the first mirror to ensure the source is never contacted, even if all mirrors fail
fallbackServer = ms.mirrors[0].host
}
result += fmt.Sprintf("server = \"https://%s\"", fallbackServer)
result += "\r\n"
fallbackRegistry := extractRegistryHostname(fallbackServer)
result += fmt.Sprintf("server = \"https://%s/v2", fallbackRegistry)
if orgPath := extractRegistryOrgPath(fallbackServer); orgPath != "" {
result += "/" + orgPath
}
result += "\"\n"
result += "\noverride_path = true\n"

// Each mirror should result in an entry followed by a set of settings for interacting with the mirror host
for _, m := range ms.mirrors {
result += "\r\n"
result += fmt.Sprintf("[host.\"https://%s\"]", m.host)
result += "\r\n"
hostRegistry := extractRegistryHostname(m.host)
hostOrgPath := extractRegistryOrgPath(m.host)
result += "\n"
result += fmt.Sprintf("[host.\"https://%s/v2", hostRegistry)
if hostOrgPath != "" {
result += "/" + hostOrgPath
}
result += "\"]\n"

// Specify the operations the registry host may perform. IDMS mirrors can only be pulled by directly by digest,
// whereas ITMS mirrors have the additional resolve capability, which allows converting a tag name into a digest
Expand All @@ -227,18 +257,22 @@ func (ms *mirrorSet) generateConfig(secretsConfig credentialprovider.DockerConfi
hostCapabilities = " capabilities = [\"pull\"]"
}
result += hostCapabilities
result += "\r\n"
result += "\n"
result += " override_path = true\n"

// Extract the mirror repo's authorization credentials, if one exists
if entry, ok := secretsConfig.Auths[extractHostname(m.host)]; ok {
if entry, ok := secretsConfig.Auths[extractRegistryHostname(m.host)]; ok {
credentials := entry.Username + ":" + entry.Password
token := base64.StdEncoding.EncodeToString([]byte(credentials))

// Add the access token as a request header
result += fmt.Sprintf(" [host.\"https://%s\".header]", m.host)
result += "\r\n"
result += fmt.Sprintf(" [host.\"https://%s/v2", hostRegistry)
if hostOrgPath != "" {
result += "/" + hostOrgPath
}
result += "\".header]\n"
result += fmt.Sprintf(" authorization = \"Basic %s\"", token)
result += "\r\n"
result += "\n"
}
}

Expand Down
138 changes: 115 additions & 23 deletions pkg/registries/registries_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func TestGetMergedMirrorSets(t *testing.T) {
Spec: config.ImageDigestMirrorSetSpec{
ImageDigestMirrors: []config.ImageDigestMirrors{
{
Source: "source1",
Source: "https://source1.local:5000",
Mirrors: []config.ImageMirror{"mirror1"},
MirrorSourcePolicy: config.AllowContactingSource,
},
Expand All @@ -102,7 +102,7 @@ func TestGetMergedMirrorSets(t *testing.T) {
},
expectedOutput: []mirrorSet{
{
source: "source1",
source: "source1.local:5000",
mirrors: []mirror{{host: "mirror1", resolveTags: false}},
mirrorSourcePolicy: config.AllowContactingSource,
},
Expand Down Expand Up @@ -133,7 +133,7 @@ func TestGetMergedMirrorSets(t *testing.T) {
Spec: config.ImageTagMirrorSetSpec{
ImageTagMirrors: []config.ImageTagMirrors{
{
Source: "mcr.microsoft.com/oss/kubernetes/pause",
Source: "docker://mcr.microsoft.com/oss/kubernetes/pause",
Mirrors: []config.ImageMirror{"quay.io/testuser/oss/kubernetes/pause"},
MirrorSourcePolicy: config.AllowContactingSource,
},
Expand Down Expand Up @@ -237,7 +237,7 @@ func TestGenerateConfig(t *testing.T) {
{
name: "empty mirrors",
input: mirrorSet{
source: "registry.access.redhat.com/ubi9/ubi-minimal",
source: "registry.access.redhat.com/ubi9",
mirrors: []mirror{},
mirrorSourcePolicy: config.AllowContactingSource,
},
Expand All @@ -246,72 +246,138 @@ func TestGenerateConfig(t *testing.T) {
{
name: "basic one digest mirror",
input: mirrorSet{
source: "registry.access.redhat.com/ubi9/ubi-minimal",
source: "registry.access.redhat.com",
mirrors: []mirror{
{host: "example.io/example/ubi-minimal", resolveTags: false},
{host: "example.io/example", resolveTags: false},
},
mirrorSourcePolicy: config.AllowContactingSource,
},
expectedOutput: "server = \"https://registry.access.redhat.com/ubi9/ubi-minimal\"\r\n\r\n[host.\"https://example.io/example/ubi-minimal\"]\r\n capabilities = [\"pull\"]\r\n",
expectedOutput: `server = "https://registry.access.redhat.com/v2"

override_path = true

[host."https://example.io/v2/example"]
capabilities = ["pull"]
override_path = true
`,
},
{
name: "basic one tag mirror",
input: mirrorSet{
source: "registry.access.redhat.com/ubi9/ubi-minimal",
source: "registry.access.redhat.com",
mirrors: []mirror{
{host: "example.io/example/ubi-minimal", resolveTags: true},
{host: "example.io/example", resolveTags: true},
},
mirrorSourcePolicy: config.AllowContactingSource,
},
expectedOutput: "server = \"https://registry.access.redhat.com/ubi9/ubi-minimal\"\r\n\r\n[host.\"https://example.io/example/ubi-minimal\"]\r\n capabilities = [\"pull\", \"resolve\"]\r\n",
expectedOutput: `server = "https://registry.access.redhat.com/v2"

override_path = true

[host."https://example.io/v2/example"]
capabilities = ["pull", "resolve"]
override_path = true
`,
},
{
name: "one digest mirror never contact source",
input: mirrorSet{
source: "registry.access.redhat.com/ubi9/ubi-minimal",
source: "registry.access.redhat.com",
mirrors: []mirror{
{host: "example.io/example/ubi-minimal", resolveTags: false},
{host: "example.io/example", resolveTags: false},
},
mirrorSourcePolicy: config.NeverContactSource,
},
expectedOutput: "server = \"https://example.io/example/ubi-minimal\"\r\n\r\n[host.\"https://example.io/example/ubi-minimal\"]\r\n capabilities = [\"pull\"]\r\n",
expectedOutput: `server = "https://example.io/v2/example"

override_path = true

[host."https://example.io/v2/example"]
capabilities = ["pull"]
override_path = true
`,
},
{
name: "tags mirror never contact source",
input: mirrorSet{
source: "registry.access.redhat.com/ubi9/ubi-minimal",
source: "registry.access.redhat.com",
mirrors: []mirror{
{host: "example.io/example/ubi-minimal", resolveTags: true},
{host: "example.io/example", resolveTags: true},
},
mirrorSourcePolicy: config.NeverContactSource,
},
expectedOutput: "server = \"https://example.io/example/ubi-minimal\"\r\n\r\n[host.\"https://example.io/example/ubi-minimal\"]\r\n capabilities = [\"pull\", \"resolve\"]\r\n",
expectedOutput: `server = "https://example.io/v2/example"

override_path = true

[host."https://example.io/v2/example"]
capabilities = ["pull", "resolve"]
override_path = true
`,
},
{
name: "multiple mirrors",
input: mirrorSet{
source: "registry.access.redhat.com/ubi9/ubi-minimal",
source: "registry.access.redhat.com",
mirrors: []mirror{
{host: "example.io/example/ubi-minimal", resolveTags: false},
{host: "example.io/example", resolveTags: false},
{host: "mirror.example.com/redhat", resolveTags: false},
{host: "mirror.example.net/image", resolveTags: true},
{host: "mirror.example.net", resolveTags: true},
},
mirrorSourcePolicy: config.AllowContactingSource,
},
expectedOutput: "server = \"https://registry.access.redhat.com/ubi9/ubi-minimal\"\r\n\r\n[host.\"https://example.io/example/ubi-minimal\"]\r\n capabilities = [\"pull\"]\r\n\r\n[host.\"https://mirror.example.com/redhat\"]\r\n capabilities = [\"pull\"]\r\n [host.\"https://mirror.example.com/redhat\".header]\r\n authorization = \"Basic dXNlcjpwYXNz\"\r\n\r\n[host.\"https://mirror.example.net/image\"]\r\n capabilities = [\"pull\", \"resolve\"]\r\n [host.\"https://mirror.example.net/image\".header]\r\n authorization = \"Basic dXNlcm5hbWU6cGFzc3dvcmQ=\"\r\n",
expectedOutput: `server = "https://registry.access.redhat.com/v2"

override_path = true

[host."https://example.io/v2/example"]
capabilities = ["pull"]
override_path = true

[host."https://mirror.example.com/v2/redhat"]
capabilities = ["pull"]
override_path = true
[host."https://mirror.example.com/v2/redhat".header]
authorization = "Basic dXNlcjpwYXNz"

[host."https://mirror.example.net/v2"]
capabilities = ["pull", "resolve"]
override_path = true
[host."https://mirror.example.net/v2".header]
authorization = "Basic dXNlcm5hbWU6cGFzc3dvcmQ="
`,
},
{
name: "multiple mirrors never contact source",
input: mirrorSet{
source: "registry.access.redhat.com/ubi9/ubi-minimal",
source: "registry.access.redhat.com",
mirrors: []mirror{
{host: "example.io/example/ubi-minimal", resolveTags: false},
{host: "example.io/example", resolveTags: false},
{host: "mirror.example.com/redhat", resolveTags: false},
{host: "mirror.example.net", resolveTags: true},
},
mirrorSourcePolicy: config.NeverContactSource,
},
expectedOutput: "server = \"https://example.io/example/ubi-minimal\"\r\n\r\n[host.\"https://example.io/example/ubi-minimal\"]\r\n capabilities = [\"pull\"]\r\n\r\n[host.\"https://mirror.example.com/redhat\"]\r\n capabilities = [\"pull\"]\r\n [host.\"https://mirror.example.com/redhat\".header]\r\n authorization = \"Basic dXNlcjpwYXNz\"\r\n\r\n[host.\"https://mirror.example.net\"]\r\n capabilities = [\"pull\", \"resolve\"]\r\n [host.\"https://mirror.example.net\".header]\r\n authorization = \"Basic dXNlcm5hbWU6cGFzc3dvcmQ=\"\r\n",
expectedOutput: `server = "https://example.io/v2/example"

override_path = true

[host."https://example.io/v2/example"]
capabilities = ["pull"]
override_path = true

[host."https://mirror.example.com/v2/redhat"]
capabilities = ["pull"]
override_path = true
[host."https://mirror.example.com/v2/redhat".header]
authorization = "Basic dXNlcjpwYXNz"

[host."https://mirror.example.net/v2"]
capabilities = ["pull", "resolve"]
override_path = true
[host."https://mirror.example.net/v2".header]
authorization = "Basic dXNlcm5hbWU6cGFzc3dvcmQ="
`,
},
}

Expand Down Expand Up @@ -637,3 +703,29 @@ func TestExtractMirrorURL(t *testing.T) {
})
}
}

func TestExtractOrgPath(t *testing.T) {
tests := []struct {
input string
expected string
}{
{
input: "registry.local/org",
expected: "org",
},
{
input: "registry.local/org/sub_org",
expected: "org/sub_org",
},
{
input: "registry.local",
expected: "",
},
}
for _, tt := range tests {
t.Run(tt.input, func(t *testing.T) {
result := extractRegistryOrgPath(tt.input)
assert.Equal(t, tt.expected, result)
})
}
}