RSpec2 Style let bindings for Chef?

api.rb

##
# binding definition API.
# Examples taken from nagios cookbook

bind(:nagios_service_name).to("nagios")

bind(:hostgroups) do
  search(:role, "*:*").map {|r| r.name }
end

bind(:nodes) do
  search(:node, "hostname:[* TO *] AND chef_environment:#{node.chef_environment}")
end

bind(:hostname_attr).to_attribute_path(:nagios, :host_name_attribute)

bind(:nodes_by_hostgroup) do
  by_hostgroup = {}
  hostgroups.each do |hostgroup|
    nodes_in_group = nodes.select {|n| n[:roles].include?(hostgroup) }.each do |n|
      by_hostgroup[hostgroup] = n[hostname_attr]
    end
  end
end

bind(:web_srv).to_attribute_path(:nagios, :server, :web_server)

##
# Binding usage when creating recipes:

# nagios conf is a resource definition, but this should be the same for any resource:
nagios_conf "hostgroups" do
  # Instance eval f***s up the scope here, so we either need method_missing
  # magic or a way to access the recipe's bindings via method call. The latter
  # style is how I roll generally, but maybe this is confusing?
  variables(:hostgroups => binding.hostgroups,
            # these next two aren't shown above, but they could work the same.
            :search_hostgroups => binding.hostgroup_list,
            :search_nodes => binding.hostgroup_nodes)
end


##
# Recipe inclusion API
# (this would be in your wrapper cookbook)

include_recipe("nagios::server") do |r|
  r.bind(:hostgroups).to(["foo", "bar", "baz"]) # use static data instead of search
  r.bind(:nodes_by_hostgroup).to_attribute_path(:chef, :solo, :for, :lyfe)
end

motivation.md

Motivation

This proposal combines a handful of ideas I've been gnawing on for a few years now. I'm not really sure I've nailed it, so comments welcome.

Haters Gonna Hate

First of all, I've been interested in having a lightweight abstraction around data sources for quite a while. I frequently see patches and discussion about adding support for Chef server specific features to chef-solo, because people want to reuse the same cookbooks for both solo and chef-client. That use case totally makes sense to me, but it still feels icky to add a "search" feature that's not actually search to chef-solo.

Another thing I'd like to improve is the undefined method [] on NilClass error. I didn't show this in the api example, but I would implement binding of a name to an attribute path in such a way that chef could detect that the user is missing attributes and give a very clear error message. Something along the lines of:

You tried to get the value of node['foo']['bar']['baz'], but node['foo']['bar'] is nil. You must set these attributes correctly for cookbook "monkeypants" to function.

I've also been unhappy with what I would call "attribute sprawl" or "attribute spaghetti" for quite a while (NOTE: This is not any cookbook author's fault, because attributes are the only real way to pass input into a recipe). Look at any popular cookbook, and you see that in order to meet everyone's use case, you have attributes for everything, even attributes used as keys to look up other attributes. This drives me nuts when I'm reading a new cookbook because I can hardly tell what it's doing without keeping the value of a half dozen attributes in my head. So I've been looking for a customization mechanism that's easier to understand.

Random Whatifs

One thing I like about this proposal is that it provides a well defined mechanism for passing input to cookbooks, and we can do cool things at this interface.

It should be relatively easy to add a validation mechanism on the bindings a cookbook exposes, which would make any errors that occur when customizing readily apparent and much easier to find and fix.

One problem that pops up a lot is duplication of loading data bags or duplicate search queries. Aside from the extra load on the server, it's really unpleasant when you go back and change one of these things. If there was a convenient way to define default bindings for a whole chef run, you could define this stuff in one place, cache the results of API calls, and change the code in one place when you need to change it.

Another thing that would be possible is to define default bindings (i.e., chef defines them for you) and pass these in to resources. For example, we could have default bindings for the cookbook that templates and cookbook_files are located in, and the user could override them on a per-cookbook basis. I'm not convinced this is the best answer to that family of use-cases, but worth discussing.

wip-implementation.rb

class Chef
  module DSL
    module DataBindings

      module BindTypes
        class Static
          def initialize(name, value)
            @name = name
            @value = value
          end

          def call(context_object)
            @value
          end
        end

        class Lambda
          def initialize(name, block)
            @name = name
            @block = block
          end

          def call(context_object)
            @block.call
          end
        end

        class AttributePath

          attr_reader :name

          class MissingNodeAttribute < StandardError
          end

          def initialize(name, *path_specs)
            @name = name
            @path_specs = path_specs
          end

          def call(context_object)
            searched_path = []
            @path_specs.inject(context_object.node) do |attrs_so_far, attr_key|
              searched_path << attr_key
              next_attrs = attrs_so_far[attr_key]
              if !next_attrs.nil?
                next_attrs
              else
                current_lookup = "node[:#{searched_path.join('][:')}]"
                desired_lookup = "node[:#{@path_specs.join('][:')}]"
                raise MissingNodeAttribute,
                 "Error finding value `#{name}' from attributes `#{desired_lookup}`: `#{current_lookup}` is nil"
              end
            end
          end
        end


      end

      class DataBindingDefinition

        attr_reader :name

        def initialize(name, &block)
          @name = name
          if block_given?
            @data_binding = BindTypes::Lambda.new(name, block)
          else
            @data_binding = nil
          end
        end

        def as(value)
          @data_binding = BindTypes::Static.new(name, value)
        end

        def as_attribute(*path_specs)
          # Target object needs to define #node to return the Chef::Node object
          @data_binding = BindTypes::AttributePath.new(name, *path_specs)
        end

        def call(context_object)
          @data_binding.call(context_object)
        end
      end

      module BindingContext

        def self.for_name(recipe_name)
          # Create a module that is the same as this one for the purposes of #extend
          extension_module = Module.new
          extension_module.extend(self)

          # Create a constant name for the module. Not required, but it makes
          # debugging easier when error messages have real class/module names
          # instead of '#<Module:0x007fe53594e270>'
          clean_name = recipe_name.gsub(/[^\w]/, '_')
          const_base_name = "BindingContextFor_#{clean_name}"
          self.const_set(const_base_name, extension_module)

          extension_module
        end

        def data_bindings
          @data_bindings ||= {}
        end

        def define(binding_name, &block)
          binding_defn = DataBindingDefinition.new(binding_name, &block)
          data_bindings[binding_name] = binding_defn
          define_method(binding_name) do
            binding_defn.call(binding_context)
          end
          binding_defn
        end

      end

      def define(*args, &block)
        setup_bindings("#{recipe_name}::#{cookbook_name}", nil, nil) if need_to_setup?
        @local_context.define(*args, &block)
      end

      def need_to_setup?
        @local_context.nil?
      end

      # Set up a "class hierarchy" of global_context < local_context < override_context
      def setup_bindings(object_name, override_context, global_context)
        # This is unfortunate, but since Chef recipes are objects and not
        # classes, we're forced to reinvent class hierarchy with metaprogramming :|
        #
        @local_context = BindingContext.for_name(object_name)

        # enable future debugging capabilities
        @override_context = override_context
        @global_context = global_context
        extend global_context if global_context
        extend @local_context
        extend override_context if override_context
        self
      end

      def binding_context
        self
      end

      def include_customized_recipe(recipe_spec, &customizer_block)
        override_context = BindingContext.for_name("Override_#{recipe_spec}")
        unless customizer_block && customizer_block.arity == 1
          raise ArgumentError, "you have to pass a block with 1 argument to #include_customized_recipe"
        end
        customizer_block.call(override_context)

        run_context.load_customized_recipe(recipe_spec, override_context)

      end
    end
  end
end

class Chef::Recipe
  include Chef::DSL::DataBindings
end

class Chef::RunContext

  # The definition #load_recipe doesn't give us access to the recipe before it
  # gets evaluated, so we have to monkey patch
  def load_customized_recipe(recipe_name, override_context)
    Chef::Log.debug("Loading Recipe #{recipe_name} via include_recipe")

    cookbook_name, recipe_short_name = Chef::Recipe.parse_recipe_name(recipe_name)
    if loaded_fully_qualified_recipe?(cookbook_name, recipe_short_name)
      Chef::Log.debug("I am not loading #{recipe_name}, because I have already seen it.")
      false
    else
      loaded_recipe(cookbook_name, recipe_short_name)

      cookbook = cookbook_collection[cookbook_name]
      cookbook.load_customized_recipe(recipe_short_name, self, override_context)
    end
  end
end


class Chef::CookbookVersion

  # The definition #load_recipe doesn't give us access to the recipe before it
  # gets evaluated, so we have to monkey patch
  def load_customized_recipe(recipe_name, run_context, override_context)
    unless recipe_filenames_by_name.has_key?(recipe_name)
      raise Chef::Exceptions::RecipeNotFound, "could not find recipe #{recipe_name} for cookbook #{name}"
    end

    Chef::Log.debug("Found recipe #{recipe_name} in cookbook #{name}")
    recipe = Chef::Recipe.new(name, recipe_name, run_context)

    # HERES THE MAGIC
    recipe.setup_bindings("#{name}::#{recipe_name}", override_context, nil) # no global context yet

    recipe_filename = recipe_filenames_by_name[recipe_name]

    unless recipe_filename
      raise Chef::Exceptions::RecipeNotFound, "could not find #{recipe_name} files for cookbook #{name}"
    end

    recipe.from_file(recipe_filename)
    recipe
  end
end

wip-specs.rb

require 'spec_helper'

module Chef::DSL::DataBindings

  module BindTypes
    class Static
      def initialize(name, value)
        @name = name
        @value = value
      end

      def call(context_object)
        @value
      end
    end

    class Lambda
      def initialize(name, block)
        @name = name
        @block = block
      end

      def call(context_object)
        @block.call
      end
    end

    class AttributePath

      attr_reader :name

      class MissingNodeAttribute < StandardError
      end

      def initialize(name, *path_specs)
        @name = name
        @path_specs = path_specs
      end

      def call(context_object)
        searched_path = []
        @path_specs.inject(context_object.node) do |attrs_so_far, attr_key|
          searched_path << attr_key
          next_attrs = attrs_so_far[attr_key]
          if !next_attrs.nil?
            next_attrs
          else
            current_lookup = "node[:#{searched_path.join('][:')}]"
            desired_lookup = "node[:#{@path_specs.join('][:')}]"
            raise MissingNodeAttribute,
             "Error finding value `#{name}' from attributes `#{desired_lookup}`: `#{current_lookup}` is nil"
          end
        end
      end
    end


  end

  class DataBindingDefinition

    attr_reader :name

    def initialize(name, &block)
      @name = name
      if block_given?
        @data_binding = BindTypes::Lambda.new(name, block)
      else
        @data_binding = nil
      end
    end

    def as(value)
      @data_binding = BindTypes::Static.new(name, value)
    end

    def as_attribute(*path_specs)
      # Target object needs to define #node to return the Chef::Node object
      @data_binding = BindTypes::AttributePath.new(name, *path_specs)
    end

    def call(context_object)
      @data_binding.call(context_object)
    end
  end

  module BindingContext

    def self.for_name(recipe_name)
      # Create a module that is the same as this one for the purposes of #extend
      extension_module = Module.new
      extension_module.extend(self)

      # Create a constant name for the module. Not required, but it makes
      # debugging easier when error messages have real class/module names
      # instead of '#<Module:0x007fe53594e270>'
      clean_name = recipe_name.gsub(/[^\w]/, '_')
      const_base_name = "BindingContextFor_#{clean_name}"
      self.const_set(const_base_name, extension_module)

      extension_module
    end

    def data_bindings
      @data_bindings ||= {}
    end

    def define(binding_name, &block)
      binding_defn = DataBindingDefinition.new(binding_name, &block)
      data_bindings[binding_name] = binding_defn
      define_method(binding_name) do
        binding_defn.call(binding_context)
      end
      binding_defn
    end

  end

  def define(*args, &block)
    @local_context.define(*args, &block)
  end

  # Set up a "class hierarchy" of global_context < local_context < override_context
  def setup_bindings(object_name, override_context, global_context)
    # This is unfortunate, but since Chef recipes are objects and not
    # classes, we're forced to reinvent class hierarchy with metaprogramming :|
    #
    @local_context = BindingContext.for_name(object_name)

    # enable future debugging capabilities
    @override_context = override_context
    @global_context = global_context
    extend global_context if global_context
    extend @local_context
    extend override_context if override_context
    self
  end

  def binding_context
    self
  end

  def include_customized_recipe(recipe_spec, &customizer_block)
    override_context = BindingContext.for_name("Override_#{recipe_spec}")
    unless customizer_block && customizer_block.arity == 1
      raise ArgumentError, "you have to pass a block with 1 argument to #include_customized_recipe"
    end
    customizer_block.call(override_context)

    run_context.load_customized_recipe(recipe_spec, override_context)

  end
end

class Chef::RunContext

  # The definition #load_recipe doesn't give us access to the recipe before it
  # gets evaluated, so we have to monkey patch
  def load_customized_recipe(recipe_name, override_context)
    Chef::Log.debug("Loading Recipe #{recipe_name} via include_recipe")

    cookbook_name, recipe_short_name = Chef::Recipe.parse_recipe_name(recipe_name)
    if loaded_fully_qualified_recipe?(cookbook_name, recipe_short_name)
      Chef::Log.debug("I am not loading #{recipe_name}, because I have already seen it.")
      false
    else
      loaded_recipe(cookbook_name, recipe_short_name)

      cookbook = cookbook_collection[cookbook_name]
      cookbook.load_recipe(recipe_short_name, self, override_context)
    end
  end
end


class Chef::CookbookVersion

  # The definition #load_recipe doesn't give us access to the recipe before it
  # gets evaluated, so we have to monkey patch
  def load_customized_recipe(recipe_name, run_context, override_context)
    unless recipe_filenames_by_name.has_key?(recipe_name)
      raise Chef::Exceptions::RecipeNotFound, "could not find recipe #{recipe_name} for cookbook #{name}"
    end

    Chef::Log.debug("Found recipe #{recipe_name} in cookbook #{name}")
    recipe = Chef::Recipe.new(name, recipe_name, run_context)

    # HERES THE MAGIC
    recipe.setup_bindings("#{name}::#{recipe_name}", override_context, nil) # no global context yet

    recipe_filename = recipe_filenames_by_name[recipe_name]

    unless recipe_filename
      raise Chef::Exceptions::RecipeNotFound, "could not find #{recipe_name} files for cookbook #{name}"
    end

    recipe.from_file(recipe_filename)
    recipe
  end
end

class DataBindingImplementor
  include Chef::DSL::DataBindings
end

describe Chef::DSL::DataBindings do

  let(:iteration) do
    $iteration ||= -1
    $iteration += 1
  end

  let(:override_context) { nil }

  let(:global_context) { nil }

  let(:recipe) do
    r = DataBindingImplementor.new
    r.setup_bindings("cookbook::some-recipe#{iteration}", override_context, global_context)
    r
  end

  describe "static data bindings" do

    it "binds a name to a value" do
      recipe.define(:named_value).as("the value")
      recipe.named_value.should == "the value"
    end

  end

  describe "binding a proc" do
    it "binds a name to a proc/lambda" do
      recipe.define(:lambda_value) do
        "this is a lambda value"
      end

      recipe.lambda_value.should == "this is a lambda value"
    end
  end

  describe "binding to node data" do
    let(:node) do
      Chef::Node.new.tap do |n|
        n.automatic_attrs[:ec2][:public_hostname] = "foo.ec2.example.com"
      end
    end

    before do
      recipe.stub!(:node).and_return(node)
    end

    it "binds a name to an attribute path" do
      recipe.define(:public_hostname).as_attribute(:ec2, :public_hostname)
      recipe.public_hostname.should == "foo.ec2.example.com"
    end

    it "raises a not-enraging error message when an intermediate value is nil" do
      recipe.define(:oops).as_attribute(:ec2, :no_attr_here, :derp)
      error_class = Chef::DSL::DataBindings::BindTypes::AttributePath::MissingNodeAttribute
      error_message = "Error finding value `oops' from attributes `node[:ec2][:no_attr_here][:derp]`: `node[:ec2][:no_attr_here]` is nil"
      lambda { recipe.oops }.should raise_error(error_class, error_message)
    end
  end

  describe "binding to search results" do
    it "binds a name to a search query" do
      pending "TODO"
      recipe.define(:searchy).as_search_result(:node, "*:*")
      # todo stubz
      recipe.searchy.should == [node1, node2]
    end

    it "binds a name to a partial search query" do
      pending "TODO"
      recipe.define(:p_searchy).as_search_result(:node, "id:*foo*") do |item|
        item.define(:name).as_attribute(:name)
        item.define(:ip).as_attribute(:ipaddress)
        item.define(:kernel_version).as_attribute(:kernel, :version)
      end
      # todo stubz
      recipe.p_searchy[0][:name].should == "node_name"
      recipe.p_searchy[0][:ip].should == "123.45.67.89"
      recipe.p_searchy[0]
    end

  end

  describe "overriding bindings" do

    let(:global_context) do
      Chef::DSL::DataBindings::BindingContext.for_name("GlobalContext#{iteration}")
    end

    let(:override_context) do
      Chef::DSL::DataBindings::BindingContext.for_name("override cookbook::some_recipe-#{iteration}")
    end

    it "defaults to a global definition if no others override it" do
      global_context.define(:global_thing).as("follow you wherever you may go")
      recipe.global_thing.should == "follow you wherever you may go"
    end

    it "overrides a local definition" do
      override_context.define(:named_thing).as("party time")
      recipe.define(:named_thing).as("sad time")

      recipe.named_thing.should == "party time"
    end

  end

end

@danielsdeleo is there a more appropriate place to weigh in on this?

I think this is a great idea and I like the proposal. However, it took me a few times through of reading before I realized what the DSL did. Then I read the motivation.md and it solidified that my understanding was correct. This might be a sign that the DSL may not be easy to comprehend for people who don't identify themselves as software engineers. It could also mean that this portion of the DSL just really needs a good "WHY DO YOU WANT THIS" description as the first thing anyone reads about this DSL feature.

I would absolutely use this in place of the techniques we use to create cookbooks which support both Chef and Chef Solo.

👍

@reset this is as good a place as any to discuss, if you think we need a broader discussion, you could start a thread on chef-dev. I worked on this as much as I could during a recent Opscode hack day and what I have so far is now added to the gist. It still needs bindings for data bags and search, and it has a half-implemented global binding feature (primary motivation there is to provide a way to do a search or data bag load one time and make that available to all recipes/cookbooks).

By the way, do you have opinions on the best way to "prove" this feature? I could release it as a cookbook, or add some notion of "experimental features" to Chef that would be opt-in (and have the Caveat that they're experimental and not guaranteed to have stable APIs).

@reset also, thanks for the feedback. I think non-rubyists should hopefully be able to understand the feature as "like a function definition, but with chef-specific magic." In the actual implementation, I've changed some names to hopefully make it simpler to understand. Would be happy to get feed back on this.

@danielsdeleo I think the best way to test DSL extensions is via a Cookbook. You just jam that cookbook in your metadata.rb as a dependency and you are good to go.

I think this is a poor approach for DSL changes, though, since you might break some other cookbooks that are loaded on a node. Aside from the monkey patch of RunContext I don't see anything that would constitute this as a DSL change; so I'd say put it in a cookbook for now.

Forking a high traffic (and a bit messy) community cookbook like MySQL and leveraging this DSL would be both a great example and help identify anything lacking in this DSL extension. I would love to see what this could do for the MySQL community cookbook.

If the extension does the job for MySQL then I think it's ready for a promotion and could be put into Chef as an experimental feature.

Things I think that will be important to get non-Rubyists on board

• Naming - it's gotta make sense and shouldn't conflict with names that people already have associated to things in Chef or Ruby
• Placement - where do we place the files in the cookbook?
• Documentation - do we extend that metadata.rb to expose and document these?

I know that this is a useful feature. If you follow a README driven development methodology and showcase the README to people who don't identify themselves as Rubyists, I think you'll get some awesome feedback for the bullets above.

@danielsdeleo This feature is an awesome idea.

Are the BindTypes for Search/DataBag/Attributes necessary? There are already simple interfaces for all of these things that people already know and they can all be implemented using the Lambda BindType. Also, I don't see how I could switch on my runtime environment (solo vs server, specific version of an os, whatever other crazy stuff) when using the #to_attribute_path binding. I'll still have to write code to choose a binding. It feels simpler to just have one kind of binding that allows me to wrap that code.

include_recipe "nginx" do |r|
  if Chef::Config[:solo]
    r.bind(:some_nodes).to_attribute_path(:some, :static, :ips)
  else
    r.bind(:some_nodes).to_search_results("some search string")
  end
end

Versus

include_recipe "nginx" do |r|
  r.bind(:some_node) do
    if Chef::Config[:solo]
      node[:some][:static][:ips]
    else
      search("some search string")
    end
  end
end

This approach also lends itself well to having a shared and "globally" available binding.

the_binding = ChefBinding.new(memoize: true, eager: true) do 
  if Chef::Config[:solo]
    node[:some][:default][:attribute]
  else
    search("some search string")
  end
end

include_recipe("nginx", :some_name => the_binding)
include_recipe("memcached", :some_name => the_binding)

# OR - if we know all the recipes use the same identifier for our binding

bind.globally(:some_name).to(the_binding) 

include_recipe "nginx"
include_recipe "memcached"

Fixing the "undefined method [] for nil" would be really nice, but this will only fix it in the context of a binding. It feels like that problem needs to be solved elsewhere. Maybe make Mash into an Option type and warn whenever you are setting a non-Nothing Mash to nil? Seems like a much bigger problem.

This is a great idea 🤘 how can we help test this out? I like the idea of adding it as an experimental API, but at the same time, when do you make the call to standardize on it if it is popular, and how do you measure that? 👍

@andrewGarson I understand and share your concerns about the size of the API and duplication with existing DSL. At the same time, I think having specific types for search, data bags and node attributes has value in being able to automatically do the right thing (memoize or not, fixing undefined method [] on nil). I could certainly add a #deep_fetch(*path_spec) method on Mash that works the same as the attribute path binding above and see if that makes more sense than a specific type. I'll probably try prototyping both approaches and get feedback from other people.

@fujin Thanks, AJ! I think I'm gonna go the route of putting it in a cookbook for now (just need some free time to finish it). From there I'll get feedback and see about merging into mainline.

I'm working on this over here now: https://github.com/danielsdeleo/chef-data-bindings