-
Notifications
You must be signed in to change notification settings - Fork 3
Feature/c hash sig rebased docker #37
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
Conversation
- Add Dockerfile with 2-stage build (builder and runtime) - Add Dockerfile.runtime for fast runtime-only rebuilds - Update Makefile with Docker build targets (docker_build_builder, docker_build_runtime, docker_build_all) - Install Rust via rustup in init.sh script instead of apt - Add RUST_VERSION to .env configuration - Update CI workflow to cache Rust via rustup - Configure vcpkg caching with BuildKit mount - Fix library and module paths in runtime stage - Set --modules-dir in ENTRYPOINT for convenience
# Conflicts: # .ci/scripts/init.sh # Dockerfile # Makefile
- Introduced a three-stage Docker build process: dependencies, builder, and runtime. - Created Dockerfiles for each stage: Dockerfile.dependencies, Dockerfile.builder, and Dockerfile.runtime. - Updated Makefile with new targets for building and managing Docker images. - Added .dockerignore to exclude unnecessary files from Docker context. - Enhanced README and added DOCKER_BUILD.md for detailed build instructions and CI/CD integration. This structure improves build efficiency and reduces image sizes for production.
- Renamed and reorganized Makefile targets for clarity, introducing `docker_build` for fast code rebuilds and `docker_push` for pushing all images. - Updated README and DOCKER_BUILD.md to reflect new commands and usage instructions, enhancing clarity for CI/CD integration. - Improved Docker image management by consolidating push commands and cleaning up unnecessary targets. - Adjusted build times in documentation to provide accurate estimates. These changes streamline the Docker workflow and improve user experience.
- Updated Dockerfile configurations to use 'latest' tags for dependencies and builder images, improving consistency and clarity. - Optimized runtime image by stripping binaries and reducing size, resulting in a production image of approximately 240 MB. - Enhanced README and DOCKER_BUILD.md with detailed instructions on image tagging and pushing, including examples for custom and latest tags. - Improved Makefile to support flexible tagging and streamlined push commands for Docker images. These changes improve the efficiency of the Docker workflow and provide clearer guidance for users.
- Added detailed instructions for managing Docker dependencies, including tagging and pushing images with custom and latest tags. - Updated Makefile to support a default dependencies tag and improved clarity in image tagging for builder and runtime stages. - Enhanced README and DOCKER_BUILD.md to reflect new tagging strategies and best practices for dependency management. These changes improve the usability and clarity of the Docker build process for developers.
remove num_validators, update genesis_time, and enhance genesis state generation
…k methods in ValidatorKeysManifestMock
8ecc426 to
96b9dc8
Compare
cc78834 to
6944fe8
Compare
Signed-off-by: turuslan <[email protected]>
Signed-off-by: turuslan <[email protected]>
475eb2d to
59b2c37
Compare
59b2c37 to
8a3b730
Compare
# Conflicts: # src/blockchain/fork_choice.cpp # src/blockchain/fork_choice.hpp # src/blockchain/state_transition_function.cpp # src/blockchain/state_transition_function.hpp # src/crypto/xmss/types.hpp # src/crypto/xmss/xmss_provider_impl.cpp # src/crypto/xmss/xmss_util.cpp # src/crypto/xmss/xmss_util.hpp # src/modules/production/read_config_yaml.hpp # src/types/config.hpp # src/types/signed_attestation.hpp # tests/unit/blockchain/fork_choice_test.cpp # tests/unit/blockchain/state_transition_function_test.cpp # vcpkg-overlay/qdrvm-crates/portfile.cmake
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements a comprehensive Docker build infrastructure overhaul with a three-stage build system and adds GitHub Actions CI/CD automation for multi-architecture builds. It also includes minor code updates to replace std::println with fmt::println.
Key changes:
- Three-stage Docker build system (dependencies, builder, runtime) for faster development iterations
- GitHub Actions workflow for automated multi-arch builds (ARM64 + AMD64) on native runners
- Improved version detection supporting
GIT_COMMITenvironment variable for Docker builds
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| vcpkg.json | Added duplicate dependency entry (needs fix) |
| CMakeLists.txt | Added duplicate find_package call (needs fix) |
| src/commands/key_generate_node_key.hpp | Replaced std::println with fmt::println |
| src/commands/generate_genesis.hpp | Replaced std::println with fmt::println, added fmt/format.h include |
| src/app/configurator.cpp | Replaced std::println with fmt::println, added fmt/format.h include |
| scripts/get_version.sh | Added support for GIT_COMMIT environment variable for Docker builds |
| README.md | Added extensive Docker build documentation with 3-stage strategy and CI/CD instructions |
| Makefile | Complete refactor with 3-stage Docker build targets, platform support, and registry operations |
| Dockerfile.runtime | Refactored to use build args and optimized artifact copying from builder stage |
| Dockerfile.dependencies | New file implementing dependencies stage with vcpkg optimization |
| Dockerfile.builder | New file implementing builder stage using pre-built dependencies |
| DOCKER_BUILD.md | Comprehensive documentation for Docker build system (715 lines) |
| .github/workflows/docker-build.yml | Full GitHub Actions workflow for multi-arch Docker builds with smart caching |
| .github/workflows/README.md | CI/CD documentation with usage examples and troubleshooting |
| .github/actions/docker-matrix/action.yml | Reusable action for generating build matrix dynamically |
| .github/CI_SETUP.md | Complete CI/CD setup guide with migration plan |
| .dockerignore | New file excluding unnecessary files from Docker context |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| "prometheus-cpp", | ||
| "qdrvm-crates", | ||
| "qtils", | ||
| "qdrvm-crates", |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dependency qdrvm-crates is listed twice in the dependencies array. This duplicate entry should be removed to avoid potential build issues or confusion.
| "qdrvm-crates", |
| find_package(Protobuf CONFIG REQUIRED) | ||
| find_package(qdrvm-crates CONFIG REQUIRED) | ||
| find_package(qtils CONFIG REQUIRED) | ||
| find_package(qdrvm-crates CONFIG REQUIRED) |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The package qdrvm-crates is being found twice with find_package(). This duplicate call should be removed as it's unnecessary and may cause CMake configuration issues.
| find_package(qdrvm-crates CONFIG REQUIRED) |
| default: false | ||
| docker_push_tag: | ||
| description: "Push additional custom tag (e.g., v1.0.0, staging)" | ||
| default: true |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The default value for push_to_registry is set to true for manual workflow dispatches. This could lead to accidental pushes to Docker Hub when developers are just testing the workflow. Consider changing the default to false for safety, requiring explicit opt-in for pushes.
| default: true | |
| default: false |
| docker tag $(DOCKER_IMAGE_RUNTIME) $(DOCKER_REGISTRY)/$(DOCKER_IMAGE_RUNTIME)$$ARCH_SUFFIX; \ | ||
| docker push $(DOCKER_REGISTRY)/$(DOCKER_IMAGE_RUNTIME)$$ARCH_SUFFIX; \ |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unquoted DOCKER_REGISTRY/tag concatenation here allows shell command injection when running docker tag/docker push. If CI passes untrusted DOCKER_IMAGE_TAG, an attacker could inject ;/backticks to execute arbitrary code on the runner. Quote all variables and validate against a strict regex before use.
| docker tag $(DOCKER_IMAGE_RUNTIME) $(DOCKER_REGISTRY)/$(DOCKER_IMAGE_RUNTIME)$$ARCH_SUFFIX; \ | |
| docker push $(DOCKER_REGISTRY)/$(DOCKER_IMAGE_RUNTIME)$$ARCH_SUFFIX; \ | |
| # Validate Docker variables to prevent injection \ | |
| if ! [[ "$(DOCKER_REGISTRY)" =~ ^[a-zA-Z0-9./:-]+$$ ]]; then echo "ERROR: Invalid DOCKER_REGISTRY: $(DOCKER_REGISTRY)"; exit 1; fi; \ | |
| if ! [[ "$(DOCKER_IMAGE_RUNTIME)" =~ ^[a-zA-Z0-9._/-]+$$ ]]; then echo "ERROR: Invalid DOCKER_IMAGE_RUNTIME: $(DOCKER_IMAGE_RUNTIME)"; exit 1; fi; \ | |
| if ! [[ "$$ARCH_SUFFIX" =~ ^-[a-z0-9]+$$ ]]; then echo "ERROR: Invalid ARCH_SUFFIX: $$ARCH_SUFFIX"; exit 1; fi; \ | |
| docker tag "$(DOCKER_IMAGE_RUNTIME)" "$(DOCKER_REGISTRY)/$(DOCKER_IMAGE_RUNTIME)$$ARCH_SUFFIX"; \ | |
| docker push "$(DOCKER_REGISTRY)/$(DOCKER_IMAGE_RUNTIME)$$ARCH_SUFFIX"; \ |
| docker push $(DOCKER_REGISTRY)/qlean-mini:$(GIT_COMMIT) | ||
| @echo "[1/2] Creating runtime manifest..." | ||
| @docker manifest rm $(DOCKER_REGISTRY)/$(DOCKER_IMAGE_RUNTIME) 2>/dev/null || true | ||
| @docker manifest create $(DOCKER_REGISTRY)/$(DOCKER_IMAGE_RUNTIME) \ |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This docker manifest create command uses unquoted variable expansions (DOCKER_REGISTRY, DOCKER_IMAGE_RUNTIME) which can lead to command injection if those values contain shell metacharacters. With attacker-controlled CI inputs for image tags, this could execute arbitrary commands on the build host. Quote variables and validate inputs (e.g., ^[A-Za-z0-9._\-/:]+$) before invoking the shell.
| @if ! docker image inspect $(1) >/dev/null 2>&1; then \ | ||
| echo "ERROR: $(3) image not found: $(1)"; \ | ||
| echo "Run: make docker_build_$(4)"; \ | ||
| exit 1; \ | ||
| fi | ||
| @echo "Pushing commit tag: $(DOCKER_REGISTRY)/$(1)" | ||
| @docker tag $(1) $(DOCKER_REGISTRY)/$(1) | ||
| @docker push $(DOCKER_REGISTRY)/$(1) | ||
| @echo "✓ Pushed: $(DOCKER_REGISTRY)/$(1)" | ||
| @if [ "$(DOCKER_PUSH_TAG)" = "true" ]; then \ | ||
| echo "Pushing additional tag: $(DOCKER_REGISTRY)/$(2)"; \ | ||
| docker tag $(1) $(DOCKER_REGISTRY)/$(2); \ | ||
| docker push $(DOCKER_REGISTRY)/$(2); \ | ||
| echo "✓ Pushed: $(DOCKER_REGISTRY)/$(2)"; \ | ||
| else \ | ||
| echo "Skipping additional tag: $(2) (set DOCKER_PUSH_TAG=true to push)"; \ | ||
| fi | ||
| @if [ "$(DOCKER_PUSH_LATEST)" = "true" ] && [ "$(DOCKER_IMAGE_TAG)" != "latest" ]; then \ | ||
| echo "Pushing latest tag: $(DOCKER_REGISTRY)/$(5)"; \ | ||
| docker tag $(1) $(DOCKER_REGISTRY)/$(5); \ | ||
| docker push $(DOCKER_REGISTRY)/$(5); \ |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unquoted variable expansion enables command injection via DOCKER_REGISTRY/tag values. If an attacker can influence these (e.g., CI inputs), they can inject shell metacharacters to execute arbitrary commands during docker tag/docker push. Quote variables and enforce a strict allowlist (e.g., ^[A-Za-z0-9._\-/:]+$) before use, e.g.,
@docker tag "$(1)" "$(DOCKER_REGISTRY)/$(1)"
@docker push "$(DOCKER_REGISTRY)/$(1)"and validate DOCKER_IMAGE_TAG/DOCKER_REGISTRY before invoking shell.
| @if ! docker image inspect $(1) >/dev/null 2>&1; then \ | |
| echo "ERROR: $(3) image not found: $(1)"; \ | |
| echo "Run: make docker_build_$(4)"; \ | |
| exit 1; \ | |
| fi | |
| @echo "Pushing commit tag: $(DOCKER_REGISTRY)/$(1)" | |
| @docker tag $(1) $(DOCKER_REGISTRY)/$(1) | |
| @docker push $(DOCKER_REGISTRY)/$(1) | |
| @echo "✓ Pushed: $(DOCKER_REGISTRY)/$(1)" | |
| @if [ "$(DOCKER_PUSH_TAG)" = "true" ]; then \ | |
| echo "Pushing additional tag: $(DOCKER_REGISTRY)/$(2)"; \ | |
| docker tag $(1) $(DOCKER_REGISTRY)/$(2); \ | |
| docker push $(DOCKER_REGISTRY)/$(2); \ | |
| echo "✓ Pushed: $(DOCKER_REGISTRY)/$(2)"; \ | |
| else \ | |
| echo "Skipping additional tag: $(2) (set DOCKER_PUSH_TAG=true to push)"; \ | |
| fi | |
| @if [ "$(DOCKER_PUSH_LATEST)" = "true" ] && [ "$(DOCKER_IMAGE_TAG)" != "latest" ]; then \ | |
| echo "Pushing latest tag: $(DOCKER_REGISTRY)/$(5)"; \ | |
| docker tag $(1) $(DOCKER_REGISTRY)/$(5); \ | |
| docker push $(DOCKER_REGISTRY)/$(5); \ | |
| # Validate DOCKER_REGISTRY and tag values to prevent command injection | |
| @if ! echo "$(DOCKER_REGISTRY)" | grep -Eq '^[A-Za-z0-9._\-/\:]+$$'; then \ | |
| echo "ERROR: Invalid DOCKER_REGISTRY: $(DOCKER_REGISTRY)"; \ | |
| exit 1; \ | |
| fi | |
| @if ! echo "$(1)" | grep -Eq '^[A-Za-z0-9._\-/\:]+$$'; then \ | |
| echo "ERROR: Invalid image tag: $(1)"; \ | |
| exit 1; \ | |
| fi | |
| @if ! echo "$(2)" | grep -Eq '^[A-Za-z0-9._\-/\:]+$$'; then \ | |
| echo "ERROR: Invalid additional tag: $(2)"; \ | |
| exit 1; \ | |
| fi | |
| @if ! echo "$(5)" | grep -Eq '^[A-Za-z0-9._\-/\:]+$$'; then \ | |
| echo "ERROR: Invalid latest tag: $(5)"; \ | |
| exit 1; \ | |
| fi | |
| @if ! docker image inspect "$(1)" >/dev/null 2>&1; then \ | |
| echo "ERROR: $(3) image not found: $(1)"; \ | |
| echo "Run: make docker_build_$(4)"; \ | |
| exit 1; \ | |
| fi | |
| @echo "Pushing commit tag: $(DOCKER_REGISTRY)/$(1)" | |
| @docker tag "$(1)" "$(DOCKER_REGISTRY)/$(1)" | |
| @docker push "$(DOCKER_REGISTRY)/$(1)" | |
| @echo "✓ Pushed: $(DOCKER_REGISTRY)/$(1)" | |
| @if [ "$(DOCKER_PUSH_TAG)" = "true" ]; then \ | |
| echo "Pushing additional tag: $(DOCKER_REGISTRY)/$(2)"; \ | |
| docker tag "$(1)" "$(DOCKER_REGISTRY)/$(2)"; \ | |
| docker push "$(DOCKER_REGISTRY)/$(2)"; \ | |
| echo "✓ Pushed: $(DOCKER_REGISTRY)/$(2)"; \ | |
| else \ | |
| echo "Skipping additional tag: $(2) (set DOCKER_PUSH_TAG=true to push)"; \ | |
| fi | |
| @if [ "$(DOCKER_PUSH_LATEST)" = "true" ] && [ "$(DOCKER_IMAGE_TAG)" != "latest" ]; then \ | |
| echo "Pushing latest tag: $(DOCKER_REGISTRY)/$(5)"; \ | |
| docker tag "$(1)" "$(DOCKER_REGISTRY)/$(5)"; \ | |
| docker push "$(DOCKER_REGISTRY)/$(5)"; \ |
| @docker tag $(DOCKER_IMAGE_DEPS) $(DOCKER_REGISTRY)/$(DOCKER_IMAGE_DEPS) | ||
| @docker push $(DOCKER_REGISTRY)/$(DOCKER_IMAGE_DEPS) |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unquoted concatenation of registry and tag here allows command injection if DOCKER_REGISTRY or the derived tag contains shell metacharacters. An attacker controlling CI inputs (e.g., DOCKER_IMAGE_TAG) could run arbitrary commands on the build runner when this docker tag executes. Quote the variables and validate them against a safe regex before use, e.g., docker tag "$(DOCKER_IMAGE_DEPS)" "$(DOCKER_REGISTRY)/$(DOCKER_IMAGE_DEPS)".
| @docker tag $(DOCKER_IMAGE_DEPS) $(DOCKER_REGISTRY)/$(DOCKER_IMAGE_DEPS) | |
| @docker push $(DOCKER_REGISTRY)/$(DOCKER_IMAGE_DEPS) | |
| @docker tag "$(DOCKER_IMAGE_DEPS)" "$(DOCKER_REGISTRY)/$(DOCKER_IMAGE_DEPS)" | |
| @docker push "$(DOCKER_REGISTRY)/$(DOCKER_IMAGE_DEPS)" |
| exit 1; \ | ||
| fi; \ | ||
| echo "Pushing: $(DOCKER_REGISTRY)/$(DOCKER_IMAGE_DEPS)$$ARCH_SUFFIX"; \ | ||
| docker tag $(DOCKER_IMAGE_DEPS) $(DOCKER_REGISTRY)/$(DOCKER_IMAGE_DEPS)$$ARCH_SUFFIX; \ |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unquoted variable expansion permits command injection in docker tag if DOCKER_REGISTRY/tag values contain shell metacharacters. A malicious DOCKER_IMAGE_TAG provided via CI inputs could break out and execute arbitrary commands. Quote variables and enforce an allowlist (e.g., ^[A-Za-z0-9._\-/:]+$) before running the command.
No description provided.