Add --compat-auth-file to login and logout #5143
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mtrmac
force-pushed
the
docker-compat-login
branch
from
9af684d to
482c6f3
Compare
|
|
mtrmac
force-pushed
the
docker-compat-login
branch
6 times, most recently
from
bcd6855 to
e3e6a18
Compare
mtrmac
commented
Nov 9, 2023
tests/authenticate.bats
Outdated
| @@ -152,3 +167,31 @@ EOM | |||
| expect_output --substring "authentication required" \ | |||
| "buildah push after buildah logout" | |||
| } | |||
|
|
|||
| @test "authenticate: --compat-auth-file" { | |||
| skip_if_no_docker | |||
There was a problem hiding this comment.
Apparently no instance of our CI runs with Docker, so this code is completely untested (and right now, incomplete WRT expected output).
I’ll smoke-test the operation manually, and drop this.
| @@ -192,7 +192,7 @@ tests/testreport/testreport: tests/testreport/testreport.go | |||
| .PHONY: test-unit | |||
| test-unit: tests/testreport/testreport | |||
| $(GO_TEST) -v -tags "$(STORAGETAGS) $(SECURITYTAGS)" -cover $(RACEFLAGS) $(shell $(GO) list ./... | grep -v vendor | grep -v tests | grep -v cmd | grep -v chroot | grep -v copier) -timeout 45m | |||
| $(GO_TEST) -v -tags "$(STORAGETAGS) $(SECURITYTAGS)" $(RACEFLAGS) ./chroot ./copier -timeout 45m | |||
| $(GO_TEST) -v -tags "$(STORAGETAGS) $(SECURITYTAGS)" $(RACEFLAGS) ./chroot ./copier -timeout 60m | |||
There was a problem hiding this comment.
Note this unrelated change. The tests keep timing out, so I just bumped the maximum.
One recent success took 47m.
It would probably be worthwhile to spend some time speeding up the tests, but that’s not this PR.
|
Manually smoke-tested operation and interoperability with # docker pull localhost:5000/foo
Using default tag: latest
Error response from daemon: Head "http://localhost:5000/v2/foo/manifests/latest": no basic auth credentials
# bin/buildah login --tls-verify=false --compat-auth-file ~/.docker/config.json localhost:5000
Username: …
Password:
Login Succeeded!
# docker pull localhost:5000/foo
Using default tag: latest
Error response from daemon: manifest for localhost:5000/foo:latest not found: manifest unknown: manifest unknown
# bin/buildah logout --compat-auth-file ~/.docker/config.json localhost:5000
Removed login credentials for localhost:5000
# docker pull localhost:5000/foo
Using default tag: latest
Error response from daemon: Head "http://localhost:5000/v2/foo/manifests/latest": no basic auth credentials |
mtrmac
force-pushed
the
docker-compat-login
branch
from
906a599 to
881337e
Compare
|
Tests are passing... |
|
Yes; c/image and c/common must be merged first. |
This was referenced Nov 10, 2023
mtrmac
force-pushed
the
docker-compat-login
branch
from
881337e to
956f2ec
Compare
|
Rebased on top of merged Cc: @flouthoc . |
mtrmac
changed the title
mtrmac
force-pushed
the
docker-compat-login
branch
from
956f2ec to
a3910f4
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: flouthoc, mtrmac 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:
Approvers can indicate their approval by writing |
... to include containers/image#2173 and containers/common#1731 . Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This mostly just inherits the c/common/pkg/auth implementation, except that AuthFilePath and DockerCompatAuthFilePath can not be set simultaneously, so don't always set AuthFilePath. c/common already defaults to the same locations internally. Test handle only invalid commands; a true interoperability test would require a running Docker on the CI systems, which is not currently available. That interoperability was tested manually (and is presumed to be integration-tested in the Podman repo). Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Warning: I don't know what I'm doing, I just don't care to deal with this now. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
mtrmac
force-pushed
the
docker-compat-login
branch
from
a3910f4 to
4cf1673
Compare
|
LGTM |
|
/lgtm |
mtrmac
added a commit
to mtrmac/libpod
that referenced
this pull request
Nov 17, 2023
... to include containers/image#2173, containers/common#1731 and containers/buildah#5143 . Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
What this PR does / why we need it:
Add
--compat-auth-fileto login/logout, allowing to update Docker-compatible files.This mostly inherits the feature from c/common/pkg/auth.
Completely untested at this point.How to verify it
Existing CI for existing features.
Some minimal smoke tests; and manual tests that the updated files can be correctly consumed by Docker.
Which issue(s) this PR fixes:
None
Special notes for your reviewer:
Does this PR introduce a user-facing change?