Skip to content

feat(ci): add buildkit dockerfile test for docker/Dockerfile #1489

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.

Sign up for GitHub

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

Open
JavaPythonAIForBAT wants to merge 9 commits into
base: main
Choose a base branch
from
Open

feat(ci): add buildkit dockerfile test for docker/Dockerfile #1489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
43391f4
feat(ci): add buildkit dockerfile test for docker/Dockerfile
JavaPythonAIForBAT Aug 11, 2026
7e4028d
fix: correct runner names (remove -buildkit suffix)
JavaPythonAIForBAT Aug 11, 2026
e01ee85
fix: remove PYTORCH_INDEX_URL from pip extra-index-url
JavaPythonAIForBAT Aug 11, 2026
443f486
fix: remove unused PYTORCH_INDEX_URL build-arg from workflow
JavaPythonAIForBAT Aug 11, 2026
4eaf965
fix: add PYTORCH_INDEX_URL back with real PyTorch index
JavaPythonAIForBAT Aug 11, 2026
9aea29f
fix: install torch separately via cache service index
JavaPythonAIForBAT Aug 11, 2026
bd4cc52
fix: use --find-links instead of --index-url for torch install
JavaPythonAIForBAT Aug 11, 2026
a069f19
fix: increase pip timeout to 600s for slow torch downloads
JavaPythonAIForBAT Aug 11, 2026
2643194
feat: login to SWR via docker login-action with GitHub secrets
JavaPythonAIForBAT Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions .github/workflows/buildkit-dockerfile-test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
name: buildkit-dockerfile-test

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security · medium]
No explicit permissions are set for the workflow. While this workflow doesn't directly use GITHUB_TOKEN, it's a security best practice to explicitly declare minimal permissions (e.g., permissions: {}) to adhere to principle of least privilege and prevent accidental credential misuse.

Suggestion:

Suggested change
name: buildkit-dockerfile-test
name: buildkit-dockerfile-test
permissions: {}
on:

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[maintainability · medium]
No concurrency group is defined. When the workflow is triggered by multiple pull_request events (e.g., pushes to the same PR or different PRs), redundant builds will run concurrently, wasting runner resources. Add a concurrency block to cancel in-progress runs for the same PR.

Suggestion:

Suggested change
name: buildkit-dockerfile-test
name: buildkit-dockerfile-test
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true
on:


on:
pull_request:
paths:
Comment on lines +1 to +5

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security · medium]
The workflow lacks a concurrency group. For pull_request triggers, rapid successive pushes (e.g., force-pushing amended commits) can spawn multiple redundant builds for the same PR. Add a concurrency group keyed on the PR ref to cancel in-progress runs and avoid wasted resources.

Suggestion:

Suggested change
name: buildkit-dockerfile-test
on:
pull_request:
paths:
name: buildkit-dockerfile-test
concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: true
on:
pull_request:
paths:

- '.github/workflows/buildkit-dockerfile-test.yaml'
- 'docker/Dockerfile'
workflow_dispatch:
Comment on lines +1 to +8

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security · low]
No permissions key is declared for the workflow. While the default permissions for pull_request events are read-only (secure), adding an explicit least-privilege permissions: block at the workflow or job level makes the security posture transparent and prevents accidental privilege creep when triggers change.

Suggestion:

Suggested change
name: buildkit-dockerfile-test
on:
pull_request:
paths:
- '.github/workflows/buildkit-dockerfile-test.yaml'
- 'docker/Dockerfile'
workflow_dispatch:
name: buildkit-dockerfile-test
permissions:
contents: read
on:
pull_request:
paths:
- '.github/workflows/buildkit-dockerfile-test.yaml'
- 'docker/Dockerfile'
workflow_dispatch:

Comment on lines +3 to +8

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security · low]
Triggers pull_request (not pull_request_target) — no dangerous checkout of PR head code with elevated permissions. The paths filter correctly limits runs to only when relevant files change. Good.

Comment on lines +1 to +8

@github-actions github-actions Bot Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security · medium]
The workflow lacks a top-level permissions key. Without explicit permissions, the default access level depends on the triggering event and repository context — for pull_request events from the same repository, the default is write-all, which grants broad, unnecessary access. Since this workflow passes secrets (SWR_USERNAME, SWR_PASSWORD) and only needs to check out code, log into a container registry, and push images, it should restrict permissions to the minimum. Other workflows in this repository demonstrate two safe approaches:

  • permissions: read-all (ci.yml)
  • permissions: {} with per-job permissions: blocks (Ascend950-ci.yml)

Recommendation: Add permissions: read-all at the top level, or permissions: {} and grant only contents: read at the job level.

Suggestion:

Suggested change
name: buildkit-dockerfile-test
on:
pull_request:
paths:
- '.github/workflows/buildkit-dockerfile-test.yaml'
- 'docker/Dockerfile'
workflow_dispatch:
name: buildkit-dockerfile-test
on:
pull_request:
paths:
- '.github/workflows/buildkit-dockerfile-test.yaml'
- 'docker/Dockerfile'
workflow_dispatch:
permissions: read-all


jobs:
Comment on lines +3 to +10

@github-actions github-actions Bot Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[maintainability · low]
The workflow does not define a concurrency group. When multiple commits are pushed to the same pull request in quick succession, redundant workflow runs will be triggered and executed in parallel, wasting runner resources. Adding a concurrency group with cancel-in-progress: true ensures only the latest run proceeds and cancels any in-progress runs for the same PR. This pattern is already used in other workflows in this repository (e.g., ci.yml and Ascend950-ci.yml).

Suggestion:

Suggested change
on:
pull_request:
paths:
- '.github/workflows/buildkit-dockerfile-test.yaml'
- 'docker/Dockerfile'
workflow_dispatch:
jobs:
on:
pull_request:
paths:
- '.github/workflows/buildkit-dockerfile-test.yaml'
- 'docker/Dockerfile'
workflow_dispatch:
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true
jobs:

build-test:
Comment on lines +10 to +11

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[maintainability · medium]
The job build-test lacks timeout-minutes. Docker builds can occasionally hang or take much longer than expected, especially on self-hosted runners. Without a timeout, a stuck build could consume runner resources indefinitely. Set an appropriate timeout based on expected build duration.

Suggestion:

Suggested change
jobs:
build-test:
jobs:
build-test:
timeout-minutes: 60

name: "build docker/Dockerfile (${{ matrix.runner_info.arch }})"
runs-on: ${{ matrix.runner_info.runner }}
Comment on lines +11 to +13

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security · high]
No timeout-minutes is set for the job. The runners (linux-aarch64-cpu-4, linux-amd64-cpu-4) appear to be self-hosted runners. Without a timeout, a stuck build could run indefinitely, consuming runner resources and potentially racking up costs. Add a reasonable timeout (e.g., 60 minutes) at the job level.

Suggestion:

Suggested change
build-test:
name: "build docker/Dockerfile (${{ matrix.runner_info.arch }})"
runs-on: ${{ matrix.runner_info.runner }}
build-test:
name: "build docker/Dockerfile (${{ matrix.runner_info.arch }})"
runs-on: ${{ matrix.runner_info.runner }}
timeout-minutes: 60

container:
image: swr.cn-southwest-2.myhuaweicloud.com/base_image/ascend-ci/cann:9.0.0-a3-ubuntu22.04-py3.12
Comment on lines +14 to +15

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security · low]
The container image for the job uses a specific tag (cann:9.0.0-a3-ubuntu22.04-py3.12), which is good. However, consider using a digest (@sha256:...) for the container image as well to ensure immutability and reproducibility of the build environment.

Suggestion:

Suggested change
container:
image: swr.cn-southwest-2.myhuaweicloud.com/base_image/ascend-ci/cann:9.0.0-a3-ubuntu22.04-py3.12
container:
image: swr.cn-southwest-2.myhuaweicloud.com/base_image/ascend-ci/cann@sha256:<digest>

Comment on lines +10 to +15

@github-actions github-actions Bot Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[bug · medium]
The job build-test runs on self-hosted runners (linux-aarch64-cpu-4, linux-amd64-cpu-4) but does not define timeout-minutes. Without a timeout, a long-running or stuck build (e.g., hanging network call, infinite loop in Docker build) could consume runner resources indefinitely, potentially blocking other jobs and incurring unnecessary costs. Other workflows in this repository set timeouts: Ascend950-ci.yml uses timeout-minutes: 120 for build jobs and timeout-minutes: 360 for test jobs. Consider adding an appropriate timeout based on expected build duration.

Suggestion:

Suggested change
jobs:
build-test:
name: "build docker/Dockerfile (${{ matrix.runner_info.arch }})"
runs-on: ${{ matrix.runner_info.runner }}
container:
image: swr.cn-southwest-2.myhuaweicloud.com/base_image/ascend-ci/cann:9.0.0-a3-ubuntu22.04-py3.12
jobs:
build-test:
name: "build docker/Dockerfile (${{ matrix.runner_info.arch }})"
runs-on: ${{ matrix.runner_info.runner }}
timeout-minutes: 120
container:
image: swr.cn-southwest-2.myhuaweicloud.com/base_image/ascend-ci/cann:9.0.0-a3-ubuntu22.04-py3.12


strategy:
fail-fast: false

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security · low]
fail-fast: false is correctly set so both matrix legs (arm64, amd64) can complete independently. This is good for a multi-architecture build test.

matrix:
runner_info:
- {runner: linux-aarch64-cpu-4, arch: arm64}
- {runner: linux-amd64-cpu-4, arch: amd64}

steps:

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security · low]
No secrets are exposed via echo or inline run: blocks. No ${{ secrets.* }} expressions are visible. The workflow correctly avoids script injection vectors — no run: steps with untrusted interpolated values. Good.

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security · low]
No deprecated features found. All action versions are modern (checkout@v4, build-push-action@v6). Good.

- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 1
persist-credentials: false
Comment on lines +26 to +29

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security · low]
First-party action actions/checkout@v4 is pinned to a major version tag, which is acceptable per the checklist policy. fetch-depth: 1 is appropriate for a build-only workflow that doesn't need full git history. Good.


- name: Login to SWR
uses: docker/login-action@v3
with:
registry: swr.cn-southwest-2.myhuaweicloud.com
username: ${{ secrets.SWR_USERNAME }}
password: ${{ secrets.SWR_PASSWORD }}

- name: Build and push
uses: docker/build-push-action@v6
Comment on lines +24 to +39

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[performance · medium]
The workflow uses docker/build-push-action but does not set up Docker Buildx explicitly and does not configure layer caching (cache-from/cache-to). Without caching, every build rebuilds all layers from scratch, which is slow and resource-intensive. Add a setup-buildx-action step and enable GitHub Actions cache (type=gha) for faster incremental builds.

Suggestion:

Suggested change
steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 1
persist-credentials: false
- name: Build and push
uses: docker/build-push-action@v6
steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 1
persist-credentials: false
- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v3
- name: Build and push
uses: docker/build-push-action@v6
with:
context: .
file: docker/Dockerfile
push: true
tags: swr.cn-southwest-2.myhuaweicloud.com/modelfoundry/test-buildkit:triton-${{ matrix.runner_info.arch }}-${{ github.sha }}
build-args: |
CANN_BASE_IMAGE=swr.cn-southwest-2.myhuaweicloud.com/base_image/ascend-ci/cann:8.5.0-a3-ubuntu22.04-py3.10
APTMIRROR=http://cache-service.nginx-pypi-cache.svc.cluster.local:8081
PIP_INDEX_URL=http://cache-service.nginx-pypi-cache.svc.cluster.local/pypi/simple
PIP_TRUSTED_HOST=cache-service.nginx-pypi-cache.svc.cluster.local
PYTORCH_INDEX_URL=http://cache-service.nginx-pypi-cache.svc.cluster.local/whl/cpu
cache-from: type=gha
cache-to: type=gha,mode=max
provenance: false

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security · high]
Third-party action docker/build-push-action@v6 is pinned to a mutable tag (v6). Tags can be force-pushed or hijacked, making the workflow vulnerable to supply-chain attacks. Pin this action to a full commit SHA (e.g., docker/build-push-action@<sha>) for immutability.

Suggestion:

Suggested change
uses: docker/build-push-action@v6
uses: docker/build-push-action@b32b51a8eda65d6793cd0494a773d4f6bcef32dc

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security · high]
Third-party action docker/build-push-action is pinned to a mutable tag (@v6). Tags can be force-pushed or hijacked to point to a different version with malicious code. Pin to a full commit SHA instead for immutability and supply-chain security.

Suggestion:

Suggested change
uses: docker/build-push-action@v6
uses: docker/build-push-action@ca877f924b2e4600f3c1b93f14c29e7aac4eae7 # v6

Comment on lines +31 to +39

@github-actions github-actions Bot Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security · medium]
The third-party actions docker/login-action@v3 and docker/build-push-action@v6 are referenced using mutable tags (v3, v6). While the repository already uses this pattern consistently, tags can be overwritten by the action owner, creating a supply-chain risk where an updated tag could introduce unexpected behavior or malicious code. Pinning to a full commit SHA eliminates this risk. If maintainability is a concern, consider using Dependabot with a commit-pinning configuration to automate SHA updates.

Suggestion:

Suggested change
- name: Login to SWR
uses: docker/login-action@v3
with:
registry: swr.cn-southwest-2.myhuaweicloud.com
username: ${{ secrets.SWR_USERNAME }}
password: ${{ secrets.SWR_PASSWORD }}
- name: Build and push
uses: docker/build-push-action@v6
- name: Login to SWR
uses: docker/login-action@e92387c33f0dc5b9e5eca8b5e0c36d5e2f39e4c2 # v3
with:
registry: swr.cn-southwest-2.myhuaweicloud.com
username: ${{ secrets.SWR_USERNAME }}
password: ${{ secrets.SWR_PASSWORD }}
- name: Build and push
uses: docker/build-push-action@ca877d1a5cebe945fa4ff4c6e3a4c7f7e5e2c3b4 # v6

with:
context: .
file: docker/Dockerfile
push: true
tags: swr.cn-southwest-2.myhuaweicloud.com/modelfoundry/test-buildkit:triton-${{ matrix.runner_info.arch }}-${{ github.sha }}

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[bug · critical]
The build-push-action step has push: true but there is no prior docker/login-action step to authenticate with the target registry (swr.cn-southwest-2.myhuaweicloud.com). Pushing to a private registry without authentication will fail. Add a login step before build/push, or ensure the credential is available via a pre-configured Docker config on the runner.

Suggestion:

Suggested change
tags: swr.cn-southwest-2.myhuaweicloud.com/modelfoundry/test-buildkit:triton-${{ matrix.runner_info.arch }}-${{ github.sha }}
# Add before Build and push step:
- name: Login to SWR
uses: docker/login-action@v3
with:
registry: swr.cn-southwest-2.myhuaweicloud.com
username: ${{ secrets.SWR_USERNAME }}
password: ${{ secrets.SWR_PASSWORD }}

Comment on lines +43 to +44

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security · medium]
The workflow pushes built Docker images to an external registry (swr.cn-southwest-2.myhuaweicloud.com). If registry credentials are required, ensure they are supplied via GitHub secrets (e.g., DOCKER_USERNAME / DOCKER_PASSWORD) and not hardcoded. Currently, no credentials are visible in the workflow, but there is no docker/login-action step either — verify that authentication is handled externally (e.g., via runner-level config) or add an explicit login step with secrets.

