podman run --authfile do not stat file

[Luap99]

No other commands do this, it makes no sense to fail hard if the given authfile does not exists. We do not even know if we have to pull anything at that point. I expect the code to error at some later point if the auth file is really needed but does not exists.

Fixes #18405

Does this PR introduce a user-facing change?

None

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [Luap99]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[vrothberg]

Test isn't happy yet

[+1516s] [Fail] Podman run [It] podman run should fail with nonexistent authfile
[+1516s] /var/tmp/go/src/github.com/containers/podman/test/e2e/run_test.go:1572

I wonder why this is explicitly tested.

[edsantiago]

Background seems to be issue #4328, but there's no explanation of why that seemed important.

[vrothberg]

Background seems to be issue #4328, but there's no explanation of why that seemed important.

I somehow expect Podman to fail if the user specified a non-existing file. Could very well be a typo etc.

[Luap99]

I somehow expect Podman to fail if the user specified a non-existing file. Could very well be a typo etc.

I agree but why should podman run fail when the file is not even used? The auth backend code should error out (I haven't verified that). And also why only podman run, podman create is fine so I find this very inconsistent.

[vrothberg]

I somehow expect Podman to fail if the user specified a non-existing file. Could very well be a typo etc.

I agree but why should podman run fail when the file is not even used? The auth backend code should error out (I haven't verified that).

That may lead to inconsistent behavior between local and remote as remote would send the credentials over to the server (independent if we need them or not).

And also why only podman run, podman create is fine so I find this very inconsistent.

I concur. The commands should behave consistently.

[Luap99]

Looking at #4337 it looks like it used to be in create as well, so this is a regression.
I think there is a important difference between podman run --authfile=/tmp/nonexistent and REGISTRY_AUTH_FILE=/tmp/nonexistent podman run.
For the --authfile version I think it is fine to error out but for the env var I think it is better to ignore it. Just consider @edsantiago's use case in the test here.

I also find it questionable how the REGISTRY_AUTH_FILE env var is handled. It only used in the cli fronted as default value for --authfile and then passed down the stack from there. This causes all code paths which do not have --authfile option to ignore this env var. For example running podman system service will not cause it to use this file for the /auth endpoint, bug??? I would expect that the default REGISTRY_AUTH_FILE is read in the auth code backend when no path is explicitly set by the caller and podman passes an empty string down if --authfile is unset. Right now this var is passed along from the podman, buildah and skopeo cli. This seems like a huge maintenance burden instead of just having exactly one place where to read this env var when the input authfile is empty.

[vrothberg]

I am not that concerned about how it's handled. The docs are pretty explicit about the env var only affecting the default of the CLI flag.

[github-actions]

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

[rhatdan]

@Luap99 @vrothberg What is going on with this PR?

[Luap99]

Seems like this is the expected behaviour so I close this it. My concerns are still valid IMO but I do not maintain any of this auth code so I don't care.