feat: self-contained container image build #446
Conversation
WalkthroughThis pull request adjusts the build and deployment processes by updating multiple components of the CI/CD pipeline. The GitHub Actions workflow now adds a new step to capture versioning details and streamlines the Docker image publishing process. The Makefile is updated with new variables and targets to support Docker buildx integration. The Dockerfile is restructured into a multi-stage build that separates application and library compilation, and the build script has been enhanced with variable renaming, dynamic build command selection, and improved error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant GR as GitHub Runner
participant BI as build_info Step
participant BP as Build & Publish Step
participant MF as Makefile Targets
participant SP as scripts/package
participant DB as Docker Buildx
GR->>BI: Execute build_info step
BI-->>GR: Return version info (version_major, version_minor, version_patch, image_tag)
GR->>BP: Proceed with publishing using version outputs
BP->>MF: Run 'make workflow-image-build-push'
MF->>MF: Check/Create buildx machine (buildx-machine)
MF->>SP: Execute build script with provided variables
SP->>DB: Invoke Docker buildx command for image build
DB-->>SP: Complete image build and push
SP-->>MF: Return built image tag
MF-->>BP: Pass back build result
BP-->>GR: Complete the job
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
package/Dockerfile (1)
58-91: Consider adding security hardening.The final stage could benefit from additional security measures.
FROM registry.suse.com/bci/bci-base:15.6 +# Create non-root user +RUN useradd -r -u 1000 -g users longhorn +USER longhorn + +# Set strict permissions +RUN chmod 500 /longhorn-share-manager
🧹 Nitpick comments (1)
scripts/package (1)
43-47: Consider caching base images.While pulling latest base images is good for security, consider implementing a caching mechanism for better build performance in CI/CD.
- docker pull "$base_image" + # Add caching with 24h timeout + if ! docker pull "$base_image" --quiet 2>/dev/null; then + echo "Cache miss or expired, pulling latest base image..." + docker pull "$base_image" + fi
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/build.yml(2 hunks)Makefile(2 hunks)package/Dockerfile(2 hunks)scripts/package(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build binaries
- GitHub Check: Summary
🔇 Additional comments (10)
scripts/package (3)
17-18: LGTM: Good security flag handling.The script properly initializes the
IS_SECUREflag with a safe default of 'false', allowing explicit opt-in for security features.
33-42: LGTM: Secure build configuration.The script properly handles security-related build flags:
- Adds SBOM generation with
--sbom=true- Includes provenance attestation with
--attest type=provenance,mode=max
These align well with SLSA Level 3 requirements.
23-29: Verify version extraction reliability.The version extraction from
longhorn-share-managercould fail silently if the binary is not present or has unexpected output format.Makefile (3)
19-21: LGTM: Proper buildx setup.The buildx machine creation is properly conditioned and includes platform specifications.
29-32: LGTM: Secure build workflow.Good separation of concerns between regular and secure builds. The secure build properly inherits from buildx-machine target.
3-6: Verify platform compatibility.The
DEFAULT_PLATFORMSincludes darwin targets which may not be supported by all components. Ensure all dependencies support these platforms..github/workflows/build.yml (2)
14-18: LGTM: Clear output definitions.The outputs are well-defined and properly scoped for version tracking.
77-82: Verify Docker Hub authentication.The Docker Hub login is commented out. Ensure this doesn't affect the security of the build process.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check if Docker Hub authentication is required elsewhere rg -l "docker.*login|docker/login-action" .github/workflows/Length of output: 90
Action: Confirm that Docker Hub authentication isn’t needed in the build workflow
The search shows that the only mention of Docker Hub login (via docker/login-action) is in .github/workflows/build.yml and it’s fully commented out. This means that no Docker Hub authentication step is executed.
Please verify that the build process doesn’t require Docker Hub authentication—for example, if your jobs are pulling only public images and aren’t pushing to private repositories, bypassing login should have no adverse security impact. If you intend to work with private images or push to Docker Hub, you'll need to re-enable and/or configure the login step accordingly.
package/Dockerfile (2)
15-57: LGTM: Secure library building.The library builder stage properly:
- Uses specific package versions
- Cleans up package manager cache
- Sets compiler environment variables
2-13: Verify Golang version compatibility.The Golang version 1.23 specified in the base image seems incorrect as it's beyond the latest stable release.
✅ Verification successful
🌐 Web query:
What is the latest stable version of Go?💡 Result:
The latest stable version of Go (Golang) as of February 2025 is **Go 1.23**, with its most recent patch being **Go 1.23.5** (released January 16, 2025)[2][6]. This version continues Go's tradition of performance improvements, security updates, and toolchain enhancements while maintaining backward compatibility. ### Key Details: - **Latest Major Release**: Go 1.23 introduced runtime optimizations and expanded generics support[6]. - **Latest Patch**: Go 1.23.5 includes security fixes for `crypto/x509` and `net/http`, along with compiler/runtime bug fixes[2]. - **Release Policy**: Each major Go version receives support until two newer major releases exist (e.g., Go 1.23 will be supported until Go 1.25 releases)[2]. For comparison: | Version | Release Date | Latest Patch | Support Status | |---------|----------------|--------------|----------------------| | Go 1.23 | August 2024 | v1.23.5 | Actively supported | | Go 1.22 | February 2024 | v1.22.11 | Supported until Go 1.24 releases[2] | | Go 1.21 | August 2023 | v1.21.12 | Unsupported since Go 1.23 release[2][4] | Developers should upgrade to **Go 1.23.x** for security updates and performance gains[2][6]. The unstable **Go 1.24rc2** exists but isn't recommended for production use[5]. Citations: - 1: https://www.easiio.com/latest-golang-version/ - 2: https://go.dev/doc/devel/release - 3: https://github.com/golang/tools/releases - 4: https://tip.golang.org/doc/go1.21 - 5: https://go.dev/dl/ - 6: https://tip.golang.org/doc/go1.23 - 7: https://groups.google.com/g/golang-nuts/c/_ROx_UGCCCg
Golang Version Compatibility Verified: Base image is up-to-date.
- The Dockerfile's
FROM registry.suse.com/bci/golang:1.23statement is correct since, as of February 2025, Go 1.23 (with patch 1.23.5) is the latest stable release.- No changes are needed.
COLDTURNIP
force-pushed
the
feat-secure_build
branch
from
c65e4ff to
26d8ff4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/build.yml (1)
34-44: 🛠️ Refactor suggestionVersion Extraction and Branch Handling
This block differentiates the handling of tags, branches, and pull request refs. However, for tag references it would be safer to validate that the tag conforms to an expected semantic version pattern (e.g., x.y.z) before processing it. This prevents injection risks or unexpected formats.For instance, you could modify the tag branch as follows:
-if [[ "$ref" =~ 'refs/tags/' ]]; then - version=$(sed -E 's/^v([0-9.]*)$/\1' <<<$ref ) - version_major=$(cut -d. -f1 <<<$version) - version_minor=$(cut -d. -f2 <<<$version) - version_build=$(cut -d. -f3 <<<$version) - image_tag="$version" +if [[ "$ref" =~ 'refs/tags/' ]]; then + if [[ ! "$ref" =~ ^refs/tags/v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + echo "Invalid version format: $ref" + exit 1 + fi + version=$(sed -E 's/^v([0-9.]*)$/\1' <<<$ref ) + version_major=$(cut -d. -f1 <<<$version) + version_minor=$(cut -d. -f2 <<<$version) + version_build=$(cut -d. -f3 <<<$version) + image_tag="$version"This additional check helps ensure that only properly formatted tags are processed.
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
77-81: Commented-Out Docker Hub Login Block
The Docker Hub login step is fully commented out. If it is no longer needed due to the new secure build process, consider removing this block to reduce clutter and improve maintainability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build.yml(2 hunks)Makefile(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Makefile
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build binaries
- GitHub Check: Summary
🔇 Additional comments (6)
.github/workflows/build.yml (6)
14-18: Output Configuration for Build Job
The outputs block is clearly defined and correctly captures versioning details from the build_info step for later usage.
24-30: Initialization of Version Variables
Initializing version_major, version_minor, version_build, and image_tag to empty strings is a clear and effective way to avoid undefined variable issues later in the script.
32-33: Branch and Reference Assignment
The extraction of the branch (using GITHUB_HEAD_REF fallback) and assignment of ref (via GitHub expression) is well implemented. Ensure that this covers all edge cases for branches and pull requests.
46-49: Setting GitHub Action Outputs
Echoing the computed version variables to the $GITHUB_OUTPUT file is done correctly. This makes the outputs available to subsequent jobs.
51-56: Logging Version Information
Using a heredoc with cat to print version information provides a clear log output, which can assist during debugging. Just ensure that none of these values are sensitive (in this case, they are not).
84-88: Build and Publish Image Step
The step correctly sets environment variables (REPO and TAG) and invokes the make target for image build and push. Ensure that the make target "workflow-image-build-push" is updated in the Makefile and fully tested for the secure image package process.
COLDTURNIP
force-pushed
the
feat-secure_build
branch
2 times, most recently
from
75bf700 to
0cc6f1e
Compare
COLDTURNIP
requested review from
innobead,
yangchiu,
derekbit,
PhanLe1010 and
shuo-wu
|
Same questions as backing-image-manager |
derekbit
force-pushed
the
feat-secure_build
branch
from
0cc6f1e to
549c48b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
scripts/package (2)
23-29: Improve error handling for version detection.The version fallback logic could be more robust:
- The error message from
jqis suppressed- No validation of
$VERSIONfallbackif [[ -z $TAG ]]; then - if api_version=$(./bin/longhorn-share-manager version --client-only | jq ".clientVersion.apiVersion"); then + if api_version=$(./bin/longhorn-share-manager version --client-only | jq -e ".clientVersion.apiVersion" 2>/dev/null); then TAG="v${api_version}_$(date -u +%Y%m%d)" else + if [[ -z $VERSION ]]; then + echo "Error: Failed to detect version and VERSION is not set" >&2 + exit 1 + fi TAG="$VERSION" fi fi
49-59: Consider adding build argument validation.The script should validate critical build arguments before execution.
+# Validate required build arguments +if [[ $IS_SECURE == 'true' && -z $IID_FILE ]]; then + echo "Error: IID_FILE must be set for secure builds" >&2 + exit 1 +fi + echo "${build_cmd[@]}" build --no-cache \package/Dockerfile (1)
27-30: Consider pinning package versions.For reproducible builds and security, consider pinning package versions.
-RUN zypper -n install autoconf bison curl cmake doxygen make git gcc14 gcc14-c++ flex Mesa-libGL-devel libdbus-1-3 \ +RUN zypper -n install \ + autoconf-2.71 \ + bison-3.8 \ + curl-7.79.1 \ + cmake-3.27.7 \
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/build.yml(2 hunks)Makefile(2 hunks)package/Dockerfile(2 hunks)scripts/package(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/build.yml
- Makefile
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build binaries
- GitHub Check: Summary
🔇 Additional comments (6)
scripts/package (3)
10-10: LGTM: Robust build command detection.The script intelligently falls back to
docker buildxif the standalonebuildxcommand is not available.
44-47: LGTM: Security enhancement for base image updates.Good practice to pull the latest base images to get security updates.
16-21: 🏁 Script executed:#!/bin/bash # Description: Verify secure build configuration # Test: Check if secure build flags are properly set when IS_SECURE is true # Create a temporary script to test the build flags cat > test_build.sh << 'EOF' #!/bin/bash source ./scripts/package if [[ $IS_SECURE != 'true' ]]; then echo "ERROR: IS_SECURE should be true for security builds" exit 1 fi # Verify SBOM and provenance attestation flags if ! echo "${buildx_args[*]}" | grep -q "sbom=true" || ! echo "${buildx_args[*]}" | grep -q "type=provenance,mode=max"; then echo "ERROR: Missing required security flags for SLSA Level 3" exit 1 fi EOF chmod +x test_build.sh IS_SECURE=true ./test_build.shLength of output: 114
package/Dockerfile (3)
60-65: LGTM: Platform validation.Good security practice to validate target platforms early in the build process.
88-90: 🏁 Script executed:#!/bin/bash # Description: Check file permissions in the final image # Test: Build a test image and verify permissions # Create a temporary Dockerfile cat > test_permissions.dockerfile << 'EOF' FROM ${REPO:-longhornio}/${IMAGE_NAME:-longhorn-share-manager}:${TAG:-latest} RUN ls -la /usr/local/lib64 /longhorn-share-manager EOF # Build and check permissions docker build -f test_permissions.dockerfile .Length of output: 151
2-14: 🌐 Web query:What is the latest version of Go?💡 Result:
The latest stable version of Go as of February 2025 is **Go 1.23.5**, released on January 16, 2025[1]. This minor update includes security fixes for `crypto/x509` and `net/http` packages, along with bug fixes to the compiler, runtime, and `net` package[1]. Key details about recent Go versions: ## Active Release Branches - **Go 1.23.x** Latest minor version: **1.23.5** (2025-01-16) Initial release: **1.23.0** (2024-08-13)[1] - **Go 1.22.x** Latest minor version: **1.22.11** (2025-01-16)[1] Initial release: **1.22.0** (2024-02-06)[1] ## Compatibility Notes - The Go tools ecosystem (like `gopls`) now requires at least Go 1.21 to build newer versions[2] - Automatic toolchain upgrades are available for systems using Go ≥1.21[2] - Official support window narrowed to: - Latest major version for building tools - Two most recent releases for `go` command integration[2] Security updates and bug fixes are typically backported to supported release branches for six months after each major release[3][4]. Citations: - 1: https://go.dev/doc/devel/release - 2: https://github.com/golang/tools/releases - 3: https://tip.golang.org/doc/go1.23 - 4: https://tip.golang.org/doc/go1.22 - 5: https://community.idx.dev/t/go-version-is-not-the-latest/1891 - 6: https://go.dev/dl/
COLDTURNIP
force-pushed
the
feat-secure_build
branch
from
549c48b to
e0659c2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
scripts/package (1)
23-29: Consider adding version validation.The version detection logic could benefit from input validation to ensure the extracted version follows the expected format.
if [[ -z $TAG ]]; then - if api_version=$(./bin/longhorn-share-manager version --client-only | jq ".clientVersion.apiVersion"); then + if api_version=$(./bin/longhorn-share-manager version --client-only | jq -r ".clientVersion.apiVersion") && [[ $api_version =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then TAG="v${api_version}_$(date -u +%Y%m%d)" else + echo "Warning: Invalid API version format, falling back to VERSION" TAG="$VERSION" fi fiMakefile (1)
19-21: Consider error handling for buildx machine creation.The buildx machine creation suppresses errors but should verify the machine is operational.
buildx-machine: - @docker buildx create --name=$(MACHINE) --platform=$(DEFAULT_PLATFORMS) 2>/dev/null || true + @docker buildx create --name=$(MACHINE) --platform=$(DEFAULT_PLATFORMS) 2>/dev/null || true docker buildx inspect $(MACHINE) + @docker buildx inspect $(MACHINE) | grep -q "Status: running" || (echo "Error: buildx machine not running" && exit 1)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/build.yml(2 hunks)Makefile(2 hunks)package/Dockerfile(2 hunks)scripts/package(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build.yml
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build binaries
- GitHub Check: Summary
🔇 Additional comments (5)
scripts/package (3)
10-10: LGTM: Robust build command detection.The script intelligently detects and uses buildx when available, falling back to
docker buildxotherwise.
39-41: LGTM: Secure build attestation.The script correctly adds SBOM generation and provenance attestation for secure builds.
16-18: Verify secure build flag usage.The
IS_SECUREflag is introduced but its implications should be verified in the context of EIO secrets handling.❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Check if IS_SECURE flag triggers any additional security measures in related files rg -A 5 "IS_SECURE.*true" .Length of output: 611
Secure Build Flag Usage Verified, but EIO Secrets Handling Needs Clarification
- In the Makefile, the build is invoked with IS_SECURE set to true (e.g.,
MACHINE=$(MACHINE) OUTPUT_ARGS='--push' IS_SECURE=true bash scripts/package), which is the intended trigger for secure builds.- In scripts/package, when IS_SECURE equals 'true', the script appends additional buildx arguments:
'--sbom=true' and '--attest' 'type=provenance,mode=max'. These parameters enhance the build process by enabling SBOM generation and image attestations.- However, aside from these enhanced security flags, there is no explicit logic or additional measures directly addressing EIO secrets handling within the inspected code.
It’s recommended to confirm whether the secure mode’s current behavior (adding SBOM and attestation options) is sufficient for your EIO secrets requirements or if further implementation is needed.
Makefile (1)
31-32: LGTM: Clear separation of secure and standard builds.The workflow targets properly separate secure and standard build processes.
package/Dockerfile (1)
88-90: LGTM: Secure multi-stage build implementation.The multi-stage build properly separates build artifacts and minimizes the final image surface area.
COLDTURNIP
force-pushed
the
feat-secure_build
branch
3 times, most recently
from
5b09d43 to
6d325ab
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
package/Dockerfile (2)
6-7: Optimize source code copying.Consider using a
.dockerignorefile to exclude unnecessary files (e.g., tests, docs) from being copied into the image. This reduces build context size and potential attack surface.
9-13: Consider setting build script permissions in source control.Instead of setting execute permissions during build, consider:
- Setting permissions in source control (git)
- Using
COPY --chmod=755in Dockerfile-# Make the build script executable -RUN chmod +x /app/scripts/build - -# Run the build script -RUN /app/scripts/build +# Copy and run build script +COPY --chmod=755 scripts/build /app/scripts/build +RUN /app/scripts/buildscripts/package (3)
23-39: Consider adding support for additional architectures.While the platform detection handles common architectures (arm64 and amd64), consider adding support for arm/v7 for broader IoT deployment compatibility.
case $(uname -m) in aarch64 | arm64) arch=arm64 ;; +armv7l) + arch=arm/v7 + ;; x86_64) arch=amd64 ;;
60-86: Improve version handling robustness.The version handling could be more robust with better error suppression and absolute path handling.
function get_branch() { - local version_file="version" + local version_file="$(dirname "$0")/../version" if [[ ! -f $version_file ]]; then echo "Error: Version file '$version_file' not found." exit 1 fi local version=$(cat "$version_file") local branch=$(convert_version_to_major_minor_x "$version") - wget --spider "https://raw.githubusercontent.com/longhorn/dep-versions/${branch}/versions.json" >/dev/null + if ! wget --spider --quiet "https://raw.githubusercontent.com/longhorn/dep-versions/${branch}/versions.json" 2>/dev/null; then + echo "Warning: Branch ${branch} not found, falling back to main" + echo "main" + return + fi - if [ $? -eq 0 ]; then - echo "${branch}" - else - echo "main" - fi + echo "${branch}"
98-114: Simplify build command execution.The build command is duplicated (once for echo, once for execution). Consider using a function or variable to avoid duplication.
+build_args=( + --no-cache + "${builder_args[@]}" + "${iid_file_args[@]}" + "${buildx_args[@]}" + --build-arg ARCH="${arch}" + --build-arg SRC_BRANCH="${SRC_BRANCH}" + --build-arg SRC_TAG="${SRC_TAG}" + -t "$image" + -f package/Dockerfile + . +) + echo "Building image ${image}" -echo "${build_cmd[@]}" build --no-cache \ - "${builder_args[@]}" \ - "${iid_file_args[@]}" \ - "${buildx_args[@]}" \ - --build-arg ARCH="${arch}" \ - --build-arg SRC_BRANCH="${SRC_BRANCH}" \ - --build-arg SRC_TAG="${SRC_TAG}" \ - -t "$image" -f package/Dockerfile . -"${build_cmd[@]}" build --no-cache \ - "${builder_args[@]}" \ - "${iid_file_args[@]}" \ - "${buildx_args[@]}" \ - --build-arg ARCH="${arch}" \ - --build-arg SRC_BRANCH="${SRC_BRANCH}" \ - --build-arg SRC_TAG="${SRC_TAG}" \ - -t "$image" -f package/Dockerfile . +echo "${build_cmd[@]}" build "${build_args[@]}" +"${build_cmd[@]}" build "${build_args[@]}"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/build.yml(2 hunks)Makefile(2 hunks)package/Dockerfile(3 hunks)scripts/package(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/build.yml
- Makefile
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build binaries
- GitHub Check: Summary
🔇 Additional comments (5)
package/Dockerfile (3)
15-15: LGTM! Clear stage naming.The rename from
buildtolib_builderimproves clarity by better describing the stage's purpose.
55-55: LGTM! Clean architecture handling.Good use of Docker's TARGETPLATFORM for architecture-specific builds.
79-81: Verify binary path and architecture variable.The binary path includes the architecture variable. Ensure that:
- The build script generates binaries with the correct architecture suffix
- The ${ARCH} variable is properly set during multi-platform builds
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify build script output paths and architecture handling # Check if build script handles architecture-specific output grep -r "bin/.*-\${ARCH\}" . # Check architecture handling in build script grep -r "TARGETPLATFORM\|ARCH" scripts/Length of output: 463
Verified Binary Path and Architecture Variable Usage
- The build script in
scripts/buildgenerates the binary with the architecture suffix (e.g.,bin/longhorn-share-manager-$arch).- The Dockerfile properly references the binary using
${ARCH}.- The build scripts consistently set the
ARCHvariable (and build args) as shown inscripts/packageandscripts/test.scripts/package (2)
1-22: LGTM! Well-structured initialization with security considerations.The script initialization and variable setup follow best practices with proper error handling and sensible defaults. The new security-related variables (
IS_SECURE,IID_FILE, etc.) align well with the PR's security objectives.
49-59: LGTM! Secure build configuration aligns with SLSA Level 3.The secure build configuration with SBOM generation and provenance attestation aligns well with the PR's security objectives and SLSA Level 3 requirements.
COLDTURNIP
force-pushed
the
feat-secure_build
branch
4 times, most recently
from
e32bd36 to
2f59fef
Compare
|
This pull request is now in conflict. Could you fix it @COLDTURNIP? 🙏 |
Signed-off-by: Raphanus Lo <[email protected]>
COLDTURNIP
force-pushed
the
feat-secure_build
branch
from
2f59fef to
14cced1
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
scripts/package (1)
48-54: 🛠️ Refactor suggestionImprove API version extraction error handling.
The API version extraction could fail silently. Consider adding error handling and logging.
if [[ -z $TAG ]]; then - if API_VERSION=$(./bin/longhorn-share-manager-"${ARCH}" version --client-only | jq ".clientVersion.apiVersion"); then + if ! API_VERSION=$(./bin/longhorn-share-manager-"${ARCH}" version --client-only | jq -e ".clientVersion.apiVersion" 2>/dev/null); then + echo "Warning: Failed to extract API version, falling back to VERSION=${VERSION}" + TAG="${VERSION}" + else TAG="v${API_VERSION}_$(date -u +%Y%m%d)" - else - TAG="${VERSION}" fi fi.github/workflows/build.yml (1)
34-39:⚠️ Potential issueAdd version format validation.
The version extraction should validate the format to prevent injection attacks.
if [[ "$ref" =~ 'refs/tags/' ]]; then + if [[ ! "$ref" =~ ^refs/tags/v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + echo "Error: Invalid version format: $ref" + exit 1 + fi version=$(sed -E 's/^v([0-9]*\.[0-9]*\.[0-9]*).*$/\1/' <<<$ref ) version_major=$(cut -d. -f1 <<<$version) version_minor=$(cut -d. -f2 <<<$version) version_patch=$(cut -d. -f3 <<<$version)
🧹 Nitpick comments (3)
scripts/package (2)
30-46: Enhance platform validation.The platform handling could be more robust by validating the format of
TARGET_PLATFORMSand providing clearer error messages.if [[ ${TARGET_PLATFORMS} ]] ; then + if [[ ! ${TARGET_PLATFORMS} =~ ^linux/(amd64|arm64)$ ]]; then + echo "Error: Invalid TARGET_PLATFORMS format. Expected 'linux/amd64' or 'linux/arm64'" + exit 1 + fi IFS='/' read -r OS ARCH <<<"${TARGET_PLATFORMS}" BUILDX_ARGS+=('--platform' "${TARGET_PLATFORMS}") else
94-95: Consider adding file permissions.The script creates the
bindirectory but doesn't set specific permissions.-mkdir ./bin || true +mkdir -p ./bin +chmod 755 ./bin echo "${IMAGE}" > ./bin/latest_image +chmod 644 ./bin/latest_image.github/workflows/build.yml (1)
82-86: Add retry mechanism for image build.Consider adding retries for the image build step to handle transient failures.
- name: Build and publish image + id: build_image env: REPO: docker.io/longhornio TAG: ${{ needs.build.outputs.image_tag }} - run: make workflow-image-build-push + run: | + max_attempts=3 + attempt=1 + while [ $attempt -le $max_attempts ]; do + if make workflow-image-build-push; then + exit 0 + fi + echo "Attempt $attempt failed. Retrying in 30 seconds..." + sleep 30 + attempt=$((attempt + 1)) + done + echo "Failed after $max_attempts attempts" + exit 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/build.yml(2 hunks)Makefile(2 hunks)package/Dockerfile(3 hunks)scripts/package(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build binaries
- GitHub Check: Summary
🔇 Additional comments (2)
scripts/package (1)
74-77: Verify base image availability.The script pulls base images but doesn't verify if the pull was successful.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check if base images are available grep 'FROM.*/' package/Dockerfile | awk '{print $2}' | while read -r STAGE_BASE_IMAGE; do if ! docker pull "${STAGE_BASE_IMAGE}"; then echo "Failed to pull base image: ${STAGE_BASE_IMAGE}" exit 1 fi doneLength of output: 366
Action: Enhance the script with a Docker availability check.
The updated code correctly verifies that pulling each base image succeeds by examining the return status of the
docker pullcommand. However, the output you shared indicates that thedockercommand wasn’t found, which caused the script to exit even though the logic is sound. This suggests that the testing environment is missing Docker. To avoid a false negative, consider adding a pre-check to ensure Docker is installed before attempting any pull operations. For example:
- Insert a check at the start of the script:
if ! command -v docker &> /dev/null; then echo "docker command not found. Please install Docker to proceed." exit 1 fiThis addition will help provide a clearer error message in environments where Docker isn’t available rather than failing during the pull step.
package/Dockerfile (1)
2-2: Verify Golang base image version.The specified golang:1.23 version seems incorrect as it's beyond the latest available version.
✅ Verification successful
🌐 Web query:
What is the latest version of Golang?💡 Result:
The latest stable version of Go (Golang) as of February 2025 is Go 1.23.6, released on February 4, 2025[4]. This minor update includes security fixes to the
crypto/ellipticpackage and bug fixes to the compiler andgocommand[4].Key details about recent Go versions:
- Go 1.23 (major release) introduced toolchain improvements while maintaining compatibility[3]
- Go 1.24rc2 exists as an unstable release candidate[6]
- The Go team maintains support for only the two most recent major versions (currently 1.22.x and 1.23.x)[1]
To install the latest version:
go install golang.org/dl/go1.23.6@latestThe release policy ensures automatic toolchain upgrades for users with Go ≥1.21 via
GOTOOLCHAIN=auto[1]. Security updates are typically backported to supported versions like 1.22.x and 1.23.x[4].Citations:
- 1: https://github.com/golang/tools/releases
- 2: https://forum.exercism.org/t/update-go-version-to-the-latest-one/9326
- 3: https://tip.golang.org/doc/go1.23
- 4: https://go.dev/doc/devel/release
- 5: https://tip.golang.org/doc/go1.22
- 6: https://go.dev/dl/
- 7: https://go.dev/doc/install
- 8: https://groups.google.com/g/golang-nuts/c/_ROx_UGCCCg
Golang Base Image Version Confirmed:
The Dockerfile line usingFROM registry.suse.com/bci/golang:1.23is valid since Go 1.23 (currently at patch level 1.23.6 as of February 2025) is indeed the latest stable release. No changes are required.
Which issue(s) this PR fixes:
Supports security build.
What this PR does / why we need it:
Special notes for your reviewer:
The secrets provided by EIO MUST NOT present in any visible output.
Additional documentation or context
Guidance on achieving SLSA Level 3