build: Add build secrets and authenticate GitHub clones - #8939
Conversation
Add an optional 'apt_sources' build secret that overlays /etc/apt/sources.list.d/nvidia-artifactory-ubuntu.list for the duration of each apt step, so package installs in Dockerfile and Dockerfile.buildbase resolve through the NVIDIA Artifactory mirror. The source list carries credentials, so it is passed via 'docker build --secret' and mounted per RUN instruction rather than copied in. It never becomes part of an image layer. A secret mount is scoped to a single RUN, so every apt block gets its own mount. The first block in the build base is deliberately left alone: it bootstraps ca-certificates from the distribution repositories, and a base image without CA certificates (ubuntu:24.04 ships none) cannot complete a TLS handshake with Artifactory until that install finishes. When the secret is absent the generated Dockerfiles are byte-identical to before and apt uses the distribution repositories, so the default build path is unchanged. Also gate the existing vllm secrets on the 'req' key instead of on a non-empty secrets dict. Passing only 'apt_sources' previously made the dict truthy and emitted '--secret id=req,src=' with an empty source, failing the build. (cherry picked from commit ac60633)
Add a --secret flag that takes a Docker build secret spec and forwards it to 'docker build --secret' unchanged, so every form Docker accepts works here: 'id=<id>', 'id=<id>,src=<path>' and 'id=<id>,env=<var>'. The flag is repeatable and the spec is never rewritten. The id is parsed by scanning the comma separated fields rather than by position, because Docker does not require 'id' to come first. A secret whose id is 'apt_sources' still drives the apt source list mount in the generated Dockerfile and Dockerfile.buildbase, whichever flag declared it, so --secret id=apt_sources,src=<path> and --build-secret apt_sources <path> produce identical output. --build-secret keeps working unchanged, including the vllm keys, which are not re-emitted through the new path. (cherry picked from commit 6b733c0)
Replace --secret and --build-secret with a single --docker-build-secret that takes a Docker build secret spec and forwards it to 'docker build --secret' unchanged, so every form Docker accepts works: 'id=<id>', 'id=<id>,src=<path>' and 'id=<id>,env=<var>'. The flag is repeatable and order is preserved. The id is parsed by scanning the comma separated fields rather than by position, because Docker does not require 'id' to come first. The id 'apt_sources' keeps its extra meaning and still drives the apt source list mount in the generated Dockerfile and Dockerfile.buildbase. Drop the vllm secret handling. The VLLM_INDEX_URL, PYTORCH_TRITON_URL and NVPL_SLIM_URL secrets were forwarded to 'docker build' but no generated Dockerfile ever mounted them, and BUILD_PUBLIC_VLLM was a build argument carried on a flag named for secrets. A requirements file can still be passed as an ordinary secret with --docker-build-secret id=req,src=<path>. This is a breaking change to the command line: --secret and --build-secret no longer exist. The only caller in the tree is the tritonserver CI template, updated alongside this commit. (cherry picked from commit 87b8f24)
Docker rejects 'target' on 'docker build --secret' because the key belongs to the Dockerfile mount, not the build command. build.py generates the Dockerfiles and runs the build, so it can accept the key on either side: split each spec, forward only what the build command understands, and apply the rest where the Dockerfile is generated. An apt_sources secret now mounts on its 'target' when the spec sets one, so the source list no longer has to land on the NVIDIA Artifactory path. Without a 'target' it keeps that path, so existing invocations are unaffected. Spec parsing moves to a shared helper, since 'id' and 'target' are both looked up by name rather than by position. (cherry picked from commit d9000d8)
cmake_build clones the component and backend repositories from GitHub while running inside the build base container, not while any image is being built, so a docker build secret cannot reach it. Configure a git credential helper in the build base instead and supply the token when the container starts. The helper stores no credential, only a reference to GITHUB_TOKEN, so it is safe both in an image layer and in the build base image that CI pushes to the registry. It also keeps the token out of .git/config, which an authenticated clone URL would not. The token is forwarded as 'docker run -e GITHUB_TOKEN', by name rather than as NAME=value, so the value is taken from the ambient environment and never written into the generated docker_build script, which CI publishes as a build artifact. The helper exits without printing anything when GITHUB_TOKEN is empty, so a build without a token keeps cloning anonymously rather than offering an empty password and failing a clone that previously worked. (cherry picked from commit 72b6570)
cmake clones the component repositories from GitHub while these images are built, so unlike the build base these steps are reachable by a docker build secret. Configure a git credential helper and mount the token on the steps that clone. The helper stores no credential, only a reference to GITHUB_TOKEN, so it is safe in a layer and in the pushed image. It also keeps the token out of .git/config, which an authenticated clone URL would not. Because a credential helper does not cross a build stage, it is configured once per stage that clones: the client build and the Model Analyzer install in Dockerfile.sdk, and the CI base stage in Dockerfile.QA. Every mount is 'required=false' and the helper prints nothing when the token is absent, so a build without the secret clones anonymously exactly as before. (cherry picked from commit 77627e5)
…secret Mount the apt source list on the package installation steps of Dockerfile.sdk and Dockerfile.QA, so they resolve through the configured mirror the same way the generated Dockerfile and Dockerfile.buildbase already do. The list carries credentials, so it is mounted per step rather than copied in, and never becomes part of an image layer. A secret mount lasts for one RUN, so each apt step carries its own. The first step of the SDK client build is deliberately left alone: it installs ca-certificates, and a base image without them cannot complete a TLS handshake with the mirror until that install finishes. Its packages continue to come from the distribution repositories. Every mount is 'required=false', so a build without the secret installs from the distribution repositories exactly as before. (cherry picked from commit 9fd5379)
Describe --docker-build-secret in the Docker build section: the spec forms it accepts, that a secret never reaches an image layer, and that target= is applied when generating the Dockerfile because docker rejects it on the command line. Cover the apt_sources id, including why the first apt step of the build base is excluded from the mount, and GITHUB_TOKEN, including why it is an environment variable rather than a build secret. (cherry picked from commit e233b05)
The first apt step of the build base was excluded from the mount because it installs ca-certificates, and a base image without them cannot complete a TLS handshake with the mirror. That left the largest step of the build base going to the distribution repositories: in a pipeline run it fetched 45 packages from archive.ubuntu.com while the mounted step fetched 47 from the mirror. The exclusion is not needed. On a base image that already carries CA certificates the step resolves through the mirror like any other. On one that does not, apt reports the failed handshake, still exits 0, and installs from the distribution repositories, so the step behaves as it did before. Verified on a bare ubuntu image with no CA certificates: apt-get update exits 0 with certificate errors reported, ca-certificates installs from the distribution repositories, and the next step then resolves through the mirror. (cherry picked from commit 0ae8837)
a939910 to
133660f
Compare
Greptile SummaryThis PR adds Docker build-secret forwarding and authenticated GitHub clone support while preserving unauthenticated behavior when credentials are absent.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains. No blocking failure remains. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
CLI["build.py options and environment"] --> SPEC["Parse Docker secret specs"]
SPEC --> BUILD["docker build --secret"]
BUILD --> APT["Temporary apt_sources mount"]
BUILD --> GITBUILD["Temporary gitconfig mount"]
CLI --> ENV["Forward TRITON_GITCONFIG by name"]
ENV --> CONTAINER["Build container"]
CONTAINER --> FILE["Write temporary Git config"]
FILE --> CMAKE["Run cmake_build with GIT_CONFIG_GLOBAL"]
Reviews (6): Last reviewed commit: "docs: Describe how the git config reache..." | Re-trigger Greptile |
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
Review of the current head (133660f7). Posting only the blocking / severe items; several lower-severity ones (dead declared_secret_ids(), silent no-op on a typo'd secret id, target= honored only for apt_sources) are left out.
The headline concern is the first comment on build.py:1245: the credential-helper design cannot fix the GitHub rate-limiting problem the PR is built to solve, because git never consults a helper for a public-repo clone. That one determines whether the PR achieves its stated goal, so it's worth resolving before the rest.
Two more items that don't sit on a changed line:
-
create_dockerfile_buildbase_rhelgets neither the credential helper nor the apt-sources mount.mount_apt_sources_secret()is called only fromcreate_dockerfile_buildbaseandcreate_dockerfile_linux, and the helperRUNis added only in the former — yetcreate_docker_build_scriptstill forwards-e GITHUB_TOKENfor every platform.--target-platform rheltherefore clones anonymously with no warning. The same gap applies to--no-container-build, wherecmake_buildruns on the host with no helper configured, while the docs say only "SetGITHUB_TOKENin the environment". -
Two claims in the PR description don't hold as written. Generated Dockerfiles are not byte-identical when no secret is supplied — the credential-helper
RUNatbuild.py:1245is emitted unconditionally. And "every apt step" is not covered (see thebuild.py:986andDockerfile.sdk:183comments).
Separately, on the --build-secret → --docker-build-secret rename already raised by the bot and answered with "addressed in complement MR": the same hunk also drops --secret id=req/VLLM_INDEX_URL/PYTORCH_TRITON_URL/NVPL_SLIM_URL and --build-arg BUILD_PUBLIC_VLLM from the final image build. Nothing in-tree consumes those, but out-of-tree vLLM build flows will break — worth confirming the companion MR covers that removal too, and calling both out in the commit message / release notes since neither appears in the PR description.
(Fixes are given as plain code blocks rather than suggestion blocks — most span multiple lines or belong on lines this PR doesn't touch.)
| # which leaves git to fall back to unauthenticated access rather than | ||
| # offering an empty password and failing a clone that used to work. | ||
| df += """ | ||
| RUN git config --global credential."https://github.com".helper \\ |
There was a problem hiding this comment.
Blocking — this mechanism cannot fix the rate-limiting problem the PR targets.
Git only invokes a credential helper after the server answers 401. Cloning a public repo over HTTPS returns 200 with no auth challenge, so the helper is never consulted and GITHUB_TOKEN is never sent. Every default component repo (common, core, backend, thirdparty, and the backends) is public, so a build on a shared CI egress IP still clones anonymously and still hits GitHub's unauthenticated limits. Only private-repo clones — which do get a 401 — actually benefit.
To authenticate public clones you have to force the credential onto the request rather than wait to be asked:
RUN git config --global url."https://x-access-token:${GITHUB_TOKEN}@github.com/".insteadOf "https://github.com/"
(or an http.extraHeader carrying the bearer token). Worth verifying with a helper that logs its own invocation before merging — git ls-remote against any public repo will show the helper is never called.
| if not mount: | ||
| return df | ||
|
|
||
| marker = "RUN apt-get update" |
There was a problem hiding this comment.
Blocking — the DCGM step never receives the mount, so a GPU build fails on a mirror-only network.
This matches only the literal string RUN apt-get update. install_dcgm_libraries() emits apt install / apt update -qq, never RUN apt-get update, so it is silently skipped. In the generated build/Dockerfile.buildbase and build/Dockerfile the DCGM step runs apt update -qq && apt install ... datacenter-gpu-manager-4-core with no --mount=type=secret. On the exact network this flag exists for — only the authenticated mirror reachable — --enable-gpu dies there.
The flag help and docs/customization_guide/build.md both claim every apt step is covered; today it's only the ones spelled apt-get update.
Suggest matching both spellings, e.g. a regex over RUN apt(-get)? update, and asserting the substitution count is non-zero so a future rename fails loudly instead of silently.
| # in a layer and in the pushed image. Each step that clones supplies the value | ||
| # with --mount=type=secret,id=github_token. The helper prints nothing when the | ||
| # token is absent, leaving git to clone anonymously as before. | ||
| RUN git config --global credential."https://github.com".helper \ |
There was a problem hiding this comment.
Severe — protocol mismatch means this helper is never offered.
The helper is registered under credential."https://github.com", but TRITON_REPO_ORGANIZATION defaults to http://github.com/triton-inference-server (line 35; Dockerfile.QA:30 is the same). Git matches credential sections on protocol as well as host — git credential fill with protocol=https,host=github.com fires the helper, protocol=http,host=github.com does not.
So with the default ARG, the cmake clones and pip3 install "git+http://github.com/.../model_analyzer@..." (line 280) request protocol=http and never match this section. The token is only reachable via GitHub's http→https redirect plus a 401.
Either bump the ARG defaults to https://, or register the helper for both protocols.
| ARG TRITON_ENABLE_GPU | ||
|
|
||
| RUN apt-get update && \ | ||
| RUN --mount=type=secret,id=apt_sources,target=/etc/apt/sources.list.d/nvidia-artifactory-ubuntu.list,required=false \ |
There was a problem hiding this comment.
Blocking for the SDK image — this is the only apt step that got the mount.
The sdk_build stage's apt step (line 64) and the DCGM install in this runtime stage (line 256) have no apt_sources mount. On a mirror-only network docker build -f Dockerfile.sdk fails on the first stage, before ever reaching this line.
Same fix as build.py's mount_apt_sources_secret: add --mount=type=secret,id=apt_sources,target=/etc/apt/sources.list.d/nvidia-artifactory-ubuntu.list,required=false to lines 64 and 256.
| RUN pip3 install "git+${TRITON_MODEL_ANALYZER_REPO}" | ||
| # Separate stage from the client build, so the credential helper is configured | ||
| # again here. It stores no credential, only a reference to GITHUB_TOKEN. | ||
| RUN git config --global credential."https://github.com".helper \ |
There was a problem hiding this comment.
Severe — this credential helper ships in the released SDK image.
This is a standalone RUN in the final (published) stage, so /root/.gitconfig persists into the shipped image carrying a helper that unconditionally forwards whatever GITHUB_TOKEN is in the container's environment to github.com. A user running the SDK container in a CI job that exports GITHUB_TOKEN for an unrelated purpose will have it sent on any github.com clone.
Unlike the build-base copy (build.py:1245), which genuinely must persist for cmake_build, this one only needs to exist for the very next step. Folding it into the same RUN as the pip3 install on line 280 keeps it out of the shipped layer entirely.
| # container. Passed by name rather than as NAME=value: 'docker run -e | ||
| # VAR' takes the value from the ambient environment, so the token stays | ||
| # out of this script, which CI publishes as a build artifact. | ||
| if os.environ.get("GITHUB_TOKEN"): |
There was a problem hiding this comment.
Severe — this bakes the generation-time environment into a script that runs later.
The comment above correctly explains that docker run -e VAR (name only) keeps the token out of the published artifact. But this if defeats that goal: whether the -e GITHUB_TOKEN line is emitted at all is decided when build.py runs, not when build/docker_build runs.
The PR body says this script is published as a CI artifact and executed in a later job. In any pipeline where the token is injected only into the executing job, the line is absent, the container never sees the token, and clones silently run unauthenticated (or fail outright on a private repo).
docker run -e VAR with VAR unset is a no-op, so the guard buys nothing:
runargs += ["-e", "GITHUB_TOKEN"]Replace the credential helper with a git config supplied as the 'gitconfig' build secret. It carries a url.insteadOf rewrite, which is the form the rest of the build tooling already uses, and it removes the shell function that had to be embedded in three Dockerfiles. The token has to be written into the config: git does not expand environment variables inside url.insteadOf, so a config that refers to one is stored literally and never authenticates. Point git at the config with GIT_CONFIG_GLOBAL rather than installing it at the default path, so only the steps that mount it are affected. Steps that clone while an image is built receive it as a build secret. cmake_build clones while the build runs inside the container, where a build secret cannot reach it, so the same file is bind mounted read only there instead. The config is never part of an image layer, and only its path reaches the generated build scripts, which CI publishes as an artifact. Export GIT_CONFIG_GLOBAL rather than using an assignment prefix. A prefix is a syntax error ahead of an 'if' keyword and does not survive '&&', so on four of the six steps it would either fail to parse or never reach the command that clones.
The build container was given the git config as a read only bind mount, which does not work: docker resolves the source path on the daemon rather than in the job, and creates a directory when it does not find one. git then refused to read its configuration at all and every clone failed. warning: unable to access '/run/secrets/gitconfig': Is a directory fatal: unknown error occurred while reading the configuration files Read the configuration from the environment instead, which crosses the daemon boundary because it needs no filesystem. The variables are forwarded by name rather than as NAME=value, so the one carrying the credential stays out of the generated build scripts, which CI publishes as an artifact. The image builds are unaffected and keep the git config secret: 'docker build --secret' reads the source file on the client, so it never crossed that boundary.
Restores the read only bind mount and GIT_CONFIG_GLOBAL for the container that runs the build, in place of reading the configuration from the environment. The mount failed before only because the config was written under /tmp, which the docker daemon does not see when the runner has a private one, leaving it to create a directory in place of the file. Writing the config somewhere the daemon shares fixes that, and keeps one mechanism across every step that clones.
Binding the git config into the build container does not work on this runner, whichever directory the file is written to. Docker resolves the source path on the daemon, which shares neither /tmp nor the project directory with the job, so it finds no file and mounts a new directory in its place. git then refuses to read any configuration and every clone fails. warning: unable to access '/run/secrets/gitconfig': Is a directory fatal: unknown error occurred while reading the configuration files Read the configuration from the environment instead, which needs no filesystem and so crosses that boundary. The variables are forwarded by name rather than as NAME=value, keeping the one that carries the credential out of the generated build scripts, which CI publishes as an artifact. The image builds keep the git config secret. 'docker build --secret' reads the source file on the client, so it never crossed the boundary: the same job that failed here built its images from that file without trouble.
The container build.py runs the build in cannot be given a file. Docker resolves a bind mount source on the daemon, which shares neither /tmp nor the project directory with the job, so it mounts a directory in place of the config and git refuses to read any configuration. Hand the container the contents instead and let it write the file there, then point git at it for that command only. The variable is forwarded by name rather than as NAME=value, so the contents, which carry a credential, stay out of the generated build scripts, which CI publishes as an artifact. Without the variable the container command is unchanged and clones stay unauthenticated.
The GitHub authentication section still described variables that no longer exist. Replace them with TRITON_GITCONFIG, and say plainly that the two mechanisms cover different parts of the build: the secret reaches the steps that clone while an image is built, the variable reaches the build itself, and a build wanting authenticated clones throughout sets both.
|
|
||
| def cmd(self, clist, check_exitcode=False): | ||
| if isinstance(clist, str): | ||
| self._file.write(f"{clist}\n") |
There was a problem hiding this comment.
is this something to actually be concerned about?
whoisj
left a comment
There was a problem hiding this comment.
LGTM. I only had the one question (above).
What does the PR do?
Adds a build secret mechanism to
build.pyand uses it to authenticate theGitHub clones the build performs and to route package installs through a
package mirror.
--docker-build-secrettakes a Docker build secret spec and forwards it todocker build --secretunchanged, so every form Docker accepts works:id=<id>,id=<id>,src=<path>andid=<id>,env=<var>. A spec may also carrytarget=<path>; Docker rejects that key on the command line because it belongsto the Dockerfile, so
build.pystrips it from the build command and applies itwhen generating the Dockerfile.
Two ids carry meaning.
apt_sourcesis mounted over the apt source list for theduration of every apt step in the generated
DockerfileandDockerfile.buildbase, and on the corresponding steps ofDockerfile.sdkandDockerfile.QA.GITHUB_TOKENis an environment variable rather than a buildsecret: the component and backend repos are cloned by
cmake_buildwhile thebuild runs inside the container, not while an image is built, so a build secret
cannot reach them.
build.pyconfigures a git credential helper in the buildbase and forwards the token to the build container instead. The helper holds no
credential of its own, only a reference, so it is safe in a layer and in the
pushed build base image, and it keeps the token out of
.git/config.Every mount is
required=falseand the helper prints nothing when the token isabsent, so a build without either secret behaves exactly as it does today: the
generated Dockerfiles are byte-identical and clones stay unauthenticated.
Checklist
<commit_type>: <Title>Commit Type:
Check the conventional commit type
box here and add the label to the github PR.
Related PRs:
A follow-up PR stacked on this branch bumps the fastapi and starlette pins in
the openai frontend; it is listed here once opened.
Where should the reviewer start?
build.py—secret_spec_id,parse_secret_spec,secret_build_args,apt_sources_secret_mountandmount_apt_sources_secret, then the twodocker buildargument lists and thedocker runinvocation that forwardsGITHUB_TOKEN.Test plan:
Verified with
--dryrun, by inspecting the generated build scripts, and withreal BuildKit builds.
./build.py --enable-all --dryrunwith no secret: generated Dockerfiles arebyte-identical to a build without this feature, and no
--secretreachesdocker build../build.py --enable-all --dryrun --docker-build-secret id=apt_sources,src=<file>:both
docker buildinvocations carry--secret, and every apt step ofDockerfileandDockerfile.buildbasecarries the matching mount.target=is absent from the build command and present on the Dockerfile mount.Real BuildKit build with an apt source list mounted: packages resolve through
the mirror, and the secret is absent from the image, from
docker history,from
docker inspectand from the exported filesystem.Real BuildKit build with
GITHUB_TOKEN: an authenticated clone succeeds, thetoken does not appear in
.git/config, and an unauthenticated build stillclones a public repo successfully.
CI Pipeline ID:
Caveats:
The apt source list carries credentials, which is why it is passed as a secret
and mounted per step rather than copied in. On a base image without CA
certificates the first apt step of
Dockerfile.buildbasereports a failed TLShandshake, still exits 0, and installs from the distribution repositories, so
that step behaves as it did before the mount was added.
Background
The build clones several repositories from GitHub and installs a large number of
packages. Both were unauthenticated, which is subject to rate limiting and
cannot reach a private repository or an authenticated mirror.
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)