e2e: inconsistent UID/GID gating across container tools

[shivansh-source]

The e2e helpers build the "run as user" setting from the same containerTestArgs.UID/GID, but the guard differs across tools:

• commonNewContainerCmd (tests/e2e/common.go, used by docker + nerdctl) → if a.UID != 0 && a.GID != 0
• crictlNewContainerConfig (tests/e2e/crictl.go) → if a.UID != 0 && a.GID != 0 (gates RunAsUser/RunAsGroup)
• ctrNewContainerCmd (tests/e2e/ctr.go) → if a.UID != 0 || a.GID != 0

With &&, a config such as UID: 1000, GID: 0 (run as user 1000, group root) drops the -u flag / RunAsUser entirely on docker, nerdctl, and crictl, while ctr honors it. The same test args produce different runtime behavior depending on the tool under test, which defeats the purpose of a shared arg struct.
|| is the correct semantics: the flag should be emitted whenever either value is non-default. 0/0 still correctly emits nothing.
Proposed fix: standardize the docker/nerdctl and crictl guards on || to match ctr. Optionally add a mismatched-pair case (e.g. UID: 65534, GID: 0)

[shivansh-source]

@cmainas i think && should be changed by ||

[ananos]

Thanks @shivansh-source, correct -- the three guards do differ, and this dates back to the commit that introduced them, so it is an accidental inconsistency rather than a deliberate choice. || is indeed the intended semantics.

That said, the impact is currently zero: every test case in test_cases.go uses matched pairs (0/0 or 65534/65534), so no test misbehaves today. This is a latent inconsistency in the test infra, not a bug in urunc.

On the proposed fix: flipping the two && to || alone is not really testable -- without a mismatched-pair case nothing exercises the change. And adding such a case across docker, nerdctl, crictl and ctr means four more entries in an already long e2e matrix, so it is not obvious the extra PR test time is worth it for a purely latent issue.

So we let's keep this open and consider both the guard alignment and whether a mismatched-pair case earns its CI time in the next e2e cleanup pass.

Regardless, thanks for the report!

[shivansh-source]

got it . @ananos thanks for the review

[shivansh-source]

the guards sit in pure builder funcs that just return a command string , and no container is involved .
test with a mismatched pair can assert the -u flag is emitted — without adding anything to the e2e matrix or spending container time .
make unittest doesn't currently run ./tests/e2e, and go test ./tests/e2e would pull in the ginkgo TestE2E suite. So it'd need a filtered target (go test ./tests/e2e -run TestNewContainerCmd) wired into unittest @ananos

[shivansh-source]

i can bundle the guard allignment + test + that make wiring into one small pr .