Creating a network bridge fails with ':' (colon) character in name

antis81 commented 1 year ago

Issue Description

Out of curiosity I ran the SailfishOS SDK installer against podman-docker. As expected this didn't run smooth at all and revealed a lot of compatibility issues (mostly related to root permissions).

However it seems that Docker allows ':' (colon) characters in network names while podman does not. Should podman actually allow ':' (colon) characters in network names?

Steps to reproduce the issue

Create a network bridge containing a ':' (colon) character:

podman network create --driver bridge sailfish-sdk:user
# where "user" is meant to be the current $USER

Describe the results you received

NOTE: I ran with podman-docker (hence the emulate… message). Hoever output is the same with podman.

Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg.
Error: network name sailfish-sdk:user invalid: names must match [a-zA-Z0-9][a-zA-Z0-9_.-]*: invalid argument

Describe the results you expected

Network entry with name sailfish-sdk:user

podman info output

Error: network name sailfish-sdk:user invalid: names must match [a-zA-Z0-9][a-zA-Z0-9_.-]*: invalid argument

Podman in a container

No

Privileged Or Rootless

Rootless

Upstream Latest Release

No

Additional environment details

podman version 4.4.1 (latest version in Manjaro distribution)

Additional information

No response

Luap99 commented 1 year ago

Oh, I wasn't aware docker allows you to create names with a colon. Adding support for it is somewhat problematic for podman, we already use the colon to specify extra option after the network name when it set with --network, see syntax documented here: https://docs.podman.io/en/latest/markdown/podman-create.1.html#network-mode-net.

We aim for compatibility so it would be great if we could figure out a way to allow it without breaking podman backwards compatibility.

antis81 commented 1 year ago

Interesting… Not the greatest parser guru on earth TBH. However this looks fairly distinguishable since every option (despite interface_name :question:) seems to require a value assignment (in the form :option=value,option=value,…). Should be okay to check for the '=' token right?

Luap99 commented 1 year ago

Copying from #17806 reported by @chiting:

I ran circleci-cli against podman and the container created tries to add a new network bridge in the following format: ZZZ_localbuild-1111111111/FFFFFFFF which is not accepted by podman because it must match a specific regex that doesn't contain the character /.

Docker allows the character / in network names while podman doesn't. If podman is aiming for full compatibility with docker, I think it should also allow it.

Steps to reproduce the issue

Create a network bridge with a / in the name:

podman network create --driver bridge ZZZ_localbuild-1111111111/FFFFFFFF

Describe the results you received

Error: network name ZZZ_localbuild-1111111111/FFFFFFFF invalid: names must match [a-zA-Z0-9][a-zA-Z0-9_.-]*: invalid argument

Describe the results you expected

It should be able to create the network and show it in the network list

$ podman network ls
NETWORK ID    NAME        DRIVER
000000000000  podman      bridge
ffffffffffff  ZZZ_localbuild-1111111111/FFFFFFFF bridge

It seems like docker allows you anything for network names, fixing this in podman will be complicated. We need to keep backwards compat for our current syntax.

Also I have not checked with CNI if they even support such names. For netavark it should be fine. Although another problem is that we currently use the network name as filename on disk, this will not work with a slash in the name.

cc @mheon

mheon commented 1 year ago

What reserved characters do we have to be careful of, besides colons? It seems like we could loosen the regex immediately to allow everything that is not a colon while we figure out how to handle those.

Luap99 commented 1 year ago

As said the slash will break the on disk format, although it should be possible to change the file names to use the ID instead. Also the comma is used to split network names for backwards compat with 3.X. --network name1,name2 is valid.

And then the question is how does CNI handle it? I mean we could ignore it there since we plan to deprecate it but it is not really nice if it blows up in the Setup() call.

antis81 commented 1 year ago

While being aware the error message origins from here I found this comment…

Now the question I have is: Isn't there a more elegant way and query the kernel (API/ABI) directly for valid interface names rather than (redundantly) writing the validator? (see is_valid_name built-in Kernel function). Then again it checks exactly for those characters… Do they have a different meaning in Docker?

github-actions[bot] commented 1 year ago

A friendly reminder that this issue had no activity for 30 days.

Luap99 commented 1 year ago

While being aware the error message origins from here I found this comment…

Now the question I have is: Isn't there a more elegant way and query the kernel (API/ABI) directly for valid interface names rather than (redundantly) writing the validator? (see is_valid_name built-in Kernel function). Then again it checks exactly for those characters… Do they have a different meaning in Docker?

You are looking at interface names, this is different from what we call network names.

A network name is just the thing we use in our db to store relations between containers and networks and what you can use to refer to the network on the cli/API. The actual interface name is limited to only 15 chars max by the kernel and is included in the config, check podman network inspect --format {{.NetworkInterface}} <network>

The CNI link is still helpful, it shows the network name is limited to the same regex as we use so changing this for cni will be impossible (unless you can get the CNI folks convinced to change it).

For netavark we can fix it but it will require some non trivial code changes if we have to keep backwards compatible. The network name is not only used on disk for the network config file it is also in use by aardvark-dns config file. So using a slash is definitely not possible right now.

As for the podman cli parsing this will complicate things a lot as well. The colon is already used by use to specify extra options for networks. And the comma is need for backwards compat.

So for characters besides /, :, , we could easily relax the regex (only with netavark) to allow more characters but given the reports here use specifically one of those characters I see little benefit in doing so.

containers / podman