Checking for multiple registries when login/logout

[lslavkov]

Creating a check for podman login if there is a multiple registries in the registry.conf to avoid any issues where credential getting replaced by docker login

Fixes #22400

[openshift-ci]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[packit-as-a-service]

Ephemeral COPR build failed. @containers/packit-build please check.

[rhatdan]

Please rebase and squash your commit.

Thanks for the PR.

[rhatdan]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lslavkov, rhatdan

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [rhatdan]

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

[lslavkov]

@rhatdan I was able to squash it but my commit description got messed up. Hope that is nothing groundbreaking issue

[rhatdan]

Any chance you could add a test?

[lslavkov]

Could try but no promises.

[lslavkov]

@rhatdan should I create separate file for this test or adding additional test case in test/e2e/login_logout_test.go?

[mheon]

@lslavkov Existing login_logout_test file is the preferred place, thanks!

[Luap99]

IMO this is a breaking change and shouldn't be done without a major version. And if we want this then I think it needs to go into c/common so we can make skopeo and buildah match the behavior.

[Luap99]

This is a breaking change so we cannot accept this until a 6.0 IMO.
And most importantly this code is broken. You always reject more than one registry even if one was specifed as argument on the cli (in the args array)

Any the login code seem shared between buildah and skopeo as well so if we decide to fix this we should fix this for all in https://github.com/containers/common/blob/main/pkg/auth/auth.go

And if you look into that code you see there is already an AcceptUnspecifiedRegistry option which is quite whay you do here but it we can add another case there for this new behavior.
https://github.com/containers/common/blob/514bf04d8e6a6ac19b71147b534875ca8001785f/pkg/auth/auth.go#L137

[lslavkov]

So if I understand correctly that registriesFromFile is gonna fail regardless if we add which registry we wish to login?

I understand your point now. The approach there is more elegant that this one. Do you want me to modify to something similar to that? https://github.com/containers/common/blob/514bf04d8e6a6ac19b71147b534875ca8001785f/pkg/auth/auth.go#L137

[lslavkov]

/assign

[lslavkov]

@Luap99 should I close this PR and redirect my focus to the c/common?

[Luap99]

I think it would be best to add this in c/common behind a new option yes.
Then we can update it in the callers by setting that new option. I still think such a change can be breaking so likely something that should wait for a major version.