Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v2 #50

Merged
merged 9 commits into from Mar 29, 2021
Merged

v2 #50

merged 9 commits into from Mar 29, 2021

Conversation

crazy-max
Copy link
Member

@crazy-max crazy-max commented Mar 24, 2021

Fixes #30
Fixes #41
Fixes #13
Fixes #27
Fixes #25
Closes #47
Closes #15

v2 introduces a new way to handle metadata for your Docker image as part of the proposal in #30.
Documentation: https://github.com/crazy-max/ghaction-docker-meta/tree/v2#readme

cc. @LongLiveCHIEF @hugopeixoto @mblaschke @felipecrs @kereis @slimcdk @whyman @CharlieC3 @lpil

@codecov
Copy link

codecov bot commented Mar 24, 2021

Codecov Report

Merging #50 (ceef405) into master (d541d2c) will decrease coverage by 1.38%.
The diff coverage is 71.80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #50      +/-   ##
==========================================
- Coverage   75.42%   74.04%   -1.39%     
==========================================
  Files           3        5       +2     
  Lines         118      339     +221     
  Branches       24       98      +74     
==========================================
+ Hits           89      251     +162     
+ Misses          7        5       -2     
- Partials       22       83      +61     
Impacted Files Coverage Δ
src/context.ts 85.71% <ø> (ø)
src/meta.ts 74.35% <70.44%> (-2.19%) ⬇️
src/tag.ts 71.11% <71.11%> (ø)
src/flavor.ts 88.23% <88.23%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9be43f0...ceef405. Read the comment docs.

@LongLiveCHIEF
Copy link

Still too tightly tied to explicit events. Decouple the events from the payload and things become even simpler, and far more versatile.

This is too narrow of a use case. What if I'm tagging the image based on the version of the software inside of it, and not the version of the image itself? What If i'm building an image based on events in another repo?

This is way more complex than it needs to be. Look at my current implementation.

  • It can solve the primary use this action does where you tag something based on refs from the event types listed here.
  • It can produce every type of tag you are producing here, without the complicated type= syntax.
  • It can additionally support parsing of "raw" tags using semver, allowing both of the use cases I described above.

It does all that, just by simply allowing the user to input the ref instead of trying to determine what ref to use based on the event type.

It's great how much this is thought through, but in the end, I think it's too over-engineered to be useful outside of a specific use-case, and that's what my proposal was about.

@crazy-max
Copy link
Member Author

crazy-max commented Mar 27, 2021

@LongLiveCHIEF

Still too tightly tied to explicit events.

That's the purpose of this action, handle metadata through events triggered by the workflow.

This is too narrow of a use case. What if I'm tagging the image based on the version of the software inside of it, and not the version of the image itself? What If i'm building an image based on events in another repo?

I think it's relative to your use case. Very few users that I know of use events with custom payloads but anyway it's still possible with v2:

      -
        id: buildxy
        run: echo '::set-output name=buildxy::${{ (github.event.action == 'deploy'|| github.event.action == 'released') && github.event.client_payload.tag_name == steps.stable-octoprint.outputs.release }}'
      -
        name: Tagging Strategy
        id: tagging
        uses: HackerHappyHour/tagging-strategy@v3
        with:
          image_name: octoprint/octoprint
          tags: |
            %X%::${{ steps.buildxy.outputs.buildxy }}
            %X.Y%::${{ steps.buildxy.outputs.buildxy }}
            %X.Y.Z%
          extra_tags: |
            latest::${{ steps.stable-octoprint.outputs.release == github.event.client_payload.tag_name }}
            edge::${{ github.event.action == 'prereleased' || github.event.client_payload.tag_name == steps.latest-octoprint.outputs.release }}
            canary::${{ github.event.action == 'canary' }}
            bleeding::${{ github.event.action == 'bleeding' }}

could be used like this with v2:

      -
        id: buildxy
        run: echo '::set-output name=buildxy::${{ (github.event.action == 'deploy'|| github.event.action == 'released') && github.event.client_payload.tag_name == steps.stable-octoprint.outputs.release }}'
      -
        name: Meta
        uses: crazy-max/ghaction-docker-meta@v2
        with:
          images: octoprint/octoprint
          tag_name: ${{ github.event.client_payload.tag_name }}
          tags: |
            ${{ github.event.client_payload.tag_name }}
            type=semver,pattern={{major}},enable=${{ steps.buildxy.outputs.buildxy }}
            type=semver,pattern={{major}}.{{minor}},enable=${{ steps.buildxy.outputs.buildxy }}
            type=semver,pattern={{version}}
            type=raw,value=edge,enable=${{ github.event.action == 'prereleased' || github.event.client_payload.tag_name == steps.latest-octoprint.outputs.release }}
            type=raw,value=canary,enable=${{ github.event.action == 'canary' }}
            type=raw,value=bleeding,enable=${{ github.event.action == 'bleeding' }}
          flavor: |
            latest=${{ steps.stable-octoprint.outputs.release == github.event.client_payload.tag_name }}

