Skip to content

Update facter regex to work for Rust sudo variant (sudo-rs)#334

Merged
saz merged 2 commits intosaz:masterfrom
ky3mr2:master
Mar 5, 2026
Merged

Update facter regex to work for Rust sudo variant (sudo-rs)#334
saz merged 2 commits intosaz:masterfrom
ky3mr2:master

Conversation

@ky3mr2
Copy link
Copy Markdown
Contributor

@ky3mr2 ky3mr2 commented Mar 4, 2026

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

@kenyon
Copy link
Copy Markdown

kenyon commented Mar 4, 2026

Fixes #331.

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

Updates the custom sudoversion Facter fact parsing to recognize Rust-based sudo-rs output, addressing failures on newer Ubuntu releases.

Changes:

  • Expand the sudo -V output regex to match both classic Sudo version … and sudo-rs … formats.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread lib/facter/sudoversion.rb Outdated
Comment thread lib/facter/sudoversion.rb Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@saz saz merged commit 822b386 into saz:master Mar 5, 2026
12 checks passed
Comment thread lib/facter/sudoversion.rb
if Facter::Util::Resolution.which('sudo')
sudoversion = Facter::Util::Resolution.exec('sudo -V 2>&1')
%r{^Sudo version ([\w.]+)}.match(sudoversion)[1]
match = %r{^(?:Sudo version|sudo-rs)\s+([\w.]+)}i.match(sudoversion)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This silences an error, but may cause existing logic to do the wrong thing as sudo-rs has a very small version number compared to traditional sudo, and the fact does not communicate which is in use.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

You're right about that, thanks for pointing it out. I guess it would be best to add another fact, sudokind or something like that, and extend any version checks to also check for the expected kind of sudo.

What do you think?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

My thought was a new structured fact that contained both the implementation and the version:

{'sudo' => {'version' => '0.2.0', 'type' => 'sudo-rs'}}

And then have the existing sudoversion return nil when an unrecognized version string appears. That should prevent any existing logic from thinking sudo-rs is a very old version of sudo and provide a new fact source that pairs the version with the type of implementation.

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.

5 participants