From 935be7a56602e5e00fac404bbe55b76913e0405d Mon Sep 17 00:00:00 2001 From: Christopher Thorn Date: Tue, 14 Nov 2023 13:29:45 -0800 Subject: [PATCH] (PE-36365) Puppet resource service should error if service is not present Previously if a service was not present on a system and a user tried to manage that service with `puppet resource service`, and that service wasn't there an error message would appear on the screen but the puppet command would return a 0 exit code. This commit is meant to bubble up those error messages so they will be reflected on the command line properly. First pass will be to update the Windows and Systemd providers. blank --- lib/puppet/.DS_Store | Bin 0 -> 6148 bytes lib/puppet/provider/.DS_Store | Bin 0 -> 8196 bytes lib/puppet/provider/service/windows.rb | 38 +++++++++++---------- lib/puppet/type/service.rb | 4 +++ spec/unit/application/resource_spec.rb | 12 ++++++- spec/unit/provider/service/systemd_spec.rb | 36 ++++++++----------- spec/unit/type/service_spec.rb | 6 ++++ 7 files changed, 56 insertions(+), 40 deletions(-) create mode 100644 lib/puppet/.DS_Store create mode 100644 lib/puppet/provider/.DS_Store diff --git a/lib/puppet/.DS_Store b/lib/puppet/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..096e719b0e184a6b787cc71828ac2c5d08f70969 GIT binary patch literal 6148 zcmeH~F$w}f3`G;&V!>uh%V|7-Hy9Q@ffuk)Y(zy=u$!a%lL>;WwTS#c@+X-I%f4b~ zBO=;gH*=9rL|VA1%q$E{kvDRYyPRZuTb&R4<6(!I)kksG*6>aS`>{w?k8$%b}%eZ5NH!Km@cXo4|nM2s5WRMbQhX<{Jp!Km@cn5c;wN%Y*gvn{an$;6<{P44~X zo^#K+GiSatw`a>3Lq|bxVXT2MCR69A*3fW~#`V0HH7Sr(5Q6L(Gg+1ymRs!1SY@k2 zK^TEB0$~Kg2!s&`BXB81fX-~*Wo)yon-<=F&6}M+5Ez|Xyo6kV1sH|F2Ek~}XQLc=p zhEpTTfibndXc|+!lb-3=?tUZh5{{|0?u0YdoptPed4ubRvW`{A+3uKHmrYyQf|d4+ zcFXEFXAF|ny4k#AS^FH<7+RYlaZrX_B>>Y8X&tXZ%rc}W#A=Dh{eyHAbFaQY`aO0sQRFl z&KQ>PrtG+dM7xKIBItpzEg?thusM}=ayfb)H`qDZCClo`gM9CvwCnXw8ur3nb<6TF z*R<<830F3(sE_tU_sZ&*@`yW;&Ktc`wvjK%`()XFza;0Lx^&ZX=9T-oQZ?Cs!E zcR^5a<#>s7@kFDz^rIVzCz=C|%vQ5jmSjC_oLG22JHn2#W9%*VHao#SVqdVY*f;D4 z_A~p1{mOo0XHkV3EW>K7M9bpuj388iR1VHALArG!Dl#wZ}A)NYcy4LC?%36>?7nn!>-18d zO;=GiEstHdam!Z9v3bG?ik4V;F57*ApO+Rs!F?)R$V@-&lxwv?$^c_}A@4*Umalh&lE5~Zb>HfyblT0uz{@oj6gCPfyPH!C|7 zwUUxgOmEZL6qWJsftYt_NloRcB>zKFf67j=(?seYiPV3g5)m|D4K|>KNZf{Ybf616 ziO6@M4+9v)L5w4fEYUg#2b0Ld#RGVdNc}J#!4pL4BX|Z!@vKknmx$P};22&dVxPb} zco*;CeSC<|aSC%6P`2lM%Hr#C$`7;ru@g={gqZ5&_N% aBn`EH{fB^6;rY+z|6~o%f1iYx;qW(kQ}Bua literal 0 HcmV?d00001 diff --git a/lib/puppet/provider/service/windows.rb b/lib/puppet/provider/service/windows.rb index 4712dd4b0ba..d77a84134cc 100644 --- a/lib/puppet/provider/service/windows.rb +++ b/lib/puppet/provider/service/windows.rb @@ -92,25 +92,27 @@ def stop end def status - return :stopped unless Puppet::Util::Windows::Service.exists?(@resource[:name]) - - current_state = Puppet::Util::Windows::Service.service_state(@resource[:name]) - state = case current_state - when :SERVICE_STOPPED, - :SERVICE_STOP_PENDING - :stopped - when :SERVICE_PAUSED, - :SERVICE_PAUSE_PENDING - :paused - when :SERVICE_RUNNING, - :SERVICE_CONTINUE_PENDING, - :SERVICE_START_PENDING - :running - else - raise Puppet::Error.new(_("Unknown service state '%{current_state}' for service '%{resource_name}'") % { current_state: current_state, resource_name: @resource[:name] }) + if Puppet::Util::Windows::Service.exists?(@resource[:name]) + current_state = Puppet::Util::Windows::Service.service_state(@resource[:name]) + state = case current_state + when :SERVICE_STOPPED, + :SERVICE_STOP_PENDING + :stopped + when :SERVICE_PAUSED, + :SERVICE_PAUSE_PENDING + :paused + when :SERVICE_RUNNING, + :SERVICE_CONTINUE_PENDING, + :SERVICE_START_PENDING + :running + else + raise Puppet::Error.new(_("Unknown service state '%{current_state}' for service '%{resource_name}'") % { current_state: current_state, resource_name: @resource[:name] }) + end + debug("Service #{@resource[:name]} is #{current_state}") + state + else + return :absent end - debug("Service #{@resource[:name]} is #{current_state}") - state rescue => detail Puppet.warning("Status for service #{@resource[:name]} could not be retrieved: #{detail}") :stopped diff --git a/lib/puppet/type/service.rb b/lib/puppet/type/service.rb index fceb66bb863..d4318233ca5 100644 --- a/lib/puppet/type/service.rb +++ b/lib/puppet/type/service.rb @@ -112,6 +112,10 @@ def insync?(current) newvalue(:absent) + validate do |val| + fail "Managing absent on a service is not supported" if val.equal?(:absent) + end + aliasvalue(:false, :stopped) aliasvalue(:true, :running) diff --git a/spec/unit/application/resource_spec.rb b/spec/unit/application/resource_spec.rb index b257696eac8..d85ac1277d7 100644 --- a/spec/unit/application/resource_spec.rb +++ b/spec/unit/application/resource_spec.rb @@ -118,12 +118,19 @@ @resource_app.main end + before :each do + allow(@res).to receive(:ref).and_return("type/name") + end + it "should add given parameters to the object" do allow(@resource_app.command_line).to receive(:args).and_return(['type','name','param=temp']) expect(Puppet::Resource.indirection).to receive(:save).with(@res, 'type/name').and_return([@res, @report]) expect(Puppet::Resource).to receive(:new).with('type', 'name', {:parameters => {'param' => 'temp'}}).and_return(@res) + resource_status = instance_double('Puppet::Resource::Status') + allow(@report).to receive(:resource_statuses).and_return({'type/name' => resource_status}) + allow(resource_status).to receive(:failed?).and_return(false) @resource_app.main end end @@ -140,11 +147,14 @@ def exists? true end + def string=(value) + end + def string Puppet::Util::Execution::ProcessOutput.new('test', 0) end end - + it "should not emit puppet class tags when printing yaml when strict mode is off" do Puppet[:strict] = :warning diff --git a/spec/unit/provider/service/systemd_spec.rb b/spec/unit/provider/service/systemd_spec.rb index 3f5d5c5f377..f947d3648e8 100644 --- a/spec/unit/provider/service/systemd_spec.rb +++ b/spec/unit/provider/service/systemd_spec.rb @@ -388,30 +388,24 @@ # Note: systemd provider does not care about hasstatus or a custom status # command. I just assume that it does not make sense for systemd. describe "#status" do - it "should return running if the command returns 0" do - provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd.service')) - expect(provider).to receive(:execute) - .with(['/bin/systemctl','is-active', '--', 'sshd.service'], {:failonfail => false, :override_locale => false, :squelch => false, :combine => true}) - .and_return(Puppet::Util::Execution::ProcessOutput.new("active\n", 0)) + it 'when called on a service that does not exist returns absent' do + provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'doesnotexist.service')) + expect(provider).to receive(:exist?).and_return(false) + expect(provider.status).to eq(:absent) + end + + it 'when called on a service that does exist and is running returns running' do + provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'doesexist.service')) + expect(provider).to receive(:exist?).and_return(true) + expect(provider).to receive(:service_command).with(:status, false).and_return(Puppet::Util::Execution::ProcessOutput.new("running\n", 0)) expect(provider.status).to eq(:running) end - [-10,-1,3,10].each { |ec| - it "should return stopped if the command returns something non-0" do - provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd.service')) - expect(provider).to receive(:execute) - .with(['/bin/systemctl','is-active', '--', 'sshd.service'], {:failonfail => false, :override_locale => false, :squelch => false, :combine => true}) - .and_return(Puppet::Util::Execution::ProcessOutput.new("inactive\n", ec)) - expect(provider.status).to eq(:stopped) - end - } - - it "should use the supplied status command if specified" do - provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd.service', :status => '/bin/foo')) - expect(provider).to receive(:execute) - .with(['/bin/foo'], {:failonfail => false, :override_locale => false, :squelch => false, :combine => true}) - .and_return(process_output) - provider.status + it 'when called on a service that does exist and is not running returns stopped' do + provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'doesexist.service')) + expect(provider).to receive(:exist?).and_return(true) + expect(provider).to receive(:service_command).with(:status, false).and_return(Puppet::Util::Execution::ProcessOutput.new("stopped\n", 3)) + expect(provider.status).to eq(:stopped) end end diff --git a/spec/unit/type/service_spec.rb b/spec/unit/type/service_spec.rb index 0b55e0997be..e60829ce53a 100644 --- a/spec/unit/type/service_spec.rb +++ b/spec/unit/type/service_spec.rb @@ -67,6 +67,12 @@ def safely_load_service_type expect(svc.should(:ensure)).to eq(:stopped) end + describe 'the absent property' do + it 'should fail validate if it is a service' do + expect { Puppet::Type.type(:service).new(:name => "service_name", :absent => 'sure') }.to raise_error(Puppet::Error) + end + end + describe "the enable property" do before :each do allow(@provider.class).to receive(:supports_parameter?).and_return(true)