search

redhat-actions / push-to-registry

GitHub Action to push a container image to an image registry.
https://github.com/marketplace/actions/push-to-registry
MIT License
100 stars 33 forks source link

[BUG] v2.4 breaks pushing multiple tags at once #57

Closed PhrozenByte closed 3 years ago

PhrozenByte commented 3 years ago

Version

redhat-actions/push-to-registry@v2 (which now references redhat-actions/push-to-registry@v2.4 by default)

Describe the bug

Prior to #50 passing multiple tags caused the action to push all listed tags separately. Since v2.4 it rather assumes the first tag to be some sort of "primary" tag and pushes all following tags as aliases of this "primary" tag. It falsely assumes that all tags reference the same digest.

The input tags: foo bar executed the following prior to #50:

podman push my-image:foo quay.io/my-user/my-image:foo
podman push my-image:bar quay.io/my-user/my-image:bar

Starting with v2.4 it executes the following:

podman push my-image:foo quay.io/my-user/my-image:foo
podman push my-image:foo quay.io/my-user/my-image:bar

This is a BC breaking change and silently broke my CI.

I'm not sure whether this was an intended change. In any case, v2.4 needs to be withdrawn. If it wasn't intended, it needs fixing. If it was intended, it needs to be released as v3 instead, because it includes BC-breaking changes (following https://semver.org).

If it was intended I'd like to suggest removing the requirement for all tags to exist locally (except the primary tag of course).

tetchel commented 3 years ago

this was not intended to be a breaking change. You can reference v2.3 in order to fix your pipeline while we investigate the problem.

divyansh42 commented 3 years ago

@PhrozenByte I have verified this and can confirm this got introduced in v2.4. However, this change is not intended, so we'll take this as a bug and fix it very soon. Meanwhile, I'd request you to use v2.3 to fix your pipeline.

If it was intended I'd like to suggest removing the requirement for all tags to exist locally (except the primary tag of course).

Thanks for your suggestion, this seems to be valid but as mentioned above this change was not intended and this enhancement requires a major release (v3) for which we haven't planned yet. So, I would request you to create an enhancement request for this, we can definitely take this in once we plan for v3.

PhrozenByte commented 3 years ago

Thanks :+1: I already fixed the issue in my CI by using the action twice for both images I build. I still push multiple tags per image, but those are indeed aliases.

Honestly I like the idea of interpreting multiple tags as aliases. It kinda feels "natural". Thus I wasn't sure whether it was an intended change or not. It just opened #58 for a discussion about how the inputs should behave in a proposed v3.

divyansh42 commented 3 years ago

@PhrozenByte We have changed back to this behaviour, when input tags: foo bar

podman push my-image:foo quay.io/my-user/my-image:foo
podman push my-image:bar quay.io/my-user/my-image:bar

Fix for this is released in v2.4.1 and also rolled forward tags v2 and v2.4. @PhrozenByte could you please confirm this fix?

PhrozenByte commented 2 years ago

@PhrozenByte could you please confirm this fix?

Works. Thank you! :+1:

divyansh42 commented 2 years ago

Works. Thank you! +1

Great! Thanks for the confirmation :slightly_smiling_face: