Please review pull request #230: Feature/2.7.x/symbolic modes for the file type opened by (daniel-pittman)
Description:
This adds a new feature, support for symbolic file modes, to Puppet. In
addition to being able to specify the octal mode, you can now use the same
symbolic mode style that chmod supports:
file { "/example": mode => "u=rw,go=r" }
This also supports relative file modes:
file { "/relative": mode = "u+w,go-wx" }Support is based on the common GNU and BSD symbolic modes of operation; you
specify a comma separated list of actions to take in each you can sit:The user (u), group (g), other (o), or all (a) of the permission map.
You can modify the ability to read (r), write (w), execute / search (x) on a
file or directory.You can also modify the sticky bit (t), or the setuid and setgid bits (s).
Finally, you can set conditional execute permissions (X), which will result in
the file having the execute bit if the target is a directory, or if the target
had any execute bit set. (eg: g+X will set x if the original was u=x,g=.)
- Opened: Tue Nov 22 21:32:35 UTC 2011
- Based on: puppetlabs:2.7.x (aef93cee8ae2133f9b2c571b23c490ef3405e3fe)
- Requested merge: daniel-pittman:feature/2.7.x/symbolic-modes-for-the-file-type (105c05b5ad0b49b4e61bb9ba81a8fa11baa6445f)
Diff follows:
diff --git a/lib/puppet/property.rb b/lib/puppet/property.rb
index ee8f3b4..9d2d98b 100644
--- a/lib/puppet/property.rb
+++ b/lib/puppet/property.rb
@@ -166,22 +166,43 @@ class Puppet::Property < Puppet::Parameter
raise "Puppet::Property#safe_insync? shouldn't be overridden; please override insync? instead" if sym == :safe_insync?
end
- # This method should be overridden by derived classes if necessary
+ # This method may be overridden by derived classes if necessary
# to provide extra logic to determine whether the property is in
- # sync.
+ # sync. In most cases, however, only `property_matches?` needs to be
+ # overridden to give the correct outcome - without reproducing all the array
+ # matching logic, etc, found here.
def insync?(is)
self.devfail "#{self.class.name}'s should is not array" unless @should.is_a?(Array)
# an empty array is analogous to no should values
return true if @should.empty?
- # Look for a matching value
- return (is == @should or is == @should.collect { |v| v.to_s }) if match_all?
+ # Look for a matching value, either for all the @should values, or any of
+ # them, depending on the configuration of this property.
+ if match_all? then
+ old = (is == @should or is == @should.collect { |v| v.to_s })
+ new = Array(is).zip(@should).all? {|is, want| property_matches?(is, want) }
- @should.each { |val| return true if is == val or is == val.to_s }
+ puts "old and new mismatch!" unless old == new
+ fail "old and new mismatch!" unless old == new
- # otherwise, return false
- false
+ # We need to pairwise compare the entries; this preserves the old
+ # behaviour while using the new pair comparison code.
+ return Array(is).zip(@should).all? {|is, want| property_matches?(is, want) }
+ else
+ return @should.any? {|want| property_matches?(is, want) }
+ end
+ end
+
+ # Compare the current and desired value of a property in a property-specific
+ # way. Invoked by `insync?`; this should be overridden if your property
+ # has a different comparison type but does not actually differentiate the
+ # overall insync? logic.
+ def property_matches?(current, desired)
+ # This preserves the older Puppet behaviour of doing raw and string
+ # equality comparisons for all equality. I am not clear this is globally
+ # desirable, but at least it is not a breaking change. --daniel 2011-11-11
+ current == desired or current == desired.to_s
end
# because the @should and @is vars might be in weird formats,
@@ -194,13 +215,10 @@ class Puppet::Property < Puppet::Parameter
# Send a log message.
def log(msg)
-
- Puppet::Util::Log.create(
-
- :level => resource[:loglevel],
+ Puppet::Util::Log.create(
+ :level => resource[:loglevel],
:message => msg,
-
- :source => self
+ :source => self
)
end
diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 20c774a..c11a2fd 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -9,11 +9,14 @@ require 'puppet/network/handler'
require 'puppet/util/diff'
require 'puppet/util/checksums'
require 'puppet/util/backups'
+require 'puppet/util/symbolic_file_mode'
Puppet::Type.newtype(:file) do
include Puppet::Util::MethodHelper
include Puppet::Util::Checksums
include Puppet::Util::Backups
+ include Puppet::Util::SymbolicFileMode
+
@doc = "Manages local files, including setting ownership and
permissions, creation of both files and directories, and
retrieving entire files from remote servers. As Puppet matures, it
@@ -729,7 +732,7 @@ Puppet::Type.newtype(:file) do
mode = self.should(:mode) # might be nil
umask = mode ? 000 : 022
- mode_int = mode ? mode.to_i(8) : nil
+ mode_int = mode ? symbolic_mode_to_int(mode, 0644) : nil
content_checksum = Puppet::Util.withumask(umask) { ::File.open(path, 'wb', mode_int ) { |f| write_content(f) } }
diff --git a/lib/puppet/type/file/ensure.rb b/lib/puppet/type/file/ensure.rb
index a846856..b7614f3 100755
--- a/lib/puppet/type/file/ensure.rb
+++ b/lib/puppet/type/file/ensure.rb
@@ -1,6 +1,10 @@
+
module Puppet
Puppet::Type.type(:file).ensurable do
require 'etc'
+ require 'puppet/util/symbolic_file_mode'
+ include Puppet::Util::SymbolicFileMode
+
desc <<-EOT
Whether to create files that don't currently exist.
Possible values are *absent*, *present*, *file*, and *directory*.
@@ -63,7 +67,7 @@ module Puppet
end
if mode
Puppet::Util.withumask(000) do
- Dir.mkdir(@resource[:path], mode.to_i(8))
+ Dir.mkdir(@resource[:path], symbolic_mode_to_int(mode, 755, true))
end
else
Dir.mkdir(@resource[:path])
diff --git a/lib/puppet/type/file/mode.rb b/lib/puppet/type/file/mode.rb
index 6313276..57963d6 100755
--- a/lib/puppet/type/file/mode.rb
+++ b/lib/puppet/type/file/mode.rb
@@ -3,6 +3,9 @@
# specifying the full mode.
module Puppet
Puppet::Type.type(:file).newproperty(:mode) do
+ require 'puppet/util/symbolic_file_mode'
+ include Puppet::Util::SymbolicFileMode
+
desc "Mode the file should be. Currently relatively limited:
you must specify the exact mode the file should be.
@@ -23,23 +26,32 @@ module Puppet
mode 644, and all of the directories will have mode 755."
validate do |value|
- if value.is_a?(String) and value !~ /^[0-7]+$/
- raise Puppet::Error, "File modes can only be octal numbers, not #{should.inspect}"
+ unless value.nil? or valid_symbolic_mode?(value)
+ raise Puppet::Error, "The file mode specification is invalid: #{value.inspect}"
end
end
- munge do |should|
- if should.is_a?(String)
- should.to_i(8).to_s(8)
- else
- should.to_s(8)
+ munge do |value|
+ return nil if value.nil?
+
+ unless valid_symbolic_mode?(value)
+ raise Puppet::Error, "The file mode specification is invalid: #{value.inspect}"
end
+
+ normalize_symbolic_mode(value)
+ end
+
+ def desired_mode_from_current(desired, current)
+ current = current.to_i(8) if current.is_a? String
+ is_a_directory = @resource.stat and @resource.stat.directory?
+ symbolic_mode_to_int(desired, current, is_a_directory)
end
# If we're a directory, we need to be executable for all cases
# that are readable. This should probably be selectable, but eh.
def dirmask(value)
- if FileTest.directory?(resource[:path])
+ orig = value
+ if FileTest.directory?(resource[:path]) and value =~ /^\d+$/ then
value = value.to_i(8)
value |= 0100 if value & 0400 != 0
value |= 010 if value & 040 != 0
@@ -61,6 +73,13 @@ module Puppet
end
end
+ def property_matches?(current, desired)
+ return false unless current
+ current_bits = normalize_symbolic_mode(current)
+ desired_bits = desired_mode_from_current(desired, current).to_s(8)
+ current_bits == desired_bits
+ end
+
# Ideally, dirmask'ing could be done at munge time, but we don't know if 'ensure'
# will eventually be a directory or something else. And unfortunately, that logic
# depends on the ensure, source, and target properties. So rather than duplicate
@@ -73,5 +92,21 @@ module Puppet
super
end
+
+ # Finally, when we sync the mode out we need to transform it; since we
+ # don't have access to the calculated "desired" value here, or the
+ # "current" value, only the "should" value we need to retrieve again.
+ def sync
+ current = @resource.stat ? @resource.stat.mode : 0644
+ set(desired_mode_from_current(@should[0], current).to_s(8))
+ end
+
+ def change_to_s(old_value, desired)
+ return super if desired =~ /^\d+$/
+
+ old_bits = normalize_symbolic_mode(old_value)
+ new_bits = normalize_symbolic_mode(desired_mode_from_current(desired, old_bits))
+ super(old_bits, new_bits) + " (#{desired})"
+ end
end
end
diff --git a/lib/puppet/util/symbolic_file_mode.rb b/lib/puppet/util/symbolic_file_mode.rb
new file mode 100644
index 0000000..de07b06
--- /dev/null
+++ b/lib/puppet/util/symbolic_file_mode.rb
@@ -0,0 +1,140 @@
+require 'puppet/util'
+
+module Puppet::Util::SymbolicFileMode
+ SetUIDBit = ReadBit = 4
+ SetGIDBit = WriteBit = 2
+ StickyBit = ExecBit = 1
+ SymbolicMode = { 'x' => ExecBit, 'w' => WriteBit, 'r' => ReadBit }
+ SymbolicSpecialToBit = {
+ 't' => { 'u' => StickyBit, 'g' => StickyBit, 'o' => StickyBit },
+ 's' => { 'u' => SetUIDBit, 'g' => SetGIDBit, 'o' => StickyBit }
+ }
+
+ def valid_symbolic_mode?(value)
+ value = normalize_symbolic_mode(value)
+ return true if value =~ /^0?[0-7]{1,4}$/
+ return true if value =~ /^([ugoa]*[-=+][-=+rstwxXugo]*)(,[ugoa]*[-=+][-=+rstwxXugo]*)*$/
+ return false
+ end
+
+ def normalize_symbolic_mode(value)
+ return nil if value.nil?
+
+ # We need to treat integers as octal numbers.
+ if value.is_a? Numeric then
+ return value.to_s(8)
+ elsif value =~ /^0?[0-7]{1,4}$/ then
+ return value.to_i(8).to_s(8)
+ else
+ return value
+ end
+ end
+
+ def symbolic_mode_to_int(modification, to_mode = 0, is_a_directory = false)
+ if modification.nil? or modification == '' then
+ raise Puppet::Error, "An empty mode string is illegal"
+ end
+ if modification =~ /^[0-7]+$/ then return modification.to_i(8) end
+ if modification =~ /^\d+$/ then
+ raise Puppet::Error, "Numeric modes must be in octal, not decimal!"
+ end
+
+ fail "non-numeric current mode (#{to_mode.inspect})" unless to_mode.is_a?(Numeric)
+
+ original_mode = {
+ 's' => (to_mode & 07000) >> 9,
+ 'u' => (to_mode & 00700) >> 6,
+ 'g' => (to_mode & 00070) >> 3,
+ 'o' => (to_mode & 00007) >> 0,
+ # Are there any execute bits set in the original mode?
+ 'any x?' => (to_mode & 00111) != 0
+ }
+ final_mode = {
+ 's' => original_mode['s'],
+ 'u' => original_mode['u'],
+ 'g' => original_mode['g'],
+ 'o' => original_mode['o'],
+ }
+
+ modification.split(/\s*,\s*/).each do |part|
+ begin
+ _, to, dsl = /^([ugoa]*)([-+=].*)$/.match(part).to_a
+ if dsl.nil? then raise Puppet::Error, 'Missing action' end
+ to = "a" unless to and to.length > 0
+
+ # We want a snapshot of the mode before we start messing with it to
+ # make actions like 'a-g' atomic. Various parts of the DSL refer to
+ # the original mode, the final mode, or the current snapshot of the
+ # mode, for added fun.
+ snapshot_mode = {}
+ final_mode.each {|k,v| snapshot_mode[k] = v }
+
+ to.gsub('a', 'ugo').split('').uniq.each do |who|
+ value = snapshot_mode[who]
+
+ action = ''
+ actions = {
+ '!' => lambda {|_,_| raise Puppet::Error, 'Missing operation (-, =, or +)' },
+ '=' => lambda {|m,v| m | v },
+ '+' => lambda {|m,v| m | v },
+ '-' => lambda {|m,v| m & ~v },
+ }
+
+ dsl.split('').each do |op|
+ case op
+ when /[-+=]/ then
+ action = ""
+ # Clear all bits, if this is assignment
+ value = 0 if op == '='
+
+ when /[ugo]/ then
+ value = actions[action].call(value, snapshot_mode[op])
+
+ when /[rwx]/ then
+ value = actions[action].call(value, SymbolicMode[op])
+
+ when 'X' then
+ # Only meaningful in combination with "set" actions.
+ if action != '+' then
+ raise Puppet::Error, "X only works with the '+' operator"
+ end
+
+ # As per the BSD manual page, set if this is a directory, or if
+ # any execute bit is set on the original (unmodified) mode.
+ # Ignored otherwise; it is "add if", not "add or clear".
+ if is_a_directory or original_mode['any x?'] then
+ value = actions[action].call(value, ExecBit)
+ end
+
+ when /[st]/ then
+ bit = SymbolicSpecialToBit[op][who] or fail "internal error"
+ final_mode['s'] = actions[action].call(final_mode['s'], bit)
+
+ else
+ raise Puppet::Error, 'Unknown operation'
+ end
+ end
+
+ # Now, assign back the value.
+ final_mode[who] = value
+ end
+
+ rescue Puppet::Error => e
+ if part.inspect != modification.inspect then
+ rest = " at #{part.inspect}"
+ else
+ rest = ''
+ end
+
+ raise Puppet::Error, "#{e}#{rest} in symbolic mode #{modification.inspect}"
+ end
+ end
+
+ result =
+ final_mode['s'] << 9 |
+ final_mode['u'] << 6 |
+ final_mode['g'] << 3 |
+ final_mode['o'] << 0
+ return result
+ end
+end
diff --git a/spec/unit/type/file/mode_spec.rb b/spec/unit/type/file/mode_spec.rb
index 8783a84..ef0db96 100755
--- a/spec/unit/type/file/mode_spec.rb
+++ b/spec/unit/type/file/mode_spec.rb
@@ -21,7 +21,7 @@ describe Puppet::Type.type(:file).attrclass(:mode) do
it "should not accept strings other than octal numbers" do
expect do
mode.value = 'readable please!'
- end.to raise_error(Puppet::Error, /File modes can only be octal numbers/)
+ end.to raise_error(Puppet::Error, /The file mode specification is invalid/)
end
end
diff --git a/spec/unit/util/symbolic_file_mode_spec.rb b/spec/unit/util/symbolic_file_mode_spec.rb
new file mode 100755
index 0000000..a6e9509
--- /dev/null
+++ b/spec/unit/util/symbolic_file_mode_spec.rb
@@ -0,0 +1,182 @@
+#!/usr/bin/env rspec
+require 'spec_helper'
+
+require 'puppet/util/symbolic_file_mode'
+
+describe Puppet::Util::SymbolicFileMode do
+ include Puppet::Util::SymbolicFileMode
+
+ describe "#valid_symbolic_mode?" do
+ %w{
+ 0 0000 1 1 7 11 77 111 777 11
+ 0 00000 01 01 07 011 077 0111 0777 011
+ = - + u= g= o= a= u+ g+ o+ a+ u- g- o- a- ugo= ugoa= ugugug=
+ a=,u=,g= a=,g+
+ =rwx +rwx -rwx
+ 644 go-w =rw,+X +X 755 u=rwx,go=rx u=rwx,go=u-w go= g=u-w
+ 755 0755
+ }.each do |input|
+ it "should treat #{input.inspect} as valid" do
+ valid_symbolic_mode?(input).should be_true
+ end
+ end
+
+ [0000, 0111, 0640, 0755, 0777].each do |input|
+ it "should treat the int #{input.to_s(8)} as value" do
+ valid_symbolic_mode?(input).should be_true
+ end
+ end
+
+ %w{
+ -1 -8 8 9 18 19 91 81 000000 11111 77777
+ 0-1 0-8 08 09 018 019 091 081 0000000 011111 077777
+ u g o a ug uo ua ag
+ }.each do |input|
+ it "should treat #{input.inspect} as invalid" do
+ valid_symbolic_mode?(input).should be_false
+ end
+ end
+ end
+
+ describe "#normalize_symbolic_mode" do
+ it "should turn an int into a string" do
+ normalize_symbolic_mode(12).should be_an_instance_of String
+ end
+
+ it "should not add a leading zero to an int" do
+ normalize_symbolic_mode(12).should_not =~ /^0/
+ end
+
+ it "should not add a leading zero to a string with a number" do
+ normalize_symbolic_mode("12").should_not =~ /^0/
+ end
+
+ it "should string a leading zero from a number" do
+ normalize_symbolic_mode("012").should == '12'
+ end
+
+ it "should pass through any other string" do
+ normalize_symbolic_mode("u=rwx").should == 'u=rwx'
+ end
+ end
+
+ describe "#symbolic_mode_to_int" do
+ {
+ "0654" => 00654,
+ "u+r" => 00400,
+ "g+r" => 00040,
+ "a+r" => 00444,
+ "a+x" => 00111,
+ "o+t" => 01000,
+ "o+t" => 01000,
+ ["o-t", 07777] => 06777,
+ ["a-x", 07777] => 07666,
+ ["a-rwx", 07777] => 07000,
+ ["ug-rwx", 07777] => 07007,
+ "a+x,ug-rwx" => 00001,
+ # My experimentation on debian suggests that +g ignores the sgid flag
+ ["a+g", 02060] => 02666,
+ # My experimentation on debian suggests that -g ignores the sgid flag
+ ["a-g", 02666] => 02000,
+ "g+x,a+g" => 00111,
+ # +X without exec set in the original should not set anything
+ "u+x,g+X" => 00100,
+ "g+X" => 00000,
+ # +X only refers to the original, *unmodified* file mode!
+ ["u+x,a+X", 0600] => 00700,
+ # Examples from the MacOS chmod(1) manpage
+ "0644" => 00644,
+ ["go-w", 07777] => 07755,
+ ["=rw,+X", 07777] => 07777,
+ ["=rw,+X", 07766] => 07777,
+ ["=rw,+X", 07676] => 07777,
+ ["=rw,+X", 07667] => 07777,
+ ["=rw,+X", 07666] => 07666,
+ "0755" => 00755,
+ "u=rwx,go=rx" => 00755,
+ "u=rwx,go=u-w" => 00755,
+ ["go=", 07777] => 07700,
+ ["g=u-w", 07777] => 07757,
+ ["g=u-w", 00700] => 00750,
+ ["g=u-w", 00600] => 00640,
+ ["g=u-w", 00500] => 00550,
+ ["g=u-w", 00400] => 00440,
+ ["g=u-w", 00300] => 00310,
+ ["g=u-w", 00200] => 00200,
+ ["g=u-w", 00100] => 00110,
+ ["g=u-w", 00000] => 00000,
+ # Cruel, but legal, use of the action set.
+ ["g=u+r-w", 0300] => 00350,
+ # Empty assignments.
+ ["u=", 00000] => 00000,
+ ["u=", 00600] => 00000,
+ ["ug=", 00000] => 00000,
+ ["ug=", 00600] => 00000,
+ ["ug=", 00660] => 00000,
+ ["ug=", 00666] => 00006,
+ ["=", 00000] => 00000,
+ ["=", 00666] => 00000,
+ ["+", 00000] => 00000,
+ ["+", 00124] => 00124,
+ ["-", 00000] => 00000,
+ ["-", 00124] => 00124,
+ }.each do |input, result|
+ from = input.is_a?(Array) ? "#{input[0]}, 0#{input[1].to_s(8)}" : input
+ it "should map #{from.inspect} to #{result.inspect}" do
+ symbolic_mode_to_int(*input).should == result
+ end
+ end
+
+ # Now, test some failure modes.
+ it "should fail if no mode is given" do
+ expect { symbolic_mode_to_int('') }.
+ to raise_error Puppet::Error, /empty mode string/
+ end
+
+ %w{u g o ug uo go ugo a uu u/x u!x u=r,,g=r}.each do |input|
+ it "should fail if no (valid) action is given: #{input.inspect}" do
+ expect { symbolic_mode_to_int(input) }.
+ to raise_error Puppet::Error, /Missing action/
+ end
+ end
+
+ %w{u+q u-rwF u+rw,g+rw,o+RW}.each do |input|
+ it "should fail with unknown op #{input.inspect}" do
+ expect { symbolic_mode_to_int(input) }.
+ to raise_error Puppet::Error, /Unknown operation/
+ end
+ end
+
+ it "should refuse to subtract the conditional execute op" do
+ expect { symbolic_mode_to_int("o-rwX") }.
+ to raise_error Puppet::Error, /only works with/
+ end
+
+ it "should refuse to set to the conditional execute op" do
+ expect { symbolic_mode_to_int("o=rwX") }.
+ to raise_error Puppet::Error, /only works with/
+ end
+
+ %w{8 08 9 09 118 119}.each do |input|
+ it "should fail for decimal modes: #{input.inspect}" do
+ expect { symbolic_mode_to_int(input) }.
+ to raise_error Puppet::Error, /octal/
+ end
+ end
+
+ it "should set the execute bit on a directory, without exec in original" do
+ symbolic_mode_to_int("u+X", 0444, true).to_s(8).should == "544"
+ symbolic_mode_to_int("g+X", 0444, true).to_s(8).should == "454"
+ symbolic_mode_to_int("o+X", 0444, true).to_s(8).should == "445"
+ symbolic_mode_to_int("+X", 0444, true).to_s(8).should == "555"
+ end
+
+ it "should set the execute bit on a file with exec in the original" do
+ symbolic_mode_to_int("+X", 0544).to_s(8).should == "555"
+ end
+
+ it "should not set the execute bit on a file without exec on the original even if set by earlier DSL" do
+ symbolic_mode_to_int("u+x,go+X", 0444).to_s(8).should == "544"
+ end
+ end
+end
-- 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.
