ci: configure Cargo Artifactory sparse registry#64
ci: configure Cargo Artifactory sparse registry#64willkill07 wants to merge 1 commit intoNVIDIA:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe ChangesCargo Registry Configuration Derivation
Sequence Diagram(s)sequenceDiagram
participant CI as GitLab CI job
participant AF as Artifactory
participant FS as Filesystem
participant Cargo as cargo client
CI->>AF: Read NEMO_FLOW_CI_ARTIFACTORY_CARGO_URL
CI->>CI: Pattern-match URL -> compute cargo_registry_index
CI->>FS: Write config.toml & credentials.toml (token auth)
Cargo->>FS: Read config.toml for registry index
Cargo->>AF: Publish crate to computed registry index
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
4dfb594 to
c8a58d6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.gitlab-ci.yml:
- Around line 264-274: The case branch matching "*/api/local/*" is too
permissive and allows empty or multi-segment repo names; update the validation
for cargo_registry_url inside the case handling so only URLs ending with a
single non-empty segment after /api/local/ are accepted (e.g. match the URL
against a stricter pattern like .*/api/local/[^/]+$), and add an explicit
rejection branch for URLs with trailing slashes or extra path segments before
computing artifactory_base, cargo_repo and cargo_registry_index; implement the
check using the same cargo_registry_url variable (via a stricter shell pattern
match or grep -E) and keep the subsequent parameter expansions that set
artifactory_base, cargo_repo and cargo_registry_index unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 0ac18b9e-bf19-4d6a-aaf9-0a375682bb43
📒 Files selected for processing (1)
.gitlab-ci.yml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
{.github/workflows/**/*.{yml,yaml},.gitlab-ci.yml}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Keep CI workflow package name references consistent with local package configurations
Files:
.gitlab-ci.yml
{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}
⚙️ CodeRabbit configuration file
{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}: Review automation changes for reproducibility, pinned versions where appropriate, secret handling, and consistency with the documented validation matrix.
Pay attention to commands that need generated native artifacts, FFI libraries, or platform-specific environment variables.
Files:
.gitlab-ci.yml
🧠 Learnings (2)
📚 Learning: 2026-04-15T18:16:52.951Z
Learnt from: bbednarski9
Repo: NVIDIA/NeMo-Flow PR: 1
File: docs/atof-event-format.md:381-381
Timestamp: 2026-04-15T18:16:52.951Z
Learning: In docs/atof-event-format.md (and when reviewing references to this format across the NeMo-Flow repo), treat `AtifExporter.events_to_steps()` as the intended public/API method name. Do not flag it as inconsistent with internal Rust symbol names (e.g., `event_to_steps` in `crates/core/src/atif.rs`)—the documentation’s public-facing naming may differ intentionally from internal implementation details.
Applied to files:
.gitlab-ci.yml
📚 Learning: 2026-05-01T13:41:07.485Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Flow PR: 34
File: .gitlab-ci.yml:95-103
Timestamp: 2026-05-01T13:41:07.485Z
Learning: In NVIDIA/NeMo-Flow’s .gitlab-ci.yml, when listing GitHub Actions runs for a tag-triggered workflow, use `gh run list --branch "$tag"` to filter runs by the tag name. For annotated tag pushes in this repo, GitHub sets `headBranch` to the tag (e.g., `0.1.0-rc.5`), so `--branch "$tag"` returns only the tag-triggered run. Prefer this over `--commit <sha>`, which can be less precise because it may include both the tag run and a main-branch run that share the same commit SHA.
Applied to files:
.gitlab-ci.yml
cab3197 to
31dbc18
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.gitlab-ci.yml (1)
262-270:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDerivation logic is missing; current sparse index is malformed for the documented CI URL format.
At Line 262, the code only prefixes
sparse+toNEMO_FLOW_CI_ARTIFACTORY_CARGO_URL. For inputs like.../artifactory/api/local/<repo>, Cargo needssparse+.../artifactory/api/cargo/<repo>/index/. Add strict validation plus conversion before writingconfig.toml.Suggested minimal fix
- cargo_registry_index="sparse+${NEMO_FLOW_CI_ARTIFACTORY_CARGO_URL}" + cargo_registry_url="${NEMO_FLOW_CI_ARTIFACTORY_CARGO_URL%/}" + case "$cargo_registry_url" in + */api/local/*/*|*/api/local/|*/api/local) + echo "Error: NEMO_FLOW_CI_ARTIFACTORY_CARGO_URL must end with /api/local/<repo>: ${NEMO_FLOW_CI_ARTIFACTORY_CARGO_URL}" >&2 + exit 1 + ;; + */api/local/*) + artifactory_base="${cargo_registry_url%%/api/local/*}" + cargo_repo="${cargo_registry_url#*/api/local/}" + cargo_registry_index="sparse+${artifactory_base}/api/cargo/${cargo_repo}/index/" + ;; + *) + echo "Error: NEMO_FLOW_CI_ARTIFACTORY_CARGO_URL must match .../artifactory/api/local/<repo>: ${NEMO_FLOW_CI_ARTIFACTORY_CARGO_URL}" >&2 + exit 1 + ;; + esac echo "Using Cargo Artifactory registry index ${cargo_registry_index}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.gitlab-ci.yml around lines 262 - 270, The sparse index value currently set via cargo_registry_index="sparse+${NEMO_FLOW_CI_ARTIFACTORY_CARGO_URL}" is malformed for Artifactory CI URLs—validate and transform NEMO_FLOW_CI_ARTIFACTORY_CARGO_URL before writing cargo_registry_index and config.toml: detect and reject unexpected formats, convert inputs like .../artifactory/api/local/<repo> into the Cargo index form .../artifactory/api/cargo/<repo>/index/ (then prefix with "sparse+"), and only write the resulting cargo_registry_index into the "${cargo_home}/config.toml"; update the echo to print the validated/converted cargo_registry_index and fail the job with a clear error if validation fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In @.gitlab-ci.yml:
- Around line 262-270: The sparse index value currently set via
cargo_registry_index="sparse+${NEMO_FLOW_CI_ARTIFACTORY_CARGO_URL}" is malformed
for Artifactory CI URLs—validate and transform
NEMO_FLOW_CI_ARTIFACTORY_CARGO_URL before writing cargo_registry_index and
config.toml: detect and reject unexpected formats, convert inputs like
.../artifactory/api/local/<repo> into the Cargo index form
.../artifactory/api/cargo/<repo>/index/ (then prefix with "sparse+"), and only
write the resulting cargo_registry_index into the "${cargo_home}/config.toml";
update the echo to print the validated/converted cargo_registry_index and fail
the job with a clear error if validation fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 1fcec25f-553a-4cbe-a77c-0dbee501dd1b
📒 Files selected for processing (1)
.gitlab-ci.yml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
{.github/workflows/**/*.{yml,yaml},.gitlab-ci.yml}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Keep CI workflow package name references consistent with local package configurations
Files:
.gitlab-ci.yml
{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}
⚙️ CodeRabbit configuration file
{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}: Review automation changes for reproducibility, pinned versions where appropriate, secret handling, and consistency with the documented validation matrix.
Pay attention to commands that need generated native artifacts, FFI libraries, or platform-specific environment variables.
Files:
.gitlab-ci.yml
🧠 Learnings (2)
📚 Learning: 2026-04-15T18:16:52.951Z
Learnt from: bbednarski9
Repo: NVIDIA/NeMo-Flow PR: 1
File: docs/atof-event-format.md:381-381
Timestamp: 2026-04-15T18:16:52.951Z
Learning: In docs/atof-event-format.md (and when reviewing references to this format across the NeMo-Flow repo), treat `AtifExporter.events_to_steps()` as the intended public/API method name. Do not flag it as inconsistent with internal Rust symbol names (e.g., `event_to_steps` in `crates/core/src/atif.rs`)—the documentation’s public-facing naming may differ intentionally from internal implementation details.
Applied to files:
.gitlab-ci.yml
📚 Learning: 2026-05-01T13:41:07.485Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Flow PR: 34
File: .gitlab-ci.yml:95-103
Timestamp: 2026-05-01T13:41:07.485Z
Learning: In NVIDIA/NeMo-Flow’s .gitlab-ci.yml, when listing GitHub Actions runs for a tag-triggered workflow, use `gh run list --branch "$tag"` to filter runs by the tag name. For annotated tag pushes in this repo, GitHub sets `headBranch` to the tag (e.g., `0.1.0-rc.5`), so `--branch "$tag"` returns only the tag-triggered run. Prefer this over `--commit <sha>`, which can be less precise because it may include both the tag run and a main-branch run that share the same commit SHA.
Applied to files:
.gitlab-ci.yml
🔇 Additional comments (2)
.gitlab-ci.yml (2)
235-237: Fail-fast input guard is solid.The required-variable check is clear and prevents ambiguous downstream publish failures.
275-279: Credential file permission hardening is good.Writing to
credentials.tomland immediately restricting it to mode600is the right pattern for CI secret handling.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.gitlab-ci.yml (1)
262-269:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate and normalize the Cargo sparse index URL before writing config.
Line 262 blindly prefixes
sparse+and accepts any value, so malformed inputs (wrong path, extra segments, already-prefixed values, missing trailing slash) can produce an invalid index and fail late incargo publish. Add a strict check and normalize to a single trailing slash.Suggested fix
- cargo_registry_index="sparse+${NEMO_FLOW_CI_ARTIFACTORY_CARGO_URL}" + cargo_registry_url="${NEMO_FLOW_CI_ARTIFACTORY_CARGO_URL%/}" + if ! printf '%s\n' "$cargo_registry_url" | grep -Eq '^https://[^/]+/artifactory/api/cargo/[^/]+/index$'; then + echo "Error: NEMO_FLOW_CI_ARTIFACTORY_CARGO_URL must match https://<host>/artifactory/api/cargo/<repo>/index: ${NEMO_FLOW_CI_ARTIFACTORY_CARGO_URL}" >&2 + exit 1 + fi + cargo_registry_index="sparse+${cargo_registry_url}/" echo "Using Cargo Artifactory registry index ${cargo_registry_index}"#!/bin/bash set -eu # Read-only sanity check for URL-shape handling behavior. for url in \ 'https://example.jfrog.io/artifactory/api/cargo/repo/index' \ 'https://example.jfrog.io/artifactory/api/cargo/repo/index/' \ 'sparse+https://example.jfrog.io/artifactory/api/cargo/repo/index/' \ 'https://example.jfrog.io/artifactory/api/local/repo' \ 'https://example.jfrog.io/artifactory/api/cargo/repo/extra/index' do v="${url%/}" if printf '%s\n' "$v" | grep -Eq '^https://[^/]+/artifactory/api/cargo/[^/]+/index$'; then echo "ACCEPT -> sparse+${v}/" else echo "REJECT -> ${url}" fi doneAs per coding guidelines: "Review automation changes for reproducibility, pinned versions where appropriate, secret handling, and consistency with the documented validation matrix."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.gitlab-ci.yml around lines 262 - 269, Validate and normalize the Cargo index URL before writing config by sanitizing the value used for cargo_registry_index: strip any existing "sparse+" prefix if present, verify the remaining URL matches a strict pattern like ^https://[^/]+/artifactory/api/cargo/[^/]+/index$ (reject otherwise), then append a single trailing slash and prepend "sparse+" only once; update the logic that currently sets cargo_registry_index and the subsequent echo/cat block to use this normalized value so you never write malformed indexes into "${cargo_home}/config.toml".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.gitlab-ci.yml:
- Around line 280-282: The cargo publish invocation in the for loop (the lines
calling cargo publish --package "$crate" --allow-dirty) can default to crates-io
if config or CARGO_HOME are misconfigured; update the command to explicitly pass
--registry artifactory (i.e. change cargo publish --package "$crate"
--allow-dirty to cargo publish --package "$crate" --allow-dirty --registry
artifactory) for each package in the loop to guarantee publishing to
artifactory.
---
Duplicate comments:
In @.gitlab-ci.yml:
- Around line 262-269: Validate and normalize the Cargo index URL before writing
config by sanitizing the value used for cargo_registry_index: strip any existing
"sparse+" prefix if present, verify the remaining URL matches a strict pattern
like ^https://[^/]+/artifactory/api/cargo/[^/]+/index$ (reject otherwise), then
append a single trailing slash and prepend "sparse+" only once; update the logic
that currently sets cargo_registry_index and the subsequent echo/cat block to
use this normalized value so you never write malformed indexes into
"${cargo_home}/config.toml".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 032c6815-2d7c-485b-a83e-2a5618de420e
📒 Files selected for processing (1)
.gitlab-ci.yml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
{.github/workflows/**/*.{yml,yaml},.gitlab-ci.yml}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Keep CI workflow package name references consistent with local package configurations
Files:
.gitlab-ci.yml
{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}
⚙️ CodeRabbit configuration file
{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}: Review automation changes for reproducibility, pinned versions where appropriate, secret handling, and consistency with the documented validation matrix.
Pay attention to commands that need generated native artifacts, FFI libraries, or platform-specific environment variables.
Files:
.gitlab-ci.yml
🧠 Learnings (2)
📚 Learning: 2026-04-15T18:16:52.951Z
Learnt from: bbednarski9
Repo: NVIDIA/NeMo-Flow PR: 1
File: docs/atof-event-format.md:381-381
Timestamp: 2026-04-15T18:16:52.951Z
Learning: In docs/atof-event-format.md (and when reviewing references to this format across the NeMo-Flow repo), treat `AtifExporter.events_to_steps()` as the intended public/API method name. Do not flag it as inconsistent with internal Rust symbol names (e.g., `event_to_steps` in `crates/core/src/atif.rs`)—the documentation’s public-facing naming may differ intentionally from internal implementation details.
Applied to files:
.gitlab-ci.yml
📚 Learning: 2026-05-01T13:41:07.485Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Flow PR: 34
File: .gitlab-ci.yml:95-103
Timestamp: 2026-05-01T13:41:07.485Z
Learning: In NVIDIA/NeMo-Flow’s .gitlab-ci.yml, when listing GitHub Actions runs for a tag-triggered workflow, use `gh run list --branch "$tag"` to filter runs by the tag name. For annotated tag pushes in this repo, GitHub sets `headBranch` to the tag (e.g., `0.1.0-rc.5`), so `--branch "$tag"` returns only the tag-triggered run. Prefer this over `--commit <sha>`, which can be less precise because it may include both the tag run and a main-branch run that share the same commit SHA.
Applied to files:
.gitlab-ci.yml
Signed-off-by: Will Killian <wkillian@nvidia.com>
31dbc18 to
3cb2d08
Compare
|
Addressed the latest
Validation rerun: |
Overview
Configure the scheduled GitLab Cargo publishing job to use Artifactory's Cargo sparse registry settings directly through Cargo config and credentials.
Details
artifactoryin${CARGO_HOME}/config.toml.NEMO_FLOW_CI_ARTIFACTORY_CARGO_URLby stripping an optionalsparse+prefix, trimming trailing slashes, validatinghttps://<host>/artifactory/api/cargo/<repo>/index, and writing a singlesparse+.../index/registry URL.Bearer ${NEMO_FLOW_CI_ARTIFACTORY_KEY}in${CARGO_HOME}/credentials.toml.--registry artifactoryso publishing does not depend only on the configured default registry.NEMO_FLOW_CI_ARTIFACTORY_USER.Validation performed locally:
git diff --check -- .gitlab-ci.yml.gitlab-ci.yml.gitlab-ci.ymlWhere should the reviewer start?
Start with
.gitlab-ci.yml, in thepublish:artifactory:cargojob where Cargo config, credentials, URL validation, and publishing are set up.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit