Avoid hardcoding ipaddress6 default #61
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for looking into this! I tested this change by pulling in your branch in the test project I created when testing #15: diff --git a/Gemfile b/Gemfile
index 284dc42..710e8bb 100644
--- a/Gemfile
+++ b/Gemfile
@@ -1,2 +1,3 @@
source 'https://rubygems.org'
gem 'puppetlabs_spec_helper'
+gem 'rspec-puppet', git: 'https://github.com/ekohl/rspec-puppet.git', branch: 'avoid-hardcoding-ipaddress6'Unfortunately it does not appear to make any difference. Show logI will be away for a while, so I won't be able to do further testing in a while, but I have attached a ZIP-file with my test project: See the output below for an example of how I tested this: Running the test from the ZIP file$ cd /tmp/
$ mkdir puppet_ipaddress6_test
$ cd puppet_ipaddress6_test/
$ wget https://github.com/puppetlabs/rspec-puppet/files/12049360/puppet_ipaddress6_test.zip
[...]
2023-07-14 12:20:09 (500 KB/s) - ‘puppet_ipaddress6_test.zip’ saved [2269/2269]
$ unzip puppet_ipaddress6_test.zip
Archive: puppet_ipaddress6_test.zip
4c946aace4e67bcbae9c71b772315d17417fd1b3
extracting: .gitignore
inflating: Gemfile
inflating: Gemfile.lock
extracting: Rakefile
creating: manifests/
inflating: manifests/init.pp
creating: spec/
creating: spec/classes/
inflating: spec/classes/init_spec.rb
$ bundle config set --local path 'vendor/bundle'
$ bundle install
Fetching gem metadata from https://rubygems.org/........
Fetching https://github.com/ekohl/rspec-puppet.git
[...]
Bundle complete! 2 Gemfile dependencies, 28 gems now installed.
Bundled gems are installed into `./vendor/bundle`
$ bundle exec rake spec
I, [2023-07-14T12:20:39.756322 #268777] INFO -- : Creating symlink from spec/fixtures/modules/puppet_ipaddress6_test to /tmp/puppet_ipaddress6_test
/usr/bin/ruby3.0 -I/tmp/puppet_ipaddress6_test/vendor/bundle/ruby/3.0.0/gems/rspec-core-3.12.2/lib:/tmp/puppet_ipaddress6_test/vendor/bundle/ruby/3.0.0/gems/rspec-support-3.12.1/lib /tmp/puppet_ipaddress6_test/vendor/bundle/ruby/3.0.0/gems/rspec-core-3.12.2/exe/rspec --pattern spec/\{aliases,classes,defines,functions,hosts,integration,plans,tasks,type_aliases,types,unit\}/\*\*/\*_spec.rb
F
Failures:
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)>'
Finished in 0.18995 seconds (files took 0.60521 seconds to load)
1 example, 1 failure
Failed examples:
rspec ./spec/classes/init_spec.rb:8 # puppet_ipaddress6_test is expected to contain File[/foo] with content =~ /^2001:db8::1$/
/usr/bin/ruby3.0 -I/tmp/puppet_ipaddress6_test/vendor/bundle/ruby/3.0.0/gems/rspec-core-3.12.2/lib:/tmp/puppet_ipaddress6_test/vendor/bundle/ruby/3.0.0/gems/rspec-support-3.12.1/lib /tmp/puppet_ipaddress6_test/vendor/bundle/ruby/3.0.0/gems/rspec-core-3.12.2/exe/rspec --pattern spec/\{aliases,classes,defines,functions,hosts,integration,plans,tasks,type_aliases,types,unit\}/\*\*/\*_spec.rb failed |
|
@olavmrk to benefit from this you will need to set up rspec-puppet-facts. You need a proper metadata.json for require 'puppetlabs_spec_helper/module_spec_helper'
require 'rspec-puppet-facts'
RSpec.configure do |c|
c.facter_implementation = :rspec
end
include RspecPuppetFacts
describe 'puppet_ipaddress6_test' do
on_supported_os.each do |os, os_facts|
context "on #{os}" do
let(:facts) { os_facts.merge(ipaddress6: '2001:db8::1') }
let(:node) { 'server.example.org' }
it { is_expected.to contain_file('/foo').with_content('2001:db8::1') }
end
end
end |
ekohl
commented
Jul 14, 2023
lib/rspec-puppet/support.rb
Outdated
| @@ -242,9 +242,17 @@ def facts_hash(node) | |||
| 'fqdn' => node, | |||
| 'domain' => node.split('.', 2).last, | |||
| 'clientcert' => node, | |||
There was a problem hiding this comment.
Can I just say I hate the Style/TrailingCommaInHashLiteral setting that's in this project? It creates bigger diffs than needed.
There was a problem hiding this comment.
Noted. Although if its a rubocop setting, then its probably a standard in all our modules and tools by now.
ekohl
force-pushed
the
avoid-hardcoding-ipaddress6
branch
from
2dc5bc4 to
1adccf5
Compare
bastelfreak
approved these changes
Jul 14, 2023
|
The PR looks good, although this has shed some light on for me on the fact that this tool appears to still be using legacy facts. Wondering if there is any specific reason or if it was simply a whoopsie on our side. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
In 4ef2f36 this was added as a workaround, but in 48c5605 a custom Facter implementation was added. This avoids any calls to the real Facter and doesn't suffer from the issue where it was attempting to load Windows gems on Linux.
Additional Context
Related Issues (if any)
Fixes #15
Checklist