networking key in default_facts.yml masks all values from rspec-puppet-facts

[silug]

Describe the Bug

Adding a top-level networking key in spec/default_facts.yml masks all values in the networking key from rspec-puppet-facts. For example, in tests networking.fqdn is not defined.

Expected Behavior

The networking facts provided by rspec-puppet-facts should work in tests.

Steps to Reproduce

Steps to reproduce the behavior:

1. pdk new module
2. Update template-url and template-ref to point to upstream.

  "template-url": "https://github.com/puppetlabs/pdk-templates#3.0.0",
  "template-ref": "tags/3.0.0-0-g5bfc1c0"

3. pdk update
4. Create an empty class with pdk new class and add the following to the generated spec test:

      let(:pre_condition) do
        <<~END
          if $facts['networking']['fqdn'].empty {
            fail("No fqdn fact: networking => ${facts['networking']}")
          }
        END
      end

5. pdk test unit

The resulting error will look like this:

     Failure/Error: it { is_expected.to compile.with_all_deps }
       error during compilation: Evaluation Error: Error while evaluating a Function Call, No fqdn fact: networking => {ip => 172.16.254.254, ip6 => FE80:0000:0000:0000:AAAA:AAAA:AAAA, mac => AA:AA:AA:AA:AA:AA}

Environment

• Version 3.0.0
• Platform Fedora 39 (using the Fedora 36 package)

Additional Context

In previous versions of the templates, only top-level keys were defined in spec/default_facts.yml, so the merge that was happening in spec/spec_helper.rb worked fine with default values. With nested facts added to spec/default_facts.yml, the merge needs to be replaced with a deep_merge.

Note that simply removing spec/default_facts.yml by adding

spec/default_facts.yml:
  delete: true

to .sync.yml and running pdk update also works since the default provided facts are being provided by rspec-puppet-facts already.

[jordanbreen28]

thanks for raising this @silug.
I've raised #544, if you get a moment could you update the template ref to that PR branch

  "template-url": "https://github.com/puppetlabs/pdk-templates#gh-543-fix_networking_facts_merge",
  "template-ref": "remotes/origin/gh-543-fix_networking_facts_merge-0-gcb66796"

run pdk update and let me know how that works out?
Thanks again

[silug]

@jordanbreen28 That change does not appear to affect the behavior. I'm guessing the add_custom_fact function is where the change needs to happen. I'm not sure where that's coming from though.

[silug]

If it helps, I generated a module with the steps necessary to demonstrate the issue at https://github.com/silug/issue_543.

[jordanbreen28]

@jordanbreen28 That change does not appear to affect the behavior. I'm guessing the add_custom_fact function is where the change needs to happen. I'm not sure where that's coming from though.

@silug From what I can see, add_custom_fact is being pulled in from rspec-puppet-facts.

What's good is your initial point still stands, we definitely should have been deep merging by default in spec_helper.rb.erb to allow using nested facts in default_facts.yml without overriding the entire fact, so thanks for bringing that up.

Just need to get thinking about the best way to proceed here.

It's an interesting one; nice catch!

[jordanbreen28]

@silug fwiw I had raised this before, but we paused it due to some unexpected behaviour we were seeing.
Looks like we may need to pick it back up

[jay7x]

Just hit that with PDK templates v3.0.1. Can we maybe delete the default_facts.yml completely and suggest to use rspec-puppet-facts way by default?