puppetlabs / rspec-puppet

RSpec tests for your Puppet manifests
https://puppetlabs.github.io/rspec-puppet/
MIT License
13 stars 18 forks source link

Impossible to set ipaddress6 fact #15

Closed olavmrk closed 1 year ago

olavmrk commented 2 years ago

Describe the Bug

It is not possible to set the ipaddress6 fact to any other value than FE80:0000:0000:0000:AAAA:AAAA:AAAA.

Expected Behavior

The ipaddress6 fact should behave like other facts.

Steps to Reproduce

Create a class manifests/init.pp:

class puppet_ipaddress6_test {
  file { '/foo':
    content => $facts['ipaddress6'],
  }
}

And a spec test spec/classes/init_spec.rb:

require 'puppetlabs_spec_helper/module_spec_helper'

describe 'puppet_ipaddress6_test', :type => :class do
    let (:node) { 'server.example.org' }
    let (:facts) do {
        'ipaddress6' => '2001:db8::1',
    } end
    it {
        is_expected.to contain_file('/foo').
            with_content(/^2001:db8::1$/)
    }
end

And a Rakefile:

require 'puppetlabs_spec_helper/rake_tasks'

Then run the spec test:

rake spec

It will fail with the following error:

  1) puppet_ipaddress6_test is expected to contain File[/foo] with content =~ /^2001:db8::1$/
     Failure/Error:
       is_expected.to contain_file('/foo').
           with_content(/^2001:db8::1$/)

       expected that the catalogue would contain File[/foo] with content set to /^2001:db8::1$/ but it is set to "FE80:0000:0000:0000:AAAA:AAAA:AAAA"
     # ./spec/classes/init_spec.rb:9:in `block (2 levels) in <top (required)>'

Environment

Additional Context

The hard-coded ipaddress6 fact was introduced in (#771) Set a default dummy ipaddress6 fact.

ekohl commented 2 years ago

In https://github.com/rodjek/rspec-puppet/pull/772#issuecomment-567584159 I noted that it's the combination of setting both node and trying to override this fact. That may not be a valid workaround for you, but it helps to understand the problem.

LukasAud commented 1 year ago

Hi @olavmrk, can you confirm whether this issue is still present?

olavmrk commented 1 year ago

Hi @olavmrk, can you confirm whether this issue is still present?

Hi! I just retested with the rspec-puppet 3.0.0, and can confirm that the issue is still present.

This is the updated Gemfile.lock:

ekohl commented 1 year ago

It's this code that causes it: https://github.com/puppetlabs/rspec-puppet/blob/b478497ac64994e588891cc72f2b363d7a9e50b0/lib/rspec-puppet/support.rb#L271-L276 So you can set the derive_node_facts_from_nodename setting to false and that should disable the behavior.

I still think it's wrong to hardcode an ipaddress6. 4ef2f3643eaab642d2f24ffda6a507699d9ae0a8 looks like a workaround and now that we have a custom FacterImpl (48c56051047667ff6f7f3915d7597e2da669e26d) it's probably even redundant: no calls to the real facter should happen if you let it stub from facterdb. It's still optional to do that, so taking that into consideration I've written https://github.com/puppetlabs/rspec-puppet/pull/61.

@olavmrk could you try out my patch?

olavmrk commented 1 year ago

@olavmrk could you try out my patch?

Thank you for looking into this! I tested your changes, but I am still seeing the same behavior. See my comment on your pull request for details.


In a way this issue has become less of a problem because ipaddress6 is a legacy fact. Since opening this issue I have moved most of the code away from $facts['ipaddress6'] and over to $facts['networking']['ip6'], which does not have the same problem.

I still think it is really weird that there is a fact that I cannot override though. Especially since the ipaddress fact can be overridden in the normal way.

ekohl commented 1 year ago

I still think it is really weird that there is a fact that I cannot override though. Especially since the ipaddress fact can be overridden in the normal way.

100% with you on that.