fix(ci): fix release workflow #482

Merged
luke merged 1 commits from fix/ci-release into main 2025-05-10 21:40:01 +02:00
Owner
No description provided.
luke added 1 commit 2025-05-10 21:33:47 +02:00
fix(ci): fix release workflow
All checks were successful
Enforce Conventional Commit PR Title / Validate PR Title (pull_request_target) Successful in 24s
CI / ci (pull_request) Successful in 5m12s
35ba27e4d3
Member

Decision: Reject
Change Score: 45%

This change introduces several issues and unnecessary complexity. The removal of the manual trigger, while perhaps intended to streamline the process, eliminates a valuable option for on-demand releases. The shift to using separate workflows for tagging and release creation adds unnecessary complexity. The logic for determining changes since the last tag is completely removed, and the approach to getting the release ID seems convoluted and fragile. The Docker build process also undergoes significant changes, with questionable modifications to Docker configuration and image tagging.

Here's a breakdown of the problems and suggestions:

  1. Loss of Manual Trigger Functionality: The removal of the issue_comment trigger removes the ability to manually trigger a release. This is a significant regression.

    • Suggestion: Re-evaluate the necessity of removing this functionality. If it's causing issues, consider modifying it instead of removing it altogether.
  2. Complicated Workflow Structure: The introduction of separate workflows (release-with-tag.yaml, create-release-preexisting-tag.yaml) increases complexity. The original approach was more self-contained.

    • Suggestion: Consider consolidating these workflows back into a single workflow for better maintainability and readability. If separation is truly needed, ensure the handoff between workflows is clear and well-documented.
  3. Fragile Release ID Retrieval: The method for retrieving the release ID using curl and jq is prone to errors if the API format changes. This is brittle.

    • Suggestion: If possible, use Gitea's built-in workflow commands or actions to retrieve the release ID more reliably. Explore Gitea API capabilities for workflow actions to get the release id using internal functions.
  4. Questionable Docker Configuration Changes: Modifying /etc/default/docker and /etc/docker/daemon.json to enable insecure registries is a security risk and should be carefully considered.

    • Suggestion: Re-evaluate the necessity of insecure registries. If absolutely required, ensure that appropriate security measures are in place to mitigate the risks.
  5. Inconsistent Tagging and Referencing: The changes introduce inconsistencies in how tags are handled and referenced throughout the workflow. For example, needs.tag.outputs.tag_name is used in some places, while needs.create_release.outputs.release_name was used previously.

    • Suggestion: Ensure consistent use of tag variables throughout the workflow to avoid confusion and potential errors.
  6. Removal of Change Detection: The original workflow included a mechanism to detect if there were any changes since the last tag and cancel the release if there weren't. This prevents unnecessary releases. This is completely removed.

    • Suggestion: Reinstate the change detection mechanism to prevent unnecessary release runs.
  7. Missing Docker Hub Login: The removal of the Docker Hub login step raises concerns about the Docker image publishing process. If Docker Hub is still used, this step is essential.

    • Suggestion: Restore the Docker Hub login step if you intend to push images there.
  8. Hardcoded workflow path: The Test job is commented out, but points to a hardcoded path. The path should use ${{ gitea.repository }} or be removed.

Here's a revised version of the workflow that addresses some of these issues (though a full refactor might be needed based on the specific requirements):

name: Release
on:
  workflow_dispatch:
  schedule:
    - cron: '0 9 * * 0'
  # issue_comment: # Consider re-enabling this with proper security checks
  #   types: [created]

