Skip to content

Commit

Permalink
(PUP-2609) Update Metadata default permission
Browse files Browse the repository at this point in the history
Update Metadata's default permission model to ignore source permissions,
and remove unreachable code. Update spec tests for new behavior.
  • Loading branch information
Michael Smith committed Oct 24, 2014
1 parent bff2bcd commit 02bf973
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 62 deletions.
43 changes: 3 additions & 40 deletions lib/puppet/file_serving/metadata.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class MetaStat

def initialize(stat, source_permissions = nil)
@stat = stat
@source_permissions_ignore = source_permissions == :ignore
@source_permissions_ignore = (!source_permissions || source_permissions == :ignore)
end

def owner
Expand All @@ -53,52 +53,15 @@ class WindowsStat < MetaStat
def initialize(stat, path, source_permissions = nil)
super(stat, source_permissions)
@path = path
raise(ArgumentError, "Unsupported Windows source permissions option #{source_permissions}") unless @source_permissions_ignore
end

{ :owner => 'S-1-5-32-544',
:group => 'S-1-0-0',
:mode => 0644
}.each do |method, default_value|
define_method method do
return default_value if @source_permissions_ignore

# this code remains for when source_permissions is not set to :ignore
begin
Puppet::Util::Windows::Security.send("get_#{method}", @path) || default_value
rescue Puppet::Util::Windows::Error => detail
# Very carefully catch only this specific error that result from
# trying to read permissions on a symlinked file that is on a volume
# that does not support ACLs.
#
# Unfortunately readlink method will not return the target path when
# the given path is not the symlink.
#
# For instance, consider:
# symlink c:\link points to c:\target
# FileSystem.readlink('c:/link') returns 'c:/target'
# FileSystem.readlink('c:/link/file') will NOT return 'c:/target/file'
#
# Since detecting this up front is costly, since the path in question
# needs to be recursively split and tested at each depth in the path,
# we catch the standard error that will result from trying to read a
# file that doesn't have a DACL - 1336 is ERROR_INVALID_DACL
#
# Note that this affects any manually created symlinks as well as
# paths like puppet:///modules
return default_value if detail.code == 1336

# Also handle a VirtualBox bug where ERROR_INVALID_FUNCTION is
# returned when following a symlink to a volume that is not NTFS.
# It appears that the VirtualBox file system is not propagating
# the standard Win32 error code above like it should.
#
# Apologies to all who enter this code path at a later date
if detail.code == 1 && Facter.value(:virtual) == 'virtualbox'
return default_value
end

raise
end
return default_value
end
end
end
Expand Down
82 changes: 60 additions & 22 deletions spec/unit/file_serving/metadata_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,20 +98,6 @@
FileUtils.touch(path)
end

it "should set the owner to the file's current owner" do
metadata.owner.should == owner
end

it "should set the group to the file's current group" do
metadata.group.should == group
end

it "should set the mode to the file's masked mode" do
set_mode(33261, path)

metadata.mode.should == 0755
end

describe "checksumming" do
with_digest_algorithms do
before :each do
Expand Down Expand Up @@ -191,17 +177,13 @@
path = tmpfile('bar')
FileUtils.touch(path)

Puppet::Util::Windows::Security.stubs(:get_owner).with(path).raises(invalid_error)
Puppet::Util::Windows::Security.stubs(:get_group).with(path).raises(invalid_error)
Puppet::Util::Windows::Security.stubs(:get_mode).with(path).raises(invalid_error)
Puppet::Util::Windows::Security.stubs(:get_owner).with(path, :use).raises(invalid_error)
Puppet::Util::Windows::Security.stubs(:get_group).with(path, :use).raises(invalid_error)
Puppet::Util::Windows::Security.stubs(:get_mode).with(path, :use).raises(invalid_error)

stat = Puppet::FileSystem.stat(path)

win_stat = Puppet::FileServing::Metadata::WindowsStat.new(stat, path)

expect { win_stat.owner }.to raise_error(ArgumentError)
expect { win_stat.group }.to raise_error(ArgumentError)
expect { win_stat.mode }.to raise_error(ArgumentError)
expect { Puppet::FileServing::Metadata::WindowsStat.new(stat, path, :use) }.to raise_error("Unsupported Windows source permissions option use")
end
end

Expand Down Expand Up @@ -280,6 +262,34 @@
File::Stat.any_instance.stubs(:gid).returns group
end

describe "when collecting attributes when managing files" do
let(:metadata) do
data = described_class.new(path)
data.collect
data
end

let(:path) { tmpfile('file_serving_metadata') }

before :each do
FileUtils.touch(path)
end

it "should set the owner to the Process's current owner" do
metadata.owner.should == Process.euid
end

it "should set the group to the Process's current group" do
metadata.group.should == Process.egid
end

it "should set the mode to the default mode" do
set_mode(33261, path)

metadata.mode.should == 0644
end
end

it_should_behave_like "metadata collector"
it_should_behave_like "metadata collector symlinks"

Expand All @@ -298,6 +308,34 @@ def set_mode(mode, path)
Puppet::Util::Windows::Security.stubs(:get_group).returns group
end

describe "when collecting attributes when managing files" do
let(:metadata) do
data = described_class.new(path)
data.collect
data
end

let(:path) { tmpfile('file_serving_metadata') }

before :each do
FileUtils.touch(path)
end

it "should set the owner to the Process's current owner" do
metadata.owner.should == "S-1-5-32-544"
end

it "should set the group to the Process's current group" do
metadata.group.should == "S-1-0-0"
end

it "should set the mode to the default mode" do
set_mode(33261, path)

metadata.mode.should == 0644
end
end

it_should_behave_like "metadata collector"
it_should_behave_like "metadata collector symlinks" if Puppet.features.manages_symlinks?

Expand Down

0 comments on commit 02bf973

Please sign in to comment.