Merging the fork [docker-seleniarm] into docker-selenium

[VietND96]

User description

Thanks for contributing to the Docker-Selenium project!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines, applied for this repository.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Motivation and Context

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

enhancement, tests, documentation

---

Description

• Added scripts and configurations for building, tagging, and pushing multi-architecture Docker images.
• Updated Dockerfiles to support multiple architectures and simplified architecture-specific conditions.
• Enhanced documentation and README to include details on building and running multi-architecture Docker images.
• Introduced new Go programs to assist in fetching related tags and SHA256 digests for Docker images.

---

Changes walkthrough 📝

	Relevant files
Enhancement	17 files build-and-push.sh Add multi-architecture Docker image build and push script build-and-push.sh Added a script to build and push Docker images with support for multiple architectures. Uses docker buildx for building images for different platforms like arm64 and amd64. Handles different build targets like base, grid, node, firefox, and chromium images. +66/-0    build-locally.sh Add script for local multi-architecture Docker image builds build-locally.sh Script for local Docker image builds, similar to build-and-push.sh but without pushing to registries. Supports building images for multiple architectures locally. +66/-0    docker-add-related-tags.sh Script to add related tags to Docker images                            docker-add-related-tags.sh Script to add related tags to Docker images after building. Utilizes a Go program to fetch all related tags from Docker Hub. +17/-0    get-access-token.sh Implement GitHub access token retrieval script                      get-access-token.sh Script to get a GitHub access token using a Python script. Sets up a virtual environment and installs necessary Python packages. +18/-0    tag-and-push-multi-arch-image.sh Add Docker multi-architecture image tagging and pushing script tag-and-push-multi-arch-image.sh Script to tag and push multi-architecture Docker images to Docker Hub. Uses digests to create and push Docker manifests. +25/-0    tag_and_push_multi-arch_browser_images.sh Script for tagging and pushing multi-arch browser Docker images tag_and_push_multi-arch_browser_images.sh Script to tag and push browser-specific Docker images for multiple architectures. Handles versioning and tagging for browsers like Chromium and Firefox. +126/-0  tag_and_push_multi-arch_major_minor.sh Add major/minor version tagging script for multi-arch Docker images tag_and_push_multi-arch_major_minor.sh Script to tag and push major and minor version tags for multi-arch Docker images. Supports pushing images without deploying them. +26/-0    Dockerfile Update Base Dockerfile to use Debian Bookworm and simplify architecture handling Base/Dockerfile Updated base Dockerfile to use Debian Bookworm as the base image. Removed architecture-specific conditions, simplifying the Dockerfile. +3/-8      Makefile Enhance Makefile with multi-architecture build targets      Makefile Added targets for building multi-architecture Docker images. Included new make targets for handling different Docker components like base, hub, and node images. +145/-4  Dockerfile Optimize NodeBase Dockerfile and remove architecture-specific conditions NodeBase/Dockerfile Updated NodeBase Dockerfile to remove architecture-specific package installations. General cleanup and optimization of package installation commands. +1/-7      Dockerfile Simplify NodeChrome Dockerfile by removing architecture-specific conditions NodeChrome/Dockerfile Updated NodeChrome Dockerfile to remove architecture-specific conditions. Simplified the handling of Chrome installation. +1/-1      Dockerfile Add new Dockerfile for NodeChromium with multi-architecture support NodeChromium/Dockerfile New Dockerfile for building NodeChromium images with multi-architecture support. Includes installation of Chromium and related binaries. +47/-0    wrap_chromium_binary Add wrapper script for Chromium binary                                      NodeChromium/wrap_chromium_binary Script to wrap the Chromium binary to ensure it runs with the --no-sandbox flag. +12/-0    Dockerfile Update NodeEdge Dockerfile to remove architecture-specific conditions NodeEdge/Dockerfile Updated NodeEdge Dockerfile to remove architecture-specific conditions. +1/-1      Dockerfile.multi-arch Add new multi-architecture Dockerfile for NodeFirefox        NodeFirefox/Dockerfile.multi-arch New Dockerfile for building NodeFirefox images for multiple architectures. Includes installation of Firefox and GeckoDriver with architecture-specific handling. +69/-0    get-all-related-tags.go Implement Go program to fetch related Docker image tags    get-all-related-tags.go Go program to fetch all related tags for a Docker image from Docker Hub. +179/-0  get-image-sha256-digest.go Add Go program to fetch Docker image SHA256 digest              get-image-sha256-digest.go Go program to fetch the SHA256 digest of a Docker image for a specific architecture from Docker Hub. +56/-0
Documentation	3 files generate_multi-arch-release_notes.sh Add release notes generation script for multi-architecture builds generate_multi-arch-release_notes.sh Generates release notes for multi-architecture Docker builds. Pulls images and tags them locally to generate comprehensive release notes. +78/-0    README.md Document multi-arch build and run procedures for NodeFirefox NodeFirefox/README.md Documentation on building and running NodeFirefox and StandaloneFirefox images for multiple architectures. +28/-0    README.md Update README with multi-architecture build and run information README.md Updated README to include information about building and running Docker images for multiple architectures. Added details on available Docker images and their Docker Hub links. +92/-7
Configuration changes	2 files config.yml Define CircleCI configuration for multi-architecture Docker builds .circleci/config.yml Extensive CircleCI configuration for building, testing, and deploying multi-architecture Docker images. Defines multiple jobs and workflows for different architectures and Docker image types. +405/-0  build-test-multi-arch.yml Setup GitHub Actions workflow for multi-architecture Docker builds .github/workflows/build-test-multi-arch.yml GitHub Actions workflow to build and test multi-architecture Docker images. Includes steps for setting up Python, building images, and running tests. +59/-0

