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

[PLA-1994] Fixes upload to Dockerhub #52

Merged
merged 1 commit into from
Sep 16, 2024
Merged

Conversation

leonardocustodio
Copy link
Contributor

@leonardocustodio leonardocustodio commented Sep 12, 2024

PR Type

enhancement, configuration changes


Description

  • The GitHub Actions workflow for pushing Docker images to DockerHub has been simplified by consolidating multiple steps into a single script.
  • The job name has been changed from docker to push, and permissions have been updated to include id-token: write and contents: read.
  • The workflow now directly logs into DockerHub, builds, tags, and pushes the Docker image in one step, removing the need for separate setup actions for QEMU and Buildx.
  • The image is tagged with both the branch name and latest before being pushed to DockerHub.

Changes walkthrough 📝

Relevant files
Enhancement
push-image-to-dockerhub.yml
Simplify and streamline Docker image push workflow             

.github/workflows/push-image-to-dockerhub.yml

  • Renamed job from docker to push.
  • Updated permissions for the job.
  • Consolidated steps for logging in, building, tagging, and pushing
    Docker images.
  • Removed individual setup steps for QEMU and Buildx.
  • +16/-20 

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Signed-off-by: Leonardo Custodio <[email protected]>
    @leonardocustodio leonardocustodio requested review from DamianFigiel and Bradez and removed request for v16Studios and enjinabner September 12, 2024 21:51
    @leonardocustodio leonardocustodio self-assigned this Sep 12, 2024
    @leonardocustodio leonardocustodio changed the title Update push-image-to-dockerhub.yml [PLA-1994] Update push-image-to-dockerhub.yml Sep 12, 2024
    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Hardcoded Repository
    The Docker repository name 'platform' is hardcoded in the environment variables. Consider using a variable or secret for flexibility and maintainability.

    Missing Error Handling
    The script does not include error handling for Docker commands. It's recommended to add error checks after commands like docker login, docker build, and docker push to ensure the workflow fails gracefully if an error occurs.

    Copy link

    github-actions bot commented Sep 12, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Security
    Enhance security by using the docker/login-action for Docker login

    Use the docker/login-action for Docker login to enhance security by avoiding the
    need to use the --password CLI option, which might expose sensitive information in
    logs.

    .github/workflows/push-image-to-dockerhub.yml [26]

    -run: |
    -  docker login --username $DOCKERHUB_API_USERNAME --password $DOCKERHUB_API_TOKEN
    +- name: Login to Docker Hub
    +  uses: docker/login-action@v3
    +  with:
    +    username: ${{ secrets.DOCKERHUB_API_USERNAME }}
    +    password: ${{ secrets.DOCKERHUB_API_TOKEN }}
     
    Suggestion importance[1-10]: 9

    Why: This suggestion significantly enhances security by avoiding the use of the --password CLI option, which can expose sensitive information in logs. It addresses a potential security concern effectively.

    9
    Best practice
    Add error handling to Docker commands to ensure the workflow fails gracefully

    Add error handling for Docker commands to ensure the workflow fails gracefully and
    provides useful error messages if any command fails.

    .github/workflows/push-image-to-dockerhub.yml [26-30]

     run: |
    +  set -e
       docker login --username $DOCKERHUB_API_USERNAME --password $DOCKERHUB_API_TOKEN
       docker build -t enjin/$DOCKER_REPOSITORY:$IMAGE_TAG .
       docker push enjin/$DOCKER_REPOSITORY:$IMAGE_TAG
       docker tag enjin/$DOCKER_REPOSITORY:$IMAGE_TAG enjin/$DOCKER_REPOSITORY:latest
       docker push enjin/$DOCKER_REPOSITORY:latest
     
    Suggestion importance[1-10]: 8

    Why: Adding error handling is a best practice that ensures the workflow fails gracefully, providing useful error messages. This improves the robustness and reliability of the workflow.

    8
    Add a cleanup step to remove local Docker images after pushing to conserve space

    Consider adding a cleanup step to remove local Docker images after pushing to
    DockerHub to conserve space on the runner.

    .github/workflows/push-image-to-dockerhub.yml [26-30]

     run: |
       docker login --username $DOCKERHUB_API_USERNAME --password $DOCKERHUB_API_TOKEN
       docker build -t enjin/$DOCKER_REPOSITORY:$IMAGE_TAG .
       docker push enjin/$DOCKER_REPOSITORY:$IMAGE_TAG
       docker tag enjin/$DOCKER_REPOSITORY:$IMAGE_TAG enjin/$DOCKER_REPOSITORY:latest
       docker push enjin/$DOCKER_REPOSITORY:latest
    +  docker rmi enjin/$DOCKER_REPOSITORY:$IMAGE_TAG enjin/$DOCKER_REPOSITORY:latest
     
    Suggestion importance[1-10]: 6

    Why: The cleanup step is a good practice to conserve space on the runner, but it is not critical for the workflow's functionality. It is a minor improvement in terms of resource management.

    6
    Maintainability
    Improve flexibility and maintainability by using environment variables for repository and tag names in Docker commands

    Replace the hardcoded repository and tag names in the Docker commands with
    environment variables to make the workflow more flexible and maintainable.

    .github/workflows/push-image-to-dockerhub.yml [26-30]

     run: |
       docker login --username $DOCKERHUB_API_USERNAME --password $DOCKERHUB_API_TOKEN
    -  docker build -t enjin/$DOCKER_REPOSITORY:$IMAGE_TAG .
    -  docker push enjin/$DOCKER_REPOSITORY:$IMAGE_TAG
    -  docker tag enjin/$DOCKER_REPOSITORY:$IMAGE_TAG enjin/$DOCKER_REPOSITORY:latest
    -  docker push enjin/$DOCKER_REPOSITORY:latest
    +  docker build -t $DOCKER_REPOSITORY:$IMAGE_TAG .
    +  docker push $DOCKER_REPOSITORY:$IMAGE_TAG
    +  docker tag $DOCKER_REPOSITORY:$IMAGE_TAG $DOCKER_REPOSITORY:latest
    +  docker push $DOCKER_REPOSITORY:latest
     
    Suggestion importance[1-10]: 7

    Why: The suggestion improves maintainability by using environment variables, which makes the workflow more flexible and easier to update. However, the improvement is not critical as the current setup is functional.

    7

    @leonardocustodio leonardocustodio changed the title [PLA-1994] Update push-image-to-dockerhub.yml [PLA-1994] Fixes upload to Dockerhub Sep 13, 2024
    @leonardocustodio leonardocustodio merged commit 78e2693 into master Sep 16, 2024
    1 check passed
    @leonardocustodio leonardocustodio deleted the update-actions branch September 16, 2024 14:25
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Development

    Successfully merging this pull request may close these issues.

    2 participants