podman: T7736: update netavark and aardvark-dns packages - #1259
Conversation
Package update required in lockstep mode to add support for renaming podman macvlan interfaces.
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Podman packaging configuration now builds ChangesPodman Rust package integration
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@scripts/package-build/podman/package.toml`:
- Around line 78-85: Update the systemd unit installation in the package build
to use mode 0644 instead of 0664, and explicitly set mode 0644 on both service
files generated by the sed commands. Apply the changes to the socket
installation and the generated netavark-dhcp-proxy.service and
netavark-firewalld-reload.service artifacts.
- Around line 95-102: The Rust setup blocks at
scripts/package-build/podman/package.toml lines 95-102 and 162-169 must be
replaced with one shared helper. Have the helper download and checksum- or
signature-verify rustup-init, install an explicit pinned Rust toolchain version
instead of stable, and prepend $HOME/.cargo/bin to PATH; remove the
/usr/local/bin symlink creation so both package hooks use the same verified
toolchain and command resolution.
🪄 Autofix
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: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 1e6c0596-ebaf-423b-b390-26aeaac94104
📒 Files selected for processing (2)
scripts/package-build/podman/.gitignorescripts/package-build/podman/package.toml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
ansible/ansible(manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: codeql-analysis-call / Analyze (python)
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
🧰 Additional context used
🔍 Remote MCP Context7, vyos.dev
Relevant review context
- T7736 is titled “Container: virtual-ethernet exception when attempting to modify container network created veth”, remains Open, and has Normal priority. Its description concerns rejecting configuration of veth interfaces owned by—or peered with—a Podman network, not macvlan interface renaming.
- A task comment links the related
vyos-1xPR#5352. Another comment notes that Podman 5.4 introducedhost_interface_namefor bridge networks, which may provide an alternative solution for deterministic host-side interface names. - Netavark’s setup API distinguishes
network_options.interface_name(the interface name to create) fromnetwork.network_interface(the network interface/parent configuration). This distinction should be verified for macvlan behavior. - Podman documentation confirms macvlan networks use a host parent device via
parent=<device>/--interface-name;interface_namecan adjust the interface name inside macvlan/ipvlan containers.
🔇 Additional comments (3)
scripts/package-build/podman/package.toml (2)
105-160: LGTM!
19-28: 📐 Maintainability & Code QualityKeep the patch removal as written.
All seven names exist in
debian/1.14.0-2. The remaining patches do not modifyCargo.tomlorCargo.lock.> Likely an incorrect or invalid review comment.scripts/package-build/podman/.gitignore (1)
2-3: LGTM!
| install -D -m 664 contrib/systemd/system/netavark-dhcp-proxy.socket \ | ||
| debian/netavark/usr/lib/systemd/system/netavark-dhcp-proxy.socket | ||
| sed -e 's;@@NETAVARK@@;/usr/lib/podman/netavark;g' \ | ||
| < contrib/systemd/system/netavark-dhcp-proxy.service.in \ | ||
| > debian/netavark/usr/lib/systemd/system/netavark-dhcp-proxy.service | ||
| sed -e 's;@@NETAVARK@@;/usr/lib/podman/netavark;g' \ | ||
| < contrib/systemd/system/netavark-firewalld-reload.service.in \ | ||
| > debian/netavark/usr/lib/systemd/system/netavark-firewalld-reload.service |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use 0644 for the systemd unit, and set explicit modes on the generated units.
Line 78 installs the socket unit group-writable (0664) into /usr/lib/systemd/system. Debian policy expects 0644 for unit files. The two sed-generated .service files inherit the build umask instead of an explicit mode.
🔧 Proposed fix
- install -D -m 664 contrib/systemd/system/netavark-dhcp-proxy.socket \
+ install -D -m 0644 contrib/systemd/system/netavark-dhcp-proxy.socket \
debian/netavark/usr/lib/systemd/system/netavark-dhcp-proxy.socket
+ install -d -m 0755 debian/netavark/usr/lib/systemd/system
sed -e 's;@@NETAVARK@@;/usr/lib/podman/netavark;g' \
< contrib/systemd/system/netavark-dhcp-proxy.service.in \
> debian/netavark/usr/lib/systemd/system/netavark-dhcp-proxy.service
+ chmod 0644 debian/netavark/usr/lib/systemd/system/netavark-dhcp-proxy.service
sed -e 's;@@NETAVARK@@;/usr/lib/podman/netavark;g' \
< contrib/systemd/system/netavark-firewalld-reload.service.in \
> debian/netavark/usr/lib/systemd/system/netavark-firewalld-reload.service
+ chmod 0644 debian/netavark/usr/lib/systemd/system/netavark-firewalld-reload.service📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| install -D -m 664 contrib/systemd/system/netavark-dhcp-proxy.socket \ | |
| debian/netavark/usr/lib/systemd/system/netavark-dhcp-proxy.socket | |
| sed -e 's;@@NETAVARK@@;/usr/lib/podman/netavark;g' \ | |
| < contrib/systemd/system/netavark-dhcp-proxy.service.in \ | |
| > debian/netavark/usr/lib/systemd/system/netavark-dhcp-proxy.service | |
| sed -e 's;@@NETAVARK@@;/usr/lib/podman/netavark;g' \ | |
| < contrib/systemd/system/netavark-firewalld-reload.service.in \ | |
| > debian/netavark/usr/lib/systemd/system/netavark-firewalld-reload.service | |
| install -D -m 0644 contrib/systemd/system/netavark-dhcp-proxy.socket \ | |
| debian/netavark/usr/lib/systemd/system/netavark-dhcp-proxy.socket | |
| install -d -m 0755 debian/netavark/usr/lib/systemd/system | |
| sed -e 's;@@NETAVARK@@;/usr/lib/podman/netavark;g' \ | |
| < contrib/systemd/system/netavark-dhcp-proxy.service.in \ | |
| > debian/netavark/usr/lib/systemd/system/netavark-dhcp-proxy.service | |
| chmod 0644 debian/netavark/usr/lib/systemd/system/netavark-dhcp-proxy.service | |
| sed -e 's;@@NETAVARK@@;/usr/lib/podman/netavark;g' \ | |
| < contrib/systemd/system/netavark-firewalld-reload.service.in \ | |
| > debian/netavark/usr/lib/systemd/system/netavark-firewalld-reload.service | |
| chmod 0644 debian/netavark/usr/lib/systemd/system/netavark-firewalld-reload.service |
🤖 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 `@scripts/package-build/podman/package.toml` around lines 78 - 85, Update the
systemd unit installation in the package build to use mode 0644 instead of 0664,
and explicitly set mode 0644 on both service files generated by the sed
commands. Apply the changes to the socket installation and the generated
netavark-dhcp-proxy.service and netavark-firewalld-reload.service artifacts.
| if [ ! -x "$HOME/.cargo/bin/rustup" ]; then | ||
| curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile minimal --default-toolchain stable | ||
| else | ||
| "$HOME/.cargo/bin/rustup" update stable | ||
| fi | ||
| sudo ln -sf "$HOME/.cargo/bin/cargo" /usr/local/bin/cargo | ||
| sudo ln -sf "$HOME/.cargo/bin/rustc" /usr/local/bin/rustc | ||
| sudo ln -sf "$HOME/.cargo/bin/rustup" /usr/local/bin/rustup |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- package hooks ---'
sed -n '80,110p;148,178p' scripts/package-build/podman/package.toml
printf '%s\n' '--- relevant build commands and Rust references ---'
rg -n -C 3 'cargo|rustc|rustup|netavark|aardvark-dns|build_cmd|pre_build_hook' scripts/package-build/podman/package.toml scripts/package-build/build.py
printf '%s\n' '--- exact duplication check ---'
python3 - <<'PY'
from pathlib import Path
p = Path("scripts/package-build/podman/package.toml")
s = p.read_text()
block = '''if [ ! -x "$HOME/.cargo/bin/rustup" ]; then
curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile minimal --default-toolchain stable
else
"$HOME/.cargo/bin/rustup" update stable
fi
sudo ln -sf "$HOME/.cargo/bin/cargo" /usr/local/bin/cargo
sudo ln -sf "$HOME/.cargo/bin/rustc" /usr/local/bin/rustc
sudo ln -sf "$HOME/.cargo/bin/rustup" /usr/local/bin/rustup'''
print("occurrences:", s.count(block))
PYRepository: vyos/vyos-build
Length of output: 20303
Unverified Software Download In Build Pipeline (CWE-494): Download of Code Without Integrity Check
Reachability: Internal · Exploitability: Difficult
Use one pinned, verified Rust toolchain setup for both package hooks. The curl ... | sh pipeline has no checksum or signature validation, and stable changes over time. The compiler builds the shipped netavark and aardvark-dns binaries. The /usr/local/bin symlinks also change command resolution for later builds. Replace both blocks with one shared helper that verifies rustup-init, installs an explicit toolchain version, and prepends $HOME/.cargo/bin to PATH.
📍 Affects 1 file
scripts/package-build/podman/package.toml#L95-L102(this comment)scripts/package-build/podman/package.toml#L162-L169
🤖 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 `@scripts/package-build/podman/package.toml` around lines 95 - 102, The Rust
setup blocks at scripts/package-build/podman/package.toml lines 95-102 and
162-169 must be replaced with one shared helper. Have the helper download and
checksum- or signature-verify rustup-init, install an explicit pinned Rust
toolchain version instead of stable, and prepend $HOME/.cargo/bin to PATH;
remove the /usr/local/bin symlink creation so both package hooks use the same
verified toolchain and command resolution.
jestabro
left a comment
There was a problem hiding this comment.
Required for fix to podman auto-naming collision with VyOS veth names; built and tested in that context.
|
Tick the box to add this pull request to the merge queue (same as
|
sever-sever
left a comment
There was a problem hiding this comment.
Binaries build correctly
vyos_bld@87df2d4231e8:/vyos2/tmp/vyos-build/scripts/package-build/podman$ ls -lh *.deb
-rw-r--r-- 1 vyos_bld vyos_bld 677K Aug 5 06:57 aardvark-dns_1.14.0-3_arm64.deb
-rw-r--r-- 1 vyos_bld vyos_bld 125K Aug 5 06:57 aardvark-dns-dbgsym_1.14.0-3_arm64.deb
-rw-r--r-- 1 vyos_bld vyos_bld 3.4M Aug 5 06:56 netavark_1.14.0-2_arm64.deb
-rw-r--r-- 1 vyos_bld vyos_bld 703K Aug 5 06:56 netavark-dbgsym_1.14.0-2_arm64.deb
-rw-r--r-- 1 vyos_bld vyos_bld 55M Aug 5 07:01 podman_5.8.4_arm64.deb
Change summary
Package update required in lockstep mode to add support for renaming podman macvlan interfaces.
Types of changes
Related Task(s)
Related PR(s)
Checklist: