From afd22ddc99af306aa476be7eaf9f02ada941c97b Mon Sep 17 00:00:00 2001 From: Jeff McCune Date: Tue, 28 Feb 2012 11:01:15 -0800 Subject: (#12357) Fix root_home fact on Windows Without this patch the root_home fact fails on windows. This patch fixes the problem by only calling methods on the object returned by the `getent passwd root` command if the object evaluates to true. Because there is no root account on Windows the code block simply returns `nil` which makes the Facter fact undefined on Windows platforms. The root cause of the failure is that we always expected the command to succeed and return something useful, and it may not on all supported platforms. --- spec/unit/facter/root_home_spec.rb | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'spec/unit') diff --git a/spec/unit/facter/root_home_spec.rb b/spec/unit/facter/root_home_spec.rb index 8946d9d..ce80684 100644 --- a/spec/unit/facter/root_home_spec.rb +++ b/spec/unit/facter/root_home_spec.rb @@ -30,13 +30,11 @@ describe Facter::Util::RootHome do end end context "windows" do - let(:root_ent) { "FIXME TBD on Windows" } - let(:expected_root_home) { "FIXME TBD on Windows" } - - it "should return FIXME TBD on windows" do - pending "FIXME: TBD on windows" - Facter::Util::Resolution.expects(:exec).with("getent passwd root").returns(root_ent) - Facter::Util::RootHome.get_root_home.should == expected_root_home + before :each do + Facter::Util::Resolution.expects(:exec).with("getent passwd root").returns(nil) + end + it "should be nil on windows" do + Facter::Util::RootHome.get_root_home.should be_nil end end end -- cgit v1.2.3 From a452f6a9af21d9ec5f34e6cda40af0ec3971b422 Mon Sep 17 00:00:00 2001 From: Jeff McCune Date: Wed, 29 Feb 2012 12:47:08 -0800 Subject: (#12357) Add puppet_vardir custom fact Without this patch the PE modules don't have a way to identify a filesystem path where it's OK to place variable data related to managing the target node. This is a problem when a module like pe_compliance needs to write a wrapper script to the node's filesystem. This patch addresses the problem by exposing the node's Puppet[:vardir] setting as a Facter fact. This fact value will be set to `nil` if Puppet is not loaded into memory. If Puppet is loaded, e.g. using `facter --puppet` or using `puppet agent` or `puppet apply` then the fact will automatically set the value to Puppet[:vardir] The value of this setting is subject to Puppet's run_mode. This patch implements a new utility method in the standard library module named `Facter::Util::PuppetSettings.with_puppet`. The method accepts a block and will only invoke the block if the Puppet library is loaded into the Ruby process. If Puppet is not loaded, the method always returns nil. This makes it easy to define Facter facts that only give values if Puppet is loaded in memory. --- spec/unit/facter/util/puppet_settings_spec.rb | 35 +++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 spec/unit/facter/util/puppet_settings_spec.rb (limited to 'spec/unit') diff --git a/spec/unit/facter/util/puppet_settings_spec.rb b/spec/unit/facter/util/puppet_settings_spec.rb new file mode 100644 index 0000000..c3ce6ea --- /dev/null +++ b/spec/unit/facter/util/puppet_settings_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' +require 'facter/util/puppet_settings' + +describe Facter::Util::PuppetSettings do + + describe "#with_puppet" do + context "Without Puppet loaded" do + before(:each) do + Module.expects(:const_get).with("Puppet").raises(NameError) + end + + it 'should be nil' do + subject.with_puppet { Puppet[:vardir] }.should be_nil + end + it 'should not yield to the block' do + Puppet.expects(:[]).never + subject.with_puppet { Puppet[:vardir] }.should be_nil + end + end + context "With Puppet loaded" do + module Puppet; end + let(:vardir) { "/var/lib/puppet" } + + before :each do + Puppet.expects(:[]).with(:vardir).returns vardir + end + it 'should yield to the block' do + subject.with_puppet { Puppet[:vardir] } + end + it 'should return the nodes vardir' do + subject.with_puppet { Puppet[:vardir] }.should eq vardir + end + end + end +end -- cgit v1.2.3 From 2ce669c7f8aea6b66b2150798a5ef4d253b00a2e Mon Sep 17 00:00:00 2001 From: Jeff McCune Date: Wed, 7 Mar 2012 11:52:30 -0800 Subject: (#12357) Add validate_absolute_path() function This patch adds a new function to validate if a string is an absolute filesystem path or not. The intent of this is to make this functionality generic and reusable. Josh left a comment in another pull request I had: If node_installdir or $node_vardir is not defined, then we should raise an error, otherwise we may create a scheduled task to an untrusted directory. One solution to this comment is to validate the Puppet variable is an absolute path. Examples of this function look like: function_validate_absolute_path Using Puppet::Parser::Scope.new Garbage inputs validate_absolute_path(nil) should fail validate_absolute_path([nil]) should fail validate_absolute_path({"foo"=>"bar"}) should fail validate_absolute_path({}) should fail validate_absolute_path("") should fail relative paths validate_absolute_path("relative1") should fail validate_absolute_path(".") should fail validate_absolute_path("..") should fail validate_absolute_path("./foo") should fail validate_absolute_path("../foo") should fail validate_absolute_path("etc/puppetlabs/puppet") should fail validate_absolute_path("opt/puppet/bin") should fail absolute paths validate_absolute_path("C:/") should not fail validate_absolute_path("C:\\") should not fail validate_absolute_path("C:\\WINDOWS\\System32") should not fail validate_absolute_path("C:/windows/system32") should not fail validate_absolute_path("X:/foo/bar") should not fail validate_absolute_path("X:\\foo\\bar") should not fail validate_absolute_path("/var/tmp") should not fail validate_absolute_path("/var/lib/puppet") should not fail validate_absolute_path("/var/opt/../lib/puppet") should not fail validate_absolute_path("C:\\Program Files (x86)\\Puppet Labs\\Puppet Enterprise") should not fail validate_absolute_path("C:/Program Files (x86)/Puppet Labs/Puppet Enterprise") should not fail Finished in 0.05637 seconds 23 examples, 0 failures --- .../functions/validate_absolute_path_spec.rb | 76 ++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 spec/unit/puppet/parser/functions/validate_absolute_path_spec.rb (limited to 'spec/unit') diff --git a/spec/unit/puppet/parser/functions/validate_absolute_path_spec.rb b/spec/unit/puppet/parser/functions/validate_absolute_path_spec.rb new file mode 100644 index 0000000..2b44f50 --- /dev/null +++ b/spec/unit/puppet/parser/functions/validate_absolute_path_spec.rb @@ -0,0 +1,76 @@ +require 'spec_helper' + +describe Puppet::Parser::Functions.function(:validate_absolute_path) do + before :all do + Puppet::Parser::Functions.autoloader.loadall + end + + let(:scope) do + scope = Puppet::Parser::Scope.new + end + + # The subject of these examplres is the method itself. + subject do + scope.method :function_validate_absolute_path + end + + context 'Using Puppet::Parser::Scope.new' do + + describe 'Garbage inputs' do + paths = [ + nil, + [ nil ], + { 'foo' => 'bar' }, + { }, + '', + ] + + paths.each do |path| + it "validate_absolute_path(#{path.inspect}) should fail" do + expect { subject.call [path] }.to raise_error Puppet::ParseError + end + end + end + describe 'relative paths' do + paths = %w{ + relative1 + . + .. + ./foo + ../foo + etc/puppetlabs/puppet + opt/puppet/bin + } + + paths.each do |path| + it "validate_absolute_path(#{path.inspect}) should fail" do + expect { subject.call [path] }.to raise_error Puppet::ParseError + end + end + end + describe 'absolute paths' do + paths = %w{ + C:/ + C:\\ + C:\\WINDOWS\\System32 + C:/windows/system32 + X:/foo/bar + X:\\foo\\bar + /var/tmp + /var/lib/puppet + /var/opt/../lib/puppet + } + + paths = paths + [ + 'C:\\Program Files (x86)\\Puppet Labs\\Puppet Enterprise', + 'C:/Program Files (x86)/Puppet Labs/Puppet Enterprise', + ] + + paths.each do |path| + it "validate_absolute_path(#{path.inspect}) should not fail" do + expect { subject.call [path] }.not_to raise_error + end + end + end + end +end -- cgit v1.2.3 From 41b07232e464f25403a8f9c786ec0061bf6dc40e Mon Sep 17 00:00:00 2001 From: Jeff McCune Date: Wed, 7 Mar 2012 15:01:11 -0800 Subject: (#12357) Add ability to display an error message from validate_re I've seen a number of times the following error displayed to the end user: validate_re(): "" does not match "^true$|^false$" at /p/t/f.pp:40 This is an absolutely horrific error message. I'm to blame for it. Users stumble over this quite often and they shouldn't have to go read the code to sort out what's happening. This patch makes an effort to fix the problem by adding a third, optional, argument to validate_re(). This third argument will be the message thrown back in the exception which will be displayed to the end user. This sets the stage for nicer error messages coming from modules we write. This patch is backwards compatible but is a new feature. --- .../puppet/parser/functions/validate_re_spec.rb | 80 ++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 spec/unit/puppet/parser/functions/validate_re_spec.rb (limited to 'spec/unit') diff --git a/spec/unit/puppet/parser/functions/validate_re_spec.rb b/spec/unit/puppet/parser/functions/validate_re_spec.rb new file mode 100644 index 0000000..c35ae14 --- /dev/null +++ b/spec/unit/puppet/parser/functions/validate_re_spec.rb @@ -0,0 +1,80 @@ +require 'spec_helper' + +describe Puppet::Parser::Functions.function(:validate_re) do + before :all do + Puppet::Parser::Functions.autoloader.loadall + end + + let(:scope) do + scope = Puppet::Parser::Scope.new + end + + # The subject of these examplres is the method itself. + subject do + scope.method :function_validate_re + end + + context 'Using Puppet::Parser::Scope.new' do + + describe 'Garbage inputs' do + inputs = [ + [ nil ], + [ [ nil ] ], + [ { 'foo' => 'bar' } ], + [ { } ], + [ '' ], + [ "one", "one", "MSG to User", "4th arg" ], + ] + + inputs.each do |input| + it "validate_re(#{input.inspect}) should fail" do + expect { subject.call [input] }.to raise_error Puppet::ParseError + end + end + end + + describe 'Valid inputs' do + inputs = [ + [ '/full/path/to/something', '^/full' ], + [ '/full/path/to/something', 'full' ], + [ '/full/path/to/something', ['full', 'absent'] ], + [ '/full/path/to/something', ['full', 'absent'], 'Message to the user' ], + ] + + inputs.each do |input| + it "validate_re(#{input.inspect}) should not fail" do + expect { subject.call input }.not_to raise_error + end + end + end + describe "Valid inputs which should raise an exception without a message" do + # The intent here is to make sure valid inputs raise exceptions when they + # don't specify an error message to display. This is the behvior in + # 2.2.x and prior. + inputs = [ + [ "hello", [ "bye", "later", "adios" ] ], + [ "greetings", "salutations" ], + ] + + inputs.each do |input| + it "validate_re(#{input.inspect}) should fail" do + expect { subject.call input }.to raise_error /validate_re.*?does not match/ + end + end + end + describe "Nicer Error Messages" do + # The intent here is to make sure the function returns the 3rd argument + # in the exception thrown + inputs = [ + [ "hello", [ "bye", "later", "adios" ], "MSG to User" ], + [ "greetings", "salutations", "Error, greetings does not match salutations" ], + ] + + inputs.each do |input| + it "validate_re(#{input.inspect}) should fail" do + expect { subject.call input }.to raise_error /#{input[2]}/ + end + end + end + end +end -- cgit v1.2.3 From 31944c986345a135a3e68247c6d3e644ea369089 Mon Sep 17 00:00:00 2001 From: Jeff McCune Date: Mon, 12 Mar 2012 15:53:02 -0700 Subject: (#12357) Fix broken compatibility with Puppet 2.6 Without this patch, the previous change set to the validate_absolute_path() parser function contains Puppet 2.6 incompatible changes. stdlib 2.x is compatible with Puppet 2.6. These changes are a problem because we cannot introduce backwards incompatible changes in a minor release. This patch fixes the problem by back porting the implementation of the `Puppet::Util.absolute_path?` from 2.7.x to the function block itself. The function block tests to see if `Puppet::Util.absolute_path?` will respond and if not, falls back to the inline back ported implementation. The spec tests have been updated to simulate the behavior of Puppet 2.6 even when running with Puppet 2.7. --- .../functions/validate_absolute_path_spec.rb | 87 ++++++++++++---------- 1 file changed, 47 insertions(+), 40 deletions(-) (limited to 'spec/unit') diff --git a/spec/unit/puppet/parser/functions/validate_absolute_path_spec.rb b/spec/unit/puppet/parser/functions/validate_absolute_path_spec.rb index 2b44f50..1e0b5ac 100644 --- a/spec/unit/puppet/parser/functions/validate_absolute_path_spec.rb +++ b/spec/unit/puppet/parser/functions/validate_absolute_path_spec.rb @@ -5,34 +5,67 @@ describe Puppet::Parser::Functions.function(:validate_absolute_path) do Puppet::Parser::Functions.autoloader.loadall end - let(:scope) do - scope = Puppet::Parser::Scope.new - end - # The subject of these examplres is the method itself. subject do - scope.method :function_validate_absolute_path + Puppet::Parser::Scope.new.method :function_validate_absolute_path end - context 'Using Puppet::Parser::Scope.new' do + describe "Valid Paths" do + def self.valid_paths + %w{ + C:/ + C:\\ + C:\\WINDOWS\\System32 + C:/windows/system32 + X:/foo/bar + X:\\foo\\bar + /var/tmp + /var/lib/puppet + /var/opt/../lib/puppet + } + end - describe 'Garbage inputs' do - paths = [ + context "Without Puppet::Util.absolute_path? (e.g. Puppet <= 2.6)" do + before :each do + # The intent here is to mock Puppet to behave like Puppet 2.6 does. + # Puppet 2.6 does not have the absolute_path? method. This is only a + # convenience test, stdlib should be run with the Puppet 2.6.x in the + # $LOAD_PATH in addition to 2.7.x and master. + Puppet::Util.expects(:respond_to?).with(:absolute_path?).returns(false) + end + valid_paths.each do |path| + it "validate_absolute_path(#{path.inspect}) should not fail" do + expect { subject.call [path] }.not_to raise_error Puppet::ParseError + end + end + end + + context "Puppet without mocking" do + valid_paths.each do |path| + it "validate_absolute_path(#{path.inspect}) should not fail" do + expect { subject.call [path] }.not_to raise_error Puppet::ParseError + end + end + end + end + + describe 'Invalid paths' do + context 'Garbage inputs' do + [ nil, [ nil ], { 'foo' => 'bar' }, { }, '', - ] - - paths.each do |path| + ].each do |path| it "validate_absolute_path(#{path.inspect}) should fail" do expect { subject.call [path] }.to raise_error Puppet::ParseError end end end - describe 'relative paths' do - paths = %w{ + + context 'Relative paths' do + %w{ relative1 . .. @@ -40,37 +73,11 @@ describe Puppet::Parser::Functions.function(:validate_absolute_path) do ../foo etc/puppetlabs/puppet opt/puppet/bin - } - - paths.each do |path| + }.each do |path| it "validate_absolute_path(#{path.inspect}) should fail" do expect { subject.call [path] }.to raise_error Puppet::ParseError end end end - describe 'absolute paths' do - paths = %w{ - C:/ - C:\\ - C:\\WINDOWS\\System32 - C:/windows/system32 - X:/foo/bar - X:\\foo\\bar - /var/tmp - /var/lib/puppet - /var/opt/../lib/puppet - } - - paths = paths + [ - 'C:\\Program Files (x86)\\Puppet Labs\\Puppet Enterprise', - 'C:/Program Files (x86)/Puppet Labs/Puppet Enterprise', - ] - - paths.each do |path| - it "validate_absolute_path(#{path.inspect}) should not fail" do - expect { subject.call [path] }.not_to raise_error - end - end - end end end -- cgit v1.2.3