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

Increasing short commit hash length is a breaking change #479

Closed
3 tasks done
RobinDaugherty opened this issue Nov 19, 2024 · 8 comments · Fixed by #480
Closed
3 tasks done

Increasing short commit hash length is a breaking change #479

RobinDaugherty opened this issue Nov 19, 2024 · 8 comments · Fixed by #480

Comments

@RobinDaugherty
Copy link

RobinDaugherty commented Nov 19, 2024

Contributing guidelines

I've found a bug, and:

  • The documentation does not mention anything about my problem
  • There are no open or closed issues that are related to my problem

Description

#467 increased the default length of the commit hash created by the sha directive

This was released in v5.6.0 which made it immediately apply to everyone whose workflow specified release v5 in their actions.

This change unfortunately broke my workflows, because other parts of our workflows were hard-coded to match the size of the tags created by this action.

Please consider pulling this release and re-releasing it as v6.

Expected behaviour

v5 creates short "sha" tags with 7-character commit hash

Actual behaviour

v5 suddenly creates short "sha" tags with 12-character commit hash

Repository URL

No response

Workflow run URL

No response

YAML workflow

- name: Docker image metadata
  id: metadata
  uses: docker/metadata-action@v5
  with:
    images: |
      ${{ steps.ghcr_image_name.outputs.name }}
    tags: |
      type=ref,event=branch
      type=ref,event=pr
      type=semver,pattern={{major}}.{{minor}}.{{patch}}
      type=sha

- name: Get deployment tag
  id: get_deployment_tag
  # The following variables are made available to subsequent jobs to be used to find the specific Docker image that was built and pushed by this job.
  run: |
    echo "deployment_tag=sha-$(echo ${{ github.sha }} | cut -c1-7)" >> $GITHUB_OUTPUT
    echo "build_image=${{ steps.ghcr_image_name.outputs.name }}:sha-$(echo ${{ github.sha }} | cut -c1-7)" >> $GITHUB_OUTPUT

Workflow logs

No response

BuildKit logs


Additional info

No response

@housejester
Copy link

housejester commented Nov 19, 2024

Came here to post the same. Good news is that there is the ENV you can set to get it working again, but feels like it could have kept the default until a more major version bump.

Workaround is to add the env to the action:

        env:
          DOCKER_METADATA_SHORT_SHA_LENGTH: 7

@crazy-max
Copy link
Member

This change unfortunately broke my workflows,

Please consider pulling this release and re-releasing it as v6.

I guess in your case it breaks for deployment_tag and build_image because you rely on github.sha instead of metadata outputs. I don't think this is a breaking change as you don't rely on metadata outputs imo.

You can either enforce commit sha length with DOCKER_METADATA_SHORT_SHA_LENGTH env var:

- name: Docker image metadata
  id: metadata
  uses: docker/metadata-action@v5
  with:
    images: |
      ${{ steps.ghcr_image_name.outputs.name }}
    tags: |
      type=ref,event=branch
      type=ref,event=pr
      type=semver,pattern={{major}}.{{minor}}.{{patch}}
      type=sha
  env:
    DOCKER_METADATA_SHORT_SHA_LENGTH: 7

Or rely on the actual metadata outputs https://github.com/docker/metadata-action?tab=readme-ov-file#outputs to get a usable tag like version with ${{ steps.metadata.outputs.version }}

@housejester
Copy link

This change unfortunately broke my workflows,

Please consider pulling this release and re-releasing it as v6.

I guess in your case it breaks for deployment_tag and build_image because you rely on github.sha instead of metadata outputs. I don't think this is a breaking change as you don't rely on metadata outputs imo.

You can either enforce commit sha length with DOCKER_METADATA_SHORT_SHA_LENGTH env var:

  • name: Docker image metadata
    id: metadata
    uses: docker/metadata-action@v5
    with:
    images: |
    ${{ steps.ghcr_image_name.outputs.name }}
    tags: |
    type=ref,event=branch
    type=ref,event=pr
    type=semver,pattern={{major}}.{{minor}}.{{patch}}
    type=sha
    env:
    DOCKER_METADATA_SHORT_SHA_LENGTH: 7

Or rely on the actual metadata outputs https://github.com/docker/metadata-action?tab=readme-ov-file#outputs to get a usable tag like version with ${{ steps.metadata.outputs.version }}

In my case, i use a different action for rendering k8s yamls (skaffold...and yeah, i don't like skaffold), and for that tool, I cannot (easily) configure/pass in the sha...it has its own internal opinion about what a short sha is. I get that probably isn't common, but wanted the team to be aware it /is/ breaking peoples flows, and a safer approach would have been to not change a default in a non-major version bump.

@crazy-max
Copy link
Member

crazy-max commented Nov 19, 2024

The previous version relying on hardcoded short commit sha to length 7 is a bug on its own as it would be unreliable for huge repositories.

As stated in git documentation: https://git-scm.com/docs/git-rev-parse#Documentation/git-rev-parse.txt---shortltlengthgt

The minimum length is 4, the default is the effective value of the core.abbrev configuration variable

core.abbrev: If unspecified or set to "auto", an appropriate value is computed based on the approximate number of packed objects in your repository, which hopefully is enough for abbreviated object names to stay unique for some time.

So it defaults to 4 but based on repo size can be higher.

We choose 12 as it seems a good defaults which is what Go modules use when computing untagged dependencies.

@crazy-max
Copy link
Member

crazy-max commented Nov 19, 2024

I'm going to set the defaults back to 7 but keep the environment variable for people who want to rely on a higher value.

@crazy-max
Copy link
Member

Should be good with latest patch release. Let us know if that fixes the issue on your side 🙏

@housejester
Copy link

Thanks @crazy-max . I've tested and can confirm the latest works w/o the ENV. (i'm planning to keep the ENV specified moving forward, fwiw).

@RobinDaugherty
Copy link
Author

Thank you @crazy-max!

I agree that increasing the default from 7-characters is desirable. In my case we don't really need the shortened hash, so I took the opportunity to switch to the full length, using:

type=sha,format=long

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

Successfully merging a pull request may close this issue.

3 participants