Merge branch 'feature/2.7.x/9051-storeconfig-backend-should-be-config…

…urable' into 2.7.x

* feature/2.7.x/9051-storeconfig-backend-should-be-configurable:
  (puppetlabs#9051) de-ActiveRecord-ify Collection expressions.
  (puppetlabs#9051) Port query tests into the indirection.
  (puppetlabs#9051) Implement the `resource` terminus for StoreConfigs.
  (puppetlabs#9051) Make generic tagging imported resource origins.
  (puppetlabs#9051) Whitespace cleanup for puppet/parser/collector
  (puppetlabs#9051) Dead code elimination in the compiler terminus.
  (puppetlabs#9051) Get the compiler out of the ActiveRecord business.
  (puppetlabs#9051) Implement the StoreConfigs indirection itself.
  (puppetlabs#9051) Add configuration around StoreConfigs indirection.

Reviewed-by: Jeff McCune

lib/puppet/application/queue.rb

@@ -151,7 +151,7 @@ def setup
     exit(Puppet.settings.print_configs ? 0 : 1) if Puppet.settings.print_configs?
 
     require 'puppet/resource/catalog'
-    Puppet::Resource::Catalog.indirection.terminus_class = :active_record
+    Puppet::Resource::Catalog.indirection.terminus_class = :store_configs
 
     daemon.daemonize if Puppet[:daemonize]
 

lib/puppet/defaults.rb

@@ -830,20 +830,36 @@ module Puppet
   )
 
   setdefaults(:master,
-    :storeconfigs => {:default => false, :desc => "Whether to store each client's configuration.  This
-      requires ActiveRecord from Ruby on Rails.",
-      :call_on_define => true, # Call our hook with the default value, so we always get the libdir set.
+    :storeconfigs => {
+      :default => false,
+      :desc => "Whether to store each client's configuration, including catalogs, facts,
+and related data.  This also enables the import and export of resources in
+the Puppet language - a mechanism for exchange resources between nodes.
+
+By default this uses ActiveRecord and an SQL database to store and query
+the data; this, in turn, will depend on Rails being available.
+
+You can adjust the backend using the storeconfigs_backend setting.",
+      # Call our hook with the default value, so we always get the libdir set.
+      :call_on_define => true,
       :hook => proc do |value|
         require 'puppet/node'
         require 'puppet/node/facts'
         if value
-          require 'puppet/rails'
-          raise "StoreConfigs not supported without ActiveRecord 2.1 or higher" unless Puppet.features.rails?
-          Puppet::Resource::Catalog.indirection.cache_class = :active_record unless Puppet.settings[:async_storeconfigs]
-          Puppet::Node::Facts.indirection.cache_class = :active_record
-          Puppet::Node.indirection.cache_class = :active_record
+          Puppet.settings[:async_storeconfigs] or
+            Puppet::Resource::Catalog.indirection.cache_class = :store_configs
+          Puppet::Node::Facts.indirection.cache_class = :store_configs
+          Puppet::Node.indirection.cache_class = :store_configs
+
+          Puppet::Resource.indirection.terminus_class = :store_configs
         end
       end
+    },
+    :storeconfigs_backend => {
+      :default => "active_record",
+      :desc => "Configure the backend terminus used for StoreConfigs.
+By default, this uses the ActiveRecord store, which directly talks to the
+database from within the Puppet Master process."
     }
   )
 

lib/puppet/indirector/catalog/compiler.rb

@@ -49,7 +49,6 @@ def filter(catalog)
 
   def initialize
     set_server_facts
-    setup_database_backend if Puppet[:storeconfigs]
   end
 
   # Is our compiler part of a network, or are we just local?
@@ -151,21 +150,4 @@ def set_server_facts
       end
     end
   end
-
-  def setup_database_backend
-    raise Puppet::Error, "Rails is missing; cannot store configurations" unless Puppet.features.rails?
-    Puppet::Rails.init
-  end
-
-  # Mark that the node has checked in. LAK:FIXME this needs to be moved into
-  # the Node class, or somewhere that's got abstract backends.
-  def update_node_check(node)
-    if Puppet.features.rails? and Puppet[:storeconfigs]
-      Puppet::Rails.connect
-
-      host = Puppet::Rails::Host.find_or_create_by_name(node.name)
-      host.last_freshcheck = Time.now
-      host.save
-    end
-  end
 end

lib/puppet/indirector/catalog/store_configs.rb

@@ -0,0 +1,5 @@
+require 'puppet/indirector/store_configs'
+require 'puppet/resource/catalog'
+
+class Puppet::Resource::Catalog::StoreConfigs < Puppet::Indirector::StoreConfigs
+end

lib/puppet/indirector/facts/store_configs.rb

@@ -0,0 +1,5 @@
+require 'puppet/node/facts'
+require 'puppet/indirector/store_configs'
+
+class Puppet::Node::Facts::StoreConfigs < Puppet::Indirector::StoreConfigs
+end

lib/puppet/indirector/node/store_configs.rb

@@ -0,0 +1,5 @@
+require 'puppet/indirector/store_configs'
+require 'puppet/node'
+
+class Puppet::Node::StoreConfigs < Puppet::Indirector::StoreConfigs
+end

lib/puppet/indirector/resource/active_record.rb

@@ -0,0 +1,90 @@
+require 'puppet/indirector/active_record'
+
+class Puppet::Resource::ActiveRecord < Puppet::Indirector::ActiveRecord
+  def search(request)
+    type   = request_to_type_name(request)
+    host   = request.options[:host]
+    filter = request.options[:filter]
+
+    query = build_active_record_query(type, host, filter)
+    Puppet::Rails::Resource.find(:all, query)
+  end
+
+  private
+  def request_to_type_name(request)
+    name = request.key.split('/', 2)[0]
+    type = Puppet::Type.type(name) or raise Puppet::Error, "Could not find type #{name}"
+    type.name
+  end
+
+  def filter_to_active_record(filter)
+    # Don't call me if you don't have a filter, please.
+    filter.is_a?(Array) or raise ArgumentError, "active record filters must be arrays"
+    a, op, b = filter
+
+    case op
+    when /^(and|or)$/i then
+      extra = []
+      first, args = filter_to_active_record a
+      extra += args
+
+      second, args = filter_to_active_record b
+      extra += args
+
+      return "(#{first}) #{op.upcase} (#{second})", extra
+
+    when "==", "!=" then
+      op = '=' if op == '=='    # SQL, yayz!
+      case a
+      when "title" then
+        return "title #{op} ?", [b]
+
+      when "tag" then
+        return "puppet_tags.name #{op} ?", [b]
+
+      else
+        return "param_names.name = ? AND param_values.value #{op} ?", [a, b]
+      end
+
+    else
+      raise ArgumentError, "unknown operator #{op.inspect} in #{filter.inspect}"
+    end
+  end
+
+  def build_active_record_query(type, host, filter)
+    raise Puppet::DevError, "Cannot collect resources for a nil host" unless host
+
+    search = "(exported=? AND restype=?)"
+    arguments = [true, type]
+
+    if filter then
+      sql, values = filter_to_active_record(filter)
+      search    += " AND #{sql}"
+      arguments += values
+    end
+
+    # note: we're not eagerly including any relations here because it can
+    # create large numbers of objects that we will just throw out later.  We
+    # used to eagerly include param_names/values but the way the search filter
+    # is built ruined those efforts and we were eagerly loading only the
+    # searched parameter and not the other ones.
+    query = {}
+    case search
+    when /puppet_tags/
+      query = { :joins => { :resource_tags => :puppet_tag } }
+    when /param_name/
+      query = { :joins => { :param_values => :param_name } }
+    end
+
+    # We're going to collect objects from rails, but we don't want any
+    # objects from this host.
+    if host = Puppet::Rails::Host.find_by_name(host)
+      search += " AND (host_id != ?)"
+      arguments << host.id
+    end
+
+    query[:conditions] = [search, *arguments]
+
+    query
+  end
+end

lib/puppet/indirector/resource/store_configs.rb

@@ -0,0 +1,3 @@
+require 'puppet/indirector/store_configs'
+class Puppet::Resource::StoreConfigs < Puppet::Indirector::StoreConfigs
+end

lib/puppet/indirector/store_configs.rb

@@ -0,0 +1,30 @@
+class Puppet::Indirector::StoreConfigs < Puppet::Indirector::Terminus
+  def initialize
+    super
+    # This will raise if the indirection can't be found, so we can assume it
+    # is always set to a valid instance from here on in.
+    @target = indirection.terminus Puppet[:storeconfigs_backend]
+  end
+
+  attr_reader :target
+
+  def head(request)
+    target.head request
+  end
+
+  def find(request)
+    target.find request
+  end
+
+  def search(request)
+    target.search request
+  end
+
+  def save(request)
+    target.save request
+  end
+
+  def destroy(request)
+    target.save request
+  end
+end

lib/puppet/parser/ast/collection.rb

@@ -13,11 +13,11 @@ class Collection < AST::Branch
 
     # We return an object that does a late-binding evaluation.
     def evaluate(scope)
-      str, code = query && query.safeevaluate(scope)
+      match, code = query && query.safeevaluate(scope)
 
       resource_type = scope.find_resource_type(@type)
       fail "Resource type #{@type} doesn't exist" unless resource_type
-      newcoll = Puppet::Parser::Collector.new(scope, resource_type.name, str, code, self.form)
+      newcoll = Puppet::Parser::Collector.new(scope, resource_type.name, match, code, self.form)
 
       scope.compiler.add_collection(newcoll)
 

lib/puppet/parser/ast/collexpr.rb

@@ -19,8 +19,8 @@ def evaluate(scope)
     end
 
     # The code is only used for virtual lookups
-    str1, code1 = @test1.safeevaluate scope
-    str2, code2 = @test2.safeevaluate scope
+    match1, code1 = @test1.safeevaluate scope
+    match2, code2 = @test2.safeevaluate scope
 
     # First build up the virtual code.
     # If we're a conjunction operator, then we're calling code.  I did
@@ -31,16 +31,16 @@ def evaluate(scope)
       when "and"; code1.call(resource) and code2.call(resource)
       when "or"; code1.call(resource) or code2.call(resource)
       when "=="
-        if str1 == "tag"
-          resource.tagged?(str2)
+        if match1 == "tag"
+          resource.tagged?(match2)
         else
-          if resource[str1].is_a?(Array)
-            resource[str1].include?(str2)
+          if resource[match1].is_a?(Array)
+            resource[match1].include?(match2)
           else
-            resource[str1] == str2
+            resource[match1] == match2
           end
         end
-      when "!="; resource[str1] != str2
+      when "!="; resource[match1] != match2
       end
     end
 
@@ -49,32 +49,13 @@ def evaluate(scope)
       Puppet.warning "Parentheses are ignored in Rails searches"
     end
 
-    case @oper
-    when "and", "or"
-      if form == :exported
-        raise Puppet::ParseError, "Puppet does not currently support collecting exported resources with more than one condition"
-      end
-      oper = @oper.upcase
-    when "=="; oper = "="
-    else
-      oper = @oper
+    if form == :exported and (@oper =~ /^(and|or)$/i) then
+      raise Puppet::ParseError, "Puppet does not currently support collecting exported resources with more than one condition"
     end
 
-    if oper == "=" or oper == "!="
-      # Add the rails association info where necessary
-      case str1
-      when "title"
-        str = "title #{oper} '#{str2}'"
-      when "tag"
-        str = "puppet_tags.name #{oper} '#{str2}'"
-      else
-        str = "param_values.value #{oper} '#{str2}' and param_names.name = '#{str1}'"
-      end
-    else
-      str = "(#{str1}) #{oper} (#{str2})"
-    end
+    match = [match1, @oper, match2]
 
-    return str, code
+    return match, code
   end
 
   def initialize(hash = {})