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

Flavor is ignored in output, despite being parsed properly #270

Open
fardog opened this issue Mar 4, 2023 · 4 comments
Open

Flavor is ignored in output, despite being parsed properly #270

fardog opened this issue Mar 4, 2023 · 4 comments

Comments

@fardog
Copy link

fardog commented Mar 4, 2023

Behaviour

Even though I've specified a flavor which contains a prefix, and it is parsed properly, it doesn't result in a prefix on the docker tags which are produced. This prefix is being used in a matrix to allow two different packages to be built into images. Apologies if I'm missing something obvious (it feels like I must be?).

I'm using the following flavor:

          flavor: |
            latest=auto
            prefix=${{ matrix.package }}-,onlatest=true
            suffix=

I've tried this with a few different combinations, including both with tags specified and without, as well as messing with the onlatest, but none of this appears to change the behavior.

Thanks tons!

Steps to reproduce this issue

  1. Configure the workflow with a flavors defined, which specifies a prefix
  2. Run the action
  3. Observe incorrect output

Expected behaviour

Docker tags should be generated with a prefix; in the case of the above, it should be ghcr.io/fardog/primebot:twitter-pr-12.

Actual behaviour

Docker tags ignore the prefix; e.g. ghcr.io/fardog/primebot:pr-12

Configuration

The repository URL contains a few different attempts within that PR, none of the configs I attempted were successful however.

name: Docker Image

on:
  push:
    branches: 
      - master
    tags:
  pull_request:

env:
  REGISTRY: ghcr.io
  IMAGE_NAME: ${{ github.repository }}

jobs:
  build-and-push-image:
    strategy:
      matrix:
        package: ["twitter", "mastodon"]
    runs-on: ubuntu-latest
    permissions:
      contents: read
      packages: write

    steps:
      - uses: actions/checkout@v3

      - uses: docker/login-action@v2
        with:
          registry: ${{ env.REGISTRY }}
          username: ${{ github.actor }}
          password: ${{ secrets.GITHUB_TOKEN }}

      - id: meta
        uses: docker/metadata-action@v4
        with:
          images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}
          flavor: |
            latest=auto
            prefix=${{ matrix.package }}-,onlatest=true
            suffix=
          tags: |
            type=schedule
            type=semver,pattern={{version}}
            type=semver,pattern={{major}}.{{minor}}
            type=semver,pattern={{major}}
            type=ref,event=branch
            type=ref,event=pr
            type=sha

      - name: Build and push Docker image
        uses: docker/build-push-action@v4
        with:
          context: .
          file: Dockerfile-${{ matrix.package }}
          push: ${{ github.event_name != 'pull_request' }}
          tags: ${{ steps.meta.outputs.tags }}
          labels: ${{ steps.meta.outputs.labels }}

Logs

logs_32.zip

Attached; also here is a screenshot of relevant output.

image

@crazy-max
Copy link
Member

crazy-max commented Mar 20, 2023

Yes indeed. type=sha already has a default prefix sha- and type=ref,event=pr too with pr- prefix. Therefore a global prefix can't be applied atm.

I think we could allow a global prefix being set if value is empty, like:

      - id: meta
        uses: docker/metadata-action@v4
        with:
          images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}
          flavor: |
            latest=auto
            prefix=${{ matrix.package }}-,onlatest=true
            suffix=
          tags: |
            type=ref,event=pr,prefix=
            type=sha,prefix=

WDYT?

As a workaround you can set the same prefix for type=sha and type=ref,event=pr:

      - id: meta
        uses: docker/metadata-action@v4
        with:
          images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}
          flavor: |
            latest=auto
            prefix=${{ matrix.package }}-,onlatest=true
            suffix=
          tags: |
            type=ref,event=pr,prefix=${{ matrix.package }}-
            type=sha,prefix=${{ matrix.package }}-

@fardog
Copy link
Author

fardog commented Mar 26, 2023

oh i see; yes, it was unexpected that sha- and pr- were already prefixes, so while i was expecting my prefix value to be written to all output, it was getting overwritten by this implicit default.

i think your workaround is probably a better solution than requiring the empty prefix, at least as it stands today. i think what i'd prefer is for the prefix to only be user-provided, and ref and sha to be instead part of the tag, so the prefix works as expected across all tags; i think the implicit prefix of the ref and sha creates unexpected behavior.

as this is my first use of this action though, i'm probably not best qualified to decide what's expected or unexpected, just my opinion. your workaround has helped me understand the problem and solved my issue; I super appreciate it. i'll leave this ticket open in case it's something you want to address, or feel free to close if not.

@crazy-max
Copy link
Member

crazy-max commented Oct 25, 2023

@fardog I think I will do another implementation using a mode attr to prefix/suffix flavors like:

      - id: meta
        uses: docker/metadata-action@v4
        with:
          images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}
          flavor: |
            latest=auto
            prefix=${{ matrix.package }}-,onlatest=true,mode=replace
            suffix=
          tags: |
            type=ref,event=pr
            type=sha

mode could be auto, merge, replace (defaults to auto):

  • auto: current behavior, set prefix/suffix if not already set in a tags entry
  • merge: if prefix/suffix is already set, then append (suffix) / prepend (prefix)
  • replace: if prefix/suffix is already set, then replace it with global one

WDYT?

@fardog
Copy link
Author

fardog commented Oct 27, 2023

@crazy-max this seems like a very reasonable way to let someone pick the behavior that they want; looks good to me!

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

No branches or pull requests

2 participants