Skip to content
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

Cannot use IPv6 literal in registry mirror endpoint #10055

Open
brandond opened this issue Apr 9, 2024 · 8 comments
Open

Cannot use IPv6 literal in registry mirror endpoint #10055

brandond opened this issue Apr 9, 2024 · 8 comments
Labels

Comments

@brandond
Copy link
Contributor

brandond commented Apr 9, 2024

Description

If I want to use an IPv6 address as a registry endpoint mirror using certs.d / hosts.toml, I would expect to be able to do the following:

host."https://[fd7c:53a5:aef5::242:ac11:7]"]
  capabilities = ["pull", "resolve"]
  skip_verify = true

This fails to load because square braces are not valid in TOML keys:

time="2024-04-09T20:26:57.094249564Z" level=error msg="failed to decode hosts.toml" error="failed to parse TOML: (8, 2): unexpected token table key cannot contain ']', was expecting a table key"

I tried removing the braces but this does not appear to work either:

[host."https://fd7c:53a5:aef5::242:ac11:7"]
  override_path = true
  capabilities = ["pull", "resolve"]

Apr 09 20:22:35 systemd-node-1 k3s[2162]: E0409 20:22:35.460639 2162 kuberuntime_sandbox.go:72] "Failed to create sandbox for pod" err="rpc error: code = Unknown desc = failed to get sandbox image \"rancher/mirrored-pause:3.6\": failed to pull image \"rancher/mirrored-pause:3.6\": failed to pull and unpack image \"docker.io/rancher/mirrored-pause:3.6\": failed to resolve reference \"docker.io/rancher/mirrored-pause:3.6\": failed to do request: Head \"https://fd7c:53a5:aef5::242:ac11:7/v2/rancher/mirrored-pause/manifests/3.6?ns=docker.io\": dial tcp [fd7c:53a5:aef5::242:ac11]:7: connect: no route to host" pod="kube-system/metrics-server-54fd9b65b-2c77m"

It appears that the URL is being mis-parsed; the final octet of the IPv6 address literal is being used as the port. Appending the port results in slightly different wrong behavior - the dial succeeds, but the server does not process the request.

Pulling the image directly seems to work OK, as the host is already properly escaped:

root@systemd-node-1:/# cat "/var/lib/rancher/k3s/agent/etc/containerd/certs.d/[fd7c:53a5:aef5::242:ac11:7]/hosts.toml"
skip_verify = true
[host]

root@systemd-node-1:/# crictl pull '[fd7c:53a5:aef5::242:ac11:7]/library/busybox:latest'
Image is up to date for sha256:ba5dc23f65d4cc4a4535bce55cf9e63b068eb02946e3422d3587e8ce803b6aab

Steps to reproduce the issue

See above

Describe the results you received and expected

Can use an IPv6 address literal in the mirror endpoint URI

What version of containerd are you using?

v1.7.11-k3s2

Any other relevant information

Tracking this in k3s as k3s-io/k3s#9897

Show configuration if it is related to CRI plugin.

No response

@aojea
Copy link
Contributor

aojea commented Apr 11, 2024

distribution/distribution#3489

@brandond
Copy link
Contributor Author

brandond commented Apr 11, 2024

Use of IPv6 literals in the image name works in Kubernetes 1.29, which uses docker/distribution v2.8.3 which uses distribution/reference as the backend for docker/distribution/reference. Older releases that are still on v2.8.2 will fail to schedule pods that use an ipv6 literal as the image registry:

    state:
      waiting:
        message: 'Failed to apply default image tag "[fd7c:53a5:aef5::242:ac11:7]/rancher/mirrored-coredns-coredns:1.10.1":
          couldn''t parse image name "[fd7c:53a5:aef5::242:ac11:7]/rancher/mirrored-coredns-coredns:1.10.1":
          invalid reference format'
        reason: InvalidImageName

This is unrelated to the issue of needing to support IPv6 addresses in mirror endpoints though.

@brandond
Copy link
Contributor Author

brandond commented Apr 11, 2024

Thanks @aojea for getting this working in distribution and eventually kubernetes ;)

@brandond
Copy link
Contributor Author

brandond commented Apr 12, 2024

fwiw go-toml itself doesn't handle round-trip this properly either. There does not appear to be any way to escape characters in table keys.

package main

import (
	"fmt"

	"github.com/pelletier/go-toml"
)

type Entry struct {
	Foo string
}

type Thing struct {
	Entries map[string]Entry
}

func main() {
	foo := Thing{
		Entries: map[string]Entry{
			"https://[::1]/v2": {
				Foo: "foo",
			},
		},
	}
	b, err := toml.Marshal(foo)
	fmt.Printf("Marshal error=%v\n%s\n", err, b)
	err = toml.Unmarshal(b, &foo)
	fmt.Printf("Unmarshal error=%v\n", err)
}
Marshal error=<nil>

[Entries]

  [Entries."https://[::1]/v2"]
    Foo = "foo"

Unmarshal error=(4, 4): unexpected token table key cannot contain ']', was expecting a table key

@brandond
Copy link
Contributor Author

Added a very hacky proposed fix in #10072

@brandond
Copy link
Contributor Author

brandond commented Apr 12, 2024

@samuelkarp would you prefer to discuss the preferred approach to this here? I'll paste what I said on the PR:

Honestly this datastructure should have been a table list with a server field for each entry, rather than trying to get clever and put the server URL in the table key. Using a list would have also avoided all the weirdness with having to re-parse the tree to get the key order from line numbers in the file.

[[host]]
  server = "https://[fd7c:53a5:aef5::242:ac11:7]/v2"
  capabilities = ["pull", "resolve"]
  skip_verify = true

Since containerd 2.0 isn't out yet, now might be a good time to fix this.

@samuelkarp
Copy link
Member

Honestly this datastructure should have been a table list with a server field for each entry, rather than trying to get clever and put the server URL in the table key.

I agree.

Since containerd 2.0 isn't out yet, now might be a good time to fix this.

If so, we'll need to get it done and in fairly quickly; 2.0.0-rc.0 is already out and we've tried to give advance warning of breaking changes like this. It may be more feasible to add an override, or to try and fix the port-parsing such that a valid IPv6 address is not mistaken for something with a port appended.

@dmcgowan I'd appreciate your thoughts on this.

@brandond
Copy link
Contributor Author

brandond commented Apr 13, 2024

I'll have to poke at the go-toml/v2 parser that containerd 2.0 uses, but I was considering trying to get it to support both the existing [host."example.com"] or proposed [[host]]/server="example.com" structure, as long as the file doesn't mix between both. I think that should be doable.

I should have some cycles for a POC next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants