Jira (FACT-2874) facter 4.0.46 should load external fact files in lexicographical order

25 views
Skip to first unread message

James Ralston (Jira)

unread,
Nov 23, 2020, 7:05:03 PM11/23/20
to puppe...@googlegroups.com
James Ralston created an issue
 
Facter / Bug FACT-2874
facter 4.0.46 should load external fact files in lexicographical order
Issue Type: Bug Bug
Assignee: Unassigned
Created: 2020/11/23 4:04 PM
Priority: Normal Normal
Reporter: James Ralston

Up through facter 3.14.14 (Puppet 6.x), when facter loaded external facts from files in the /etc/facter/facts.d directory, if the same fact appeared in multiple files, the version of the fact that appeared in the file Puppet loaded last won.

Starting in facter 4.0.46 (Puppet 7.x), if a fact is duplicated in multiple files, facter appears to use the first value it sees for the ultimate value of that fact, not the last.

This is problematic in the case where events can dynamically add and remove files from the /etc/facter/facts.d directory.

Here's an example. I have a NetworkManager dispatch script, /etc/NetworkManager/dispatcher.d/50-facter, that drops facter facts into /etc/facter/facts.d that contribute interface information. E.g.:

$ ls -lsa /etc/facter/facts.d
total 12
0 drwxr-xr-x. 1 root root  112 Nov 23 16:56 ./
0 drwxr-xr-x. 1 root root   14 Jul 13  2015 ../
4 -rw-r--r--. 1 root root 3459 Nov 23 16:56 if_enp6s0.yaml
4 -rw-r--r--. 1 root root 2136 Nov 23 16:06 if_tun0.yaml
4 -rw-r--r--. 1 root root 1840 Nov 22 04:53 if_virbr0.yaml

Any time the dispatch script updates (adds, rewrites, removes) any of these files, it sends the Puppet agent a SIGUSR1 signal to immediately trigger a Puppet agent run. In the case where I have, say, connected to my work VPN, this will update various settings (proxy settings, nameserver settings, et. al.) to match the values that they must have in order for functional network settings while using the VPN.

Some of the settings conflict across files. For example, the DHCP domain name fact appears in both if_enp6s0.yaml and if_tun0.yaml. But I want the settings in if_tun0.yaml to win, and with facter's old behavior, that was what occurred.

facter appears to sort the entries in the /etc/facter/facts.d directory before loading them. From class Facter::Util::DotD in /opt/puppetlabs/puppet/cache/lib/facter/facter_dot_d.rb:

class Facter::Util::DotD
  require 'yaml'
  # These will be nil if Puppet is not available.
  def initialize(dir = '/etc/facts.d', cache_file = File.join(Puppet[:libdir], 'facts_dot_d.cache'))
    @dir = dir
    @cache_file = cache_file
    @cache = nil
    @types = { '.txt' => :txt, '.json' => :json, '.yaml' => :yaml }
  end
 
  # entries
  def entries
    Dir.entries(@dir).reject { |f| f =~ %r{^\.|\.ttl$} }.sort.map { |f| File.join(@dir, f) }
  rescue
    []
  end

But if I strace puppet facts, it is clearly not processing the file entries in lexicographical order:

$ strace -f puppet facts 2>&1 | grep 'open.*if_'
[pid 124657] openat(AT_FDCWD, "/etc/facter/facts.d/if_virbr0.yaml", O_RDONLY|O_CLOEXEC) = 5
[pid 124657] openat(AT_FDCWD, "/etc/facter/facts.d/if_enp6s0.yaml", O_RDONLY|O_CLOEXEC) = 5
[pid 124657] openat(AT_FDCWD, "/etc/facter/facts.d/if_tun0.yaml", O_RDONLY|O_CLOEXEC) = 5

Because the if_tun0.yaml file will always be the last directory entry (because it is removed and added, while the other files are not), and because facter now uses the first fact definition it finds, instead of the last definition, there is no way to get the facts in if_tun0.yaml to override the facts in if_enp6so.yaml.

Even if I update my dispatch script to order the interface files in lexicographical order by priority:

$ ls -lsa /etc/facter/facts.d
total 12
0 drwxr-xr-x. 1 root root  130 Nov 23 17:39 ./
0 drwxr-xr-x. 1 root root   14 Jul 13  2015 ../
4 -rw-r--r--. 1 root root 2138 Nov 23 17:37 if_25_tun0.yaml
4 -rw-r--r--. 1 root root 3382 Nov 23 17:39 if_50_enp6s0.yaml
4 -rw-r--r--. 1 root root 1840 Nov 22 04:53 if_50_virbr0.yaml

It won't matter, because facter is using the directory order:

$ strace -f puppet facts 2>&1 | grep 'open.*if_'
[pid 128586] openat(AT_FDCWD, "/etc/facter/facts.d/if_50_virbr0.yaml", O_RDONLY|O_CLOEXEC) = 5
[pid 128586] openat(AT_FDCWD, "/etc/facter/facts.d/if_50_enp6s0.yaml", O_RDONLY|O_CLOEXEC) = 5
[pid 128586] openat(AT_FDCWD, "/etc/facter/facts.d/if_25_tun0.yaml", O_RDONLY|O_CLOEXEC) = 5

There are two simple changes that will resolve this issue:

  1. Make sure all facter code paths sort the files in /etc/facter/facts.d in lexicographical order before loading them.
  2. Clearly document how facter resolves loading external facts when the same fact is defined in multiple files. (Does the first fact definition win? The last?)

To be clear: #1 is not a recent issue; Puppet has never loaded the fact files in /etc/facter/facts.d in lexicographical order as far as I know. But without lexicographical ordering, changing the external fact duplicate resolution precedence from last to first breaks certain use cases.

Perhaps #2 is already documented, but if so, I was unable to locate it. (And I also saw nothing in the release notes that mentioned the external fact duplicate resolution precedence change.)

Add Comment Add Comment
 
This message was sent by Atlassian Jira (v8.5.2#805002-sha1:a66f935)
Atlassian logo

Bogdan Irimie (Jira)

unread,
Nov 24, 2020, 2:58:03 AM11/24/20
to puppe...@googlegroups.com

Bogdan Irimie (Jira)

unread,
Nov 24, 2020, 3:05:04 AM11/24/20
to puppe...@googlegroups.com

Oana Tanasoiu (Jira)

unread,
Nov 24, 2020, 9:24:04 AM11/24/20
to puppe...@googlegroups.com

Bogdan Irimie (Jira)

unread,
Nov 25, 2020, 9:04:06 AM11/25/20
to puppe...@googlegroups.com
Bogdan Irimie updated an issue
Change By: Bogdan Irimie
Sprint: ghost 25.11 , ready for triage 2

Bogdan Irimie (Jira)

unread,
Dec 2, 2020, 2:40:04 AM12/2/20
to puppe...@googlegroups.com
Bogdan Irimie updated an issue
Change By: Bogdan Irimie
Sprint: ghost 25.11, ghost-2.12 , ready for triage 2

Bogdan Irimie (Jira)

unread,
Dec 2, 2020, 2:43:04 AM12/2/20
to puppe...@googlegroups.com
Bogdan Irimie updated an issue
Change By: Bogdan Irimie
Sprint: ghost 25.11, ghost-2.12, ready for triage ghost- 2 .12

Bogdan Irimie (Jira)

unread,
Dec 2, 2020, 6:51:03 AM12/2/20
to puppe...@googlegroups.com

Josh Cooper (Jira)

unread,
Dec 2, 2020, 2:15:03 PM12/2/20
to puppe...@googlegroups.com

Josh Cooper (Jira)

unread,
Dec 2, 2020, 2:15:04 PM12/2/20
to puppe...@googlegroups.com

James Ralston (Jira)

unread,
Dec 16, 2020, 2:15:13 PM12/16/20
to puppe...@googlegroups.com
James Ralston commented on Bug FACT-2874
 
Re: facter 4.0.46 should load external fact files in lexicographical order

When testing Puppet Platform 7.1, this appears to be fixed: for duplicate facts, the one that appears in the file that sorts lexicographically last is the one that is used. Thank you!

Claire Cadman (Jira)

unread,
Dec 16, 2020, 2:58:14 PM12/16/20
to puppe...@googlegroups.com
Reply all
Reply to author
Forward
0 new messages