Merge pull request #5938 from chef/lcg/dsl-args

Chef-13: Support nameless resources and remove deprecated multi-arg resources

13 files changed, 130 insertions, 29 deletions

diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index 4e3c7dd050..a1e5ceab45 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -142,3 +142,18 @@ available in the [`poise-python`](https://github.com/poise/poise-python) cookboo
 
 We've upgraded to the latest stable release of the Ruby programming
 language.
+
+### Resource can now declare a default name
+
+The core `apt_update` resource can now be declared without any name argument, no need for `apt_update "this string doesn't matter but
+why do i have to type it?"`.
+
+This can be used by any other resource by just overriding the name property and supplying a default:
+
+```ruby
+  property :name, String, default: ""
+```
+
+Notifications to resources with empty strings as their name is also supported via either the bare resource name (`apt_update` --
+matches what the user types in the DSL) or with empty brackets (`apt_update[]` -- matches the resource notification pattern).
+
diff --git a/lib/chef/dsl/resources.rb b/lib/chef/dsl/resources.rb
index 36ec018500..638e626ae2 100644
--- a/lib/chef/dsl/resources.rb
+++ b/lib/chef/dsl/resources.rb
@@ -34,9 +34,8 @@ class Chef
 
       def self.add_resource_dsl(dsl_name)
         module_eval(<<-EOM, __FILE__, __LINE__ + 1)
-            def #{dsl_name}(*args, &block)
-              Chef.deprecated(:internal_api, "Cannot create resource #{dsl_name} with more than one argument. All arguments except the name (\#{args[0].inspect}) will be ignored. This will cause an error in Chef 13. Arguments: \#{args}") if args.size > 1
-              declare_resource(#{dsl_name.inspect}, args[0], created_at: caller[0], &block)
+            def #{dsl_name}(args = nil, &block)
+              declare_resource(#{dsl_name.inspect}, args, created_at: caller[0], &block)
             end
           EOM
       end
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 2aee8536a4..9dc01a8d3a 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -84,7 +84,7 @@ class Chef
     # @param name [Object] The name to set, typically a String or Array
     # @return [String] The name of this Resource.
     #
-    property :name, String, coerce: proc { |v| v.is_a?(Array) ? v.join(", ") : v.to_s }, desired_state: false
+    property :name, String, coerce: proc { |v| v.is_a?(Array) ? v.join(", ") : v.to_s }, desired_state: false, required: true
 
     #
     # The node the current Chef run is using.
diff --git a/lib/chef/resource/apt_update.rb b/lib/chef/resource/apt_update.rb
index 1a25ec2ef5..67ca7fbfea 100644
--- a/lib/chef/resource/apt_update.rb
+++ b/lib/chef/resource/apt_update.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Thom May (<thom@chef.io>)
-# Copyright:: Copyright (c) 2016 Chef Software, Inc.
+# Copyright:: Copyright (c) 2016-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -24,6 +24,8 @@ class Chef
       resource_name :apt_update
       provides :apt_update
 
+      # allow bare apt_update with no name
+      property :name, String, default: ""
       property :frequency, Integer, default: 86_400
 
       default_action :periodic
diff --git a/lib/chef/resource_builder.rb b/lib/chef/resource_builder.rb
index 275f795362..43a2495cba 100644
--- a/lib/chef/resource_builder.rb
+++ b/lib/chef/resource_builder.rb
@@ -43,8 +43,6 @@ class Chef
     end
 
     def build(&block)
-      raise ArgumentError, "You must supply a name when declaring a #{type} resource" if name.nil?
-
       @resource = resource_class.new(name, run_context)
       if resource.resource_name.nil?
         raise Chef::Exceptions::InvalidResourceSpecification, "#{resource}.resource_name is `nil`!  Did you forget to put `provides :blah` or `resource_name :blah` in your resource class?"
diff --git a/lib/chef/resource_collection/resource_set.rb b/lib/chef/resource_collection/resource_set.rb
index 2c7c0a0b91..111d23dc09 100644
--- a/lib/chef/resource_collection/resource_set.rb
+++ b/lib/chef/resource_collection/resource_set.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Tyler Ball (<tball@chef.io>)
-# Copyright:: Copyright 2014-2016, Chef Software, Inc.
+# Copyright:: Copyright 2014-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -30,7 +30,10 @@ class Chef
 
       # Matches a single resource lookup specification,
       # e.g., "service[nginx]"
-      SINGLE_RESOURCE_MATCH = /^(.+)\[(.+)\]$/
+      SINGLE_RESOURCE_MATCH = /^(.+)\[(.*)\]$/
+
+      # Matches e.g. "apt_update" with no name
+      NAMELESS_RESOURCE_MATCH = /^([^\[\]\s]+)$/
 
       def initialize
         @resources_by_key = Hash.new
@@ -118,7 +121,7 @@ class Chef
         case query_object
           when Chef::Resource
             true
-          when SINGLE_RESOURCE_MATCH, MULTIPLE_RESOURCE_MATCH
+          when SINGLE_RESOURCE_MATCH, MULTIPLE_RESOURCE_MATCH, NAMELESS_RESOURCE_MATCH
             true
           when Hash
             true
@@ -171,6 +174,10 @@ class Chef
             resource_type = $1
             name = $2
             results << lookup(create_key(resource_type, name))
+          when NAMELESS_RESOURCE_MATCH
+            resource_type = $1
+            name = ""
+            results << lookup(create_key(resource_type, name))
           else
             raise ArgumentError, "Bad string format #{arg}, you must have a string like resource_type[name]!"
         end
diff --git a/spec/integration/recipes/notifies_spec.rb b/spec/integration/recipes/notifies_spec.rb
index 000f5e37bf..6f6a0f06b0 100644
--- a/spec/integration/recipes/notifies_spec.rb
+++ b/spec/integration/recipes/notifies_spec.rb
@@ -8,6 +8,37 @@ describe "notifications" do
   let(:chef_dir) { File.expand_path("../../../../bin", __FILE__) }
   let(:chef_client) { "ruby '#{chef_dir}/chef-client' --minimal-ohai" }
 
+  when_the_repository "notifies a nameless resource" do
+    before do
+      directory "cookbooks/x" do
+        file "recipes/default.rb", <<-EOM
+          apt_update do
+            action :nothing
+          end
+          log "foo" do
+            notifies :nothing, 'apt_update', :delayed
+          end
+          log "bar" do
+            notifies :nothing, 'apt_update[]', :delayed
+          end
+        EOM
+      end
+    end
+
+    it "should complete with success" do
+      file "config/client.rb", <<EOM
+local_mode true
+cookbook_path "#{path_to('cookbooks')}"
+log_level :warn
+EOM
+
+      result = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" --no-color -F doc -o 'x::default'", :cwd => chef_dir)
+      # our delayed notification should run at the end of the parent run_context after the baz resource
+      expect(result.stdout).to match(/\* apt_update\[\] action nothing \(skipped due to action :nothing\)\s+\* log\[foo\] action write\s+\* log\[bar\] action write\s+\* apt_update\[\] action nothing \(skipped due to action :nothing\)/)
+      result.error!
+    end
+  end
+
   when_the_repository "notifies delayed one" do
     before do
       directory "cookbooks/x" do
diff --git a/spec/integration/recipes/recipe_dsl_spec.rb b/spec/integration/recipes/recipe_dsl_spec.rb
index a408bbb0d7..f7a0e8b6e8 100644
--- a/spec/integration/recipes/recipe_dsl_spec.rb
+++ b/spec/integration/recipes/recipe_dsl_spec.rb
@@ -59,21 +59,30 @@ describe "Recipe DSL methods" do
       expect(BaseThingy.created_resource).to eq BaseThingy
     end
 
-    it "errors out when you call base_thingy do ... end in a recipe" do
+    it "errors when you call base_thingy do ... end in a recipe" do
       expect_converge do
         base_thingy { ; }
-      end.to raise_error(ArgumentError, "You must supply a name when declaring a base_thingy resource")
+      end.to raise_error(Chef::Exceptions::ValidationFailed)
     end
 
-    it "emits a warning when you call base_thingy 'foo', 'bar' do ... end in a recipe" do
-      Chef::Config[:treat_deprecation_warnings_as_errors] = false
-      recipe = converge do
-        base_thingy "foo", "bar" do
+    context "nameless resources" do
+      before(:context) do
+        class NamelessThingy < BaseThingy
+          resource_name :nameless_thingy
+          provides :nameless_thingy
+
+          property :name, String, default: ""
         end
       end
-      expect(recipe.logged_warnings).to match(/Cannot create resource base_thingy with more than one argument. All arguments except the name \("foo"\) will be ignored. This will cause an error in Chef 13. Arguments: \["foo", "bar"\]/)
-      expect(BaseThingy.created_name).to eq "foo"
-      expect(BaseThingy.created_resource).to eq BaseThingy
+
+      it "does not error when not given a name" do
+        recipe = converge do
+          nameless_thingy {}
+        end
+        expect(recipe.logged_warnings).to eq ""
+        expect(BaseThingy.created_name).to eq ""
+        expect(BaseThingy.created_resource).to eq NamelessThingy
+      end
     end
 
     context "Deprecated automatic resource DSL" do
diff --git a/spec/unit/property_spec.rb b/spec/unit/property_spec.rb
index bd90891b63..79ec05ea6d 100644
--- a/spec/unit/property_spec.rb
+++ b/spec/unit/property_spec.rb
@@ -301,7 +301,7 @@ describe "Chef::Resource.property" do
       expect(resource.property_is_set?(:name)).to be_truthy
       resource.reset_property(:name)
       expect(resource.property_is_set?(:name)).to be_falsey
-      expect(resource.name).to be_nil
+      expect { resource.name }.to raise_error Chef::Exceptions::ValidationFailed
     end
 
     it "when referencing an undefined property, reset_property(:x) raises an error" do
diff --git a/spec/unit/recipe_spec.rb b/spec/unit/recipe_spec.rb
index cb500da34a..7a538b721b 100644
--- a/spec/unit/recipe_spec.rb
+++ b/spec/unit/recipe_spec.rb
@@ -74,12 +74,6 @@ describe Chef::Recipe do
         expect { recipe.not_home("not_home_resource") }.to raise_error(NameError)
       end
 
-      it "should require a name argument" do
-        expect do
-          recipe.cat
-        end.to raise_error(ArgumentError)
-      end
-
       it "should allow regular errors (not NameErrors) to pass unchanged" do
         expect do
           recipe.cat("felix") { raise ArgumentError, "You Suck" }
diff --git a/spec/unit/resource/conditional_spec.rb b/spec/unit/resource/conditional_spec.rb
index e84b0980c4..135f798676 100644
--- a/spec/unit/resource/conditional_spec.rb
+++ b/spec/unit/resource/conditional_spec.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Daniel DeLeo (<dan@chef.io>)
-# Copyright:: Copyright 2011-2016, Chef Software Inc.
+# Copyright:: Copyright 2011-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -24,7 +24,7 @@ describe Chef::Resource::Conditional do
     allow_any_instance_of(Mixlib::ShellOut).to receive(:run_command).and_return(nil)
     @status = OpenStruct.new(:success? => true)
     allow_any_instance_of(Mixlib::ShellOut).to receive(:status).and_return(@status)
-    @parent_resource = Chef::Resource.new(nil, Chef::Node.new)
+    @parent_resource = Chef::Resource.new("", Chef::Node.new)
   end
 
   it "raises an exception when neither a block or command is given" do
diff --git a/spec/unit/resource_collection/resource_set_spec.rb b/spec/unit/resource_collection/resource_set_spec.rb
index 20f6f70911..ecf3e2707f 100644
--- a/spec/unit/resource_collection/resource_set_spec.rb
+++ b/spec/unit/resource_collection/resource_set_spec.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Tyler Ball (<tball@chef.io>)
-# Copyright:: Copyright 2014-2016, Chef Software, Inc.
+# Copyright:: Copyright 2014-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -162,6 +162,35 @@ describe Chef::ResourceCollection::ResourceSet do
     it "raises an error when attempting to find a resource that does not exist" do
       expect { collection.find("script[nonesuch]") }.to raise_error(Chef::Exceptions::ResourceNotFound)
     end
+
+    context "nameless resources" do
+      let(:zen_master_name) { "" }
+
+      it "looks up nameless resources with find without brackets" do
+        collection.insert_as(zen_master)
+        expect(collection.find("zen_master")).to eq(zen_master)
+      end
+
+      it "looks up nameless resources with find with empty brackets" do
+        collection.insert_as(zen_master)
+        expect(collection.find("zen_master[]")).to eq(zen_master)
+      end
+
+      it "looks up nameless resources with find with empty string hash key" do
+        collection.insert_as(zen_master)
+        expect(collection.find(zen_master: "")).to eq(zen_master)
+      end
+
+      it "looks up nameless resources with lookup with empty brackets" do
+        collection.insert_as(zen_master)
+        expect(collection.lookup("zen_master[]")).to eq(zen_master)
+      end
+
+      it "looks up nameless resources with lookup with the resource" do
+        collection.insert_as(zen_master)
+        expect(collection.lookup(zen_master)).to eq(zen_master)
+      end
+    end
   end
 
   describe "validate_lookup_spec!" do
diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
index a806c5d1d9..6285bad50a 100644
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -284,6 +284,23 @@ describe Chef::Resource do
       resource.notifies :reload, run_context.resource_collection.find(:zen_master => "coffee, tea")
       expect(resource.delayed_notifications.detect { |e| e.resource.name == "coffee, tea" && e.action == :reload }).not_to be_nil
     end
+
+    it "notifies a resource without a name via a string name with brackets" do
+      run_context.resource_collection << Chef::Resource::ZenMaster.new("")
+      resource.notifies :reload, "zen_master[]"
+    end
+
+    it "notifies a resource without a name via a string name without brackets" do
+      run_context.resource_collection << Chef::Resource::ZenMaster.new("")
+      resource.notifies :reload, "zen_master"
+      expect(resource.delayed_notifications.first.resource).to eql("zen_master")
+    end
+
+    it "notifies a resource without a name via a hash name with an empty string" do
+      run_context.resource_collection << Chef::Resource::ZenMaster.new("")
+      resource.notifies :reload, zen_master: ""
+      expect(resource.delayed_notifications.first.resource).to eql(zen_master: "")
+    end
   end
 
   describe "subscribes" do