cockpit-project / cockpit-podman

Cockpit UI for podman containers
GNU Lesser General Public License v2.1
435 stars 90 forks source link

fix searching on non-searchable image registries #1821

Closed tomasmatus closed 3 days ago

tomasmatus commented 1 month ago

fixes: #1220

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.

Screen Shot 2024-10-01 at 08 48 35

garrett commented 1 month ago

I already had this container on my system:

image

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

image

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 image

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

image

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.

tomasmatus commented 1 month ago

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 commented 1 month ago

I believe this failure is our well-known flake #1836, so ignore here.

The TF failures are due to

Failed to start nginx.service: Unit nginx.service not found.

You can fix that by adding nginx to the dependency list in test/browser/main.fmf

garrett commented 1 month ago

It's getting there!

I found two interaction problems so far:

  1. 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.
  2. 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).

martinpitt commented 1 month ago

@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 commented 1 month ago

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

yes, I agree with this, let's not

tomasmatus commented 1 week ago

@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 commented 1 week ago

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.

martinpitt commented 1 week ago

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

martinpitt commented 3 days ago

@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)