diff options
| -rw-r--r-- | lib/puppet/util/autoload.rb | 2 | ||||
| -rw-r--r-- | lib/puppet/util/config.rb | 278 | ||||
| -rwxr-xr-x | spec/unit/util/config.rb | 120 | ||||
| -rwxr-xr-x | test/util/config.rb | 108 |
4 files changed, 261 insertions, 247 deletions
diff --git a/lib/puppet/util/autoload.rb b/lib/puppet/util/autoload.rb index be9e14671..65cd3affb 100644 --- a/lib/puppet/util/autoload.rb +++ b/lib/puppet/util/autoload.rb @@ -150,5 +150,3 @@ class Puppet::Util::Autoload [module_lib_dirs, Puppet[:libdir], $:].flatten end end - -# $Id$ diff --git a/lib/puppet/util/config.rb b/lib/puppet/util/config.rb index 9ec777e99..27004cf41 100644 --- a/lib/puppet/util/config.rb +++ b/lib/puppet/util/config.rb @@ -16,39 +16,29 @@ class Puppet::Util::Config # Retrieve a config value def [](param) - param = symbolize(param) - - # Yay, recursion. - self.reparse() unless param == :filetimeout - - # Cache the returned values; this method was taking close to - # 10% of the compile time. - unless @returned[param] - if @config.include?(param) - if @config[param] - @returned[param] = @config[param].value - end - else - raise ArgumentError, "Undefined configuration parameter '%s'" % param - end - end - - return @returned[param] + value(param) end # Set a config value. This doesn't set the defaults, it sets the value itself. def []=(param, value) @@sync.synchronize do # yay, thread-safe param = symbolize(param) - unless @config.include?(param) + unless element = @config[param] raise ArgumentError, "Attempt to assign a value to unknown configuration parameter %s" % param.inspect end - unless @order.include?(param) - @order << param + if element.respond_to?(:munge) + value = element.munge(value) end - @config[param].value = value - @returned.clear + if element.respond_to?(:handle) + element.handle(value) + end + # Reset the name, so it's looked up again. + if param == :name + @name = nil + end + @values[:memory][param] = value + @values[:cache].clear end return value @@ -57,7 +47,7 @@ class Puppet::Util::Config # A simplified equality operator. def ==(other) self.each { |myname, myobj| - unless other[myname] == myobj.value + unless other[myname] == value(myname) return false end } @@ -112,21 +102,42 @@ class Puppet::Util::Config obj.clear end } - @returned.clear + @values.each do |name, values| + next if name == :cli and exceptcli + @values.delete(name) + end # Don't clear the 'used' in this case, since it's a config file reparse, # and we want to retain this info. unless exceptcli @used = [] end + + @name = nil end # This is mostly just used for testing. def clearused - @returned.clear + @values[:cache].clear @used = [] end + # Do variable interpolation on the value. + def convert(value) + return value unless value + return value unless value.is_a? String + newval = value.gsub(/\$(\w+)|\$\{(\w+)\}/) do |value| + varname = $2 || $1 + if pval = self.value(varname) + pval + else + raise Puppet::DevError, "Could not find value for %s" % parent + end + end + + return newval + end + # Return a value's description. def description(name) if obj = @config[symbolize(name)] @@ -137,29 +148,21 @@ class Puppet::Util::Config end def each - @order.each { |name| - if @config.include?(name) - yield name, @config[name] - else - raise Puppet::DevError, "%s is in the order but does not exist" % name - end + @config.each { |name, object| + yield name, object } end # Iterate over each section name. def eachsection yielded = [] - @order.each { |name| - if @config.include?(name) - section = @config[name].section - unless yielded.include? section - yield section - yielded << section - end - else - raise Puppet::DevError, "%s is in the order but does not exist" % name + @config.each do |name, object| + section = object.section + unless yielded.include? section + yield section + yielded << section end - } + end end # Return an object by name. @@ -178,16 +181,13 @@ class Puppet::Util::Config str = newstr bool = false end + str = str.intern if self.valid?(str) if self.boolean?(str) - self[str] = bool + @values[:cli][str] = bool else - self[str] = value + @values[:cli][str] = value end - - # Mark that this was set on the cli, so it's not overridden if the - # config gets reread. - @config[str.intern].setbycli = true else raise ArgumentError, "Invalid argument %s" % opt end @@ -206,12 +206,17 @@ class Puppet::Util::Config # Create a new config object def initialize - @order = [] @config = {} @shortnames = {} @created = [] - @returned = {} + @searchpath = nil + + # Keep track of set values. + @values = Hash.new { |hash, key| hash[key] = {} } + + # A central concept of a name. + @name = nil end # Return a given object's file metadata. @@ -245,6 +250,23 @@ class Puppet::Util::Config end end + # Figure out our name. + def name + unless @name + unless @config[:name] + return nil + end + searchpath.each do |source| + next if source == :name + break if @name = @values[source][:name] + end + unless @name + @name = convert(@config[:name].default).intern + end + end + @name + end + # Return all of the parameters associated with a given section. def params(section = nil) if section @@ -261,16 +283,18 @@ class Puppet::Util::Config # Parse the configuration file. def parse(file) - configmap = parse_file(file) + clear(true) - # We know we want the 'main' section - if main = configmap[:main] - set_parameter_hash(main) + parse_file(file).each do |area, values| + @values[area] = values end - # Otherwise, we only want our named section - if @config.include?(:name) and named = configmap[symbolize(self[:name])] - set_parameter_hash(named) + # We have to do it in the reverse of the search path, + # because multiple sections could set the same value. + searchpath.reverse.each do |source| + if meta = @values[source][:_meta] + set_metadata(meta) + end end end @@ -335,7 +359,8 @@ class Puppet::Util::Config # If the parameter is valid, then set it. if section == Puppet[:name] and @config.include?(var) - @config[var].value = value + #@config[var].value = value + @values[:main][var] = value end next end @@ -383,11 +408,12 @@ class Puppet::Util::Config hash[:parent] = self element = klass.new(hash) - @order << element.name - return element end + # This has to be private, because it doesn't add the elements to @config + private :newelement + # Iterate across all of the objects in a given section. def persection(section) section = symbolize(section) @@ -419,6 +445,17 @@ class Puppet::Util::Config end end + # The order in which to search for values. + def searchpath(environment = nil) + # Start with a stupid list. + [:cache, :cli, :memory, :name, :main] +# unless @searchpath +# @searchpath = [:cache, :memory, :cli, :name, :main].collect do |source| +# end +# end +# @searchpath + end + # Get a list of objects per section def sectionlist sectionlist = [] @@ -439,7 +476,7 @@ class Puppet::Util::Config done ||= Hash.new { |hash, key| hash[key] = {} } objects = [] persection(section) do |obj| - if @config[:mkusers] and @config[:mkusers].value + if @config[:mkusers] and value(:mkusers) [:owner, :group].each do |attr| type = nil if attr == :owner @@ -487,7 +524,7 @@ class Puppet::Util::Config end if obj.respond_to? :to_transportable - next if obj.value =~ /^\/dev/ + next if value(obj.name) =~ /^\/dev/ transobjects = obj.to_transportable transobjects = [transobjects] unless transobjects.is_a? Array transobjects.each do |trans| @@ -683,13 +720,41 @@ Generated on #{Time.now}. @config.has_key?(param) end - def value(param) + def value(param, environment = nil) param = symbolize(param) - if obj = @config[param] - obj.value - else - nil + + # Short circuit to nil for undefined parameters. + return nil unless @config.include?(param) + + # Yay, recursion. + self.reparse() unless param == :filetimeout + + # See if we can find it within our searchable list of values + searchpath(environment).each do |source| + # Modify the source as necessary. + source = case source + when :name: + self.name + else + source + end + + # Look for the value. + if @values[source].include?(param) + val = @values[source][param] + # Cache the value, because we do so many parameter lookups. + unless source == :cache + val = convert(val) + @values[:cache][param] = val + end + return val + end end + + # No normal source, so get the default and cache it + val = convert(@config[param].default) + @values[:cache][param] = val + return val end # Open a file with the appropriate user, group, and mode @@ -718,7 +783,7 @@ Generated on #{Time.now}. args << mode - File.open(obj.value, *args) do |file| + File.open(value(obj.name), *args) do |file| yield file end end @@ -897,12 +962,13 @@ Generated on #{Time.now}. self[param] = value end end + end - if meta = params[:_meta] - meta.each do |var, values| - values.each do |param, value| - @config[var].send(param.to_s + "=", value) - end + # Set file metadata. + def set_metadata(meta) + meta.each do |var, values| + values.each do |param, value| + @config[var].send(param.to_s + "=", value) end end end @@ -917,22 +983,6 @@ Generated on #{Time.now}. @value = nil end - # Do variable interpolation on the value. - def convert(value) - return value unless value - return value unless value.is_a? String - newval = value.gsub(/\$(\w+)|\$\{(\w+)\}/) do |value| - varname = $2 || $1 - if pval = @parent[varname] - pval - else - raise Puppet::DevError, "Could not find value for %s" % parent - end - end - - return newval - end - def desc=(value) @desc = value.gsub(/^\s*/, '') end @@ -1007,13 +1057,13 @@ Generated on #{Time.now}. str += "# The default value is '%s'.\n" % @default end - line = "%s = %s" % [@name, self.value] - # If the value has not been overridden, then print it out commented # and unconverted, so it's clear that that's the default and how it # works. - if defined? @value and ! @value.nil? - line = "%s = %s" % [@name, self.value] + value = @parent.value(self.name) + + if value != @default + line = "%s = %s" % [@name, value] else line = "# %s = %s" % [@name, @default] end @@ -1025,37 +1075,7 @@ Generated on #{Time.now}. # Retrieves the value, or if it's not set, retrieves the default. def value - retval = nil - if defined? @value and ! @value.nil? - retval = @value - elsif defined? @default - retval = @default - else - return nil - end - - if retval.is_a? String - return convert(retval) - else - return retval - end - end - - # Set the value. - def value=(value) - if respond_to?(:validate) - validate(value) - end - - if respond_to?(:munge) - @value = munge(value) - else - @value = value - end - - if respond_to?(:handle) - handle(@value) - end + @parent.value(self.name) end end @@ -1066,7 +1086,7 @@ Generated on #{Time.now}. def group if defined? @group - return convert(@group) + return @parent.convert(@group) else return nil end @@ -1074,7 +1094,7 @@ Generated on #{Time.now}. def owner if defined? @owner - return convert(@owner) + return @parent.convert(@owner) else return nil end @@ -1092,7 +1112,7 @@ Generated on #{Time.now}. # Return the appropriate type. def type - value = self.value + value = @parent.value(self.name) if @name.to_s =~ /dir/ return :directory elsif value.to_s =~ /\/$/ @@ -1111,7 +1131,7 @@ Generated on #{Time.now}. def to_transportable type = self.type return nil unless type - path = self.value.split(File::SEPARATOR) + path = @parent.value(self.name).split(File::SEPARATOR) path.shift # remove the leading nil objects = [] @@ -1124,7 +1144,7 @@ Generated on #{Time.now}. end [:mode].each { |var| if value = self.send(var) - # Don't both converting the mode, since the file type + # Don't bother converting the mode, since the file type # can handle it any old way. obj[var] = value end diff --git a/spec/unit/util/config.rb b/spec/unit/util/config.rb index ef0c00262..69a2268ab 100755 --- a/spec/unit/util/config.rb +++ b/spec/unit/util/config.rb @@ -15,7 +15,7 @@ describe Puppet::Util::Config, " when specifying defaults" do @config.setdefaults(:section, :myvalue => ["defaultval", "my description"]) end - it "should fail when a parameter has already been defined" do + it "should not allow duplicate parameter specifications" do @config.setdefaults(:section, :myvalue => ["a", "b"]) lambda { @config.setdefaults(:section, :myvalue => ["c", "d"]) }.should raise_error(ArgumentError) end @@ -85,12 +85,32 @@ describe Puppet::Util::Config, " when setting values" do #@config.set(:myval, "new value", :cli) #@config[:myval].should == "new value" end + + it "should call passed blocks when values are set" do + values = [] + @config.setdefaults(:section, :hooker => {:default => "yay", :desc => "boo", :hook => lambda { |v| values << v }}) + values.should == [] + + @config[:hooker] = "something" + values.should == %w{something} + end + + it "should munge values using the element-specific methods" do + @config[:bool] = "false" + @config[:bool].should == false + end + + it "should prefer cli values to values set in Ruby code" do + @config.handlearg("--myval", "cliarg") + @config[:myval] = "memarg" + @config[:myval].should == "cliarg" + end end describe Puppet::Util::Config, " when returning values" do before do @config = Puppet::Util::Config.new - @config.setdefaults :section, :one => ["ONE", "a"], :two => ["$one TWO", "b"], :three => ["$one $two THREE", "c"] + @config.setdefaults :section, :one => ["ONE", "a"], :two => ["$one TWO", "b"], :three => ["$one $two THREE", "c"], :four => ["$two $three FOUR", "d"] end it "should provide a mechanism for returning set values" do @@ -110,17 +130,39 @@ describe Puppet::Util::Config, " when returning values" do pending "Still no clear idea how this will work" end - it "should interpolate other parameters into returned parameter values" do + it "should interpolate default values for other parameters into returned parameter values" do @config[:one].should == "ONE" @config[:two].should == "ONE TWO" @config[:three].should == "ONE ONE TWO THREE" end + it "should interpolate default values that themselves need to be interpolated" do + @config[:four].should == "ONE TWO ONE ONE TWO THREE FOUR" + end + + it "should interpolate set values for other parameters into returned parameter values" do + @config[:one] = "on3" + @config[:two] = "$one tw0" + @config[:three] = "$one $two thr33" + @config[:four] = "$one $two $three f0ur" + @config[:one].should == "on3" + @config[:two].should == "on3 tw0" + @config[:three].should == "on3 on3 tw0 thr33" + @config[:four].should == "on3 on3 tw0 on3 on3 tw0 thr33 f0ur" + end + it "should not cache interpolated values such that stale information is returned" do @config[:two].should == "ONE TWO" @config[:one] = "one" @config[:two].should == "one TWO" end + + it "should have a name determined by the 'name' parameter" do + @config.setdefaults(:whatever, :name => ["something", "yayness"]) + @config.name.should == :something + @config[:name] = :other + @config.name.should == :other + end end describe Puppet::Util::Config, " when parsing its configuration" do @@ -142,6 +184,14 @@ describe Puppet::Util::Config, " when parsing its configuration" do @config[:two].should == "mval TWO" end + #484 - this should probably be in the regression area + it "should not throw an exception on unknown parameters" do + text = "[main]\nnosuchparam = mval\n" + file = "/some/file" + @config.expects(:read_file).with(file).returns(text) + lambda { @config.parse(file) }.should_not raise_error + end + it "should support an old parse method when per-executable configuration files still exist" do # I'm not going to bother testing this method. @config.should respond_to(:old_parse) @@ -223,27 +273,57 @@ describe Puppet::Util::Config, " when reparsing its configuration" do end it "should retain parameters set by cli when configuration files are reparsed" do - @config.handlearg("--one", "myval") - @config[:two] = "otherval" - end -end + @config.handlearg("--one", "clival") -describe Puppet::Util::Config, " when being used to manage the host machine" do - it "should provide a method that writes files with the correct modes" - - it "should provide a method that creates directories with the correct modes" - - it "should provide a method to declare what directories should exist" - - it "should provide a method to trigger enforcing of file modes on existing files and directories" + text = "[main]\none = on-disk\n" + file = mock 'file' + file.stubs(:file).returns("/test/file") + @config.stubs(:read_file).with(file).returns(text) + @config.parse(file) - it "should provide a method to convert the file mode enforcement into a Puppet manifest" + @config[:one].should == "clival" + end - it "should provide an option to create needed users and groups" + it "should remove in-memory values that are no longer set in the file" do + # Init the value + text = "[main]\none = disk-init\n" + file = mock 'file' + file.stubs(:changed?).returns(true) + file.stubs(:file).returns("/test/file") + @config.expects(:read_file).with(file).returns(text) + @config.parse(file) + @config[:one].should == "disk-init" - it "should provide a method to print out the current configuration" + # Now replace the value + text = "[main]\ntwo = disk-replace\n" + @config.expects(:read_file).with(file).returns(text) + @config.parse(file) + #@config.reparse - it "should be able to provide all of its parameters in a format compatible with GetOpt::Long" + # The originally-overridden value should be replaced with the default + @config[:one].should == "ONE" - it "should not attempt to manage files within /dev" + # and we should now have the new value in memory + @config[:two].should == "disk-replace" + end end + +#describe Puppet::Util::Config, " when being used to manage the host machine" do +# it "should provide a method that writes files with the correct modes" +# +# it "should provide a method that creates directories with the correct modes" +# +# it "should provide a method to declare what directories should exist" +# +# it "should provide a method to trigger enforcing of file modes on existing files and directories" +# +# it "should provide a method to convert the file mode enforcement into a Puppet manifest" +# +# it "should provide an option to create needed users and groups" +# +# it "should provide a method to print out the current configuration" +# +# it "should be able to provide all of its parameters in a format compatible with GetOpt::Long" +# +# it "should not attempt to manage files within /dev" +#end diff --git a/test/util/config.rb b/test/util/config.rb index b75f8e73d..2e9105010 100755 --- a/test/util/config.rb +++ b/test/util/config.rb @@ -226,7 +226,7 @@ class TestConfig < Test::Unit::TestCase @config.clear } - assert_equal(default, @config[:yayness]) + assert_equal(default, @config[:yayness], "'clear' did not remove old values") assert_nothing_raised { @config[:yayness] = "not default" @@ -375,27 +375,26 @@ yay = /a/path :shoe => ["puppet", "b"] # our default name ) @config.handlearg("--cliparam", "changed") - @config.expects(:parse_file).returns(result).times(2) + @config.stubs(:parse_file).returns(result) # First do it with our name being 'puppet' assert_nothing_raised("Could not handle parse results") do @config.parse(tempfile) end - assert_logged(:warning, /unknown configuration parameter bad/, "Did not log invalid config param") - + assert_equal(:puppet, @config.name, "Did not get correct name") assert_equal("main", @config[:main], "Did not get main value") assert_equal("puppet", @config[:other], "Did not get name value") assert_equal("changed", @config[:cliparam], "CLI values were overridden by config") # Now switch names and make sure the parsing switches, too. @config.clear(true) - @config[:name] = :puppetd assert_nothing_raised("Could not handle parse results") do @config.parse(tempfile) end - assert_logged(:warning, /unknown configuration parameter bad/, "Did not log invalid config param") + @config[:name] = :puppetd + assert_equal(:puppetd, @config.name, "Did not get correct name") assert_equal("main", @config[:main], "Did not get main value") assert_equal("puppetd", @config[:other], "Did not get name value") assert_equal("changed", @config[:cliparam], "CLI values were overridden by config") @@ -495,7 +494,7 @@ yay = /a/path # Get the actual object, so we can verify metadata file = @config.element(:file) - assert_equal("/other", file.value, "Did not get correct value") + assert_equal("/other", @config[:file], "Did not get correct value") assert_equal("you", file.owner, "Did not pass on user") assert_equal("you", file.group, "Did not pass on group") assert_equal("644", file.mode, "Did not pass on mode") @@ -859,37 +858,6 @@ yay = /a/path end end - def test_booleans_and_integers - config = mkconfig - config.setdefaults(:mysection, - :booltest => [false, "yay"], - :inttest => [14, "yay"] - ) - - file = tempfile() - - File.open(file, "w") do |f| - f.puts %{ -[main] -booltest = true -inttest = 27 -} - end - - assert_nothing_raised { - config.parse(file) - } - - assert_equal(true, config[:booltest], "Boolean was not converted") - assert_equal(27, config[:inttest], "Integer was not converted") - - # Now make sure that they get converted through handlearg - config.handlearg("--inttest", "true") - assert_equal(true, config[:inttest], "Boolean was not converted") - config.handlearg("--no-booltest", "false") - assert_equal(false, config[:booltest], "Boolean was not converted") - end - # Make sure that tags are ignored when configuring def test_configs_ignore_tags config = mkconfig @@ -945,15 +913,10 @@ inttest = 27 ["$server/yayness.conf", file] ].each do |ary| value, type = ary - elem = nil assert_nothing_raised { - elem = config.newelement( - :name => value, - :default => value, - :desc => name.to_s, - :section => :yayness - ) + config.setdefaults(:yayness, value => { :default => value, :desc => name.to_s}) } + elem = config.element(value) assert_instance_of(type, elem, "%s got created as wrong type" % value.inspect) @@ -1079,21 +1042,17 @@ inttest = 27 config = mkconfig() testing = nil - elem = nil assert_nothing_raised do - elem = config.newelement :default => "yay", - :name => :blocktest, - :desc => "boo", - :section => :test, - :hook => proc { |value| testing = value } + config.setdefaults :test, :blocktest => {:default => "yay", :desc => "boo", :hook => proc { |value| testing = value }} end + elem = config.element(:blocktest) assert_nothing_raised do assert_equal("yay", elem.value) end assert_nothing_raised do - elem.value = "yaytest" + config[:blocktest] = "yaytest" end assert_nothing_raised do @@ -1102,7 +1061,7 @@ inttest = 27 assert_equal("yaytest", testing) assert_nothing_raised do - elem.value = "another" + config[:blocktest] = "another" end assert_nothing_raised do @@ -1196,9 +1155,6 @@ inttest = 27 assert_nothing_raised("Unknown parameter threw an exception") do config.parse(file) end - - assert(@logs.detect { |l| l.message =~ /unknown configuration/ and l.level == :warning }, - "Did not generate warning message") end def test_multiple_interpolations @@ -1224,46 +1180,6 @@ inttest = 27 "Did not interpolate curlied variables") end - # Discovered from #734 - def test_set_parameter_hash - @config.setdefaults(:section, - :unchanged => ["unval", "yay"], - :normal => ["normalval", "yay"], - :cliparam => ["clival", "yay"], - :file => ["/my/file", "yay"] - ) - - # Set the cli param using the cli method - @config.handlearg("--cliparam", "other") - - # Make sure missing params just throw warnings, not errors - assert_nothing_raised("Could not call set_parameter_hash with an invalid option") do - @config.send(:set_parameter_hash, :missing => "something") - end - - # Make sure normal values get set - assert_nothing_raised("Could not call set_parameter_hash with a normal value") do - @config.send(:set_parameter_hash, :normal => "abnormal") - end - assert_equal("abnormal", @config[:normal], "Value did not get set") - - # Make sure cli-set values don't get overridden - assert_nothing_raised("Could not call set_parameter_hash with an override of a cli value") do - @config.send(:set_parameter_hash, :cliparam => "something else") - end - assert_equal("other", @config[:cliparam], "CLI value was overridden by config value") - - # Make sure the meta stuff works - assert_nothing_raised("Could not call set_parameter_hash with meta info") do - @config.send(:set_parameter_hash, :file => "/other/file", :_meta => {:file => {:mode => "0755"}}) - end - assert_equal("/other/file", @config[:file], "value with meta info was overridden by config value") - assert_equal("0755", @config.element(:file).mode, "Did not set mode from meta info") - - # And make sure other params are unchanged - assert_equal("unval", @config[:unchanged], "Unchanged value has somehow changed") - end - # Test to make sure that we can set and get a short name def test_celement_short_name element = nil |