On 13 Jul 2011, at 08:46, Luke Kanies <[email protected]> wrote:

> This primarily takes Volcane's code[1], adds tests, and
> splits it into multiple files.

This is awesome and thanks for the rewrite/redesign mine was mostly prototype 
style stuff but the is sweet. :)


> 
> It has support for facts in non-ruby files, using either plain test,
> json, or yaml.  You can also have a script that returns facts on
> execution.
> 
> Things of note:
> 
> * We're defaulting to /etc/facts.d, just like the original code. The
>  ticket calls for this to be /etc/facter.d.
> * We're defaulting to a cache file in /tmp rather than, say, /var/tmp.
>  This is probably fine, but worthy of note.
> * We're actually supporting scripts, which seems kind of frightening,
>  but the original code did it, too.
> * There's no way to turn this off or configure any of this at this
>  point.
> 
> Even with all of those caveats, the code works and is quite well tested.
> 
> 1 - https://github.com/ripienaar/facter-facts/tree/master/facts-dot-d
> 
> Signed-off-by: Luke Kanies <[email protected]>
> ---
> Local-branch: tickets/master/2157-external_fact_support
> lib/facter/util/cache.rb                |   52 ++++++++++++
> lib/facter/util/directory_loader.rb     |   47 +++++++++++
> lib/facter/util/loader.rb               |    9 ++
> lib/facter/util/parser.rb               |  136 ++++++++++++++++++++++++++++++
> spec/unit/util/cache_spec.rb            |  138 +++++++++++++++++++++++++++++++
> spec/unit/util/directory_loader_spec.rb |   90 ++++++++++++++++++++
> spec/unit/util/loader_spec.rb           |    7 ++
> spec/unit/util/parser_spec.rb           |  130 +++++++++++++++++++++++++++++
> 8 files changed, 609 insertions(+), 0 deletions(-)
> create mode 100644 lib/facter/util/cache.rb
> create mode 100644 lib/facter/util/directory_loader.rb
> create mode 100644 lib/facter/util/parser.rb
> create mode 100644 spec/unit/util/cache_spec.rb
> create mode 100644 spec/unit/util/directory_loader_spec.rb
> create mode 100644 spec/unit/util/parser_spec.rb
> 
> diff --git a/lib/facter/util/cache.rb b/lib/facter/util/cache.rb
> new file mode 100644
> index 0000000..cd6f63a
> --- /dev/null
> +++ b/lib/facter/util/cache.rb
> @@ -0,0 +1,52 @@
> +class Facter::Util::Cache
> +  attr_reader :filename
> +
> +  def data
> +    if @data.nil?
> +      self.load()
> +    end
> +    @data
> +  end
> +
> +  def initialize(filename)
> +    @filename = filename
> +  end
> +
> +  def []=(file, stuff)
> +    data[file] = {:data => stuff, :stored => Time.now.to_i}
> +    write!
> +  end
> +
> +  def [](file)
> +    ttl = ttl(file)
> +
> +    return nil unless data[file]
> +
> +    now = Time.now.to_i
> +
> +    return data[file][:data] if ttl < 1
> +    return data[file][:data] if (now - data[file][:stored]) <= ttl
> +    return nil
> +  end
> +
> +  def ttl(file)
> +    meta = file + ".ttl"
> +
> +    return 0 unless File.exist?(meta)
> +    return File.read(meta).chomp.to_i
> +  end
> +
> +  def load
> +    if File.exist?(filename)
> +      @data = YAML.load_file(filename)
> +    else
> +      @data = {}
> +    end
> +
> +    return @data
> +  end
> +
> +  def write!
> +    File.open(filename, "w", 0600) {|f| f.write(YAML.dump(data)) }
> +  end
> +end
> diff --git a/lib/facter/util/directory_loader.rb 
> b/lib/facter/util/directory_loader.rb
> new file mode 100644
> index 0000000..ad61c03
> --- /dev/null
> +++ b/lib/facter/util/directory_loader.rb
> @@ -0,0 +1,47 @@
> +# A Facter plugin that loads facts from /etc/facts.d.
> +#
> +# Facts can be in the form of JSON, YAML or Text files
> +# and any executable that returns key=value pairs.
> +#
> +# In the case of scripts you can also create a file that
> +# contains a cache TTL.  For foo.sh store the ttl as just
> +# a number in foo.sh.ttl
> +#
> +# The cache is stored in /tmp/facts_cache.yaml as a mode
> +# 600 file and will have the end result of not calling your
> +# fact scripts more often than is needed
> +
> +require 'facter/util/cache'
> +require 'facter/util/parser'
> +
> +class Facter::Util::DirectoryLoader
> +  require 'yaml'
> +
> +  attr_reader :directory, :cache
> +
> +  def cache_file
> +    @cache.filename
> +  end
> +
> +  def initialize(dir="/etc/facts.d", cache_file="/tmp/facts_cache.yml")
> +    @directory = dir
> +    @cache = Facter::Util::Cache.new(cache_file)
> +  end
> +
> +  def entries
> +    Dir.entries(directory).reject{|f| f =~ /^\.|\.ttl$/}.sort.map {|f| 
> File.join(directory, f) }
> +  rescue
> +    []
> +  end
> +
> +  def load
> +    cache.load
> +    entries.each do |file|
> +      unless data = Facter::Util::Parser.new(file, cache).results
> +        raise "Could not interpret fact file #{file}"
> +      end
> +
> +      data.each { |p,v| Facter.add(p, :value => v) }
> +    end
> +  end
> +end
> diff --git a/lib/facter/util/loader.rb b/lib/facter/util/loader.rb
> index a52012c..f7cd5cd 100644
> --- a/lib/facter/util/loader.rb
> +++ b/lib/facter/util/loader.rb
> @@ -1,7 +1,14 @@
> require 'facter'
> +require 'facter/util/directory_loader'
> 
> # Load facts on demand.
> class Facter::Util::Loader
> +    attr_reader :directory_loader
> +
> +    def initialize
> +        @directory_loader = Facter::Util::DirectoryLoader.new
> +    end
> +
>     # Load all resolutions for a single fact.
>     def load(fact)
>         # Now load from the search path
> @@ -27,6 +34,8 @@ class Facter::Util::Loader
> 
>         load_env
> 
> +        directory_loader.load
> +
>         search_path.each do |dir|
>             next unless FileTest.directory?(dir)
> 
> diff --git a/lib/facter/util/parser.rb b/lib/facter/util/parser.rb
> new file mode 100644
> index 0000000..88de97e
> --- /dev/null
> +++ b/lib/facter/util/parser.rb
> @@ -0,0 +1,136 @@
> +require 'facter/util/cache'
> +
> +class Facter::Util::Parser
> +  attr_reader :filename
> +  attr_accessor :cache
> +  
> +  class << self
> +    # Retrieve the set extension, if any
> +    attr_reader :extension
> +  end
> +
> +  # Register the extension that this parser matches.
> +  def self.matches_extension(ext)
> +    @extension = ext
> +  end
> +
> +  def self.file_extension(filename)
> +    File.extname(filename).sub(".", '')
> +  end
> +
> +  def self.inherited(klass)
> +    @subclasses ||= []
> +    @subclasses << klass
> +  end
> +
> +  def self.matches?(filename)
> +    raise "Must override the 'matches?' method for #{self}" unless extension
> +
> +    file_extension(filename) == extension
> +  end
> +
> +  def self.subclasses
> +    @subclasses ||= []
> +    @subclasses
> +  end
> +
> +  def self.which_parser(filename)
> +    unless klass = subclasses.detect {|k| k.matches?(filename) }
> +      raise ArgumentError, "Could not find parser for #{filename}"
> +    end
> +    klass
> +  end
> +
> +  def self.new(filename, cache = nil)
> +    klass = which_parser(filename)
> +    
> +    object = klass.allocate
> +    object.send(:initialize, filename)
> +
> +    if cache
> +      object.cache = cache
> +    end
> +
> +    object
> +  end
> +  
> +  def initialize(filename)
> +    @filename = filename
> +  end
> +
> +  class YamlParser < self
> +    matches_extension "yaml"
> +
> +    def results
> +      require 'yaml'
> +
> +      YAML.load_file(filename)
> +    rescue Exception => e
> +      Facter.warn("Failed to handle #{filename} as yaml facts: #{e.class}: 
> #{e}")
> +    end
> +  end
> +
> +  class TextParser < self
> +    matches_extension "txt"
> +
> +    def results
> +      result = {}
> +      File.readlines(filename).each do |line|
> +
> +        if line.chomp =~ /^(.+)=(.+)$/
> +          result[$1] = $2
> +        end
> +      end
> +      result
> +    rescue Exception => e
> +      Facter.warn("Failed to handle #{filename} as text facts: #{e.class}: 
> #{e}")
> +    end
> +  end
> +
> +  class JsonParser < self
> +    matches_extension "json"
> +    
> +    def results
> +      begin
> +        require 'json'
> +      rescue LoadError
> +        require 'rubygems'
> +        retry
> +      end
> +
> +      JSON.load(File.read(filename))
> +    end
> +  end
> +
> +  class ScriptParser < self
> +    def self.matches?(file)
> +      File.executable?(file)
> +    end
> +
> +    def results
> +      if cache and result = cache[filename]
> +        Facter.debug("Using cached data for #{filename}")
> +        return result
> +      end
> +
> +      output = Facter::Util::Resolution.exec(filename)
> +
> +      result = {}
> +      output.split("\n").each do |line|
> +        if line =~ /^(.+)=(.+)$/
> +          result[$1] = $2
> +        end
> +      end
> +
> +      if cache and ttl > 0
> +        Facter.debug("Updating cache for #{filename}")
> +        cache[filename] = result
> +      end
> +
> +      result
> +    rescue Exception => e
> +      Facter.warn("Failed to handle #{filename} as script facts: #{e.class}: 
> #{e}")
> +      Facter.debug(e.backtrace.join("\n\t"))
> +    end
> +  end
> +end
> diff --git a/spec/unit/util/cache_spec.rb b/spec/unit/util/cache_spec.rb
> new file mode 100644
> index 0000000..6ede7a7
> --- /dev/null
> +++ b/spec/unit/util/cache_spec.rb
> @@ -0,0 +1,138 @@
> +#!/usr/bin/env ruby
> +
> +require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper')
> +
> +require 'facter/util/cache'
> +require 'tempfile'
> +
> +describe Facter::Util::Cache do
> +  def mk_test_dir
> +    file = Tempfile.new "testing_fact_caching_dir"
> +    @dir = file.path
> +    file.delete
> +
> +    Dir.mkdir(@dir)
> +    @dirs << @dir # for cleanup
> +
> +    @dir
> +  end
> +
> +  def mk_test_file
> +    file = Tempfile.new "testing_fact_caching_file"
> +    @filename = file.path
> +    file.delete
> +    @files << @filename # for cleanup
> +
> +    @filename
> +  end
> +
> +  before {
> +    @files = []
> +    @dirs = []
> +    @cache = Facter::Util::Cache.new(mk_test_file)
> +    @filename = @cache.filename
> +  }
> +
> +  after do
> +    @files.each do |file|
> +      File.unlink(file) if File.exist?(file)
> +    end
> +    @dirs.each do |dir|
> +      FileUtils.rm_f(dir) if File.exist?(dir)
> +    end
> +  end
> +
> +  it "should make the required filename available" do
> +    @cache.filename.should be_instance_of(String)
> +  end
> +
> +  describe "when determining TTL" do
> +    it "should determine a file's TTL by looking in a file named after the 
> file with a '.ttl' extension" do
> +      dir = mk_test_dir
> +      file = File.join(dir, "myscript")
> +      File.open(file + ".ttl", "w") { |f| f.print 300 }
> +
> +      @cache.ttl(file).should == 300
> +    end
> +  end
> +
> +  describe "when storing data" do
> +    it "should store the provided data in the cache" do
> +      @cache["/my/file"] = {:foo => :bar}
> +      @cache["/my/file"].should == {:foo => :bar}
> +    end
> +
> +    it "should save the data to disk immediately" do
> +      @cache["foo"] = "bar"
> +
> +      other_cache = @cache.class.new(@cache.filename)
> +      other_cache.load
> +      other_cache["foo"].should == "bar"
> +    end
> +
> +    it "should load data the first time data is asked for" do
> +      @cache["foo"] = "bar"
> +
> +      other_cache = @cache.class.new(@cache.filename)
> +      other_cache["foo"].should == "bar"
> +    end
> +
> +    it "should be able to return both old and new data when loading from 
> disk" do
> +      @cache["foo"] = "bar"
> +
> +      other_cache = @cache.class.new(@cache.filename)
> +      other_cache["biz"] = "baz"
> +
> +      third_cache = @cache.class.new(@cache.filename)
> +      third_cache["foo"].should == "bar"
> +      third_cache["biz"].should == "baz"
> +    end
> +
> +    it "should forever cache data whose TTL is set to less than 1" do
> +      @cache.stubs(:ttl).returns 0
> +      @cache["/my/file"] = "foo"
> +      @cache["/my/file"].should == "foo"
> +      @cache["/my/file"].should == "foo"
> +    end
> +
> +    it "should discard data that has expired according to the TTL" do
> +      now = Time.now
> +      @cache["/my/file"] = "foo"
> +      @cache["/my/file"].should == "foo"
> +
> +      Time.expects(:now).returns(now + 30)
> +      @cache.expects(:ttl).returns 1
> +      @cache["/my/file"].should be_nil
> +    end
> +  end
> +
> +  describe "when reading and writing to disk" do
> +    it "should be able to save the data to disk" do
> +      @cache.write!
> +      File.should be_exist(@cache.filename)
> +    end
> +
> +    it "should be able to return data saved to disk" do
> +      @cache["foo"] = "bar"
> +      @cache.write!
> +
> +      other_cache = @cache.class.new(@cache.filename)
> +      other_cache.load
> +      other_cache["foo"].should == "bar"
> +    end
> +
> +    it "should retain the data age when storing on disk" do
> +      now = Time.now
> +      @cache["/my/file"] = "foo"
> +
> +      @cache.write!
> +
> +      other_cache = @cache.class.new(@cache.filename)
> +      other_cache.load
> +
> +      Time.expects(:now).returns(now + 30)
> +      other_cache.expects(:ttl).returns 1
> +      other_cache["/my/file"].should be_nil
> +    end
> +  end
> +end
> diff --git a/spec/unit/util/directory_loader_spec.rb 
> b/spec/unit/util/directory_loader_spec.rb
> new file mode 100644
> index 0000000..85fe2ac
> --- /dev/null
> +++ b/spec/unit/util/directory_loader_spec.rb
> @@ -0,0 +1,90 @@
> +#!/usr/bin/env ruby
> +
> +require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper')
> +
> +require 'facter/util/directory_loader'
> +require 'tempfile'
> +
> +describe Facter::Util::DirectoryLoader do
> +  subject { Facter::Util::DirectoryLoader.new("/my/dir.d") }
> +
> +  def mk_test_dir
> +    file = Tempfile.new "testing_fact_caching_dir"
> +    @dir = file.path
> +    file.delete
> +
> +    Dir.mkdir(@dir)
> +    @dirs << @dir # for cleanup
> +
> +    @dir
> +  end
> +
> +  def mk_test_file
> +    file = Tempfile.new "testing_fact_caching_file"
> +    @filename = file.path
> +    file.delete
> +    @files << @filename # for cleanup
> +
> +    @filename
> +  end
> +
> +  before {
> +    @files = []
> +    @dirs = []
> +    @loader = Facter::Util::DirectoryLoader.new(mk_test_dir)
> +  }
> +
> +  after do
> +    @files.each do |file|
> +      File.unlink(file) if File.exist?(file)
> +    end
> +    @dirs.each do |dir|
> +      FileUtils.rm_f(dir) if File.exist?(dir)
> +    end
> +  end
> +
> +  it "should make the directory available" do
> +    @loader.directory.should be_instance_of(String)
> +  end
> +
> +  it "should default to '/etc/facts.d' for the directory" do
> +    Facter::Util::DirectoryLoader.new.directory.should == "/etc/facts.d"
> +  end
> +
> +  describe "when loading facts from disk" do
> +    it "should be able to load files from disk and set facts" do
> +      data = {"f1" => "one", "f2" => "two"}
> +      file = File.join(@loader.directory, "data" + ".yaml")
> +      File.open(file, "w") { |f| f.print YAML.dump(data) }
> +
> +      @loader.load
> +
> +      Facter.value("f1").should == "one"
> +      Facter.value("f2").should == "two"
> +    end
> +
> +    it "should use the cache when loading data" do
> +      cache_file = mk_test_file
> +      cache = Facter::Util::Cache.new(cache_file)
> +
> +      @loader = Facter::Util::DirectoryLoader.new(mk_test_dir, cache_file)
> +
> +      data = "#!/bin/sh
> +echo one=two
> +echo three=four
> +"
> +      file = File.join(@loader.directory, "myscript")
> +
> +      File.open(file, "w") { |f| f.print data }
> +      File.chmod(0755, file)
> +
> +      cache[file] = {"foo" => "bar"}
> +      cache.write!
> +
> +      @loader.load
> +
> +      # Make sure it's use the cache, not the disk
> +      Facter.value("foo").should == "bar"
> +    end
> +  end
> +end
> diff --git a/spec/unit/util/loader_spec.rb b/spec/unit/util/loader_spec.rb
> index 1bc909f..f193d03 100755
> --- a/spec/unit/util/loader_spec.rb
> +++ b/spec/unit/util/loader_spec.rb
> @@ -160,11 +160,17 @@ describe Facter::Util::Loader do
>     describe "when loading all facts" do
>         before do
>             @loader = Facter::Util::Loader.new
> +            @loader.directory_loader.stubs(:load)
>             @loader.stubs(:search_path).returns []
> 
>             FileTest.stubs(:directory?).returns true
>         end
> 
> +        it "should load all facts from the directory loader" do
> +            @loader.directory_loader.expects(:load)
> +            @loader.load_all
> +        end
> +
>         it "should skip directories that do not exist" do
>             @loader.expects(:search_path).returns %w{/one/dir}
> 
> @@ -215,6 +221,7 @@ describe Facter::Util::Loader do
>           Kernel.expects(:load).with(f)
>         end
> 
> +        @loader.directory_loader.stubs(:load)
>         @loader.load_all
> 
>         @loader.loaded_files.should == %w{/one/dir/bar.rb /one/dir/foo.rb}
> diff --git a/spec/unit/util/parser_spec.rb b/spec/unit/util/parser_spec.rb
> new file mode 100644
> index 0000000..0ba8516
> --- /dev/null
> +++ b/spec/unit/util/parser_spec.rb
> @@ -0,0 +1,130 @@
> +#!/usr/bin/env ruby
> +
> +require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper')
> +
> +require 'facter/util/parser'
> +require 'tempfile'
> +require 'json'
> +
> +describe Facter::Util::Parser do
> +  def mk_test_file
> +    file = Tempfile.new "testing_fact_caching_file"
> +    @filename = file.path
> +    file.delete
> +    @files << @filename # for cleanup
> +
> +    @filename
> +  end
> +
> +  before {
> +    @files = []
> +  }
> +
> +  after do
> +    @files.each do |file|
> +      File.unlink(file) if File.exist?(file)
> +    end
> +  end
> +
> +  it "should fail when asked to parse a file type it does not support" do
> +    lambda { Facter::Util::Parser.new("/my/file.foobar") }.should 
> raise_error(ArgumentError)
> +  end
> +
> +  describe "yaml" do
> +    subject { Facter::Util::Parser::YamlParser }
> +    it "should match the 'yaml' extension" do
> +      subject.extension.should == "yaml"
> +    end
> +
> +    it "should return a hash of whatever is stored on disk" do
> +      file = mk_test_file + ".yaml"
> +
> +      data = {"one" => "two", "three" => "four"}
> +
> +      File.open(file, "w") { |f| f.print YAML.dump(data) }
> +
> +      Facter::Util::Parser.new(file).results.should == data
> +    end
> +
> +    it "should handle exceptions and warn" do
> +      file = mk_test_file + ".yaml"
> +
> +      data = {"one" => "two", "three" => "four"}
> +
> +      File.open(file, "w") { |f| f.print "}" }
> +      Facter.expects(:warn)
> +      lambda { 
> Facter::Util::Parser.new("/some/path/that/doesn't/exist.yaml").results 
> }.should_not raise_error
> +    end
> +  end
> +
> +  describe "json" do
> +    subject { Facter::Util::Parser::JsonParser }
> +    it "should match the 'json' extension" do
> +      subject.extension.should == "json"
> +    end
> +
> +    it "should return a hash of whatever is stored on disk" do
> +      file = mk_test_file + ".json"
> +
> +      data = {"one" => "two", "three" => "four"}
> +
> +      File.open(file, "w") { |f| f.print data.to_json }
> +
> +      Facter::Util::Parser.new(file).results.should == data
> +    end
> +  end
> +
> +  describe "txt" do
> +    subject { Facter::Util::Parser::TextParser }
> +    it "should match the 'txt' extension" do
> +      subject.extension.should == "txt"
> +    end
> +
> +    it "should return a hash of whatever is stored on disk" do
> +      file = mk_test_file + ".txt"
> +
> +      data = "one=two\nthree=four\n"
> +
> +      File.open(file, "w") { |f| f.print data }
> +
> +      Facter::Util::Parser.new(file).results.should == {"one" => "two", 
> "three" => "four"}
> +    end
> +
> +    it "should ignore any non-setting lines" do
> +      file = mk_test_file + ".txt"
> +
> +      data = "one=two\nfive\nthree=four\n"
> +
> +      File.open(file, "w") { |f| f.print data }
> +
> +      Facter::Util::Parser.new(file).results.should == {"one" => "two", 
> "three" => "four"}
> +    end
> +  end
> +
> +  describe "scripts" do
> +    before do
> +      @script = mk_test_file
> +      data = "#!/bin/sh
> +echo one=two
> +echo three=four
> +"
> +
> +      File.open(@script, "w") { |f| f.print data }
> +      File.chmod(0755, @script)
> +    end
> +
> +    it "should use any cache provided at initialization time" do
> +      cache_file = mk_test_file
> +      cache = Facter::Util::Cache.new(mk_test_file)
> +
> +      cache.stubs(:write!)
> +      cache[@script] = {"one" => "yay"}
> +
> +      Facter::Util::Parser.new(@script, cache).results.should == {"one" => 
> "yay"}
> +    end
> +
> +    it "should return a hash of whatever is returned by the executable" do
> +      Facter::Util::Parser.new(@script).results.should == {"one" => "two", 
> "three" => "four"}
> +    end
> +  end
> +end
> -- 
> 1.7.3.1
> 
> -- 
> 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.
> 

-- 
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.

Reply via email to