Please review pull request #360: (#11930) Fix more places where Puppet::Util.absolute_path? wasn't being used opened by (joshcooper)
Description:
Previously, the --modulepath parameter only accepted forward slashes on Windows, due to it using POSIX specific regex for path validation. While researching that, I found more code that had the same problem. I've updated them to use Puppet::Util.absolute_path? instead and some tests where appropriate.
- Opened: Sat Jan 21 04:42:42 UTC 2012
- Based on: puppetlabs:2.7.x (470a6646b4faa34874ab853355730dc0471b1246)
- Requested merge: joshcooper:ticket/2.7.x/11930-forward-slashes (89fb2188da0236eb8c61d4026d638acd14db3427)
Diff follows:
diff --git a/lib/puppet.rb b/lib/puppet.rb
index 0329e5b..0b13ccf 100644
--- a/lib/puppet.rb
+++ b/lib/puppet.rb
@@ -110,36 +110,6 @@ def self.parse_config
Puppet.settings.parse
end
- # XXX this should all be done using puppet objects, not using
- # normal mkdir
- def self.recmkdir(dir,mode = 0755)
- if FileTest.exist?(dir)
- return false
- else
- tmp = dir.sub(/^\//,'')
- path = [File::SEPARATOR]
- tmp.split(File::SEPARATOR).each { |dir|
- path.push dir
- if ! FileTest.exist?(File.join(path))
- begin
- Dir.mkdir(File.join(path), mode)
- rescue Errno::EACCES => detail
- Puppet.err detail.to_s
- return false
- rescue => detail
- Puppet.err "Could not create #{path}: #{detail}"
- return false
- end
- elsif FileTest.directory?(File.join(path))
- next
- else FileTest.exist?(File.join(path))
- raise Puppet::Error, "Cannot create #{dir}: basedir #{File.join(path)} is a file"
- end
- }
- return true
- end
- end
-
# Create a new type. Just proxy to the Type class. The mirroring query
# code was deprecated in 2008, but this is still in heavy use. I suppose
# this can count as a soft deprecation for the next dev. --daniel 2011-04-12
diff --git a/lib/puppet/file_serving/base.rb b/lib/puppet/file_serving/base.rb
index e936b5e..b14c51c 100644
--- a/lib/puppet/file_serving/base.rb
+++ b/lib/puppet/file_serving/base.rb
@@ -1,4 +1,5 @@
require 'puppet/file_serving'
+require 'puppet/util'
# The base class for Content and Metadata; provides common
# functionality like the behaviour around links.
@@ -49,7 +50,7 @@ def links=(value)
# Set our base path.
attr_reader :path
def path=(path)
- unless path =~ /^#{::File::SEPARATOR}/ or path =~ /^[a-z]:[\/\\]/i
+ unless Puppet::Util.absolute_path?(path)
raise ArgumentError.new("Paths must be fully qualified")
end
@@ -60,7 +61,7 @@ def path=(path)
# the file's path relative to the initial recursion point.
attr_reader :relative_path
def relative_path=(path)
- raise ArgumentError.new("Relative paths must not be fully qualified") if path =~ /^#{::File::SEPARATOR}/
+ raise ArgumentError.new("Relative paths must not be fully qualified") if Puppet::Util.absolute_path?(path)
@relative_path = path
end
diff --git a/lib/puppet/indirector/exec.rb b/lib/puppet/indirector/exec.rb
index e6325ad..63809e0 100644
--- a/lib/puppet/indirector/exec.rb
+++ b/lib/puppet/indirector/exec.rb
@@ -28,7 +28,7 @@ def query(name)
raise Puppet::DevError, "Exec commands must be an array" unless external_command.is_a?(Array)
# Make sure it's fully qualified.
- raise ArgumentError, "You must set the exec parameter to a fully qualified command" unless external_command[0][0] == File::SEPARATOR[0]
+ raise ArgumentError, "You must set the exec parameter to a fully qualified command" unless Puppet::Util.absolute_path?(external_command[0])
# Add our name to it.
external_command << name
diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
index 4fc314a..9a20244 100644
--- a/lib/puppet/node/environment.rb
+++ b/lib/puppet/node/environment.rb
@@ -1,3 +1,4 @@
+require 'puppet/util'
require 'puppet/util/cacher'
require 'monitor'
@@ -130,18 +131,14 @@ def to_zaml(z)
end
def validate_dirs(dirs)
- dir_regex = Puppet.features.microsoft_windows? ? /^[A-Za-z]:#{File::SEPARATOR}/ : /^#{File::SEPARATOR}/
- # REMIND: Dir.getwd on windows returns a path containing backslashes, which when joined with
- # dir containing forward slashes, breaks our regex matching. In general, path validation needs
- # to be refactored which will be handled in a future commit.
dirs.collect do |dir|
- if dir !~ dir_regex
+ unless Puppet::Util.absolute_path?(dir)
File.expand_path(File.join(Dir.getwd, dir))
else
dir
end
end.find_all do |p|
- p =~ dir_regex && FileTest.directory?(p)
+ Puppet::Util.absolute_path?(p) && FileTest.directory?(p)
end
end
diff --git a/lib/puppet/parameter/path.rb b/lib/puppet/parameter/path.rb
index 26e4933..11d0969 100644
--- a/lib/puppet/parameter/path.rb
+++ b/lib/puppet/parameter/path.rb
@@ -13,10 +13,6 @@ def validate_path(paths)
fail "#{name} only accepts a single path, not an array of paths"
end
- # We *always* support Unix path separators, as Win32 does now too.
- absolute = "[/#{::Regexp.quote(::File::SEPARATOR)}]"
- win32 = Puppet.features.microsoft_windows?
-
fail("#{name} must be a fully qualified path") unless Array(paths).all? {|path| absolute_path?(path)}
paths
diff --git a/lib/puppet/parser/functions/file.rb b/lib/puppet/parser/functions/file.rb
index 19ab9ba..d7bf5a1 100644
--- a/lib/puppet/parser/functions/file.rb
+++ b/lib/puppet/parser/functions/file.rb
@@ -7,7 +7,7 @@
can be passed, and the first file that exists will be read in.") do |vals|
ret = nil
vals.each do |file|
- unless file =~ /^#{File::SEPARATOR}/
+ unless Puppet::Util.absolute_path?(file)
raise Puppet::ParseError, "Files must be fully qualified"
end
if FileTest.exists?(file)
diff --git a/lib/puppet/parser/functions/generate.rb b/lib/puppet/parser/functions/generate.rb
index 91f7b22..226a5b7 100644
--- a/lib/puppet/parser/functions/generate.rb
+++ b/lib/puppet/parser/functions/generate.rb
@@ -11,9 +11,15 @@
generators, so all shell metacharacters are passed directly to
the generator.") do |args|
- raise Puppet::ParseError, "Generators must be fully qualified" unless args[0] =~ /^#{File::SEPARATOR}/
+ raise Puppet::ParseError, "Generators must be fully qualified" unless Puppet::Util.absolute_path?(args[0])
- unless args[0] =~ /^[-#{File::SEPARATOR}\w.]+$/
+ if Puppet.features.microsoft_windows?
+ valid = args[0] =~ /^[a-z]:(?:[\/\\][\w.-]+)+$/i
+ else
+ valid = args[0] =~ /^[-\/\w.]+$/
+ end
+
+ unless valid
raise Puppet::ParseError,
"Generators can only contain alphanumerics, file separators, and dashes"
end
diff --git a/lib/puppet/parser/type_loader.rb b/lib/puppet/parser/type_loader.rb
index 68def06..6cfe77c 100644
--- a/lib/puppet/parser/type_loader.rb
+++ b/lib/puppet/parser/type_loader.rb
@@ -80,8 +80,7 @@ def import(file, current_file = nil)
loaded_asts = []
files.each do |file|
- regex = Puppet.features.microsoft_windows? ? /^[A-Za-z]:#{File::SEPARATOR}/ : /^#{File::SEPARATOR}/
- unless file =~ regex
+ unless Puppet::Util.absolute_path?(file)
file = File.join(dir, file)
end
@loading_helper.do_once(file) do
diff --git a/lib/puppet/type/k5login.rb b/lib/puppet/type/k5login.rb
index 09114e9..e7fbac0 100644
--- a/lib/puppet/type/k5login.rb
+++ b/lib/puppet/type/k5login.rb
@@ -18,7 +18,7 @@
desc "The path to the `.k5login` file to manage. Must be fully qualified."
validate do |value|
- unless value =~ /^#{File::SEPARATOR}/
+ unless absolute_path?(value)
raise Puppet::Error, "File paths must be fully qualified."
end
end
diff --git a/lib/puppet/type/package.rb b/lib/puppet/type/package.rb
index d0bdfdb..ed797f5 100644
--- a/lib/puppet/type/package.rb
+++ b/lib/puppet/type/package.rb
@@ -313,10 +313,8 @@ def should_to_s(newvalue = @should)
end
}
- if source = self[:source]
- if source =~ /^#{File::SEPARATOR}/
- autos << source
- end
+ if source = self[:source] and absolute_path?(source)
+ autos << source
end
autos
end
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 39b5964..82e3b27 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -119,28 +119,6 @@ def self.proxy(klass, objmethod, *methods)
end
end
- # XXX this should all be done using puppet objects, not using
- # normal mkdir
- def self.recmkdir(dir,mode = 0755)
- if FileTest.exist?(dir)
- return false
- else
- tmp = dir.sub(/^\//,'')
- path = [File::SEPARATOR]
- tmp.split(File::SEPARATOR).each { |dir|
- path.push dir
- if ! FileTest.exist?(File.join(path))
- Dir.mkdir(File.join(path), mode)
- elsif FileTest.directory?(File.join(path))
- next
- else FileTest.exist?(File.join(path))
- raise "Cannot create #{dir}: basedir #{File.join(path)} is a file"
- end
- }
- return true
- end
- end
-
# Execute a given chunk of code with a new umask.
def self.withumask(mask)
cur = File.umask(mask)
diff --git a/lib/puppet/util/log/destinations.rb b/lib/puppet/util/log/destinations.rb
index 5aaf7c7..0ba036c 100644
--- a/lib/puppet/util/log/destinations.rb
+++ b/lib/puppet/util/log/destinations.rb
@@ -41,6 +41,8 @@ def handle(msg)
end
Puppet::Util::Log.newdesttype :file do
+ require 'fileutils'
+
def self.match?(obj)
Puppet::Util.absolute_path?(obj)
end
@@ -64,7 +66,7 @@ def initialize(path)
# We can't just use 'Config.use' here, because they've
# specified a "special" destination.
unless FileTest.exist?(File.dirname(path))
- Puppet.recmkdir(File.dirname(path))
+ FileUtils.mkdir_p(File.dirname(path), :mode => 0755)
Puppet.info "Creating log directory #{File.dirname(path)}"
end
@@ -167,7 +169,7 @@ def handle(msg)
@domain = Facter["domain"].value
@hostname += ".#{@domain}" if @domain
end
- if msg.source =~ /^\//
+ if Puppet::Util.absolute_path?(msg.source)
msg.source = @hostname + ":#{msg.source}"
elsif msg.source == "Puppet"
msg.source = @hostname + " #{msg.source}"
diff --git a/spec/unit/file_serving/base_spec.rb b/spec/unit/file_serving/base_spec.rb
index 17d5946..91e6962 100755
--- a/spec/unit/file_serving/base_spec.rb
+++ b/spec/unit/file_serving/base_spec.rb
@@ -4,8 +4,11 @@
require 'puppet/file_serving/base'
describe Puppet::FileServing::Base do
+ let(:path) { File.expand_path('/module/dir/file') }
+ let(:file) { File.expand_path('/my/file') }
+
it "should accept a path" do
- Puppet::FileServing::Base.new("/module/dir/file").path.should == "/module/dir/file"
+ Puppet::FileServing::Base.new(path).path.should == path
end
it "should require that paths be fully qualified" do
@@ -13,119 +16,115 @@
end
it "should allow specification of whether links should be managed" do
- Puppet::FileServing::Base.new("/module/dir/file", :links => :manage).links.should == :manage
+ Puppet::FileServing::Base.new(path, :links => :manage).links.should == :manage
end
it "should have a :source attribute" do
- file = Puppet::FileServing::Base.new("/module/dir/file")
+ file = Puppet::FileServing::Base.new(path)
file.should respond_to(:source)
file.should respond_to(:source=)
end
it "should consider :ignore links equivalent to :manage links" do
- Puppet::FileServing::Base.new("/module/dir/file", :links => :ignore).links.should == :manage
+ Puppet::FileServing::Base.new(path, :links => :ignore).links.should == :manage
end
it "should fail if :links is set to anything other than :manage, :follow, or :ignore" do
- proc { Puppet::FileServing::Base.new("/module/dir/file", :links => :else) }.should raise_error(ArgumentError)
+ proc { Puppet::FileServing::Base.new(path, :links => :else) }.should raise_error(ArgumentError)
end
it "should allow links values to be set as strings" do
- Puppet::FileServing::Base.new("/module/dir/file", :links => "follow").links.should == :follow
+ Puppet::FileServing::Base.new(path, :links => "follow").links.should == :follow
end
it "should default to :manage for :links" do
- Puppet::FileServing::Base.new("/module/dir/file").links.should == :manage
+ Puppet::FileServing::Base.new(path).links.should == :manage
end
it "should allow specification of a path" do
FileTest.stubs(:exists?).returns(true)
- Puppet::FileServing::Base.new("/module/dir/file", :path => "/my/file").path.should == "/my/file"
+ Puppet::FileServing::Base.new(path, :path => file).path.should == file
end
it "should allow specification of a relative path" do
FileTest.stubs(:exists?).returns(true)
- Puppet::FileServing::Base.new("/module/dir/file", :relative_path => "my/file").relative_path.should == "my/file"
+ Puppet::FileServing::Base.new(path, :relative_path => "my/file").relative_path.should == "my/file"
end
it "should have a means of determining if the file exists" do
- Puppet::FileServing::Base.new("/blah").should respond_to(:exist?)
+ Puppet::FileServing::Base.new(file).should respond_to(:exist?)
end
it "should correctly indicate if the file is present" do
- File.expects(:lstat).with("/my/file").returns(mock("stat"))
- Puppet::FileServing::Base.new("/my/file").exist?.should be_true
+ File.expects(:lstat).with(file).returns(mock("stat"))
+ Puppet::FileServing::Base.new(file).exist?.should be_true
end
it "should correctly indicate if the file is absent" do
- File.expects(:lstat).with("/my/file").raises RuntimeError
- Puppet::FileServing::Base.new("/my/file").exist?.should be_false
+ File.expects(:lstat).with(file).raises RuntimeError
+ Puppet::FileServing::Base.new(file).exist?.should be_false
end
describe "when setting the relative path" do
it "should require that the relative path be unqualified" do
- @file = Puppet::FileServing::Base.new("/module/dir/file")
+ @file = Puppet::FileServing::Base.new(path)
FileTest.stubs(:exists?).returns(true)
- proc { @file.relative_path = "/qualified/file" }.should raise_error(ArgumentError)
+ proc { @file.relative_path = File.expand_path("/qualified/file") }.should raise_error(ArgumentError)
end
end
describe "when determining the full file path" do
- before do
- @file = Puppet::FileServing::Base.new("/this/file")
- end
+ let(:path) { File.expand_path('/this/file') }
+ let(:file) { Puppet::FileServing::Base.new(path) }
it "should return the path if there is no relative path" do
- @file.full_path.should == "/this/file"
+ file.full_path.should == path
end
it "should return the path if the relative_path is set to ''" do
- @file.relative_path = ""
- @file.full_path.should == "/this/file"
+ file.relative_path = ""
+ file.full_path.should == path
end
it "should return the path if the relative_path is set to '.'" do
- @file.relative_path = "."
- @file.full_path.should == "/this/file"
+ file.relative_path = "."
+ file.full_path.should == path
end
it "should return the path joined with the relative path if there is a relative path and it is not set to '/' or ''" do
- @file.relative_path = "not/qualified"
- @file.full_path.should == "/this/file/not/qualified"
+ file.relative_path = "not/qualified"
+ file.full_path.should == File.join(path, "not/qualified")
end
it "should strip extra slashes" do
- file = Puppet::FileServing::Base.new("//this//file")
- file.full_path.should == "/this/file"
+ file = Puppet::FileServing::Base.new(File.join(File.expand_path('/'), "//this//file"))
+ file.full_path.should == path
end
end
describe "when stat'ing files" do
- before do
- @file = Puppet::FileServing::Base.new("/this/file")
- end
+ let(:path) { File.expand_path('/this/file') }
+ let(:file) { Puppet::FileServing::Base.new(path) }
it "should stat the file's full path" do
- @file.stubs(:full_path).returns("/this/file")
- File.expects(:lstat).with("/this/file").returns stub("stat", :ftype => "file")
- @file.stat
+ File.expects(:lstat).with(path).returns stub("stat", :ftype => "file")
+ file.stat
end
it "should fail if the file does not exist" do
- @file.stubs(:full_path).returns("/this/file")
- File.expects(:lstat).with("/this/file").raises(Errno::ENOENT)
- proc { @file.stat }.should raise_error(Errno::ENOENT)
+ File.expects(:lstat).with(path).raises(Errno::ENOENT)
+ proc { file.stat }.should raise_error(Errno::ENOENT)
end
it "should use :lstat if :links is set to :manage" do
- File.expects(:lstat).with("/this/file").returns stub("stat", :ftype => "file")
- @file.stat
+ File.expects(:lstat).with(path).returns stub("stat", :ftype => "file")
+ file.stat
end
it "should use :stat if :links is set to :follow" do
- File.expects(:stat).with("/this/file").returns stub("stat", :ftype => "file")
- @file.links = :follow
- @file.stat
+ File.expects(:stat).with(path).returns stub("stat", :ftype => "file")
+ file.links = :follow
+ file.stat
end
end
end
diff --git a/spec/unit/file_serving/content_spec.rb b/spec/unit/file_serving/content_spec.rb
index c1627c1..99295e1 100755
--- a/spec/unit/file_serving/content_spec.rb
+++ b/spec/unit/file_serving/content_spec.rb
@@ -4,6 +4,8 @@
require 'puppet/file_serving/content'
describe Puppet::FileServing::Content do
+ let(:path) { File.expand_path('/path') }
+
it "should should be a subclass of Base" do
Puppet::FileServing::Content.superclass.should equal(Puppet::FileServing::Base)
end
@@ -21,38 +23,38 @@
end
it "should have a method for collecting its attributes" do
- Puppet::FileServing::Content.new("/path").should respond_to(:collect)
+ Puppet::FileServing::Content.new(path).should respond_to(:collect)
end
it "should not retrieve and store its contents when its attributes are collected if the file is a normal file" do
- content = Puppet::FileServing::Content.new("/path")
+ content = Puppet::FileServing::Content.new(path)
result = "foo"
File.stubs(:lstat).returns(stub("stat", :ftype => "file"))
- File.expects(:read).with("/path").never
+ File.expects(:read).with(path).never
content.collect
content.instance_variable_get("@content").should be_nil
end
it "should not attempt to retrieve its contents if the file is a directory" do
- content = Puppet::FileServing::Content.new("/path")
+ content = Puppet::FileServing::Content.new(path)
result = "foo"
File.stubs(:lstat).returns(stub("stat", :ftype => "directory"))
- File.expects(:read).with("/path").never
+ File.expects(:read).with(path).never
content.collect
content.instance_variable_get("@content").should be_nil
end
it "should have a method for setting its content" do
- content = Puppet::FileServing::Content.new("/path")
+ content = Puppet::FileServing::Content.new(path)
content.should respond_to(:content=)
end
it "should make content available when set externally" do
- content = Puppet::FileServing::Content.new("/path")
+ content = Puppet::FileServing::Content.new(path)
content.content = "foo/bar"
content.content.should == "foo/bar"
end
@@ -71,47 +73,45 @@
end
it "should return an opened File when converted to raw" do
- content = Puppet::FileServing::Content.new("/path")
+ content = Puppet::FileServing::Content.new(path)
- File.expects(:new).with("/path","rb").returns :file
+ File.expects(:new).with(path, "rb").returns :file
content.to_raw.should == :file
end
end
describe Puppet::FileServing::Content, "when returning the contents" do
- before do
- @path = "/my/path"
- @content = Puppet::FileServing::Content.new(@path, :links => :follow)
- end
+ let(:path) { File.expand_path('/my/path') }
+ let(:content) { Puppet::FileServing::Content.new(path, :links => :follow) }
it "should fail if the file is a symlink and links are set to :manage" do
- @content.links = :manage
- File.expects(:lstat).with(@path).returns stub("stat", :ftype => "symlink")
- proc { @content.content }.should raise_error(ArgumentError)
+ content.links = :manage
+ File.expects(:lstat).with(path).returns stub("stat", :ftype => "symlink")
+ proc { content.content }.should raise_error(ArgumentError)
end
it "should fail if a path is not set" do
- proc { @content.content }.should raise_error(Errno::ENOENT)
+ proc { content.content }.should raise_error(Errno::ENOENT)
end
it "should raise Errno::ENOENT if the file is absent" do
- @content.path = "/there/is/absolutely/no/chance/that/this/path/exists"
- proc { @content.content }.should raise_error(Errno::ENOENT)
+ content.path = File.expand_path("/there/is/absolutely/no/chance/that/this/path/exists")
+ proc { content.content }.should raise_error(Errno::ENOENT)
end
it "should return the contents of the path if the file exists" do
- File.expects(:stat).with(@path).returns stub("stat", :ftype => "file")
- Puppet::Util.expects(:binread).with(@path).returns(:mycontent)
- @content.content.should == :mycontent
+ File.expects(:stat).with(path).returns stub("stat", :ftype => "file")
+ Puppet::Util.expects(:binread).with(path).returns(:mycontent)
+ content.content.should == :mycontent
end
it "should cache the returned contents" do
- File.expects(:stat).with(@path).returns stub("stat", :ftype => "file")
- Puppet::Util.expects(:binread).with(@path).returns(:mycontent)
- @content.content
+ File.expects(:stat).with(path).returns stub("stat", :ftype => "file")
+ Puppet::Util.expects(:binread).with(path).returns(:mycontent)
+ content.content
# The second run would throw a failure if the content weren't being cached.
- @content.content
+ content.content
end
end
diff --git a/spec/unit/file_serving/metadata_spec.rb b/spec/unit/file_serving/metadata_spec.rb
index 3842b05..b458c12 100755
--- a/spec/unit/file_serving/metadata_spec.rb
+++ b/spec/unit/file_serving/metadata_spec.rb
@@ -4,6 +4,8 @@
require 'puppet/file_serving/metadata'
describe Puppet::FileServing::Metadata do
+ let(:foobar) { File.expand_path('/foo/bar') }
+
it "should should be a subclass of Base" do
Puppet::FileServing::Metadata.superclass.should equal(Puppet::FileServing::Base)
end
@@ -17,15 +19,15 @@
end
it "should have a method that triggers attribute collection" do
- Puppet::FileServing::Metadata.new("/foo/bar").should respond_to(:collect)
+ Puppet::FileServing::Metadata.new(foobar).should respond_to(:collect)
end
it "should support pson serialization" do
- Puppet::FileServing::Metadata.new("/foo/bar").should respond_to(:to_pson)
+ Puppet::FileServing::Metadata.new(foobar).should respond_to(:to_pson)
end
it "should support to_pson_data_hash" do
- Puppet::FileServing::Metadata.new("/foo/bar").should respond_to(:to_pson_data_hash)
+ Puppet::FileServing::Metadata.new(foobar).should respond_to(:to_pson_data_hash)
end
it "should support pson deserialization" do
@@ -33,67 +35,66 @@
end
describe "when serializing" do
- before do
- @metadata = Puppet::FileServing::Metadata.new("/foo/bar")
- end
+ let(:metadata) { Puppet::FileServing::Metadata.new(foobar) }
+
it "should perform pson serialization by calling to_pson on it's pson_data_hash" do
pdh = mock "data hash"
pdh_as_pson = mock "data as pson"
- @metadata.expects(:to_pson_data_hash).returns pdh
+ metadata.expects(:to_pson_data_hash).returns pdh
pdh.expects(:to_pson).returns pdh_as_pson
- @metadata.to_pson.should == pdh_as_pson
+ metadata.to_pson.should == pdh_as_pson
end
it "should serialize as FileMetadata" do
- @metadata.to_pson_data_hash['document_type'].should == "FileMetadata"
+ metadata.to_pson_data_hash['document_type'].should == "FileMetadata"
end
it "the data should include the path, relative_path, links, owner, group, mode, checksum, type, and destination" do
- @metadata.to_pson_data_hash['data'].keys.sort.should == %w{ path relative_path links owner group mode checksum type destination }.sort
+ metadata.to_pson_data_hash['data'].keys.sort.should == %w{ path relative_path links owner group mode checksum type destination }.sort
end
it "should pass the path in the hash verbatum" do
- @metadata.to_pson_data_hash['data']['path'] == @metadata.path
+ metadata.to_pson_data_hash['data']['path'] == metadata.path
end
it "should pass the relative_path in the hash verbatum" do
- @metadata.to_pson_data_hash['data']['relative_path'] == @metadata.relative_path
+ metadata.to_pson_data_hash['data']['relative_path'] == metadata.relative_path
end
it "should pass the links in the hash verbatum" do
- @metadata.to_pson_data_hash['data']['links'] == @metadata.links
+ metadata.to_pson_data_hash['data']['links'] == metadata.links
end
it "should pass the path owner in the hash verbatum" do
- @metadata.to_pson_data_hash['data']['owner'] == @metadata.owner
+ metadata.to_pson_data_hash['data']['owner'] == metadata.owner
end
it "should pass the group in the hash verbatum" do
- @metadata.to_pson_data_hash['data']['group'] == @metadata.group
+ metadata.to_pson_data_hash['data']['group'] == metadata.group
end
it "should pass the mode in the hash verbatum" do
- @metadata.to_pson_data_hash['data']['mode'] == @metadata.mode
+ metadata.to_pson_data_hash['data']['mode'] == metadata.mode
end
it "should pass the ftype in the hash verbatum as the 'type'" do
- @metadata.to_pson_data_hash['data']['type'] == @metadata.ftype
+ metadata.to_pson_data_hash['data']['type'] == metadata.ftype
end
it "should pass the destination verbatum" do
- @metadata.to_pson_data_hash['data']['destination'] == @metadata.destination
+ metadata.to_pson_data_hash['data']['destination'] == metadata.destination
end
it "should pass the checksum in the hash as a nested hash" do
- @metadata.to_pson_data_hash['data']['checksum'].should be_is_a(Hash)
+ metadata.to_pson_data_hash['data']['checksum'].should be_is_a(Hash)
end
it "should pass the checksum_type in the hash verbatum as the checksum's type" do
- @metadata.to_pson_data_hash['data']['checksum']['type'] == @metadata.checksum_type
+ metadata.to_pson_data_hash['data']['checksum']['type'] == metadata.checksum_type
end
it "should pass the checksum in the hash verbatum as the checksum's value" do
- @metadata.to_pson_data_hash['data']['checksum']['value'] == @metadata.checksum
+ metadata.to_pson_data_hash['data']['checksum']['value'] == metadata.checksum
end
end
diff --git a/spec/unit/indirector/exec_spec.rb b/spec/unit/indirector/exec_spec.rb
index 87778cd..45a087a 100755
--- a/spec/unit/indirector/exec_spec.rb
+++ b/spec/unit/indirector/exec_spec.rb
@@ -14,15 +14,17 @@ module Testing; end
end
end
+ let(:path) { File.expand_path('/echo') }
+
before :each do
@searcher = @exec_class.new
- @searcher.command = ["/echo"]
+ @searcher.command = [path]
@request = stub 'request', :key => "foo"
end
it "should throw an exception if the command is not an array" do
- @searcher.command = "/usr/bin/echo"
+ @searcher.command = path
proc { @searcher.find(@request) }.should raise_error(Puppet::DevError)
end
@@ -32,22 +34,22 @@ module Testing; end
end
it "should execute the command with the object name as the only argument" do
- @searcher.expects(:execute).with(%w{/echo foo}, :combine => false)
+ @searcher.expects(:execute).with([path, 'foo'], :combine => false)
@searcher.find(@request)
end
it "should return the output of the script" do
- @searcher.expects(:execute).with(%w{/echo foo}, :combine => false).returns("whatever")
+ @searcher.expects(:execute).with([path, 'foo'], :combine => false).returns("whatever")
@searcher.find(@request).should == "whatever"
end
it "should return nil when the command produces no output" do
- @searcher.expects(:execute).with(%w{/echo foo}, :combine => false).returns(nil)
+ @searcher.expects(:execute).with([path, 'foo'], :combine => false).returns(nil)
@searcher.find(@request).should be_nil
end
it "should raise an exception if there's an execution failure" do
- @searcher.expects(:execute).with(%w{/echo foo}, :combine => false).raises(Puppet::ExecutionFailure.new("message"))
+ @searcher.expects(:execute).with([path, 'foo'], :combine => false).raises(Puppet::ExecutionFailure.new("message"))
lambda {@searcher.find(@request)}.should raise_exception(Puppet::Error, 'Failed to find foo via exec: message')
end
diff --git a/spec/unit/indirector/node/exec_spec.rb b/spec/unit/indirector/node/exec_spec.rb
index 1dbfcd1..79f3251 100755
--- a/spec/unit/indirector/node/exec_spec.rb
+++ b/spec/unit/indirector/node/exec_spec.rb
@@ -6,7 +6,7 @@
describe Puppet::Node::Exec do
before do
@indirection = mock 'indirection'
- Puppet.settings.stubs(:value).with(:external_nodes).returns("/echo")
+ Puppet.settings.stubs(:value).with(:external_nodes).returns(File.expand_path("/echo"))
@searcher = Puppet::Node::Exec.new
end
diff --git a/spec/unit/parser/functions/generate_spec.rb b/spec/unit/parser/functions/generate_spec.rb
index 6c90ae5..e508053 100755
--- a/spec/unit/parser/functions/generate_spec.rb
+++ b/spec/unit/parser/functions/generate_spec.rb
@@ -6,38 +6,80 @@
Puppet::Parser::Functions.autoloader.loadall
end
- before :each do
- @scope = Puppet::Parser::Scope.new
- end
+ let(:scope) { Puppet::Parser::Scope.new }
it "should exist" do
Puppet::Parser::Functions.function("generate").should == "function_generate"
end
- it "should accept a fully-qualified path as a command" do
- command = File::SEPARATOR + "command"
- Puppet::Util.expects(:execute).with([command]).returns("yay")
- lambda { @scope.function_generate([command]) }.should_not raise_error(Puppet::ParseError)
+ it " accept a fully-qualified path as a command" do
+ command = File.expand_path('/command/foo')
+ Dir.expects(:chdir).with(File.dirname(command)).returns("yay")
+ scope.function_generate([command]).should == "yay"
end
it "should not accept a relative path as a command" do
- command = "command"
- lambda { @scope.function_generate([command]) }.should raise_error(Puppet::ParseError)
+ lambda { scope.function_generate(["command"]) }.should raise_error(Puppet::ParseError)
+ end
+
+ it "should not accept a command containing illegal characters" do
+ lambda { scope.function_generate([File.expand_path('/##/command')]) }.should raise_error(Puppet::ParseError)
end
- # Really not sure how to implement this test, just sure it needs
- # to be implemented.
- it "should not accept a command containing illegal characters"
+ it "should not accept a command containing spaces" do
+ lambda { scope.function_generate([File.expand_path('/com mand')]) }.should raise_error(Puppet::ParseError)
+ end
it "should not accept a command containing '..'" do
- command = File::SEPARATOR + "command#{File::SEPARATOR}..#{File::SEPARATOR}"
- lambda { @scope.function_generate([command]) }.should raise_error(Puppet::ParseError)
+ command = File.expand_path("/command/../")
+ lambda { scope.function_generate([command]) }.should raise_error(Puppet::ParseError)
end
it "should execute the generate script with the correct working directory" do
- command = File::SEPARATOR + "command"
- Dir.expects(:chdir).with(File.dirname(command)).yields
- Puppet::Util.expects(:execute).with([command]).returns("yay")
- lambda { @scope.function_generate([command]) }.should_not raise_error(Puppet::ParseError)
+ command = File.expand_path("/command")
+ Dir.expects(:chdir).with(File.dirname(command)).returns("yay")
+ scope.function_generate([command]).should == 'yay'
+ end
+
+ describe "on Windows" do
+ before :each do
+ Puppet.features.stubs(:microsoft_windows?).returns(true)
+ end
+
+ it "should accept lower-case drive letters" do
+ command = 'd:/command/foo'
+ Dir.expects(:chdir).with(File.dirname(command)).returns("yay")
+ scope.function_generate([command]).should == 'yay'
+ end
+
+ it "should accept upper-case drive letters" do
+ command = 'D:/command/foo'
+ Dir.expects(:chdir).with(File.dirname(command)).returns("yay")
+ scope.function_generate([command]).should == 'yay'
+ end
+
+ it "should accept forward and backslashes in the path" do
+ command = 'D:\command/foo\bar'
+ Dir.expects(:chdir).with(File.dirname(command)).returns("yay")
+ scope.function_generate([command]).should == 'yay'
+ end
+
+ it "should reject colons when not part of the drive letter" do
+ lambda { scope.function_generate(['C:/com:mand']) }.should raise_error(Puppet::ParseError)
+ end
+
+ it "should reject root drives" do
+ lambda { scope.function_generate(['C:/']) }.should raise_error(Puppet::ParseError)
+ end
+ end
+
+ describe "on non-Windows" do
+ before :each do
+ Puppet.features.stubs(:microsoft_windows?).returns(false)
+ end
+
+ it "should reject backslashes" do
+ lambda { scope.function_generate(['/com\\mand']) }.should raise_error(Puppet::ParseError)
+ end
end
end
diff --git a/spec/unit/type/file_spec.rb b/spec/unit/type/file_spec.rb
index caedf4d..19ae22b 100755
--- a/spec/unit/type/file_spec.rb
+++ b/spec/unit/type/file_spec.rb
@@ -749,11 +749,13 @@
end
describe "#recurse_remote" do
+ let(:my) { File.expand_path('/my') }
+
before do
file[:source] = "puppet://foo/bar"
- @first = Puppet::FileServing::Metadata.new("/my", :relative_path => "first")
- @second = Puppet::FileServing::Metadata.new("/my", :relative_path => "second")
+ @first = Puppet::FileServing::Metadata.new(my, :relative_path => "first")
+ @second = Puppet::FileServing::Metadata.new(my, :relative_path => "second")
@first.stubs(:ftype).returns "directory"
@second.stubs(:ftype).returns "directory"
@@ -762,14 +764,14 @@
end
it "should pass its source to the :perform_recursion method" do
- data = "" :relative_path => "foobar")
+ data = "" :relative_path => "foobar")
file.expects(:perform_recursion).with("puppet://foo/bar").returns [data]
file.stubs(:newchild).returns @resource
file.recurse_remote({})
end
it "should not recurse when the remote file is not a directory" do
- data = "" :relative_path => ".")
+ data = "" :relative_path => ".")
data.stubs(:ftype).returns "file"
file.expects(:perform_recursion).with("puppet://foo/bar").returns [data]
file.expects(:newchild).never
@@ -850,7 +852,7 @@
describe "and :sourceselect is set to :first" do
it "should create file instances for the results for the first source to return any values" do
- data = "" :relative_path => "foobar")
+ data = "" :relative_path => "foobar")
file[:source] = sources.keys.map { |key| File.expand_path(key) }
file.expects(:perform_recursion).with(sources['/one']).returns nil
file.expects(:perform_recursion).with(sources['/two']).returns []
@@ -868,18 +870,19 @@
it "should return every found file that is not in a previous source" do
klass = Puppet::FileServing::Metadata
- file[:source] = %w{/one /two /three /four}.map {|f| File.expand_path(f) }
+ abs_path = %w{/one /two /three /four}.map {|f| File.expand_path(f) }
+ file[:source] = abs_path
file.stubs(:newchild).returns @resource
- _one_ = [klass.new("/one", :relative_path => "a")]
+ _one_ = [klass.new(abs_path[0], :relative_path => "a")]
file.expects(:perform_recursion).with(sources['/one']).returns one
file.expects(:newchild).with("a").returns @resource
- two = [klass.new("/two", :relative_path => "a"), klass.new("/two", :relative_path => "b")]
+ two = [klass.new(abs_path[1], :relative_path => "a"), klass.new(abs_path[1], :relative_path => "b")]
file.expects(:perform_recursion).with(sources['/two']).returns two
file.expects(:newchild).with("b").returns @resource
- three = [klass.new("/three", :relative_path => "a"), klass.new("/three", :relative_path => "c")]
+ three = [klass.new(abs_path[2], :relative_path => "a"), klass.new(abs_path[2], :relative_path => "c")]
file.expects(:perform_recursion).with(sources['/three']).returns three
file.expects(:newchild).with("c").returns @resource
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
