Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resolvers/networking.rb: avoid calling ipconfig unless Windows #2481

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

spchamp
Copy link

@spchamp spchamp commented Apr 21, 2022

The following message was observed when running puppet catalog compile under RUBYOPT=--debug with Puppet 7 and Facter 4.2.9 on FreeBSD 12.3

Exception `Errno::ENOENT' at /opt/puppet/puppet_wk/puppet_ctl/tools/bundle/ruby/3.0/gems/facter-4.2.9/lib/facter/custom_facts/core/execution/popen3.rb:17 - No such file or directory - ipconfig

In order to prevent the call to the Microsoft Windows ipconfig tool on non-Microsoft Windows platforms, this changeset adds an additional test under the method Facter::Resolvers::Networking.extract_dhcp

The patched networking.rb has been tested with Puppet 7 from a local installation of Puppet, using Ruby 3.0 from FreeBSD ports, on a FreeBSD 12.3 platform

(cherry picked from commit 614213a)

The following message was observed when running 'puppet catalog compile'
under RUBYOPT="--debug" with Puppet 7 and Facter 4.2.9 on FreeBSD 12.3

~~~~
Exception `Errno::ENOENT' at /opt/puppet/puppet_wk/puppet_ctl/tools/bundle/ruby/3.0/gems/facter-4.2.9/lib/facter/custom_facts/core/execution/popen3.rb:17 - No such file or directory - ipconfig
~~~~

In order to prevent the call to the Microsoft Windows ipconfig tool,
this changeset adds an additional test under the method
 Facter::Resolvers::Networking.extract_dhcp

The patched networking.rb has been tested with Puppet 7 from a local
installation under Ruby 3.0 from FreeBSD ports, on FreeBSD 12.3

(cherry picked from commit 614213a)
@spchamp spchamp requested a review from a team as a code owner April 21, 2022 05:31
@puppetlabs-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@CLAassistant
Copy link

CLAassistant commented Apr 21, 2022

CLA assistant check
All committers have signed the CLA.

@@ -71,7 +71,8 @@ def extract_mac(raw_data, parsed_interface_data)
end

def extract_dhcp(interface_name, raw_data, parsed_interface_data)
return unless raw_data =~ /status:\s+active/
return unless ((raw_data =~ /status:\s+active/) &&
(OsDetector.instance.identifier == :windows))
Copy link
Contributor

Choose a reason for hiding this comment

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

ipconfig is valid on macOS too:

# uname -a
Darwin taxable-stump 21.3.0 Darwin Kernel Version 21.3.0: Wed Jan  5 21:37:58 PST 2022; root:xnu-8019.80.24~20/RELEASE_X86_64 x86_64
# ipconfig getoption en0 server_identifier
10.32.22.9

Copy link
Author

Choose a reason for hiding this comment

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

Candidly, I'm not familiar with the OS X environment, or so much of Windows networking. I was aware that there is an ipfconfig tool on Windows, did not take a look at the cmd syntax though.

It looks like :macosx is actually where the matching ipconfig would be found. I'll update the patch

Updating the previous change for Facter::Resolvers::Networking.extract_dhcp
to check for :macosx instead of :windows before calling the platform's
ipconfig cmd
@spchamp
Copy link
Author

spchamp commented Apr 26, 2022

I wasn't sure of adding any require calls to the change - assuming it all gets linked together properly once facter is all loaded

@joshcooper
Copy link
Contributor

@spchamp Thanks for your contribution! Could you squash your commit and update the commit summary (since we're only calling ipconfig on macOS)? It's a little bit of a code smell to check of macos in the "base" resolver, but I think it's ok for now. If we need more macOS specific networking behavior then we can move that to a subclass later.

Copy link
Contributor

@joshcooper joshcooper left a comment

Choose a reason for hiding this comment

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

Hi @spchamp could you squash your commits?

@joshcooper joshcooper added the bug Something isn't working label Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants