fix searching on non-searchable image registries #1821

tomasmatus · 2024-08-14T12:28:12Z

Add support to pull images from registries which do not support search API.

https://issues.redhat.com/browse/COCKPIT-1148

Podman: pull images from registries without search API

When creating a new container it is now possible to use registries that do not support API for searching containers such as ghcr.io. It is also possible to select a specific tag for the image.

src/ImageRunModal.jsx

garrett · 2024-08-14T13:33:04Z

I already had this container on my system:

I tried a few example ones, but perhaps they're not actually there...

Then I tried a container that GitHub talked about called "Super Linter", which has this as a container URI:
ghcr.io/github/super-linter@sha256:b723c817982b85f6f98c9c24a653f9f760bf5941577f5ea9532d31bd89aa8823
...as seen @ https://github.com/orgs/github/packages/container/super-linter/207523715

If you add the URI without the @..., it seems like it'd work:

But you have to specifically click on the item in the dropdown. (Is that intended?) If you change the focus, the entry goes blank. Also, if you try to edit the text (like if you have a typo, like copying the URI incorrectly), the entry also goes blank.

However, it does seem to work if you click on the entry, so that's great!

I see a related discussion at https://github.com/orgs/community/discussions/26279 which uses tags/list but it seems things need a token, so I guess that won't work for us.

martinpitt

Cheers! Please update the commit message to explain the change more verbosely (use case, reference to https://issues.redhat.com/browse/COCKPIT-1148 and the correspondig upstream and RHEL issues).

src/ImageRunModal.jsx

test/check-application

tomasmatus · 2024-08-20T12:55:49Z

fedora 41 fails because of this:


[root@fedora-41-127-0-0-2-2201 ~]# podman manifest inspect localhost:80/my-busybox
Error: parsing manifest blob "{\"schemaVersion\":2,\"mediaType\":\"application/vnd.oci.image.manifest.v1+json\",\"config\":{\"mediaType\":\"application/vnd.oci.image.config.v1+json\",\"digest\":\"sha256:02154912ea3e0a26dd3d9878cc35592995a559c1894ed9f1f3ad2e67c601e8a3\",\"size\":1051},\"layers\":[{\"mediaType\":\"application/vnd.oci.image.layer.v1.tar+zstd\",\"digest\":\"sha256:979ef0eafe970832b8df3c3233c50a1ceb863501afd0fdd0819f4e74459a269b\",\"size\":783654,\"annotations\":{\"io.github.containers.zstd-chunked.manifest-checksum\":\"sha256:1818e53836a49be9f33ab634b8cecc242402366faff167cc29c346acba6c786c\",\"io.github.containers.zstd-chunked.manifest-position\":\"769759:3878:48899:1\",\"io.github.containers.zstd-chunked.tarsplit-position\":\"773645:9937:339414\"}},{\"mediaType\":\"application/vnd.oci.image.layer.v1.tar+zstd\",\"digest\":\"sha256:5126ce266a4b0023a57d535a42dfdedee252be565ab10967140a8cf5c2babfc6\",\"size\":973884,\"annotations\":{\"io.github.containers.zstd-chunked.manifest-checksum\":\"sha256:9a91d82526737b46f2f6e5d9478f597fe989943445cda912aa8af5e28b7351c9\",\"io.github.containers.zstd-chunked.manifest-position\":\"859954:57329:277072:1\",\"io.github.containers.zstd-chunked.tarsplit-position\":\"917291:56521:1414602\"}}]}" as a "application/vnd.oci.image.manifest.v1+json": Treating single images as manifest lists is not implemented

I checked f40 and it gives a warning rather than "not implemented" error. Both are on version 5.2.0 idk what this means/how to fix for now, will look into it

martinpitt

Thanks for the fixes! Next round.

src/ImageRunModal.jsx

test/check-application

garrett · 2024-09-03T08:17:57Z

It's getting there!

I found two interaction problems so far:

It doesn't strip spaces at the end (or start), so if I copy a URI from somewhere and it has a space around it, it fails.
Can't use the keyboard: After pasting the URI, hitting enter fails; it empties the line. I would expect it to accept the URL, so I could continue configuring it.

I was testing with ghcr.io/cockpit-project/tasks-tmp:10449236342.1-arm64 (which had a space when I copied it), which I found at https://github.com/cockpit-project/cockpituous/pkgs/container/tasks-tmp (as it's on ghcr and isn't our standard tasks container, which I'm using as my dev environment in toolbox already).

garrett

(fix the issues with the spaces and hitting enter, as pointed out above; thanks!)

src/ImageRunModal.jsx

martinpitt · 2024-09-03T08:53:21Z

src/ImageRunModal.jsx

+                searches.push(...utils.fallbackRegistries.map(registry =>
+                    this.activeConnection.call({
+                        method: "GET",
+                        path: client.VERSION + "libpod/images/search",
+                        body: "",
+                        params: {
+                            term: registry + "/" + value


This is a significant piece of code, please add a test for fallback registries.

martinpitt · 2024-09-03T08:54:52Z

test/check-application

+        # Searching for container by prefix fails
+        b.set_input_text("#create-image-image-select-typeahead", "localhost:80/my-busy")
+        b.wait_text("button.pf-v5-c-select__menu-item.pf-m-disabled", "No images found")
+        b.wait_in_text(".pf-v5-c-alert.pf-m-danger", "couldn't search registry")


This shouldn't be "danger", but "info" or "default". (It needs to be changed in the code, but it's visible here)

I can't think of a way how to set it to "info" for this specific error message. I've learnt from the previous reviews that msg.includes("couldn't search registry") is a no go 😅

src/ImageRunModal.jsx

martinpitt · 2024-09-03T08:57:28Z

test/check-application

+        b.wait_text("button.pf-v5-c-select__menu-item.pf-m-disabled", "No images found")
+        b.wait_in_text(".pf-v5-c-alert.pf-m-danger", "couldn't search registry")
+
+        # Searching for full url with tag finds the image


s/url with //

src/ImageRunModal.jsx

martinpitt · 2024-09-03T09:03:10Z

@tomasmatus thanks! I cleaned up outdated review threads and comments to make the remaining stuff easier to see. There's a few small things to fix; if you really don't want to rewrite/simplify the search result loop (which is so hard to read and so easy to get wrong!), perhaps @jelly can help with that, or also me when I'm back from PTO. I also wouldn't veto landing it as-is and cleaning up later, if @jelly is ok with it. But in three weeks nobody of us will be able to follow that logic..

tomasmatus · 2024-09-03T09:42:23Z

I also wouldn't veto landing it as-is and cleaning up later.

yes, I agree with this, let's not

tomasmatus · 2024-09-25T14:43:43Z

@martinpitt if you'd like to have a look it is this Select that is misbehaving.

What happens is when you type something in and press enter the input bar is cleared rather than preserve the value. It is also cleared once the input loses focus. This shouldn't be happening as the isInputValuePersisted prop is used. I haven't figured out if this is a bug in the now deprecated PF4 Select or if it is somehow cleared in our code.

martinpitt · 2024-09-26T05:32:25Z

I can reproduce the issue on main. AFAICS the only sensible thing is to ignore the Enter key completely then -- if you type a random string into the image bar, it cannot be "accepted" as a valid value (which is usually the meaning of "Enter"). You must actually select a valid image for the dialog to make sense. Clearing an invalid value on focus change also makes sense IMHO.

However, I do see how this would collide with the workflow of this PR -- it'd still force you to actually click on the search result instead of just copy&pasting an image name and doing nothing else. Did I get this right?

The demo on the PF docs (v4, as that's the deprecated one) behaves in the above way: "Enter" is a no-op, and changing focus clears the input value. This doesn't do any special keyboard input tricks.

My hunch is that it's the <FormGroup> or its <Popover> which react to Enter, as it focuses the (?) and opens the popup help. The same happens on e.g. the "Memory limit" checkbox -- pressing Enter there also opens the "Owner" popup help.

Pressing "Enter" in the dialog on any active form element (such as the search input or the checkboxes) previously bubbled up to the Form, which then activated the first popover help. This is unexpected and very irritating especially when using the search input. Just ignore the Enter key to fix that, similar to what the image search modal already does. See cockpit-project#1821 (comment)

martinpitt · 2024-09-26T08:19:21Z

@tomasmatus Fixed in #1861 . This is independent from this PR.

Pressing "Enter" in the dialog on any active form element (such as the search input or the checkboxes) previously bubbled up to the Form, which then activated the first popover help. This is unexpected and very irritating especially when using the search input. Just ignore the Enter key to fix that, similar to what the image search modal already does. See cockpit-project#1821 (comment)

martinpitt

Cheers @tomasmatus ! The previous review still has three current threads, and I have some more questions. This is coming together!

src/ImageRunModal.jsx

hit the wrong button, sorry

Pressing "Enter" in the dialog on any active form element (such as the search input or the checkboxes) previously bubbled up to the Form, which then activated the first popover help. This is unexpected and very irritating especially when using the search input. Just ignore the Enter key to fix that, similar to what the image search modal already does. See cockpit-project#1821 (comment)

Pressing "Enter" in the dialog on any active form element (such as the search input or the checkboxes) previously bubbled up to the Form, which then activated the first popover help. This is unexpected and very irritating especially when using the search input. Just ignore the Enter key to fix that, similar to what the image search modal already does. See #1821 (comment)

Garrett's feedback was addressed

fixes: cockpit-project#1220 Add support to pull images from registries which do not support search API.

cockpituous · 2024-09-30T16:44:07Z

src/ImageRunModal.jsx

+            if (dialogError === "" && dialogErrorDetail === "") {
+                dialogError = _("Failed to search for new images");
+                // TODO: add registry context, podman does not include it in the reply.
+                dialogErrorDetail = reason ? cockpit.format(_("Failed to search for images: $0"), reason.message) : _("Failed to search for images.");


This added line is not executed by any test.

cockpituous · 2024-09-30T16:44:07Z

src/ImageRunModal.jsx

+        if (!RE_CONTAINER_TAG.test(value)) {
+            // If there are registries configured search in them, or if a user searches for `docker.io/cockpit` let
+            // podman search in the user specified registry.
+            if (Object.keys(this.props.podmanInfo.registries).length !== 0 || value.includes('/')) {


This added line is not executed by any test.

cockpituous · 2024-09-30T16:44:07Z

src/ImageRunModal.jsx

+                searches.push(...utils.fallbackRegistries.map(registry =>
+                    this.activeConnection.call({
+                        method: "GET",
+                        path: client.VERSION + "libpod/images/search",
+                        body: "",
+                        params: {
+                            term: registry + "/" + value


These 7 added lines are not executed by any test.

cockpituous · 2024-09-30T16:44:07Z

src/ImageRunModal.jsx

-                                dialogError = _("Failed to search for new images");
-                                // TODO: add registry context, podman does not include it in the reply.
-                                dialogErrorDetail = result.reason ? cockpit.format(_("Failed to search for images: $0"), result.reason.message) : _("Failed to search for images.");
+                            } else if (!manifestResult && !imageExistsLocally(value, this.props.localImages)) {


This added line is not executed by any test.

martinpitt

Wohoo! Thanks!

martinpitt · 2024-09-30T17:00:27Z

@tomasmatus Can you please add an initial release note to the description, and an up to date screenshot? (Please use the inspector to cut out the dialog precisely, and add a border: 2px solid)

tomasmatus commented Aug 14, 2024

View reviewed changes

src/ImageRunModal.jsx Outdated Show resolved Hide resolved

tomasmatus force-pushed the non-searchable-repos branch from 9934989 to c0b19df Compare August 14, 2024 14:14

tomasmatus requested a review from martinpitt August 19, 2024 08:16

tomasmatus force-pushed the non-searchable-repos branch from c0b19df to bfa093e Compare August 19, 2024 08:29

martinpitt requested changes Aug 19, 2024

View reviewed changes

tomasmatus force-pushed the non-searchable-repos branch 2 times, most recently from f916975 to 5a5fe4a Compare August 20, 2024 11:37

tomasmatus force-pushed the non-searchable-repos branch from 5a5fe4a to 24d666e Compare August 21, 2024 08:01

martinpitt reviewed Aug 21, 2024

View reviewed changes

src/ImageRunModal.jsx Outdated Show resolved Hide resolved

src/ImageRunModal.jsx Outdated Show resolved Hide resolved

src/ImageRunModal.jsx Outdated Show resolved Hide resolved

martinpitt reviewed Aug 29, 2024

View reviewed changes

src/ImageRunModal.jsx Outdated Show resolved Hide resolved

src/ImageRunModal.jsx Outdated Show resolved Hide resolved

martinpitt reviewed Aug 29, 2024

View reviewed changes

src/ImageRunModal.jsx Show resolved Hide resolved

src/ImageRunModal.jsx Outdated Show resolved Hide resolved

src/ImageRunModal.jsx Outdated Show resolved Hide resolved

test/check-application Outdated Show resolved Hide resolved

test/check-application Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

tomasmatus requested a review from martinpitt September 2, 2024 11:28

garrett previously requested changes Sep 3, 2024

View reviewed changes

martinpitt reviewed Sep 3, 2024

View reviewed changes

tomasmatus force-pushed the non-searchable-repos branch from a1ceaef to 839df1d Compare September 24, 2024 11:05

martinpitt mentioned this pull request Sep 26, 2024

Fix irritating Enter key behaviour in container creation dialog #1861

Merged

martinpitt previously approved these changes Sep 26, 2024

View reviewed changes

src/ImageRunModal.jsx Outdated Show resolved Hide resolved

src/ImageRunModal.jsx Show resolved Hide resolved

src/ImageRunModal.jsx Show resolved Hide resolved

tomasmatus force-pushed the non-searchable-repos branch from 839df1d to e3a89f2 Compare September 30, 2024 12:50

martinpitt added the release-note label Sep 30, 2024

tomasmatus added 2 commits September 30, 2024 16:04

fix searching on non-searchable image registries

c0c87a7

fixes: cockpit-project#1220 Add support to pull images from registries which do not support search API.

fmf: add nginx dependency

5d7da5a

tomasmatus force-pushed the non-searchable-repos branch from e3a89f2 to 5d7da5a Compare September 30, 2024 14:06

cockpituous reviewed Sep 30, 2024

View reviewed changes

martinpitt approved these changes Sep 30, 2024

View reviewed changes

martinpitt merged commit 132f41f into cockpit-project:main Sep 30, 2024
34 checks passed

martinpitt removed the release-note label Oct 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix searching on non-searchable image registries #1821

fix searching on non-searchable image registries #1821

tomasmatus commented Aug 14, 2024 •

edited

Loading

garrett commented Aug 14, 2024

martinpitt left a comment

tomasmatus commented Aug 20, 2024

martinpitt left a comment

This comment was marked as resolved.

garrett commented Sep 3, 2024

garrett left a comment

martinpitt Sep 3, 2024

martinpitt Sep 3, 2024

tomasmatus Sep 30, 2024

martinpitt Sep 3, 2024

martinpitt commented Sep 3, 2024

tomasmatus commented Sep 3, 2024

tomasmatus commented Sep 25, 2024

martinpitt commented Sep 26, 2024 •

edited

Loading

martinpitt commented Sep 26, 2024

martinpitt left a comment

cockpituous Sep 30, 2024

cockpituous Sep 30, 2024

cockpituous Sep 30, 2024

cockpituous Sep 30, 2024

martinpitt left a comment

martinpitt commented Sep 30, 2024

fix searching on non-searchable image registries #1821

fix searching on non-searchable image registries #1821

Conversation

tomasmatus commented Aug 14, 2024 • edited Loading

Podman: pull images from registries without search API

garrett commented Aug 14, 2024

martinpitt left a comment

Choose a reason for hiding this comment

tomasmatus commented Aug 20, 2024

martinpitt left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

garrett commented Sep 3, 2024

garrett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinpitt commented Sep 3, 2024

tomasmatus commented Sep 3, 2024

tomasmatus commented Sep 25, 2024

martinpitt commented Sep 26, 2024 • edited Loading

martinpitt commented Sep 26, 2024

martinpitt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinpitt left a comment

Choose a reason for hiding this comment

martinpitt commented Sep 30, 2024

tomasmatus commented Aug 14, 2024 •

edited

Loading

martinpitt commented Sep 26, 2024 •

edited

Loading