Merge pull request #4732 from chef/lcg/use-inline-resources-core

allow use_inline_resources for core chef providers

6 files changed, 47 insertions, 95 deletions

diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index 8f947cd5e6..c6e3c40db2 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -1,84 +1,35 @@
-# Chef Client Release Notes 12.8:
+# Chef Client Release Notes 12.9:
 
-# Chef Client Release Notes 12.7:
+## Possible Compat Break with `use_inline_resources` and non-LWRPBase library providers
 
-## Updates to versioning strategy
-
-We recently updated the [versioning specification](https://github.com/chef/chef-rfc/pull/175) in our [release process](https://github.com/chef/chef-rfc/commits/master/rfc047-release-process.md) to facilitate faster releases with lower risk between each release.  Soon an automated process will begin updating the "patch" version (the third number in the version).
-
-For consumers of Chef this means that the first release of 12.7 you use may be 12.7.2 or 12.7.5 without earlier versions of 12.7 having been released first.  Those earlier versions may be available in the _current_ channel that you can access through the install.sh and install.ps1 scripts.
-
-## Net-ssh updates
-
-We updated the version of [net-ssh](https://github.com/net-ssh/net-ssh) in Chef from version 2.9 to 3.0 to take in an upstream bug fix.  The biggest change here is that they dropped support for Ruby 1.9 (which Chef already dropped support for).  Because this is such a low level dependency we found that many other projects had to be updated in lock-step (like Test Kitchen and Berkshelf) for the ChefDK packaging to succeed without dependency conflicts.
-
-## Zypper Package Multipackage Support
-
-On SuSE systems the `package` provider (aka `zypper_package` provider) now accepts arrays and will install them with a single zypper command together:
+The correct pattern for LWRPBase-style library providers looks something like:
 
 ```ruby
-package [ 'git', 'nmap' ]
-```
-
-Some additional code-cleanup was done to the provider and long-standing bugs may have been fixed.
-
-## Chocolatey Package Provider
-
-There is now a `chocolatey_package` provider in core chef.  It is named `chocolatey_package` instead of `chocolatey` in order to not conflict with the existing resource in the chocolatey cookbook and to
-comply with existing naming standards for package resources in core chef.
-
-The API for `chocolatey_package` conforms to the `package` API in core chef, rather than being a straight port of the cookbook version, and there are some API differences (e.g. it favors the `:remove`
-action over the `:uninstall` action since that is the API standard for core chef package providers).  The `chocolatey_package` provider also supports multipackage installations and will execute them
-in a single statement where possible:
-
-```ruby
-chocolatey_package [ 'googlechrome', 'flashplayerplugin', '7zip', 'git' ]
-```
-
-The `choco.exe` binary must be installed prior to using the resource, so the chocolatey cookbook recipe should still be used to install it.
-
-## EMEA Customers and UTF-8 Support
-
-EMEA customers in particular, and those customers who need reliable UTF-8 support, are highly encouraged to upgrade to the 12.7.0 release.  The 12.4.x/12.5.x/12.6.x releases of chef-client had an
-extremely bad UTF-8 handling bug in them which corrupted all UTF-8 data in the node.  In 12.7.0 that bug was fixed, along with another fix to make resource and audit reporting more reliable when fed
-non-UTF-8 (e.g. Latin-1/ISO-8859-1) characters.
-
-## Chef Solo -r (--recipe-url) changes
-
-The use of the `-r` option to chef-client result in setting the `--run-list`:
+class MyProvider < Chef::Provider::LWRPBase
+  use_inline_resources
 
+  action :stuff do
+    file "/tmp/whatever.txt"
+  end
+end
 ```
-chef-client -r 'role[foo]'
-```
-
-Passing the same argument to chef-solo:
 
-```
-chef-solo -r 'role[foo]'
-```
-
-Instead invokes the `--recipe-url` code, which had the side effect of running an immediate unprompted `rm -rf *` in the current working directory of the user.   Due to this problem and other issues
-around this `rm -rf *` behavior it has been removed from the `--recipe-url` code in chef-solo.  The use of `-r` in chef-solo to mean `--recipe-url` has also been deprecated.
-
-The `rm -rf *` behavior has been moved to a `--delete-entire-chef-repo` option.  Users of chef-solo who want the old pre-12.7 behavior of `-r XXX` should therefore use `--recipe-url XXX --delete-entire-chef-repo`.
-
-## Chef::REST
-
-We recently completed moving our internal API calls from `Chef::REST` to
-`Chef::ServerAPI`. As part of that move, `Chef::REST` is no longer globally
-required, so if your code uses `Chef::REST`, you must ensure that you
-require it correctly.
+Starting in 12.5 the `use_inline_resources` directive was mixed into Chef::Provider directly and had the
+side effect of mixing in the DSL.  After 12.5 it would have worked to write code like this:
 
 ```ruby
-require 'chef/rest'
-```
-
-We strongly encourage users to move away from using `Chef::REST`; if
-your code is run inside `knife` or `chef` then consider using
-`Chef::ServerAPI`, otherwise please investigate [ChefAPI](http://sethvargo.github.io/chef-api/).
+class MyProvider < Chef::Provider
+  use_inline_resources
 
-## Nokogiri
+  action :stuff do
+    file "/tmp/whatever.txt"
+  end
+end
+```
 
-The latest version of the nokogiri gem will now be included in all omnibus-chef builds.  See
-[RFC 063](https://github.com/chef/chef-rfc/blob/master/rfc063-omnibus-chef-native-gems.md) and
-[RFC 063 PR discussion](https://github.com/chef/chef-rfc/pull/162) for more information.
+But that code would be broken (with a hard syntax error on `use_inline_resources` on prior versions of
+chef-client).  After 12.9 that code will now be broken again on the use of the Recipe DSL which has been removed
+from Chef::Provider when mixing `use_inline_resources` into classes that only inherit from the core
+class.  If any code has been written like this, it should be modified to correctly inherit from
+Chef::Provider::LWRPBase instead (which will have the side effect of fixing it so that it correctly works
+on Chef 11.0-12.5 as well).
diff --git a/lib/chef/provider.rb b/lib/chef/provider.rb
index cef95a4db8..ebabb7b9eb 100644
--- a/lib/chef/provider.rb
+++ b/lib/chef/provider.rb
@@ -390,9 +390,6 @@ class Chef
           end
         end
       end
-
-      require "chef/dsl/recipe"
-      include Chef::DSL::Recipe
     end
 
     protected
diff --git a/lib/chef/provider/apt_update.rb b/lib/chef/provider/apt_update.rb
index 449b298804..d2dd5cfb14 100644
--- a/lib/chef/provider/apt_update.rb
+++ b/lib/chef/provider/apt_update.rb
@@ -22,7 +22,7 @@ require "chef/dsl/declare_resource"
 class Chef
   class Provider
     class AptUpdate < Chef::Provider
-      include Chef::DSL::DeclareResource
+      use_inline_resources
 
       provides :apt_update, os: "linux"
 
@@ -36,7 +36,7 @@ class Chef
       def load_current_resource
       end
 
-      def action_periodic
+      action :periodic do
         if !apt_up_to_date?
           converge_by "update new lists of packages" do
             do_update
@@ -44,7 +44,7 @@ class Chef
         end
       end
 
-      def action_update
+      action :update do
         converge_by "force update new lists of packages" do
           do_update
         end
@@ -62,16 +62,17 @@ class Chef
 
       def do_update
         [STAMP_DIR, APT_CONF_DIR].each do |d|
-          build_resource(:directory, d, caller[0]) do
+          declare_resource(:directory, d) do
             recursive true
-          end.run_action(:create)
+          end
         end
 
-        build_resource(:file, "#{APT_CONF_DIR}/15update-stamp", caller[0]) do
+        declare_resource(:file, "#{APT_CONF_DIR}/15update-stamp") do
           content "APT::Update::Post-Invoke-Success {\"touch #{STAMP_DIR}/update-success-stamp 2>/dev/null || true\";};"
-        end.run_action(:create_if_missing)
+          action :create_if_missing
+        end
 
-        shell_out!("apt-get -q update")
+        declare_resource(:execute, "apt-get -q update")
       end
 
     end
diff --git a/lib/chef/resource/action_class.rb b/lib/chef/resource/action_class.rb
index 3d9f2f3e7c..89b23499d0 100644
--- a/lib/chef/resource/action_class.rb
+++ b/lib/chef/resource/action_class.rb
@@ -17,10 +17,13 @@
 #
 
 require "chef/exceptions"
+require "chef/dsl/recipe"
 
 class Chef
   class Resource
     module ActionClass
+      include Chef::DSL::Recipe
+
       def to_s
         "#{new_resource || "<no resource>"} action #{action ? action.inspect : "<no action>"}"
       end
diff --git a/spec/unit/lwrp_spec.rb b/spec/unit/lwrp_spec.rb
index 6eba001af4..6574a91f13 100644
--- a/spec/unit/lwrp_spec.rb
+++ b/spec/unit/lwrp_spec.rb
@@ -754,7 +754,7 @@ describe "LWRP" do
 
     it "lets you extend the recipe DSL" do
       expect(Chef::Recipe).to receive(:include).with(MyAwesomeDSLExensionClass)
-      expect(Chef::Provider::InlineResources).to receive(:include).with(MyAwesomeDSLExensionClass)
+      expect(Chef::Resource::ActionClass).to receive(:include).with(MyAwesomeDSLExensionClass)
       Chef::DSL::Recipe.send(:include, MyAwesomeDSLExensionClass)
     end
 
diff --git a/spec/unit/provider/apt_update_spec.rb b/spec/unit/provider/apt_update_spec.rb
index b72f7d9a76..3e3e6ba07a 100644
--- a/spec/unit/provider/apt_update_spec.rb
+++ b/spec/unit/provider/apt_update_spec.rb
@@ -44,19 +44,19 @@ describe Chef::Provider::AptUpdate do
   context "when the apt config directory does not exist" do
     before do
       FileUtils.rmdir config_dir
-      expect(File.exist?(config_dir)).to be_falsey
-      allow(provider).to receive(:shell_out!).with("apt-get -q update")
+      expect(File.exist?(config_dir)).to be false
+      allow_any_instance_of(Chef::Provider::Execute).to receive(:shell_out!).with("apt-get -q update", anything())
     end
 
     it "should create the directory" do
       provider.run_action(:update)
-      expect(File.exist?(config_dir)).to be_truthy
-      expect(File.directory?(config_dir)).to be_truthy
+      expect(File.exist?(config_dir)).to be true
+      expect(File.directory?(config_dir)).to be true
     end
 
     it "should create the config file" do
       provider.run_action(:update)
-      expect(File.exist?(config_file)).to be_truthy
+      expect(File.exist?(config_file)).to be true
       expect(File.read(config_file)).to match(/^APT::Update.*#{stamp_dir}/)
     end
   end
@@ -64,7 +64,7 @@ describe Chef::Provider::AptUpdate do
   describe "#action_update" do
     it "should update the apt cache" do
       provider.load_current_resource
-      expect(provider).to receive(:shell_out!).with("apt-get -q update").and_return(double)
+      expect_any_instance_of(Chef::Provider::Execute).to receive(:shell_out!).with("apt-get -q update", anything())
       provider.run_action(:update)
       expect(new_resource).to be_updated_by_last_action
     end
@@ -78,14 +78,14 @@ describe Chef::Provider::AptUpdate do
 
     it "should run if the time stamp is old" do
       expect(File).to receive(:mtime).with("#{stamp_dir}/update-success-stamp").and_return(Time.now - 86_500)
-      expect(provider).to receive(:shell_out!).with("apt-get -q update")
+      expect_any_instance_of(Chef::Provider::Execute).to receive(:shell_out!).with("apt-get -q update", anything())
       provider.run_action(:periodic)
       expect(new_resource).to be_updated_by_last_action
     end
 
     it "should not run if the time stamp is new" do
       expect(File).to receive(:mtime).with("#{stamp_dir}/update-success-stamp").and_return(Time.now)
-      expect(provider).to_not receive(:shell_out!).with("apt-get -q update")
+      expect_any_instance_of(Chef::Provider::Execute).not_to receive(:shell_out!).with("apt-get -q update", anything())
       provider.run_action(:periodic)
       expect(new_resource).to_not be_updated_by_last_action
     end
@@ -97,14 +97,14 @@ describe Chef::Provider::AptUpdate do
 
       it "should run if the time stamp is old" do
         expect(File).to receive(:mtime).with("#{stamp_dir}/update-success-stamp").and_return(Time.now - 500)
-        expect(provider).to receive(:shell_out!).with("apt-get -q update")
+        expect_any_instance_of(Chef::Provider::Execute).to receive(:shell_out!).with("apt-get -q update", anything())
         provider.run_action(:periodic)
         expect(new_resource).to be_updated_by_last_action
       end
 
       it "should not run if the time stamp is new" do
         expect(File).to receive(:mtime).with("#{stamp_dir}/update-success-stamp").and_return(Time.now - 300)
-        expect(provider).to_not receive(:shell_out!).with("apt-get -q update")
+        expect_any_instance_of(Chef::Provider::Execute).not_to receive(:shell_out!).with("apt-get -q update", anything())
         provider.run_action(:periodic)
         expect(new_resource).to_not be_updated_by_last_action
       end