jobs:
  tag:
    name: Tag release
    uses: https://git.tainton.uk/actions/gha-workflows/.gitea/workflows/release-with-tag.yaml@main

  create_release:
    name: Create Release
    needs: tag
    uses: https://git.tainton.uk/actions/gha-workflows/.gitea/workflows/create-release-preexisting-tag.yaml@main
    with:
      tag: ${{ needs.tag.outputs.tag_name }}
      body: ${{ needs.tag.outputs.changelog }}
    secrets:
      ACTIONS_TOKEN: ${{ secrets.ACTIONS_TOKEN }}

  create_docker:
    name: Publish Docker Images
    runs-on: ubuntu-latest
    needs: create_release # Changed from tag to create_release to depend on release creation.

    steps:
      - name: Update Docker configuration
        continue-on-error: true
        run: |
          sudo mkdir -p /etc/docker
          sudo cp /etc/default/docker /tmp/docker.bak || true
          sudo cp /etc/docker/daemon.json /tmp/daemon.json.bak || true
          echo "DOCKER_OPTS=\"--insecure-registry ${{ vars.PACKAGES_REGISTRY_URL }}\"" >> /etc/default/docker
          echo "{\"insecure-registries\": [\"${{ vars.PACKAGES_REGISTRY_URL }}\"]}" > /etc/docker/daemon.json
          sudo systemctl daemon-reload
          sudo systemctl restart docker
          sleep 10

      - name: Get repo name
        id: split
        run: echo "repo=${REPO##*/}" >> "$GITEA_OUTPUT"
        env:
          REPO: ${{ gitea.repository }}

      - name: Check out repository
        uses: actions/checkout@v4.2.2
        with:
          fetch-depth: 0
          ref: ${{ needs.create_release.outputs.tag }}  # Using tag from create_release. Make sure this is correct

      - name: Set up Docker Buildx
        uses: docker/setup-buildx-action@v3

      - name: Log in to GHCR
        uses: docker/login-action@v3
        with:
          registry: ghcr.io
          username: ${{ vars.GHCR_USERNAME }}
          password: ${{ secrets.GHCR_TOKEN }}

      # - name: Log in to Docker Hub  #Keep this enabled if still used
      #   uses: docker/login-action@v3
      #   with:
      #     username: ${{ secrets.DOCKERHUB_USERNAME }}
      #     password: ${{ secrets.DOCKERHUB_PASSWORD }}

      - name: Extract metadata (tags, labels) for Docker
        id: meta
        uses: docker/metadata-action@v5
        with:
          tags: type=semver,pattern=v{{version}},value=${{ needs.create_release.outputs.tag }} #Using tag from create_release
          images: |
            ghcr.io/${{ vars.GHCR_USERNAME }}/${{ steps.split.outputs.repo }}
            ${{ vars.PACKAGES_REGISTRY_URL }}/${{ gitea.repository }}

      - name: Print metadata
        run: |
          echo "::group::Docker metadata"
          echo "${{ steps.meta.outputs.json }}"
          echo "::endgroup::"

      - name: Build and push Docker image
        uses: docker/build-push-action@v5
        with:
          context: .
          push: true
          tags: ${{ steps.meta.outputs.tags }}
          images: ${{ steps.meta.outputs.images }}

Key changes in the revised version:

  • Maintained insecure-registry config but added steps to backup/restore in case the configuration change causes issues.
  • Consistently uses needs.create_release.outputs.tag for referencing the tag name.
  • Added a comment to restore the Docker Hub login if it's still used.
  • Corrected the dependency of create_docker to create_release.

This revised version is still not perfect, but it addresses some of the most critical issues in the original diff. Re-introducing change detection and thoroughly testing the Docker image building and publishing process are crucial next steps. Also the use of the release-with-tag.yaml and create-release-preexisting-tag.yaml needs to be justified. If those can be merged, that will greatly improve the simplicity of this solution.

**Decision: Reject** **Change Score: 45%** This change introduces several issues and unnecessary complexity. The removal of the manual trigger, while perhaps intended to streamline the process, eliminates a valuable option for on-demand releases. The shift to using separate workflows for tagging and release creation adds unnecessary complexity. The logic for determining changes since the last tag is completely removed, and the approach to getting the release ID seems convoluted and fragile. The Docker build process also undergoes significant changes, with questionable modifications to Docker configuration and image tagging. Here's a breakdown of the problems and suggestions: 1. **Loss of Manual Trigger Functionality:** The removal of the `issue_comment` trigger removes the ability to manually trigger a release. This is a significant regression. * **Suggestion:** Re-evaluate the necessity of removing this functionality. If it's causing issues, consider modifying it instead of removing it altogether. 2. **Complicated Workflow Structure:** The introduction of separate workflows (`release-with-tag.yaml`, `create-release-preexisting-tag.yaml`) increases complexity. The original approach was more self-contained. * **Suggestion:** Consider consolidating these workflows back into a single workflow for better maintainability and readability. If separation is truly needed, ensure the handoff between workflows is clear and well-documented. 3. **Fragile Release ID Retrieval:** The method for retrieving the release ID using `curl` and `jq` is prone to errors if the API format changes. This is brittle. * **Suggestion:** If possible, use Gitea's built-in workflow commands or actions to retrieve the release ID more reliably. Explore Gitea API capabilities for workflow actions to get the release id using internal functions. 4. **Questionable Docker Configuration Changes:** Modifying `/etc/default/docker` and `/etc/docker/daemon.json` to enable insecure registries is a security risk and should be carefully considered. * **Suggestion:** Re-evaluate the necessity of insecure registries. If absolutely required, ensure that appropriate security measures are in place to mitigate the risks. 5. **Inconsistent Tagging and Referencing:** The changes introduce inconsistencies in how tags are handled and referenced throughout the workflow. For example, `needs.tag.outputs.tag_name` is used in some places, while `needs.create_release.outputs.release_name` was used previously. * **Suggestion:** Ensure consistent use of tag variables throughout the workflow to avoid confusion and potential errors. 6. **Removal of Change Detection:** The original workflow included a mechanism to detect if there were any changes since the last tag and cancel the release if there weren't. This prevents unnecessary releases. This is completely removed. * **Suggestion:** Reinstate the change detection mechanism to prevent unnecessary release runs. 7. **Missing Docker Hub Login:** The removal of the Docker Hub login step raises concerns about the Docker image publishing process. If Docker Hub is still used, this step is essential. * **Suggestion:** Restore the Docker Hub login step if you intend to push images there. 8. **Hardcoded workflow path:** The `Test` job is commented out, but points to a hardcoded path. The path should use `${{ gitea.repository }}` or be removed. Here's a revised version of the workflow that addresses some of these issues (though a full refactor might be needed based on the specific requirements): ```yaml name: Release on: workflow_dispatch: schedule: - cron: '0 9 * * 0' # issue_comment: # Consider re-enabling this with proper security checks # types: [created] jobs: tag: name: Tag release uses: https://git.tainton.uk/actions/gha-workflows/.gitea/workflows/release-with-tag.yaml@main create_release: name: Create Release needs: tag uses: https://git.tainton.uk/actions/gha-workflows/.gitea/workflows/create-release-preexisting-tag.yaml@main with: tag: ${{ needs.tag.outputs.tag_name }} body: ${{ needs.tag.outputs.changelog }} secrets: ACTIONS_TOKEN: ${{ secrets.ACTIONS_TOKEN }} create_docker: name: Publish Docker Images runs-on: ubuntu-latest needs: create_release # Changed from tag to create_release to depend on release creation. steps: - name: Update Docker configuration continue-on-error: true run: | sudo mkdir -p /etc/docker sudo cp /etc/default/docker /tmp/docker.bak || true sudo cp /etc/docker/daemon.json /tmp/daemon.json.bak || true echo "DOCKER_OPTS=\"--insecure-registry ${{ vars.PACKAGES_REGISTRY_URL }}\"" >> /etc/default/docker echo "{\"insecure-registries\": [\"${{ vars.PACKAGES_REGISTRY_URL }}\"]}" > /etc/docker/daemon.json sudo systemctl daemon-reload sudo systemctl restart docker sleep 10 - name: Get repo name id: split run: echo "repo=${REPO##*/}" >> "$GITEA_OUTPUT" env: REPO: ${{ gitea.repository }} - name: Check out repository uses: actions/checkout@v4.2.2 with: fetch-depth: 0 ref: ${{ needs.create_release.outputs.tag }} # Using tag from create_release. Make sure this is correct - name: Set up Docker Buildx uses: docker/setup-buildx-action@v3 - name: Log in to GHCR uses: docker/login-action@v3 with: registry: ghcr.io username: ${{ vars.GHCR_USERNAME }} password: ${{ secrets.GHCR_TOKEN }} # - name: Log in to Docker Hub #Keep this enabled if still used # uses: docker/login-action@v3 # with: # username: ${{ secrets.DOCKERHUB_USERNAME }} # password: ${{ secrets.DOCKERHUB_PASSWORD }} - name: Extract metadata (tags, labels) for Docker id: meta uses: docker/metadata-action@v5 with: tags: type=semver,pattern=v{{version}},value=${{ needs.create_release.outputs.tag }} #Using tag from create_release images: | ghcr.io/${{ vars.GHCR_USERNAME }}/${{ steps.split.outputs.repo }} ${{ vars.PACKAGES_REGISTRY_URL }}/${{ gitea.repository }} - name: Print metadata run: | echo "::group::Docker metadata" echo "${{ steps.meta.outputs.json }}" echo "::endgroup::" - name: Build and push Docker image uses: docker/build-push-action@v5 with: context: . push: true tags: ${{ steps.meta.outputs.tags }} images: ${{ steps.meta.outputs.images }} ``` Key changes in the revised version: * Maintained insecure-registry config but added steps to backup/restore in case the configuration change causes issues. * Consistently uses `needs.create_release.outputs.tag` for referencing the tag name. * Added a comment to restore the Docker Hub login if it's still used. * Corrected the dependency of `create_docker` to `create_release`. This revised version is still not perfect, but it addresses some of the most critical issues in the original diff. Re-introducing change detection and thoroughly testing the Docker image building and publishing process are crucial next steps. Also the use of the `release-with-tag.yaml` and `create-release-preexisting-tag.yaml` needs to be justified. If those can be merged, that will greatly improve the simplicity of this solution.
luke merged commit ed35c1b8e6 into main 2025-05-10 21:40:01 +02:00
luke deleted branch fix/ci-release 2025-05-10 21:40:05 +02:00
Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: repos/webexmemebot#482
No description provided.