I think it's too over-engineered to be useful outside of a specific use-case, and that's what my proposal was about.

I rather think that your workflow in the above example is too complex and does not reflect a nominal case where a user simply wants to release his Docker image following a standard GitFlow (push, tag). But again you can do that with v2 if you want.

If you have a use case where v2 would not be effective I would be happy to review its structure. Thanks again for your feedback!

@LongLiveCHIEF
Copy link

Your example doesn't work. The first entry in tags is just a raw tag, and isn't parsed at all. The other entries presume there is a corresponding ref, which the workflow wouldn't have, and thus would fail.

let's say i get a github.event.client_payload.tag_name value of 2.0.0. I want to produce the tags image:2.0.0, image:2.0, image:2.

I can't do this with the v1 implementation of your action, or the v2 implementation as its currently written.

Very few users that I know of use events with custom payloads

I can tell you've been in this business as long as I have, and undoubtedly have learned by now that this is a trap. 😏

Use cases where you'd want to use external payloads:

  • creating an organization specific customization of a base image. say to do things like modify environment variables or trusted certificates for the base image so that it works inside your organizations networks as a drop-in replacement for the base image
  • you maintain a fork of a product with your own customizations to the software
  • The container deployment of a product has it's own development lifecycle and is thus maintained in a different repo than the software it packages.

I rather think that your workflow in the above example is too complex and does not reflect a nominal case where a user simply wants to release his Docker image following a standard GitFlow

I think it would be very simple to maintain this behavior as the default, and implement the ability to parse type=raw in a future feature. If this is the case, v2.0 is just a migration to the action usage, and future versions would build on that with new features.


I also still wonder if type= is actually needed? See my thoughts on that in my comment on the proposal thread

@crazy-max
Copy link
Member Author

@LongLiveCHIEF

The first entry in tags is just a raw tag, and isn't parsed at all.

No it will be parsed as a type=raw if no type is defined: https://github.com/crazy-max/ghaction-docker-meta/tree/v2#typeraw

tags: |
  type=raw,value=mytag1
  # or
  type=raw,mytag1
  # or
  mytag1

let's say i get a github.event.client_payload.tag_name value of 2.0.0. I want to produce the tags image:2.0.0, image:2.0, image:2.

Agree about this one, semver will not be processed because it's not a tag event. I will improve this behavior.

Use cases where you'd want to use external payloads...

I'm fine with that, I'm going to make some changes in that direction.

@LongLiveCHIEF
Copy link

Based on your latest comments, I believe my primary concerns are resolved at least for the moment, and I agree with this direction. At this point I'm going with 👍 based on the docs.

While not a full replacement yet, it does give me a path to deprecate my own action as those improvements you mentioned are rolled out, and I'd be happy to contribute those features and join you as a maintainer for this project if you want that.

@crazy-max
Copy link
Member Author

@LongLiveCHIEF

While not a full replacement yet, it does give me a path to deprecate my own action as those improvements you mentioned are rolled out

Sure we can bring enhancements in folllow up PRs. Just want with v2 a solid foundation to build on.

I'd be happy to contribute those features and join you as a maintainer for this project if you want that.

Ofc with pleasure!

@crazy-max
Copy link
Member Author

@LongLiveCHIEF Ok now you can use a custom value for semver and match type.

@crazy-max crazy-max marked this pull request as ready for review March 28, 2021 16:37
@crazy-max crazy-max merged commit 2f83320 into master Mar 29, 2021
@crazy-max crazy-max deleted the v2 branch March 29, 2021 11:05
@MarcelCoding
Copy link

MarcelCoding commented Mar 30, 2021

Maybe I am to stupid, but I am not able to use the branch name as tag and on the default branch edge as tag. In the default branch the tag <default_branch> will be also created.

PS: Am I able to tell the flavor to do latest only on a tag and use edge in the default branch?

      - name: Docker meta
        id: docker_meta
        uses: crazy-max/ghaction-docker-meta@v2
        with:
          images: |
            ${{ env.IMAGE_NAME }}
            ghcr.io/${{ env.IMAGE_NAME }}
          tags: |
            type=edge
            type=ref,event=pr
            type=ref,event=branch
            type=semver,pattern={{version}}
            type=semver,pattern={{major}}.{{minor}}
          flavor: |
            prefix=${{ matrix.jvm-impl }}
            latest=${{ matrix.jvm-impl == 'hotspot' }}

@crazy-max
Copy link
Member Author

@MarcelCoding Can you create a new issue please? I would need logs and link to your repo/pipeline. Thanks.

@MarcelCoding
Copy link

@crazy-max #55

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants