Merge pull request #7946 from chef/properties

Convert service resource to use properties

6 files changed, 198 insertions, 260 deletions

diff --git a/lib/chef/resource/macosx_service.rb b/lib/chef/resource/macosx_service.rb
index deff6e1927..37681ca930 100644
--- a/lib/chef/resource/macosx_service.rb
+++ b/lib/chef/resource/macosx_service.rb
@@ -27,10 +27,6 @@ class Chef
 
       description "Use the macosx_service resource to manage services on the macOS platform."
 
-      identity_attr :service_name
-
-      state_attrs :enabled, :running
-
       property :plist, String,
                description: "A plist to use in the case where the filename and label for the service do not match."
 
diff --git a/lib/chef/resource/service.rb b/lib/chef/resource/service.rb
index c5197d5f06..4e808d6035 100644
--- a/lib/chef/resource/service.rb
+++ b/lib/chef/resource/service.rb
@@ -1,7 +1,7 @@
 #
 # Author:: AJ Christensen (<aj@hjksolutions.com>)
 # Author:: Tyler Cloke (<tyler@chef.io>)
-# Copyright:: Copyright 2008-2017, Chef Software Inc.
+# Copyright:: Copyright 2008-2018, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -25,8 +25,6 @@ class Chef
     class Service < Chef::Resource
       identity_attr :service_name
 
-      state_attrs :enabled, :running, :masked
-
       description "Use the service resource to manage a service."
 
       default_action :nothing
@@ -35,137 +33,64 @@ class Chef
 
       # this is a poor API please do not re-use this pattern
       property :supports, Hash, default: { restart: nil, reload: nil, status: nil },
-                                coerce: proc { |x| x.is_a?(Array) ? x.each_with_object({}) { |i, m| m[i] = true } : x }
-
-      def initialize(name, run_context = nil)
-        super
-        @service_name = name
-        @enabled = nil
-        @running = nil
-        @masked = nil
-        @options = nil
-        @parameters = nil
-        @pattern = service_name
-        @start_command = nil
-        @stop_command = nil
-        @status_command = nil
-        @restart_command = nil
-        @reload_command = nil
-        @init_command = nil
-        @priority = nil
-        @timeout = nil
-        @run_levels = nil
-        @user = nil
-      end
-
-      def service_name(arg = nil)
-        set_or_return(
-          :service_name,
-          arg,
-          kind_of: [ String ]
-        )
-      end
+               description: "A list of properties that controls how the chef-client is to attempt to manage a service: :restart, :reload, :status. For :restart, the init script or other service provider can use a restart command; if :restart is not specified, the chef-client attempts to stop and then start a service. For :reload, the init script or other service provider can use a reload command. For :status, the init script or other service provider can use a status command to determine if the service is running; if :status is not specified, the chef-client attempts to match the service_name against the process table as a regular expression, unless a pattern is specified as a parameter property. Default value: { restart: false, reload: false, status: false } for all platforms (except for the Red Hat platform family, which defaults to { restart: false, reload: false, status: true }.)",
+               coerce: proc { |x| x.is_a?(Array) ? x.each_with_object({}) { |i, m| m[i] = true } : x }
+
+      property :service_name, String,
+               description: "An optional property to set the service name if it differs from the resource block's name.",
+               name_property: true, identity: true
 
       # regex for match against ps -ef when !supports[:has_status] && status == nil
-      def pattern(arg = nil)
-        set_or_return(
-          :pattern,
-          arg,
-          kind_of: [ String ]
-        )
-      end
+      property :pattern, String,
+               description: "The pattern to look for in the process table.",
+               default_description: "The value provided to 'service_name' or the resource block's name",
+               default: lazy { service_name }, desired_state: false
 
       # command to call to start service
-      def start_command(arg = nil)
-        set_or_return(
-          :start_command,
-          arg,
-          kind_of: [ String, NilClass, FalseClass ]
-        )
-      end
+      property :start_command, [ String, nil, FalseClass ],
+               description: "The command used to start a service.",
+               desired_state: false
 
       # command to call to stop service
-      def stop_command(arg = nil)
-        set_or_return(
-          :stop_command,
-          arg,
-          kind_of: [ String, NilClass, FalseClass ]
-        )
-      end
+      property :stop_command, [ String, nil, FalseClass ],
+               description: "The command used to stop a service.",
+               desired_state: false
 
       # command to call to get status of service
-      def status_command(arg = nil)
-        set_or_return(
-          :status_command,
-          arg,
-          kind_of: [ String, NilClass, FalseClass ]
-        )
-      end
+      property :status_command, [ String, nil, FalseClass ],
+               description: "The command used to check the run status for a service.",
+               desired_state: false
 
       # command to call to restart service
-      def restart_command(arg = nil)
-        set_or_return(
-          :restart_command,
-          arg,
-          kind_of: [ String, NilClass, FalseClass ]
-        )
-      end
-
-      def reload_command(arg = nil)
-        set_or_return(
-          :reload_command,
-          arg,
-          kind_of: [ String, NilClass, FalseClass ]
-        )
-      end
+      property :restart_command, [ String, nil, FalseClass ],
+               description: "The command used to restart a service.",
+               desired_state: false
+
+      property :reload_command, [ String, nil, FalseClass ],
+               description: "The command used to tell a service to reload its configuration.",
+               desired_state: false
 
       # The path to the init script associated with the service. On many
       # distributions this is '/etc/init.d/SERVICE_NAME' by default. In
       # non-standard configurations setting this value will save having to
       # specify overrides for the start_command, stop_command and
       # restart_command properties.
-      def init_command(arg = nil)
-        set_or_return(
-          :init_command,
-          arg,
-          kind_of: [ String ]
-        )
-      end
+      property :init_command, String,
+               description: "The path to the init script that is associated with the service. Use init_command to prevent the need to specify overrides for the start_command, stop_command, and restart_command properties. When this property is not specified, the chef-client will use the default init command for the service provider being used.",
+               desired_state: false
 
       # if the service is enabled or not
-      def enabled(arg = nil)
-        set_or_return(
-          :enabled,
-          arg,
-          kind_of: [ TrueClass, FalseClass ]
-        )
-      end
+      property :enabled, [ TrueClass, FalseClass ], skip_docs: true
 
       # if the service is running or not
-      def running(arg = nil)
-        set_or_return(
-          :running,
-          arg,
-          kind_of: [ TrueClass, FalseClass ]
-        )
-      end
+      property :running, [ TrueClass, FalseClass ], skip_docs: true
 
       # if the service is masked or not
-      def masked(arg = nil)
-        set_or_return(
-          :masked,
-          arg,
-          kind_of: [ TrueClass, FalseClass ]
-        )
-      end
-
-      def options(arg = nil)
-        set_or_return(
-          :options,
-          arg.respond_to?(:split) ? arg.shellsplit : arg,
-          kind_of: [ Array, String ]
-        )
-      end
+      property :masked, [ TrueClass, FalseClass ], skip_docs: true
+
+      property :options, [ Array, String ],
+               description: "Solaris platform only. Options to pass to the service command. See the svcadm manual for details of possible options.",
+               coerce: proc { |x| x.respond_to?(:split) ? x.shellsplit : x }
 
       # Priority arguments can have two forms:
       #
@@ -177,45 +102,23 @@ class Chef
       #   runlevel 2, stopped in 3 with priority 55 and no symlinks or
       #   similar for other runlevels
       #
-      def priority(arg = nil)
-        set_or_return(
-          :priority,
-          arg,
-          kind_of: [ Integer, String, Hash ]
-        )
-      end
+      property :priority, [ Integer, String, Hash ],
+               description: "Debian platform only. The relative priority of the program for start and shutdown ordering. May be an integer or a Hash. An integer is used to define the start run levels; stop run levels are then 100-integer. A Hash is used to define values for specific run levels. For example, { 2 => [:start, 20], 3 => [:stop, 55] } will set a priority of twenty for run level two and a priority of fifty-five for run level three."
 
       # timeout only applies to the windows service manager
