Is this comment still needed?

We might not be able to reimplement Config-reading filters with c8d filter right now, but maybe in future we could work out something that would allow us to perform more filtering on the containerd side.
So while the TODO itself is not immediately actionable, it still mentions an existing issue.
But yeah, at least It could be rephrased a bit.

This is confusing me; are we checking the same thing twice here? (Is this correct?)

No, consider this:
label!=some.image.label=asdf

fltrName=label!
k=some.image.label
v=asdf

negate := strings.HasSuffix(fltrName, "!") checks if the filter name implies negation.
if strings.HasSuffix(k, "!") { checks if the "operator" is !=

This is a bit awkward, because now we have both label! as a filter (to be backwards compatible with non-c8d Docker) and != as a operator filter expression (to make it consistent with containerd filters, and we probably might want to switch completely to them in future).
So something like this will also work: label!=some.image.label!=asdf (but would be equal to label=some.image.label=asdf).

This is a bit awkward, because now we have both label! as a filter (to be backwards compatible with non-c8d Docker) and != as a operator filter expression

Yeah the label! was one giant hack, but I wondered if we weren't already handling that with the first step (if we see the filter "name" is label! we could trim that, and set the operator to !=.

(I'm rambling; I need to have another look at the code)

So something like this will also work: label!=some.image.label!=asdf (but would be equal to label=some.image.label=asdf).

Did that work before? 🤯 I wonder if we ever designed that, or if it's (like many things with our filtering) just "happened to be there".

Did that work before? 🤯

Yep! (it was available only for image prune though, not image list)

$ docker info -f '{{.Driver}}'
overlay2

$ docker image prune -a --filter 'label!=org.opencontainers.image.version!=22.04'
WARNING! This will remove all images without at least one container associated to them.
Are you sure you want to continue? [y/N] y
Deleted Images:
untagged: ubuntu:latest
untagged: ubuntu@sha256:67211c14fa74f070d27cc59d69a7fa9aeff8e28ea118ef3babc295a0428a6d21
deleted: sha256:bab8ce5c00ca3ef91e0d3eb4c6e6d6ec7cffa9574c447fd8d54a8d96e7c1c80e
deleted: sha256:874b048c963ab55b06939c39d59303fb975d323822a4ea48a02ac8dc635ea371

Wondering if the reverse would work, and if that would be a more "natural" approach; return an errdefs.NotFound(), and then handle the error like;

		if err != nil {
			if !errdefs.IsNotFound(err) {
				logrus.WithFields(logrus.Fields{
					logrus.ErrorKey: err,
					"image":         image.Name,
					"checks":        checks,
				}).Error("failed to check image labels")
			}
			return false
		}
		return true

Nope, we want to exit as soon as we find a matching config from that image - if we already know that this image has a config that matches filters, we don't need to check anything else. Returning an "operation failed successfully" will make the whole Dispatch stop and we'll know it stopped because it found what we were looking for.

Oh! Gotcha, yes, the error is to make the images.Dispatch cancel (it doesn't if images.HandlerFunc() returns a nil-error)!

Got it now. Perhaps that would warrant a comment either at the return nil, errFoundConfig line, or at the errFoundConfig := errors.New(.... line (or perhaps both); WDYT?

Added small comments to both.

oh! just opened vvoland#5 committing my suggestons; let me know if those look good to you 😂 (classic race condition)