diff options
author | Lamont Granquist <lamont@scriptkiddie.org> | 2015-02-23 10:16:23 -0800 |
---|---|---|
committer | Lamont Granquist <lamont@scriptkiddie.org> | 2015-02-23 10:16:23 -0800 |
commit | 2603e2153d6ab50179d2278025a51579edb9033f (patch) | |
tree | ebfb51d1f24e2dabc40d971b83f39e630c47542d | |
parent | a7f5c92960aedf8d5bfc71abbce430ab075e016a (diff) | |
parent | 2cb917555aeb0e01061711c811aa4f8f436b4790 (diff) | |
download | chef-2603e2153d6ab50179d2278025a51579edb9033f.tar.gz |
Merge pull request #2956 from chef/lcg/deploy-fixes
nillable deploy resource + nillable LWRP args
-rw-r--r-- | CHANGELOG.md | 4 | ||||
-rw-r--r-- | lib/chef/mixin/params_validate.rb | 61 | ||||
-rw-r--r-- | lib/chef/resource.rb | 9 | ||||
-rw-r--r-- | lib/chef/resource/deploy.rb | 266 | ||||
-rw-r--r-- | lib/chef/resource/lwrp_base.rb | 8 | ||||
-rw-r--r-- | spec/unit/mixin/params_validate_spec.rb | 136 | ||||
-rw-r--r-- | spec/unit/resource/deploy_spec.rb | 27 |
7 files changed, 207 insertions, 304 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 949c2b8bfb..7988ba2dd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ## Unreleased +* make deploy resource attributes nillable (`symlink_before_migrate nil`) works now +* mixin the LWRP attribute DSL method into Chef::Resource directly +* make all LWRP attributes nillable + ## 12.1.0 * [**Andre Elizondo**](https://github.com/andrewelizondo) diff --git a/lib/chef/mixin/params_validate.rb b/lib/chef/mixin/params_validate.rb index 78d72dc801..baf210bfc5 100644 --- a/lib/chef/mixin/params_validate.rb +++ b/lib/chef/mixin/params_validate.rb @@ -81,34 +81,58 @@ class Chef DelayedEvaluator.new(&block) end - def set_or_return(symbol, arg, validation) + NULL_ARG = Object.new + + def nillable_set_or_return(symbol, arg, validation) iv_symbol = "@#{symbol.to_s}".to_sym - if arg == nil && self.instance_variable_defined?(iv_symbol) == true - ivar = self.instance_variable_get(iv_symbol) - if(ivar.is_a?(DelayedEvaluator)) - validate({ symbol => ivar.call }, { symbol => validation })[symbol] + if NULL_ARG.equal?(arg) + if self.instance_variable_defined?(iv_symbol) == true + get_ivar(iv_symbol, symbol, validation) else - ivar + # on access we create the iv and set it to nil for back-compat + set_ivar(iv_symbol, symbol, nil, validation) end else - if(arg.is_a?(DelayedEvaluator)) - val = arg - else - val = validate({ symbol => arg }, { symbol => validation })[symbol] + set_ivar(iv_symbol, symbol, arg, validation) + end + end - # Handle the case where the "default" was a DelayedEvaluator. In - # this case, the block yields an optional parameter of +self+, - # which is the equivalent of "new_resource" - if val.is_a?(DelayedEvaluator) - val = val.call(self) - end - end - self.instance_variable_set(iv_symbol, val) + def set_or_return(symbol, arg, validation) + iv_symbol = "@#{symbol.to_s}".to_sym + if arg == nil && self.instance_variable_defined?(iv_symbol) == true + get_ivar(iv_symbol, symbol, validation) + else + set_ivar(iv_symbol, symbol, arg, validation) end end private + def get_ivar(iv_symbol, symbol, validation) + ivar = self.instance_variable_get(iv_symbol) + if(ivar.is_a?(DelayedEvaluator)) + validate({ symbol => ivar.call }, { symbol => validation })[symbol] + else + ivar + end + end + + def set_ivar(iv_symbol, symbol, arg, validation) + if(arg.is_a?(DelayedEvaluator)) + val = arg + else + val = validate({ symbol => arg }, { symbol => validation })[symbol] + + # Handle the case where the "default" was a DelayedEvaluator. In + # this case, the block yields an optional parameter of +self+, + # which is the equivalent of "new_resource" + if val.is_a?(DelayedEvaluator) + val = val.call(self) + end + end + self.instance_variable_set(iv_symbol, val) + end + # Return the value of a parameter, or nil if it doesn't exist. def _pv_opts_lookup(opts, key) if opts.has_key?(key.to_s) @@ -239,4 +263,3 @@ class Chef end end end - diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb index ea220b6c70..2df73a52e8 100644 --- a/lib/chef/resource.rb +++ b/lib/chef/resource.rb @@ -997,6 +997,15 @@ class Chef end # + # DSL method used to define attribute on a resource wrapping params_validate + # + def self.attribute(attr_name, validation_opts={}) + define_method(attr_name) do |arg=NULL_ARG| + nillable_set_or_return(attr_name.to_sym, arg, validation_opts) + end + end + + # # The cookbook in which this Resource was defined (if any). # # @return Chef::CookbookVersion The cookbook in which this Resource was defined. diff --git a/lib/chef/resource/deploy.rb b/lib/chef/resource/deploy.rb index 4252aa230f..f886f856df 100644 --- a/lib/chef/resource/deploy.rb +++ b/lib/chef/resource/deploy.rb @@ -82,6 +82,7 @@ class Chef @keep_releases = 5 @enable_checkout = true @checkout_branch = "deploy" + @timeout = nil end # where the checked out/cloned code goes @@ -104,42 +105,18 @@ class Chef end # note: deploy_to is your application "meta-root." - def deploy_to(arg=nil) - set_or_return( - :deploy_to, - arg, - :kind_of => [ String ] - ) - end + attribute :deploy_to, :kind_of => [ String ] - def repo(arg=nil) - set_or_return( - :repo, - arg, - :kind_of => [ String ] - ) - end + attribute :repo, :kind_of => [ String ] alias :repository :repo - def remote(arg=nil) - set_or_return( - :remote, - arg, - :kind_of => [ String ] - ) - end + attribute :remote, :kind_of => [ String ] - def role(arg=nil) - set_or_return( - :role, - arg, - :kind_of => [ String ] - ) - end + attribute :role, :kind_of => [ String ] - def restart_command(arg=nil, &block) - arg ||= block - set_or_return( + def restart_command(arg=NULL_ARG, &block) + arg = block if block_given? + nillable_set_or_return( :restart_command, arg, :kind_of => [ String, Proc ] @@ -147,155 +124,60 @@ class Chef end alias :restart :restart_command - def migrate(arg=nil) - set_or_return( - :migrate, - arg, - :kind_of => [ TrueClass, FalseClass ] - ) - end + attribute :migrate, :kind_of => [ TrueClass, FalseClass ] - def migration_command(arg=nil) - set_or_return( - :migration_command, - arg, - :kind_of => [ String ] - ) - end + attribute :migration_command, kind_of: String - def rollback_on_error(arg=nil) - set_or_return( - :rollback_on_error, - arg, - :kind_of => [ TrueClass, FalseClass ] - ) - end + attribute :rollback_on_error, :kind_of => [ TrueClass, FalseClass ] - def user(arg=nil) - set_or_return( - :user, - arg, - :kind_of => [ String ] - ) - end + attribute :user, kind_of: String - def group(arg=nil) - set_or_return( - :group, - arg, - :kind_of => [ String ] - ) - end + attribute :group, kind_of: [ String ] - def enable_submodules(arg=nil) - set_or_return( - :enable_submodules, - arg, - :kind_of => [ TrueClass, FalseClass ] - ) - end + attribute :enable_submodules, kind_of: [ TrueClass, FalseClass ] - def shallow_clone(arg=nil) - set_or_return( - :shallow_clone, - arg, - :kind_of => [ TrueClass, FalseClass ] - ) - end + attribute :shallow_clone, kind_of: [ TrueClass, FalseClass ] - def repository_cache(arg=nil) - set_or_return( - :repository_cache, - arg, - :kind_of => [ String ] - ) - end + attribute :repository_cache, kind_of: String - def copy_exclude(arg=nil) - set_or_return( - :copy_exclude, - arg, - :kind_of => [ String ] - ) - end + attribute :copy_exclude, kind_of: String - def revision(arg=nil) - set_or_return( - :revision, - arg, - :kind_of => [ String ] - ) - end + attribute :revision, kind_of: String alias :branch :revision - def git_ssh_wrapper(arg=nil) - set_or_return( - :git_ssh_wrapper, - arg, - :kind_of => [ String ] - ) - end + attribute :git_ssh_wrapper, kind_of: String alias :ssh_wrapper :git_ssh_wrapper - def svn_username(arg=nil) - set_or_return( - :svn_username, - arg, - :kind_of => [ String ] - ) - end + attribute :svn_username, kind_of: String - def svn_password(arg=nil) - set_or_return( - :svn_password, - arg, - :kind_of => [ String ] - ) - end + attribute :svn_password, kind_of: String - def svn_arguments(arg=nil) - set_or_return( - :svn_arguments, - arg, - :kind_of => [ String ] - ) - end + attribute :svn_arguments, kind_of: String - def svn_info_args(arg=nil) - set_or_return( - :svn_arguments, - arg, - :kind_of => [ String ]) - end + attribute :svn_info_args, kind_of: String - def scm_provider(arg=nil) + def scm_provider(arg=NULL_ARG) klass = if arg.kind_of?(String) || arg.kind_of?(Symbol) lookup_provider_constant(arg) else arg end - set_or_return( + nillable_set_or_return( :scm_provider, klass, :kind_of => [ Class ] ) end - def svn_force_export(arg=nil) - set_or_return( - :svn_force_export, - arg, - :kind_of => [ TrueClass, FalseClass ] - ) - end + attribute :svn_force_export, kind_of: [ TrueClass, FalseClass ] - def environment(arg=nil) + def environment(arg=NULL_ARG) if arg.is_a?(String) Chef::Log.debug "Setting RAILS_ENV, RACK_ENV, and MERB_ENV to `#{arg}'" Chef::Log.warn "[DEPRECATED] please modify your deploy recipe or attributes to set the environment using a hash" arg = {"RAILS_ENV"=>arg,"MERB_ENV"=>arg,"RACK_ENV"=>arg} end - set_or_return( + nillable_set_or_return( :environment, arg, :kind_of => [ Hash ] @@ -303,8 +185,8 @@ class Chef end # The number of old release directories to keep around after cleanup - def keep_releases(arg=nil) - [set_or_return( + def keep_releases(arg=NULL_ARG) + [nillable_set_or_return( :keep_releases, arg, :kind_of => [ Integer ]), 1].max @@ -314,13 +196,7 @@ class Chef # SCM clone/checkout before symlinking. Use this to get rid of files and # directories you want to be shared between releases. # Default: ["log", "tmp/pids", "public/system"] - def purge_before_symlink(arg=nil) - set_or_return( - :purge_before_symlink, - arg, - :kind_of => Array - ) - end + attribute :purge_before_symlink, kind_of: Array # An array of paths, relative to your app's root, where you expect dirs to # exist before symlinking. This runs after #purge_before_symlink, so you @@ -330,13 +206,7 @@ class Chef # then specify tmp here so that the tmp directory will exist when you # symlink the pids directory in to the current release. # Default: ["tmp", "public", "config"] - def create_dirs_before_symlink(arg=nil) - set_or_return( - :create_dirs_before_symlink, - arg, - :kind_of => Array - ) - end + attribute :create_dirs_before_symlink, kind_of: Array # A Hash of shared/dir/path => release/dir/path. This attribute determines # which files and dirs in the shared directory get symlinked to the current @@ -344,13 +214,7 @@ class Chef # $shared/pids that you would like to symlink as $current_release/tmp/pids # you specify it as "pids" => "tmp/pids" # Default {"system" => "public/system", "pids" => "tmp/pids", "log" => "log"} - def symlinks(arg=nil) - set_or_return( - :symlinks, - arg, - :kind_of => Hash - ) - end + attribute :symlinks, kind_of: Hash # A Hash of shared/dir/path => release/dir/path. This attribute determines # which files in the shared directory get symlinked to the current release @@ -359,74 +223,44 @@ class Chef # For a rails/merb app, this is used to link in a known good database.yml # (with the production db password) before running migrate. # Default {"config/database.yml" => "config/database.yml"} - def symlink_before_migrate(arg=nil) - set_or_return( - :symlink_before_migrate, - arg, - :kind_of => Hash - ) - end + attribute :symlink_before_migrate, kind_of: Hash # Callback fires before migration is run. - def before_migrate(arg=nil, &block) - arg ||= block - set_or_return(:before_migrate, arg, :kind_of => [Proc, String]) + def before_migrate(arg=NULL_ARG, &block) + arg = block if block_given? + nillable_set_or_return(:before_migrate, arg, kind_of: [Proc, String]) end # Callback fires before symlinking - def before_symlink(arg=nil, &block) - arg ||= block - set_or_return(:before_symlink, arg, :kind_of => [Proc, String]) + def before_symlink(arg=NULL_ARG, &block) + arg = block if block_given? + nillable_set_or_return(:before_symlink, arg, kind_of: [Proc, String]) end # Callback fires before restart - def before_restart(arg=nil, &block) - arg ||= block - set_or_return(:before_restart, arg, :kind_of => [Proc, String]) + def before_restart(arg=NULL_ARG, &block) + arg = block if block_given? + nillable_set_or_return(:before_restart, arg, kind_of: [Proc, String]) end # Callback fires after restart - def after_restart(arg=nil, &block) - arg ||= block - set_or_return(:after_restart, arg, :kind_of => [Proc, String]) + def after_restart(arg=NULL_ARG, &block) + arg = block if block_given? + nillable_set_or_return(:after_restart, arg, kind_of: [Proc, String]) end - def additional_remotes(arg=nil) - set_or_return( - :additional_remotes, - arg, - :kind_of => Hash - ) - end + attribute :additional_remotes, kind_of: Hash - def enable_checkout(arg=nil) - set_or_return( - :enable_checkout, - arg, - :kind_of => [TrueClass, FalseClass] - ) - end + attribute :enable_checkout, kind_of: [ TrueClass, FalseClass ] - def checkout_branch(arg=nil) - set_or_return( - :checkout_branch, - arg, - :kind_of => String - ) - end + attribute :checkout_branch, kind_of: String # FIXME The Deploy resource may be passed to an SCM provider as its # resource. The SCM provider knows that SCM resources can specify a # timeout for SCM operations. The deploy resource must therefore support # a timeout method, but the timeout it describes is for SCM operations, # not the overall deployment. This is potentially confusing. - def timeout(arg=nil) - set_or_return( - :timeout, - arg, - :kind_of => Integer - ) - end + attribute :timeout, kind_of: Integer end end diff --git a/lib/chef/resource/lwrp_base.rb b/lib/chef/resource/lwrp_base.rb index ce72e98028..a777c511f0 100644 --- a/lib/chef/resource/lwrp_base.rb +++ b/lib/chef/resource/lwrp_base.rb @@ -70,14 +70,6 @@ class Chef alias_method :resource_name=, :resource_name end - # Define an attribute on this resource, including optional validation - # parameters. - def self.attribute(attr_name, validation_opts={}) - define_method(attr_name) do |arg=nil| - set_or_return(attr_name.to_sym, arg, validation_opts) - end - end - # Sets the default action def self.default_action(action_name=NULL_ARG) unless action_name.equal?(NULL_ARG) diff --git a/spec/unit/mixin/params_validate_spec.rb b/spec/unit/mixin/params_validate_spec.rb index 85e1c1abab..1b61f9b238 100644 --- a/spec/unit/mixin/params_validate_spec.rb +++ b/spec/unit/mixin/params_validate_spec.rb @@ -339,69 +339,83 @@ describe Chef::Mixin::ParamsValidate do end.to raise_error(Chef::Exceptions::ValidationFailed) end - it "should set and return a value, then return the same value" do - value = "meow" - expect(@vo.set_or_return(:test, value, {}).object_id).to eq(value.object_id) - expect(@vo.set_or_return(:test, nil, {}).object_id).to eq(value.object_id) + def self.test_set_or_return_method(method) + # caller is responsible for passing in the right arg to get 'return' behavior + return_arg = method == :nillable_set_or_return ? TinyClass::NULL_ARG : nil + + it "#{method} should set and return a value, then return the same value" do + value = "meow" + expect(@vo.send(method,:test, value, {}).object_id).to eq(value.object_id) + expect(@vo.send(method,:test, return_arg, {}).object_id).to eq(value.object_id) + end + + it "#{method} should set and return a default value when the argument is nil, then return the same value" do + value = "meow" + expect(@vo.send(method,:test, return_arg, { :default => value }).object_id).to eq(value.object_id) + expect(@vo.send(method,:test, return_arg, {}).object_id).to eq(value.object_id) + end + + it "#{method} should raise an ArgumentError when argument is nil and required is true" do + expect { + @vo.send(method,:test, return_arg, { :required => true }) + }.to raise_error(ArgumentError) + end + + it "#{method} should not raise an error when argument is nil and required is false" do + expect { + @vo.send(method,:test, return_arg, { :required => false }) + }.not_to raise_error + end + + it "#{method} should set and return @name, then return @name for foo when argument is nil" do + value = "meow" + expect(@vo.send(method,:name, value, { }).object_id).to eq(value.object_id) + expect(@vo.send(method,:foo, return_arg, { :name_attribute => true }).object_id).to eq(value.object_id) + end + + it "#{method} should allow DelayedEvaluator instance to be set for value regardless of restriction" do + value = Chef::DelayedEvaluator.new{ 'test' } + @vo.send(method,:test, value, {:kind_of => Numeric}) + end + + it "#{method} should raise an error when delayed evaluated attribute is not valid" do + value = Chef::DelayedEvaluator.new{ 'test' } + @vo.send(method,:test, value, {:kind_of => Numeric}) + expect do + @vo.send(method,:test, return_arg, {:kind_of => Numeric}) + end.to raise_error(Chef::Exceptions::ValidationFailed) + end + + it "#{method} should create DelayedEvaluator instance when #lazy is used" do + @vo.send(method,:delayed, @vo.lazy{ 'test' }, {}) + expect(@vo.instance_variable_get(:@delayed)).to be_a(Chef::DelayedEvaluator) + end + + it "#{method} should execute block on each call when DelayedEvaluator" do + value = 'fubar' + @vo.send(method,:test, @vo.lazy{ value }, {}) + expect(@vo.send(method,:test, return_arg, {})).to eq('fubar') + value = 'foobar' + expect(@vo.send(method,:test, return_arg, {})).to eq('foobar') + value = 'fauxbar' + expect(@vo.send(method,:test, return_arg, {})).to eq('fauxbar') + end + + it "#{method} should not evaluate non DelayedEvaluator instances" do + value = lambda{ 'test' } + @vo.send(method,:test, value, {}) + expect(@vo.send(method,:test, return_arg, {}).object_id).to eq(value.object_id) + expect(@vo.send(method,:test, return_arg, {})).to be_a(Proc) + end end - it "should set and return a default value when the argument is nil, then return the same value" do - value = "meow" - expect(@vo.set_or_return(:test, nil, { :default => value }).object_id).to eq(value.object_id) - expect(@vo.set_or_return(:test, nil, {}).object_id).to eq(value.object_id) - end - - it "should raise an ArgumentError when argument is nil and required is true" do - expect { - @vo.set_or_return(:test, nil, { :required => true }) - }.to raise_error(ArgumentError) - end - - it "should not raise an error when argument is nil and required is false" do - expect { - @vo.set_or_return(:test, nil, { :required => false }) - }.not_to raise_error - end - - it "should set and return @name, then return @name for foo when argument is nil" do - value = "meow" - expect(@vo.set_or_return(:name, value, { }).object_id).to eq(value.object_id) - expect(@vo.set_or_return(:foo, nil, { :name_attribute => true }).object_id).to eq(value.object_id) - end + test_set_or_return_method(:set_or_return) + test_set_or_return_method(:nillable_set_or_return) - it "should allow DelayedEvaluator instance to be set for value regardless of restriction" do - value = Chef::DelayedEvaluator.new{ 'test' } - @vo.set_or_return(:test, value, {:kind_of => Numeric}) + it "nillable_set_or_return supports nilling values" do + expect(@vo.nillable_set_or_return(:test, "meow", {})).to eq("meow") + expect(@vo.nillable_set_or_return(:test, TinyClass::NULL_ARG, {})).to eq("meow") + expect(@vo.nillable_set_or_return(:test, nil, {})).to be_nil + expect(@vo.nillable_set_or_return(:test, TinyClass::NULL_ARG, {})).to be_nil end - - it "should raise an error when delayed evaluated attribute is not valid" do - value = Chef::DelayedEvaluator.new{ 'test' } - @vo.set_or_return(:test, value, {:kind_of => Numeric}) - expect do - @vo.set_or_return(:test, nil, {:kind_of => Numeric}) - end.to raise_error(Chef::Exceptions::ValidationFailed) - end - - it "should create DelayedEvaluator instance when #lazy is used" do - @vo.set_or_return(:delayed, @vo.lazy{ 'test' }, {}) - expect(@vo.instance_variable_get(:@delayed)).to be_a(Chef::DelayedEvaluator) - end - - it "should execute block on each call when DelayedEvaluator" do - value = 'fubar' - @vo.set_or_return(:test, @vo.lazy{ value }, {}) - expect(@vo.set_or_return(:test, nil, {})).to eq('fubar') - value = 'foobar' - expect(@vo.set_or_return(:test, nil, {})).to eq('foobar') - value = 'fauxbar' - expect(@vo.set_or_return(:test, nil, {})).to eq('fauxbar') - end - - it "should not evaluate non DelayedEvaluator instances" do - value = lambda{ 'test' } - @vo.set_or_return(:test, value, {}) - expect(@vo.set_or_return(:test, nil, {}).object_id).to eq(value.object_id) - expect(@vo.set_or_return(:test, nil, {})).to be_a(Proc) - end - end diff --git a/spec/unit/resource/deploy_spec.rb b/spec/unit/resource/deploy_spec.rb index 0403a7ba6b..07f5f973c0 100644 --- a/spec/unit/resource/deploy_spec.rb +++ b/spec/unit/resource/deploy_spec.rb @@ -30,12 +30,35 @@ describe Chef::Resource::Deploy do class << self + + def resource_has_a_hash_attribute(attr_name) + it "has a Hash attribute for #{attr_name.to_s}" do + @resource.send(attr_name, {foo: "bar"}) + expect(@resource.send(attr_name)).to eql({foo: "bar"}) + expect {@resource.send(attr_name, 8675309)}.to raise_error(ArgumentError) + end + + it "the Hash attribute for #{attr_name.to_s} is nillable" do + @resource.send(attr_name, {foo: "bar"}) + expect(@resource.send(attr_name)).to eql({foo: "bar"}) + @resource.send(attr_name, nil) + expect(@resource.send(attr_name)).to eql(nil) + end + end + def resource_has_a_string_attribute(attr_name) it "has a String attribute for #{attr_name.to_s}" do @resource.send(attr_name, "this is a string") expect(@resource.send(attr_name)).to eql("this is a string") expect {@resource.send(attr_name, 8675309)}.to raise_error(ArgumentError) end + + it "the String attribute for #{attr_name.to_s} is nillable" do + @resource.send(attr_name, "this is a string") + expect(@resource.send(attr_name)).to eql("this is a string") + @resource.send(attr_name, nil) + expect(@resource.send(attr_name)).to eql(nil) + end end def resource_has_a_boolean_attribute(attr_name, opts={:defaults_to=>false}) @@ -189,6 +212,10 @@ describe Chef::Resource::Deploy do expect(@resource.symlink_before_migrate).to eq({"wtf?" => "wtf is going on"}) end + resource_has_a_hash_attribute :symlink_before_migrate + resource_has_a_hash_attribute :symlinks + resource_has_a_hash_attribute :additional_remotes + resource_has_a_callback_attribute :before_migrate resource_has_a_callback_attribute :before_symlink resource_has_a_callback_attribute :before_restart |