Merge pull request #9420 from chef/remove_macos_10.9_support

Remove support for macOS < 10.12 in the service resource

2 files changed, 215 insertions, 224 deletions

diff --git a/lib/chef/provider/service/macosx.rb b/lib/chef/provider/service/macosx.rb
index 746581df73..2c74cb2629 100644
--- a/lib/chef/provider/service/macosx.rb
+++ b/lib/chef/provider/service/macosx.rb
@@ -1,6 +1,7 @@
 #
 # Author:: Igor Afonov <afonov@gmail.com>
 # Copyright:: Copyright 2011-2016, Igor Afonov
+# Copyright:: Copyright 2020, Chef Software, Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -42,10 +43,6 @@ class Chef
 
         PLIST_DIRS = gather_plist_dirs
 
-        def this_version_or_newer?(this_version)
-          Gem::Version.new(node["platform_version"]) >= Gem::Version.new(this_version)
-        end
-
         def load_current_resource
           @current_resource = Chef::Resource::MacosxService.new(@new_resource.name)
           @current_resource.service_name(@new_resource.service_name)
@@ -59,10 +56,8 @@ class Chef
           if @console_user
             @console_user = Etc.getpwuid(::File.stat("/dev/console").uid).name
             logger.trace("#{new_resource} console_user: '#{@console_user}'")
-            cmd = "su "
-            param = this_version_or_newer?("10.10") ? "" : "-l "
-            param = "-l " if this_version_or_newer?("10.12")
-            @base_user_cmd = cmd + param + "#{@console_user} -c"
+            cmd = "su -l"
+            @base_user_cmd = cmd + "#{@console_user} -c"
             # Default LaunchAgent session should be Aqua
             @session_type = "Aqua" if @session_type.nil?
           end
@@ -140,10 +135,10 @@ class Chef
           end
         end
 
-        # On OS/X, enabling a service has the side-effect of starting it,
+        # On macOS, enabling a service has the side-effect of starting it,
         # and disabling a service has the side-effect of stopping it.
         #
-        # This makes some sense on OS/X since launchctl is an "init"-style
+        # This makes some sense on macOS since launchctl is an "init"-style
         # supervisor that will restart daemons that are crashing, etc.
         def enable_service
           if @current_resource.enabled
diff --git a/spec/unit/provider/service/macosx_spec.rb b/spec/unit/provider/service/macosx_spec.rb
index 420fd329f6..9327d07cdd 100644
--- a/spec/unit/provider/service/macosx_spec.rb
+++ b/spec/unit/provider/service/macosx_spec.rb
@@ -1,6 +1,7 @@
 #
 # Author:: Igor Afonov <afonov@gmail.com>
 # Copyright:: Copyright 2011-2016, Igor Afonov
+# Copyright:: Copyright 2020, Chef Software, Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -61,276 +62,271 @@ describe Chef::Provider::Service::Macosx do
 
     %w{Daemon Agent}.each do |service_type|
       ["redis-server", "io.redis.redis-server"].each do |service_name|
