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

feat: allow use of consistent container tagging #271

Merged
merged 1 commit into from
May 24, 2024
Merged

Conversation

wilsonjord
Copy link
Contributor

@wilsonjord wilsonjord commented May 16, 2024

Motivating ticket: https://github.com/GeoNet/tickets/issues/14398

This PR adds a composite action providing a consistent tag, or at least, consistent outputs to construct such a tag.
This should be suitable for use in the reusable workflows, as well as explicitly within users own workflow jobs.

Currently, the proposed format is commit sha-timestamp-build id...for example, 1da9c71-20240515111413-9094587538

Points to consider are:

  • order of fields
  • need for timestamp (which was part of my original work to better ensure "uniqueness", but may not be required in presence of build id
  • any other missing fields

@wilsonjord wilsonjord self-assigned this May 16, 2024
@wilsonjord wilsonjord force-pushed the feature-tagging branch 2 times, most recently from aea31df to db10d2d Compare May 16, 2024 01:05
@wilsonjord wilsonjord requested a review from Mossman1215 May 16, 2024 01:11
@ardrigh
Copy link
Contributor

ardrigh commented May 16, 2024

Currently, the proposed format is commit sha-timestamp-build id...for example, 1da9c71-20240515111413-9094587538

Points to consider are:

* order of fields 
* need for timestamp (which was part of my original work to better ensure "uniqueness", but may not be required in presence of build id
* any other missing fields

I would recommend putting the date timestamp field first, adding git to the front of the SHA as is common practice. Then you shouldn't need the build ID. Tools that need to verify images should also check attestation files or additional metadata.

That would look like 20240515111413-git1da9c71

This aligns it with the packaging tags using 1.21-gitdfe7b0f
https://github.com/GeoNet/haz-sc3-producer/releases/tag/1.21-gitdfe7b0f

@wilsonjord
Copy link
Contributor Author

Then you shouldn't need the build ID. Tools that need to verify images should also check attestation files or additional metadata.

The build id was to allow, for a given image, what version of docker-gamit was used (via the commit sha), and what version of docker-gamit-base was used (via examination of the build associated with the build id). I'll need to understand how attestation / additional metadata would solve this particular issue.

Anyway, I'm happy to drop build id from the default tag, if it's not useful, as the end user can add build id themselves to their tagging scheme if they really needed it.

An alternative would be to have build id as an option behind some sort of include_build_id type flag.

That would look like 20240515111413-git1da9c71

That looks good, I'll make the change, thanks.

Adds ability to generate consistent container tag across workflows.
- name: Generate tags
id: tags
env:
RELEVANT_SHA: ${{ github.event.pull_request.head.sha || github.sha }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what scenarios does the || github.sha account for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most commonly, it accounts for push to main, which typically happens on PR merge; github.sha will be the last commit of main.

Copy link
Contributor

@CallumNZ CallumNZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wilsonjord wilsonjord merged commit 3985c68 into main May 24, 2024
16 checks passed
@wilsonjord wilsonjord deleted the feature-tagging branch May 24, 2024 01:27
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 this pull request may close these issues.

3 participants