Revamp the yumrepo type to deal with repositories defined anywhere in yum's

config files. Adds a generic module Puppet::IniConfig for parsing ini-style
files


git-svn-id: https://reductivelabs.com/svn/puppet/trunk@1096 980ebf18-57e1-0310-9a29-db15c13687c0

5 files changed, 500 insertions, 101 deletions

diff --git a/lib/puppet/inifile.rb b/lib/puppet/inifile.rb
new file mode 100644
index 000000000..bb8137d10
--- /dev/null
+++ b/lib/puppet/inifile.rb
@@ -0,0 +1,201 @@
+# Module Puppet::IniConfig
+# A generic way to parse .ini style files and manipulate them in memory
+# One 'file' can be made up of several physical files. Changes to sections
+# on the file are tracked so that only the physical files in which
+# something has changed are written back to disk
+# Great care is taken to preserve comments and blank lines from the original
+# files
+module Puppet
+    module IniConfig
+
+        # A section in a .ini file
+        class Section
+            attr_reader :name, :file
+
+            def initialize(name, file)
+                @name = name
+                @file = file
+                @dirty = false
+                @entries = []
+            end
+
+            # Has this section been modified since it's been read in
+            # or written back to disk
+            def dirty?
+                @dirty
+            end
+
+            # Should only be used internally
+            def mark_clean
+                @dirty = false
+            end
+
+            # Add a line of text (e.g., a comment) Such lines
+            # will be written back out in exactly the same
+            # place they were read in
+            def add_line(line)
+                @entries << line
+            end
+
+            # Set the entry 'key=value'. If no entry with the
+            # given key exists, one is appended to teh end of the section
+            def []=(key, value)
+                entry = find_entry(key)
+                @dirty = true
+                if entry.nil?
+                    @entries << [key, value]
+                else
+                    entry[1] = value
+                end
+            end
+
+            # Return the value associated with KEY. If no such entry
+            # exists, return nil
+            def [](key)
+                entry = find_entry(key)
+                if entry.nil?
+                    return nil
+                end
+                return entry[1]
+            end
+
+            # Format the section as text in the way it should be
+            # written to file
+            def format
+                text = "[#{name}]\n"
+                @entries.each do |entry|
+                    if entry.is_a?(Array)
+                        key, value = entry
+                        unless value.nil?
+                            text << "#{key}=#{value}\n"
+                        end
+                    else
+                        text << entry
+                    end
+                end
+                return text
+            end
+
+            private 
+            def find_entry(key)
+                @entries.each do |entry|
+                    if entry.is_a?(Array) && entry[0] == key
+                        return entry
+                    end
+                end
+                return nil
+            end
+            
+        end
+
+        # A logical .ini-file that can be spread across several physical
+        # files. For each physical file, call #read with the filename
+        class File
+            def initialize
+                @files = {}
+            end
+
+            # Add the contents of the file with name FILE to the
+            # already existing sections
+            def read(file)
+                text = Puppet::FileType.filetype(:flat).new(file).read
+                if text.nil?
+                    raise "Could not find #{file}"
+                end
+
+                section = nil
+                line = 0
+                @files[file] = []
+                text.each_line do |l|
+                    line += 1
+                    if l =~ /^\[(.+)\]$/
+                        section.mark_clean unless section.nil?
+                        section = add_section($1, file)
+                    elsif l =~ /^(\s*\#|\s*$)/
+                        if section.nil?
+                            @files[file] << l
+                        else
+                            section.add_line(l)
+                        end
+                    elsif l =~ /^\s*(\S+)\s*\=(.+)$/
+                        # We allow space around the keys, but not the values
+                        # For the values, we don't know if space is significant
+                        if section.nil?
+                            raise "#{file}:#{line}:Key/value pair outside of a section for key #{$1}"
+                        else
+                            section[$1] = $2
+                        end
+                    else
+                        # FIXME: We can't deal with continuation lines
+                        # that at least yum allows (lines that start with
+                        # whitespace, and that should really be appended
+                        # to the value of the previous key)
+                        raise "#{file}:#{line}: Can't parse '#{l.chomp}'"
+                    end
+                end
+                section.mark_clean unless section.nil?
+            end
+
+            # Store all modifications made to sections in this file back
+            # to the physical files. If no modifications were made to 
+            # a physical file, nothing is written
+            def store
+                @files.each do |file, lines|
+                    text = ""
+                    dirty = false
+                    lines.each do |l|
+                        if l.is_a?(Section)
+                            dirty ||= l.dirty?
+                            text << l.format
+                            l.mark_clean
+                        else
+                            text << l
+                        end
+                    end
+                    if dirty
+                        Puppet::FileType.filetype(:flat).new(file).write(text)
+                    end
+                end
+            end
+
+            # Execute BLOCK, passing each section in this file
+            # as an argument
+            def each_section(&block)
+                @files.each do |file, list|
+                    list.each do |entry|
+                        if entry.is_a?(Section)
+                            yield(entry)
+                        end
+                    end
+                end
+            end
+
+            # Return the Section with the given name or nil
+            def [](name)
+                name = name.to_s
+                each_section do |section|
+                    return section if section.name == name
+                end
+                return nil
+            end
+
+            # Return true if the file contains a section with name NAME
+            def include?(name)
+                return ! self[name].nil?
+            end
+
+            # Add a section to be stored in FILE when store is called
+            def add_section(name, file)
+                if include?(name)
+                    raise "A section with name #{name} already exists"
+                end
+                result = Section.new(name, file)
+                @files[file] ||= []
+                @files[file] << result
+                return result
+            end
+        end
+
+        
+    end
+end
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 73897776f..8ce39ce29 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -2246,7 +2246,5 @@ require 'puppet/type/symlink'
 require 'puppet/type/user'
 require 'puppet/type/tidy'
 require 'puppet/type/parsedtype'
-#This needs some more work before it is ready for primetime
-#require 'puppet/type/yumrepo'
 
 # $Id$
diff --git a/lib/puppet/type/yumrepo.rb b/lib/puppet/type/yumrepo.rb
index 48e1f8f4b..c7add289b 100644
--- a/lib/puppet/type/yumrepo.rb
+++ b/lib/puppet/type/yumrepo.rb
@@ -1,6 +1,7 @@
 # Description of yum repositories
 
 require 'puppet/statechange'
+require 'puppet/inifile'
 require 'puppet/type/parsedtype'
 
 module Puppet
@@ -10,80 +11,163 @@ module Puppet
 
         def insync?
             # A should state of :absent is the same as nil
-            if self.is.nil? && (self.should.nil? || self.should == :absent)
+            if is.nil? && (should.nil? || should == :absent)
                 return true
             end
             return super
         end
 
-        def inikey
-            self.name
+        def sync
+            if insync?
+                result = nil
+            else
+                result = set
+                parent.section[inikey] = should
+            end
+            return result
         end
 
-        def format
-            "#{self.inikey}=#{self.should}"
+        def retrieve
+            @is = parent.section[inikey]
         end
-
-        def emit
-            self.should.nil? || self.should == :absent ? "" : "#{format}\n"
+        
+        def inikey
+            name.to_s
         end
-    end
 
-    # A state for the section header in a .ini-style file
-    class IniSectionState < IniState
-        def format
-            "[#{self.should}]"
+        # Set the key associated with this state to KEY, instead
+        # of using the state's NAME
+        def self.inikey(key)
+            # Override the inikey instance method
+            # Is there a way to do this without resorting to strings ?
+            # Using a block fails because the block can't access
+            # the variable 'key' in the outer scope
+            self.class_eval("def inikey ; \"#{key.to_s}\" ; end")
         end
+
     end
 
     # Doc string for states that can be made 'absent'
     ABSENT_DOC="Set this to 'absent' to remove it from the file completely"
 
     newtype(:yumrepo) do
-        @doc = "The client-side description of a yum repository. Manages
-                the yum repository configuration in the file '$name.repo',
-                usually in the directory /etc/yum.repos.d, though the 
-                directory can be set with the **repodir** parameter.
+        @doc = "The client-side description of a yum repository. Repository
+                configurations are found by parsing /etc/yum.conf and
+                the files indicated by reposdir in that file (see yum.conf(5)
+                for details)
 
                 Most parameters are identical to the ones documented 
                 in yum.conf(5)
 
-                Note that the proper working of this type requires that
-                configurations for individual repos are kept in
-                separate files in **repodir**, and that no attention
-                is paid to the overall /etc/yum.conf"
+                Continuation lines that yum supports for example for the
+                baseurl are not supported. No attempt is made to access
+                files included with the **include** directive"
 
         class << self
             attr_accessor :filetype
+            # The writer is only used for testing, there should be no need
+            # to change yumconf in any other context
+            attr_accessor :yumconf
         end
 
         self.filetype = Puppet::FileType.filetype(:flat)
 
-        def path
-            File.join(self[:repodir], "#{self[:name]}.repo")
+        @inifile = nil
+        
+        @yumconf = "/etc/yum.conf"
+
+        # Where to put files for brand new sections
+        @defaultrepodir = nil
+
+        # Return the Puppet::IniConfig::File for the whole yum config
+        def self.inifile
+            if @inifile.nil?
+                @inifile = read()
+                main = @inifile['main']
+                if main.nil?
+                    raise Puppet::Error, "File #{yumconf} does not contain a main section" 
+                end
+                reposdir = main['reposdir'] 
+                reposdir ||= "/etc/yum.repos.d, /etc/yum/repos.d"
+                reposdir.gsub!(/[\n,]/, " ")
+                reposdir.split.each do |dir|
+                    Dir::glob("#{dir}/*.repo").each do |file|
+                        if File.file?(file)
+                            @inifile.read(file)
+                        end
+                    end
+                end
+                reposdir.split.each do |dir|
+                    if File::directory?(dir) && File::writable?(dir)
+                        @defaultrepodir = dir
+                        break
+                    end
+                end
+            end
+            return @inifile
         end
 
-        def retrieve
-            Puppet.debug "Parsing yum config %s" % path
-            text = self.class.filetype().new(path).read
-            # Keep track of how entries were in the initial file
-            # and preserve comments. @lines holds either original
-            # lines (for comments) or a symbol for the entry that was there
-            @lines = []
-            text.each_line do |l|
-                if l =~ /^\[(.+)\]$/
-                    self.is = [:repoid, $1]
-                    @lines << :repoid
-                elsif l =~ /^(\s*\#|\s*$)/
-                    # Preserve comments and empty lines
-                    @lines << l
-                elsif l =~ /^(.+)\=(.+)$/
-                    key = $1.to_sym
-                    key = :descr if $1 == "name"
-                    self.is = [key, $2]
-                    @lines << key
+        # Parse the yum config files. Only exposed for the tests
+        # Non-test code should use self.inifile to get at the
+        # underlying file
+        def self.read
+            result = Puppet::IniConfig::File.new()
+            result.read(yumconf)
+            main = result['main']
+            if main.nil?
+                raise Puppet::Error, "File #{yumconf} does not contain a main section" 
+            end
+            reposdir = main['reposdir']
+            reposdir ||= "/etc/yum.repos.d, /etc/yum/repos.d"
+            reposdir.gsub!(/[\n,]/, " ")
+            reposdir.split.each do |dir|
+                Dir::glob("#{dir}/*.repo").each do |file|
+                    if File.file?(file)
+                        result.read(file)
+                    end
+                end
+            end
+            if @defaultrepodir.nil?
+                reposdir.split.each do |dir|
+                    if File::directory?(dir) && File::writable?(dir)
+                        @defaultrepodir = dir
+                        break
+                    end
                 end
             end
+            return result
+        end
+
+        # Return the Puppet::IniConfig::Section with name NAME
+        # from the yum config
+        def self.section(name)
+            result = inifile[name]
+            if result.nil?
+                # Brand new section
+                path = yumconf
+                unless @defaultrepodir.nil?
+                    path = File::join(@defaultrepodir, "#{name}.repo")
+                end
+                Puppet::info "create new repo #{name} in file #{path}"
+                result = inifile.add_section(name, path)
+            end
+            return result
+        end
+
+        # Store all modifications back to disk
+        def self.store
+            inifile.store
+        end
+
+        def self.clear
+            @inifile = nil
+            @yumconf = "/etc/yum.conf"
+            @defaultrepodir = nil
+        end
+
+        # Return the Puppet::IniConfig::Section for this yumrepo element
+        def section
+            self.class.section(self[:name])
         end
 
         def evaluate
@@ -110,53 +194,22 @@ module Puppet
             return changes
         end
 
+        # Store modifications to this yumrepo element back to disk
         def store
-            text = ""
-            @lines.each do |l|
-                if l.is_a?(String)
-                    text << l
-                else
-                    text << state(l).emit
-                end
-            end
-            self.each do |state|
-                if state.is.nil? || state.is == :absent
-                    # State was not in the parsed config file
-                    text << state.emit
-                end
-            end
-            Puppet.debug "Writing yum config %s" % path
-            self.class.filetype().new(path).write(text)
+            self.class.store
         end
 
         newparam(:name) do
-            desc "The name of the repository. This is used to find the config
-                  file as $repodir/$name.repo. The 'name' parameter in the yum
-                  config file has to be set through **descr**"
+            desc "The name of the repository."
             isnamevar
         end
 
-        newparam(:repodir) do
-            desc "The directory in which repo config files are to be found. 
-                  Defaults to /etc/yum.repos.d"
-            defaultto("/etc/yum.repos.d")
-        end
-
-        newstate(:repoid, Puppet::IniSectionState) do
-            desc "The id that yum uses internally to keep track of 
-                  the repository"
-            newvalue(/.*/) { }
-        end
-
         newstate(:descr, Puppet::IniState) do
-            desc "A human readable description of the repository. Corresponds
-                  to the 'name' parameter in the yum config file.
+            desc "A human readable description of the repository. 
                   #{ABSENT_DOC}"
-            newvalue(:absent) { self.should = nil }
+            newvalue(:absent) { self.should = :absent }
             newvalue(/.*/) { }
-            def inikey
-                :name
-            end
+            inikey "name"
         end
         
         newstate(:mirrorlist, Puppet::IniState) do
diff --git a/test/other/inifile.rb b/test/other/inifile.rb
new file mode 100644
index 000000000..eb2fde83d
--- /dev/null
+++ b/test/other/inifile.rb
@@ -0,0 +1,114 @@
+if __FILE__ == $0
+    $:.unshift '..'
+    $:.unshift '../../lib'
+    $puppetbase = "../.."
+end
+
+require 'puppet'
+require 'puppet/inifile'
+require 'puppettest'
+require 'test/unit'
+
+class TestFileType < Test::Unit::TestCase
+	include TestPuppet
+
+    def setup
+        super
+        @file = Puppet::IniConfig::File.new
+    end
+    
+    def teardown
+        @file = nil
+        super
+    end
+
+    def test_simple
+        fname = mkfile("[main]\nkey1=value1\n# Comment\nkey2=value2")
+        assert_nothing_raised {
+            @file.read(fname)
+        }
+        s = get_section('main')
+        assert_entries(s, { 'key1' => 'value1', 'key2' => 'value2' })
+        @file['main']['key2'] = 'newvalue2'
+        @file['main']['key3'] = 'newvalue3'
+        text = s.format
+        assert_equal("[main]\nkey1=value1\n# Comment\nkey2=newvalue2\nkey3=newvalue3\n",
+                     s.format)
+    end
+
+    def test_multi
+        fmain = mkfile("[main]\nkey1=main.value1\n# Comment\nkey2=main.value2")
+        fsub = mkfile("[sub1]\nkey1=sub1 value1\n\n" +
+                        "[sub2]\nkey1=sub2.value1")
+        main_mtime = File::stat(fmain).mtime
+        assert_nothing_raised {
+            @file.read(fmain)
+            @file.read(fsub)
+        }
+        main = get_section('main')
+        assert_entries(main, 
+                       { 'key1' => 'main.value1', 'key2' => 'main.value2' })
+        sub1 = get_section('sub1')
+        assert_entries(sub1, { 'key1' => 'sub1 value1' })
+        sub2 = get_section('sub2')
+        assert_entries(sub2, { 'key1' => 'sub2.value1' })
+        [main, sub1, sub2].each { |s| assert( !s.dirty? ) }
+        sub1['key1'] = 'sub1 newvalue1'
+        sub2['key2'] = 'sub2 newvalue2'
+        assert(! main.dirty?)
+        [sub1, sub2].each { |s| assert( s.dirty? ) }
+        @file.store
+        [main, sub1, sub2].each { |s| assert( !s.dirty? ) }
+        assert( File.exists?(fmain) )
+        assert( File.exists?(fsub) )
+        assert_equal(main_mtime, File::stat(fmain).mtime)
+        subtext = File.read(fsub)
+        assert_equal("[sub1]\nkey1=sub1 newvalue1\n\n" +
+                     "[sub2]\nkey1=sub2.value1\nkey2=sub2 newvalue2\n",
+                     subtext)
+    end
+
+    def test_format_nil
+        fname = mkfile("[main]\nkey1=value1\n# Comment\nkey2=value2\n" +
+                       "# Comment2\n")
+        assert_nothing_raised {
+            @file.read(fname)
+        }
+        s = get_section('main')
+        s['key2'] = nil
+        s['key3'] = nil
+        text = s.format
+        assert_equal("[main]\nkey1=value1\n# Comment\n# Comment2\n",
+                     s.format)
+    end
+
+    def test_whitespace
+        fname = mkfile("[main]\n  key1=v1\nkey2  =v2\n")
+        assert_nothing_raised {
+            @file.read(fname)
+        }
+        s = get_section('main')
+        assert_equal('v1', s['key1'])
+        assert_equal('v2', s['key2'])
+    end
+
+    def assert_entries(section, hash)
+        hash.each do |k, v| 
+            assert_equal(v, section[k], 
+                        "Expected <#{v}> for #{section.name}[#{k}] " + 
+                         "but got <#{section[k]}>")
+        end
+    end
+
+    def get_section(name)
+        result = @file[name]
+        assert_not_nil(result)
+        return result
+    end
+
+    def mkfile(content)
+        file = tempfile()
+        File.open(file, "w") { |f| f.print(content) }
+        return file
+    end
+end
diff --git a/test/types/yumrepo.rb b/test/types/yumrepo.rb
index fc39888cb..89bdf9590 100644
--- a/test/types/yumrepo.rb
+++ b/test/types/yumrepo.rb
@@ -7,48 +7,81 @@ if __FILE__ == $0
 end
 
 require 'puppettest'
-require 'puppet/type/yumrepo'
 require 'puppet'
 require 'test/unit'
+require 'fileutils'
 
 class TestYumRepo < Test::Unit::TestCase
 	include TestPuppet
 
-    def test_parse
-        fakedata("data/types/yumrepos").each { |file|
-            next unless file =~ /\.repo$/
-            repo = make_repo(file)
-            Puppet.info "Parsing %s" % file
-            assert_nothing_raised {
-                repo.retrieve
-            }
-            # Lame tests that we actually parsed something in
-            assert ! repo[:descr].empty?
-            assert ! repo[:repoid].empty?
-        }
+    def setup
+        super
+        @yumdir = tempfile()
+        Dir.mkdir(@yumdir)
+        @yumconf = File.join(@yumdir, "yum.conf")
+        File.open(@yumconf, "w") do |f|
+            f.print "[main]\nreposdir=#{@yumdir} /no/such/dir\n"
+        end
+        Puppet.type(:yumrepo).yumconf = @yumconf
+    end
+
+    # Modify one existing section
+    def test_modify
+        copy_datafiles
+        devel = make_repo("development", { :descr => "New description" })
+        devel.retrieve
+        assert_equal("development", devel[:name])
+        assert_equal('Fedora Core $releasever - Development Tree', 
+                     devel.state(:descr).is)
+        assert_equal('New description', 
+                     devel.state(:descr).should)
+        assert_apply(devel)
+        inifile = Puppet.type(:yumrepo).read()
+        assert_equal('New description', inifile['development']['name'])
+        assert_equal('Fedora Core $releasever - $basearch - Base',
+                     inifile['base']['name'])
+        assert_equal(['base', 'development', 'main'],
+                     all_sections(inifile))
     end
 
+    # Create a new section
     def test_create
-        file = "#{tempfile()}.repo"
         values = {
-            :repoid => "base",
             :descr => "Fedora Core $releasever - $basearch - Base",
             :baseurl => "http://example.com/yum/$releasever/$basearch/os/",
             :enabled => "1",
             :gpgcheck => "1",
+            :includepkgs => "absent",
             :gpgkey => "file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora"
         }
-        repo = make_repo(file, values)
+        repo = make_repo("base", values)
         assert_apply(repo)
-        text = Puppet::FileType.filetype(:flat).new(repo.path).read
+        inifile = Puppet.type(:yumrepo).read()
+        sections = all_sections(inifile)
+        assert_equal(['base', 'main'], sections)
+        text = inifile["base"].format
         assert_equal(CREATE_EXP, text)
     end
 
-    def make_repo(file, hash={})
-        hash[:repodir] = File::dirname(file)
-        hash[:name]  = File::basename(file, ".repo")
+    def make_repo(name, hash={})
+        hash[:name] = name
         Puppet.type(:yumrepo).create(hash)
     end
+
+    def all_sections(inifile)
+        sections = []
+        inifile.each_section { |section| sections << section.name }
+        return sections.sort
+    end
+
+    def copy_datafiles
+        fakedata("data/types/yumrepos").select { |file|
+            file =~ /\.repo$/
+        }.each { |src|
+            dst = File::join(@yumdir, File::basename(src))
+            FileUtils::copy(src, dst)
+        }
+    end
     
     CREATE_EXP = <<'EOF'
 [base]