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

Build Docker image and push to GHCR #230

Open
wants to merge 10 commits into
base: unstable/v1
Choose a base branch
from
39 changes: 39 additions & 0 deletions .github/workflows/build-and-push-docker-image.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
---

name: 🏗️

on: # yamllint disable-line rule:truthy
pull_request:
push:
branches: ["release/*", "unstable/*"]
workflow_dispatch:
inputs:
tag:
description: Docker image tag
required: true
type: string

jobs:
build-and-push:
runs-on: ubuntu-latest
timeout-minutes: 10
steps:
- uses: actions/checkout@v4
- name: Build Docker image
run: |
IMAGE="ghcr.io/$GITHUB_REPOSITORY:${DOCKER_TAG/'/'/'-'}"
echo "IMAGE=$IMAGE" >>"$GITHUB_ENV"
docker build . \
--build-arg BUILDKIT_INLINE_CACHE=1 \
--cache-from $IMAGE \
--tag $IMAGE
env:
DOCKER_TAG: ${{ inputs.tag || github.ref_name }}
- name: Log in to GHCR
if: github.event_name != 'pull_request'
run: >-
echo ${{ secrets.GITHUB_TOKEN }} |
webknjaz marked this conversation as resolved.
Show resolved Hide resolved
docker login ghcr.io -u $GITHUB_ACTOR --password-stdin
- name: Push Docker image to GHCR
if: github.event_name != 'pull_request'
run: docker push $IMAGE
7 changes: 6 additions & 1 deletion .github/workflows/self-smoke-test-action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
name: 🧪

on: # yamllint disable-line rule:truthy
push:
pull_request:
workflow_run:
Copy link
Member

Choose a reason for hiding this comment

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

Two separate workflow runs is often hard to track. Instead, I adopted a practice of modularizing the workflow pieces as reusable workflows having the reusable- prefix in their names. This allows embedding everything in all the right places. Let's try this, WDYT?

Copy link
Author

Choose a reason for hiding this comment

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

I will return to this suggestion at a later time.

workflows: [🏗️]
types: [completed]

env:
devpi-password: abcd1234
Expand All @@ -28,6 +30,9 @@ env:
jobs:
smoke-test:
if: >-
github.event_name == 'pull_request' ||
github.event.workflow_run.conclusion == 'success'
runs-on: ubuntu-latest

services:
Expand Down
67 changes: 56 additions & 11 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,59 @@ branding:
color: yellow
icon: upload-cloud
runs:
using: docker
image: Dockerfile
args:
- ${{ inputs.user }}
- ${{ inputs.password }}
- ${{ inputs.repository-url }}
- ${{ inputs.packages-dir }}
- ${{ inputs.verify-metadata }}
- ${{ inputs.skip-existing }}
- ${{ inputs.verbose }}
- ${{ inputs.print-hash }}
using: composite
steps:
- name: Reset path if needed
run: |
# Reset path if needed
Copy link
Member

Choose a reason for hiding this comment

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

Why would this be needed outside the container?

Copy link
Author

Choose a reason for hiding this comment

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

Because you've set up a test that modifies the $PATH (#112, 1350b8b).

- name: ✅ Smoke-test the locally checked out action
uses: ./test
env:
DEBUG: >-
true
PATH: utter-nonsense

# https://github.com/pypa/gh-action-pypi-publish/issues/112
if [[ $PATH != *"/usr/bin"* ]]; then
echo "\$PATH=$PATH. Resetting \$PATH for GitHub Actions."
PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
echo "PATH=$PATH" >>"$GITHUB_ENV"
echo "$PATH" >>"$GITHUB_PATH"
echo "\$PATH reset. \$PATH=$PATH"
fi
shell: bash
- name: Set repo and ref from which to run Docker container action
id: set-repo-and-ref
run: |
# Set repo and ref from which to run Docker container action
# to handle cases in which `github.action_` context is not set
# https://github.com/actions/runner/issues/2473
REF=${{ env.ACTION_REF || github.ref_name }}
REPO=${{ env.ACTION_REPO || github.repository }}
echo "ref=$REF" >>"$GITHUB_OUTPUT"
echo "repo=$REPO" >>"$GITHUB_OUTPUT"
shell: bash
env:
ACTION_REF: ${{ github.action_ref }}
ACTION_REPO: ${{ github.action_repository }}
Comment on lines +107 to +114
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it easier to do it like this?

Suggested change
REF=${{ env.ACTION_REF || github.ref_name }}
REPO=${{ env.ACTION_REPO || github.repository }}
echo "ref=$REF" >>"$GITHUB_OUTPUT"
echo "repo=$REPO" >>"$GITHUB_OUTPUT"
shell: bash
env:
ACTION_REF: ${{ github.action_ref }}
ACTION_REPO: ${{ github.action_repository }}
REF=${{ github.action_ref || github.ref_name }}
REPO=${{ github.action_repository || github.repository }}
echo "ref=${REF}" >>"${GITHUB_OUTPUT}"
echo "repo=${REPO}" >>"${GITHUB_OUTPUT}"
shell: bash

Copy link
Author

Choose a reason for hiding this comment

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

You would think so, but the syntax is needed to work around a limitation of composite actions. See:

actions/runner#2473
github/docs#25336 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Oh, good to know! It's probably safer too.

- name: Check out action repo
uses: actions/checkout@v4
with:
path: action-repo
ref: ${{ steps.set-repo-and-ref.outputs.ref }}
repository: ${{ steps.set-repo-and-ref.outputs.repo }}
- name: Create Docker container action
run: |
# Create Docker container action
python -m pip install -r requirements/github-actions.txt
Copy link
Member

Choose a reason for hiding this comment

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

This is not the right way of using constraint files. Here's what should be done instead:

Suggested change
python -m pip install -r requirements/github-actions.txt
PIP_CONSTRAINT=requirements/github-actions.txt python -Im pip install --only-binary=:all: -r requirements/github-actions.in

Copy link
Member

Choose a reason for hiding this comment

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

Another thing to consider, provided that it's actually needed (see https://github.com/pypa/gh-action-pypi-publish/pull/230/files#r1619055808), is making a virtualenv managed by this action (in its checkout/tmp dir so that the end-user's projects aren't polluted).

python create-docker-action.py ${{ steps.set-image.outputs.image }}
env:
EVENT: ${{ github.event_name }}
REF: ${{ steps.set-repo-and-ref.outputs.ref }}
REPO: ${{ steps.set-repo-and-ref.outputs.repo }}
shell: bash
working-directory: action-repo
- name: Run Docker container
uses: ./action-repo/.github/actions/run-docker-container
with:
user: ${{ inputs.user }}
password: ${{ inputs.password }}
repository-url: ${{ inputs.repository-url || inputs.repository_url }}
packages-dir: ${{ inputs.packages-dir || inputs.packages_dir }}
Copy link
Member

Choose a reason for hiding this comment

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

This would break the deprecation messages, it seems. Have you checked?

Copy link
Author

Choose a reason for hiding this comment

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

I don't think so. The new inputs don't have defaults, so for example, ${{ inputs.repository-url || inputs.repository_url }} will default to inputs.repository_url and deprecationMessage will be logged.

verify-metadata: ${{ inputs.verify-metadata || inputs.verify_metadata }}
skip-existing: ${{ inputs.skip-existing || inputs.skip_existing }}
verbose: ${{ inputs.verbose }}
print-hash: ${{ inputs.print-hash || inputs.print_hash }}
79 changes: 79 additions & 0 deletions create-docker-action.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import os
import pathlib

import yaml

DESCRIPTION = 'description'
REQUIRED = 'required'

EVENT = os.environ['EVENT']
REF = os.environ['REF']
REPO = os.environ['REPO']


def set_image(event: str, ref: str, repo: str) -> str:
if event == 'pull_request' and 'gh-action-pypi-publish' in repo:
Copy link
Member

Choose a reason for hiding this comment

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

Why do you check for 'gh-action-pypi-publish' in repo? How about forks that renamed it?

return '../../../Dockerfile'
docker_ref = ref.replace('/', '-')
return f'docker://ghcr.io/{repo}:{docker_ref}'


image = set_image(EVENT, REF, REPO)

action = {
'name': '🏃',
DESCRIPTION: (
'Run Docker container to upload Python distribution packages to PyPI'
),
'inputs': {
'user': {DESCRIPTION: 'PyPI user', REQUIRED: False},
'password': {
DESCRIPTION: 'Password for your PyPI user or an access token',
REQUIRED: False,
},
'repository-url': {
DESCRIPTION: 'The repository URL to use',
REQUIRED: False,
},
'packages-dir': {
DESCRIPTION: 'The target directory for distribution',
REQUIRED: False,
},
'verify-metadata': {
DESCRIPTION: 'Check metadata before uploading',
REQUIRED: False,
},
'skip-existing': {
DESCRIPTION: (
'Do not fail if a Python package distribution'
' exists in the target package index'
),
REQUIRED: False,
},
'verbose': {DESCRIPTION: 'Show verbose output.', REQUIRED: False},
'print-hash': {
DESCRIPTION: 'Show hash values of files to be uploaded',
REQUIRED: False,
},
},
'runs': {
'using': 'docker',
'image': image,
'args': [
'${{ inputs.user }}',
'${{ inputs.password }}',
'${{ inputs.repository-url }}',
'${{ inputs.packages-dir }}',
'${{ inputs.verify-metadata }}',
'${{ inputs.skip-existing }}',
'${{ inputs.verbose }}',
'${{ inputs.print-hash }}',
],
},
}

action_path = pathlib.Path('.github/actions/run-docker-container/action.yml')
action_path.parent.mkdir(parents=True, exist_ok=True)

with action_path.open(mode='w', encoding='utf-8') as file:
yaml.dump(action, file, allow_unicode=True, sort_keys=False)
Comment on lines +78 to +79
Copy link
Member

Choose a reason for hiding this comment

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

Could you check if using just

Suggested change
with action_path.open(mode='w', encoding='utf-8') as file:
yaml.dump(action, file, allow_unicode=True, sort_keys=False)
action_path.write_text(json.dumps(action), encoding='utf-8')

would work?

Most of the time, any valid JSON is also valid YAML so GHA should be able to read it...

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, we could have a template for the YAML file that would be post-processed by Template strings, str.format_map() or even just f-strings.

Any of these would let us avoid having an external dependency.

2 changes: 2 additions & 0 deletions requirements/github-actions.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# NOTE: used by create-docker-action.py
pyyaml
8 changes: 8 additions & 0 deletions requirements/github-actions.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#
# This file is autogenerated by pip-compile with Python 3.12
# by the following command:
#
# pip-compile --allow-unsafe --config=../.pip-tools.toml --output-file=github-actions.txt --strip-extras github-actions.in
Copy link
Member

Choose a reason for hiding this comment

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

Compiling PyYAML on various VMs might be problematic, so I'd pass --only-binary=:all: via pip-opts or even put it into the config. This way, installing would be more predictable on different versions of Ubuntu VMs that GitHub has.

Another concern is that originally, I opted for a docker-based action so that the external runtime is not mutated (that's one of a few reasons). By installing things with pip we violate that promise. I was kinda hoping that it'd be possible to avoid using a third-party dependency at all.

#
pyyaml==6.0.1
# via -r github-actions.in