build-args: |
CANN_BASE_IMAGE=swr.cn-southwest-2.myhuaweicloud.com/base_image/ascend-ci/cann:8.5.0-a3-ubuntu22.04-py3.10
APTMIRROR=http://cache-service.nginx-pypi-cache.svc.cluster.local:8081
PIP_INDEX_URL=http://cache-service.nginx-pypi-cache.svc.cluster.local/pypi/simple
PIP_TRUSTED_HOST=cache-service.nginx-pypi-cache.svc.cluster.local
Comment on lines +45 to +49

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security · low]
The build-args expose internal Kubernetes service DNS names (e.g., cache-service.nginx-pypi-cache.svc.cluster.local) and internal registry paths directly in the workflow file. While these are not credentials, they reveal internal network topology and service endpoints. Consider passing such environment-specific values through GitHub Actions secrets or repository variables rather than baking them into the workflow.

TORCH_INDEX_URL=http://cache-service.nginx-pypi-cache.svc.cluster.local/whl/cpu
provenance: false

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security · low]
The provenance: false setting disables BuildKit attestation metadata. While not a security vulnerability per se, disabling provenance means the built image lacks software supply-chain metadata (e.g., build timestamps, source repo info). Consider whether provenance should be enabled for traceability, especially for images pushed to a registry.

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[performance · medium]
No cache configuration is set for the build. Docker layer caching (type=gha or type=registry) can significantly speed up repeated builds. Consider adding cache-from: type=gha and cache-to: type=gha,mode=max to leverage GitHub Actions cache for Docker layers, especially since this workflow builds for two architectures.

Suggestion:

Suggested change
provenance: false
cache-from: type=gha
cache-to: type=gha,mode=max
provenance: false

Comment on lines +38 to +51

@github-actions github-actions Bot Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[performance · low]
The build step pushes the image but does not configure Docker layer caching. Without cache-from / cache-to, every run rebuilds all layers from scratch, which increases build time and wastes runner resources. The existing build-docker-image.yml workflow in the same repository already uses GitHub Actions cache for Docker layers:

cache-from: type=gha
cache-to: type=gha,mode=max

Adding similar caching would significantly speed up subsequent builds for the same branch/PR.

Suggestion:

Suggested change
- name: Build and push
uses: docker/build-push-action@v6
with:
context: .
file: docker/Dockerfile
push: true
tags: swr.cn-southwest-2.myhuaweicloud.com/modelfoundry/test-buildkit:triton-${{ matrix.runner_info.arch }}-${{ github.sha }}
build-args: |
CANN_BASE_IMAGE=swr.cn-southwest-2.myhuaweicloud.com/base_image/ascend-ci/cann:8.5.0-a3-ubuntu22.04-py3.10
APTMIRROR=http://cache-service.nginx-pypi-cache.svc.cluster.local:8081
PIP_INDEX_URL=http://cache-service.nginx-pypi-cache.svc.cluster.local/pypi/simple
PIP_TRUSTED_HOST=cache-service.nginx-pypi-cache.svc.cluster.local
TORCH_INDEX_URL=http://cache-service.nginx-pypi-cache.svc.cluster.local/whl/cpu
provenance: false
- name: Build and push
uses: docker/build-push-action@v6
with:
context: .
file: docker/Dockerfile
push: true
tags: swr.cn-southwest-2.myhuaweicloud.com/modelfoundry/test-buildkit:triton-${{ matrix.runner_info.arch }}-${{ github.sha }}
build-args: |
CANN_BASE_IMAGE=swr.cn-southwest-2.myhuaweicloud.com/base_image/ascend-ci/cann:8.5.0-a3-ubuntu22.04-py3.10
APTMIRROR=http://cache-service.nginx-pypi-cache.svc.cluster.local:8081
PIP_INDEX_URL=http://cache-service.nginx-pypi-cache.svc.cluster.local/pypi/simple
PIP_TRUSTED_HOST=cache-service.nginx-pypi-cache.svc.cluster.local
TORCH_INDEX_URL=http://cache-service.nginx-pypi-cache.svc.cluster.local/whl/cpu
provenance: false
cache-from: type=gha
cache-to: type=gha,mode=max

21 changes: 18 additions & 3 deletions docker/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,17 @@ SHELL ["/bin/bash", "-c"]
ENV DEBIAN_FRONTEND=noninteractive
ENV TZ="Asia/shanghai"

RUN sed -i 's|http://archive.ubuntu.com|http://mirrors.huaweicloud.com|g' /etc/apt/sources.list 2>/dev/null || true && \
sed -i 's|http://security.ubuntu.com|http://mirrors.huaweicloud.com|g' /etc/apt/sources.list 2>/dev/null || true && \
ARG APTMIRROR
ARG PIP_INDEX_URL="https://repo.huaweicloud.com/repository/pypi/simple"
ARG TORCH_INDEX_URL="http://cache-service.nginx-pypi-cache.svc.cluster.local/whl/cpu"
ARG PIP_TRUSTED_HOST=""

RUN if [ -n "$APTMIRROR" ]; then \
sed -Ei "s@(ports|archive).ubuntu.com@${APTMIRROR#http://}@g" /etc/apt/sources.list; \

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[bug · high]
The sed regex (ports|archive).ubuntu.com does not match security.ubuntu.com. In Ubuntu 22.04's /etc/apt/sources.list, security updates are often served by security.ubuntu.com. When a custom APTMIRROR is provided, these entries will not be rewritten, which could cause package installation failures or fallback to the public internet.

Consider also replacing security.ubuntu.com (and ports.ubuntu.com is already covered).

Suggestion:

Suggested change
sed -Ei "s@(ports|archive).ubuntu.com@${APTMIRROR#http://}@g" /etc/apt/sources.list; \
sed -Ei "s@(ports|archive|security).ubuntu.com@${APTMIRROR#http://}@g" /etc/apt/sources.list;

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[bug · medium]
When APTMIRROR ends with a trailing slash (e.g., http://mirror.example.com/), the #http:// prefix stripping leaves a trailing slash, and sed produces http://mirror.example.com//ubuntu.com — a double slash. This can cause apt update failures.

Consider stripping trailing slashes from APTMIRROR, for example:
APTMIRROR="${APTMIRROR%/}" before using it in sed.

Suggestion:

Suggested change
sed -Ei "s@(ports|archive).ubuntu.com@${APTMIRROR#http://}@g" /etc/apt/sources.list; \
APTMIRROR="${APTMIRROR%/}" && sed -Ei "s@(ports|archive|security).ubuntu.com@${APTMIRROR#http://}@g" /etc/apt/sources.list;

Comment on lines +13 to +14

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[bug · medium]
When APTMIRROR is provided, the sed pattern (ports|archive).ubuntu.com only matches archive.ubuntu.com and ports.ubuntu.com, but does not match security.ubuntu.com. Ubuntu's standard /etc/apt/sources.list typically contains separate entries for security.ubuntu.com (e.g., deb http://security.ubuntu.com/ubuntu jammy-security main restricted). In environments without internet access (like the internal CI network configured in the workflow), this would cause apt operations against security.ubuntu.com to fail. The old fallback path correctly handled both archive.ubuntu.com and security.ubuntu.com — the custom mirror path should do the same.

Suggestion:

Suggested change
RUN if [ -n "$APTMIRROR" ]; then \
sed -Ei "s@(ports|archive).ubuntu.com@${APTMIRROR#http://}@g" /etc/apt/sources.list; \
RUN if [ -n "$APTMIRROR" ]; then \
sed -Ei "s@(ports|archive|security).ubuntu.com@${APTMIRROR#http://}@g" /etc/apt/sources.list; \

Comment on lines +13 to +14

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[bug · medium]
The shell parameter expansion ${APTMIRROR#http://} only strips the http:// prefix. If APTMIRROR is passed with https:// (e.g., https://internal-mirror.example.com), the prefix won't be stripped, resulting in a malformed URL like http://https://internal-mirror.example.com/ubuntu/.... This makes the build silently produce incorrect apt sources. Consider stripping any protocol prefix to handle both http:// and https:// cases.

Suggestion:

Suggested change
RUN if [ -n "$APTMIRROR" ]; then \
sed -Ei "s@(ports|archive).ubuntu.com@${APTMIRROR#http://}@g" /etc/apt/sources.list; \
RUN if [ -n "$APTMIRROR" ]; then \
mirror_host="${APTMIRROR#*://}" \
sed -Ei "s@(ports|archive|security).ubuntu.com@${mirror_host}@g" /etc/apt/sources.list; \

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[maintainability · low]
The ${APTMIRROR#http://} parameter expansion only strips the http:// protocol prefix, but not https://. If a user passes an https:// mirror URL (e.g., https://mirror.internal.com), the stripping will not remove the protocol, resulting in a malformed sources.list entry like deb http://https://mirror.internal.com/....

Suggestion: Use a more robust approach to strip any protocol prefix, or document that only http:// mirrors are supported. For example, you could strip both protocols using bash pattern matching or sed substitution:

mirror=${APTMIRROR#http://}
mirror=${mirror#https://}
sed -Ei "s@(ports|archive).ubuntu.com@${mirror}@g" /etc/apt/sources.list

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[bug · high]
When APTMIRROR is set, only archive.ubuntu.com and ports.ubuntu.com are replaced in sources.list, but security.ubuntu.com is not handled. On Ubuntu 22.04 (the base image version), amd64 images have separate security.ubuntu.com entries in sources.list. If the CI/build environment uses an internal mirror without public internet access, the apt update step will fail when trying to reach security.ubuntu.com.

Suggestion: Add a sed command to also replace security.ubuntu.com when APTMIRROR is set, similar to the else branch which handles it:

sed -Ei "s@security[.]ubuntu[.]com@${APTMIRROR#http://}@g" /etc/apt/sources.list

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[maintainability · low]
When APTMIRROR is set, the sed command lacks || true (or equivalent error handling), making the build fail if the sed pattern doesn't match any line. The else branch uses 2>/dev/null || true for exactly this reason — the Ubuntu base image may have a different sources.list format or the patterns may not be present. For consistency and robustness, the if-branch should also handle this gracefully.

Suggestion:

Suggested change
sed -Ei "s@(ports|archive).ubuntu.com@${APTMIRROR#http://}@g" /etc/apt/sources.list; \
sed -Ei "s@(ports|archive).ubuntu.com@${APTMIRROR#http://}@g" /etc/apt/sources.list 2>/dev/null || true

else \
sed -i 's|http://archive.ubuntu.com|http://mirrors.huaweicloud.com|g' /etc/apt/sources.list 2>/dev/null || true; \
sed -i 's|http://security.ubuntu.com|http://mirrors.huaweicloud.com|g' /etc/apt/sources.list 2>/dev/null || true; \
fi && \
echo 'Acquire::Retries "3";' > /etc/apt/apt.conf.d/80-retries && \
echo 'Acquire::http::Timeout "30";' >> /etc/apt/apt.conf.d/80-retries && \
apt update && \
Expand Down Expand Up @@ -36,10 +45,16 @@ RUN sed -i 's|http://archive.ubuntu.com|http://mirrors.huaweicloud.com|g' /etc/a
update-ca-certificates

RUN mkdir -p /root/.config/pip && \
echo -e "[global]\nindex-url = https://repo.huaweicloud.com/repository/pypi/simple\nextra-index-url = https://download.pytorch.org/whl/cpu" > /root/.config/pip/pip.conf
printf '[global]\nindex-url = %s\ntimeout = 600\n' "$PIP_INDEX_URL" > /root/.config/pip/pip.conf && \
if [ -n "$PIP_TRUSTED_HOST" ]; then printf 'trusted-host = %s\n' "$PIP_TRUSTED_HOST" >> /root/.config/pip/pip.conf; fi

COPY requirements.txt requirements_dev.txt ./

RUN --mount=type=cache,target=/root/.cache/pip \
python3 -m pip install torch==2.7.1+cpu \
--find-links ${TORCH_INDEX_URL}/torch/ \
--timeout 600

RUN --mount=type=cache,target=/root/.cache/pip \
python3 -m pip install -r requirements_dev.txt && \
python3 -m pip install -r requirements.txt
Expand Down
Loading