-      def timeout(arg = nil)
-        set_or_return(
-          :timeout,
-          arg,
-          kind_of: Integer
-        )
-      end
-
-      def parameters(arg = nil)
-        set_or_return(
-          :parameters,
-          arg,
-          kind_of: [ Hash ]
-        )
-      end
-
-      def run_levels(arg = nil)
-        set_or_return(
-          :run_levels,
-          arg,
-          kind_of: [ Array ] )
-      end
-
-      def user(arg = nil)
-        set_or_return(
-          :user,
-          arg,
-          kind_of: [ String ]
-        )
-      end
+      property :timeout, Integer,
+               description: "Microsoft Windows platform only. The amount of time (in seconds) to wait before timing out.",
+               desired_state: false
+
+      property :parameters, Hash,
+               description: "Upstart only: A hash of parameters to pass to the service command for use in the service definition."
+
+      property :run_levels, Array,
+               description: "RHEL platforms only: Specific run_levels the service will run under."
+
+      property :user, String,
+               description: "systemd only: A username to run the service under.",
+               introduced: "12.21"
     end
   end
 end
diff --git a/lib/chef/resource/windows_service.rb b/lib/chef/resource/windows_service.rb
index 7b6593f037..33805ed482 100644
--- a/lib/chef/resource/windows_service.rb
+++ b/lib/chef/resource/windows_service.rb
@@ -41,29 +41,23 @@ class Chef
 
       allowed_actions :configure_startup, :create, :delete, :configure
 
-      state_attrs :enabled, :running
-
-      property :service_name, String,
-               description: "The name of the service.",
-               name_property: true, identity: true
-
       # The display name to be used by user interface programs to identify the
       # service. This string has a maximum length of 256 characters.
       property :display_name, String, regex: /^.{1,256}$/,
                validation_message: "The display_name can only be a maximum of 256 characters!",
                introduced: "14.0"
 
-      # https://github.com/djberg96/win32-service/blob/ffi/lib/win32/windows/constants.rb#L19-L29
+      # https://github.com/chef/win32-service/blob/ffi/lib/win32/windows/constants.rb#L19-L29
       property :desired_access, Integer, default: SERVICE_ALL_ACCESS
 
-      # https://github.com/djberg96/win32-service/blob/ffi/lib/win32/windows/constants.rb#L31-L41
+      # https://github.com/chef/win32-service/blob/ffi/lib/win32/windows/constants.rb#L31-L41
       property :service_type, Integer, default: SERVICE_WIN32_OWN_PROCESS
 
       # Valid options:
       #   - :automatic
       #   - :manual
       #   - :disabled
-      # Reference: https://github.com/djberg96/win32-service/blob/ffi/lib/win32/windows/constants.rb#L49-L54
+      # Reference: https://github.com/chef/win32-service/blob/ffi/lib/win32/windows/constants.rb#L49-L54
       property :startup_type, [Symbol], equal_to: [:automatic, :manual, :disabled], default: :automatic, coerce: proc { |x|
         if x.is_a?(Integer)
           ALLOWED_START_TYPES.invert.fetch(x) do
@@ -90,7 +84,7 @@ class Chef
                  end
                }
 
-      # https://github.com/djberg96/win32-service/blob/ffi/lib/win32/windows/constants.rb#L43-L47
+      # https://github.com/chef/win32-service/blob/ffi/lib/win32/windows/constants.rb#L43-L47
       property :error_control, Integer, default: SERVICE_ERROR_NORMAL
 
       property :binary_path_name, String,
