Skip to content

engineering: Port osmodifier from Go binary to native Rust crate#638

Open
bfjelds wants to merge 60 commits into
mainfrom
user/bfjelds/mjolnir/port-osmodifier-to-rust
Open

engineering: Port osmodifier from Go binary to native Rust crate#638
bfjelds wants to merge 60 commits into
mainfrom
user/bfjelds/mjolnir/port-osmodifier-to-rust

Conversation

@bfjelds
Copy link
Copy Markdown
Member

@bfjelds bfjelds commented May 11, 2026

Port the Go osmodifier from azure-linux-image-tools to a native Rust library crate (crates/osmodifier). Trident now calls osmodifier functions directly instead of serializing config to YAML, writing a temp file, and exec'ing the Go binary.

See crates/osmodifier/README.md for full documentation including file mapping, function mapping, intentional divergences, and sync guide.

Architecture

Three public entry points replace the single Go binary:

Rust function Replaces
modify_os(&ctx, &config) OS config: users, hostname, services, modules, SELinux config file, extra kernel cmdline
modify_boot(&ctx, &boot_config) Boot config: SELinux grub args, overlays, verity, root device
update_default_grub(&ctx) osmodifier --update-grub: sync grub.cfg values back to /etc/default/grub

Key design decisions:

  1. Library, not binary — eliminates YAML serialization, temp files, process spawning. Errors propagate as Result.
  2. No chroot codepath — trident already chroots into newroot before calling osmodifier, so root is always /.
  3. No BootCustomizer / bootConfigType dispatch — Go dispatches across grub-mkconfig, grub-legacy, and UKI boot types. Rust only implements grub-mkconfig because trident exclusively targets Azure Linux. UKI images return early in boot/mod.rs before modify_boot() is called.
  4. Simplified grub.cfg parsing — string-based keyword matching instead of Go's full grub tokenizer. Sufficient for read-only extraction from grub2-mkconfig output.
  5. Split APImodify_os() vs modify_boot() for pipeline stage separation.

What changed

New crate: crates/osmodifier

Rust library implementing all Go osmodifier functionality:

  • Usersuseradd, password hashing (chpasswd -e via stdin, not cmdline), SSH authorized_keys (preserving existing keys for existing users), groups, startup command, password expiry (shadow field 7 with absolute date matching Go's Chage())
  • Hostname/etc/hostname write (no trailing newline, matching Go)
  • Servicessystemctl enable/disable with Go-faithful error handling (distinguishes "disabled" from "not found")
  • Kernel modulesmodules-load.d, modprobe.d with option merging, bare-flag preservation, duplicate-line consolidation
  • SELinux/etc/selinux/config update, kernel cmdline args (security=selinux, selinux=, enforcing=), first-match-only replacement
  • GRUB default/etc/default/grub parsing/writing, targets GRUB_CMDLINE_LINUX_DEFAULT, append-only cmdline args
  • GRUB cfggrub.cfg boot-arg extraction with string-based parsing (no regex), case-sensitive recovery detection scoped to quoted title token

Security

  • Password hashes passed via stdin to chpasswd -e (not process cmdline)
  • Startup command validation rejects :, \n, and \r to prevent /etc/passwd corruption
  • Hashed password values validated against : and newlines before writing to /etc/shadow
  • Atomic file writes for /etc/shadow and /etc/passwd with uid/gid ownership preservation via fchown
  • PasswordType::Locked writes ! marker for existing users

Integration

  • All system tool calls use osutils::Dependency enum (systemctl, grub2-mkconfig, id, useradd, usermod, chown, openssl, chpasswd)
  • Removed osutils::osmodifier re-export shim to break cyclic dependency

Build and packaging

  • Removed: download-osmodifier.yml template, Makefile osmodifier targets, Dockerfile osmodifier stages, functional test binary upload steps
  • RPM spec: added Requires: openssl, shadow-utils; added Suggests: grub2-tools
  • Removed OS_MODIFIER_BINARY_PATH and OS_MODIFIER_NEWROOT_PATH constants

Tests

  • Unit tests for all modules (grub_cfg, default_grub, services, modules, selinux, hostname, users, config)
  • Functional tests running in test VM: hostname, modules, selinux, services, and integration scenarios

Documentation

  • crates/osmodifier/README.md: Go-to-Rust file mapping, function-by-function mapping (17 functions), intentional divergences, not-ported table with rationale, sync guide with git commands and change-type-to-file mapping

Fidelity

Two rounds of systematic Go-vs-Rust comparison, plus a 9-agent adversarial deep review (3 roles × 3 model families). All behavioral divergences found were fixed:

  • grub_cfg.rs — string-based parsing (no regex), case-sensitive recovery detection scoped to quoted title token only, error on multiple non-recovery linux lines
  • services.rs — distinguish "disabled" (exit 1 + stdout "disabled") from "error" (service not found), matching Go's systemd.IsServiceEnabled
  • modules.rs — error on Inherit+options for disabled modules, preserve existing options during updates, preserve bare options without =, merge duplicate options <module> lines
  • selinux.rs — first-match-only replacement, security=selinux kernel arg for Enforcing/Permissive, Enforcing matches Go's EMU path (no enforcing=1)
  • default_grub.rs — target GRUB_CMDLINE_LINUX_DEFAULT (not GRUB_CMDLINE_LINUX), append-only (no key dedup)
  • users.rs — password expiry writes shadow field 7 (account expiration) with absolute date matching Go's Chage(), preserve existing authorized_keys for existing users, error on unparseable shadow lastChange

Source mapping

Ported from azure-linux-image-tools (toolkit/tools/):

Rust file Go source(s) Go commit
lib.rs osmodifierlib/osmodifier.go, modifierutils.go f4de1a0
config.rs osmodifierapi/os.go, overlay.go, verity.go, identifiedpartition.go 8bd4ef3
users.rs imagecustomizerlib/customizeusers.go 8bd4ef3
hostname.rs imagecustomizerlib/customizehostname.go 8bd4ef3
modules.rs imagecustomizerlib/kernelmoduleutils.go 8bd4ef3
services.rs imagecustomizerlib/customizeservices.go dc90945
selinux.rs modifierutils.go (SELinux functions) f4de1a0
default_grub.rs modifydefaultgrub.go f4de1a0
grub_cfg.rs modifydefaultgrub.go, modifierutils.go f4de1a0

@bfjelds
Copy link
Copy Markdown
Member Author

bfjelds commented May 11, 2026

/azp run [GITHUB]-trident-pr-e2e

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bfjelds bfjelds changed the title Port osmodifier from Go binary to native Rust crate WIP: Port osmodifier from Go binary to native Rust crate May 11, 2026
@bfjelds
Copy link
Copy Markdown
Member Author

bfjelds commented May 11, 2026

/azp run [GITHUB]-trident-pr-e2e

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bfjelds
Copy link
Copy Markdown
Member Author

bfjelds commented May 11, 2026

/azp run [GITHUB]-trident-pr-e2e

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bfjelds bfjelds force-pushed the user/bfjelds/mjolnir/port-osmodifier-to-rust branch 3 times, most recently from 9fff08e to 2fb0625 Compare May 11, 2026 21:27
@bfjelds
Copy link
Copy Markdown
Member Author

bfjelds commented May 11, 2026

/azp run [GITHUB]-trident-pr

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bfjelds
Copy link
Copy Markdown
Member Author

bfjelds commented May 11, 2026

/azp run [GITHUB]-trident-pr-e2e

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bfjelds
Copy link
Copy Markdown
Member Author

bfjelds commented May 12, 2026

/azp run [GITHUB]-trident-pr-e2e

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bfjelds
Copy link
Copy Markdown
Member Author

bfjelds commented May 12, 2026

/azp run [GITHUB]-trident-pr-e2e

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bfjelds bfjelds force-pushed the user/bfjelds/mjolnir/port-osmodifier-to-rust branch 5 times, most recently from becb22b to ccabced Compare May 14, 2026 16:25
@bfjelds
Copy link
Copy Markdown
Member Author

bfjelds commented May 14, 2026

/azp run [GITHUB]-trident-pr-e2e

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

@bfjelds
Copy link
Copy Markdown
Member Author

bfjelds commented May 14, 2026

/azp run [GITHUB]-trident-pr-e2e

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

bfjelds and others added 3 commits May 14, 2026 09:41
Replace the external Go osmodifier binary from azure-linux-image-tools
with a native Rust library crate (crates/osmodifier). Trident now calls
osmodifier functions directly instead of serializing config to YAML,
writing a temp file, and exec'ing the Go binary.

The new crate implements:
- User management (useradd, password hashing, SSH keys, groups)
- Hostname configuration (/etc/hostname)
- Service management (systemctl enable/disable)
- Kernel module configuration (modules-load.d, modprobe.d)
- SELinux configuration (/etc/selinux/config and kernel cmdline)
- /etc/default/grub parsing and writing
- grub.cfg parsing for update-default-grub flow
- grub2-mkconfig execution

Public API:
- modify_os() - replaces osmodifier --config-file for OS modifications
- update_default_grub() - replaces osmodifier --update-grub
- modify_boot() - replaces osmodifier --config-file for boot config

This eliminates:
- External Go binary build dependency (azure-linux-image-tools clone)
- Binary bind-mounting into newroot
- YAML serialization round-trip overhead
- OS_MODIFIER_BINARY_PATH and OS_MODIFIER_NEWROOT_PATH constants
- Makefile, Dockerfile, RPM spec, and pipeline osmodifier references

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ed import

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…l tests

Remove all download-osmodifier.yml template invocations, make
artifacts/osmodifier targets, and functional test binary upload steps.
The osmodifier is now compiled into the trident binary as a library crate.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 25, 2026 18:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 37 out of 38 changed files in this pull request and generated 4 comments.

Comment thread docs/Reference/Host-Configuration/Storage-Rules.md Outdated
Comment thread crates/trident_api/src/config/host/os/mod.rs
Comment thread crates/trident_api/src/config/host/os/mod.rs
Comment thread crates/osmodifier/src/lib.rs
bfjelds and others added 2 commits May 25, 2026 11:16
Change days_since_unix_epoch from returning a bare i64 (with
unwrap_or_default silently producing 0 on pre-1970 clocks) to
returning Result<i64>. A pre-epoch system clock now fails the
operation explicitly instead of silently setting account expiry
to day 0.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add doc comment on find_non_recovery_linux_lines explaining the known
limitation where submenu blocks are not tracked. This matches Go's
FindNonRecoveryLinuxLine behavior and is acceptable because AZL images
built by trident have exactly one kernel installed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 25, 2026 18:18
- Storage-Rules.md: rename first column header from 'Mount Path' to
  'Partition Type' — column lists partition types, not mount paths.
- trident_api SelinuxMode: add 'security=selinux' to Permissive and
  Enforcing doc comments to match actual kernel args set by osmodifier.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 37 out of 38 changed files in this pull request and generated 3 comments.

Comment thread docs/Reference/Host-Configuration/Storage-Rules.md
Comment thread crates/osmodifier/src/lib.rs Outdated
Comment thread packaging/rpm/trident.spec
bfjelds and others added 2 commits May 25, 2026 11:27
Correct the doc comment to reflect that both the chroot'd boot path
and the MOS configuration path use OsModifierContext::default() (root
'/') in production. The non-'/' option is for unit tests only.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 25, 2026 18:35
Route the error through parse_err like the other error paths in the
shadow file map closure, since the closure returns String not Result.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 37 out of 38 changed files in this pull request and generated 6 comments.

Comment thread crates/osmodifier/Cargo.toml
Comment thread crates/trident_api/schemas/host-config-schema.json Outdated
Comment thread crates/trident_api/schemas/host-config-schema.json Outdated
Comment thread crates/osmodifier/src/users.rs
Comment thread crates/trident/src/subsystems/osconfig/mod.rs
Comment thread .pipelines/templates/stages/trident_rpms/build-source.yml
bfjelds and others added 2 commits May 25, 2026 11:46
Add blank lines before paragraphs that follow numbered/bulleted lists
in doc comments to prevent clippy from treating them as list items.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The schema descriptions for permissive and enforcing modes omitted
the security=selinux kernel arg that the code actually appends.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 25, 2026 18:58
bfjelds and others added 2 commits May 25, 2026 12:01
The valid_mount_paths table header said 'Mount Path' but the rows
contain partition type names (esp, root, etc.), not mount paths.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Run make build-api-docs to regenerate:
- host-config-schema.json: enforcing description now includes
  security=selinux LSM explanation from Rust doc comment
- SelinuxMode.md: permissive and enforcing descriptions updated
  with security=selinux kernel arg

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 38 out of 39 changed files in this pull request and generated 4 comments.

Comment thread docs/Reference/Host-Configuration/API-Reference/SelinuxMode.md
Comment thread docs/Reference/Host-Configuration/API-Reference/SelinuxMode.md
Comment thread crates/osmodifier/Cargo.toml
Comment thread crates/osmodifier/src/users.rs
bfjelds and others added 2 commits May 25, 2026 12:16
The osmodifier crate uses serde but not serde_yaml directly.
Config deserialization happens in the caller (trident crate).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 25, 2026 19:33
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 38 out of 39 changed files in this pull request and generated 5 comments.

Comment thread crates/osmodifier/src/users.rs
Comment thread crates/osmodifier/src/users.rs
Comment thread crates/osmodifier/src/lib.rs
Comment thread crates/trident/src/subsystems/osconfig/mod.rs
Comment thread crates/osmodifier/src/users.rs
Use child.stdin.take() to explicitly close the pipe after writing,
ensuring the child process receives EOF. While wait_with_output()
does this internally, explicit take() is idiomatic and clearer.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bfjelds
Copy link
Copy Markdown
Member Author

bfjelds commented May 25, 2026

/azp run [GITHUB]-trident-pr-e2e

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants