This patch changes the internal representation of a file's mode to a
string instead of an integer. This simplifies the problem of displaying
the value consistently throughout all of puppet.
Signed-off-by: Jesse Wolfe <jes5...@gmail.com>
---
lib/puppet/type/file.rb | 4 +-
lib/puppet/type/file/ensure.rb | 3 +-
lib/puppet/type/file/mode.rb | 55 +++++-------------------
lib/puppet/util/octal.rb | 9 ++++
spec/unit/transaction/resource_harness_spec.rb | 12 +++---
spec/unit/type/file/source_spec.rb | 4 +-
test/language/snippets.rb | 4 ++
test/ral/type/file.rb | 2 +-
8 files changed, 38 insertions(+), 55 deletions(-)
create mode 100644 lib/puppet/util/octal.rb
diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 3843212..cfefabc 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -8,6 +8,7 @@ require 'puppet/util/diff'
require 'puppet/util/checksums'
require 'puppet/network/client'
require 'puppet/util/backups'
+require 'puppet/util/octal'
Puppet::Type.newtype(:file) do
include Puppet::Util::MethodHelper
@@ -701,8 +702,9 @@ Puppet::Type.newtype(:file) do
mode = self.should(:mode) # might be nil
umask = mode ? 000 : 022
+ mode_int = mode ? Puppet::Util::Octal.integerForOctal(mode) : nil
- content_checksum = Puppet::Util.withumask(umask) { File.open(path, 'w',
mode) { |f| write_content(f) } }
+ content_checksum = Puppet::Util.withumask(umask) { File.open(path, 'w',
mode_int ) { |f| write_content(f) } }
# And put our new file in place
if use_temporary_file # This is only not true when our file is empty.
diff --git a/lib/puppet/type/file/ensure.rb b/lib/puppet/type/file/ensure.rb
index 967e06a..8307087 100755
--- a/lib/puppet/type/file/ensure.rb
+++ b/lib/puppet/type/file/ensure.rb
@@ -1,6 +1,7 @@
module Puppet
Puppet::Type.type(:file).ensurable do
require 'etc'
+ require 'puppet/util/octal'
desc "Whether to create files that don't currently exist.
Possible values are *absent*, *present*, *file*, and *directory*.
Specifying `present` will match any form of file existence, and
@@ -66,7 +67,7 @@ module Puppet
end
if mode
Puppet::Util.withumask(000) do
- Dir.mkdir(@resource[:path],mode)
+ Dir.mkdir(@resource[:path],
Puppet::Util::Octal.integerForOctal(mode))
end
else
Dir.mkdir(@resource[:path])
diff --git a/lib/puppet/type/file/mode.rb b/lib/puppet/type/file/mode.rb
index 1ce56c8..57d13b8 100755
--- a/lib/puppet/type/file/mode.rb
+++ b/lib/puppet/type/file/mode.rb
@@ -1,6 +1,7 @@
# Manage file modes. This state should support different formats
# for specification (e.g., u+rwx, or -0011), but for now only supports
# specifying the full mode.
+require 'puppet/util/octal'
module Puppet
Puppet::Type.type(:file).newproperty(:mode) do
require 'etc'
@@ -25,60 +26,26 @@ module Puppet
@event = :file_changed
- # Our modes are octal, so make sure they print correctly. Other
- # valid values are symbols, basically
- def is_to_s(currentvalue)
- case currentvalue
- when Integer
- return "%o" % currentvalue
- when Symbol
- return currentvalue
- else
- raise Puppet::DevError, "Invalid current value for mode:
#{currentvalue.inspect}"
- end
- end
-
- def should_to_s(newvalue = @should)
- case newvalue
- when Integer
- return "%o" % newvalue
- when Symbol
- return newvalue
- else
- raise Puppet::DevError, "Invalid 'should' value for mode:
#{newvalue.inspect}"
- end
- end
-
munge do |should|
- # this is pretty hackish, but i need to make sure the number is in
- # octal, yet the number can only be specified as a string right now
- value = should
- if value.is_a?(String)
- unless value =~ /^\d+$/
- raise Puppet::Error, "File modes can only be numbers, not
#{value.inspect}"
- end
- # Make sure our number looks like octal.
- unless value =~ /^0/
- value = "0#{value}"
- end
- old = value
- begin
- value = Integer(value)
- rescue ArgumentError => detail
- raise Puppet::DevError, "Could not convert #{old.inspect} to integer"
+ if should.is_a?(String)
+ unless should =~ /^[0-7]+$/
+ raise Puppet::Error, "File modes can only be octal numbers, not
#{should.inspect}"
end
+ Puppet::Util::Octal.octalForInteger(
Puppet::Util::Octal.integerForOctal( should ) )
+ else
+ Puppet::Util::Octal.octalForInteger( should )
end
-
- return value
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])
+ value = Puppet::Util::Octal.integerForOctal(value)
value |= 0100 if value & 0400 != 0
value |= 010 if value & 040 != 0
value |= 01 if value & 04 != 0
+ value = Puppet::Util::Octal.octalForInteger(value)
end
value
@@ -101,7 +68,7 @@ module Puppet
unless defined?(@fixed)
@should &&= @should.collect { |s| self.dirmask(s) }
end
- return stat.mode & 007777
+ return Puppet::Util::Octal.octalForInteger(stat.mode & 007777)
else
return :absent
end
@@ -111,7 +78,7 @@ module Puppet
mode = self.should
begin
- File.chmod(mode, @resource[:path])
+ File.chmod(Puppet::Util::Octal.integerForOctal(mode), @resource[:path])
rescue => detail
error = Puppet::Error.new("failed to chmod #...@resource[:path]}:
#{detail.message}")
error.set_backtrace detail.backtrace
diff --git a/lib/puppet/util/octal.rb b/lib/puppet/util/octal.rb
new file mode 100644
index 0000000..d820bd6
--- /dev/null
+++ b/lib/puppet/util/octal.rb
@@ -0,0 +1,9 @@
+module Puppet::Util::Octal
+ def self.integerForOctal( str )
+ Integer( "0" + str )
+ end
+
+ def self.octalForInteger( int )
+ "%o" % int
+ end
+end
diff --git a/spec/unit/transaction/resource_harness_spec.rb
b/spec/unit/transaction/resource_harness_spec.rb
index 255481a..f820402 100755
--- a/spec/unit/transaction/resource_harness_spec.rb
+++ b/spec/unit/transaction/resource_harness_spec.rb
@@ -46,7 +46,7 @@ describe Puppet::Transaction::ResourceHarness do
@harness.cache(@resource, :mode, "755")
@harness.copy_audited_parameters(@resource, {}).should == [:mode]
- @resource[:mode].should == 0755
+ @resource[:mode].should == "755"
end
it "should cache and log the current value if no cached values are
present" do
@@ -169,11 +169,11 @@ describe Puppet::Transaction::ResourceHarness do
@resource[:audit] = :mode
@harness.cache(@resource, :mode, "755")
@harness.changes_to_perform(@status, @resource)
- @resource[:mode].should == 0755
+ @resource[:mode].should == "755"
end
it "should mark changes created as a result of auditing as auditing
changes" do
- @current_state[:mode] = 0644
+ @current_state[:mode] = "644"
@resource[:audit] = :mode
@harness.cache(@resource, :mode, "755")
@harness.changes_to_perform(@status, @resource)[0].must be_auditing
@@ -222,12 +222,12 @@ describe Puppet::Transaction::ResourceHarness do
@resource[:mode] = "755"
@resource[:owner] = 0
@current_state[:ensure] = :present
- @current_state[:mode] = 0444
+ @current_state[:mode] = "444"
@current_state[:owner] = 50
mode = stub 'mode_change'
owner = stub 'owner_change'
-
Puppet::Transaction::Change.expects(:new).with(@resource.parameter(:mode),
0444).returns mode
+
Puppet::Transaction::Change.expects(:new).with(@resource.parameter(:mode),
"444").returns mode
Puppet::Transaction::Change.expects(:new).with(@resource.parameter(:owner),
50).returns owner
changes = @harness.changes_to_perform(@status, @resource)
@@ -242,7 +242,7 @@ describe Puppet::Transaction::ResourceHarness do
@resource[:ensure] = :present
@resource[:mode] = "755"
@current_state[:ensure] = :present
- @current_state[:mode] = 0755
+ @current_state[:mode] = "755"
@harness.changes_to_perform(@status, @resource).should == []
end
end
diff --git a/spec/unit/type/file/source_spec.rb
b/spec/unit/type/file/source_spec.rb
index a45a1f7..522ae1f 100755
--- a/spec/unit/type/file/source_spec.rb
+++ b/spec/unit/type/file/source_spec.rb
@@ -154,7 +154,7 @@ describe Puppet::Type.type(:file).attrclass(:source) do
@resource[:owner].must == 100
@resource[:group].must == 200
- @resource[:mode].must == 123
+ @resource[:mode].must == "173"
# Metadata calls it checksum, we call it content.
@resource[:content].must == @metadata.checksum
@@ -170,7 +170,7 @@ describe Puppet::Type.type(:file).attrclass(:source) do
@resource[:owner].must == 1
@resource[:group].must == 2
- @resource[:mode].must == 3
+ @resource[:mode].must == "3"
@resource[:content].should_not == @metadata.checksum
end
diff --git a/test/language/snippets.rb b/test/language/snippets.rb
index 51c5e23..d7763a9 100755
--- a/test/language/snippets.rb
+++ b/test/language/snippets.rb
@@ -7,6 +7,7 @@ require 'puppet/parser/parser'
require 'puppet/network/client'
require 'puppet/network/handler'
require 'puppettest'
+require 'puppet/util/octal'
class TestSnippets < Test::Unit::TestCase
include PuppetTest
@@ -37,6 +38,9 @@ class TestSnippets < Test::Unit::TestCase
end
def assert_mode_equal(mode, path)
+ if mode.is_a? Integer
+ mode = Puppet::Util::Octal.octalForInteger( mode )
+ end
unless file = @catalog.resource(:file, path)
raise "Could not find file #{path}"
end
diff --git a/test/ral/type/file.rb b/test/ral/type/file.rb
index 6322529..386c3ca 100755
--- a/test/ral/type/file.rb
+++ b/test/ral/type/file.rb
@@ -612,7 +612,7 @@ class TestFile < Test::Unit::TestCase
:mode => "0777"
)
- assert_equal(0777, file.should(:mode), "Mode did not get set correctly")
+ assert_equal("777", file.should(:mode), "Mode did not get set correctly")
assert_apply(file)
assert_equal(0777, File.stat(path).mode & 007777, "file mode is incorrect")
File.unlink(path)
--
1.7.0.4
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to puppet-...@googlegroups.com.
To unsubscribe from this group, send email to
puppet-dev+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