---

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

[qodo-merge-pro]

PR Description updated to latest commit (d200a00)

• Copy walkthrough table to "Files Changed" Tab

[qodo-merge-pro]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	4, because the PR introduces a large number of changes across multiple scripts and configuration files. It includes modifications to shell scripts, Dockerfiles, and CI configuration, which requires a detailed review to ensure compatibility and correctness across different architectures and environments.
🧪 Relevant tests	Yes
⚡ Possible issues	Possible Bug: Inconsistent architecture handling in Dockerfiles. Some Dockerfiles have removed the TARGETARCH argument without ensuring the base images support multi-architecture or providing an alternative method to handle architecture-specific configurations.
	Possible Performance Issue: The use of 'sleep 5' in build scripts to avoid "failed to solve" errors may indicate underlying timing or dependency issues in the build process that could lead to flaky builds or performance inefficiencies.
	Possible Maintenance Issue: The complexity of the build scripts and multiple similar Dockerfiles for different architectures could lead to higher maintenance costs and potential for errors during updates or changes.
🔒 Security concerns	No

[qodo-merge-pro]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible issue	Add qemu_user_static as a dependency for the test_multi_arch target to ensure proper setup The test_multi_arch target is missing a dependency on qemu_user_static, which is essential for setting up the environment for multi-architecture builds. Adding this dependency ensures that the necessary setup is performed before tests are run. Makefile [559] -test_multi_arch: test_chromium_multi \ +test_multi_arch: qemu_user_static test_chromium_multi \ Suggestion importance[1-10]: 10 Why: This suggestion is highly relevant as it addresses a critical setup requirement for multi-architecture testing that was overlooked in the PR. Adding qemu_user_static as a dependency ensures that the environment is correctly prepared, which is crucial for successful multi-architecture tests.	10
	Add error handling for docker run commands to improve script robustness Consider adding error handling for the docker run commands to ensure that the script handles potential failures gracefully. This can be done by checking the exit status of each command and taking appropriate action if an error occurs. tag_and_push_multi-arch_browser_images.sh [25] CHROMIUM_VERSION=$(docker run --rm ${NAMESPACE}/node-chromium:${TAG_VERSION} chromium --version | awk '{print $2}') +if [ $? -ne 0 ]; then + echo "Failed to get Chromium version" + exit 1 +fi Suggestion importance[1-10]: 8 Why: Adding error handling for docker run commands is crucial to ensure the script does not proceed with incorrect data, enhancing its robustness and reliability.	8
	Add error handling for the docker tag command to ensure it succeeds Consider checking if the docker tag command succeeds before proceeding. This can be done by checking the exit status of the command. If the command fails, you can handle the error appropriately, for example, by logging an error message and exiting the script. docker-add-related-tags.sh [15] -docker tag $NAMESPACE/$IMAGE:$TAG $NAMESPACE/$IMAGE:$related_tag +if ! docker tag $NAMESPACE/$IMAGE:$TAG $NAMESPACE/$IMAGE:$related_tag; then + echo "Error: Failed to tag $NAMESPACE/$IMAGE:$related_tag" + exit 1 +fi Suggestion importance[1-10]: 8 Why: Adding error handling for the docker tag command is crucial to ensure the script does not proceed with erroneous states, which is a significant improvement in robustness.	8
	Remove the duplicate --load flag from the Docker build command The --load flag is duplicated in the Docker build command, which could lead to confusion or errors in the build process. Removing the duplicate flag will clean up the command and potentially avoid issues during execution. Makefile [58] -cd ./Base && docker buildx build --platform $(PLATFORMS) $(BUILD_ARGS) --load --build-arg VERSION=$(BASE_VERSION) --build-arg RELEASE=$(BASE_RELEASE) --build-arg AUTHORS=$(AUTHORS) --load -t $(NAME)/base:$(TAG_VERSION) . +cd ./Base && docker buildx build --platform $(PLATFORMS) $(BUILD_ARGS) --load --build-arg VERSION=$(BASE_VERSION) --build-arg RELEASE=$(BASE_RELEASE) --build-arg AUTHORS=$(AUTHORS) -t $(NAME)/base:$(TAG_VERSION) . Suggestion importance[1-10]: 7 Why: The suggestion correctly identifies a redundancy in the Docker build command that could lead to confusion or errors. Removing the duplicate --load flag improves the clarity and correctness of the build command.	7
