[PLA-1994] Fixes upload to Dockerhub

leonardocustodio commented 3 weeks ago

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

github-actions[bot] commented 3 weeks ago

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.

github-actions[bot] commented 3 weeks ago

PR Code Suggestions ✨

Category	Suggestion	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]](https://github.com/enjin/platform/pull/52/files#diff-375807204432883d2fe0f02e08e633760f5802823833f872fd5e68d8b0939c6fR26-R26) ```diff -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]](https://github.com/enjin/platform/pull/52/files#diff-375807204432883d2fe0f02e08e633760f5802823833f872fd5e68d8b0939c6fR26-R30) ```diff 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
Best practice	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]](https://github.com/enjin/platform/pull/52/files#diff-375807204432883d2fe0f02e08e633760f5802823833f872fd5e68d8b0939c6fR26-R30) ```diff 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]](https://github.com/enjin/platform/pull/52/files#diff-375807204432883d2fe0f02e08e633760f5802823833f872fd5e68d8b0939c6fR26-R30) ```diff 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

enjin / platform