diff --git a/spec/unit/resource/macosx_service.rb b/spec/unit/resource/macosx_service.rb
new file mode 100644
index 0000000000..aebe5e0dd3
--- /dev/null
+++ b/spec/unit/resource/macosx_service.rb
@@ -0,0 +1,37 @@
+#
+# Author:: Tim Smith (tsmith@chef.io>)
+# Copyright:: Copyright 2018, Chef Software Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require "spec_helper"
+
+describe Chef::Resource::MacosxService do
+  let(:resource) { Chef::Resource::MacosxService.new("chef") }
+
+  it "sets the resource_name to :macosx_service" do
+    expect(resource.resource_name).to eql(:macosx_service)
+  end
+
+  it "accepts a String for the session_type property" do
+    resource.session_type "foo"
+    expect(resource.session_type).to eql("foo")
+  end
+
+  it "accepts a String for the plist property" do
+    resource.plist "foo"
+    expect(resource.plist).to eql("foo")
+  end
+end
diff --git a/spec/unit/resource/service_spec.rb b/spec/unit/resource/service_spec.rb
index 963c2b2d89..7c2b46a0dc 100644
--- a/spec/unit/resource/service_spec.rb
+++ b/spec/unit/resource/service_spec.rb
@@ -1,7 +1,7 @@
 #
 # Author:: AJ Christensen (<aj@hjksolutions.com>)
 # Author:: Tyler Cloke (<tyler@chef.io>)
-# Copyright:: Copyright 2008-2017, Chef Software Inc.
+# Copyright:: Copyright 2008-2018, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -26,7 +26,7 @@ describe Chef::Resource::Service do
     expect(resource.provider).to eq(nil)
   end
 
-  it "sets the service_name to the first argument to new" do
+  it "sets the service_name property as the name_property" do
     expect(resource.service_name).to eql("chef")
   end
 
@@ -45,16 +45,17 @@ describe Chef::Resource::Service do
     expect { resource.action :unmask }.not_to raise_error
   end
 
-  it "sets the pattern to be the service name by default" do
-    expect(resource.pattern).to eql("chef")
+  it "Uses the service_name property as the default for the pattern property" do
+    resource.service_name "something"
+    expect(resource.pattern).to eql("something")
   end
 
-  it "accepts a string for the service name" do
+  it "accepts a String for the service name property" do
     resource.service_name "something"
     expect(resource.service_name).to eql("something")
   end
 
-  it "accepts a string for the service pattern" do
+  it "accepts a String for the service pattern" do
     resource.pattern ".*"
     expect(resource.pattern).to eql(".*")
   end
@@ -65,127 +66,119 @@ describe Chef::Resource::Service do
     end.to raise_error(ArgumentError)
   end
 
-  it "accepts a string for the service start command" do
-    resource.start_command "/etc/init.d/chef start"
-    expect(resource.start_command).to eql("/etc/init.d/chef start")
-  end
-
-  it "does not accept a regexp for the service start command" do
-    expect do
-      resource.start_command /.*/
-    end.to raise_error(ArgumentError)
-  end
-
-  it "accepts a string for the service stop command" do
-    resource.stop_command "/etc/init.d/chef stop"
-    expect(resource.stop_command).to eql("/etc/init.d/chef stop")
-  end
-
-  it "does not accept a regexp for the service stop command" do
-    expect do
-      resource.stop_command /.*/
-    end.to raise_error(ArgumentError)
-  end
-
-  it "accepts a string for the service status command" do
-    resource.status_command "/etc/init.d/chef status"
-    expect(resource.status_command).to eql("/etc/init.d/chef status")
-  end
-
-  it "does not accept a regexp for the service status command" do
-    expect do
-      resource.status_command /.*/
-    end.to raise_error(ArgumentError)
-  end
-
-  it "accepts a string for the service restart command" do
-    resource.restart_command "/etc/init.d/chef restart"
-    expect(resource.restart_command).to eql("/etc/init.d/chef restart")
+  it "accepts a String for the user property" do
+    resource.user "fakey_fakerton"
+    expect(resource.user).to eql("fakey_fakerton")
   end
 
-  it "does not accept a regexp for the service restart command" do
-    expect do
-      resource.restart_command /.*/
-    end.to raise_error(ArgumentError)
+  it "accepts an Array for the run_levels property" do
+    resource.run_levels ["foo"]
+    expect(resource.run_levels).to eql(["foo"])
   end
 
-  it "accepts a string for the service reload command" do
-    resource.reload_command "/etc/init.d/chef reload"
-    expect(resource.reload_command).to eql("/etc/init.d/chef reload")
-  end
-
-  it "does not accept a regexp for the service reload command" do
-    expect do
-      resource.reload_command /.*/
-    end.to raise_error(ArgumentError)
+  it "accepts a Hash for the parameters property" do
+    param_hash = { something: nil }
+    resource.parameters param_hash
+    expect(resource.parameters).to eql(param_hash)
   end
 
-  it "accepts a string for the service init command" do
+  it "accepts a String for the init_command property" do
     resource.init_command "/etc/init.d/chef"
     expect(resource.init_command).to eql("/etc/init.d/chef")
   end
 
-  it "does not accept a regexp for the service init command" do
+  it "does not accept a regexp for the init_command property" do
     expect do
       resource.init_command /.*/
     end.to raise_error(ArgumentError)
   end
 
-  it "accepts an array for options" do
+  it "accepts an array for options property" do
     resource.options ["-r", "-s"]
     expect(resource.options).to eql(["-r", "-s"])
   end
 
-  it "accepts a string for options" do
+  it "accepts a String for options property" do
     resource.options "-r"
     expect(resource.options).to eql(["-r"])
   end
 
-  it "accepts a string with multiple flags for options" do
+  it "accepts a String with multiple flags for options property" do
     resource.options "-r -s"
     expect(resource.options).to eql(["-r", "-s"])
   end
 
-  it "does not accept a boolean for options" do
+  it "does not accept a boolean for options property" do
     expect do
       resource.options true
     end.to raise_error(ArgumentError)
   end
 
-  %w{enabled running}.each do |attrib|
-    it "accepts true for #{attrib}" do
-      resource.send(attrib, true)
-      expect(resource.send(attrib)).to eql(true)
+  %w{restart_command start_command stop_command status_command reload_command}.each do |prop|
+    it "accepts a String for the #{prop} property" do
+      resource.send(prop, "service foo bar")
+      expect(resource.send(prop)).to eql("service foo bar")
     end
 
-    it "accepts false for #{attrib}" do
-      resource.send(attrib, false)
-      expect(resource.send(attrib)).to eql(false)
+    it "accepts false for #{prop} property" do
+      resource.send(prop, false)
+      expect(resource.send(prop)).to eql(false)
     end
 
-    it "does not accept a string for #{attrib}" do
-      expect { resource.send(attrib, "poop") }.to raise_error(ArgumentError)
+    it "does not accept a regexp for the #{prop} property" do
+      expect { resource.send(prop, /.*/) }.to raise_error(ArgumentError)
     end
+  end
 
-    it "defaults all the feature support to nil" do
-      support_hash = { status: nil, restart: nil, reload: nil }
-      expect(resource.supports).to eq(support_hash)
+  it "accepts a String for priority property" do
+    resource.priority "1"
+    expect(resource.priority).to eql("1")
+  end
+
+  it "accepts an Integer for priority property" do
+    resource.priority 1
+    expect(resource.priority).to eql(1)
+  end
+
+  it "accepts an Integer for timeout property" do
+    resource.timeout 1
+    expect(resource.timeout).to eql(1)
+  end
+
+  %w{enabled running}.each do |prop|
+    it "accepts true for #{prop} property" do
+      resource.send(prop, true)
+      expect(resource.send(prop)).to eql(true)
     end
 
-    it "allows you to set what features this resource supports as a array" do
-      support_array = [ :status, :restart ]
-      support_hash = { status: true, restart: true }
-      resource.supports(support_array)
-      expect(resource.supports).to eq(support_hash)
+    it "accepts false for #{prop} property" do
+      resource.send(prop, false)
+      expect(resource.send(prop)).to eql(false)
     end
 
-    it "allows you to set what features this resource supports as a hash" do
-      support_hash = { status: true, restart: true }
-      resource.supports(support_hash)
-      expect(resource.supports).to eq(support_hash)
+    it "does not accept a String for #{prop} property" do
+      expect { resource.send(prop, "poop") }.to raise_error(ArgumentError)
     end
   end
 
+  it "defaults all the feature support to nil" do
+    support_hash = { status: nil, restart: nil, reload: nil }
+    expect(resource.supports).to eq(support_hash)
+  end
+
+  it "allows you to set what features this resource supports as an array" do
+    support_array = [ :status, :restart ]
+    support_hash = { status: true, restart: true }
+    resource.supports(support_array)
+    expect(resource.supports).to eq(support_hash)
+  end
+
+  it "allows you to set what features this resource supports as a hash" do
+    support_hash = { status: true, restart: true }
+    resource.supports(support_hash)
+    expect(resource.supports).to eq(support_hash)
+  end
+
   describe "when it has pattern and supports" do
     before do
       resource.service_name("superfriend")
@@ -199,7 +192,7 @@ describe Chef::Resource::Service do
       expect(state[:running]).to eql(false)
     end
 
-    it "returns the service name as its identity" do
+    it "returns the service_name property as its identity" do
       expect(resource.identity).to eq("superfriend")
     end
   end
diff --git a/spec/unit/resource/windows_service_spec.rb b/spec/unit/resource/windows_service_spec.rb
index 8648b52a17..b44a86d04a 100644
--- a/spec/unit/resource/windows_service_spec.rb
+++ b/spec/unit/resource/windows_service_spec.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Bryan McLellan <btm@loftninjas.org>
-# Copyright:: Copyright 2014-2016, Chef Software, Inc.
+# Copyright:: Copyright 2014-2018, Chef Software, Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -19,14 +19,6 @@
 require "spec_helper"
 
 describe Chef::Resource::WindowsService, "initialize" do
-  static_provider_resolution(
-    resource: Chef::Resource::WindowsService,
-    provider: Chef::Provider::Service::Windows,
-    os: "windows",
-    name: :windows_service,
-    action: :start
-  )
-
   let(:resource) { Chef::Resource::WindowsService.new("fakey_fakerton") }
 
   it "sets the resource_name to :windows_service" do
@@ -56,9 +48,32 @@ describe Chef::Resource::WindowsService, "initialize" do
     expect { resource.action :unmask }.not_to raise_error
   end
 
-  it "supports setting startup_type" do
-    resource.startup_type(:manual)
-    expect(resource.startup_type).to eql(:manual)
+  [:automatic, :manual, :disabled].each do |type|
+    it "supports setting startup_type property to #{type.inspect}" do
+      resource.startup_type type
+      expect(resource.startup_type).to eql(type)
+    end
+  end
+
+  { 2 => :automatic, 3 => :manual, 4 => :disabled }.each_pair do |k, v|
+    it "it coerces startup_type property #{k} to #{v.inspect}" do
+      resource.startup_type k
+      expect(resource.startup_type).to eql(v)
+    end
+  end
+
+  %w{automatic manual disabled}.each do |type|
+    it "it coerces startup_type property #{type} to :#{type}" do
+      resource.startup_type type
+      expect(resource.startup_type).to eql(type.to_sym)
+    end
+  end
+
+  [:automatic, :manual, :disabled].each do |type|
+    it "supports setting startup_type property to #{type.inspect}" do
+      resource.startup_type type
+      expect(resource.startup_type).to eql(type)
+    end
   end
 
   it "allows the action to be 'configure_startup'" do
@@ -69,7 +84,7 @@ describe Chef::Resource::WindowsService, "initialize" do
   # Properties that are Strings
   %i{description service_name binary_path_name load_order_group dependencies
      run_as_user run_as_password display_name}.each do |prop|
-    it "support setting #{prop}" do
+    it "support setting #{prop} property with a String" do
       resource.send("#{prop}=", "some value")
       expect(resource.send(prop)).to eq("some value")
     end
@@ -77,7 +92,7 @@ describe Chef::Resource::WindowsService, "initialize" do
 
   # Properties that are Integers
   %i{desired_access error_control service_type}.each do |prop|
-    it "support setting #{prop}" do
+    it "support setting #{prop} property with an Integer" do
       resource.send("#{prop}=", 1)
       expect(resource.send(prop)).to eq(1)
     end