-        ["10.9", "10.10", "10.11"].each do |platform_version|
-          let(:plist) { "/Library/LaunchDaemons/io.redis.redis-server.plist" }
-          let(:session) { StringIO.new }
-          if service_type == "Agent"
-            let(:plist) { "/Library/LaunchAgents/io.redis.redis-server.plist" }
-            let(:session) { "-S Aqua " }
-            let(:su_cmd) { "su -l igor -c" }
-            if platform_version == "10.9"
-              let(:su_cmd) { "su igor -c" }
-            end
-          end
-          let(:service_label) { "io.redis.redis-server" }
-          before do
-            allow(run_context).to receive(:logger).and_return(logger)
-            allow(Dir).to receive(:glob).and_return([plist], [])
-            @stat = double("File::Stat", { uid: 501 })
-            allow(File).to receive(:stat).and_return(@stat)
-            @getpwuid = double("Etc::Passwd", { name: "mikedodge04" })
-            allow(Etc).to receive(:getpwuid).and_return(@getpwuid)
-            allow(node).to receive(:[]).with("platform_version").and_return(platform_version)
-            cmd = "launchctl list #{service_label}"
-            allow(provider).to receive(:shell_out)
-              .with(/(#{su_cmd} '#{cmd}'|#{cmd})/, default_env: false)
-              .and_return(double("Status",
-                stdout: launchctl_stdout, exitstatus: 0))
-            allow(File).to receive(:exists?).and_return([true], [])
-            allow(provider).to receive(:shell_out!)
-              .with(/plutil -convert xml1 -o/, default_env: false)
-              .and_return(double("Status", stdout: plutil_stdout))
-          end
+        let(:plist) { "/Library/LaunchDaemons/io.redis.redis-server.plist" }
+        let(:session) { StringIO.new }
+        if service_type == "Agent"
+          let(:plist) { "/Library/LaunchAgents/io.redis.redis-server.plist" }
+          let(:session) { "-S Aqua " }
+          let(:su_cmd) { "su -l igor -c" }
+        end
+        let(:service_label) { "io.redis.redis-server" }
+        before do
+          allow(run_context).to receive(:logger).and_return(logger)
+          allow(Dir).to receive(:glob).and_return([plist], [])
+          @stat = double("File::Stat", { uid: 501 })
+          allow(File).to receive(:stat).and_return(@stat)
+          @getpwuid = double("Etc::Passwd", { name: "mikedodge04" })
+          allow(Etc).to receive(:getpwuid).and_return(@getpwuid)
+          allow(node).to receive(:[]).with("platform_version").and_return("10.11.1")
+          cmd = "launchctl list #{service_label}"
+          allow(provider).to receive(:shell_out)
+            .with(/(#{su_cmd} '#{cmd}'|#{cmd})/, default_env: false)
+            .and_return(double("Status",
+              stdout: launchctl_stdout, exitstatus: 0))
+          allow(File).to receive(:exists?).and_return([true], [])
+          allow(provider).to receive(:shell_out!)
+            .with(/plutil -convert xml1 -o/, default_env: false)
+            .and_return(double("Status", stdout: plutil_stdout))
+        end
 
-          context "#{service_name} that is a #{service_type} running Osx #{platform_version}" do
-            let(:new_resource) { Chef::Resource::MacosxService.new(service_name) }
-            let!(:current_resource) { Chef::Resource::MacosxService.new(service_name) }
+        context "#{service_name} that is a #{service_type}" do
+          let(:new_resource) { Chef::Resource::MacosxService.new(service_name) }
+          let!(:current_resource) { Chef::Resource::MacosxService.new(service_name) }
 
-            describe "#load_current_resource" do
+          describe "#load_current_resource" do
 
-              # CHEF-5223 "you can't glob for a file that hasn't been converged
-              # onto the node yet."
-              context "when the plist doesn't exist" do
+            # CHEF-5223 "you can't glob for a file that hasn't been converged
+            # onto the node yet."
+            context "when the plist doesn't exist" do
 
-                def run_resource_setup_for_action(action)
-                  new_resource.action(action)
-                  provider.action = action
-                  provider.load_current_resource
-                  provider.define_resource_requirements
-                  provider.process_resource_requirements
-                end
+              def run_resource_setup_for_action(action)
+                new_resource.action(action)
+                provider.action = action
+                provider.load_current_resource
+                provider.define_resource_requirements
+                provider.process_resource_requirements
+              end
 
-                before do
-                  allow(Dir).to receive(:glob).and_return([])
-                  allow(File).to receive(:exists?).and_return([true], [])
-                  allow(provider).to receive(:shell_out!)
-                    .with(/plutil -convert xml1 -o/)
-                    .and_raise(Mixlib::ShellOut::ShellCommandFailed)
-                end
+              before do
+                allow(Dir).to receive(:glob).and_return([])
+                allow(File).to receive(:exists?).and_return([true], [])
+                allow(provider).to receive(:shell_out!)
+                  .with(/plutil -convert xml1 -o/)
+                  .and_raise(Mixlib::ShellOut::ShellCommandFailed)
+              end
 
-                it "works for action :nothing" do
-                  expect { run_resource_setup_for_action(:nothing) }.not_to raise_error
-                end
+              it "works for action :nothing" do
+                expect { run_resource_setup_for_action(:nothing) }.not_to raise_error
+              end
 
-                it "works for action :start" do
-                  expect { run_resource_setup_for_action(:start) }.not_to raise_error
-                end
+              it "works for action :start" do
+                expect { run_resource_setup_for_action(:start) }.not_to raise_error
+              end
 
-                it "errors if action is :enable" do
-                  expect { run_resource_setup_for_action(:enable) }.to raise_error(Chef::Exceptions::Service)
-                end
+              it "errors if action is :enable" do
+                expect { run_resource_setup_for_action(:enable) }.to raise_error(Chef::Exceptions::Service)
+              end
 
-                it "errors if action is :disable" do
-                  expect { run_resource_setup_for_action(:disable) }.to raise_error(Chef::Exceptions::Service)
-                end
+              it "errors if action is :disable" do
+                expect { run_resource_setup_for_action(:disable) }.to raise_error(Chef::Exceptions::Service)
               end
+            end
 
-              context "when launchctl returns pid in service list" do
-                let(:launchctl_stdout) { StringIO.new <<~SVC_LIST }
-                  {
-                    "LimitLoadToSessionType" = "System";
-                    "Label" = "io.redis.redis-server";
-                    "TimeOut" = 30;
-                    "OnDemand" = false;
-                    "LastExitStatus" = 0;
-                    "PID" = 62803;
-                    "Program" = "do_some.sh";
-                    "ProgramArguments" = (
-                      "path/to/do_something.sh";
-                      "-f";
-                    );
-                  };
-                SVC_LIST
+            context "when launchctl returns pid in service list" do
+              let(:launchctl_stdout) { StringIO.new <<~SVC_LIST }
+                {
+                  "LimitLoadToSessionType" = "System";
+                  "Label" = "io.redis.redis-server";
+                  "TimeOut" = 30;
+                  "OnDemand" = false;
+                  "LastExitStatus" = 0;
+                  "PID" = 62803;
+                  "Program" = "do_some.sh";
+                  "ProgramArguments" = (
+                    "path/to/do_something.sh";
+                    "-f";
+                  );
+                };
+              SVC_LIST
 
-                before do
-                  provider.load_current_resource
-                end
+              before do
+                provider.load_current_resource
+              end
 
-                it "sets resource running state to true" do
-                  expect(provider.current_resource.running).to be_truthy
-                end
+              it "sets resource running state to true" do
+                expect(provider.current_resource.running).to be_truthy
+              end
 
-                it "sets resouce enabled state to true" do
-                  expect(provider.current_resource.enabled).to be_truthy
-                end
+              it "sets resouce enabled state to true" do
+                expect(provider.current_resource.enabled).to be_truthy
               end
+            end
 
-              describe "running unsupported actions" do
-                before do
-                  allow(Dir).to receive(:glob).and_return([(plist).to_s], [])
-                  allow(File).to receive(:exists?).and_return([true], [])
-                end
-                it "should throw an exception when reload action is attempted" do
-                  expect { provider.run_action(:reload) }.to raise_error(Chef::Exceptions::UnsupportedAction)
-                end
+            describe "running unsupported actions" do
+              before do
+                allow(Dir).to receive(:glob).and_return([(plist).to_s], [])
+                allow(File).to receive(:exists?).and_return([true], [])
               end
-              context "when launchctl returns empty service pid" do
-                let(:launchctl_stdout) { StringIO.new <<~SVC_LIST }
-                  {
-                    "LimitLoadToSessionType" = "System";
-                    "Label" = "io.redis.redis-server";
-                    "TimeOut" = 30;
-                    "OnDemand" = false;
-                    "LastExitStatus" = 0;
-                    "Program" = "do_some.sh";
-                    "ProgramArguments" = (
-                      "path/to/do_something.sh";
-                      "-f";
-                    );
-                  };
-                SVC_LIST
+              it "should throw an exception when reload action is attempted" do
+                expect { provider.run_action(:reload) }.to raise_error(Chef::Exceptions::UnsupportedAction)
+              end
+            end
+            context "when launchctl returns empty service pid" do
+              let(:launchctl_stdout) { StringIO.new <<~SVC_LIST }
+                {
+                  "LimitLoadToSessionType" = "System";
+                  "Label" = "io.redis.redis-server";
+                  "TimeOut" = 30;
+                  "OnDemand" = false;
+                  "LastExitStatus" = 0;
+                  "Program" = "do_some.sh";
+                  "ProgramArguments" = (
+                    "path/to/do_something.sh";
+                    "-f";
+                  );
+                };
+              SVC_LIST
 
-                before do
-                  provider.load_current_resource
-                end
+              before do
+                provider.load_current_resource
+              end
 
-                it "sets resource running state to false" do
-                  expect(provider.current_resource.running).to be_falsey
-                end
+              it "sets resource running state to false" do
+                expect(provider.current_resource.running).to be_falsey
+              end
 
-                it "sets resouce enabled state to true" do
-                  expect(provider.current_resource.enabled).to be_truthy
-                end
+              it "sets resouce enabled state to true" do
+                expect(provider.current_resource.enabled).to be_truthy
               end
+            end
 
-              context "when launchctl doesn't return service entry at all" do
-                let(:launchctl_stdout) { StringIO.new <<~SVC_LIST }
-                  Could not find service "io.redis.redis-server" in domain for system
-                SVC_LIST
+            context "when launchctl doesn't return service entry at all" do
+              let(:launchctl_stdout) { StringIO.new <<~SVC_LIST }
+                Could not find service "io.redis.redis-server" in domain for system
+              SVC_LIST
 
-                it "sets service running state to false" do
+              it "sets service running state to false" do
+                provider.load_current_resource
+                expect(provider.current_resource.running).to be_falsey
+              end
+
+              context "and plist for service is not available" do
+                before do
+                  allow(Dir).to receive(:glob).and_return([])
                   provider.load_current_resource
-                  expect(provider.current_resource.running).to be_falsey
                 end
 
-                context "and plist for service is not available" do
-                  before do
-                    allow(Dir).to receive(:glob).and_return([])
-                    provider.load_current_resource
-                  end
-
-                  it "sets resouce enabled state to false" do
-                    expect(provider.current_resource.enabled).to be_falsey
-                  end
+                it "sets resouce enabled state to false" do
+                  expect(provider.current_resource.enabled).to be_falsey
                 end
+              end
 
-                context "and plist for service is available" do
-                  before do
-                    allow(Dir).to receive(:glob).and_return([(plist).to_s], [])
-                    provider.load_current_resource
-                  end
+              context "and plist for service is available" do
+                before do
+                  allow(Dir).to receive(:glob).and_return([(plist).to_s], [])
+                  provider.load_current_resource
+                end
 
-                  it "sets resouce enabled state to true" do
-                    expect(provider.current_resource.enabled).to be_truthy
-                  end
+                it "sets resouce enabled state to true" do
+                  expect(provider.current_resource.enabled).to be_truthy
                 end
+              end
 
-                describe "and several plists match service name" do
-                  it "throws exception" do
-                    allow(Dir).to receive(:glob).and_return([(plist).to_s,
-                                                "/Users/wtf/something.plist"])
-                    provider.load_current_resource
-                    provider.define_resource_requirements
-                    expect { provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Service)
-                  end
+              describe "and several plists match service name" do
+                it "throws exception" do
+                  allow(Dir).to receive(:glob).and_return([(plist).to_s,
+                                              "/Users/wtf/something.plist"])
+                  provider.load_current_resource
+                  provider.define_resource_requirements
+                  expect { provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Service)
                 end
               end
             end
-            describe "#start_service" do
-              before do
-                allow(Chef::Resource::MacosxService).to receive(:new).and_return(current_resource)
-                provider.load_current_resource
-                allow(current_resource).to receive(:running).and_return(false)
-              end
+          end
+          describe "#start_service" do
+            before do
+              allow(Chef::Resource::MacosxService).to receive(:new).and_return(current_resource)
+              provider.load_current_resource
+              allow(current_resource).to receive(:running).and_return(false)
+            end
 
-              it "calls the start command if one is specified and service is not running" do
-                allow(new_resource).to receive(:start_command).and_return("cowsay dirty")
+            it "calls the start command if one is specified and service is not running" do
+              allow(new_resource).to receive(:start_command).and_return("cowsay dirty")
 
-                expect(provider).to receive(:shell_out!).with("cowsay dirty", default_env: false)
-                provider.start_service
-              end
+              expect(provider).to receive(:shell_out!).with("cowsay dirty", default_env: false)
+              provider.start_service
+            end
 
-              it "shows warning message if service is already running" do
-                allow(current_resource).to receive(:running).and_return(true)
-                expect(logger).to receive(:trace).with("macosx_service[#{service_name}] already running, not starting")
+            it "shows warning message if service is already running" do
+              allow(current_resource).to receive(:running).and_return(true)
+              expect(logger).to receive(:trace).with("macosx_service[#{service_name}] already running, not starting")
 
-                provider.start_service
-              end
+              provider.start_service
+            end
 
-              it "starts service via launchctl if service found" do
-                cmd = "launchctl load -w " + session + plist
-                expect(provider).to receive(:shell_out)
-                  .with(/(#{su_cmd} .#{cmd}.|#{cmd})/, default_env: false)
-                  .and_return(0)
+            it "starts service via launchctl if service found" do
+              cmd = "launchctl load -w " + session + plist
+              expect(provider).to receive(:shell_out)
+                .with(/(#{su_cmd} .#{cmd}.|#{cmd})/, default_env: false)
+                .and_return(0)
 
-                provider.start_service
-              end
+              provider.start_service
             end
+          end
 
-            describe "#stop_service" do
-              before do
-                allow(Chef::Resource::MacosxService).to receive(:new).and_return(current_resource)
+          describe "#stop_service" do
+            before do
+              allow(Chef::Resource::MacosxService).to receive(:new).and_return(current_resource)
 
-                provider.load_current_resource
-                allow(current_resource).to receive(:running).and_return(true)
-              end
+              provider.load_current_resource
+              allow(current_resource).to receive(:running).and_return(true)
+            end
 
-              it "calls the stop command if one is specified and service is running" do
-                allow(new_resource).to receive(:stop_command).and_return("kill -9 123")
+            it "calls the stop command if one is specified and service is running" do
+              allow(new_resource).to receive(:stop_command).and_return("kill -9 123")
 
-                expect(provider).to receive(:shell_out!).with("kill -9 123", default_env: false)
-                provider.stop_service
-              end
+              expect(provider).to receive(:shell_out!).with("kill -9 123", default_env: false)
+              provider.stop_service
+            end
 
-              it "shows warning message if service is not running" do
-                allow(current_resource).to receive(:running).and_return(false)
-                expect(logger).to receive(:trace).with("macosx_service[#{service_name}] not running, not stopping")
+            it "shows warning message if service is not running" do
+              allow(current_resource).to receive(:running).and_return(false)
+              expect(logger).to receive(:trace).with("macosx_service[#{service_name}] not running, not stopping")
 
-                provider.stop_service
-              end
+              provider.stop_service
+            end
 
-              it "stops the service via launchctl if service found" do
-                cmd = "launchctl unload -w " + plist
-                expect(provider).to receive(:shell_out)
-                  .with(/(#{su_cmd} .#{cmd}.|#{cmd})/, default_env: false)
-                  .and_return(0)
+            it "stops the service via launchctl if service found" do
+              cmd = "launchctl unload -w " + plist
+              expect(provider).to receive(:shell_out)
+                .with(/(#{su_cmd} .#{cmd}.|#{cmd})/, default_env: false)
+                .and_return(0)
 
-                provider.stop_service
-              end
+              provider.stop_service
             end
+          end
 
-            describe "#restart_service" do
-              before do
-                allow(Chef::Resource::Service).to receive(:new).and_return(current_resource)
+          describe "#restart_service" do
+            before do
+              allow(Chef::Resource::Service).to receive(:new).and_return(current_resource)
 
-                provider.load_current_resource
-                allow(current_resource).to receive(:running).and_return(true)
-                allow(provider).to receive(:sleep)
-              end
+              provider.load_current_resource
+              allow(current_resource).to receive(:running).and_return(true)
+              allow(provider).to receive(:sleep)
+            end
 
-              it "issues a command if given" do
-                allow(new_resource).to receive(:restart_command).and_return("reload that thing")
+            it "issues a command if given" do
+              allow(new_resource).to receive(:restart_command).and_return("reload that thing")
 
-                expect(provider).to receive(:shell_out!).with("reload that thing", default_env: false)
-                provider.restart_service
-              end
+              expect(provider).to receive(:shell_out!).with("reload that thing", default_env: false)
+              provider.restart_service
+            end
 
-              it "stops and then starts service" do
-                expect(provider).to receive(:unload_service)
-                expect(provider).to receive(:load_service)
+            it "stops and then starts service" do
+              expect(provider).to receive(:unload_service)
+              expect(provider).to receive(:load_service)
 
-                provider.restart_service
-              end
+              provider.restart_service
             end
           end
         end