Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

(PUP-2609) Windows error on use source permissions #3226

Merged
merged 2 commits into from Oct 27, 2014
Merged

(PUP-2609) Windows error on use source permissions #3226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bff2bcd
(PUP-2609) Make copying permissions on Win an error
Oct 16, 2014
02bf973
(PUP-2609) Update Metadata default permission
Oct 22, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 3 additions & 40 deletions lib/puppet/file_serving/metadata.rb
View file
Open in desktop
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
7 changes: 3 additions & 4 deletions lib/puppet/type/file/source.rb
View file
Open in desktop
Expand Up @@ -127,11 +127,10 @@ def copy_source_values
if [:use, :use_when_creating].include?(resource[:source_permissions]) &&
(resource[:owner] == nil || resource[:group] == nil || resource[:mode] == nil)

warning = "Copying %s from the source" <<
" file on Windows is deprecated;" <<
err_msg = "Copying %s from the source" <<
" file on Windows is not supported;" <<
" use source_permissions => ignore."
Puppet.deprecation_warning(warning % 'owner/mode/group')
resource.debug(warning % metadata_method.to_s)
self.fail Puppet::Error, err_msg % 'owner/mode/group'
end
# But never try to copy remote owner/group on Windows
next if [:owner, :group].include?(metadata_method) && !local?
Expand Down
132 changes: 24 additions & 108 deletions spec/integration/type/file_spec.rb
View file
Open in desktop
Expand Up @@ -949,23 +949,25 @@ def build_path(dir)
describe "when sourcing" do
let(:source) { tmpfile_with_contents("source_default_values", "yay") }

it "should apply the source metadata values" do
set_mode(0770, source)
describe "on POSIX systems", :if => Puppet.features.posix? do
it "should apply the source metadata values" do
set_mode(0770, source)

file = described_class.new(
:path => path,
:ensure => :file,
:source => source,
:source_permissions => :use,
:backup => false
)
file = described_class.new(
:path => path,
:ensure => :file,
:source => source,
:source_permissions => :use,
:backup => false
)

catalog.add_resource file
catalog.apply
catalog.add_resource file
catalog.apply

get_owner(path).should == get_owner(source)
get_group(path).should == get_group(source)
(get_mode(path) & 07777).should == 0770
get_owner(path).should == get_owner(source)
get_group(path).should == get_group(source)
(get_mode(path) & 07777).should == 0770
end
end

it "should override the default metadata values" do
Expand All @@ -975,7 +977,6 @@ def build_path(dir)
:path => path,
:ensure => :file,
:source => source,
:source_permissions => :use,
:backup => false,
:mode => '0440'
)
Expand Down Expand Up @@ -1019,26 +1020,6 @@ def expects_at_least_one_inherited_system_ace_grants_full_access(path)
expects_at_least_one_inherited_ace_grants_full_access(path, @sids[:system])
end

it "should provide valid default values when ACLs are not supported" do
Puppet::Util::Windows::Security.stubs(:supports_acl?).returns(false)
Puppet::Util::Windows::Security.stubs(:supports_acl?).with(source).returns false

file = described_class.new(
:path => path,
:ensure => :file,
:source => source,
:backup => false,
:source_permissions => :use
)

catalog.add_resource file
catalog.apply

get_owner(path).should =~ /^S\-1\-5\-.*$/
get_group(path).should =~ /^S\-1\-0\-0.*$/
get_mode(path).should == 0644
end

describe "when processing SYSTEM ACEs" do
before do
@sids = {
Expand All @@ -1063,56 +1044,26 @@ def expects_at_least_one_inherited_system_ace_grants_full_access(path)
catalog.add_resource @file
end

describe "when source permissions are ignored" do
it "preserves the inherited SYSTEM ACE" do
catalog.apply

expects_at_least_one_inherited_system_ace_grants_full_access(path)
end
end

describe "when permissions are insync?" do
before :each do
@file[:source_permissions] = :use
end

it "preserves the explicit SYSTEM ACE" do
FileUtils.touch(path)

sd = Puppet::Util::Windows::Security.get_security_descriptor(path)
sd.protect = true
sd.owner = @sids[:none]
sd.group = @sids[:none]
Puppet::Util::Windows::Security.set_security_descriptor(source, sd)
Puppet::Util::Windows::Security.set_security_descriptor(path, sd)

catalog.apply

expects_system_granted_full_access_explicitly(path)
end
end

describe "when permissions are not insync?" do
before :each do
@file[:owner] = 'None'
@file[:group] = 'None'
@file[:source_permissions] = :use
end

it "replaces inherited SYSTEM ACEs with an uninherited one for an existing file" do
it "preserves the inherited SYSTEM ACE for an existing file" do
FileUtils.touch(path)

expects_at_least_one_inherited_system_ace_grants_full_access(path)

catalog.apply

expects_system_granted_full_access_explicitly(path)
expects_at_least_one_inherited_system_ace_grants_full_access(path)
end

it "replaces inherited SYSTEM ACEs for a new file with an uninherited one" do
it "applies the inherited SYSTEM ACEs for a new file" do
catalog.apply

expects_system_granted_full_access_explicitly(path)
expects_at_least_one_inherited_system_ace_grants_full_access(path)
end
end

Expand All @@ -1121,7 +1072,6 @@ def expects_at_least_one_inherited_system_ace_grants_full_access(path)
@file[:owner] = @sids[:users]
@file[:group] = @sids[:system]
@file[:mode] = '0644'
@file[:source_permissions] = :use

catalog.apply
end
Expand Down Expand Up @@ -1182,60 +1132,26 @@ def grant_everyone_full_access(path)
grant_everyone_full_access(dir)
end

describe "when source permissions are ignored" do
it "preserves the inherited SYSTEM ACE" do
catalog.apply

expects_at_least_one_inherited_system_ace_grants_full_access(dir)
end
end

describe "when permissions are insync?" do
before :each do
@directory[:source_permissions] = :use
end

it "preserves the explicit SYSTEM ACE" do
Dir.mkdir(dir)

source_dir = tmpdir('source_dir')
@directory[:source] = source_dir

sd = Puppet::Util::Windows::Security.get_security_descriptor(source_dir)
sd.protect = true
sd.owner = @sids[:none]
sd.group = @sids[:none]
Puppet::Util::Windows::Security.set_security_descriptor(source_dir, sd)
Puppet::Util::Windows::Security.set_security_descriptor(dir, sd)

catalog.apply

expects_system_granted_full_access_explicitly(dir)
end
end

describe "when permissions are not insync?" do
before :each do
@directory[:owner] = 'None'
@directory[:group] = 'None'
@directory[:mode] = '0444'
@directory[:source_permissions] = :use
end

it "replaces inherited SYSTEM ACEs with an uninherited one for an existing directory" do
it "preserves the inherited SYSTEM ACEs for an existing directory" do
FileUtils.mkdir(dir)

expects_at_least_one_inherited_system_ace_grants_full_access(dir)

catalog.apply

expects_system_granted_full_access_explicitly(dir)
expects_at_least_one_inherited_system_ace_grants_full_access(dir)
end

it "replaces inherited SYSTEM ACEs with an uninherited one for an existing directory" do
it "applies the inherited SYSTEM ACEs for a new directory" do
catalog.apply

expects_system_granted_full_access_explicitly(dir)
expects_at_least_one_inherited_system_ace_grants_full_access(dir)
end

describe "created with SYSTEM as the group" do
Expand Down