T7092: Add Container Registry Mirror #4321
Conversation
sskaje
requested review from
dmbaturin,
sarthurdev,
jestabro,
sever-sever,
c-po,
fett0,
nicolas-fort and
zdc
as code owners
|
👍 |
| @@ -538,6 +538,17 @@ | |||
| <children> | |||
| #include <include/interface/authentication.xml.i> | |||
| #include <include/generic-disable-node.xml.i> | |||
| <leafNode name="insecure"> | |||
There was a problem hiding this comment.
How common are HTTP-only mirrors in the age of Let's Encrypt? Are there compelling arguments to support them?
There was a problem hiding this comment.
I can't tell how common, but those I have access to are all ip:port only and only for development servers and developer's machines.
There was a problem hiding this comment.
Should insecure & authentication nodes be mutually exclusive?
Can't see it ever being advisable to send credentials over plaintext...
There was a problem hiding this comment.
Should insecure & authentication nodes be mutually exclusive? Can't see it ever being advisable to send credentials over plaintext...
Theoritically yes, but you can't expect small company do everything right in their LAN.
throw a warning is enough.
Also, insecure can be used for self-signed certificate: https://github.com/containers/image/blob/main/docs/containers-registries.conf.5.md
insecure : true or false. By default, container runtimes require TLS when retrieving images from a registry. If insecure is set to true, unencrypted HTTP as well as TLS connections with untrusted certificates are allowed.
| </leafNode> | ||
| <leafNode name="mirror"> | ||
| <properties> | ||
| <help>Registry mirror, use host:port</help> |
There was a problem hiding this comment.
I wonder if it's better to make host and port separate options so that we can validate them. URLs are quite difficult to validate but leaving an option without any validation at all certainly rubs me the wrong way.
Even more so in cases where the effect of a keyboard cat entering something like '[[fdfs89. will not cause any immediate errors in the target component, either.
There was a problem hiding this comment.
My opinion, simple regex is enough, like [a-z0-9\-\.]+(:\d+)?.
There was a problem hiding this comment.
all we need to support
1.1.1.1:80
2.2.2.2
simple:80
simplewithoutport
test-server-with-dash
test.lan
test.lan:8080
normal.domain.com
domain.with.port:80
There was a problem hiding this comment.
https://github.com/containers/image/blob/main/docs/containers-registries.conf.5.md
pattern with url path should also be supported, then the regex would be sth like
^(?:[a-zA-Z0-9-]+(?:\.[a-zA-Z0-9-]+)*|(?:\d{1,3}\.){3}\d{1,3}|\[[a-fA-F0-9:]+])(?::\d+)?(?:\/[^\s?#]*)?$
for host name, ipv4, ipv6 + optional port + optional path
There was a problem hiding this comment.
Can't we just use URL validator?
Something like: <validator name="url" argument="--scheme http --scheme https"/> might cover most of what's needed without the regex hieroglyphs.
There was a problem hiding this comment.
Can't we just use URL validator? Something like:
<validator name="url" argument="--scheme http --scheme https"/>might cover most of what's needed without the regex hieroglyphs.
prefix: A prefix of the user-specified image name, i.e. using one of the following formats:
host[:port]
host[:port]/namespace[/namespace…]
host[:port]/namespace[/namespace…]/repo
host[:port]/namespace[/namespace…]/repo(:_tag|@digest)
[*.]host
location : Accepts the same format as the prefix field, and specifies the physical location of the prefix-rooted namespace.
https://github.com/containers/image/blob/main/docs/containers-registries.conf.5.md
I don't ever use the line with (:_tag|@digest) before, but I used other formats before.
If the url validator can disallow scheme and query string, it should work
There was a problem hiding this comment.
If the url validator can disallow scheme and query string, it should work
I don't think url validator works without --scheme. It is unlikely to work in this case, unfortunately.
|
CI integration 👍 passed! Details
|
Change summary
Add Container Registry Mirror
Types of changes
Related Task(s)
https://vyos.dev/T7092
Related PR(s)
How to test / Smoketest result
Checklist: