Skip to content

Fix recognizing “tag not found” in oci:, and add that for oci-archive:#2613

Merged
mtrmac merged 3 commits intocontainers:mainfrom
mtrmac:archive-not-found
Jun 25, 2025
Merged

Fix recognizing “tag not found” in oci:, and add that for oci-archive:#2613
mtrmac merged 3 commits intocontainers:mainfrom
mtrmac:archive-not-found

Conversation

@mtrmac
Copy link
Copy Markdown
Contributor

@mtrmac mtrmac commented May 28, 2025

It turns out the oci: “tag not found” handling in #1757 never worked (it used a value equality check instead of a type conformance check). Fix that, and add a unit test.

Also, implement the same semantics for oci-archive:, per #2114. Only manually tested.

Unlike #2114, this does not add “exit status 2” for “the whole archive is missing”. I don’t think that needs special handling, and it seems no caller currently cares.

@mtrmac mtrmac changed the title Fix recognizing "tag not found" in oci:, and add that for oci-archive: Fix recognizing “tag not found” in oci:, and add that for oci-archive: May 28, 2025
@mtrmac mtrmac added kind/bug A defect in an existing functionality (or a PR fixing it) kind/feature A request for, or a PR adding, new functionality labels May 28, 2025
@cgwalters
Copy link
Copy Markdown
Collaborator

It turns out the oci: “tag not found” handling in #1757 never worked (it used a value equality check instead of a type conformance check). Fix that, and add a unit test.

Thanks for fixing that. Out of curiosity how did you find this?

@cgwalters
Copy link
Copy Markdown
Collaborator

BTW it looks like I lost my "official reviewer" status on this repo, do you want to readd it?

@mtrmac
Copy link
Copy Markdown
Contributor Author

mtrmac commented May 29, 2025

Out of curiosity how did you find this?

I added oci-archive: using the same code pattern, and in manual testing it turned out not to work.


BTW it looks like I lost my "official reviewer" status on this repo, do you want to readd it?

That might have happened while formalizing the rules (for CNCF, around #2570) due to a disconnect between GitHub permissions and text records… and now there’s a bit of a process involved. I’ll ask around.

@mtrmac mtrmac force-pushed the archive-not-found branch 4 times, most recently from 124b952 to 1166a33 Compare June 4, 2025 19:37
@mtrmac mtrmac force-pushed the archive-not-found branch from 1166a33 to cda678d Compare June 5, 2025 21:00
@mtrmac mtrmac force-pushed the archive-not-found branch 2 times, most recently from a584d98 to bc3c2c8 Compare June 23, 2025 17:19
@lsm5
Copy link
Copy Markdown
Member

lsm5 commented Jun 23, 2025

@mtrmac do we need any systemtest updates here?

@mtrmac
Copy link
Copy Markdown
Contributor Author

mtrmac commented Jun 24, 2025

@lsm5 Specifically system tests? This is upstream-motivated, not tracked in a downstream product, so I don’t think there’s a reason to have a system test and not an integration tests (but there could well be something I’m missing).

Arguably the oci-archive: code path should have an automated test, it doesn’t have one now.

Copy link
Copy Markdown
Member

@lsm5 lsm5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

mtrmac added 3 commits June 25, 2025 20:54
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
No tests because we aren't testing oci-archive: anywhere
else either.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@mtrmac mtrmac force-pushed the archive-not-found branch from bc3c2c8 to 99c6fb0 Compare June 25, 2025 18:55
@mtrmac mtrmac merged commit c49bbbe into containers:main Jun 25, 2025
22 checks passed
@mtrmac mtrmac deleted the archive-not-found branch June 25, 2025 21:18
@stale-locking-app stale-locking-app Bot locked as resolved and limited conversation to collaborators Sep 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

kind/bug A defect in an existing functionality (or a PR fixing it) kind/feature A request for, or a PR adding, new functionality locked - please file new issue/PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants