Skip to content

engineering: [osmodifier follow-up] add hostname and overlay input validation#654

Merged
bfjelds merged 4 commits into
mainfrom
user/bfjelds/mjolnir/osmodifier-input-validation
May 27, 2026
Merged

engineering: [osmodifier follow-up] add hostname and overlay input validation#654
bfjelds merged 4 commits into
mainfrom
user/bfjelds/mjolnir/osmodifier-input-validation

Conversation

@bfjelds
Copy link
Copy Markdown
Member

@bfjelds bfjelds commented May 26, 2026

🔍 Description

Follow up to #638

Port validation parity from Go osmodifierapi.OS.IsValid():

  • Hostname validation matching govalidator.IsDNSName() plus underscore rejection: label length 1-63, total ≤255, alphanumeric+hyphen only, no leading/trailing hyphens, no underscores, no IP addresses, optional trailing dot.

  • Duplicate overlay path detection: reject duplicate upper_dir or work_dir values across overlays (separate sets, matching Go).

Both validate() methods are called at the top of modify_os() and modify_boot() respectively, failing fast before any filesystem writes.

Addresses review comment on PR #638 about input validation parity gap.

@bfjelds bfjelds changed the title osmodifier: add hostname and overlay input validation engineering: add hostname and overlay input validation May 26, 2026
@bfjelds bfjelds changed the title engineering: add hostname and overlay input validation engineering: [osmodifier follow-up] add hostname and overlay input validation May 26, 2026
bfjelds and others added 3 commits May 26, 2026 11:14
Port validation parity from Go osmodifierapi.OS.IsValid():

- Hostname validation matching govalidator.IsDNSName() plus underscore
  rejection: label length 1-63, total ≤255, alphanumeric+hyphen only,
  no leading/trailing hyphens, no underscores, no IP addresses,
  optional trailing dot.

- Duplicate overlay path detection: reject duplicate upper_dir or
  work_dir values across overlays (separate sets, matching Go).

Both validate() methods are called at the top of modify_os() and
modify_boot() respectively, failing fast before any filesystem writes.

Addresses review comment on PR #638 about input validation parity gap.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bfjelds bfjelds force-pushed the user/bfjelds/mjolnir/osmodifier-input-validation branch from e328637 to c22cbcb Compare May 26, 2026 18:14
@bfjelds bfjelds changed the base branch from user/bfjelds/mjolnir/port-osmodifier-to-rust to main May 26, 2026 18:15
@bfjelds
Copy link
Copy Markdown
Member Author

bfjelds commented May 26, 2026

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

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bfjelds bfjelds marked this pull request as ready for review May 26, 2026 20:18
@bfjelds bfjelds requested a review from a team as a code owner May 26, 2026 20:18
Copilot AI review requested due to automatic review settings May 26, 2026 20:18
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

Adds early, Go-parity input validation to the Rust crates/osmodifier entry points so invalid hostname/overlay inputs fail fast before any filesystem modifications occur.

Changes:

  • Call validate() at the start of modify_os() and modify_boot() to fail fast on invalid configs.
  • Add hostname validation to OSModifierConfig (DNS-name rules + underscore rejection + reject IP literals + allow trailing dot).
  • Add duplicate overlay path detection to BootConfig (unique upper_dir and work_dir within their respective sets), plus unit tests.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
crates/osmodifier/src/lib.rs Ensures OS/boot modifications validate configuration before performing any operations.
crates/osmodifier/src/config.rs Implements hostname and overlay validation logic and adds unit tests for the new validation behavior.

Comment thread crates/osmodifier/src/config.rs Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 27, 2026 15:02
@bfjelds
Copy link
Copy Markdown
Member Author

bfjelds commented May 27, 2026

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

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

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 2 out of 2 changed files in this pull request and generated no new comments.

@bfjelds
Copy link
Copy Markdown
Member Author

bfjelds commented May 27, 2026

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

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bfjelds bfjelds merged commit ed90b18 into main May 27, 2026
97 checks passed
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