image: replace GetStoreImage with ResolveReference and bump c/image to 373c52a9466f

[flouthoc]

GetStoreImage is deprecated after containers/image#2056, similar to PR: containers/podman#20560

What type of PR is this?

/kind api-change
/kind bug
/kind cleanup
/kind deprecation
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake
/kind other

What this PR does / why we need it:

How to verify it

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

vendor: bump `c/image` to `373c52a9466f`

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc

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:

• OWNERS [flouthoc]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[flouthoc]

@containers/buildah-maintainers @giuseppe @vrothberg PTAL

[rhatdan]

LGTM

[flouthoc]

Rebased.

[mtrmac]

ACK overall, it seems that the surrounding code really has no use for the other return value.

[flouthoc]

@mtrmac @nalind PTAL I think now patch implements #5129 (comment) and removes the need for error matching.

[flouthoc]

@mtrmac Done.

[mtrmac]

LGTM. Thanks for dealing with this breakage!

[nalind]

I don't love moving from a tagged release to a commit, but otherwise it's fine.

[mtrmac]

From my POV this is not urgent — it can be included in the next vendor dance. It’s just that with the added deprecation notices, these changes will probably have to be included with that vendor dance.

I’m also open to not immediately adding the depreciation notices, so that there is a tagged version of c/image which contains ResolveImage, and callers can update it without the urgency of a deprecation warning breaking a build. OTOH that deprecation notice / build breakage usually is what motivates the migration… and, as this PR shows, revisiting the call sites to see if they should use ResolveReference is probably worthwhile.

[flouthoc]

I don't love moving from a tagged release to a commit, but otherwise it's fine.

@nalind I'll vendor c/image as soon as it out, just need c/image to be bumped so i can add integration test for ( containers/image#1645 ) to buildah upstream.

[flouthoc]

@mtrmac I have checked podman already and bumped c/image there containers/podman#20560 and for buildah this PR will do the trick other than this there is only skopeo which can be affected by c/image change ( not sure if there are any other tools )

[mtrmac]

Skopeo does not call the deprecated functions directly.

There’s also c/common, and at a first glance the two calls probably should use ResolveReference to create a storage reference which both fixes an image ID and references a specific digest (instead of creating an ID-only image.storageReference). That would affect the 11 callers of libimage.Image.StorageReference. From a quick check that should all work fine, but it’s possible I have missed a corner case.

[mtrmac]

@flouthoc Do you need a new c/image release soonish, then, or should this all happen with the Podman 4.8 vendor dance?

[flouthoc]

@flouthoc Do you need a new c/image release soonish, then, or should this all happen with the Podman 4.8 vendor dance?

@mtrmac I don't think there is any need for c/image release from my-end so there no rush from my side.

[mtrmac]

#5143 will need a c/image updated with these changes, so I’d prefer merging this before the main vendor dance.

[mtrmac]

I don't love moving from a tagged release to a commit, but otherwise it's fine.

The Podman 4.8 vendoring process was discussed today in context of #5143 , and the plan for that PR is to refer to a commit in dependencies, with the expectation that vendor dance will clean that up soon.

@flouthoc I am seeing “Unit tests w/overlay” consistently falling on a total timeout, with about the same place. This seems to also affect other PRs.

[flouthoc]

I think timout was a temp flake.

[mtrmac]

/lgtm