Skip to content

Conversation

@cthorn42
Copy link
Collaborator

Backport from facter-private a number of fixes and improvements to 4.15.0 branch

Key changes:

  • Run integration tests consistently (07090afef)
  • Improve Docker container detection via /.dockerenv check (9896c95)
  • Handle binary data gracefully without failing (67051e492)

mhashizume and others added 26 commits September 19, 2025 08:42
As part of our presuite actions, we verify that we can run Facter as a
standalone Ruby application. During this, we run a `bundle install`,
which installs documentation dependencies.

It is a known issue that the manpage generation library Facter uses,
ronn, is out-of-date and errors on modern compilers. See:
#2703

This commit updates our presuit action to omit installing documentation
dependencies. This also removes the option of omitting development
dependencies, as there is no such group in Facter's gemfile.
Rake has long automatically imported additional Rake tasks from the
rakelib directory. This commit renames the tasks directory to rakelib
and removes custom logic from the top-level Rakefile that imported Rake
tasks from the tasks directory.
… xenhvm, xenu hypervisors

Previously, Facter incorrectly assumes AWS when kvm, xen, and xenhvm
hypervisors are used and there is ec2 metadata. For example, Facter
incorrectly assumes AWS when something is running on OpenStack and is using
kvm/xen as their hypervisor.

This commit updates lib/facter/facts/linux/cloud/provider.rb to try to use
virt-what to determine the cloud.provider fact when on kvm, xen, xenhvm, and
xenu hypervisors. If unable to use virt-what and there ec2 metadata,
provider.rb will fallback on the previous behavior and return AWS. virt-what
needs to be run as root due to using dmidecode.

This commit also updates lib/facter/resolvers/processors.rb to resolve rubocop
violations.
Commit 7bc38cc combined the call to
`FileHelper.safe_read(`/etc/machine-id`, nil)` and `strip`. However, if we ever
fail to read `/etc/machine-id`, then we would attempt to call `strip` on nil.

We could use safe-nav operator, however, that would result in the hypervisor
containing:

    { systemd_nspawn: { 'id' => nil } }

This commit restores the original behavior of only including hypervisor
information for `systemd-nspawn` if the machine_id is not nil and protects
against the `nil` case.
If /.dockerenv exists, assume we're in docker and include cgroup info if
present.

Some people recommend checking for /.dockerinit, but that was dropped awhile
ago, so don't start depending on it now.
See puppet-private#92d2b0311ed4357d6315a2db6f2c28eee6e50249 and 4755ca430b952345669569ffe8f23103aafb3033
Rubocop started failing due to

```
lib/facter/facts/windows/os/release.rb:20:54: C: [Correctable] Style/RedundantParentheses: Don't use parentheses around a method argument.
          arr << Facter::ResolvedFact.new(FACT_NAME, ({ full: fact_value, major: fact_value } if fact_value))
                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/facter/facts/windows/os/release.rb:20:94: F: Lint/Syntax: unexpected token kIF_MOD
(Using Ruby 2.5 parser; configure using TargetRubyVersion parameter, under AllCops)
          arr << Facter::ResolvedFact.new(FACT_NAME, ({ full: fact_value, major: fact_value } if fact_value))
...
```
Currently, chassis types exist in a hash called types in the function
chassis_to_name in dmi.rb. Each type's position and ordering in the hash
corresponds with its corresponding number/hex. This commit adds the following
missing chassis types to the hash:
  - IoT Gateway (byte value: 21h)
  - Embedded PC (byte value: 22h)
  - Mini PC (byte value: 23h)
  - Stick PC (byte value: 24h)

For more information on chassis types, see:
https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.8.0.pdf
The `rake spec_integration` and `rspec spec_integration` commands loaded
spec_helpers differently. The latter caused unit test stubs to incorrectly be
loaded when running integration tests.

Specifically, the first command (rake) loaded:

    spec_integration/spec_helper.rb
    spec_integration/**/*_spec.rb

But the second command (rspec) loaded:

    spec/spec_helper.rb
    spec_integration/**/*_spec.rb
    spec_integration/spec_helper.rb

Rake didn't have the problem because it specified the `default-path` as
`spec_integration`. The `--require spec_helper` option in `.rspec`, then
loaded spec_integration/spec_helper as expected.

When running `rspec` directory, the `default-path` wasn't modified, so it
unexpectedly loaded `spec/spec_helper.rb`. I noticed this behavior earlier in
72241e6, but didn't realize the extent of the
problem. For example, the "unit_test" guard shouild have prevented the
`colorize` method from being defined, registering webmock, etc.

This commit changes the behavior:

* rake and rspec behave the same
* unit and integration tests uses different rspec status files, so
  --only-failures works as expected
* webmock is only registered for unit tests
* The `skip_outside_ci` tag only filters integration tests
* The unit and integration spec_helper behaviors are clearly separated.
* It's not necessary to explicitly `require 'spec_helper'` in integration tests.
Commit c3350f8 removed the `facter/spec`
directory from the load path so that Facter wouldn't attempt to load custom
facts in "spec/**/*.rb". However, when we forked the repo, the name changed
to `facter-private`, reintroducing the problem.

This commit resolves the absolute path to the spec directory, so we're not
sensitive to the name of the git checkout directory.
The socket APIs return a frozen string, so we have to dup before forcing the
encoding. However, in some cases, we make a copy, due to regex matching or
string splitting, so dup is not needed.
ConnectServer returns a WIN32OLE object, whose to_s method returns a binary
string. Since we're just verifying win32ole doesn't hang, return the class name
from the custom fact, which is UTF-8 encoded.
Ruby < 3 returned ENV key/values based on the active code page, but the returned
strings had ASCII-8BIT encoding.

In Ruby 3.0, ENV key/values are UTF-8 encoded. See ruby/ruby@ca76337

We should drop this special case when we drop Ruby 2.7
If fact contains a binary string, a warning will be printed:

    Fact 'networking.interfaces' contains an invalid value: String 'en0,lo' contains binary data

If --trace is specified, then a backtrace is printed too
If one test caused Facter, FactLoader or FactManager to lazily inititalize a
logger using an instance double, then that state leaked into later tests. If a
later test tried to call the logger, rspec reported

```
  1) LegacyFacter::Util::Loader when loading all facts when loads all facts from the environment loaded fact two
     Failure/Error: @log.debugonce('Resolving facts sequentially')
       #<InstanceDouble(Facter::Log) (anonymous)> was originally created in one example but has leaked into another example and can no longer be used. rspec-mocks' doubles are designed to only last for one example, and you need to create a new one in each example you wish to use it for.
     # ./lib/facter/framework/core/fact_manager.rb:93:in `log_resolving_method'
     # ./lib/facter/framework/core/fact_manager.rb:73:in `resolve_core'
     # ./lib/facter.rb:134:in `core_value'
```

For now, clear the global logger state after each test.
These were set to binary:

en0.ip
en0.netmask
en0.network
lo0.ip6
lo0.netmask6
lo0.network6

scope6 & mac are already UTF-8
These were set to binary:

lo0.ip
lo0.ip6
net0.ip
net0.ip6
…working fact

Redhat 10 release string looks like:
NAME="Red Hat Enterprise Linux" VERSION="10.0 (Coughlan)" ID="rhel" ID_LIKE="centos fedora" VERSION_ID="10.0" PLATFORM_ID="platform:el10" PRETTY_NAME="Red Hat Enterprise Linux 10.0 (Coughlan)"
which passes centos condition, hence added one more condition with rhel
The built gem didn't include the help header fixture. Just inline
the text.
This commit adds the following files to the built gem. It also removes
directories from the list of "files" to include in the gemspec.

```
$ gem fetch facter --version 4.13.0
$ gem build facter.gemspec
$ diff \
 <(tar xOf facter-4.13.0.gem data.tar.gz | tar zt | sort) \
 <(tar xOf facter-4.14.0.gem data.tar.gz | tar zt | sort)
1a2,3
> facter.gemspec
> Gemfile
949a953
> README.md
```

This reverts absolute path dir globbing added in commit 61564e2. That issue
will be fixed in PA-7583.
This commit updates LegacyFacter::Util::Normalization.normalize_string to
accept fact values with ASCII 8-BIT without immediately failing. When a fact
value with ASCII 8-BIT is detected but can be force encoded to UTF-8, Facter
will log a debug message with the fact name and value. Facter will only error
(with the fact name & value) when the fact value cannot be successfully force
encoded to UTF-8.

This commit also updates all the normalize methods in
LegacyFacter::Util::Normalization to take in 2 parameters now: the fact name &
value so the debug and error messages can include the fact name. This helps
users more easily figure out which fact is causing issues and has binary.
Prior to commit 7bc38cc, facter checked for
docker and lxc in `/proc/1/cgroups` and fell back to the `container` environment
variable in `/proc/1/environ`.

The above commit reversed the order and labeled unknown container types as
'container_other'. This broke the VirtualDetecter, because it calls the
`containers` resolver and assumes it can fallback to other methods like
check_freebsd, etc

https://github.com/puppetlabs/facter/blob/0656d9a34ce4790129f1fd6eba5bb4d49a9b9ad1/lib/facter/util/facts/posix/virtual_detector.rb#L11

The regression was noticed and corrected for Illumox LX in commit
31d32da, but we didn't realize other
forms of virtualization were affected.

This commit restores the original behavior of checking `/proc/1/cgroups` first,
then `/proc/1/environ`. If a container type is not recognized, we log a debug
message and continue, so that VirtualDetector works as expected.
@cthorn42 cthorn42 requested a review from a team as a code owner October 15, 2025 15:35
The acceptance_tests workflow runs the facter acceptance tests against the last
passing nightly puppet-agent build. While we could test against internal nightly
builds, there are some obstacles:

* Twingate doesn't support mac
* Our internal nightly builds don't understand "latest" for windows builds, so
  the GH action would need to look the latest version up and request it
* We're already testing facter acceptance tests in nightly CI on all platforms
* The Ubuntu-20.04 GH runner was retired. To get the tests working on 24.04,
  we'd need to install 'apt install isc-dhcp-client' for dhclient. Without that
  package the dhcp related network facts will be omitted, causing
  'tests/facts/networking_facts.rb' to fail.

So for now, remove this workflow.
Ubuntu 20.04 was retired 2025-04-15

This follows the same thing we did in puppetlabs/phoenix-github-actions@36a826a
@joshcooper joshcooper merged commit c222602 into main Oct 17, 2025
16 checks passed
@joshcooper joshcooper deleted the backport_4.15.0 branch October 17, 2025 16:30
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.

7 participants