Skip to content

ci: Docker - Prefer dist-local stage #2771

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

Merged
merged 2 commits into from
May 18, 2025

Conversation

polarathene
Copy link
Contributor

@polarathene polarathene commented May 12, 2025

Describe your changes

Changes the Alpine Dockerfile to also default to dist-local stage as the final image output, like the openSUSE Leap Dockerfile already does. The dist-git stage remains for now as opt-in, but I'm not sure if it has any notable advantage to any potential users, perhaps drop it in the 3.3 release series?

This will affect anyone relying on the dist-git default target (which recently had a breaking change of BUILD_VERSION => GIT_BRANCH build arg), they would now need to also be explicit about the --target stage when building. However just like mentioned in #2769 there is an alternative without --target that works effectively the same with dist-local:

docker build --tag localhost/testssl.sh:3.2 https://github.com/testssl/testssl.sh.git#3.2

# Alpine variant requires `--file`:
docker build \
  --tag localhost/testssl.sh:3.2-alpine \
  --file https://raw.githubusercontent.com/testssl/testssl.sh/3.2/Dockerfile-alpine \
  https://github.com/testssl/testssl.sh.git#3.2

Resolves: #2769
Closes: #2770

What is your pull request about?

  • Improvement
  • Update of other files

@polarathene
Copy link
Contributor Author

@drwetter you may prefer this PR :)

@drwetter
Copy link
Collaborator

drwetter commented May 16, 2025

Hi @polarathene :

I lost track a bit whether it was me editing the file or the PR... long story short: Is change I needed to do fine?

@polarathene
Copy link
Contributor Author

whether it was me editing the file or the PR...

I'm not sure what you are referring to. This PR removes the need for the CI build arg and makes the Alpine Dockerfile more consistent with the openSUSE Leap Dockerfile, they would both be dist-local stage by default which is what you'd typically expect.

In your 3.3 series you might want to remove dist-git as it's unlikely to be of value to anyone, your Dockerfile docs show how to do the equivalent without dist-git.

@drwetter
Copy link
Collaborator

@polarathene: I had to edit the PR as there was a merge conflict. Could you just have a look that it's correct?

@polarathene
Copy link
Contributor Author

polarathene commented May 17, 2025

TL;DR: LGTM, feel free to merge.

Could you just have a look that it's correct?

This diff? It looks fine to me.

The only change to the docker-3.2.yaml workflow is removal of:

build-args: GIT_BRANCH

Which is presumably what the merge conflict was regarding.


This PR is simple, it just swaps the position of dist-git and dist-local stages in the Alpine Dockerfile, that makes the build-args: GIT_BRANCH workflow setting redundant as dist-local stage is built instead of dist-git 👍

@drwetter drwetter merged commit 0d0c5d0 into testssl:3.2 May 18, 2025
2 checks passed
@drwetter
Copy link
Collaborator

Which is presumably what the merge conflict was regarding.

yes, I believe so. Was just confusing to me that this popped up. Nevermind. Thx!

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.

Preferring the dist-local stage ?
2 participants