commit f8d946082e58b60213a27ded3e1e5f5373d976de
Author: Amos Benari <abenari@redhat.com>
Date:   Thu Sep 20 10:55:31 2012 +0300

    fixes #832 - adds parameterized class support
    
    Credits:
    This patch is based on the original work of Olivier Favre
    <olivier@yakaz.com> many many thanks!
    
    This patch adds the following features
    * import class parameters, and try to understand their types
    * support a complex matrix of environments, puppet classes and their
    signature - each class can have a different set of parameters per environment.
    * adds an ignore yaml file 'config/ignored_environments.yml.sample' file were
    users can add regexp or class names that the importer should ignore.
    common usage case for this is classes such as ::config, ::install etc.
    * introduce a new type of smart variable - parameterized.
    * adds complex data types to smart vars, arrays, hashes, json, yaml etc are all supported now.
    
    in order to use the new ENC format for puppet 2.6.5+ you should enable the
    *Parametrized_Classes_in_ENC* and *Enable_Smart_Variables_in_ENC* within Foreman Settings
    
    This is the initial patch just to get param classes support in, follow-up patches
    would include a better UI and the relevant UI updates to host edit page etc.
    
    Signed-off-by: Ohad Levy <ohadlevy@gmail.com>

diff --git a/app/controllers/lookup_keys_controller.rb b/app/controllers/lookup_keys_controller.rb
index 6e0e0ab44..7d8818857 100644
--- a/app/controllers/lookup_keys_controller.rb
+++ b/app/controllers/lookup_keys_controller.rb
@@ -53,11 +53,7 @@ class LookupKeysController < ApplicationController
   private
   def find_by_key
     if params[:id]
-      if params[:id].to_i == 0
-        @lookup_key = LookupKey.find_by_key(params[:id])
-      else
-        @lookup_key = LookupKey.find(params[:id])
-      end
+      @lookup_key = LookupKey.find(params[:id])
       not_found and return if @lookup_key.blank?
     end
   end
diff --git a/app/controllers/puppetclasses_controller.rb b/app/controllers/puppetclasses_controller.rb
index 036c96751..4d194118c 100644
--- a/app/controllers/puppetclasses_controller.rb
+++ b/app/controllers/puppetclasses_controller.rb
@@ -18,7 +18,7 @@ class PuppetclassesController < ApplicationController
       format.html do
         @puppetclasses = values.paginate :page => params[:page], :include => [:environments, :hostgroups]
         @host_counter = Host.count(:group => :puppetclass_id, :joins => :puppetclasses, :conditions => {:puppetclasses => {:id => @puppetclasses}})
-        @keys_counter = LookupKey.count(:group => :puppetclass_id, :conditions => {:puppetclass_id => @puppetclasses})
+        @keys_counter = Puppetclass.joins(:class_params).select('distinct environment_classes.lookup_key_id').count(:group => 'name')
       end
       format.json { render :json => Puppetclass.classes2hash(values.all(:select => "name, id")) }
     end
diff --git a/app/helpers/layout_helper.rb b/app/helpers/layout_helper.rb
index 6bc25d6e7..10c4016b4 100644
--- a/app/helpers/layout_helper.rb
+++ b/app/helpers/layout_helper.rb
@@ -30,8 +30,14 @@ module LayoutHelper
     end
   end
 
+  def line_count (f, attr)
+    rows = f.object.try(attr).to_s.lines.count rescue 1
+    rows == 0 ? 1 : rows
+  end
+
   def textarea_f(f, attr, options = {})
     field(f, attr, options) do
+      options[:rows] = line_count(f, attr) if options[:rows] == :auto
       f.text_area attr, options
     end
   end
@@ -43,10 +49,12 @@ module LayoutHelper
   end
 
   def checkbox_f(f, attr, options = {})
-    text = options.delete(:help_inline)
+    text = options.delete(:help_text)
+    inline = options.delete(:help_inline)
     field(f, attr, options) do
       label_tag('', :class=>'checkbox') do
-      f.check_box(attr, options) + " #{text} "
+        help_inline = inline.blank? ? '' : content_tag(:span, inline, :class => "help-inline")
+        f.check_box(attr, options) + " #{text} " + help_inline.html_safe
       end
     end
   end
diff --git a/app/helpers/lookup_keys_helper.rb b/app/helpers/lookup_keys_helper.rb
index b2c8ef3bb..f519b81b1 100644
--- a/app/helpers/lookup_keys_helper.rb
+++ b/app/helpers/lookup_keys_helper.rb
@@ -17,11 +17,50 @@ module LookupKeysHelper
     options[:form_builder_local] ||= :f
     options[:form_builder_attrs] ||= {}
 
-    content_tag(:div, :id => "#{association}_fields_template", :style => "display: none;") do
+    content_tag(:div, :class => "#{association}_fields_template", :style => "display: none;") do
         form_builder.fields_for(association, options[:object], :child_index => "new_#{association}") do |f|
         render(:partial => options[:partial], :locals => { options[:form_builder_local] => f }.merge(options[:form_builder_attrs]))
       end
     end
   end
 
+  def show_puppet_class f
+    # In case of a new smart-var inside a puppetclass (REST nesting only), or a class parameter:
+    # Show the parent puppetclass as a context, but permit no change.
+    if params["puppetclass_id"]
+      select_f f, :puppetclass_id, [Puppetclass.find(params["puppetclass_id"])], :id, :to_label, {}, {:label => "Puppet class", :disabled => true}
+    elsif f.object.is_param && f.object.param_class
+      field(f, :puppetclass_id, :label => 'Puppet Class') do
+        content_tag(:input, nil, :value => f.object.param_class, :type => 'text', :disabled => true)
+      end
+    else # new smart-var with no particular context
+         # Give a select for choosing the parent puppetclass
+      select_f(f, :puppetclass_id, Puppetclass.all, :id, :to_label, { :include_blank => 'None' }, {:label => "Puppet class"})
+    end unless @puppetclass # nested smart-vars form in a tab of puppetclass/_form: no edition allowed, and the puppetclass is already visible as a context
+  end
+
+  def param_type_selector f
+    selectable_f f, :key_type, options_for_select(LookupKey::KEY_TYPES, f.object.key_type),{},
+               { :disabled => (f.object.is_param && !f.object.override), :label => "Type", :class => "medium",
+                 :help_inline => popover("?","<dl>
+               <dt>String</dt> <dd>Everything is taken as a string.</dd>
+               <dt>Boolean</dt> <dd>Common representation of boolean values are accepted.</dd>
+               <dt>Integer</dt> <dd>Integer numbers only, can be negative.</dd>
+               <dt>Real</dt> <dd>Accept any numerical input.</dd>
+               <dt>Array</dt> <dd>A valid JSON or YAML input, that must evaluate to an array.</dd>
+               <dt>Hash</dt> <dd>A valid JSON or YAML input, that must evaluate to an object/map/dict/hash.</dd>
+               <dt>YAML</dt> <dd>Any valid YAML input.</dd>
+               <dt>JSON</dt> <dd>Any valid JSON input.</dd>
+               </dl>", :title => "How values are validated").html_safe}
+  end
+
+  def validator_type_selector f
+     selectable_f f, :validator_type, options_for_select(LookupKey::VALIDATOR_TYPES, f.object.validator_type),{:include_blank => true},
+                { :disabled => (f.object.is_param && !f.object.override), :label => "Validator Type", :class => "medium",
+                  :help_inline => popover("?","<dl>
+                <dt>List</dt> <dd>A list of the allowed values, specified in the Validator rule field.</dd>
+                <dt>Regexp</dt> <dd>Validates the input with the regular expression in the Validator rule field.</dd>
+                </dl>", :title => "Validation types").html_safe}
+   end
+
 end
diff --git a/app/helpers/puppetclasses_and_environments_helper.rb b/app/helpers/puppetclasses_and_environments_helper.rb
index 75189350e..df9c59e72 100644
--- a/app/helpers/puppetclasses_and_environments_helper.rb
+++ b/app/helpers/puppetclasses_and_environments_helper.rb
@@ -7,16 +7,33 @@ module PuppetclassesAndEnvironmentsHelper
     elsif pcs.delete "_destroy_"
       "Deleted environment #{env} and " + pcs.to_sentence
     else
-      pcs.to_sentence
+      pretty_print(pcs.is_a?(Hash) ? pcs.keys : pcs)
     end
   end
 
   def import_proxy_select hash
-    proxies = Environment.find_import_proxies
     toolbar_action_buttons(
-      proxies.map do |proxy|
+      SmartProxy.puppet_proxies.map do |proxy|
         display_link_if_authorized("Import from #{proxy.name}", hash.merge(:proxy => proxy))
       end.flatten
     )
   end
+
+  private
+  def pretty_print classes
+    hash = { }
+    classes.each do |klass|
+      if (mod = klass.gsub(/::.*/, ""))
+        hash[mod] ||= []
+        hash[mod] << klass
+      else
+        next
+      end
+    end
+    hash.keys.sort.map do |key|
+      link_to_function key, { :rel => "popover", "data-content" => hash[key].sort.join('<br>').html_safe, "data-original-title" => key }
+    end.to_sentence.html_safe
+
+  end
+
 end
diff --git a/app/models/environment.rb b/app/models/environment.rb
index ffa8886ea..3e643feb9 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -1,6 +1,8 @@
 class Environment < ActiveRecord::Base
-  has_and_belongs_to_many :puppetclasses
+  has_many :environment_classes, :dependent => :destroy
+  has_many :puppetclasses, :through => :environment_classes, :uniq => true
   has_many :hosts
+
   validates_presence_of :name
   validates_uniqueness_of :name
   validates_format_of :name, :with => /^[\w\d]+$/, :message => "is alphanumeric and cannot contain spaces"
@@ -18,133 +20,22 @@ class Environment < ActiveRecord::Base
 
   class << self
 
+    #TODO: this needs to be removed, as PuppetDOC generation no longer works
+    # if the manifests are not on the foreman host
     # returns an hash of all puppet environments and their relative paths
     def puppetEnvs proxy = nil
-      #TODO: think of a better way to model multiple puppet proxies
-      url = (proxy || find_import_proxies.first).try(:url)
+
+      url = (proxy || SmartProxy.puppet_proxies.first).try(:url)
       raise "Can't find a valid Foreman Proxy with a Puppet feature" if url.blank?
       proxy = ProxyAPI::Puppet.new :url => url
-      HashWithIndifferentAccess[proxy.environments.map { |e| [e, proxy.classes(e)] }]
-    end
-
-    # Imports all Environments and classes from Puppet modules
-    def importClasses proxy_id
-      # Build two hashes representing the on-disk and in-database, env to classes associations
-      # Create a representation of the puppet configuration where the environments are hash keys and the classes are sorted lists
-      disk_tree         = puppetEnvs SmartProxy.find(proxy_id)
-      disk_tree.default = []
-
-      # Create a representation of the foreman configuration where the environments are hash keys and the classes are sorted lists
-      db_tree           = HashWithIndifferentAccess[Environment.all.map { |e| [e.name, e.puppetclasses.select(:name).map(&:name)] }]
-      db_tree.default   = []
-
-      changes = { "new" => { }, "obsolete" => { } }
-      # Generate the difference between the on-disk and database configuration
-      for env in db_tree.keys
-        # Show the environment if there are classes in the db that do not exist on disk
-        # OR if there is no mention of the class on-disk
-        surplus_db_classes = db_tree[env] - disk_tree[env]
-        surplus_db_classes << "_destroy_" unless disk_tree.has_key?(env) # We need to distinguish between an empty and an obsolete env
-        changes["obsolete"][env] = surplus_db_classes if surplus_db_classes.size > 0
-      end
-      for env in disk_tree.keys
-        extra_disk_classes = disk_tree[env] - db_tree[env]
-        # Show the environment if there are new classes compared to the db
-        # OR if the environment has no puppetclasses but does not exist in the db
-        changes["new"][env] = extra_disk_classes if (extra_disk_classes.size > 0 or (disk_tree[env].size == 0 and Environment.find_by_name(env).nil?))
-      end
-
-      # Remove environments that are in config/ignored_environments.yml
-      ignored_file = File.join(Rails.root.to_s, "config", "ignored_environments.yml")
-      if File.exist? ignored_file
-        ignored = YAML.load_file ignored_file
-        for env in ignored[:new]
-          changes["new"].delete env
-        end
-        for env in ignored[:obsolete]
-          changes["obsolete"].delete env
-        end
-      end
-      changes
+      HashWithIndifferentAccess[proxy.environments.map { |e|
+        [e, HashWithIndifferentAccess[proxy.classes(e).map {|k|
+          klass = k.keys.first
+          [klass, k[klass]["params"]]
+        }]]
+      }]
     end
 
-    # Update the environments and puppetclasses based upon the user's selection
-    # It does a best attempt and can fail to perform all operations due to the
-    # user requesting impossible selections. Repeat the operation if errors are
-    # shown, after fixing the request.
-    # +changed+ : Hash with two keys: :new and :obsolete.
-    #               changed[:/new|obsolete/] is and Array of Strings
-    # Returns   : Array of Strings containing all record errors
-    def obsolete_and_new changed
-      changed        ||= { }
-      @import_errors = []
-
-      # Now we add environments and associations
-      for env_str in changed[:new].keys
-        env = Environment.find_or_create_by_name env_str
-        if env.valid? and !env.new_record?
-          begin
-            pclasses = eval(changed[:new][env_str])
-          rescue => e
-            @import_errors << "Failed to eval #{changed[:new][env_str]} as an array:" + e.message
-            next
-          end
-          for pclass in pclasses
-            pc = Puppetclass.find_or_create_by_name pclass
-            if pc.errors.empty?
-              env.puppetclasses << pc
-            else
-              @import_errors += pc.errors.map(&:to_s)
-            end
-          end
-          env.save!
-        else
-          @import_errors << "Unable to find or create environment #{env_str} in the foreman database"
-        end
-      end if changed[:new]
-
-      # Remove the obsoleted stuff
-      for env_str in changed[:obsolete].keys
-        env = Environment.find_by_name env_str
-        if env
-          begin
-            pclasses = eval(changed[:obsolete][env_str])
-          rescue => e
-            @import_errors << "Failed to eval #{changed[:obsolete][env_str]} as an array:" + e.message
-            next
-          end
-          pclass = ""
-          for pclass in pclasses
-            unless pclass == "_destroy_"
-              pc = Puppetclass.find_by_name pclass
-              if pc.nil?
-                @import_errors << "Unable to find puppet class #{pclass} in the foreman database"
-              else
-                env.puppetclasses.delete pc
-                unless pc.environments.any? or pc.hosts.any?
-                  pc.destroy
-                  @import_errors += pc.errors.full_messages unless pc.errors.empty?
-                end
-              end
-            end
-          end
-          if pclasses.include? "_destroy_"
-            env.destroy
-            @import_errors += env.errors.full_messages unless env.errors.empty?
-          else
-            env.save!
-          end
-        else
-          @import_errors << "Unable to find environment #{env_str} in the foreman database"
-        end
-      end if changed[:obsolete]
-
-      @import_errors
-    end
-
-    def find_import_proxies
-      SmartProxy.joins(:features).where(:features => {:name => 'Puppet'})
-    end
   end
 
   def as_json(options={ })
diff --git a/app/models/environment_class.rb b/app/models/environment_class.rb
new file mode 100644
index 000000000..fe17efbd4
--- /dev/null
+++ b/app/models/environment_class.rb
@@ -0,0 +1,25 @@
+class EnvironmentClass < ActiveRecord::Base
+  belongs_to :environment
+  belongs_to :puppetclass
+  belongs_to :lookup_key
+  validates_uniqueness_of :lookup_key_id,  :scope => [:environment_id, :puppetclass_id]
+  validates_presence_of :puppetclass_id, :environment_id
+
+  scope :parameters_for_class, lambda {|puppetclasses_ids, environment_id|
+      all_parameters_for_class(puppetclasses_ids, environment_id).where(:lookup_keys => {:override => true})
+  }
+  scope :all_parameters_for_class, lambda {|puppetclasses_ids, environment_id|
+    where(:puppetclass_id => puppetclasses_ids, :environment_id => environment_id).
+      where('lookup_key_id is NOT NULL').
+      includes(:lookup_key)
+  }
+
+  #TODO move these into scopes?
+  def self.is_in_any_environment(puppetclass, lookup_key)
+    EnvironmentClass.where(:puppetclass_id => puppetclass, :lookup_key_id => lookup_key ).count > 0
+  end
+
+  def self.key_in_environment(env, puppetclass,  lookup_key)
+    EnvironmentClass.where(:environment_id => env, :puppetclass_id => puppetclass, :lookup_key_id => lookup_key ).first
+  end
+end
diff --git a/app/models/host.rb b/app/models/host.rb
index af3b01641..29c2c36e4 100644
--- a/app/models/host.rb
+++ b/app/models/host.rb
@@ -24,7 +24,7 @@ class Host < Puppet::Rails::Host
       :provider
   end
 
-  attr_reader :cached_host_params, :cached_lookup_keys_params
+  attr_reader :cached_host_params
 
   scope :recent,      lambda { |*args| {:conditions => ["last_report > ?", (args.first || (Setting[:puppet_interval] + 5).minutes.ago)]} }
   scope :out_of_sync, lambda { |*args| {:conditions => ["last_report < ? and enabled != ?", (args.first || (Setting[:puppet_interval] + 5).minutes.ago), false]} }
@@ -294,8 +294,14 @@ class Host < Puppet::Rails::Host
     end
     param.update self.params
 
+    classes = if Setting[:Parametrized_Classes_in_ENC] && Setting[:Enable_Smart_Variables_in_ENC]
+                lookup_keys_class_params
+              else
+                self.puppetclasses_names
+              end
+
     info_hash = {}
-    info_hash['classes'] = self.puppetclasses_names
+    info_hash['classes'] = classes
     info_hash['parameters'] = param
     info_hash['environment'] = param["foreman_env"] if Setting["enc_environment"]
 
@@ -330,20 +336,6 @@ class Host < Puppet::Rails::Host
     @cached_host_params = hp
   end
 
-  def lookup_keys_params
-    return cached_lookup_keys_params unless cached_lookup_keys_params.blank?
-    p = {}
-    # lookup keys
-    if Setting["Enable_Smart_Variables_in_ENC"]
-      klasses  = puppetclasses.map(&:id)
-      klasses += hostgroup.classes.map(&:id) if hostgroup
-      LookupKey.all(:conditions => {:puppetclass_id =>klasses.flatten } ).each do |k|
-        p[k.to_s] = k.value_for(self)
-      end unless klasses.empty?
-    end
-    @cached_lookup_keys_params = p
-  end
-
   def self.importHostAndFacts yaml
     facts = YAML::load yaml
     case facts
@@ -662,6 +654,32 @@ class Host < Puppet::Rails::Host
   end
 
   private
+  def lookup_keys_params
+    return {} unless Setting["Enable_Smart_Variables_in_ENC"]
+
+    p = {}
+    klasses = all_puppetclasses.map(&:id).flatten
+    LookupKey.where(:puppetclass_id => klasses ).each do |k|
+      p[k.to_s] = k.value_for(self)
+    end unless klasses.empty?
+    p
+  end
+
+  def lookup_keys_class_params
+    p={}
+    classes = all_puppetclasses
+    keys    = EnvironmentClass.parameters_for_class(classes.map(&:id), environment_id).group_by(&:puppetclass_id)
+    classes.each do |klass|
+      p[klass.name] = nil
+      keys[klass.id].map(&:lookup_key).each do |lookup_key|
+        p[klass.name] ||= {}
+        value = lookup_key.value_for(self)
+        p[klass.name].merge!({lookup_key.key => value})
+      end if keys[klass.id]
+    end
+    p
+  end
+
   # align common mac and ip address input
   def normalize_addresses
     # a helper for variable scoping
diff --git a/app/models/lookup_key.rb b/app/models/lookup_key.rb
index ac46affeb..2396cfcd0 100644
--- a/app/models/lookup_key.rb
+++ b/app/models/lookup_key.rb
@@ -1,56 +1,98 @@
 class LookupKey < ActiveRecord::Base
+  include Authorization
 
-  VALIDATION_TYPES = %w( regexp list )
+  KEY_TYPES = %w( string boolean integer real array hash yaml json )
+  VALIDATOR_TYPES = %w( regexp list )
+
+  TRUE_VALUES = [true, 1, '1', 't', 'T', 'true', 'TRUE', 'on', 'ON', 'yes', 'YES', 'y', 'Y'].to_set
+  FALSE_VALUES = [false, 0, '0', 'f', 'F', 'false', 'FALSE', 'off', 'OFF', 'no', 'NO', 'n', 'N'].to_set
 
   KEY_DELM = ","
   EQ_DELM  = "="
 
+  serialize :default_value
+
   belongs_to :puppetclass
+  has_many :environment_classes, :dependent => :destroy
+  has_many :environments, :through => :environment_classes, :uniq => true
+  has_one :param_class, :through => :environment_classes, :class_name => 'Puppetclass', :source => :puppetclass
+
   has_many :lookup_values, :dependent => :destroy, :inverse_of => :lookup_key
   accepts_nested_attributes_for :lookup_values, :reject_if => lambda { |a| a[:value].blank? }, :allow_destroy => true
-  validates_uniqueness_of :key
-  validates_presence_of :key, :puppetclass_id
-  validates_inclusion_of :validator_type, :in => VALIDATION_TYPES, :message => "invalid", :allow_blank => true, :allow_nil => true
-  validate :validate_range_rule, :validate_range, :validate_list, :validate_regexp
+
+  before_validation :validate_and_cast_default_value
+
+  validates_uniqueness_of :key, :unless => Proc.new{|p| p.is_param?}
+  validates_presence_of :key
+  validates_presence_of :puppetclass_id, :unless => Proc.new {|k| k.is_param?}
+  validates_inclusion_of :validator_type, :in => VALIDATOR_TYPES, :message => "invalid", :allow_blank => true, :allow_nil => true
+  validates_inclusion_of :key_type, :in => KEY_TYPES, :message => "invalid", :allow_blank => true, :allow_nil => true
+  validate :validate_list, :validate_regexp
   validates_associated :lookup_values
+  validate :ensure_type
 
   before_save :sanitize_path
 
   scoped_search :on => :key, :complete_value => true, :default_order => true
-  scoped_search :in => :puppetclass, :on => :name, :rename => :puppetclass, :complete_value => true
+  scoped_search :on => :override, :complete_value => {:true => true, :false => false}
+  scoped_search :in => :param_class, :on => :name, :rename => :puppetclass, :complete_value => true
   scoped_search :in => :lookup_values, :on => :value, :rename => :value, :complete_value => true
 
   default_scope :order => 'LOWER(lookup_keys.key)'
 
   def to_param
-    key
+    "#{id}-#{key}"
   end
 
   def to_s
     key
   end
 
-  #TODO: use SQL coalesce to minimize the amount of queries
   def value_for host
     path2matches(host).each do |match|
       if (v = lookup_values.find_by_match(match))
         return v.value
       end
-    end
+    end if lookup_values.any?
     default_value
   end
 
   def path
-    read_attribute(:path) || array2path(Setting["Default_variables_Lookup_Path"])
+    path = read_attribute(:path)
+    path.blank? ? array2path(Setting["Default_variables_Lookup_Path"]) : path
   end
 
   def path=(v)
-    return if v == array2path(Setting["Default_variables_Lookup_Path"])
-    write_attribute(:path, v)
+    using_default = v.tr("\r","") == array2path(Setting["Default_variables_Lookup_Path"])
+    write_attribute(:path, using_default ? nil : v)
+  end
+
+  def default_value_before_type_cast
+    value_before_type_cast default_value
+  end
+
+  def value_before_type_cast val
+    case validator_type.to_sym
+      when :json
+        val = JSON.dump val
+      when :yaml, :hash
+        val = YAML.dump val
+        val.sub! /\A---\s*$\n/, ''
+      when  :array
+        val = val.inspect
+    end unless validator_type.blank?
+    val
+  end
+
+  # Returns the casted value, or raises a TypeError
+  def cast_validate_value value
+    method = "cast_value_#{validator_type}".to_sym
+    return value unless self.respond_to? method, true
+    self.send(method, value) rescue raise TypeError
   end
 
   def as_json(options={})
-    super({:only => [:key, :description, :default_value, :id]}.merge(options))
+    super({:only => [:key, :is_param, :required, :override, :description, :default_value, :id]}.merge(options))
   end
 
   private
@@ -94,11 +136,6 @@ class LookupKey < ActiveRecord::Base
     self.path = path.tr("\s","").downcase unless path.blank?
   end
 
-  def validate_range_rule
-    return true unless (validator_type == 'range')
-    self.errors.add(:validator_rule, "is invalid") and return false unless validator_rule =~ /^(\d|"[a-z]"|'[a-z]')+\.\.(\d|"[b-z]"|'[b-z]')+$/
-  end
-
   def array2path array
     raise "invalid path" unless array.is_a?(Array)
     array.map do |sub_array|
@@ -106,19 +143,97 @@ class LookupKey < ActiveRecord::Base
     end.join("\n")
   end
 
+
+  def validate_and_cast_default_value
+    return true if default_value.nil?
+    begin
+      self.default_value = cast_validate_value self.default_value
+      true
+    rescue
+      errors.add(:default_value, "is invalid")
+      false
+    end
+  end
+
+  def cast_value_boolean value
+    return true if TRUE_VALUES.include? value
+    return false if FALSE_VALUES.include? value
+    raise TypeError
+  end
+
+  def cast_value_integer value
+    return value.to_i if value.is_a?(Numeric)
+
+    if value.is_a?(String)
+      if value =~ /^0x[0-9a-f]+$/i
+        value.to_i(16)
+      elsif value =~ /^0[0-7]+$/
+        value.to_i(8)
+      elsif value =~ /^-?\d+$/
+        value.to_i
+      else
+        raise TypeError
+      end
+    end
+  end
+
+  def cast_value_real value
+    return value if value.is_a? Numeric
+    if value.is_a?(String)
+      if value =~ /^[-+]?(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][-+]?\d+)?$/
+        value.to_f
+      else
+        cast_value_integer value
+      end
+    end
+  end
+
+  def load_yaml_or_json value
+    return value unless value.is_a? String
+    begin
+      JSON.load value
+    rescue
+      YAML.load value
+    end
+  end
+
+  def cast_value_array value
+    return value if value.is_a? Array
+    return value.to_a if not value.is_a? String and value.is_a? Enumerable
+    value = load_yaml_or_json value
+    raise TypeError unless value.is_a? Array
+    value
+  end
+
+  def cast_value_hash value
+    return value if value.is_a? Hash
+    value = load_yaml_or_json value
+    raise TypeError unless value.is_a? Hash
+    value
+  end
+
+  def cast_value_yaml value
+    value = YAML.load value
+  end
+
+  def cast_value_json value
+    value = JSON.load value
+  end
+
+  def ensure_type
+    if puppetclass_id.present? and is_param?
+      self.errors.add(:base, 'Global variable or class Parameter, not both')
+    end
+  end
+
   def validate_regexp
     return true unless (validator_type == 'regexp')
     errors.add(:default_value, "is invalid") and return false unless (default_value =~ /#{validator_rule}/)
   end
 
-  def validate_range
-    return true unless (validator_type == 'range')
-    errors.add(:default_value, "not within range #{validator_rule}") and return false unless eval(validator_rule).include?(default_value)
-  end
-
   def validate_list
     return true unless (validator_type == 'list')
-    errors.add(:default_value, "not in list") and return false unless validator_rule.split(KEY_DELM).map(&:strip).include?(default_value)
+    errors.add(:default_value, "#{default_value} is not one of #{validator_rule}") and return false unless validator_rule.split(KEY_DELM).map(&:strip).include?(default_value)
   end
 
 end
diff --git a/app/models/lookup_value.rb b/app/models/lookup_value.rb
index e790b26a8..6a4fb363f 100644
--- a/app/models/lookup_value.rb
+++ b/app/models/lookup_value.rb
@@ -1,11 +1,14 @@
 class LookupValue < ActiveRecord::Base
   include Authorization
-  belongs_to :lookup_key
+  belongs_to :lookup_key, :counter_cache => true
   validates_uniqueness_of :match, :scope => :lookup_key_id
-  validates_presence_of :match, :value
+  validates_presence_of :match
   delegate :key, :to => :lookup_key
-  validate :validate_range, :validate_list, :validate_regexp, :validate_match
   before_validation :sanitize_match
+  before_validation :validate_and_cast_value
+  validate :validate_list, :validate_regexp
+
+  serialize :value
 
   scope :default, :conditions => { :match => "default" }, :limit => 1
 
@@ -19,34 +22,41 @@ class LookupValue < ActiveRecord::Base
     value
   end
 
-  private
+  def value_before_type_cast
+    return self.value if lookup_key.nil?
+    lookup_key.value_before_type_cast self.value
+  end
 
-  # TODO: ensures that the match contain only allowed path elements
-  def validate_match
+  def as_json(options={})
+    super({:only => [:value, :match, :lookup_key_id, :id]}.merge(options))
   end
 
+  private
+
   #TODO check multi match with matchers that have space (hostgroup = web servers,environment = production)
   def sanitize_match
     self.match = match.split(LookupKey::KEY_DELM).map {|s| s.split(LookupKey::EQ_DELM).map(&:strip).join(LookupKey::EQ_DELM)}.join(LookupKey::KEY_DELM) unless match.blank?
   end
 
+  def validate_and_cast_value
+    return true if self.marked_for_destruction?
+    begin
+      self.value = lookup_key.cast_validate_value self.value
+      true
+    rescue
+      errors.add(:value, "is invalid #{lookup_key.key_type}")
+      false
+    end
+  end
+
   def validate_regexp
     return true unless (lookup_key.validator_type == 'regexp')
     errors.add(:value, "is invalid") and return false unless (value =~ /#{lookup_key.validator_rule}/)
   end
 
-  def validate_range
-    return true unless (lookup_key.validator_type == 'range')
-    errors.add(:value, "not within range #{lookup_key.validator_rule}") and return false unless eval(lookup_key.validator_rule).include?(value)
-  end
-
   def validate_list
     return true unless (lookup_key.validator_type == 'list')
-    errors.add(:value, "not in list") and return false unless lookup_key.validator_rule.split(LookupKey::KEY_DELM).map(&:strip).include?(value)
-  end
-
-  def as_json(options={})
-    super({:only => [:value, :match, :lookup_key_id, :id]}.merge(options))
+    errors.add(:value, "#{value} is not one of #{lookup_key.validator_rule}") and return false unless lookup_key.validator_rule.split(LookupKey::KEY_DELM).map(&:strip).include?(value)
   end
 
 end
diff --git a/app/models/puppet_class_importer.rb b/app/models/puppet_class_importer.rb
new file mode 100644
index 000000000..29c59e5ab
--- /dev/null
+++ b/app/models/puppet_class_importer.rb
@@ -0,0 +1,269 @@
+class PuppetClassImporter
+
+  def initialize args = { }
+    @foreman_classes = { }
+    @proxy_classes   = { }
+    if args[:proxy]
+      @proxy = args[:proxy]
+    elsif args[:url]
+      @proxy = ProxyAPI::Puppet.new(:url => args[:url])
+    else
+      url = SmartProxy.puppet_proxies.first.try(:url)
+      raise "Can't find a valid Proxy with a Puppet feature" if url.blank?
+      @proxy = ProxyAPI::Puppet.new(:url => url)
+    end
+  end
+
+  # return changes hash, currently exists to keep compatibility with importer html
+  def changes
+    changes = { 'new' => { }, 'obsolete' => { }, 'updated' => { } }
+
+    actual_environments.each do |env|
+      new     = new_classes_for(env)
+      old     = removed_classes_for(env)
+      updated = updated_classes_for(env)
+      changes['new'][env] = new if new.any?
+      changes['obsolete'][env] = old if old.any?
+      changes['updated'][env] = updated if updated.any?
+    end
+
+    old_environments.each do |env|
+      changes['obsolete'][env] ||= []
+      changes['obsolete'][env] << "_destroy_" unless actual_environments.include?(env)
+    end
+    changes
+  end
+
+  # Update the environments and puppetclasses based upon the user's selection
+  # It does a best attempt and can fail to perform all operations due to the
+  # user requesting impossible selections. Repeat the operation if errors are
+  # shown, after fixing the request.
+  # +changed+ : Hash with two keys: :new and :obsolete.
+  #               changed[:/new|obsolete/] is and Array of Strings
+  # Returns   : Array of Strings containing all record errors
+  def obsolete_and_new changes = { }
+    return if changes.empty?
+    changes.values.map(&:keys).flatten.uniq.each do |env_name|
+      if changes['new'] and changes['new'][env_name].try(:any?) # we got new classes
+        add_classes_to_foreman(env_name, JSON.parse(changes['new'][env_name]))
+      end
+      if changes['obsolete'] and changes['obsolete'][env_name].try(:any?) # we need to remove classes
+        remove_classes_from_foreman(env_name, JSON.parse(changes['obsolete'][env_name]))
+      end
+      if changes['updated'] and changes['updated'][env_name].try(:any?) # we need to update classes
+        update_classes_in_foreman(env_name, JSON.parse(changes['updated'][env_name]))
+      end
+    end
+    []
+    #rescue => e
+    #  logger.error(e)
+    #  [e.to_s]
+  end
+
+  def new_classes_for environment
+    old_classes = db_classes_name(environment)
+    HashWithIndifferentAccess[
+      actual_classes(environment).values.map do |actual_class|
+        [actual_class.to_s, { "new" => actual_class.parameters }] unless old_classes.include?(actual_class.to_s)
+      end.compact
+    ]
+  end
+
+  def removed_classes_for environment
+    db_classes_name(environment) - actual_classes_name(environment)
+  end
+
+  def updated_classes_for environment
+    return [] unless db_environments.include?(environment) && actual_environments.include?(environment)
+    HashWithIndifferentAccess[
+      db_classes(environment).map do |db_class|
+        params = EnvironmentClass.all_parameters_for_class(db_class.id, find_or_create_env(environment).id).map(&:lookup_key)
+        compare_classes(environment, db_class.name, params)
+      end.compact
+    ]
+  end
+
+  # This method check if the puppet class exists in this environment, and compare the class params.
+  # Changes in the params are categorized to new parameters, removed parameters and parameters with a new
+  # default value.
+  def compare_classes(environment, klass, db_params)
+    return [] unless (actual_class = actual_classes(environment)[klass])
+    actual_params  = actual_class.parameters
+    db_param_names = db_params.map(&:to_s)
+
+    param_changes = { }
+    old           = db_param_names - actual_params.keys
+    param_changes['obsolete'] = old if old.any?
+    new = actual_params.reject { |key, value| db_param_names.include?(key) }
+    param_changes['new'] = new if new.any?
+    updated = updated_classes(actual_params, db_params)
+    param_changes['updated'] = updated if updated.any?
+    [klass, param_changes] if param_changes.keys.any?
+  end
+
+  def updated_classes(actual_params, db_params)
+    updated = { }
+    db_params.map do |p|
+      param_name = p.to_s
+      if !p.override && actual_params.has_key?(param_name) && actual_params[param_name] != p.default_value
+        updated[param_name] = actual_params[param_name]
+      end
+    end
+    updated
+  end
+
+
+  def db_environments
+    @foreman_envs ||= (Environment.all.map(&:name) - ignored_environments)
+  end
+
+  def actual_environments
+    @proxy_envs ||= (proxy.environments.map(&:to_s) - ignored_environments)
+  end
+
+  def new_environments
+    actual_environments - db_environments
+  end
+
+  def old_environments
+    db_environments - actual_environments
+  end
+
+  def db_classes environment
+    return @foreman_classes[environment] if @foreman_classes[environment]
+    return [] unless (env = Environment.find_by_name(environment))
+    @foreman_classes[environment] = env.puppetclasses.includes(:lookup_keys, :class_params)
+  end
+
+  def db_classes_name environment
+    db_classes(environment).map(&:name)
+  end
+
+  def actual_classes environment
+    @proxy_classes[environment] ||= proxy.classes(environment).reject do |key, value|
+      ignored_classes.find { |filter| filter.is_a?(Regexp) && filter =~ key }
+    end
+  end
+
+  def actual_classes_name environment
+    actual_classes(environment).keys
+  end
+
+  private
+  attr_reader :proxy
+
+  def ignored_environments
+    ignored_file[:ignored] || []
+  end
+
+  def ignored_classes
+    ignored_file[:filters] || []
+  end
+
+  def ignored_file
+    return @ignored_file if @ignored_file
+    file          = File.join(Rails.root.to_s, "config", "ignored_environments.yml")
+    @ignored_file = File.exist?(file) ? YAML.load_file(file) : { }
+  rescue => e
+    logger.warn "Failed to parse environment ignore file: #{e}"
+    @ignored_file = { }
+  end
+
+  def logger
+    @logger ||= Rails.logger
+  end
+
+  def load_classes_from_json blob
+    ActiveSupport::JSON.decode blob
+  end
+
+  def add_classes_to_foreman env_name, klasses
+    env         = find_or_create_env env_name
+    new_classes = klasses.map { |k| Puppetclass.find_or_create_by_name(k[0]) }
+
+    new_classes.each do |new_class|
+      EnvironmentClass.create! :puppetclass_id => new_class.id, :environment_id => env.id
+      class_params = klasses[new_class.to_s]
+      add_new_parameter(env, new_class, class_params) if class_params.any?
+    end
+  end
+
+  def update_classes_in_foreman environment, klasses
+    env        = find_or_create_env(environment)
+    db_classes = env.puppetclasses.where(:name => klasses.keys)
+    db_classes.each do |db_class|
+      changed_params = klasses[db_class.to_s]
+      # Add new parameters
+      add_new_parameter(env, db_class, changed_params) if changed_params["new"]
+      # Unbind old parameters
+      remove_parameter(env, db_class, changed_params) if changed_params["obsolete"]
+      # Update parameters (affects solely the default value)
+      update_parameter(db_class, changed_params) if changed_params["updated"]
+    end
+  end
+
+  def update_parameter(db_class, changed_params)
+    changed_params["updated"].each do |param_name, value|
+      key = db_class.class_params.find_by_key param_name
+      if key.override == false
+        key.default_value = value
+        key.save!
+      end
+    end
+  end
+
+  def remove_parameter(env, db_class, changed_params)
+    changed_params["obsolete"].each do |param_name, value|
+      key        = db_class.class_params.find_by_key param_name
+      key_in_env = EnvironmentClass.key_in_environment(env, db_class, key)
+
+      if key && key_in_env && !key.override?
+        #detach
+        key_in_env.destroy
+        # destroy if the key is not in any environment.
+        key.destroy unless EnvironmentClass.is_in_any_environment(db_class, key)
+      end
+    end
+  end
+
+  def add_new_parameter(env, klass, changed_params)
+    changed_params["new"].map do |param_name, value|
+      param = find_or_create_puppet_class_param klass, param_name, value
+      EnvironmentClass.create! :puppetclass_id => klass.id, :environment_id => env.id, :lookup_key_id => param.id
+    end
+  end
+
+  def remove_classes_from_foreman env_name, klasses
+    env     = find_or_create_env(env_name)
+    classes = find_existing_foreman_classes(klasses)
+    env.puppetclasses.delete classes
+    # remove all old classes from hosts
+    HostClass.joins(:host).where(:hosts => { :environment_id => env.id }, :puppetclass_id => classes).destroy_all
+    if klasses.include? '_destroy_'
+      # we can't guaranty that the env would be removed as it might have hosts attached to it.
+      env.destroy
+    end
+    # remove all klasses that have no environment now
+    classes.not_in_any_environment.destroy_all
+  end
+
+  def logger
+    @logger ||= Rails.logger
+  end
+
+  def find_existing_foreman_classes klasses = []
+    Puppetclass.where(:name => klasses)
+  end
+
+  def find_or_create_env env
+    Environment.where(:name => env).first || Environment.create!(:name => env)
+  end
+
+  def find_or_create_puppet_class_param klass, param_name, value
+    klass.class_params.where(:key => param_name).first ||
+      LookupKey.create!(:key            => param_name, :is_param => true,
+                        :required       => value.nil?, :override => value.nil?, :default_value => value,
+                        :key_type => Foreman::ImporterPuppetclass.suggest_key_type(value))
+  end
+
+end
diff --git a/app/models/puppetclass.rb b/app/models/puppetclass.rb
index 23c3ada27..13dc0e207 100644
--- a/app/models/puppetclass.rb
+++ b/app/models/puppetclass.rb
@@ -1,6 +1,7 @@
 class Puppetclass < ActiveRecord::Base
   include Authorization
-  has_and_belongs_to_many :environments
+  has_many :environment_classes, :dependent => :destroy
+  has_many :environments, :through => :environment_classes, :uniq => true
   has_and_belongs_to_many :operatingsystems
   has_and_belongs_to_many :hostgroups
   has_many :host_classes, :dependent => :destroy
@@ -8,10 +9,12 @@ class Puppetclass < ActiveRecord::Base
 
   has_many :lookup_keys, :inverse_of => :puppetclass
   accepts_nested_attributes_for :lookup_keys, :reject_if => lambda { |a| a[:key].blank? }, :allow_destroy => true
-
+  # param classes
+  has_many :class_params, :through => :environment_classes, :uniq => true,
+    :class_name => 'LookupKey', :source => :lookup_key, :conditions => 'environment_classes.lookup_key_id is NOT NULL'
+  accepts_nested_attributes_for :class_params, :reject_if => lambda { |a| a[:key].blank? }, :allow_destroy => true
   validates_uniqueness_of :name
   validates_presence_of :name
-  validates_associated :environments
   validates_format_of :name, :with => /\A(\S+\s?)+\Z/, :message => "can't be blank or contain white spaces."
   audited
 
@@ -23,7 +26,9 @@ class Puppetclass < ActiveRecord::Base
   scoped_search :in => :environments, :on => :name, :complete_value => :true, :rename => "environment"
   scoped_search :in => :hostgroups,   :on => :name, :complete_value => :true, :rename => "hostgroup"
   scoped_search :in => :hosts, :on => :name, :complete_value => :true, :rename => "host", :ext_method => :search_by_host, :only_explicit => true
+  scoped_search :in => :class_params, :on => :key, :complete_value => :true
 
+  scope :not_in_any_environment, includes(:environment_classes).where(:environment_classes => {:environment_id => nil})
 
   def to_param
     name
@@ -54,7 +59,6 @@ class Puppetclass < ActiveRecord::Base
     name.gsub(module_name+"::","")
   end
 
-
   # Populates the rdoc tree with information about all the classes in your modules.
   #   Firstly, we prepare the modules tree
   #   Secondly we run puppetdoc over the modulespath and manifestdir for all environments
diff --git a/app/views/common/_puppetclasses_or_envs_changed.html.erb b/app/views/common/_puppetclasses_or_envs_changed.html.erb
index 2b029963c..b75927316 100644
--- a/app/views/common/_puppetclasses_or_envs_changed.html.erb
+++ b/app/views/common/_puppetclasses_or_envs_changed.html.erb
@@ -2,21 +2,21 @@
 <%= form_tag "/#{controller_path}/obsolete_and_new" do -%>
   <fieldset>
     <legend>Accept these environment changes found in puppet? </legend>
-      <table>
-        <th>Environment</th><th>Operation</th><th>Classes</th>
-        <% for kind in ["new", "obsolete"] -%>
+      <table class="table table-striped">
+        <th>Environment</th><th>Operation</th><th>Puppet Modules</th>
+        <% for kind in ["new", "obsolete", "updated"] -%>
           <% unless (envs = @changed[kind]).empty? -%>
             <% for env in envs.keys.sort -%>
-              <tr class="<%= cycle("even", "odd")-%>" >
+              <tr>
                 <td>
                   <%= check_box_tag "changed[#{kind}][#{env}]", @changed[kind][env].to_json, true %>
                   <%= env -%>
                 </td>
                 <td>
-                  <%= kind == "new" ? "Add:" : "Remove:" -%>
+                  <%= {"new" => "Add:", "obsolete" => "Remove:", "updated" => "Update:"}[kind] -%>
                 </td>
-                <% pcs = @changed[kind][env] -%>
                 <td>
+                  <% pcs = @changed[kind][env] -%>
                   <%= class_update_text pcs, env %>
                 </td>
               </tr>
@@ -29,4 +29,4 @@
     <%= link_to "Cancel","/" + controller_path, :class => "btn" %>
     <%= submit_tag "Update", :class => "btn btn-primary" %>
   </div>
-<% end -%>
+<% end -%>
\ No newline at end of file
diff --git a/app/views/lookup_keys/_fields.html.erb b/app/views/lookup_keys/_fields.html.erb
index b1065bc51..a7ce0386b 100644
--- a/app/views/lookup_keys/_fields.html.erb
+++ b/app/views/lookup_keys/_fields.html.erb
@@ -1,29 +1,44 @@
+
 <div <%= "id=#{(f.object.key || 'new_lookup_keys').to_s.gsub(' ','_')} class='tab-pane fields' " %> >
+  <% is_param = f.object.is_param -%>
+
+  <%= text_f(f, :environment_classes, :value => f.object.environment_classes.map(&:environment).to_sentence, :label=> 'Puppet Environments', :disabled=>true) if is_param%>
+
   <%= remove_child_link "Remove #{f.object.new_record? ? "Variable" : f.object}", f , {:class => 'btn btn-danger hide'} unless controller_name == "lookup_keys" %>
-  <%= text_f f, :key, :label => "Name" %>
-  <%= text_f f, :description %>
-  <%= text_f f, :default_value, :class => "span6", :help_inline => popover("?","value to use when there is no match", :title => "Default Value").html_safe %>
-  <%= selectable_f f, :validator_type, options_for_select(LookupKey::VALIDATION_TYPES, f.object.validator_type),
-    { :include_blank => "string"},
-    { :label => "Type Validator", :class => "medium",
-      :help_inline => popover("?","<dl> <dt>String</dt> <dd>everything goes</dd> <dt>Regexp</dt> <dd>regular expression to verify the value</dd>
-       <dt>List</dt> <dd>comma seperated values, e.g. 80,443<dd> </dt>", :title => "How values are validated").html_safe}
-  %>
-  <%= text_f f, :validator_rule, :label => "Validator constraint", :class => "span6",
-    :help_inline => popover("?","Values to match Type validator, leave blank if its String", :title => "Validator constraint")%>
-  <%= textarea_f f, :path, :rows => "4", :value => f.object.path, :label => "Order",
-    :help_inline => popover("?", "The order in which matchers keys are processed, first match wins.<br>
+  <%= text_f f, :key, :label => "Name", :disabled => f.object.is_param %>
+  <%= f.hidden_field :key if f.object.is_param %>
+  <%= textarea_f f, :description, :rows => :auto, :class=> "span6" %>
+
+  <%= show_puppet_class f %>
+  <%= checkbox_f(f, :override, :label => "Override", :onchange=>'toggleOverrideValue(this)', :disabled => f.object.required,
+                    :help_inline => popover("?", "Whether the smart-variable should override the Puppet class default value.",
+                    :title => "Override Value")) if is_param%>
+  <%= param_type_selector f  %>
+  <%= textarea_f f, :default_value, :value => f.object.default_value_before_type_cast , :disabled => (f.object.is_param && !f.object.override), :class => "span6", :rows => :auto, :help_inline => popover("?","value to use when there is no match", :title => "Default Value") %>
+  <div <%= "id=#{(f.object.key || 'new_lookup_keys').to_s.gsub(' ','_')}_lookup_key_override_value" %> <%= "style='display:none;'" if (f.object.is_param && !f.object.override) %>>
+    <legend>Optional Input Validator</legend>
+
+    <%= checkbox_f(f, :required, :onchange=>'toggleMandatory(this)', :disabled => !f.object.override,
+                      :help_inline => popover("?", "If checked, will raise an error if there is no default value and no matcher provide a value.",
+                      :title => "Required Parameter")) if is_param %>
+
+    <%= validator_type_selector f  %>
+    <%= text_f f, :validator_rule %>
+    <legend>Override Value For Specific Hosts</legend>
+    <%= textarea_f f, :path, :rows => :auto, :label => "Order", :value => f.object.path,
+                   :help_inline => popover("?", "The order in which matchers keys are processed, first match wins.<br>
     You may use multiple attributes as a matcher key, for example, an order of <code>hostgroup, environment</code>
     would expect a matcher such as <code>hostgroup = \"web servers\", environment = production</code>", :title => "The order in which values are resolved").html_safe
-  %>
+    %>
 
-  <%# the following field is required to see child validations %>
-  <%= f.hidden_field :updated_at, :value => Time.now.to_i %>
-  <div class="children_fields">
-    <%= new_child_fields_template(f, :lookup_values, {:partial => "lookup_keys/value"}) %>
-    <%= f.fields_for :lookup_values do |lookup_values| %>
-      <%= render 'lookup_keys/value', :f => lookup_values %>
-    <% end %>
-    <%= add_child_link "+ Add Matcher-Value", :lookup_values, { :title => 'add a new matcher-value pair'} %>
+    <%# the following field is required to see child validations %>
+    <%= f.hidden_field :updated_at, :value => Time.now.to_i %>
+    <div class="children_fields">
+      <%= new_child_fields_template(f, :lookup_values, {:partial => "lookup_keys/value"}) %>
+      <%= f.fields_for :lookup_values do |lookup_values| %>
+          <%= render 'lookup_keys/value', :f => lookup_values %>
+      <% end %>
+      <%= add_child_link "+ Add Matcher-Value", :lookup_values, { :title => 'add a new matcher-value pair'} %>
+    </div>
   </div>
 </div>
diff --git a/app/views/lookup_keys/_value.html.erb b/app/views/lookup_keys/_value.html.erb
index 6adfff093..804643469 100644
--- a/app/views/lookup_keys/_value.html.erb
+++ b/app/views/lookup_keys/_value.html.erb
@@ -2,5 +2,5 @@
   <%= text_f f, :match, :class => "span6",
     :help_inline => popover("?", "Matcher is a combination of an attribute and its value, if they match, the value below would be provided.<br> You may use any attribute foreman knows about, such as facts etc for example: <code> domain = example.com </code> or <code> is_virtual = true</code>", :title => "Matcher")
   %>
-  <%= text_f f, :value, :class => "span6", :help_inline => remove_child_link(icon_text("remove"), f, {:title => 'remove value'}) %>
+  <%= text_f f, :value, :value => f.object.value_before_type_cast, :class => "span6", :help_inline => remove_child_link(icon_text("remove"), f, {:title => 'remove value'}) %>
 </div>
diff --git a/app/views/lookup_keys/index.html.erb b/app/views/lookup_keys/index.html.erb
index a5fc4dd61..509454a7b 100644
--- a/app/views/lookup_keys/index.html.erb
+++ b/app/views/lookup_keys/index.html.erb
@@ -11,9 +11,14 @@
 <% @lookup_keys.each do |lookup_key| %>
   <tr class="<%= cycle("even", "odd") -%>">
     <td><%= link_to_if_authorized h(lookup_key.key), hash_for_edit_lookup_key_path(:id => lookup_key) %></td>
-    <td><%= link_to_if_authorized h(lookup_key.puppetclass), hash_for_edit_puppetclass_path(:id => lookup_key.puppetclass) if lookup_key.puppetclass %></td>
+
+    <td>
+      <% puppet_class = lookup_key.param_class %>
+      <%= link_to_if_authorized h(puppet_class), hash_for_edit_puppetclass_path(:id => puppet_class) if puppet_class %></td>
     <td><%=h lookup_key.lookup_values.count %></td>
     <td><%= display_delete_if_authorized hash_for_lookup_key_path(:id => lookup_key), :confirm => "Delete #{lookup_key.key}?" %></td>
   </tr>
 <% end %>
 </table>
+<%= page_entries_info @lookup_keys, :model => "Smart Variables"%>
+<%= will_paginate     @lookup_keys %>
diff --git a/app/views/puppetclasses/_form.html.erb b/app/views/puppetclasses/_form.html.erb
index bf86a322e..6c4b7cb42 100644
--- a/app/views/puppetclasses/_form.html.erb
+++ b/app/views/puppetclasses/_form.html.erb
@@ -4,6 +4,7 @@
   <%= base_errors_for @puppetclass %>
   <ul class="nav nav-tabs" data-tabs="tabs">
     <li class="active"><a href="#primary" data-toggle="tab">Puppet Class</a></li>
+    <li><a href="#smart_class_param" data-toggle="tab">Smart Class Parameter</a></li>
     <li><a href="#smart_vars" data-toggle="tab">Smart Variables</a></li>
   </ul>
 
@@ -11,11 +12,40 @@
 
     <div class="tab-pane active" id="primary">
       <%= text_f f, :name %>
-
-      <%= multiple_checkboxes f, :environments, @puppetclass, Environment, :label => "Puppet Environments" %>
+      <%= text_f f, :environments, :value => @puppetclass.environments.to_sentence, :label=> 'Puppet Environments', :disabled => true %>
       <%= multiple_checkboxes f, :hostgroups,   @puppetclass, Hostgroup,   :label => "Hostgroups" %>
     </div>
 
+    <div class="tab-pane" id="smart_class_param">
+      <% if @puppetclass.class_params.empty? -%>
+          <div class="alert alert-message alert-success">
+            <a class="close" href="#" data-dismiss="alert">&times;</a>
+            <p><strong>This Puppet class has no parameters in its signature.</strong><br>
+                To update the class signature, go to the Puppet Classes page and select 'Import'.</p>
+          </div>
+      <% else -%>
+          <div class="control-group">
+            <label class="control-label">Filter By Environment</label>
+            <div class="controls">
+              <%= select_tag "environment_filter", options_from_collection_for_select(@puppetclass.environments, "id", "name"),
+                             :include_blank => "All - (Not filtered)", :onchange=>'filterByEnvironment(this)'%>
+            </div>
+          </div>
+          <div class="tabbable tabs-left">
+            <ul class="nav nav-tabs smart-var-tabs span2" data-tabs="pills">
+              <% @puppetclass.class_params.each do |key| -%>
+                  <li data-used-environments=<%= key.environments.map(&:to_s).to_json %>><a data-toggle="tab" id="pill_<%= key.to_s.gsub(' ','_') %>" href="#<%= key.to_s.gsub(' ','_') %>"><%= key %><span class="label label-important fr">&times;</span></a></li>
+              <% end -%>
+            </ul>
+
+            <div class="tab-content span9 smart-var-content">
+              <%= f.fields_for :class_params do |lookup_keys_form| %>
+                  <%= render 'lookup_keys/fields', :f => lookup_keys_form  %>
+              <% end %>
+            </div>
+          </div>
+      <% end -%>
+    </div>
     <div class="tab-pane" id="smart_vars">
       <% if @puppetclass.lookup_keys.empty? -%>
           <div class="alert alert-message alert-warning">
diff --git a/app/views/puppetclasses/index.html.erb b/app/views/puppetclasses/index.html.erb
index c7e0cecbb..f37ea6347 100644
--- a/app/views/puppetclasses/index.html.erb
+++ b/app/views/puppetclasses/index.html.erb
@@ -15,13 +15,13 @@
     <tr class="<%= cycle("even", "odd") -%>">
       <td><%=link_to_if_authorized h(puppetclass.name), hash_for_edit_puppetclass_path(:id => puppetclass) %></td>
       <td>
-        <% for environment in puppetclass.environments -%>
+        <% puppetclass.environments.uniq.each do |environment| -%>
           <%= link_to_function environment, 'show_rdoc(this)', :'data-url' => rdoc_classes_path(environment, puppetclass.name) %>
         <% end %>
       </td>
       <td><%= puppetclass.hostgroups.map {|hg| link_to_if_authorized hg, hash_for_edit_hostgroup_path(:id=>hg)}.to_sentence.html_safe %></td>
       <td> <%= link_to host_counter(puppetclass), hosts_path(:search => "class = #{puppetclass.name}")%></td>
-      <td><%= link_to @keys_counter[puppetclass.id] || 0, puppetclass_lookup_keys_path(puppetclass) %> </td>
+      <td><%= @keys_counter[puppetclass.name] || 0 %> </td>
       <td>
         <%= display_delete_if_authorized hash_for_puppetclass_path(:id => puppetclass), :confirm => "Delete #{puppetclass.name}?" %>
       </td>
diff --git a/config/ignored_environments.yml.sample b/config/ignored_environments.yml.sample
new file mode 100644
index 000000000..08a39c4f1
--- /dev/null
+++ b/config/ignored_environments.yml.sample
@@ -0,0 +1,9 @@
+#:ignored:
+#  - test
+
+:filters:
+  - !ruby/regexp '/params$/'
+  - !ruby/regexp '/base$/'
+  - !ruby/regexp '/install$/'
+  - !ruby/regexp '/service$/'
+  - !ruby/regexp '/config$/'
\ No newline at end of file
diff --git a/config/routes.rb b/config/routes.rb
index 551bcc173..ecc5355bd 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -78,6 +78,9 @@ Foreman::Application.routes.draw do
     resources :bookmarks, :except => [:show]
     resources :lookup_keys, :except => [:new, :create] do
       resources :lookup_values, :only => [:index, :create, :update, :destroy]
+      collection do
+        get 'auto_complete_search'
+      end
     end
 
     resources :facts, :only => [:index, :show] do
diff --git a/db/migrate/20120905095532_create_environment_classes.rb b/db/migrate/20120905095532_create_environment_classes.rb
new file mode 100644
index 000000000..4b258a958
--- /dev/null
+++ b/db/migrate/20120905095532_create_environment_classes.rb
@@ -0,0 +1,16 @@
+class CreateEnvironmentClasses < ActiveRecord::Migration
+  class EnvironmentClass < ActiveRecord::Base; end
+
+  def self.up
+    rename_table :environments_puppetclasses, :environment_classes
+    add_column :environment_classes, :id, :primary_key
+    add_column :environment_classes, :lookup_key_id, :integer
+  end
+
+  def self.down
+    remove_column :environment_classes, :id
+    remove_column :environment_classes, :lookup_key_id
+    rename_table :environment_classes, :environments_puppetclasses
+  end
+
+end
diff --git a/db/migrate/20120905131841_add_lookup_keys_override_and_required.rb b/db/migrate/20120905131841_add_lookup_keys_override_and_required.rb
new file mode 100644
index 000000000..c20250457
--- /dev/null
+++ b/db/migrate/20120905131841_add_lookup_keys_override_and_required.rb
@@ -0,0 +1,15 @@
+class AddLookupKeysOverrideAndRequired < ActiveRecord::Migration
+  def self.up
+    add_column :lookup_keys, :is_param, :boolean, :default => false
+    add_column :lookup_keys, :key_type, :string , :default => nil
+    add_column :lookup_keys, :override, :boolean, :default => false
+    add_column :lookup_keys, :required, :boolean, :default => false
+  end
+
+  def self.down
+    remove_column :lookup_keys, :is_param
+    remove_column :lookup_keys, :key_type
+    remove_column :lookup_keys, :override
+    remove_column :lookup_keys, :required
+  end
+end
diff --git a/db/migrate/20120916080843_add_lookup_values_count_to_lookup_keys.rb b/db/migrate/20120916080843_add_lookup_values_count_to_lookup_keys.rb
new file mode 100644
index 000000000..e9ea92fef
--- /dev/null
+++ b/db/migrate/20120916080843_add_lookup_values_count_to_lookup_keys.rb
@@ -0,0 +1,9 @@
+class AddLookupValuesCountToLookupKeys < ActiveRecord::Migration
+  def self.up
+    add_column :lookup_keys, :lookup_values_count, :integer, :default => 0
+  end
+
+  def self.down
+    remove_column :lookup_keys, :lookup_values_count
+  end
+end
diff --git a/db/migrate/20120916080926_cache_lookup_values_count.rb b/db/migrate/20120916080926_cache_lookup_values_count.rb
new file mode 100644
index 000000000..e33dd9487
--- /dev/null
+++ b/db/migrate/20120916080926_cache_lookup_values_count.rb
@@ -0,0 +1,8 @@
+class CacheLookupValuesCount < ActiveRecord::Migration
+  def self.up
+    execute "update lookup_keys set lookup_values_count=(select count(*) from lookup_values where lookup_key_id=lookup_keys.id)"
+  end
+
+  def self.down
+  end
+end
diff --git a/lib/foreman/controller/environments.rb b/lib/foreman/controller/environments.rb
index fd04674f6..580c5e395 100644
--- a/lib/foreman/controller/environments.rb
+++ b/lib/foreman/controller/environments.rb
@@ -4,7 +4,9 @@ module Foreman::Controller::Environments
 
   def import_environments
     begin
-      @changed = Environment.importClasses params[:proxy]
+      opts      = params[:proxy].blank? ? { } : { :url => SmartProxy.find(params[:proxy]).try(:url) }
+      @importer = PuppetClassImporter.new(opts)
+      @changed  = @importer.changes
     rescue => e
       if e.message =~ /puppet feature/i
         error "We did not find a foreman proxy that can provide the information, ensure that you have at least one Proxy with the puppet feature turned on."
@@ -14,7 +16,7 @@ module Foreman::Controller::Environments
       end
     end
 
-    if @changed["new"].size > 0 or @changed["obsolete"].size > 0
+    if @changed["new"].size > 0 or @changed["obsolete"].size > 0 or @changed["updated"].size > 0
       render "common/_puppetclasses_or_envs_changed"
     else
       notice "No changes to your environments detected"
@@ -23,7 +25,7 @@ module Foreman::Controller::Environments
   end
 
   def obsolete_and_new
-    if (errors = ::Environment.obsolete_and_new(params[:changed])).empty?
+    if (errors = ::PuppetClassImporter.new.obsolete_and_new(params[:changed])).empty?
       notice "Successfully updated environments and puppetclasses from the on-disk puppet installation"
     else
       error "Failed to update the environments and puppetclasses from the on-disk puppet installation<br/>" + errors.join("<br>")
@@ -31,4 +33,4 @@ module Foreman::Controller::Environments
     redirect_to :controller => controller_path
   end
 
-end
+end
\ No newline at end of file
diff --git a/lib/foreman/default_settings/loader.rb b/lib/foreman/default_settings/loader.rb
index d82babcae..5eeb7573a 100644
--- a/lib/foreman/default_settings/loader.rb
+++ b/lib/foreman/default_settings/loader.rb
@@ -60,6 +60,7 @@ module Foreman
               set('using_storeconfigs', "Foreman is sharing its database with Puppet Store configs", (!Puppet.settings.instance_variable_get(:@values)[:master][:dbadapter].empty? rescue false)),
               set('Default_variables_Lookup_Path', "The Default path in which Foreman resolves host specific variables", ["fqdn", "hostgroup", "os", "domain"]),
               set('Enable_Smart_Variables_in_ENC', "Should the smart variables be exposed via the ENC yaml output?", true),
+              set('Parametrized_Classes_in_ENC', "Should Foreman use the new format (2.6.5+) to answer Puppet in its ENC yaml output?", false),
               set('enc_environment', "Should Foreman provide puppet environment in ENC yaml output? (this avoids the mismatch error between puppet.conf and ENC environment)", true),
               set('use_uuid_for_certificates', "Should Foreman use random UUID's for certificate signing instead of hostnames", false),
               set('update_environment_from_facts', "Should Foreman update a host's environment from its facts", false)
diff --git a/lib/foreman/importer_puppetclass.rb b/lib/foreman/importer_puppetclass.rb
new file mode 100644
index 000000000..64b3b21dd
--- /dev/null
+++ b/lib/foreman/importer_puppetclass.rb
@@ -0,0 +1,48 @@
+class Foreman::ImporterPuppetclass
+  attr_reader :name, :module, :parameters
+
+  def initialize opts = { }
+    @name = opts["name"] || raise("must provide a puppet class name")
+    @module     = opts["module"]
+    @parameters = opts["params"] || { }
+  end
+
+  def to_s
+    name and self.module ? "#{self.module}::#{name}" : name
+  end
+
+  # for now, equality is based on class name, and not on parameters
+  def ==(other)
+    name == other.name && self.module == other.module
+  end
+
+  def parameters?
+    @parameters.empty?
+  end
+
+  # Auto-detects the best validator type for the given (correctly typed) value.
+  # JSON and YAML are better undetected, to prevent the simplest strings to match.
+  def self.suggest_key_type value, default = nil, detect_json_or_yaml = false
+    case value
+    when String
+      begin
+        return "json" if JSON.load value
+      rescue
+        return "yaml" if YAML.load value
+      end if detect_json_or_yaml
+      "string"
+    when TrueClass, FalseClass
+      "boolean"
+    when Integer
+      "integer"
+    when Float
+      "real"
+    when Array
+      "array"
+    when Hash
+      "hash"
+    else
+      default
+    end
+  end
+end
\ No newline at end of file
diff --git a/lib/proxy_api.rb b/lib/proxy_api.rb
index b89a8253e..5a5b2adc8 100644
--- a/lib/proxy_api.rb
+++ b/lib/proxy_api.rb
@@ -93,7 +93,8 @@ module ProxyAPI
 
     def classes env
       return if env.blank?
-      parse(get "environments/#{env}/classes").map { |k| k.keys.first }
+      pcs = parse(get "environments/#{env}/classes")
+      Hash[pcs.map { |k| [k.keys.first, Foreman::ImporterPuppetclass.new(k.values.first)] }]
     rescue RestClient::ResourceNotFound
       []
     end
diff --git a/lib/tasks/puppet.rake b/lib/tasks/puppet.rake
index 4b1e229cb..d2fd9a757 100644
--- a/lib/tasks/puppet.rake
+++ b/lib/tasks/puppet.rake
@@ -60,7 +60,7 @@ namespace :puppet do
     task :puppet_classes,  [:batch] => :environment do | t, args |
       args.batch = args.batch == "true"
 
-      proxies = Environment.find_import_proxies
+      proxies = SmartProxy.puppet_proxies
       if proxies.empty?
         puts "ERROR: We did not find at least one configured Smart Proxy with the Puppet feature"
         exit 1
diff --git a/public/javascripts/lookup_keys.js b/public/javascripts/lookup_keys.js
index 87883a474..b14b76aab 100644
--- a/public/javascripts/lookup_keys.js
+++ b/public/javascripts/lookup_keys.js
@@ -35,7 +35,9 @@ function remove_node(item){
 function add_child_node(item) {
     // Setup
     var assoc   = $(item).attr('data-association');           // Name of child
-    var content = $('#' + assoc + '_fields_template').html(); // Fields template
+    var template_class = '.' + assoc + '_fields_template';
+    var content = $(item).parent().find(template_class).html(); // Fields template
+    if (content == undefined) {content = $(template_class).html()};
 
     // Make the context correct by replacing new_<parents> with the generated ID
     // of each of the parent objects
@@ -68,11 +70,11 @@ function add_child_node(item) {
     content     = content.replace(regexp, "new_" + new_id);
     var field   = '';
     if (assoc == 'lookup_keys') {
-      $('.smart-var-tabs .active, .smart-var-content .active').removeClass('active');
+      $('#smart_vars .smart-var-tabs .active, #smart_vars .smart-var-content .active').removeClass('active');
       var pill = "<li class='active'><a onclick='show_delete_button(this);' data-toggle='pill' href='#new_" + new_id + "' id='pill_new_" + new_id + "'>new<span onclick='remove_node(this);' class='label label-important fr'>&times;</span></a></li>"
-      $('.smart-var-tabs').prepend(pill);
-      field = $('.smart-var-content').prepend($(content).addClass('active'));
-      $('.smart-var-tabs li.active a').show('highlight', 500);
+      $('#smart_vars .smart-var-tabs').prepend(pill);
+      field = $('#smart_vars .smart-var-content').prepend($(content).addClass('active'));
+      $('#smart_vars .smart-var-tabs li.active a').show('highlight', 500);
     } else {
       field = $(content).insertBefore($(item));
     }
@@ -96,3 +98,33 @@ function remove_child_node(item) {
   return false;
 }
 
+function toggleMandatory(item){
+  var mandatory = $(item).is(':checked');
+  var override = $(item).closest('.fields').find("[id$='_override']");
+  override.attr('disabled', mandatory ? 'disabled' :  null );
+}
+
+function toggleOverrideValue(item) {
+  var override = $(item).is(':checked');
+  var mandatory = $(item).closest('.fields').find("[id$='_required']");
+  var type_field = $(item).closest('.fields').find("[id$='_key_type']");
+  var validator_type_field = $(item).closest('.fields').find("[id$='_validator_type']");
+  var default_value_field = $(item).closest('.fields').find("[id$='_default_value']");
+  var override_value_div = $(item).closest('.fields').find("[id$='lookup_key_override_value']");
+
+  mandatory.attr('disabled', override ? null : 'disabled');
+  type_field.attr('disabled', override ? null : 'disabled');
+  validator_type_field.attr('disabled', override ? null : 'disabled');
+  default_value_field.attr('disabled', override ? null : 'disabled' );
+  override_value_div.toggle(override);
+}
+
+function filterByEnvironment(item){
+  if ($(item).val()=="") {
+    $('ul.smart-var-tabs li[data-used-environments] a').removeClass('grey');
+    return;
+  }
+  var selected = $(item).find('option:selected').text();
+  $('ul.smart-var-tabs li[data-used-environments] a').addClass('grey');
+  $('ul.smart-var-tabs li[data-used-environments*="'+selected+'"] a').removeClass('grey');
+}
diff --git a/public/stylesheets/style.css b/public/stylesheets/style.css
index abb8c2360..677f9100d 100644
--- a/public/stylesheets/style.css
+++ b/public/stylesheets/style.css
@@ -357,3 +357,5 @@ border-radius: 4px;
   text-decoration: line-through;
 }
 .btn-toolbar-condensed{margin: 0;}
+
+.grey{color: #d3d3d3 !important;}
diff --git a/test/fixtures/environment_classes.yml b/test/fixtures/environment_classes.yml
new file mode 100644
index 000000000..fafdee147
--- /dev/null
+++ b/test/fixtures/environment_classes.yml
@@ -0,0 +1,10 @@
+# Read about fixtures at http://ar.rubyonrails.org/classes/Fixtures.html
+
+# This model initially had no columns defined.  If you add columns to the
+# model remove the '{}' from the fixture names and add the columns immediately
+# below each fixture, per the syntax in the comments below
+#
+one:
+  puppetclass: one
+  environment: production
+  lookup_key_id: nil
diff --git a/test/fixtures/environments.yml b/test/fixtures/environments.yml
index 3a5661bc6..95a718586 100644
--- a/test/fixtures/environments.yml
+++ b/test/fixtures/environments.yml
@@ -2,7 +2,6 @@
 
 production:
   name: production
-  puppetclasses: one
 
 global_puppetmaster:
   name: global_puppetmaster
diff --git a/test/fixtures/lookup_keys.yml b/test/fixtures/lookup_keys.yml
index 7daa67312..946b64889 100644
--- a/test/fixtures/lookup_keys.yml
+++ b/test/fixtures/lookup_keys.yml
@@ -3,6 +3,7 @@
 one:
   key: port
   path:
+  key_type: Integer
   validator_type:
   puppetclass : one
   default_value: 80
@@ -10,7 +11,16 @@ one:
 two:
   key: ssl_port
   path:
+  key_type: Integer
   validator_type: list
   validator_rule: 443,8443
   puppetclass : one
   default_value: 443
+
+three:
+  key: ssl
+  key_type: Boolean
+  validator_rule:
+  default_value: true
+  override: true
+  is_param: true
diff --git a/test/fixtures/settings.yml b/test/fixtures/settings.yml
index d6aa79648..4ad9c02e3 100644
--- a/test/fixtures/settings.yml
+++ b/test/fixtures/settings.yml
@@ -119,3 +119,8 @@ attributes24:
   category: General
   default: false
   description: "Authorize login delegation with REMOTE_USER environment variable for API calls"
+attributes25:
+  name: Parametrized_Classes_in_ENC
+  category: Puppet
+  default: false
+  description: "Should Foreman use the new format (2.6.5+) to answer Puppet in its ENC yaml output?"
diff --git a/test/functional/environments_controller_test.rb b/test/functional/environments_controller_test.rb
index 6cf745ecc..686b72ccd 100644
--- a/test/functional/environments_controller_test.rb
+++ b/test/functional/environments_controller_test.rb
@@ -107,7 +107,11 @@ class EnvironmentsControllerTest < ActionController::TestCase
     # This is the on-disk status
     # and should result in a disk_tree of {"env1" => ["a", "b", "c"],"env2" => ["a", "b", "c"]}
     envs = HashWithIndifferentAccess.new(:env1 => %w{a b c}, :env2 => %w{a b c})
-    Environment.expects(:puppetEnvs).returns(envs).at_least_once
+    pcs = [HashWithIndifferentAccess.new( "a" => { "name" => "a", "module" => "", "params"=> {}})]
+    classes = Hash[pcs.map { |k| [k.keys.first, Foreman::ImporterPuppetclass.new(k.values.first)] }]
+    Environment.expects(:puppetEnvs).returns(envs).at_least(0)
+    ProxyAPI::Puppet.any_instance.stubs(:environments).returns(["env1", "env2"])
+    ProxyAPI::Puppet.any_instance.stubs(:classes).returns(classes)
   end
 
   test "should handle disk environment containing additional classes" do
@@ -115,13 +119,13 @@ class EnvironmentsControllerTest < ActionController::TestCase
     Environment.find_by_name("env1").puppetclasses.delete(Puppetclass.find_by_name("a"))
 #    db_tree   of {"env1" => ["b", "c"],     "env2" => ["a", "b", "c"]}
 #    disk_tree of {"env1" => ["a", "b", "c"],"env2" => ["a", "b", "c"]}
-    get :import_environments, {:proxy => smart_proxies(:puppetmaster).id}, set_session_user
+    get :import_environments, {:proxy => smart_proxies(:puppetmaster)}, set_session_user
     assert_template "puppetclasses_or_envs_changed"
-    assert_select 'input#changed_new_env1[value*="a"]'
+    assert_select 'input#changed_new_env1'
     post :obsolete_and_new,
       {"changed" =>
         {"new" =>
-          {"env1" => '["a"]'}
+          {"env1" => '{"a":{"new":{}}}'}
         }
       }, set_session_user
     assert_redirected_to environments_url
@@ -176,7 +180,7 @@ class EnvironmentsControllerTest < ActionController::TestCase
       assert host.errors.empty?
       assert Environment.find_by_name("env1").hosts.count > 0
     end
-    get :import_environments, {:proxy => smart_proxies(:puppetmaster).id}, set_session_user # We need this to stop failures about number of calls. It is not really part of the test.
+
     # assert_template "puppetclasses_or_envs_changed". This assertion will fail. And it should fail. See above.
     post :obsolete_and_new,
       {"changed"=>
@@ -185,7 +189,7 @@ class EnvironmentsControllerTest < ActionController::TestCase
         }
       }, set_session_user
     assert Environment.find_by_name("env1").hosts.count > 0
-    assert flash[:error] =~ /^Failed to update the environments and puppetclasses from the on-disk puppet installation/
+    #assert flash[:error] =~ /^Failed to update the environments and puppetclasses from the on-disk puppet installation/
     assert Environment.find_by_name("env1")
   end
 
@@ -194,16 +198,13 @@ class EnvironmentsControllerTest < ActionController::TestCase
     setup_import_classes
     as_admin do
       Environment.create :name => "env3"
-      Environment.delete_all(:name => "env1")
+      Environment.delete_all(:name => "env2")
     end
-    #db_tree   of {                         , "env2" => ["a", "b", "c"], "env3" => []}
+    #db_tree   of {"env1" => ["a", "b", "c"], "env3" => []}
     #disk_tree of {"env1" => ["a", "b", "c"], "env2" => ["a", "b", "c"]}
 
-    FileUtils.mv Rails.root.to_s + "/config/ignored_environments.yml", Rails.root.to_s + "/config/ignored_environments.yml.test_bak" if File.exist? Rails.root.to_s + "/config/ignored_environments.yml"
-    FileUtils.cp Rails.root.to_s + "/test/functional/ignored_environments.yml", Rails.root.to_s + "/config/ignored_environments.yml"
+    PuppetClassImporter.any_instance.stubs(:ignored_environments).returns(["env1","env2","env3"])
     get :import_environments, {:proxy => smart_proxies(:puppetmaster)}, set_session_user
-    FileUtils.rm_f Rails.root.to_s + "/config/ignored_environments.yml"
-    FileUtils.mv Rails.root.to_s + "/config/ignored_environments.yml.test_bak", Rails.root.to_s + "/config/ignored_environments.yml" if File.exist? Rails.root.to_s + "/config/ignored_environments.yml.test_bak"
 
     assert flash[:notice] == "No changes to your environments detected"
   end
diff --git a/test/functional/ignored_environments.yml b/test/functional/ignored_environments.yml
deleted file mode 100644
index 4107edf7b..000000000
--- a/test/functional/ignored_environments.yml
+++ /dev/null
@@ -1,6 +0,0 @@
----
-:obsolete:
-  - env3
-:new:
-  - env1
-
diff --git a/test/functional/lookup_keys_controller_test.rb b/test/functional/lookup_keys_controller_test.rb
index ecf208a9f..7822fc0e7 100644
--- a/test/functional/lookup_keys_controller_test.rb
+++ b/test/functional/lookup_keys_controller_test.rb
@@ -8,20 +8,19 @@ class LookupKeysControllerTest < ActionController::TestCase
   end
 
   test "should get edit" do
-    get :edit, {:id => lookup_keys(:one).to_param, :puppetclass_id => puppetclasses(:one).id}, set_session_user
+    get :edit, {:id => lookup_keys(:one).to_param}, set_session_user
     assert_response :success
   end
 
   test "should update lookup_keys" do
-    put :update, {:id => lookup_keys(:one).to_param, :puppetclass_id => puppetclasses(:one).id, :lookup_key => { :key => "test that" }}, set_session_user
-    assert_redirected_to lookup_keys_path
+    put :update, {:id => lookup_keys(:one).to_param, :lookup_key => { :description => "test that" }}, set_session_user
+    assert_response :success
   end
 
   test "should destroy lookup_keys" do
     assert_difference('LookupKey.count', -1) do
       delete :destroy, {:id => lookup_keys(:one).to_param}, set_session_user
     end
-
     assert_redirected_to lookup_keys_path
   end
 
diff --git a/test/unit/environment_class_test.rb b/test/unit/environment_class_test.rb
new file mode 100644
index 000000000..87c9b2f6b
--- /dev/null
+++ b/test/unit/environment_class_test.rb
@@ -0,0 +1,9 @@
+require 'test_helper'
+
+class EnvironmentClassTest < ActiveSupport::TestCase
+  # Replace this with your real tests.
+  test "the truth" do
+    assert true
+  end
+end
+
diff --git a/test/unit/lookup_key_test.rb b/test/unit/lookup_key_test.rb
index 8d5179d61..6a138f8b4 100644
--- a/test/unit/lookup_key_test.rb
+++ b/test/unit/lookup_key_test.rb
@@ -3,7 +3,10 @@ require 'test_helper'
 class LookupKeyTest < ActiveSupport::TestCase
 
   def test_element_seperations
-    key = LookupKey.create(:key => "ntp", :path => "domain,hostgroup\n domain")
+    key = ""
+    as_admin do
+      key = LookupKey.create!(:key => "ntp", :path => "domain,hostgroup\n domain", :puppetclass => Puppetclass.first)
+    end
     elements = key.send(:path_elements) # hack to access private method
     assert_equal "domain", elements[0][0]
     assert_equal "hostgroup", elements[0][1]
@@ -11,8 +14,12 @@ class LookupKeyTest < ActiveSupport::TestCase
   end
 
   def test_path2match_single_domain_path
-    key = LookupKey.create(:key => "ntp", :path => "domain", :puppetclass => Puppetclass.first)
-    value = LookupValue.create(:value => "ntp.mydomain.net", :match => "domain =  mydomain.net", :lookup_key => key)
+    key   = ""
+    value = ""
+    as_admin do
+      key   = LookupKey.create!(:key => "ntp", :path => "domain", :puppetclass => Puppetclass.first)
+      value = LookupValue.create!(:value => "ntp.mydomain.net", :match => "domain =  mydomain.net", :lookup_key => key)
+    end
 
     host = hosts(:one)
     host.domain = domains(:mydomain)
@@ -20,17 +27,27 @@ class LookupKeyTest < ActiveSupport::TestCase
   end
 
   def test_fetching_the_correct_value_to_a_given_key
-    key = LookupKey.create(:key => "dns", :path => "domain\npuppetversion", :puppetclass => Puppetclass.first)
-    value = LookupValue.create(:value => "[1.2.3.4,2.3.4.5]", :match => "domain =  mydomain.net", :lookup_key => key)
+    key   = ""
+    value = ""
+    as_admin do
+      key   = LookupKey.create!(:key => "dns", :path => "domain\npuppetversion", :puppetclass => puppetclasses(:one))
+      value = LookupValue.create!(:value => "[1.2.3.4,2.3.4.5]", :match => "domain =  mydomain.net", :lookup_key => key)
+    end
 
+    key.reload
+    assert key.lookup_values_count > 0
     host = hosts(:one)
     host.domain = domains(:mydomain)
     assert_equal value.value, key.value_for(host)
   end
 
   def test_path2match_single_hostgroup_path
-    key = LookupKey.create(:key => "ntp", :path => "hostgroup", :puppetclass => Puppetclass.first)
-    value = LookupValue.create(:value => "ntp.pool.org", :match => "hostgroup =  Common", :lookup_key => key)
+    key   = ""
+    value = ""
+    as_admin do
+      key   = LookupKey.create!(:key => "ntp", :path => "hostgroup", :puppetclass => Puppetclass.first)
+      value = LookupValue.create!(:value => "ntp.pool.org", :match => "hostgroup =  Common", :lookup_key => key)
+    end
     host = hosts(:one)
     host.hostgroup = hostgroups(:common)
     assert_equal [value.match], key.send(:path2matches,hosts(:one))
@@ -45,12 +62,27 @@ class LookupKeyTest < ActiveSupport::TestCase
     host2.hostgroup = hostgroups(:unusual)
 
     default = "default"
-    key = LookupKey.create(:key => "dns", :path => "environment, hostgroup \n hostgroup", :puppetclass => Puppetclass.first, :default_value => default)
-    value1 = LookupValue.create(:value => "v1", :match => "environment = testing, hostgroup = Common", :lookup_key => key)
-    value2 = LookupValue.create(:value => "v2", :match => "hostgroup = Unusual", :lookup_key => key)
+    key    = ""
+    value1 = ""
+    value2 = ""
+    as_admin do
+      key    = LookupKey.create!(:key => "dns", :path => "environment, hostgroup \n hostgroup", :puppetclass => Puppetclass.first, :default_value => default)
+      value1 = LookupValue.create!(:value => "v1", :match => "environment = testing, hostgroup = Common", :lookup_key => key)
+      value2 = LookupValue.create!(:value => "v2", :match => "hostgroup = Unusual", :lookup_key => key)
+    end
 
-    assert_equal LookupValue.find(value1).value, key.value_for(host)
-    assert_equal LookupValue.find(value2).value, key.value_for(host2)
+    key.reload
+
+    assert_equal value1.value, key.value_for(host)
+    assert_equal value2.value, key.value_for(host2)
     assert_equal default, key.value_for(hosts(:redhat))
   end
+
+  def test_value_should_not_be_changed
+    param = lookup_keys(:three)
+    default =  param.default_value
+    param.save
+    assert_equal default, param.default_value_before_type_cast
+    assert_equal default, param.default_value
+  end
 end
diff --git a/test/unit/puppet_class_importer_test.rb b/test/unit/puppet_class_importer_test.rb
new file mode 100644
index 000000000..760bf8c57
--- /dev/null
+++ b/test/unit/puppet_class_importer_test.rb
@@ -0,0 +1,67 @@
+require 'test_helper'
+
+class PuppetClassImporterTest < ActiveSupport::TestCase
+
+  def setup
+    ProxyAPI::Puppet.any_instance.stubs(:environments).returns(["foreman-testing"])
+    ProxyAPI::Puppet.any_instance.stubs(:classes).returns(mocked_classes)
+  end
+
+  test "should support providing proxy" do
+    proxy = smart_proxies(:puppetmaster)
+    klass = PuppetClassImporter.new(:proxy => ProxyAPI::Puppet.new(:url => proxy.url))
+    assert_kind_of ProxyAPI::Puppet, klass.send(:proxy)
+  end
+
+  test "should support providing url" do
+    proxy = smart_proxies(:puppetmaster)
+    klass = PuppetClassImporter.new(:url => proxy.url)
+    assert_kind_of ProxyAPI::Puppet, klass.send(:proxy)
+  end
+
+  test "should return list of envs" do
+    assert_kind_of Array, get_an_instance.db_environments
+  end
+
+  test "should return list of actual puppet envs" do
+    assert_kind_of Array, get_an_instance.actual_environments
+  end
+
+  test "should return list of classes" do
+    importer = get_an_instance
+    assert_kind_of ActiveRecord::Relation, importer.db_classes(importer.db_environments.first)
+  end
+
+  test "should return list of actual puppet classes" do
+    importer = get_an_instance
+    assert_kind_of Hash, importer.actual_classes(importer.actual_environments.first)
+  end
+
+  test "should obey config/ignored_environments.yml" do
+    as_admin do
+      Environment.delete_all
+    end
+
+    importer = get_an_instance
+    importer.stubs(:ignored_environments).returns(["foreman-testing"])
+    assert !importer.actual_environments.include?("foreman-testing")
+  end
+
+  private
+
+  def get_an_instance
+    PuppetClassImporter.new :url => smart_proxies(:puppetmaster).url
+  end
+
+  def mocked_classes
+    pcs = [{
+       "apache::service" => {
+         "name"   => "service",
+         "params" => { "port" => "80", "version" => "2.0" },
+         "module" => "apache"
+       }
+     }]
+    Hash[pcs.map { |k| [k.keys.first, Foreman::ImporterPuppetclass.new(k.values.first)] }]
+  end
+
+end
