This fixes the following staticcheck warning:
> libnetwork/netavark/config.go:297:2: QF1007: could merge conditional assignment into variable declaration (staticcheck)
> isMacVlan := true
> ^
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
... and add a deprecated alias so backward compatibility is still
preserved (and users can gradually switch to the new name).
Done because this is now also reported by staticcheck
(in addition to revive) linter.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Implements interface name length validation during network creation to prevent
netlink errors when names exceed the kernel's 15-character limit.
This prevents creation of networks with interface names that would cause failures
when running containers.
Signed-off-by: Lucas Pablo Calisi <lucas.calisi@mercadolibre.com>
While we already did clean up the allocated ips on the regular setup
error this did not worked for errors from the rootlessNetns.Setup()
call. To ensure we dealloc the ips on all error paths use a defer
function that checks the return error value.
Fixescontainers/podman#25422
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Mainly type casting issues. I ignored some of them where I don't think
it can fail or when it is in tests where we would notice anyway.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
When a vlan is used there should be no bridge name conflict check. It is
totally valid to have the same bridge with different vlans in two
configs and that is the intended use case.
Fixes#2095
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
It is possible that the netns file where we bind mount the netns already
exists. This can happen if a previous setup process was killed between
creating the file and mounting to it. Or likely more common as described
in the podman issue if the runroot is not a tmpfs and not deleted after
boot.
Fixescontainers/podman#25144
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Podman creates/initializes the network backend for every command. However
most commands will not need it so we should keep the required actions we
do to a minimum.
In this case the config directory /etc/containers/networks by default as
root may not exists and then we try to create it which can fail, i.e.
when /etc is read only[1].
The code here are a bit more changes then I would have liked but we must
make sure the default in memory network always exists and do not create
the directory there.
[1] https://github.com/containers/common/pull/2265
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
* Added ContainerHostname to NetworkOptions. Podman will set this
and Netavark will read it.
* Added the `container_name_as_hostname` option to the
CONTAINERS table in containers.conf. Currently, if you don't
explicitly set a hostname when creating a container, podman will
set it to the short ID. If this option set to `true` and a
hostname isn't explicitly set, podman will use the container's
name, with characters not in the set `[0-9a-zA-Z.-]` removed,
as the hostname instead of the short ID. Set to false by default
to preserve existing behavior.
Signed-off-by: George Joseph <g.devel@wxy78.net>
Starting with pasta 2024_11_27.c0fbc7e there is new "local mode"[1] in
pasta that defaults to setting up link local addresses in the netns when
no suitable interface was found. this is done to fix the podman issue[2]
where we fail to start in these cases which was a poor UX. Now the pasta
change alone works fine for these users but there is one problem.
Podman adds hosts entries for the container ip/name tuple and for the
host.containers.internal. These entries are filtered out thus neither
ipv4 or ipv6 bool was set and no addresses where added to IPAddresses.
Thus podman had no info to add entries and just left them empty, while
for most cases this is fine there might be a few users who expect
host.containers.internal and the container name to resolve correctly.
This commit changes the logic to only skip ipv6 link local addresses but
allow ipv4 link local addresses. With that podman will add the proper
entry.
[1] https://archives.passt.top/passt-dev/20241127042725.3133538-1-sbrivio@redhat.com/
[2] https://github.com/containers/podman/issues/24614
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
L2 will be used to allow using existing bridges which netavark will
neither create nor delete.
Signed-off-by: Michael Zimmermann <sigmaepsilon92@gmail.com>
This allows us to pass through data to netavaark without bloating this
struct. See #24523.
Signed-off-by: Michael Zimmermann <sigmaepsilon92@gmail.com>
One thing I noticed in the recent aardvark-dns bug[1] that we copy link
local nameservers into the container. This makes no sense as the link
local address contains a zone (interface name/index) and cannot work
without it. However a container by design will have a different
interface name/index so the address can never work in the normal case.
Only when we do share the host netns then we should keep it.
[1] https://github.com/containers/aardvark-dns/pull/537
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Contains fixes for new linters, removed depracted and removed linters
from the config.
Most notably because we use go 1.22 now we can get rid of the copy for
loop vars[1]. Also as of the go 1..2 we can use the new int range syntax
in for loops the new intrange linter checks that.
[1] https://go.dev/blog/loopvar-preview
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
For the pasta network mode we now use --map-guest-addr which means we
have a specific ip that we want to use as host.containers.internal
address. I first thought we could handle it in podman but that doesn't
work as the contianers.conf option must have a higher priority.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
GetHostContainersInternalIP() is no longer called in podman or buildah
as they use GetHostContainersInternalIPExcluding(). I need to add a new
option so chnage the function to accept the parameters as struct so we
do not have to break the API every time we add a new parameter.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Use %s as %q just quotes/escapes everything which makes it harder to
read and trim of the last newline and spaces as well.
Also update the warnings comment, we still see warnings by default on
our debian VMs in podman CI so this cannot be on the warning level yet.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
When using the rootless netns (bridge mode) so far podman ignored the
proper pasta or slirp4netns dns sever for networks without aardvark-dns.
This is not good. We should try to use them by default, and with the new
MapGuestAddr option we need to use that as well for
host.containers.internal. The problem is that becuase we only know what
options we uses when we started the process later container starts from
a new podman process do not really see these options if we just cache
the result in memory. So in order to make all following podman process
aware we serialize this info struct as json and later processes read it
when needed.
It also means we do not have to lookup the netns ip evey time so I
removed that code.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
I already switch all user from the old Setup over to Setup2(), so no we
can again reuse the Setup() name. As such alias Setup and Setup for the
same function and then once I migrated all callers in podman and buildah
I will remove Setup2() here.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
--map-guest-addr was just added in 20240814, we cannot yet hard require
this option to be present. This means we must deal with the case where
the option is not working. Both a version check or checking --help would
add extra overhead in the good case. To avoid this we try first with the
new option and if this fails check the error message for the right
error. If it didn't know about the new option we remove it and try to
exec pasta again.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The --map-guest-addr option allows us to sepcify a ip that is remapped
to the actual host ip that was used by pasta. This is done to fix the
problem where connecting to the host ip was not possible as the same ip
was used in the netns.
We now set --map-guest-addr 169.254.1.2 which follows the same idea we
already used for the --dns-forward option. With that podman can use this
ip to set it for host.containers.internal which should the case where
there was no second host ip available, see
https://github.com/containers/podman/issues/19213
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Per feedback[1] the 169.254.0.0/24 range is reserved for future use in
RFC 3927. As such we should not use it here as it might break in the
future if the range gets assigned a new meaning. Switch to 169.254.1.1.
[1] https://github.com/containers/podman/pull/23791#discussion_r1737913730
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The Run() function is used to run long running command in the netns,
namly podman unshare --rootless-netns used that. As such the function
actually unlocks for the main command as otherwise a user could hold the
lock forever effectively causing deadlocks.
Now because we unlock the ref count might change during that time. And
just because we create the netns doesn't mean there are now other users
of the netns. Therefore the cleanup in runInner() was wrong in that
case causing problems for other running containers.
To fix this make sure we do not cleanup in the Run() case unless the
count is 0.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
When I wrote this originally I thought we must avoid leaking the netns
so I tried to decrement first. However now I think this wrong because
podman actially calls into the cleanup function again if it returned an
error on the next cleanup attempt. As such we ended up doing a double
decrement and the ref counter went below zero causing a sort of issues[1].
Now if we have a bug the other way around were we not decrement
correctly this is much less of a problem. It simply means we leak once
netns file and the pasta/slirp4netns process which isn't a problem other
than needed a bit of resources.
[1] https://github.com/containers/podman/issues/21569
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Commit 56c6a9ac07 added check for the env var inside the network
interface setup code as this is something that is always set by podman.
However if you try to run the unit tests as rootless they now always
failed as the code assumed we have access to a rootful path.
The easy fix is to just fix the test to set the env when running
rootless.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Currently it does the mkdir only implicitly because the code creates
run/systemd but this only happens when /run/systemd exists on the host.
As such the rootless code was broken on all non systemd distros[1].
[1] https://github.com/containers/podman/discussions/22903
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
For some unknown reason the podman container image sets the
_CONTAINERS_USERNS_CONFIGURED env to an empty value. I don't know what
the purpose of this is but is will trigger the check here which is wrong
when the container is privileged.
To fix this check that the value is set to done like it is by the reexec
logic. Also make sure the lock dir uses the same condition to stay
consistent.
Fixescontainers/podman#22791
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
When using the bridge network mode as rootless we use the rootless netns
logic, for podman this looks like just as using bridge as root. The
issue is however due the extra namespace we block certain address there.
This can be seen best with pasta but actually effects other cases too.
The podman logic tries to use any host ip address for
host.containers.internal but we must make sure to exculde all these
address in the rootless netns as they are not actually the hostns as
thus cause great confusion.
For the --network pasta case I already fixed this by returning the ips on
the pasta.Setup2() call in 83573fa60c.
For the bridge mode this more complicated due several layers of function
calls. I decided to implement this as extra function call on the interface
to return the ips as this makes the usage in podman the easiest. And I
also didn't want to break the API as we only have to fix this in podman
not buildah.
It is needed to address #22653 but it needs podman changes as well to
use this new function.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Skip or remove tests that need dnsname in order to function.
As of fedora 40 dnsname is no longer packaged so our CI VM cannot use
it.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
The main advantage is that we do not have to iterate through all the
ports we added again to check for the custom ports.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
If a port option was given after --map-gw then parsing failed as the
next arg was always skipped due the modification of the slice.
Modifing the slice inside the loop is bad and does not do what some
might think. Append here basically creates a new slice (thus you always
have to assign the result to the variable) with the same pointer to the
same underlying array of data[1]. The loop however will still continue to
loop over the slice as it saw it at the begining of the loop.
So in the bug case the underlying array would look like this:
{"--config-net", "--map-gw", "-T", "80"}
and after the append call to remove --map-gw like this:
{"--config-net", "-T", "80", "80"}
The loop iterator has no idea this happen and just moves to the next
index 2 ("80") and thus we never passed "-T" causing this bug.
[1] https://go.dev/blog/slices-introFixescontainers/podman#22477
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Upper case is used for exported types/vars and these are local variables
so make sure the start lower case to not confuse readers.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
This is good to prevent any leaks but more important here there is a
bug because we cache the last assigned ip. However when a network is
removed the recreated with a different LeaseRange that ip might be very
well outside the expected range and the logic seems to handle this
correctly. I could fix it there but deleting the full bucket seems best
as it avoid other issues and leaking the bucket forever.
Fixescontainers/podman#22034
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
This here just logs unnecessary errors in case there is an error during
the Run() call (podman unshare --rootless-netns). runInner() will
already call cleanup on errors if it created a new netns so we only need
to cleanup when there is no error.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Kir Kolyshkin