Security	Quote variables in command substitutions to prevent command injection To avoid potential command injection vulnerabilities, consider quoting all variables used in command substitutions and ensure that inputs are sanitized or validated. tag_and_push_multi-arch_browser_images.sh [25] -CHROMIUM_VERSION=$(docker run --rm ${NAMESPACE}/node-chromium:${TAG_VERSION} chromium --version | awk '{print $2}') +CHROMIUM_VERSION=$(docker run --rm "${NAMESPACE}/node-chromium:${TAG_VERSION}" chromium --version | awk '{print $2}') Suggestion importance[1-10]: 9 Why: Quoting variables in command substitutions is critical for security to prevent potential command injection vulnerabilities, especially when the script involves external inputs.	9
Best practice	Use dynamic date generation for BUILD_DATE to accurately reflect the build date The BUILD_DATE environment variable is set to 'today' which might not reflect the actual date of the build. It is better to use dynamic date generation to ensure the date reflects when the build actually occurs. .circleci/config.yml [28] -BUILD_DATE: today +BUILD_DATE: $(date '+%Y-%m-%d') Suggestion importance[1-10]: 8 Why: Using a dynamic date for BUILD_DATE is a significant improvement for accuracy in build processes. This change ensures that the build date always reflects the actual date of the build.	8
	Use specific exceptions for better error handling Instead of raising a generic Exception, use more specific exceptions to provide better error context and handling. For example, use ValueError for invalid values. get-access-token.py [15-19] -raise Exception(standard_error_msg + 'Valid GITHUB_APP_ID is required to obtain an access token.') +raise ValueError(standard_error_msg + 'Valid GITHUB_APP_ID is required to obtain an access token.') Suggestion importance[1-10]: 7 Why: Using more specific exceptions like ValueError instead of a generic Exception provides clearer error information and is a good practice in Python for better error handling.	7
	Validate script inputs to ensure they are not empty Consider validating the inputs ($1, $2, $3) to ensure they are not empty before proceeding with the script. This can prevent errors related to undefined variables. docker-add-related-tags.sh [4-6] +if [ -z "$1" ] || [ -z "$2" ]; then + echo "Error: Image and tag must be specified." + exit 1 +fi IMAGE=$1 TAG=$2 -NO_PULL=$3 +NO_PULL=${3:-false} Suggestion importance[1-10]: 7 Why: Validating inputs to ensure they are not empty can prevent the script from failing unexpectedly due to undefined variables, which improves the script's robustness and user experience.	7
	Enhance the deployment condition check by adding a clear error message when deployment is skipped The script checks for the '[deploy]' tag in the commit message to proceed with deployment. It's recommended to also log a warning or error if the deployment is skipped due to the absence of this tag, to make it clear in the logs why the deployment did not proceed. .circleci/config.yml [155-160] if [ -z "$CI_DEPLOY" ] || [ "$CIRCLE_BRANCH" != "$DEPLOY_BRANCH" ]; then - echo "Cancelling run. Pass [deploy] in commit message on $DEPLOY_BRANCH to deploy" + echo "ERROR: Deployment skipped. Ensure your commit message includes '[deploy]' and is on the $DEPLOY_BRANCH branch." circleci-agent step halt else - echo "[deploy] is present in commit message to $DEPLOY_BRANCH. Running workflow to deploy container images" + echo "Proceeding with deployment as '[deploy]' is present in the commit message on $DEPLOY_BRANCH." Suggestion importance[1-10]: 7 Why: Adding a clear error message when skipping deployment due to missing [deploy] tag in the commit message is a good practice. It enhances clarity and debugging capabilities in the deployment process.	7
	Use raw strings for regex patterns to avoid escape sequence errors It's a good practice to ensure that the re.match function is used with raw strings for regex patterns to avoid unintended escape sequences. tests/test.py [177] -return re.match("(Standalone)(Chromium|Chrome|Firefox|Edge)", container) +return re.match(r"(Standalone)(Chromium|Chrome|Firefox|Edge)", container) Suggestion importance[1-10]: 6 Why: Using raw strings for regex patterns is a good practice to avoid issues with escape sequences in Python, enhancing code reliability and maintainability.	6
Enhancement	Include multiple architectures in the PLATFORMS variable to support a broader user base Ensure that the PLATFORMS variable includes all necessary platforms. The removal of linux/amd64 and addition of linux/arm64 might exclude users on AMD64 architectures. Consider supporting multiple architectures. Makefile [23] -PLATFORMS := $(or $(PLATFORMS),$(PLATFORMS),linux/arm64) +PLATFORMS := $(or $(PLATFORMS),$(PLATFORMS),linux/arm64,linux/amd64) Suggestion importance[1-10]: 8 Why: This suggestion is valid as it addresses potential compatibility issues by recommending the inclusion of multiple architectures, which is crucial for broader support.	8
Maintainability	Refactor repeated code into a function to enhance maintainability To improve the readability and maintainability of the script, consider refactoring the repeated docker run and echo commands into a function. This function can take parameters for the specific commands and messages. tag_and_push_multi-arch_browser_images.sh [25-26] -CHROMIUM_VERSION=$(docker run --rm ${NAMESPACE}/node-chromium:${TAG_VERSION} chromium --version | awk '{print $2}') -echo "Chromium version -> "${CHROMIUM_VERSION} +function log_version { + local version=$1 + local message=$2 + local tag=$3 + local version_output=$(docker run --rm ${NAMESPACE}/${tag} ${version} --version | awk '{print $2}') + echo "${message} -> ${version_output}" +} +log_version "chromium" "Chromium version" "node-chromium:${TAG_VERSION}" Suggestion importance[1-10]: 7 Why: Refactoring repeated code into a function improves maintainability and readability, making the script easier to manage and modify.	7
	Define the NAMESPACE environment variable at the workflow level to ensure consistency and reduce redundancy To ensure that the NAMESPACE environment variable is consistently set across all jobs, it is recommended to define it at the workflow level instead of repeating it in each job. This approach reduces redundancy and potential errors in variable assignment. .circleci/config.yml [27] -NAMESPACE: seleniarm +NAMESPACE: << pipeline.parameters.NAMESPACE >> Suggestion importance[1-10]: 7 Why: Setting the NAMESPACE variable at the workflow level would indeed reduce redundancy and potential errors. This suggestion is relevant and improves maintainability.	7
	Use more descriptive variable names in the short_version function Consider using more descriptive variable names for __long_version and __version_split in the short_version function to enhance code readability and maintainability. tag_and_push_multi-arch_browser_images.sh [12-14] -local __long_version=$1 -local __version_split=( ${__long_version//./ } ) +local full_version=$1 +local version_parts=( ${full_version//./ } ) Suggestion importance[1-10]: 6 Why: Using more descriptive variable names improves code readability and maintainability, making it easier for others to understand the purpose of the variables.	6
Performance	Simplify the cache key in the save_cache step to improve cache hits and reduce build times The save_cache step uses a complex key that includes workflow ID and platforms, which might lead to cache misses if the platform changes. Simplify the cache key to improve cache hits and reduce build times. .circleci/config.yml [67] -key: multi-arch-images-{{ .Branch }}-{{ .Environment.CIRCLE_WORKFLOW_ID }}-<< parameters.platforms >> +key: multi-arch-images-{{ .Branch }}-{{ checksum "Dockerfile" }} Suggestion importance[1-10]: 6 Why: Simplifying the cache key could potentially improve cache hits, although the suggestion to use checksum "Dockerfile" may not be directly applicable without verifying the presence and relevance of a Dockerfile. The concept is good but needs more context-specific validation.	6