diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 6ef6955d307..ac883a54eab 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -646,52 +646,6 @@ Style/OptionalBooleanParameter: Style/PreferredHashMethods: Enabled: false -# This cop supports safe auto-correction (--auto-correct). -# Configuration parameters: EnforcedStyle, AllowedCompactTypes. -# SupportedStyles: compact, exploded -Style/RaiseArgs: - Enabled: false - -# This cop supports safe auto-correction (--auto-correct). -Style/RedundantAssignment: - Exclude: - - 'lib/puppet/file_serving/mount/locales.rb' - - 'lib/puppet/file_serving/mount/pluginfacts.rb' - - 'lib/puppet/file_serving/mount/plugins.rb' - - 'lib/puppet/parameter.rb' - - 'lib/puppet/parser/ast/pops_bridge.rb' - - 'lib/puppet/pops/model/factory.rb' - - 'lib/puppet/pops/parser/parser_support.rb' - - 'lib/puppet/provider/nameservice/directoryservice.rb' - - 'lib/puppet/provider/nameservice/objectadd.rb' - - 'lib/puppet/provider/nameservice/pw.rb' - - 'lib/puppet/settings.rb' - - 'lib/puppet/type.rb' - - 'lib/puppet/type/file/ensure.rb' - - 'lib/puppet/util.rb' - - 'lib/puppet/util/execution.rb' - -# This cop supports safe auto-correction (--auto-correct). -Style/RedundantBegin: - Enabled: false - -# This cop supports safe auto-correction (--auto-correct). -Style/RedundantCondition: - Exclude: - - 'ext/windows/service/daemon.rb' - - 'lib/puppet/application/describe.rb' - - 'lib/puppet/environments.rb' - - 'lib/puppet/external/dot.rb' - - 'lib/puppet/graph/simple_graph.rb' - - 'lib/puppet/indirector/request.rb' - - 'lib/puppet/parser/templatewrapper.rb' - - 'lib/puppet/provider/package/blastwave.rb' - - 'lib/puppet/provider/package/pkgutil.rb' - - 'lib/puppet/provider/package/portage.rb' - - 'lib/puppet/type.rb' - - 'lib/puppet/util/command_line/trollop.rb' - - 'lib/puppet/util/inifile.rb' - # This cop supports safe auto-correction (--auto-correct). Style/RedundantConditional: Exclude: diff --git a/ext/windows/service/daemon.rb b/ext/windows/service/daemon.rb index daba16133d9..1bb2be1bb78 100755 --- a/ext/windows/service/daemon.rb +++ b/ext/windows/service/daemon.rb @@ -75,25 +75,23 @@ def service_main(*argsv) service = self @run_thread = Thread.new do - begin - while service.running? do - runinterval = service.parse_runinterval(ruby_puppet_cmd) - - if service.state == RUNNING or service.state == IDLE - service.log_notice("Executing agent with arguments: #{args}") - pid = Process.create(:command_line => "#{ruby_puppet_cmd} agent --onetime #{args}", :creation_flags => CREATE_NEW_CONSOLE).process_id - service.log_debug("Process created: #{pid}") - else - service.log_debug("Service is paused. Not invoking Puppet agent") - end - - service.log_debug("Service worker thread waiting for #{runinterval} seconds") - sleep(runinterval) - service.log_debug('Service worker thread woken up') + while service.running? do + runinterval = service.parse_runinterval(ruby_puppet_cmd) + + if service.state == RUNNING or service.state == IDLE + service.log_notice("Executing agent with arguments: #{args}") + pid = Process.create(:command_line => "#{ruby_puppet_cmd} agent --onetime #{args}", :creation_flags => CREATE_NEW_CONSOLE).process_id + service.log_debug("Process created: #{pid}") + else + service.log_debug("Service is paused. Not invoking Puppet agent") end - rescue Exception => e - service.log_exception(e) + + service.log_debug("Service worker thread waiting for #{runinterval} seconds") + sleep(runinterval) + service.log_debug('Service worker thread woken up') end + rescue Exception => e + service.log_exception(e) end @run_thread.join rescue Exception => e @@ -142,20 +140,18 @@ def log(msg, level) end def report_windows_event(type, id, message) - begin - eventlog = nil - eventlog = Puppet::Util::Windows::EventLog.open("Puppet") - eventlog.report_event( - :event_type => type, # EVENTLOG_ERROR_TYPE, etc - :event_id => id, # 0x01 or 0x02, 0x03 etc. - :data => message # "the message" - ) - rescue Exception - # Ignore all errors - ensure - unless eventlog.nil? - eventlog.close - end + eventlog = nil + eventlog = Puppet::Util::Windows::EventLog.open("Puppet") + eventlog.report_event( + :event_type => type, # EVENTLOG_ERROR_TYPE, etc + :event_id => id, # 0x01 or 0x02, 0x03 etc. + :data => message # "the message" + ) + rescue Exception + # Ignore all errors + ensure + unless eventlog.nil? + eventlog.close end end @@ -186,30 +182,28 @@ def parse_log_level(puppet_path, cmdline_debug) loglevel = :notice end - LEVELS.index(cmdline_debug ? cmdline_debug : loglevel.to_sym) + LEVELS.index(cmdline_debug || loglevel.to_sym) end private def load_env(base_dir) - begin - # ENV that uses backward slashes - ENV['FACTER_env_windows_installdir'] = base_dir.tr('/', '\\') - ENV['PL_BASEDIR'] = base_dir.tr('/', '\\') - ENV['PUPPET_DIR'] = File.join(base_dir, 'puppet').tr('/', '\\') - ENV['OPENSSL_CONF'] = File.join(base_dir, 'puppet', 'ssl', 'openssl.cnf').tr('/', '\\') - ENV['SSL_CERT_DIR'] = File.join(base_dir, 'puppet', 'ssl', 'certs').tr('/', '\\') - ENV['SSL_CERT_FILE'] = File.join(base_dir, 'puppet', 'ssl', 'cert.pem').tr('/', '\\') - ENV['Path'] = [ - File.join(base_dir, 'puppet', 'bin'), - File.join(base_dir, 'bin'), - ].join(';').tr('/', '\\') + ';' + ENV.fetch('Path', nil) - - # ENV that uses forward slashes - ENV['RUBYLIB'] = "#{File.join(base_dir, 'puppet', 'lib')};#{ENV.fetch('RUBYLIB', nil)}" - rescue => e - log_exception(e) - end + # ENV that uses backward slashes + ENV['FACTER_env_windows_installdir'] = base_dir.tr('/', '\\') + ENV['PL_BASEDIR'] = base_dir.tr('/', '\\') + ENV['PUPPET_DIR'] = File.join(base_dir, 'puppet').tr('/', '\\') + ENV['OPENSSL_CONF'] = File.join(base_dir, 'puppet', 'ssl', 'openssl.cnf').tr('/', '\\') + ENV['SSL_CERT_DIR'] = File.join(base_dir, 'puppet', 'ssl', 'certs').tr('/', '\\') + ENV['SSL_CERT_FILE'] = File.join(base_dir, 'puppet', 'ssl', 'cert.pem').tr('/', '\\') + ENV['Path'] = [ + File.join(base_dir, 'puppet', 'bin'), + File.join(base_dir, 'bin'), + ].join(';').tr('/', '\\') + ';' + ENV.fetch('Path', nil) + + # ENV that uses forward slashes + ENV['RUBYLIB'] = "#{File.join(base_dir, 'puppet', 'lib')};#{ENV.fetch('RUBYLIB', nil)}" + rescue => e + log_exception(e) end end diff --git a/lib/puppet/application.rb b/lib/puppet/application.rb index 96edb1ae897..afa51f4e9cb 100644 --- a/lib/puppet/application.rb +++ b/lib/puppet/application.rb @@ -251,7 +251,7 @@ def find(application_name) ################################################################ if clazz.nil? - raise Puppet::Error.new(_("Unable to load application class '%{class_name}' from file 'puppet/application/%{application_name}.rb'") % { class_name: class_name, application_name: application_name }) + raise Puppet::Error, _("Unable to load application class '%{class_name}' from file 'puppet/application/%{application_name}.rb'") % { class_name: class_name, application_name: application_name } end return clazz @@ -481,12 +481,10 @@ def handle_logdest_arg(arg) Puppet[:logdest] = arg logdest.each do |dest| - begin - Puppet::Util::Log.newdestination(dest) - options[:setdest] = true - rescue => detail - Puppet.log_and_raise(detail, _("Could not set logdest to %{dest}.") % { dest: arg }) - end + Puppet::Util::Log.newdestination(dest) + options[:setdest] = true + rescue => detail + Puppet.log_and_raise(detail, _("Could not set logdest to %{dest}.") % { dest: arg }) end end diff --git a/lib/puppet/application/describe.rb b/lib/puppet/application/describe.rb index 332901d29a0..95b2fe0cbbd 100644 --- a/lib/puppet/application/describe.rb +++ b/lib/puppet/application/describe.rb @@ -11,7 +11,7 @@ def wrap(txt, opts) return "" unless txt && !txt.empty? work = (opts[:scrub] ? scrub(txt) : txt) - indent = (opts[:indent] ? opts[:indent] : 0) + indent = (opts[:indent] || 0) textLen = @width - indent patt = Regexp.new("\\A(.{0,#{textLen}})[ \n]") prefix = " " * indent diff --git a/lib/puppet/application/device.rb b/lib/puppet/application/device.rb index 6b290e952d9..bdae357f901 100644 --- a/lib/puppet/application/device.rb +++ b/lib/puppet/application/device.rb @@ -263,115 +263,114 @@ def main end devices.collect do |_devicename, device| # TODO when we drop support for ruby < 2.5 we can remove the extra block here - begin - device_url = URI.parse(device.url) - # Handle nil scheme & port - scheme = "#{device_url.scheme}://" if device_url.scheme - port = ":#{device_url.port}" if device_url.port - - # override local $vardir and $certname - Puppet[:ssldir] = ::File.join(Puppet[:deviceconfdir], device.name, 'ssl') - Puppet[:confdir] = ::File.join(Puppet[:devicedir], device.name) - Puppet[:libdir] = options[:libdir] || ::File.join(Puppet[:devicedir], device.name, 'lib') - Puppet[:vardir] = ::File.join(Puppet[:devicedir], device.name) - Puppet[:certname] = device.name - ssl_context = nil - - # create device directory under $deviceconfdir - Puppet::FileSystem.dir_mkpath(Puppet[:ssldir]) unless Puppet::FileSystem.dir_exist?(Puppet[:ssldir]) - - # this will reload and recompute default settings and create device-specific sub vardir - Puppet.settings.use :main, :agent, :ssl - - # Workaround for PUP-8736: store ssl certs outside the cache directory to prevent accidental removal and keep the old path as symlink - optssldir = File.join(Puppet[:confdir], 'ssl') - Puppet::FileSystem.symlink(Puppet[:ssldir], optssldir) unless Puppet::FileSystem.exist?(optssldir) - - unless options[:resource] || options[:facts] || options[:apply] - # Since it's too complicated to fix properly in the default settings, we workaround for PUP-9642 here. - # See https://github.com/puppetlabs/puppet/pull/7483#issuecomment-483455997 for details. - # This has to happen after `settings.use` above, so the directory is created and before `setup_host` below, where the SSL - # routines would fail with access errors - if Puppet.features.root? && !Puppet::Util::Platform.windows? - user = Puppet::Type.type(:user).new(name: Puppet[:user]).exists? ? Puppet[:user] : nil - group = Puppet::Type.type(:group).new(name: Puppet[:group]).exists? ? Puppet[:group] : nil - Puppet.debug("Fixing perms for #{user}:#{group} on #{Puppet[:confdir]}") - FileUtils.chown(user, group, Puppet[:confdir]) if user || group - end - ssl_context = setup_context + device_url = URI.parse(device.url) + # Handle nil scheme & port + scheme = "#{device_url.scheme}://" if device_url.scheme + port = ":#{device_url.port}" if device_url.port + + # override local $vardir and $certname + Puppet[:ssldir] = ::File.join(Puppet[:deviceconfdir], device.name, 'ssl') + Puppet[:confdir] = ::File.join(Puppet[:devicedir], device.name) + Puppet[:libdir] = options[:libdir] || ::File.join(Puppet[:devicedir], device.name, 'lib') + Puppet[:vardir] = ::File.join(Puppet[:devicedir], device.name) + Puppet[:certname] = device.name + ssl_context = nil + + # create device directory under $deviceconfdir + Puppet::FileSystem.dir_mkpath(Puppet[:ssldir]) unless Puppet::FileSystem.dir_exist?(Puppet[:ssldir]) + + # this will reload and recompute default settings and create device-specific sub vardir + Puppet.settings.use :main, :agent, :ssl + + # Workaround for PUP-8736: store ssl certs outside the cache directory to prevent accidental removal and keep the old path as symlink + optssldir = File.join(Puppet[:confdir], 'ssl') + Puppet::FileSystem.symlink(Puppet[:ssldir], optssldir) unless Puppet::FileSystem.exist?(optssldir) + + unless options[:resource] || options[:facts] || options[:apply] + # Since it's too complicated to fix properly in the default settings, we workaround for PUP-9642 here. + # See https://github.com/puppetlabs/puppet/pull/7483#issuecomment-483455997 for details. + # This has to happen after `settings.use` above, so the directory is created and before `setup_host` below, where the SSL + # routines would fail with access errors + if Puppet.features.root? && !Puppet::Util::Platform.windows? + user = Puppet::Type.type(:user).new(name: Puppet[:user]).exists? ? Puppet[:user] : nil + group = Puppet::Type.type(:group).new(name: Puppet[:group]).exists? ? Puppet[:group] : nil + Puppet.debug("Fixing perms for #{user}:#{group} on #{Puppet[:confdir]}") + FileUtils.chown(user, group, Puppet[:confdir]) if user || group + end + + ssl_context = setup_context - unless options[:libdir] - Puppet.override(ssl_context: ssl_context) do - Puppet::Configurer::PluginHandler.new.download_plugins(env) if Puppet::Configurer.should_pluginsync? - end + unless options[:libdir] + Puppet.override(ssl_context: ssl_context) do + Puppet::Configurer::PluginHandler.new.download_plugins(env) if Puppet::Configurer.should_pluginsync? end end + end - # this inits the device singleton, so that the facts terminus - # and the various network_device provider can use it - Puppet::Util::NetworkDevice.init(device) - - if options[:resource] - type, name = parse_args(command_line.args) - Puppet.info _("retrieving resource: %{resource} from %{target} at %{scheme}%{url_host}%{port}%{url_path}") % { resource: type, target: device.name, scheme: scheme, url_host: device_url.host, port: port, url_path: device_url.path } - resources = find_resources(type, name) - if options[:to_yaml] - data = resources.map do |resource| - resource.prune_parameters(:parameters_to_include => @extra_params).to_hiera_hash - end.inject(:merge!) - text = YAML.dump(type.downcase => data) - else - text = resources.map do |resource| - resource.prune_parameters(:parameters_to_include => @extra_params).to_manifest.force_encoding(Encoding.default_external) - end.join("\n") - end - (puts text) - 0 - elsif options[:facts] - Puppet.info _("retrieving facts from %{target} at %{scheme}%{url_host}%{port}%{url_path}") % { resource: type, target: device.name, scheme: scheme, url_host: device_url.host, port: port, url_path: device_url.path } - remote_facts = Puppet::Node::Facts.indirection.find(name, :environment => env) - # Give a proper name to the facts - remote_facts.name = remote_facts.values['clientcert'] - renderer = Puppet::Network::FormatHandler.format(:console) - puts renderer.render(remote_facts) - 0 - elsif options[:apply] - # avoid reporting to server - Puppet::Transaction::Report.indirection.terminus_class = :yaml - Puppet::Resource::Catalog.indirection.cache_class = nil - - require_relative '../../puppet/application/apply' - begin - Puppet[:node_terminus] = :plain - Puppet[:catalog_terminus] = :compiler - Puppet[:catalog_cache_terminus] = nil - Puppet[:facts_terminus] = :network_device - Puppet.override(:network_device => true) do - Puppet::Application::Apply.new(Puppet::Util::CommandLine.new('puppet', ["apply", options[:apply]])).run_command - end - end + # this inits the device singleton, so that the facts terminus + # and the various network_device provider can use it + Puppet::Util::NetworkDevice.init(device) + + if options[:resource] + type, name = parse_args(command_line.args) + Puppet.info _("retrieving resource: %{resource} from %{target} at %{scheme}%{url_host}%{port}%{url_path}") % { resource: type, target: device.name, scheme: scheme, url_host: device_url.host, port: port, url_path: device_url.path } + resources = find_resources(type, name) + if options[:to_yaml] + data = resources.map do |resource| + resource.prune_parameters(:parameters_to_include => @extra_params).to_hiera_hash + end.inject(:merge!) + text = YAML.dump(type.downcase => data) else - Puppet.info _("starting applying configuration to %{target} at %{scheme}%{url_host}%{port}%{url_path}") % { target: device.name, scheme: scheme, url_host: device_url.host, port: port, url_path: device_url.path } - - overrides = {} - overrides[:ssl_context] = ssl_context if ssl_context - Puppet.override(overrides) do - configurer = Puppet::Configurer.new - configurer.run(:network_device => true, :pluginsync => false) + text = resources.map do |resource| + resource.prune_parameters(:parameters_to_include => @extra_params).to_manifest.force_encoding(Encoding.default_external) + end.join("\n") + end + (puts text) + 0 + elsif options[:facts] + Puppet.info _("retrieving facts from %{target} at %{scheme}%{url_host}%{port}%{url_path}") % { resource: type, target: device.name, scheme: scheme, url_host: device_url.host, port: port, url_path: device_url.path } + remote_facts = Puppet::Node::Facts.indirection.find(name, :environment => env) + # Give a proper name to the facts + remote_facts.name = remote_facts.values['clientcert'] + renderer = Puppet::Network::FormatHandler.format(:console) + puts renderer.render(remote_facts) + 0 + elsif options[:apply] + # avoid reporting to server + Puppet::Transaction::Report.indirection.terminus_class = :yaml + Puppet::Resource::Catalog.indirection.cache_class = nil + + require_relative '../../puppet/application/apply' + begin + Puppet[:node_terminus] = :plain + Puppet[:catalog_terminus] = :compiler + Puppet[:catalog_cache_terminus] = nil + Puppet[:facts_terminus] = :network_device + Puppet.override(:network_device => true) do + Puppet::Application::Apply.new(Puppet::Util::CommandLine.new('puppet', ["apply", options[:apply]])).run_command end end - rescue => detail - Puppet.log_exception(detail) - # If we rescued an error, then we return 1 as the exit code - 1 - ensure - Puppet[:libdir] = libdir - Puppet[:vardir] = vardir - Puppet[:confdir] = confdir - Puppet[:ssldir] = ssldir - Puppet[:certname] = certname + else + Puppet.info _("starting applying configuration to %{target} at %{scheme}%{url_host}%{port}%{url_path}") % { target: device.name, scheme: scheme, url_host: device_url.host, port: port, url_path: device_url.path } + + overrides = {} + overrides[:ssl_context] = ssl_context if ssl_context + Puppet.override(overrides) do + configurer = Puppet::Configurer.new + configurer.run(:network_device => true, :pluginsync => false) + end end + rescue => detail + Puppet.log_exception(detail) + # If we rescued an error, then we return 1 as the exit code + 1 + ensure + Puppet[:libdir] = libdir + Puppet[:vardir] = vardir + Puppet[:confdir] = confdir + Puppet[:ssldir] = ssldir + Puppet[:certname] = certname end end diff --git a/lib/puppet/application/face_base.rb b/lib/puppet/application/face_base.rb index ef6eeb65096..eeb3a633efe 100644 --- a/lib/puppet/application/face_base.rb +++ b/lib/puppet/application/face_base.rb @@ -105,7 +105,7 @@ def parse_options if option index += 1 if option[:argument] and !(option[:optional]) else - raise OptionParser::InvalidOption.new(item.sub(/=.*$/, '')) + raise OptionParser::InvalidOption, item.sub(/=.*$/, '') end end end diff --git a/lib/puppet/application/ssl.rb b/lib/puppet/application/ssl.rb index c25bc2a62ea..4434163bac5 100644 --- a/lib/puppet/application/ssl.rb +++ b/lib/puppet/application/ssl.rb @@ -192,7 +192,7 @@ def submit_request(ssl_context) Puppet.notice _("Submitted certificate request for '%{name}' to %{url}") % { name: Puppet[:certname], url: route.url } rescue Puppet::HTTP::ResponseError => e if e.response.code == 400 - raise Puppet::Error.new(_("Could not submit certificate request for '%{name}' to %{url} due to a conflict on the server") % { name: Puppet[:certname], url: route.url }) + raise Puppet::Error, _("Could not submit certificate request for '%{name}' to %{url} due to a conflict on the server") % { name: Puppet[:certname], url: route.url } else raise Puppet::Error.new(_("Failed to submit certificate request: %{message}") % { message: e.message }, e) end diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb index f2d2ce54a9c..dc340305be5 100644 --- a/lib/puppet/configurer.rb +++ b/lib/puppet/configurer.rb @@ -536,7 +536,7 @@ def valid_server_environment? rescue Puppet::HTTP::ResponseError => detail if detail.response.code == 404 if Puppet[:strict_environment_mode] - raise Puppet::Error.new(_("Environment '%{environment}' not found on server, aborting run.") % { environment: @environment }) + raise Puppet::Error, _("Environment '%{environment}' not found on server, aborting run.") % { environment: @environment } else Puppet.notice(_("Environment '%{environment}' not found on server, skipping initial pluginsync.") % { environment: @environment }) end @@ -748,14 +748,12 @@ def retrieve_new_catalog(facts, query_options) end def download_plugins(remote_environment_for_plugins) - begin - @handler.download_plugins(remote_environment_for_plugins) - rescue Puppet::Error => detail - if !Puppet[:ignore_plugin_errors] && Puppet[:usecacheonfailure] - @running_failure = true - else - raise detail - end + @handler.download_plugins(remote_environment_for_plugins) + rescue Puppet::Error => detail + if !Puppet[:ignore_plugin_errors] && Puppet[:usecacheonfailure] + @running_failure = true + else + raise detail end end end diff --git a/lib/puppet/configurer/downloader.rb b/lib/puppet/configurer/downloader.rb index eb46e3c7924..52928f8eccd 100644 --- a/lib/puppet/configurer/downloader.rb +++ b/lib/puppet/configurer/downloader.rb @@ -20,7 +20,7 @@ def evaluate if first_failure event = (first_failure.events || []).first detail = event ? event.message : 'unknown' - raise Puppet::Error.new(_("Failed to retrieve %{name}: %{detail}") % { name: name, detail: detail }) + raise Puppet::Error, _("Failed to retrieve %{name}: %{detail}") % { name: name, detail: detail } end end diff --git a/lib/puppet/configurer/fact_handler.rb b/lib/puppet/configurer/fact_handler.rb index eb5e449a2b9..64f8fb9da9f 100644 --- a/lib/puppet/configurer/fact_handler.rb +++ b/lib/puppet/configurer/fact_handler.rb @@ -13,20 +13,19 @@ def find_facts # This works because puppet agent configures Facts to use 'facter' for # finding facts and the 'rest' terminus for caching them. Thus, we'll # compile them and then "cache" them on the server. - begin - facts = Puppet::Node::Facts.indirection.find(Puppet[:node_name_value], :environment => Puppet::Node::Environment.remote(@environment)) - unless Puppet[:node_name_fact].empty? - Puppet[:node_name_value] = facts.values[Puppet[:node_name_fact]] - facts.name = Puppet[:node_name_value] - end - facts - rescue SystemExit, NoMemoryError - raise - rescue Exception => detail - message = _("Could not retrieve local facts: %{detail}") % { detail: detail } - Puppet.log_exception(detail, message) - raise Puppet::Error, message, detail.backtrace + + facts = Puppet::Node::Facts.indirection.find(Puppet[:node_name_value], :environment => Puppet::Node::Environment.remote(@environment)) + unless Puppet[:node_name_fact].empty? + Puppet[:node_name_value] = facts.values[Puppet[:node_name_fact]] + facts.name = Puppet[:node_name_value] end + facts + rescue SystemExit, NoMemoryError + raise + rescue Exception => detail + message = _("Could not retrieve local facts: %{detail}") % { detail: detail } + Puppet.log_exception(detail, message) + raise Puppet::Error, message, detail.backtrace end def facts_for_uploading diff --git a/lib/puppet/datatypes.rb b/lib/puppet/datatypes.rb index 54f5163f83a..158c5a3771a 100644 --- a/lib/puppet/datatypes.rb +++ b/lib/puppet/datatypes.rb @@ -128,12 +128,11 @@ def self.create_type(type_name, &block) # and it will fail unless protected with an if defined? if the local # variable does not exist in the block's binder. # - begin - loader = block.binding.eval('loader_injected_arg if defined?(loader_injected_arg)') - create_loaded_type(type_name, loader, &block) - rescue StandardError => e - raise ArgumentError, _("Data Type Load Error for type '%{type_name}': %{message}") % { type_name: type_name, message: e.message } - end + + loader = block.binding.eval('loader_injected_arg if defined?(loader_injected_arg)') + create_loaded_type(type_name, loader, &block) + rescue StandardError => e + raise ArgumentError, _("Data Type Load Error for type '%{type_name}': %{message}") % { type_name: type_name, message: e.message } end def self.create_loaded_type(type_name, loader, &block) diff --git a/lib/puppet/environments.rb b/lib/puppet/environments.rb index 5e5989cd21f..a3c4d829d66 100644 --- a/lib/puppet/environments.rb +++ b/lib/puppet/environments.rb @@ -35,11 +35,7 @@ module EnvironmentLoader # @!macro loader_get_or_fail def get!(name) environment = get(name) - if environment - environment - else - raise EnvironmentNotFound, name - end + environment || raise(EnvironmentNotFound, name) end def clear_all diff --git a/lib/puppet/external/dot.rb b/lib/puppet/external/dot.rb index d804bb3c467..ac2cc255921 100644 --- a/lib/puppet/external/dot.rb +++ b/lib/puppet/external/dot.rb @@ -119,7 +119,7 @@ class DOTSimpleElement attr_accessor :name def initialize(params = {}) - @label = params['name'] ? params['name'] : '' + @label = params['name'] || '' end def to_s @@ -135,8 +135,8 @@ class DOTElement < DOTSimpleElement def initialize(params = {}, option_list = []) super(params) - @name = params['name'] ? params['name'] : nil - @parent = params['parent'] ? params['parent'] : nil + @name = params['name'] || nil + @parent = params['parent'] || nil @options = {} option_list.each { |i| @options[i] = params[i] if params[i] @@ -161,7 +161,7 @@ class DOTPort < DOTSimpleElement def initialize(params = {}) super(params) - @name = params['label'] ? params['label'] : '' + @name = params['label'] || '' end def to_s @@ -174,7 +174,7 @@ def to_s class DOTNode < DOTElement def initialize(params = {}, option_list = NODE_OPTS) super(params, option_list) - @ports = params['ports'] ? params['ports'] : [] + @ports = params['ports'] || [] end def each_port @@ -233,7 +233,7 @@ def stringify(s) class DOTSubgraph < DOTElement def initialize(params = {}, option_list = GRAPH_OPTS) super(params, option_list) - @nodes = params['nodes'] ? params['nodes'] : [] + @nodes = params['nodes'] || [] @dot_string = 'graph' end @@ -287,8 +287,8 @@ class DOTEdge < DOTElement def initialize(params = {}, option_list = EDGE_OPTS) super(params, option_list) - @from = params['from'] ? params['from'] : nil - @to = params['to'] ? params['to'] : nil + @from = params['from'] || nil + @to = params['to'] || nil end def edge_link diff --git a/lib/puppet/face/epp.rb b/lib/puppet/face/epp.rb index d1911187193..4508ca4112a 100644 --- a/lib/puppet/face/epp.rb +++ b/lib/puppet/face/epp.rb @@ -350,12 +350,10 @@ show_filename = args.count > 1 file_nbr = 0 args.each do |file| - begin - buffer.print render_file(file, compiler, options, show_filename, file_nbr += 1) - rescue Puppet::ParseError => detail - Puppet.err(detail.message) - status = false - end + buffer.print render_file(file, compiler, options, show_filename, file_nbr += 1) + rescue Puppet::ParseError => detail + Puppet.err(detail.message) + status = false end end raise Puppet::Error, _("error while rendering epp") unless status diff --git a/lib/puppet/face/parser.rb b/lib/puppet/face/parser.rb index e9005848e1b..59e7bd8a081 100644 --- a/lib/puppet/face/parser.rb +++ b/lib/puppet/face/parser.rb @@ -214,13 +214,11 @@ def validate_manifest(manifest = nil) loaders = Puppet::Pops::Loaders.new(env) Puppet.override({ :loaders => loaders }, _('For puppet parser validate')) do - begin - validation_environment = manifest ? env.override_with(:manifest => manifest) : env - validation_environment.check_for_reparse - validation_environment.known_resource_types.clear - rescue Puppet::ParseError => parse_error - return parse_error - end + validation_environment = manifest ? env.override_with(:manifest => manifest) : env + validation_environment.check_for_reparse + validation_environment.known_resource_types.clear + rescue Puppet::ParseError => parse_error + return parse_error end nil diff --git a/lib/puppet/face/report.rb b/lib/puppet/face/report.rb index 19cfef5710d..5ed1fc44b54 100644 --- a/lib/puppet/face/report.rb +++ b/lib/puppet/face/report.rb @@ -41,13 +41,11 @@ return report EOT when_invoked do |report, _options| - begin - Puppet::Transaction::Report.indirection.terminus_class = :rest - Puppet::Face[:report, "0.0.1"].save(report) - Puppet.notice _("Uploaded report for %{name}") % { name: report.name } - rescue => detail - Puppet.log_exception(detail, _("Could not send report: %{detail}") % { detail: detail }) - end + Puppet::Transaction::Report.indirection.terminus_class = :rest + Puppet::Face[:report, "0.0.1"].save(report) + Puppet.notice _("Uploaded report for %{name}") % { name: report.name } + rescue => detail + Puppet.log_exception(detail, _("Could not send report: %{detail}") % { detail: detail }) end end deactivate_action(:find) diff --git a/lib/puppet/feature/base.rb b/lib/puppet/feature/base.rb index f75a81dcedc..b7efedb291f 100644 --- a/lib/puppet/feature/base.rb +++ b/lib/puppet/feature/base.rb @@ -59,15 +59,13 @@ module WindowsSymlink extend FFI::Library def self.is_implemented # rubocop:disable Naming/PredicateName - begin - ffi_lib :kernel32 - attach_function :CreateSymbolicLinkW, [:lpwstr, :lpwstr, :dword], :boolean - - true - rescue LoadError - Puppet.debug { "CreateSymbolicLink is not available" } - false - end + ffi_lib :kernel32 + attach_function :CreateSymbolicLinkW, [:lpwstr, :lpwstr, :dword], :boolean + + true + rescue LoadError + Puppet.debug { "CreateSymbolicLink is not available" } + false end end diff --git a/lib/puppet/feature/telnet.rb b/lib/puppet/feature/telnet.rb index 71add4df6d6..cd6aceeb60a 100644 --- a/lib/puppet/feature/telnet.rb +++ b/lib/puppet/feature/telnet.rb @@ -3,9 +3,7 @@ require_relative '../../puppet/util/feature' Puppet.features.add :telnet do - begin - require 'net/telnet' - rescue LoadError - false - end + require 'net/telnet' +rescue LoadError + false end diff --git a/lib/puppet/file_bucket/file.rb b/lib/puppet/file_bucket/file.rb index 02eba72bf97..55639ac6bfe 100644 --- a/lib/puppet/file_bucket/file.rb +++ b/lib/puppet/file_bucket/file.rb @@ -26,12 +26,12 @@ def initialize(contents, options = {}) when Pathname @contents = FileContents.new(contents) else - raise ArgumentError.new(_("contents must be a String or Pathname, got a %{contents_class}") % { contents_class: contents.class }) + raise ArgumentError, _("contents must be a String or Pathname, got a %{contents_class}") % { contents_class: contents.class } end @bucket_path = options.delete(:bucket_path) @checksum_type = Puppet[:digest_algorithm].to_sym - raise ArgumentError.new(_("Unknown option(s): %{opts}") % { opts: options.keys.join(', ') }) unless options.empty? + raise ArgumentError, _("Unknown option(s): %{opts}") % { opts: options.keys.join(', ') } unless options.empty? end # @return [Num] The size of the contents diff --git a/lib/puppet/file_serving/base.rb b/lib/puppet/file_serving/base.rb index 6af121b9ed1..d2713a9f25e 100644 --- a/lib/puppet/file_serving/base.rb +++ b/lib/puppet/file_serving/base.rb @@ -59,7 +59,7 @@ def links=(value) attr_reader :path def path=(path) - raise ArgumentError.new(_("Paths must be fully qualified")) unless Puppet::FileServing::Base.absolute?(path) + raise ArgumentError, _("Paths must be fully qualified") unless Puppet::FileServing::Base.absolute?(path) @path = path end @@ -69,7 +69,7 @@ def path=(path) attr_reader :relative_path def relative_path=(path) - raise ArgumentError.new(_("Relative paths must not be fully qualified")) if Puppet::FileServing::Base.absolute?(path) + raise ArgumentError, _("Relative paths must not be fully qualified") if Puppet::FileServing::Base.absolute?(path) @relative_path = path end diff --git a/lib/puppet/file_serving/configuration/parser.rb b/lib/puppet/file_serving/configuration/parser.rb index ebcf5e7022b..ba5aca54ee0 100644 --- a/lib/puppet/file_serving/configuration/parser.rb +++ b/lib/puppet/file_serving/configuration/parser.rb @@ -43,13 +43,13 @@ def parse end else error_location_str = Puppet::Util::Errors.error_location(@file.filename, @count) - raise ArgumentError.new(_("Invalid argument '%{var}' at %{error_location}") % - { var: var, error_location: error_location_str }) + raise ArgumentError, _("Invalid argument '%{var}' at %{error_location}") % + { var: var, error_location: error_location_str } end else error_location_str = Puppet::Util::Errors.error_location(@file.filename, @count) - raise ArgumentError.new(_("Invalid entry at %{error_location}: '%{file_text}'") % - { file_text: line.chomp, error_location: error_location_str }) + raise ArgumentError, _("Invalid entry at %{error_location}: '%{file_text}'") % + { file_text: line.chomp, error_location: error_location_str } end end end @@ -73,8 +73,8 @@ def changed? def newmount(name) if @mounts.include?(name) error_location_str = Puppet::Util::Errors.error_location(@file, @count) - raise ArgumentError.new(_("%{mount} is already mounted at %{name} at %{error_location}") % - { mount: @mounts[name], name: name, error_location: error_location_str }) + raise ArgumentError, _("%{mount} is already mounted at %{name} at %{error_location}") % + { mount: @mounts[name], name: name, error_location: error_location_str } end case name when "modules" diff --git a/lib/puppet/file_serving/fileset.rb b/lib/puppet/file_serving/fileset.rb index a9d31690615..75c82285dc8 100644 --- a/lib/puppet/file_serving/fileset.rb +++ b/lib/puppet/file_serving/fileset.rb @@ -33,7 +33,7 @@ def initialize(path, options = {}) else path = path.chomp(File::SEPARATOR) unless path == File::SEPARATOR end - raise ArgumentError.new(_("Fileset paths must be fully qualified: %{path}") % { path: path }) unless Puppet::Util.absolute_path?(path) + raise ArgumentError, _("Fileset paths must be fully qualified: %{path}") % { path: path } unless Puppet::Util.absolute_path?(path) @path = path @@ -50,9 +50,9 @@ def initialize(path, options = {}) initialize_from_hash(options) end - raise ArgumentError.new(_("Fileset paths must exist")) unless valid?(path) + raise ArgumentError, _("Fileset paths must exist") unless valid?(path) # TRANSLATORS "recurse" and "recurselimit" are parameter names and should not be translated - raise ArgumentError.new(_("Fileset recurse parameter must not be a number anymore, please use recurselimit")) if @recurse.is_a?(Integer) + raise ArgumentError, _("Fileset recurse parameter must not be a number anymore, please use recurselimit") if @recurse.is_a?(Integer) end # Return a list of all files in our fileset. This is different from the @@ -68,7 +68,7 @@ def files munged_max_files = max_files == '-1' ? -1 : max_files if munged_max_files > 0 && files.size > munged_max_files - raise Puppet::Error.new _("The directory '%{path}' contains %{entries} entries, which exceeds the limit of %{munged_max_files} specified by the max_files parameter for this resource. The limit may be increased, but be aware that large number of file resources can result in excessive resource consumption and degraded performance. Consider using an alternate method to manage large directory trees") % { path: path, entries: files.size, munged_max_files: munged_max_files } + raise Puppet::Error, _("The directory '%{path}' contains %{entries} entries, which exceeds the limit of %{munged_max_files} specified by the max_files parameter for this resource. The limit may be increased, but be aware that large number of file resources can result in excessive resource consumption and degraded performance. Consider using an alternate method to manage large directory trees") % { path: path, entries: files.size, munged_max_files: munged_max_files } elsif munged_max_files == 0 && files.size > soft_max_files Puppet.warning _("The directory '%{path}' contains %{entries} entries, which exceeds the default soft limit %{soft_max_files} and may cause excessive resource consumption and degraded performance. To remove this warning set a value for `max_files` parameter or consider using an alternate method to manage large directory trees") % { path: path, entries: files.size, soft_max_files: soft_max_files } end diff --git a/lib/puppet/file_serving/mount/file.rb b/lib/puppet/file_serving/mount/file.rb index bd0ac8b5780..6454e0c8bcf 100644 --- a/lib/puppet/file_serving/mount/file.rb +++ b/lib/puppet/file_serving/mount/file.rb @@ -17,7 +17,7 @@ def self.localmap def complete_path(relative_path, node) full_path = path(node) - raise ArgumentError.new(_("Mounts without paths are not usable")) unless full_path + raise ArgumentError, _("Mounts without paths are not usable") unless full_path # If there's no relative path name, then we're serving the mount itself. return full_path unless relative_path @@ -72,7 +72,7 @@ def search(path, request) # Verify our configuration is valid. This should really check to # make sure at least someone will be allowed, but, eh. def validate - raise ArgumentError.new(_("Mounts without paths are not usable")) if @path.nil? + raise ArgumentError, _("Mounts without paths are not usable") if @path.nil? end private diff --git a/lib/puppet/file_serving/mount/locales.rb b/lib/puppet/file_serving/mount/locales.rb index c674b689adf..70243552bf0 100644 --- a/lib/puppet/file_serving/mount/locales.rb +++ b/lib/puppet/file_serving/mount/locales.rb @@ -11,9 +11,7 @@ def find(relative_path, request) mod = request.environment.modules.find { |m| m.locale(relative_path) } return nil unless mod - path = mod.locale(relative_path) - - path + mod.locale(relative_path) end def search(relative_path, request) diff --git a/lib/puppet/file_serving/mount/pluginfacts.rb b/lib/puppet/file_serving/mount/pluginfacts.rb index 7d7aa2c589b..425a4fa7cae 100644 --- a/lib/puppet/file_serving/mount/pluginfacts.rb +++ b/lib/puppet/file_serving/mount/pluginfacts.rb @@ -11,9 +11,7 @@ def find(relative_path, request) mod = request.environment.modules.find { |m| m.pluginfact(relative_path) } return nil unless mod - path = mod.pluginfact(relative_path) - - path + mod.pluginfact(relative_path) end def search(relative_path, request) diff --git a/lib/puppet/file_serving/mount/plugins.rb b/lib/puppet/file_serving/mount/plugins.rb index 5cf4fb764b5..606ab24108e 100644 --- a/lib/puppet/file_serving/mount/plugins.rb +++ b/lib/puppet/file_serving/mount/plugins.rb @@ -11,9 +11,7 @@ def find(relative_path, request) mod = request.environment.modules.find { |m| m.plugin(relative_path) } return nil unless mod - path = mod.plugin(relative_path) - - path + mod.plugin(relative_path) end def search(relative_path, request) diff --git a/lib/puppet/file_system/uniquefile.rb b/lib/puppet/file_system/uniquefile.rb index 33611d20992..83a5b783267 100644 --- a/lib/puppet/file_system/uniquefile.rb +++ b/lib/puppet/file_system/uniquefile.rb @@ -57,11 +57,9 @@ def open end def _close - begin - @tmpfile.close if @tmpfile - ensure - @tmpfile = nil - end + @tmpfile.close if @tmpfile + ensure + @tmpfile = nil end protected :_close @@ -142,11 +140,9 @@ def create_tmpname(basename, *rest) end def try_convert_to_hash(h) - begin - h.to_hash - rescue NoMethodError - nil - end + h.to_hash + rescue NoMethodError + nil end @@systmpdir ||= defined?(Etc.systmpdir) ? Etc.systmpdir : '/tmp' diff --git a/lib/puppet/file_system/windows.rb b/lib/puppet/file_system/windows.rb index 0ec1d70364e..82c35c10d94 100644 --- a/lib/puppet/file_system/windows.rb +++ b/lib/puppet/file_system/windows.rb @@ -11,7 +11,7 @@ def open(path, mode, options, &block) # PUP-6959 mode is explicitly ignored until it can be implemented # Ruby on Windows uses mode for setting file attributes like read-only and # archived, not for setting permissions like POSIX - raise TypeError.new('mode must be specified as an Integer') if mode && !mode.is_a?(Numeric) + raise TypeError, 'mode must be specified as an Integer' if mode && !mode.is_a?(Numeric) ::File.open(path, options, nil, &block) end @@ -87,7 +87,7 @@ def unlink(*file_names) if Puppet::Util::Windows::File.symlink?(file_name) Dir.rmdir(file_name) else - raise Errno::EPERM.new(file_name) + raise Errno::EPERM, file_name end else ::File.unlink(file_name) @@ -172,7 +172,7 @@ def replace_file(path, mode = nil) when ACCESS_DENIED, SHARING_VIOLATION, LOCK_VIOLATION raise Errno::EACCES.new(path_string(path), e) else - raise SystemCallError.new(e.message) + raise SystemCallError, e.message end end @@ -206,7 +206,7 @@ def get_dacl_from_file(path) def raise_if_symlinks_unsupported unless Puppet.features.manages_symlinks? msg = _("This version of Windows does not support symlinks. Windows Vista / 2008 or higher is required.") - raise Puppet::Util::Windows::Error.new(msg) + raise Puppet::Util::Windows::Error, msg end unless Puppet::Util::Windows::Process.process_privilege_symlink? diff --git a/lib/puppet/forge.rb b/lib/puppet/forge.rb index 5b7dc1c1246..8d12613d46a 100644 --- a/lib/puppet/forge.rb +++ b/lib/puppet/forge.rb @@ -134,13 +134,11 @@ def initialize(source, data) if meta['dependencies'] dependencies = meta['dependencies'].collect do |dep| - begin - Puppet::ModuleTool::Metadata.new.add_dependency(dep['name'], dep['version_requirement'], dep['repository']) - Puppet::ModuleTool.parse_module_dependency(release, dep)[0..1] - rescue ArgumentError => e - raise ArgumentError, _("Malformed dependency: %{name}.") % { name: dep['name'] } + - ' ' + _("Exception was: %{detail}") % { detail: e } - end + Puppet::ModuleTool::Metadata.new.add_dependency(dep['name'], dep['version_requirement'], dep['repository']) + Puppet::ModuleTool.parse_module_dependency(release, dep)[0..1] + rescue ArgumentError => e + raise ArgumentError, _("Malformed dependency: %{name}.") % { name: dep['name'] } + + ' ' + _("Exception was: %{detail}") % { detail: e } end else dependencies = [] @@ -227,11 +225,9 @@ def validate_checksum(file, checksum, digest_class) end def unpack(file, destination) - begin - Puppet::ModuleTool::Applications::Unpacker.unpack(file.path, destination) - rescue Puppet::ExecutionFailure => e - raise RuntimeError, _("Could not extract contents of module archive: %{message}") % { message: e.message } - end + Puppet::ModuleTool::Applications::Unpacker.unpack(file.path, destination) + rescue Puppet::ExecutionFailure => e + raise RuntimeError, _("Could not extract contents of module archive: %{message}") % { message: e.message } end def deprecated? diff --git a/lib/puppet/functions.rb b/lib/puppet/functions.rb index 6949b30365f..a00d6e3990e 100644 --- a/lib/puppet/functions.rb +++ b/lib/puppet/functions.rb @@ -188,12 +188,11 @@ def self.create_function(func_name, function_base = Function, &block) # and it will fail unless protected with an if defined? if the local # variable does not exist in the block's binder. # - begin - loader = block.binding.eval('loader_injected_arg if defined?(loader_injected_arg)') - create_loaded_function(func_name, loader, function_base, &block) - rescue StandardError => e - raise ArgumentError, _("Function Load Error for function '%{function_name}': %{message}") % { function_name: func_name, message: e.message } - end + + loader = block.binding.eval('loader_injected_arg if defined?(loader_injected_arg)') + create_loaded_function(func_name, loader, function_base, &block) + rescue StandardError => e + raise ArgumentError, _("Function Load Error for function '%{function_name}': %{message}") % { function_name: func_name, message: e.message } end # Creates a function in, or in a local loader under the given loader. @@ -591,14 +590,12 @@ def create_callable(types, block_type, return_type, from, to) end def internal_type_parse(type_string, loader) - begin - Puppet::Pops::Types::TypeParser.singleton.parse(type_string, loader) - rescue StandardError => e - raise ArgumentError, _("Parsing of type string '\"%{type_string}\"' failed with message: <%{message}>.\n") % { - type_string: type_string, - message: e.message - } - end + Puppet::Pops::Types::TypeParser.singleton.parse(type_string, loader) + rescue StandardError => e + raise ArgumentError, _("Parsing of type string '\"%{type_string}\"' failed with message: <%{message}>.\n") % { + type_string: type_string, + message: e.message + } end private :internal_type_parse end @@ -746,25 +743,23 @@ def method3x # used to support other features than calling. # def call(scope, *args, &block) + result = catch(:return) do + mapped_args = Puppet::Pops::Evaluator::Runtime3FunctionArgumentConverter.map_args(args, scope, '') + # this is the scope.function_xxx(...) call + return scope.send(self.class.method3x, mapped_args) + end + return result.value + rescue Puppet::Pops::Evaluator::Next => jumper + begin + throw :next, jumper.value + rescue Puppet::Parser::Scope::UNCAUGHT_THROW_EXCEPTION + raise Puppet::ParseError.new("next() from context where this is illegal", jumper.file, jumper.line) + end + rescue Puppet::Pops::Evaluator::Return => jumper begin - result = catch(:return) do - mapped_args = Puppet::Pops::Evaluator::Runtime3FunctionArgumentConverter.map_args(args, scope, '') - # this is the scope.function_xxx(...) call - return scope.send(self.class.method3x, mapped_args) - end - return result.value - rescue Puppet::Pops::Evaluator::Next => jumper - begin - throw :next, jumper.value - rescue Puppet::Parser::Scope::UNCAUGHT_THROW_EXCEPTION - raise Puppet::ParseError.new("next() from context where this is illegal", jumper.file, jumper.line) - end - rescue Puppet::Pops::Evaluator::Return => jumper - begin - throw :return, jumper - rescue Puppet::Parser::Scope::UNCAUGHT_THROW_EXCEPTION - raise Puppet::ParseError.new("return() from context where this is illegal", jumper.file, jumper.line) - end + throw :return, jumper + rescue Puppet::Parser::Scope::UNCAUGHT_THROW_EXCEPTION + raise Puppet::ParseError.new("return() from context where this is illegal", jumper.file, jumper.line) end end end diff --git a/lib/puppet/functions/eyaml_lookup_key.rb b/lib/puppet/functions/eyaml_lookup_key.rb index 6bee40c0024..77f1c13c065 100644 --- a/lib/puppet/functions/eyaml_lookup_key.rb +++ b/lib/puppet/functions/eyaml_lookup_key.rb @@ -47,21 +47,19 @@ def eyaml_lookup_key(key, options, context) def load_data_hash(options, context) path = options['path'] context.cached_file_data(path) do |content| - begin - data = Puppet::Util::Yaml.safe_load(content, [Symbol], path) - if data.is_a?(Hash) - Puppet::Pops::Lookup::HieraConfig.symkeys_to_string(data) - else - msg = _("%{path}: file does not contain a valid yaml hash") % { path: path } - raise Puppet::DataBinding::LookupError, msg if Puppet[:strict] == :error && data != false + data = Puppet::Util::Yaml.safe_load(content, [Symbol], path) + if data.is_a?(Hash) + Puppet::Pops::Lookup::HieraConfig.symkeys_to_string(data) + else + msg = _("%{path}: file does not contain a valid yaml hash") % { path: path } + raise Puppet::DataBinding::LookupError, msg if Puppet[:strict] == :error && data != false - Puppet.warning(msg) - {} - end - rescue Puppet::Util::Yaml::YamlLoadError => ex - # YamlLoadErrors include the absolute path to the file, so no need to add that - raise Puppet::DataBinding::LookupError, _("Unable to parse %{message}") % { message: ex.message } + Puppet.warning(msg) + {} end + rescue Puppet::Util::Yaml::YamlLoadError => ex + # YamlLoadErrors include the absolute path to the file, so no need to add that + raise Puppet::DataBinding::LookupError, _("Unable to parse %{message}") % { message: ex.message } end end diff --git a/lib/puppet/functions/hocon_data.rb b/lib/puppet/functions/hocon_data.rb index 5dd55980de5..403966685b5 100644 --- a/lib/puppet/functions/hocon_data.rb +++ b/lib/puppet/functions/hocon_data.rb @@ -29,11 +29,9 @@ def hocon_data(options, context) path = options['path'] context.cached_file_data(path) do |content| - begin - Hocon.parse(content) - rescue Hocon::ConfigError => ex - raise Puppet::DataBinding::LookupError, _("Unable to parse (%{path}): %{message}") % { path: path, message: ex.message } - end + Hocon.parse(content) + rescue Hocon::ConfigError => ex + raise Puppet::DataBinding::LookupError, _("Unable to parse (%{path}): %{message}") % { path: path, message: ex.message } end end diff --git a/lib/puppet/functions/import.rb b/lib/puppet/functions/import.rb index d1b77a0c166..b9305c92c8a 100644 --- a/lib/puppet/functions/import.rb +++ b/lib/puppet/functions/import.rb @@ -4,6 +4,6 @@ # Puppet::Functions.create_function(:import) do def import(*args) - raise Puppet::Pops::SemanticError.new(Puppet::Pops::Issues::DISCONTINUED_IMPORT) + raise Puppet::Pops::SemanticError, Puppet::Pops::Issues::DISCONTINUED_IMPORT end end diff --git a/lib/puppet/functions/json_data.rb b/lib/puppet/functions/json_data.rb index cc6592e8ba2..976d5f22f68 100644 --- a/lib/puppet/functions/json_data.rb +++ b/lib/puppet/functions/json_data.rb @@ -20,12 +20,10 @@ def json_data(options, context) path = options['path'] context.cached_file_data(path) do |content| - begin - Puppet::Util::Json.load(content) - rescue Puppet::Util::Json::ParseError => ex - # Filename not included in message, so we add it here. - raise Puppet::DataBinding::LookupError, "Unable to parse (%{path}): %{message}" % { path: path, message: ex.message } - end + Puppet::Util::Json.load(content) + rescue Puppet::Util::Json::ParseError => ex + # Filename not included in message, so we add it here. + raise Puppet::DataBinding::LookupError, "Unable to parse (%{path}): %{message}" % { path: path, message: ex.message } end end diff --git a/lib/puppet/functions/reduce.rb b/lib/puppet/functions/reduce.rb index 4681747cf10..f4a0366195e 100644 --- a/lib/puppet/functions/reduce.rb +++ b/lib/puppet/functions/reduce.rb @@ -142,22 +142,18 @@ def reduce_without_memo(enumerable) enum = Puppet::Pops::Types::Iterable.asserted_iterable(self, enumerable) enum.reduce do |memo, x| - begin - yield(memo, x) - rescue StopIteration - return memo - end + yield(memo, x) + rescue StopIteration + return memo end end def reduce_with_memo(enumerable, given_memo) enum = Puppet::Pops::Types::Iterable.asserted_iterable(self, enumerable) enum.reduce(given_memo) do |memo, x| - begin - yield(memo, x) - rescue StopIteration - return memo - end + yield(memo, x) + rescue StopIteration + return memo end end end diff --git a/lib/puppet/functions/require.rb b/lib/puppet/functions/require.rb index dad7f3bf406..ed283b2f6ae 100644 --- a/lib/puppet/functions/require.rb +++ b/lib/puppet/functions/require.rb @@ -70,7 +70,7 @@ def require_impl(scope, *classes) classes.each do |klass| # lookup the class in the scopes klass = (classobj = krt.find_hostclass(klass)) ? classobj.name : nil - raise Puppet::ParseError.new(_("Could not find class %{klass}") % { klass: klass }) unless klass + raise Puppet::ParseError, _("Could not find class %{klass}") % { klass: klass } unless klass ref = Puppet::Resource.new(:class, klass) resource = scope.resource diff --git a/lib/puppet/functions/yaml_data.rb b/lib/puppet/functions/yaml_data.rb index 895de520da1..f122e734826 100644 --- a/lib/puppet/functions/yaml_data.rb +++ b/lib/puppet/functions/yaml_data.rb @@ -22,21 +22,19 @@ def yaml_data(options, context) path = options['path'] context.cached_file_data(path) do |content| - begin - data = Puppet::Util::Yaml.safe_load(content, [Symbol], path) - if data.is_a?(Hash) - Puppet::Pops::Lookup::HieraConfig.symkeys_to_string(data) - else - msg = _("%{path}: file does not contain a valid yaml hash" % { path: path }) - raise Puppet::DataBinding::LookupError, msg if Puppet[:strict] == :error && data != false + data = Puppet::Util::Yaml.safe_load(content, [Symbol], path) + if data.is_a?(Hash) + Puppet::Pops::Lookup::HieraConfig.symkeys_to_string(data) + else + msg = _("%{path}: file does not contain a valid yaml hash" % { path: path }) + raise Puppet::DataBinding::LookupError, msg if Puppet[:strict] == :error && data != false - Puppet.warning(msg) - {} - end - rescue Puppet::Util::Yaml::YamlLoadError => ex - # YamlLoadErrors include the absolute path to the file, so no need to add that - raise Puppet::DataBinding::LookupError, _("Unable to parse %{message}") % { message: ex.message } + Puppet.warning(msg) + {} end + rescue Puppet::Util::Yaml::YamlLoadError => ex + # YamlLoadErrors include the absolute path to the file, so no need to add that + raise Puppet::DataBinding::LookupError, _("Unable to parse %{message}") % { message: ex.message } end end diff --git a/lib/puppet/graph/simple_graph.rb b/lib/puppet/graph/simple_graph.rb index 68e842bd379..a050b365df1 100644 --- a/lib/puppet/graph/simple_graph.rb +++ b/lib/puppet/graph/simple_graph.rb @@ -449,7 +449,7 @@ def path_between(f, t) # which match +dot+ properties will be used as well. def to_dot_graph(params = {}) params['name'] ||= self.class.name.tr(':', '_') - fontsize = params['fontsize'] ? params['fontsize'] : '8' + fontsize = params['fontsize'] || '8' graph = (directed? ? DOT::DOTDigraph : DOT::DOTSubgraph).new(params) edge_klass = directed? ? DOT::DOTDirectedEdge : DOT::DOTEdge vertices.each do |v| diff --git a/lib/puppet/http/external_client.rb b/lib/puppet/http/external_client.rb index 804e6c1d6cb..193474640c1 100644 --- a/lib/puppet/http/external_client.rb +++ b/lib/puppet/http/external_client.rb @@ -39,7 +39,7 @@ def get(url, headers: {}, params: {}, options: {}, &block) # (see Puppet::HTTP::Client#post) # @api private def post(url, body, headers: {}, params: {}, options: {}, &block) - raise ArgumentError.new("'post' requires a string 'body' argument") unless body.is_a?(String) + raise ArgumentError, "'post' requires a string 'body' argument" unless body.is_a?(String) url = encode_query(url, params) diff --git a/lib/puppet/http/redirector.rb b/lib/puppet/http/redirector.rb index 4c3d1934661..4460eefcf96 100644 --- a/lib/puppet/http/redirector.rb +++ b/lib/puppet/http/redirector.rb @@ -43,7 +43,7 @@ def redirect?(request, response) # # @api private def redirect_to(request, response, redirects) - raise Puppet::HTTP::TooManyRedirects.new(request.uri) if redirects >= @redirect_limit + raise Puppet::HTTP::TooManyRedirects, request.uri if redirects >= @redirect_limit location = parse_location(response) url = request.uri.merge(location) @@ -78,7 +78,7 @@ def redirect_to(request, response, redirects) def parse_location(response) location = response['location'] - raise Puppet::HTTP::ProtocolError.new(_("Location response header is missing")) unless location + raise Puppet::HTTP::ProtocolError, _("Location response header is missing") unless location URI.parse(location) rescue URI::InvalidURIError => e diff --git a/lib/puppet/http/retry_after_handler.rb b/lib/puppet/http/retry_after_handler.rb index fbe95a1e117..d1aefdb97bd 100644 --- a/lib/puppet/http/retry_after_handler.rb +++ b/lib/puppet/http/retry_after_handler.rb @@ -48,7 +48,7 @@ def retry_after?(request, response) # # @api private def retry_after_interval(request, response, retries) - raise Puppet::HTTP::TooManyRetryAfters.new(request.uri) if retries >= @retry_limit + raise Puppet::HTTP::TooManyRetryAfters, request.uri if retries >= @retry_limit retry_after = response['Retry-After'] return nil unless retry_after @@ -72,7 +72,7 @@ def parse_retry_after(retry_after) seconds = (tm.to_time - DateTime.now.to_time).to_i [seconds, 0].max rescue ArgumentError - raise Puppet::HTTP::ProtocolError.new(_("Failed to parse Retry-After header '%{retry_after}' as an integer or RFC 2822 date") % { retry_after: retry_after }) + raise Puppet::HTTP::ProtocolError, _("Failed to parse Retry-After header '%{retry_after}' as an integer or RFC 2822 date") % { retry_after: retry_after } end end end diff --git a/lib/puppet/http/service.rb b/lib/puppet/http/service.rb index 2e8da3db871..e031450506f 100644 --- a/lib/puppet/http/service.rb +++ b/lib/puppet/http/service.rb @@ -129,30 +129,26 @@ def get_mime_types(model) def formatter_for_response(response) header = response['Content-Type'] - raise Puppet::HTTP::ProtocolError.new(_("No content type in http response; cannot parse")) unless header + raise Puppet::HTTP::ProtocolError, _("No content type in http response; cannot parse") unless header header.gsub!(/\s*;.*$/, '') # strip any charset formatter = Puppet::Network::FormatHandler.mime(header) - raise Puppet::HTTP::ProtocolError.new("Content-Type is unsupported") if EXCLUDED_FORMATS.include?(formatter.name) + raise Puppet::HTTP::ProtocolError, "Content-Type is unsupported" if EXCLUDED_FORMATS.include?(formatter.name) formatter end def serialize(formatter, object) - begin - formatter.render(object) - rescue => err - raise Puppet::HTTP::SerializationError.new("Failed to serialize #{object.class} to #{formatter.name}: #{err.message}", err) - end + formatter.render(object) + rescue => err + raise Puppet::HTTP::SerializationError.new("Failed to serialize #{object.class} to #{formatter.name}: #{err.message}", err) end def serialize_multiple(formatter, object) - begin - formatter.render_multiple(object) - rescue => err - raise Puppet::HTTP::SerializationError.new("Failed to serialize multiple #{object.class} to #{formatter.name}: #{err.message}", err) - end + formatter.render_multiple(object) + rescue => err + raise Puppet::HTTP::SerializationError.new("Failed to serialize multiple #{object.class} to #{formatter.name}: #{err.message}", err) end def deserialize(response, model) @@ -176,6 +172,6 @@ def deserialize_multiple(response, model) def process_response(response) @session.process_response(response) - raise Puppet::HTTP::ResponseError.new(response) unless response.success? + raise Puppet::HTTP::ResponseError, response unless response.success? end end diff --git a/lib/puppet/http/service/ca.rb b/lib/puppet/http/service/ca.rb index dd41bd9cd99..4245499af57 100644 --- a/lib/puppet/http/service/ca.rb +++ b/lib/puppet/http/service/ca.rb @@ -124,7 +124,7 @@ def post_certificate_renewal(ssl_context) options: { ssl_context: ssl_context } ) - raise ArgumentError.new(_('SSL context must contain a client certificate.')) unless ssl_context.client_cert + raise ArgumentError, _('SSL context must contain a client certificate.') unless ssl_context.client_cert process_response(response) diff --git a/lib/puppet/http/service/compiler.rb b/lib/puppet/http/service/compiler.rb index b403087da04..688f7b2f9cd 100644 --- a/lib/puppet/http/service/compiler.rb +++ b/lib/puppet/http/service/compiler.rb @@ -163,9 +163,9 @@ def post_catalog(name, facts:, environment:, configured_environment: nil, check_ # def post_catalog4(certname, persistence:, environment:, facts: nil, trusted_facts: nil, transaction_uuid: nil, job_id: nil, options: nil) unless persistence.is_a?(Hash) && (missing = [:facts, :catalog] - persistence.keys.map(&:to_sym)).empty? - raise ArgumentError.new("The 'persistence' hash is missing the keys: #{missing.join(', ')}") + raise ArgumentError, "The 'persistence' hash is missing the keys: #{missing.join(', ')}" end - raise ArgumentError.new("Facts must be a Hash not a #{facts.class}") unless facts.nil? || facts.is_a?(Hash) + raise ArgumentError, "Facts must be a Hash not a #{facts.class}" unless facts.nil? || facts.is_a?(Hash) body = { certname: certname, diff --git a/lib/puppet/http/service/report.rb b/lib/puppet/http/service/report.rb index 6ab0be33136..3b86482080b 100644 --- a/lib/puppet/http/service/report.rb +++ b/lib/puppet/http/service/report.rb @@ -56,7 +56,7 @@ def put_report(name, report, environment:) if response.success? response else - raise Puppet::HTTP::ResponseError.new(response) + raise Puppet::HTTP::ResponseError, response end end end diff --git a/lib/puppet/indirector/face.rb b/lib/puppet/indirector/face.rb index 69406828592..8593c581aa9 100644 --- a/lib/puppet/indirector/face.rb +++ b/lib/puppet/indirector/face.rb @@ -134,11 +134,9 @@ def indirection end def set_terminus(from) - begin - indirection.terminus_class = from - rescue => detail - msg = _("Could not set '%{indirection}' terminus to '%{from}' (%{detail}); valid terminus types are %{types}") % { indirection: indirection.name, from: from, detail: detail, types: self.class.terminus_classes(indirection.name).join(", ") } - raise detail, msg, detail.backtrace - end + indirection.terminus_class = from + rescue => detail + msg = _("Could not set '%{indirection}' terminus to '%{from}' (%{detail}); valid terminus types are %{types}") % { indirection: indirection.name, from: from, detail: detail, types: self.class.terminus_classes(indirection.name).join(", ") } + raise detail, msg, detail.backtrace end end diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb index 02a2147067f..63afaa7d2d2 100644 --- a/lib/puppet/indirector/indirection.rb +++ b/lib/puppet/indirector/indirection.rb @@ -243,12 +243,10 @@ def find(key, options = {}) filtered = result if terminus.respond_to?(:filter) Puppet::Util::Profiler.profile(_("Filtered result for %{indirection} %{request}") % { indirection: self.name, request: request.key }, [:indirector, :filter, self.name, request.key]) do - begin - filtered = terminus.filter(result) - rescue Puppet::Error => detail - Puppet.log_exception(detail) - raise detail - end + filtered = terminus.filter(result) + rescue Puppet::Error => detail + Puppet.log_exception(detail) + raise detail end end filtered diff --git a/lib/puppet/indirector/memory.rb b/lib/puppet/indirector/memory.rb index 66ba6048e59..701f7a77cfa 100644 --- a/lib/puppet/indirector/memory.rb +++ b/lib/puppet/indirector/memory.rb @@ -13,7 +13,7 @@ def clear end def destroy(request) - raise ArgumentError.new(_("Could not find %{request} to destroy") % { request: request.key }) unless @instances.include?(request.key) + raise ArgumentError, _("Could not find %{request} to destroy") % { request: request.key } unless @instances.include?(request.key) @instances.delete(request.key) end diff --git a/lib/puppet/indirector/request.rb b/lib/puppet/indirector/request.rb index 23111addd2c..12238a0ee4b 100644 --- a/lib/puppet/indirector/request.rb +++ b/lib/puppet/indirector/request.rb @@ -140,7 +140,7 @@ def to_hash end def description - return(uri ? uri : "/#{indirection_name}/#{key}") + return(uri || "/#{indirection_name}/#{key}") end def remote? diff --git a/lib/puppet/indirector/yaml.rb b/lib/puppet/indirector/yaml.rb index a70b9023ba3..ae3ca8fc736 100644 --- a/lib/puppet/indirector/yaml.rb +++ b/lib/puppet/indirector/yaml.rb @@ -19,7 +19,7 @@ def find(request) # Convert our object to YAML and store it to the disk. def save(request) - raise ArgumentError.new(_("You can only save objects that respond to :name")) unless request.instance.respond_to?(:name) + raise ArgumentError, _("You can only save objects that respond to :name") unless request.instance.respond_to?(:name) file = path(request.key) diff --git a/lib/puppet/info_service/class_information_service.rb b/lib/puppet/info_service/class_information_service.rb index 67a0e270ea9..df6f666afb5 100644 --- a/lib/puppet/info_service/class_information_service.rb +++ b/lib/puppet/info_service/class_information_service.rb @@ -89,12 +89,10 @@ def extract_default(structure, p) end def typeexpr_to_string(type_expr) - begin - type_parser.interpret_any(type_expr, nil).to_s - rescue Puppet::ParseError - # type is to complex - contains expressions that are not literal - nil - end + type_parser.interpret_any(type_expr, nil).to_s + rescue Puppet::ParseError + # type is to complex - contains expressions that are not literal + nil end def value_as_literal(value_expr) diff --git a/lib/puppet/info_service/task_information_service.rb b/lib/puppet/info_service/task_information_service.rb index dfc3a36e98e..062e90d8003 100644 --- a/lib/puppet/info_service/task_information_service.rb +++ b/lib/puppet/info_service/task_information_service.rb @@ -10,13 +10,12 @@ def self.tasks_per_environment(environment_name) env.modules.map do |mod| mod.tasks.map do |task| # If any task is malformed continue to list other tasks in module - begin - task.validate - { :module => { :name => task.module.name }, :name => task.name, :metadata => task.metadata } - rescue Puppet::Module::Task::Error => err - Puppet.log_exception(err) - nil - end + + task.validate + { :module => { :name => task.module.name }, :name => task.name, :metadata => task.metadata } + rescue Puppet::Module::Task::Error => err + Puppet.log_exception(err) + nil end end.flatten.compact end diff --git a/lib/puppet/module.rb b/lib/puppet/module.rb index fae20803059..34cb93f2b8e 100644 --- a/lib/puppet/module.rb +++ b/lib/puppet/module.rb @@ -100,12 +100,10 @@ def validate_puppet_version end def has_metadata? - begin - load_metadata - @metadata.is_a?(Hash) && !@metadata.empty? - rescue Puppet::Module::MissingMetadata - false - end + load_metadata + @metadata.is_a?(Hash) && !@metadata.empty? + rescue Puppet::Module::MissingMetadata + false end FILETYPES.each do |type, location| diff --git a/lib/puppet/module/task.rb b/lib/puppet/module/task.rb index 0d3b089dbab..acc987c3e40 100644 --- a/lib/puppet/module/task.rb +++ b/lib/puppet/module/task.rb @@ -130,7 +130,7 @@ def self.find_extra_files(metadata, envname = nil) unless File.exist?(path) msg = _("Could not find %{path} on disk" % { path: path }) - raise InvalidFile.new(msg) + raise InvalidFile, msg end last_char = file[-1] == '/' diff --git a/lib/puppet/module_tool/applications/application.rb b/lib/puppet/module_tool/applications/application.rb index e44aa03d89e..50544607027 100644 --- a/lib/puppet/module_tool/applications/application.rb +++ b/lib/puppet/module_tool/applications/application.rb @@ -50,11 +50,9 @@ def metadata(require_metadata = false) if File.file?(metadata_path) File.open(metadata_path) do |f| - begin - @metadata.update(Puppet::Util::Json.load(f)) - rescue Puppet::Util::Json::ParseError => ex - raise ArgumentError, _("Could not parse JSON %{metadata_path}") % { metadata_path: metadata_path }, ex.backtrace - end + @metadata.update(Puppet::Util::Json.load(f)) + rescue Puppet::Util::Json::ParseError => ex + raise ArgumentError, _("Could not parse JSON %{metadata_path}") % { metadata_path: metadata_path }, ex.backtrace end end diff --git a/lib/puppet/module_tool/applications/unpacker.rb b/lib/puppet/module_tool/applications/unpacker.rb index ad6e6510851..0f9eacdcec9 100644 --- a/lib/puppet/module_tool/applications/unpacker.rb +++ b/lib/puppet/module_tool/applications/unpacker.rb @@ -54,11 +54,9 @@ def sanity_check # @api private def unpack - begin - Puppet::ModuleTool::Tar.instance.unpack(@filename.to_s, tmpdir, [@module_path.stat.uid, @module_path.stat.gid].join(':')) - rescue Puppet::ExecutionFailure => e - raise RuntimeError, _("Could not extract contents of module archive: %{message}") % { message: e.message } - end + Puppet::ModuleTool::Tar.instance.unpack(@filename.to_s, tmpdir, [@module_path.stat.uid, @module_path.stat.gid].join(':')) + rescue Puppet::ExecutionFailure => e + raise RuntimeError, _("Could not extract contents of module archive: %{message}") % { message: e.message } end # @api private diff --git a/lib/puppet/module_tool/local_tarball.rb b/lib/puppet/module_tool/local_tarball.rb index bd144ecc029..a76d2f80105 100644 --- a/lib/puppet/module_tool/local_tarball.rb +++ b/lib/puppet/module_tool/local_tarball.rb @@ -84,11 +84,9 @@ def tmpdir # rubocop:enable Naming/MemoizedInstanceVariableName def unpack(file, destination) - begin - Puppet::ModuleTool::Applications::Unpacker.unpack(file, destination) - rescue Puppet::ExecutionFailure => e - raise RuntimeError, _("Could not extract contents of module archive: %{message}") % { message: e.message } - end + Puppet::ModuleTool::Applications::Unpacker.unpack(file, destination) + rescue Puppet::ExecutionFailure => e + raise RuntimeError, _("Could not extract contents of module archive: %{message}") % { message: e.message } end end end diff --git a/lib/puppet/network/http/api/indirected_routes.rb b/lib/puppet/network/http/api/indirected_routes.rb index 90a2d1e8dd0..36f95c1b892 100644 --- a/lib/puppet/network/http/api/indirected_routes.rb +++ b/lib/puppet/network/http/api/indirected_routes.rb @@ -62,9 +62,7 @@ def uri2indirection(http_method, uri, params) environment = params.delete(:environment) if indirection_name !~ /^\w+$/ - raise Puppet::Network::HTTP::Error::HTTPBadRequestError.new( - _("The indirection name must be purely alphanumeric, not '%{indirection_name}'") % { indirection_name: indirection_name } - ) + raise Puppet::Network::HTTP::Error::HTTPBadRequestError, _("The indirection name must be purely alphanumeric, not '%{indirection_name}'") % { indirection_name: indirection_name } end # this also depluralizes the indirection_name if it is a search @@ -73,9 +71,7 @@ def uri2indirection(http_method, uri, params) # check whether this indirection matches the prefix and version in the # request if url_prefix != IndirectionType.url_prefix_for(indirection_name) - raise Puppet::Network::HTTP::Error::HTTPBadRequestError.new( - _("Indirection '%{indirection_name}' does not match url prefix '%{url_prefix}'") % { indirection_name: indirection_name, url_prefix: url_prefix } - ) + raise Puppet::Network::HTTP::Error::HTTPBadRequestError, _("Indirection '%{indirection_name}' does not match url prefix '%{url_prefix}'") % { indirection_name: indirection_name, url_prefix: url_prefix } end indirection = Puppet::Indirector::Indirection.instance(indirection_name.to_sym) @@ -87,15 +83,11 @@ def uri2indirection(http_method, uri, params) end unless environment - raise Puppet::Network::HTTP::Error::HTTPBadRequestError.new( - _("An environment parameter must be specified") - ) + raise Puppet::Network::HTTP::Error::HTTPBadRequestError, _("An environment parameter must be specified") end unless Puppet::Node::Environment.valid_name?(environment) - raise Puppet::Network::HTTP::Error::HTTPBadRequestError.new( - _("The environment must be purely alphanumeric, not '%{environment}'") % { environment: environment } - ) + raise Puppet::Network::HTTP::Error::HTTPBadRequestError, _("The environment must be purely alphanumeric, not '%{environment}'") % { environment: environment } end configured_environment = Puppet.lookup(:environments).get(environment) @@ -105,17 +97,13 @@ def uri2indirection(http_method, uri, params) end if configured_environment.nil? && indirection.terminus.require_environment? - raise Puppet::Network::HTTP::Error::HTTPNotFoundError.new( - _("Could not find environment '%{environment}'") % { environment: environment } - ) + raise Puppet::Network::HTTP::Error::HTTPNotFoundError, _("Could not find environment '%{environment}'") % { environment: environment } end params.delete(:bucket_path) if key == "" or key.nil? - raise Puppet::Network::HTTP::Error::HTTPBadRequestError.new( - _("No request key specified in %{uri}") % { uri: uri } - ) + raise Puppet::Network::HTTP::Error::HTTPBadRequestError, _("No request key specified in %{uri}") % { uri: uri } end [indirection, method, key, params] @@ -196,19 +184,17 @@ def do_save(indirection, key, params, request, response) def first_response_formatter_for(model, request, key, &block) formats = accepted_response_formatters_for(model, request) formatter = formats.find do |format| - begin - yield format - true - rescue Puppet::Network::FormatHandler::FormatError => err - msg = _("Failed to serialize %{model} for '%{key}': %{detail}") % - { model: model, key: key, detail: err } - if Puppet[:allow_pson_serialization] - Puppet.warning(msg) - else - raise Puppet::Network::FormatHandler::FormatError.new(msg) - end - false + yield format + true + rescue Puppet::Network::FormatHandler::FormatError => err + msg = _("Failed to serialize %{model} for '%{key}': %{detail}") % + { model: model, key: key, detail: err } + if Puppet[:allow_pson_serialization] + Puppet.warning(msg) + else + raise Puppet::Network::FormatHandler::FormatError, msg end + false end return formatter if formatter @@ -239,9 +225,7 @@ def read_body_into_model(model_class, request) begin return model_class.convert_from(formatter.name.to_s, data) rescue => e - raise Puppet::Network::HTTP::Error::HTTPBadRequestError.new( - _("The request body is invalid: %{message}") % { message: e.message } - ) + raise Puppet::Network::HTTP::Error::HTTPBadRequestError, _("The request body is invalid: %{message}") % { message: e.message } end end @@ -253,15 +237,11 @@ def read_body_into_model(model_class, request) end def indirection_method(http_method, indirection) - raise Puppet::Network::HTTP::Error::HTTPMethodNotAllowedError.new( - _("No support for http method %{http_method}") % { http_method: http_method } - ) unless METHOD_MAP[http_method] + raise Puppet::Network::HTTP::Error::HTTPMethodNotAllowedError, _("No support for http method %{http_method}") % { http_method: http_method } unless METHOD_MAP[http_method] method = METHOD_MAP[http_method][plurality(indirection)] unless method - raise Puppet::Network::HTTP::Error::HTTPBadRequestError.new( - _("No support for plurality %{indirection} for %{http_method} operations") % { indirection: plurality(indirection), http_method: http_method } - ) + raise Puppet::Network::HTTP::Error::HTTPBadRequestError, _("No support for plurality %{indirection} for %{http_method} operations") % { indirection: plurality(indirection), http_method: http_method } end method diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb index d7813f7a319..976c5a5d8aa 100644 --- a/lib/puppet/node/environment.rb +++ b/lib/puppet/node/environment.rb @@ -366,12 +366,10 @@ def modules end @modules = module_references.filter_map do |reference| - begin - Puppet::Module.new(reference[:name], reference[:path], self) - rescue Puppet::Module::Error => e - Puppet.log_exception(e) - nil - end + Puppet::Module.new(reference[:name], reference[:path], self) + rescue Puppet::Module::Error => e + Puppet.log_exception(e) + nil end end @modules diff --git a/lib/puppet/pal/pal_impl.rb b/lib/puppet/pal/pal_impl.rb index 38e2f0a3ccb..deabc48e858 100644 --- a/lib/puppet/pal/pal_impl.rb +++ b/lib/puppet/pal/pal_impl.rb @@ -443,64 +443,62 @@ def self.main( # TRANSLATORS, the string "For puppet PAL" is not user facing Puppet.override({ :current_environment => apply_environment }, "For puppet PAL") do - begin - node.sanitize() - compiler = create_internal_compiler(internal_compiler_class, node) - - case internal_compiler_class - when :script - pal_compiler = ScriptCompiler.new(compiler) - overrides[:pal_script_compiler] = overrides[:pal_compiler] = pal_compiler - when :catalog - pal_compiler = CatalogCompiler.new(compiler) - overrides[:pal_catalog_compiler] = overrides[:pal_compiler] = pal_compiler - end + node.sanitize() + compiler = create_internal_compiler(internal_compiler_class, node) + + case internal_compiler_class + when :script + pal_compiler = ScriptCompiler.new(compiler) + overrides[:pal_script_compiler] = overrides[:pal_compiler] = pal_compiler + when :catalog + pal_compiler = CatalogCompiler.new(compiler) + overrides[:pal_catalog_compiler] = overrides[:pal_compiler] = pal_compiler + end - # When scripting the trusted data are always local; default is to set them anyway - # When compiling for a catalog, the catalog compiler does this - if set_local_facts - compiler.topscope.set_trusted(node.trusted_data) + # When scripting the trusted data are always local; default is to set them anyway + # When compiling for a catalog, the catalog compiler does this + if set_local_facts + compiler.topscope.set_trusted(node.trusted_data) - # Server facts are always about the local node's version etc. - compiler.topscope.set_server_facts(node.server_facts) + # Server facts are always about the local node's version etc. + compiler.topscope.set_server_facts(node.server_facts) - # Set $facts for the node running the script - facts_hash = node.facts.nil? ? {} : node.facts.values - compiler.topscope.set_facts(facts_hash) + # Set $facts for the node running the script + facts_hash = node.facts.nil? ? {} : node.facts.values + compiler.topscope.set_facts(facts_hash) - # create the $settings:: variables - compiler.topscope.merge_settings(node.environment.name, false) - end + # create the $settings:: variables + compiler.topscope.merge_settings(node.environment.name, false) + end - # Make compiler available to Puppet#lookup and injection in functions - # TODO: The compiler instances should be available under non PAL use as well! - # TRANSLATORS: Do not translate, symbolic name - Puppet.override(overrides, "PAL::with_#{internal_compiler_class}_compiler") do - compiler.compile do |_compiler_yield| - # In case the variables passed to the compiler are PCore types defined in modules, they - # need to be deserialized and added from within the this scope, so that loaders are - # available during deserizlization. - pal_variables = Puppet::Pops::Serialization::FromDataConverter.convert(pal_variables) - variables = Puppet::Pops::Serialization::FromDataConverter.convert(variables) - - # Merge together target variables and plan variables. This will also shadow any - # collisions with facts and emit a warning. - topscope_vars = pal_variables.merge(merge_vars(target_variables, variables, node.facts.values)) - - add_variables(compiler.topscope, topscope_vars) - # wrap the internal compiler to prevent it from leaking in the PAL API - if block_given? - yield(pal_compiler) - end + # Make compiler available to Puppet#lookup and injection in functions + # TODO: The compiler instances should be available under non PAL use as well! + # TRANSLATORS: Do not translate, symbolic name + Puppet.override(overrides, "PAL::with_#{internal_compiler_class}_compiler") do + compiler.compile do |_compiler_yield| + # In case the variables passed to the compiler are PCore types defined in modules, they + # need to be deserialized and added from within the this scope, so that loaders are + # available during deserizlization. + pal_variables = Puppet::Pops::Serialization::FromDataConverter.convert(pal_variables) + variables = Puppet::Pops::Serialization::FromDataConverter.convert(variables) + + # Merge together target variables and plan variables. This will also shadow any + # collisions with facts and emit a warning. + topscope_vars = pal_variables.merge(merge_vars(target_variables, variables, node.facts.values)) + + add_variables(compiler.topscope, topscope_vars) + # wrap the internal compiler to prevent it from leaking in the PAL API + if block_given? + yield(pal_compiler) end end - rescue Puppet::Error - # already logged and handled by the compiler, including Puppet::ParseErrorWithIssue - raise - rescue => detail - Puppet.log_exception(detail) - raise end + rescue Puppet::Error + # already logged and handled by the compiler, including Puppet::ParseErrorWithIssue + raise + rescue => detail + Puppet.log_exception(detail) + raise end end private_class_method :main diff --git a/lib/puppet/parameter.rb b/lib/puppet/parameter.rb index 098c3dc1f10..6a73868c81e 100644 --- a/lib/puppet/parameter.rb +++ b/lib/puppet/parameter.rb @@ -385,8 +385,7 @@ def name # def noop @noop ||= false - tmp = @noop || self.resource.noop || Puppet[:noop] || false - tmp + @noop || self.resource.noop || Puppet[:noop] || false end # Returns an array of strings representing the containment hierarchy diff --git a/lib/puppet/parser/ast.rb b/lib/puppet/parser/ast.rb index 6cd14847d75..a389f13fdf7 100644 --- a/lib/puppet/parser/ast.rb +++ b/lib/puppet/parser/ast.rb @@ -28,21 +28,20 @@ def evaluate(scope) def safeevaluate(scope) # We duplicate code here, rather than using exceptwrap, because this # is called so many times during parsing. - begin - return self.evaluate(scope) - rescue Puppet::Pops::Evaluator::PuppetStopIteration => detail - raise detail - # # Only deals with StopIteration from the break() function as a general - # # StopIteration is a general runtime problem - # raise Puppet::ParseError.new(detail.message, detail.file, detail.line, detail) - rescue Puppet::Error => detail - raise adderrorcontext(detail) - rescue => detail - error = Puppet::ParseError.new(detail.to_s, nil, nil, detail) - # We can't use self.fail here because it always expects strings, - # not exceptions. - raise adderrorcontext(error, detail) - end + + return self.evaluate(scope) + rescue Puppet::Pops::Evaluator::PuppetStopIteration => detail + raise detail + # # Only deals with StopIteration from the break() function as a general + # # StopIteration is a general runtime problem + # raise Puppet::ParseError.new(detail.message, detail.file, detail.line, detail) + rescue Puppet::Error => detail + raise adderrorcontext(detail) + rescue => detail + error = Puppet::ParseError.new(detail.to_s, nil, nil, detail) + # We can't use self.fail here because it always expects strings, + # not exceptions. + raise adderrorcontext(error, detail) end def initialize(file: nil, line: nil, pos: nil) diff --git a/lib/puppet/parser/ast/pops_bridge.rb b/lib/puppet/parser/ast/pops_bridge.rb index eb3f531f670..3217afd002e 100644 --- a/lib/puppet/parser/ast/pops_bridge.rb +++ b/lib/puppet/parser/ast/pops_bridge.rb @@ -173,7 +173,7 @@ def create_type_map(definition) # Obtains the scope or issues a warning if :global_scope is not bound def obtain_scope - scope = Puppet.lookup(:global_scope) do + Puppet.lookup(:global_scope) do # This occurs when testing and when applying a catalog (there is no scope available then), and # when running tests that run a partial setup. # This is bad if the logic is trying to compile, but a warning can not be issues since it is a normal @@ -181,7 +181,6 @@ def obtain_scope Puppet.debug { _("Instantiating Resource with type checked parameters - scope is missing, skipping type checking.") } nil end - scope end # Produces a hash with data for Definition and HostClass diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb index 8662a6b1cd7..10a373e4d6a 100644 --- a/lib/puppet/parser/compiler.rb +++ b/lib/puppet/parser/compiler.rb @@ -345,17 +345,15 @@ def evaluate_definitions exceptwrap do Puppet::Util::Profiler.profile(_("Evaluated definitions"), [:compiler, :evaluate_definitions]) do urs = unevaluated_resources.each do |resource| - begin - resource.evaluate - rescue Puppet::Pops::Evaluator::PuppetStopIteration => detail - # needs to be handled specifically as the error has the file/line/position where this - # occurred rather than the resource - fail(Puppet::Pops::Issues::RUNTIME_ERROR, detail, { :detail => detail.message }, detail) - rescue Puppet::Error => e - # PuppetError has the ability to wrap an exception, if so, use the wrapped exception's - # call stack instead - fail(Puppet::Pops::Issues::RUNTIME_ERROR, resource, { :detail => e.message }, e.original || e) - end + resource.evaluate + rescue Puppet::Pops::Evaluator::PuppetStopIteration => detail + # needs to be handled specifically as the error has the file/line/position where this + # occurred rather than the resource + fail(Puppet::Pops::Issues::RUNTIME_ERROR, detail, { :detail => detail.message }, detail) + rescue Puppet::Error => e + # PuppetError has the ability to wrap an exception, if so, use the wrapped exception's + # call stack instead + fail(Puppet::Pops::Issues::RUNTIME_ERROR, resource, { :detail => e.message }, e.original || e) end !urs.empty? end diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb index febdccdfbc8..ca7386e4f54 100644 --- a/lib/puppet/parser/scope.rb +++ b/lib/puppet/parser/scope.rb @@ -773,7 +773,7 @@ def transform_setting(val) # when you need to set options. def setvar(name, value, options = EMPTY_HASH) if name =~ /^[0-9]+$/ - raise Puppet::ParseError.new(_("Cannot assign to a numeric match result variable '$%{name}'") % { name: name }) # unless options[:ephemeral] + raise Puppet::ParseError, _("Cannot assign to a numeric match result variable '$%{name}'") % { name: name } # unless options[:ephemeral] end unless name.is_a? String raise Puppet::ParseError, _("Scope variable name %{name} is a %{class_type}, not a string") % { name: name.inspect, class_type: name.class } diff --git a/lib/puppet/parser/templatewrapper.rb b/lib/puppet/parser/templatewrapper.rb index 5781cd9a72c..cbeb2b891b0 100644 --- a/lib/puppet/parser/templatewrapper.rb +++ b/lib/puppet/parser/templatewrapper.rb @@ -100,6 +100,6 @@ def result(string = nil) end def to_s - "template[#{(@__file__ ? @__file__ : "inline")}]" + "template[#{(@__file__ || "inline")}]" end end diff --git a/lib/puppet/parser/type_loader.rb b/lib/puppet/parser/type_loader.rb index f0710d2a2a7..874190bc91f 100644 --- a/lib/puppet/parser/type_loader.rb +++ b/lib/puppet/parser/type_loader.rb @@ -67,17 +67,15 @@ def try_load_fqname(type, fqname) return nil if fqname == "" # special-case main. files_to_try_for(fqname).each do |filename| - begin - imported_types = import_from_modules(filename) - result = imported_types.find { |t| t.type == type and t.name == fqname } - if result - Puppet.debug { "Automatically imported #{fqname} from #{filename} into #{environment}" } - return result - end - rescue TypeLoaderError - # I'm not convinced we should just drop these errors, but this - # preserves existing behaviours. + imported_types = import_from_modules(filename) + result = imported_types.find { |t| t.type == type and t.name == fqname } + if result + Puppet.debug { "Automatically imported #{fqname} from #{filename} into #{environment}" } + return result end + rescue TypeLoaderError + # I'm not convinced we should just drop these errors, but this + # preserves existing behaviours. end # Nothing found. return nil diff --git a/lib/puppet/pops/evaluator/callable_signature.rb b/lib/puppet/pops/evaluator/callable_signature.rb index ec6f71322b1..c14fbb9cb71 100644 --- a/lib/puppet/pops/evaluator/callable_signature.rb +++ b/lib/puppet/pops/evaluator/callable_signature.rb @@ -22,7 +22,7 @@ class Puppet::Pops::Evaluator::CallableSignature # @api public # def parameter_names - raise NotImplementedError.new + raise NotImplementedError end # Returns a PCallableType with the type information, required and optional count, and type information about @@ -34,7 +34,7 @@ def parameter_names # @api public # def type - raise NotImplementedError.new + raise NotImplementedError end # Returns the expected type for an optional block. The type may be nil, which means that the callable does @@ -56,7 +56,7 @@ def block_type # @api public # def block_name - raise NotImplementedError.new + raise NotImplementedError end # Returns a range indicating the optionality of a block. One of [0,0] (does not accept block), [0,1] (optional @@ -87,7 +87,7 @@ def args_range # @api public # def last_captures_rest? - raise NotImplementedError.new + raise NotImplementedError end # Returns true if the given x is infinity diff --git a/lib/puppet/pops/evaluator/compare_operator.rb b/lib/puppet/pops/evaluator/compare_operator.rb index a43c58c6834..c5f0dff3e60 100644 --- a/lib/puppet/pops/evaluator/compare_operator.rb +++ b/lib/puppet/pops/evaluator/compare_operator.rb @@ -50,7 +50,7 @@ def include?(a, b, scope) def cmp_String(a, b) return a.casecmp(b) if b.is_a?(String) - raise ArgumentError.new(_("A String is not comparable to a non String")) + raise ArgumentError, _("A String is not comparable to a non String") end # Equality is case independent. @@ -67,7 +67,7 @@ def cmp_Numeric(a, b) when Time::Timespan, Time::Timestamp -(b <=> a) # compare other way and invert result else - raise ArgumentError.new(_("A Numeric is not comparable to non Numeric")) + raise ArgumentError, _("A Numeric is not comparable to non Numeric") end end @@ -97,30 +97,30 @@ def cmp_Symbol(a, b) if b.is_a?(Symbol) a <=> b else - raise ArgumentError.new(_("Symbol not comparable to non Symbol")) + raise ArgumentError, _("Symbol not comparable to non Symbol") end end def cmp_Timespan(a, b) - raise ArgumentError.new(_('Timespans are only comparable to Timespans, Integers, and Floats')) unless b.is_a?(Time::Timespan) || b.is_a?(Integer) || b.is_a?(Float) + raise ArgumentError, _('Timespans are only comparable to Timespans, Integers, and Floats') unless b.is_a?(Time::Timespan) || b.is_a?(Integer) || b.is_a?(Float) a <=> b end def cmp_Timestamp(a, b) - raise ArgumentError.new(_('Timestamps are only comparable to Timestamps, Integers, and Floats')) unless b.is_a?(Time::Timestamp) || b.is_a?(Integer) || b.is_a?(Float) + raise ArgumentError, _('Timestamps are only comparable to Timestamps, Integers, and Floats') unless b.is_a?(Time::Timestamp) || b.is_a?(Integer) || b.is_a?(Float) a <=> b end def cmp_Version(a, b) - raise ArgumentError.new(_('Versions not comparable to non Versions')) unless b.is_a?(SemanticPuppet::Version) + raise ArgumentError, _('Versions not comparable to non Versions') unless b.is_a?(SemanticPuppet::Version) a <=> b end def cmp_Object(a, b) - raise ArgumentError.new(_('Only Strings, Numbers, Timespans, Timestamps, and Versions are comparable')) + raise ArgumentError, _('Only Strings, Numbers, Timespans, Timestamps, and Versions are comparable') end def equals_Object(a, b) diff --git a/lib/puppet/pops/evaluator/evaluator_impl.rb b/lib/puppet/pops/evaluator/evaluator_impl.rb index 05ea42ba2d9..2b9baa6e64f 100644 --- a/lib/puppet/pops/evaluator/evaluator_impl.rb +++ b/lib/puppet/pops/evaluator/evaluator_impl.rb @@ -74,46 +74,44 @@ def type_calculator # @api public # def evaluate(target, scope) - begin - @@eval_visitor.visit_this_1(self, target, scope) - rescue SemanticError => e - # A raised issue may not know the semantic target, use errors call stack, but fill in the - # rest from a supplied semantic object, or the target instruction if there is not semantic - # object. - # - fail(e.issue, e.semantic || target, e.options, e) - rescue Puppet::PreformattedError => e - # Already formatted with location information, and with the wanted call stack. - # Note this is currently a specialized ParseError, so rescue-order is important - # - raise e - rescue Puppet::ParseError => e - # ParseError may be raised in ruby code without knowing the location - # in puppet code. - # Accept a ParseError that has file or line information available - # as an error that should be used verbatim. (Tests typically run without - # setting a file name). - # ParseError can supply an original - it is impossible to determine which - # call stack that should be propagated, using the ParseError's backtrace. - # - if e.file || e.line - raise e - else - # Since it had no location information, treat it as user intended a general purpose - # error. Pass on its call stack. - fail(Issues::RUNTIME_ERROR, target, { :detail => e.message }, e) - end - rescue Puppet::Error => e - # PuppetError has the ability to wrap an exception, if so, use the wrapped exception's - # call stack instead - fail(Issues::RUNTIME_ERROR, target, { :detail => e.message }, e.original || e) - rescue StopIteration => e - # Ensure these are not rescued as StandardError + @@eval_visitor.visit_this_1(self, target, scope) + rescue SemanticError => e + # A raised issue may not know the semantic target, use errors call stack, but fill in the + # rest from a supplied semantic object, or the target instruction if there is not semantic + # object. + # + fail(e.issue, e.semantic || target, e.options, e) + rescue Puppet::PreformattedError => e + # Already formatted with location information, and with the wanted call stack. + # Note this is currently a specialized ParseError, so rescue-order is important + # + raise e + rescue Puppet::ParseError => e + # ParseError may be raised in ruby code without knowing the location + # in puppet code. + # Accept a ParseError that has file or line information available + # as an error that should be used verbatim. (Tests typically run without + # setting a file name). + # ParseError can supply an original - it is impossible to determine which + # call stack that should be propagated, using the ParseError's backtrace. + # + if e.file || e.line raise e - rescue StandardError => e - # All other errors, use its message and call stack + else + # Since it had no location information, treat it as user intended a general purpose + # error. Pass on its call stack. fail(Issues::RUNTIME_ERROR, target, { :detail => e.message }, e) end + rescue Puppet::Error => e + # PuppetError has the ability to wrap an exception, if so, use the wrapped exception's + # call stack instead + fail(Issues::RUNTIME_ERROR, target, { :detail => e.message }, e.original || e) + rescue StopIteration => e + # Ensure these are not rescued as StandardError + raise e + rescue StandardError => e + # All other errors, use its message and call stack + fail(Issues::RUNTIME_ERROR, target, { :detail => e.message }, e) end # Assigns the given _value_ to the given _target_. The additional argument _o_ is the instruction that @@ -746,17 +744,15 @@ def eval_Definition(o, scope) end def eval_Program(o, scope) - begin - file = o.locator.file - line = 0 - # Add stack frame for "top scope" logic. See Puppet::Pops::PuppetStack - return Puppet::Pops::PuppetStack.stack(file, line, self, 'evaluate', [o.body, scope]) - # evaluate(o.body, scope) - rescue Puppet::Pops::Evaluator::PuppetStopIteration => ex - # breaking out of a file level program is not allowed - # TRANSLATOR break() is a method that should not be translated - raise Puppet::ParseError.new(_("break() from context where this is illegal"), ex.file, ex.line) - end + file = o.locator.file + line = 0 + # Add stack frame for "top scope" logic. See Puppet::Pops::PuppetStack + return Puppet::Pops::PuppetStack.stack(file, line, self, 'evaluate', [o.body, scope]) + # evaluate(o.body, scope) + rescue Puppet::Pops::Evaluator::PuppetStopIteration => ex + # breaking out of a file level program is not allowed + # TRANSLATOR break() is a method that should not be translated + raise Puppet::ParseError.new(_("break() from context where this is illegal"), ex.file, ex.line) end # Produces Array[PAnyType], an array of resource references @@ -1236,15 +1232,15 @@ def concatenate(x, y) Hash[*y] end else - raise ArgumentError.new(_('Can only append Array or Hash to a Hash')) + raise ArgumentError, _('Can only append Array or Hash to a Hash') end x.merge y # new hash with overwrite when URI - raise ArgumentError.new(_('An URI can only be merged with an URI or String')) unless y.is_a?(String) || y.is_a?(URI) + raise ArgumentError, _('An URI can only be merged with an URI or String') unless y.is_a?(String) || y.is_a?(URI) x + y when Types::PBinaryType::Binary - raise ArgumentError.new(_('Can only append Binary to a Binary')) unless y.is_a?(Types::PBinaryType::Binary) + raise ArgumentError, _('Can only append Binary to a Binary') unless y.is_a?(Types::PBinaryType::Binary) Types::PBinaryType::Binary.from_binary_string(x.binary_buffer + y.binary_buffer) else @@ -1276,7 +1272,7 @@ def delete(x, y) end y.each { |e| result.delete(e) } else - raise ArgumentError.new(_("Can only delete from an Array or Hash.")) + raise ArgumentError, _("Can only delete from an Array or Hash.") end result end diff --git a/lib/puppet/pops/evaluator/relationship_operator.rb b/lib/puppet/pops/evaluator/relationship_operator.rb index fdb36fa4cd5..1bd753021ab 100644 --- a/lib/puppet/pops/evaluator/relationship_operator.rb +++ b/lib/puppet/pops/evaluator/relationship_operator.rb @@ -47,7 +47,7 @@ def transform(o, scope) # Catch all non transformable objects # @api private def transform_Object(o, scope) - raise IllegalRelationshipOperandError.new(o) + raise IllegalRelationshipOperandError, o end # A Resource is by definition a Catalog type, but of 3.x type @@ -98,7 +98,7 @@ def transform_Array(o, scope) # def assert_catalog_type(o, scope) unless @type_calculator.assignable?(@catalog_type, o) - raise NotCatalogTypeError.new(o) + raise NotCatalogTypeError, o end # TODO must check if this is an abstract PResourceType (i.e. without a type_name) - which should fail ? diff --git a/lib/puppet/pops/evaluator/runtime3_support.rb b/lib/puppet/pops/evaluator/runtime3_support.rb index 99ffeba5d07..7e6a6c316bb 100644 --- a/lib/puppet/pops/evaluator/runtime3_support.rb +++ b/lib/puppet/pops/evaluator/runtime3_support.rb @@ -37,7 +37,7 @@ def optionally_fail(issue, semantic, options = {}, except = nil) if except.nil? && diagnostic_producer.severity_producer[issue] == :error # Want a stacktrace, and it must be passed as an exception begin - raise EvaluationError.new() + raise EvaluationError rescue EvaluationError => e except = e end diff --git a/lib/puppet/pops/functions/function.rb b/lib/puppet/pops/functions/function.rb index da8ca1ebe97..c8cf5a574be 100644 --- a/lib/puppet/pops/functions/function.rb +++ b/lib/puppet/pops/functions/function.rb @@ -40,23 +40,21 @@ def initialize(closure_scope, loader) # # @api public def call(scope, *args, &block) + result = catch(:return) do + return self.class.dispatcher.dispatch(self, scope, args, &block) + end + return result.value + rescue Puppet::Pops::Evaluator::Next => jumper begin - result = catch(:return) do - return self.class.dispatcher.dispatch(self, scope, args, &block) - end - return result.value - rescue Puppet::Pops::Evaluator::Next => jumper - begin - throw :next, jumper.value - rescue Puppet::Parser::Scope::UNCAUGHT_THROW_EXCEPTION - raise Puppet::ParseError.new("next() from context where this is illegal", jumper.file, jumper.line) - end - rescue Puppet::Pops::Evaluator::Return => jumper - begin - throw :return, jumper - rescue Puppet::Parser::Scope::UNCAUGHT_THROW_EXCEPTION - raise Puppet::ParseError.new("return() from context where this is illegal", jumper.file, jumper.line) - end + throw :next, jumper.value + rescue Puppet::Parser::Scope::UNCAUGHT_THROW_EXCEPTION + raise Puppet::ParseError.new("next() from context where this is illegal", jumper.file, jumper.line) + end + rescue Puppet::Pops::Evaluator::Return => jumper + begin + throw :return, jumper + rescue Puppet::Parser::Scope::UNCAUGHT_THROW_EXCEPTION + raise Puppet::ParseError.new("return() from context where this is illegal", jumper.file, jumper.line) end end diff --git a/lib/puppet/pops/issue_reporter.rb b/lib/puppet/pops/issue_reporter.rb index 199db003ab4..e8b6bfe26f5 100644 --- a/lib/puppet/pops/issue_reporter.rb +++ b/lib/puppet/pops/issue_reporter.rb @@ -48,7 +48,7 @@ def self.assert_and_report(acceptor, options) errors = acceptor.errors if errors.size > 0 unless emit_errors - raise emit_exception.new(emit_message) + raise emit_exception, emit_message end formatter = Validation::DiagnosticFormatterPuppetStyle.new diff --git a/lib/puppet/pops/loader/loader.rb b/lib/puppet/pops/loader/loader.rb index 48571382535..acdc94c9219 100644 --- a/lib/puppet/pops/loader/loader.rb +++ b/lib/puppet/pops/loader/loader.rb @@ -164,7 +164,7 @@ def synchronize(&block) # @api private # def set_entry(type, name, value, origin = nil) - raise NotImplementedError.new + raise NotImplementedError end # Produces a NamedEntry if a value is bound to the given name, or nil if nothing is bound. @@ -175,7 +175,7 @@ def set_entry(type, name, value, origin = nil) # @api private # def get_entry(typed_name) - raise NotImplementedError.new + raise NotImplementedError end # A loader is by default a loader for all kinds of loadables. An implementation may override diff --git a/lib/puppet/pops/loader/loader_paths.rb b/lib/puppet/pops/loader/loader_paths.rb index 8a8db8f37a6..d349cf58178 100644 --- a/lib/puppet/pops/loader/loader_paths.rb +++ b/lib/puppet/pops/loader/loader_paths.rb @@ -108,11 +108,11 @@ def valid_name?(typed_name) end def relative_path - raise NotImplementedError.new + raise NotImplementedError end def instantiator - raise NotImplementedError.new + raise NotImplementedError end end diff --git a/lib/puppet/pops/loader/module_loaders.rb b/lib/puppet/pops/loader/module_loaders.rb index c7fae73ee01..1a3dd37bb31 100644 --- a/lib/puppet/pops/loader/module_loaders.rb +++ b/lib/puppet/pops/loader/module_loaders.rb @@ -297,7 +297,7 @@ def instantiate(smart_path, typed_name, origin) # @return [Boolean] true if there is content in the directory appointed by the relative path # def meaningful_to_search?(smart_path) - raise NotImplementedError.new + raise NotImplementedError end # Abstract method that subclasses override to answer if the given relative path exists, and if so returns that path @@ -306,7 +306,7 @@ def meaningful_to_search?(smart_path) # @return [String, nil] the found path or nil if no such path was found # def existing_path(resolved_path) - raise NotImplementedError.new + raise NotImplementedError end # Abstract method that subclasses override to return an array of paths that may be associated with the resolved path. @@ -315,7 +315,7 @@ def existing_path(resolved_path) # @return [Array] # def candidate_paths(resolved_path) - raise NotImplementedError.new + raise NotImplementedError end # Abstract method that subclasses override to produce the content of the effective path. @@ -325,7 +325,7 @@ def candidate_paths(resolved_path) # @return [String] the content of the file # def get_contents(effective_path) - raise NotImplementedError.new + raise NotImplementedError end # Abstract method that subclasses override to produce a source reference String used to identify the @@ -335,7 +335,7 @@ def get_contents(effective_path) # @return [String] a reference to the source file (in file system, zip file, or elsewhere). # def get_source_ref(relative_path) - raise NotImplementedError.new + raise NotImplementedError end # Answers the question if this loader represents a global component (true for resource type loader and environment loader) @@ -370,7 +370,7 @@ def private_loader # @param smart_path [SmartPath] the path to find relative paths for # @return [Array] found paths def relative_paths(smart_path) - raise NotImplementedError.new + raise NotImplementedError end private diff --git a/lib/puppet/pops/loaders.rb b/lib/puppet/pops/loaders.rb index b53ab9a6174..5699bf41006 100644 --- a/lib/puppet/pops/loaders.rb +++ b/lib/puppet/pops/loaders.rb @@ -37,7 +37,7 @@ def self.new(environment, for_agent = false, load_from_pcore = true) def initialize(environment, for_agent, load_from_pcore = true) # Protect against environment havoc - raise ArgumentError.new(_("Attempt to redefine already initialized loaders for environment")) unless environment.loaders.nil? + raise ArgumentError, _("Attempt to redefine already initialized loaders for environment") unless environment.loaders.nil? environment.loaders = self @environment = environment diff --git a/lib/puppet/pops/lookup/lookup_adapter.rb b/lib/puppet/pops/lookup/lookup_adapter.rb index 5cd09856bb1..972abe472b6 100644 --- a/lib/puppet/pops/lookup/lookup_adapter.rb +++ b/lib/puppet/pops/lookup/lookup_adapter.rb @@ -297,18 +297,18 @@ def set_global_only def validate_lookup_options(options, module_name) return nil if options.nil? - raise Puppet::DataBinding::LookupError.new(_("value of %{opts} must be a hash") % { opts: LOOKUP_OPTIONS }) unless options.is_a?(Hash) + raise Puppet::DataBinding::LookupError, _("value of %{opts} must be a hash") % { opts: LOOKUP_OPTIONS } unless options.is_a?(Hash) return options if module_name.nil? pfx = "#{module_name}::" options.each_pair do |key, _value| if key.start_with?(LOOKUP_OPTIONS_PATTERN_START) unless key[1..pfx.length] == pfx - raise Puppet::DataBinding::LookupError.new(_("all %{opts} patterns must match a key starting with module name '%{module_name}'") % { opts: LOOKUP_OPTIONS, module_name: module_name }) + raise Puppet::DataBinding::LookupError, _("all %{opts} patterns must match a key starting with module name '%{module_name}'") % { opts: LOOKUP_OPTIONS, module_name: module_name } end else unless key.start_with?(pfx) - raise Puppet::DataBinding::LookupError.new(_("all %{opts} keys must start with module name '%{module_name}'") % { opts: LOOKUP_OPTIONS, module_name: module_name }) + raise Puppet::DataBinding::LookupError, _("all %{opts} keys must start with module name '%{module_name}'") % { opts: LOOKUP_OPTIONS, module_name: module_name } end end end @@ -514,7 +514,7 @@ def initialize_env_provider(lookup_invocation) ep.config = HieraConfigV5.v4_function_config(env_path, 'environment::data', ep) ep else - raise Puppet::Error.new(_("Environment '%{env}', cannot find environment_data_provider '%{provider}'") % { env: environment.name, provider: provider_name }) + raise Puppet::Error, _("Environment '%{env}', cannot find environment_data_provider '%{provider}'") % { env: environment.name, provider: provider_name } end end end diff --git a/lib/puppet/pops/model/ast_transformer.rb b/lib/puppet/pops/model/ast_transformer.rb index ecf4fe7b032..9ccb275b877 100644 --- a/lib/puppet/pops/model/ast_transformer.rb +++ b/lib/puppet/pops/model/ast_transformer.rb @@ -61,14 +61,12 @@ def merge_location(hash, o) # Transforms pops expressions into AST 3.1 statements/expressions def transform(o) - begin - @@transform_visitor.visit_this_0(self, o) - rescue StandardError => e - loc_data = {} - merge_location(loc_data, o) - raise Puppet::ParseError.new(_("Error while transforming to Puppet 3 AST: %{message}") % { message: e.message }, - loc_data[:file], loc_data[:line], loc_data[:pos], e) - end + @@transform_visitor.visit_this_0(self, o) + rescue StandardError => e + loc_data = {} + merge_location(loc_data, o) + raise Puppet::ParseError.new(_("Error while transforming to Puppet 3 AST: %{message}") % { message: e.message }, + loc_data[:file], loc_data[:line], loc_data[:pos], e) end # Transforms pops expressions into AST 3.1 query expressions diff --git a/lib/puppet/pops/model/factory.rb b/lib/puppet/pops/model/factory.rb index 62e9353c3a2..081c99bebfb 100644 --- a/lib/puppet/pops/model/factory.rb +++ b/lib/puppet/pops/model/factory.rb @@ -1052,8 +1052,7 @@ def self.transform_resource_wo_title(left, attribute_ops, lbrace_token, rbrace_t end a_hash = HASH(keyed_entries) a_hash.record_position(left[KEY_LOCATOR], lbrace_token, rbrace_token) - result = block_or_expression(transform_calls([left, a_hash])) - result + block_or_expression(transform_calls([left, a_hash])) end def interpolate_Factory(c) diff --git a/lib/puppet/pops/parser/parser_support.rb b/lib/puppet/pops/parser/parser_support.rb index f5141714252..0f6bdca2687 100644 --- a/lib/puppet/pops/parser/parser_support.rb +++ b/lib/puppet/pops/parser/parser_support.rb @@ -193,16 +193,15 @@ def transform_calls(expressions) # where the "10, notice" forms an argument list. The parser builds an Array with the expressions and includes # the comma tokens to enable the error to be reported against the first comma. # - begin - Factory.transform_calls(expressions) - rescue Factory::ArgsToNonCallError => e - # e.args[1] is the first comma token in the list - # e.name_expr is the function name expression - if e.name_expr.is_a?(Factory) && e.name_expr.model_class <= Model::QualifiedName - error(e.args[1], _("attempt to pass argument list to the function '%{name}' which cannot be called without parentheses") % { name: e.name_expr['value'] }) - else - error(e.args[1], _("illegal comma separated argument list")) - end + + Factory.transform_calls(expressions) + rescue Factory::ArgsToNonCallError => e + # e.args[1] is the first comma token in the list + # e.name_expr is the function name expression + if e.name_expr.is_a?(Factory) && e.name_expr.model_class <= Model::QualifiedName + error(e.args[1], _("attempt to pass argument list to the function '%{name}' which cannot be called without parentheses") % { name: e.name_expr['value'] }) + else + error(e.args[1], _("illegal comma separated argument list")) end end @@ -229,8 +228,7 @@ def create_empty_program _, token = @lexer.emit_completed([:NOOP, '', 0], locator.string.bytesize) loc(no_op, token) # Program with a Noop - program = Factory.PROGRAM(no_op, [], locator) - program + Factory.PROGRAM(no_op, [], locator) end # Performs the parsing and returns the resulting model. diff --git a/lib/puppet/pops/types/class_loader.rb b/lib/puppet/pops/types/class_loader.rb index 1726a8fbdec..bddf002bd95 100644 --- a/lib/puppet/pops/types/class_loader.rb +++ b/lib/puppet/pops/types/class_loader.rb @@ -37,7 +37,7 @@ def self.provide(name) def self.provide_from_type(type) case type when PRuntimeType - raise ArgumentError.new("Only Runtime type 'ruby' is supported, got #{type.runtime}") unless type.runtime == :ruby + raise ArgumentError, "Only Runtime type 'ruby' is supported, got #{type.runtime}" unless type.runtime == :ruby provide_from_string(type.runtime_type_name) @@ -105,11 +105,9 @@ def self.provide_from_name_path(name, name_path) def self.find_class(name_path) name_path.reduce(Object) do |ns, name| - begin - ns.const_get(name, false) # don't search ancestors - rescue NameError - return nil - end + ns.const_get(name, false) # don't search ancestors + rescue NameError + return nil end end private_class_method :find_class diff --git a/lib/puppet/pops/types/p_init_type.rb b/lib/puppet/pops/types/p_init_type.rb index 25c522ef4c3..fd2b9ced42f 100644 --- a/lib/puppet/pops/types/p_init_type.rb +++ b/lib/puppet/pops/types/p_init_type.rb @@ -152,7 +152,7 @@ def assert_initialized # The Optional is the same as Variant[T,Undef]. # The NotUndef is not meaningful to create instances of if @type.instance_of?(PInitType) || @type.instance_of?(POptionalType) || @type.instance_of?(PNotUndefType) - raise ArgumentError.new + raise ArgumentError end new_func = @type.new_function diff --git a/lib/puppet/pops/types/p_object_type_extension.rb b/lib/puppet/pops/types/p_object_type_extension.rb index dfcb5550797..c74cb424e76 100644 --- a/lib/puppet/pops/types/p_object_type_extension.rb +++ b/lib/puppet/pops/types/p_object_type_extension.rb @@ -209,12 +209,10 @@ def test_assignable?(param_values, guard) def test_instance?(o, guard) eval = Parser::EvaluatingParser.singleton.evaluator @parameters.keys.all? do |pn| - begin - m = o.public_method(pn) - m.arity == 0 ? eval.match?(m.call, @parameters[pn]) : false - rescue NameError - false - end + m = o.public_method(pn) + m.arity == 0 ? eval.match?(m.call, @parameters[pn]) : false + rescue NameError + false end end diff --git a/lib/puppet/pops/types/tree_iterators.rb b/lib/puppet/pops/types/tree_iterators.rb index 76a51c977de..68aea19f7f6 100644 --- a/lib/puppet/pops/types/tree_iterators.rb +++ b/lib/puppet/pops/types/tree_iterators.rb @@ -108,12 +108,10 @@ def indexer_on(val) private :indexer_on def has_next?(iterator) - begin - iterator.peek - true - rescue StopIteration - false - end + iterator.peek + true + rescue StopIteration + false end private :has_next? diff --git a/lib/puppet/pops/types/type_formatter.rb b/lib/puppet/pops/types/type_formatter.rb index 47505360a5d..490151710e0 100644 --- a/lib/puppet/pops/types/type_formatter.rb +++ b/lib/puppet/pops/types/type_formatter.rb @@ -687,37 +687,35 @@ def range_array_part(t) end def append_object_hash(hash) - begin - @expanded = false - append_array('Object') do - append_hash(hash, proc { |k| @bld << symbolic_key(k) }) do |k, v| - case k - when KEY_ATTRIBUTES, KEY_FUNCTIONS - # Types might need to be output as type references - append_hash(v) do |_, fv| - if fv.is_a?(Hash) - append_hash(fv, proc { |fak| @bld << symbolic_key(fak) }) do |fak, fav| - case fak - when KEY_KIND - @bld << fav - else - append_string(fav) - end + @expanded = false + append_array('Object') do + append_hash(hash, proc { |k| @bld << symbolic_key(k) }) do |k, v| + case k + when KEY_ATTRIBUTES, KEY_FUNCTIONS + # Types might need to be output as type references + append_hash(v) do |_, fv| + if fv.is_a?(Hash) + append_hash(fv, proc { |fak| @bld << symbolic_key(fak) }) do |fak, fav| + case fak + when KEY_KIND + @bld << fav + else + append_string(fav) end - else - append_string(fv) end + else + append_string(fv) end - when KEY_EQUALITY - append_array('') { append_strings(v) } if v.is_a?(Array) - else - append_string(v) end + when KEY_EQUALITY + append_array('') { append_strings(v) } if v.is_a?(Array) + else + append_string(v) end end - ensure - @expanded = true end + ensure + @expanded = true end def append_elements(array, to_be_continued = false) diff --git a/lib/puppet/pops/types/type_mismatch_describer.rb b/lib/puppet/pops/types/type_mismatch_describer.rb index 72c91a948da..c32610a488f 100644 --- a/lib/puppet/pops/types/type_mismatch_describer.rb +++ b/lib/puppet/pops/types/type_mismatch_describer.rb @@ -533,10 +533,10 @@ def validate_parameters(subject, params_struct, given_hash, missing_ok = false, when 0 # do nothing when 1 - raise Puppet::ParseError.new("#{subject}:#{errors[0].format}") + raise Puppet::ParseError, "#{subject}:#{errors[0].format}" else errors_str = errors.map { |error| error.format }.join("\n ") - raise Puppet::ParseError.new("#{subject}:\n #{errors_str}") + raise Puppet::ParseError, "#{subject}:\n #{errors_str}" end end @@ -574,10 +574,10 @@ def validate_default_parameter(subject, param_name, param_type, value, tense = : when 0 # do nothing when 1 - raise Puppet::ParseError.new("#{subject}:#{errors[0].format}") + raise Puppet::ParseError, "#{subject}:#{errors[0].format}" else errors_str = errors.map { |error| error.format }.join("\n ") - raise Puppet::ParseError.new("#{subject}:\n #{errors_str}") + raise Puppet::ParseError, "#{subject}:\n #{errors_str}" end end end diff --git a/lib/puppet/pops/types/types.rb b/lib/puppet/pops/types/types.rb index 2a003c61d8f..9d3bbafe61d 100644 --- a/lib/puppet/pops/types/types.rb +++ b/lib/puppet/pops/types/types.rb @@ -342,7 +342,7 @@ def new_function # @raises ArgumentError # def self.new_function(type) - raise ArgumentError.new("Creation of new instance of type '#{type}' is not supported") + raise ArgumentError, "Creation of new instance of type '#{type}' is not supported" end # Answers the question if instances of this type can represent themselves as a string that @@ -919,9 +919,9 @@ def from_convertible(from) Puppet::Pops::Utils.to_n(from) rescue TypeError => e - raise TypeConversionError.new(e.message) + raise TypeConversionError, e.message rescue ArgumentError => e - raise TypeConversionError.new(e.message) + raise TypeConversionError, e.message end end end @@ -1146,7 +1146,7 @@ def from_convertible(from, radix) begin radix == :default ? Integer(from) : Integer(from, radix) rescue TypeError => e - raise TypeConversionError.new(e.message) + raise TypeConversionError, e.message rescue ArgumentError => e # Test for special case where there is whitespace between sign and number match = Patterns::WS_BETWEEN_SIGN_AND_NUMBER.match(from) @@ -1158,7 +1158,7 @@ def from_convertible(from, radix) # Ignored to retain original error end end - raise TypeConversionError.new(e.message) + raise TypeConversionError, e.message end end end @@ -1188,7 +1188,7 @@ def assert_radix(radix) when 2, 8, 10, 16 # do nothing else - raise ArgumentError.new(_("Illegal radix: %{radix}, expected 2, 8, 10, 16, or default") % { radix: radix }) + raise ArgumentError, _("Illegal radix: %{radix}, expected 2, 8, 10, 16, or default") % { radix: radix } end radix end @@ -1281,7 +1281,7 @@ def from_convertible(from) end Float(from) rescue TypeError => e - raise TypeConversionError.new(e.message) + raise TypeConversionError, e.message rescue ArgumentError => e # Test for special case where there is whitespace between sign and number match = Patterns::WS_BETWEEN_SIGN_AND_NUMBER.match(from) @@ -1293,7 +1293,7 @@ def from_convertible(from) # Ignored to retain original error end end - raise TypeConversionError.new(e.message) + raise TypeConversionError, e.message end end end @@ -2846,7 +2846,7 @@ def from_array(from) {} else unless from.size.even? - raise TypeConversionError.new(_('odd number of arguments for Hash')) + raise TypeConversionError, _('odd number of arguments for Hash') end Hash[*from] @@ -2858,7 +2858,7 @@ def from_array(from) Hash[*Iterable.on(from).to_a] else t = TypeCalculator.singleton.infer(from).generalize - raise TypeConversionError.new(_("Value of type %{type} cannot be converted to Hash") % { type: t }) + raise TypeConversionError, _("Value of type %{type} cannot be converted to Hash") % { type: t } end end end diff --git a/lib/puppet/pops/utils.rb b/lib/puppet/pops/utils.rb index 56d44bf6992..e09552aa982 100644 --- a/lib/puppet/pops/utils.rb +++ b/lib/puppet/pops/utils.rb @@ -45,37 +45,35 @@ def self.match_to_fp(match) # @api public # def self.to_n_with_radix o - begin - case o - when String - match = Patterns::NUMERIC.match(relativize_name(o)) - if !match - nil - elsif match[5].to_s.length > 0 - fp_value = match_to_fp(match) - fp_value.nil? ? nil : [fp_value, 10] - else - # Set radix (default is decimal == 10) - radix = 10 - if match[3].to_s.length > 0 - radix = 16 - elsif match[4].to_s.length > 1 && match[4][0, 1] == '0' - radix = 8 - end - # Ruby 1.8.7 does not have a second argument to Kernel method that creates an - # integer from a string, it relies on the prefix 0x, 0X, 0 (and unsupported in puppet binary 'b') - # We have the correct string here, match[2] is safe to parse without passing on radix - match[1] == '-' ? [-Integer(match[2]), radix] : [Integer(match[2]), radix] - end - when Numeric - # Impossible to calculate radix, assume decimal - [o, 10] - else + case o + when String + match = Patterns::NUMERIC.match(relativize_name(o)) + if !match nil + elsif match[5].to_s.length > 0 + fp_value = match_to_fp(match) + fp_value.nil? ? nil : [fp_value, 10] + else + # Set radix (default is decimal == 10) + radix = 10 + if match[3].to_s.length > 0 + radix = 16 + elsif match[4].to_s.length > 1 && match[4][0, 1] == '0' + radix = 8 + end + # Ruby 1.8.7 does not have a second argument to Kernel method that creates an + # integer from a string, it relies on the prefix 0x, 0X, 0 (and unsupported in puppet binary 'b') + # We have the correct string here, match[2] is safe to parse without passing on radix + match[1] == '-' ? [-Integer(match[2]), radix] : [Integer(match[2]), radix] end - rescue ArgumentError + when Numeric + # Impossible to calculate radix, assume decimal + [o, 10] + else nil end + rescue ArgumentError + nil end # To Numeric (or already numeric) @@ -84,25 +82,23 @@ def self.to_n_with_radix o # A leading '::' is accepted (and ignored) # def self.to_n o - begin - case o - when String - match = Patterns::NUMERIC.match(relativize_name(o)) - if !match - nil - elsif match[5].to_s.length > 0 - match_to_fp(match) - else - match[1] == '-' ? -Integer(match[2]) : Integer(match[2]) - end - when Numeric - o - else + case o + when String + match = Patterns::NUMERIC.match(relativize_name(o)) + if !match nil + elsif match[5].to_s.length > 0 + match_to_fp(match) + else + match[1] == '-' ? -Integer(match[2]) : Integer(match[2]) end - rescue ArgumentError + when Numeric + o + else nil end + rescue ArgumentError + nil end # is the name absolute (i.e. starts with ::) diff --git a/lib/puppet/pops/validation.rb b/lib/puppet/pops/validation.rb index 44f835e7036..23771708211 100644 --- a/lib/puppet/pops/validation.rb +++ b/lib/puppet/pops/validation.rb @@ -126,13 +126,13 @@ def [] issue # def []=(issue, level) unless issue.is_a? Issues::Issue - raise Puppet::DevError.new(_("Attempt to set validation severity for something that is not an Issue. (Got %{issue})") % { issue: issue.class }) + raise Puppet::DevError, _("Attempt to set validation severity for something that is not an Issue. (Got %{issue})") % { issue: issue.class } end unless SEVERITIES[level] - raise Puppet::DevError.new(_("Illegal severity level: %{level} for '%{issue_code}'") % { issue_code: issue.issue_code, level: level }) + raise Puppet::DevError, _("Illegal severity level: %{level} for '%{issue_code}'") % { issue_code: issue.issue_code, level: level } end unless issue.demotable? || level == :error - raise Puppet::DevError.new(_("Attempt to demote the hard issue '%{issue_code}' to %{level}") % { issue_code: issue.issue_code, level: level }) + raise Puppet::DevError, _("Attempt to demote the hard issue '%{issue_code}' to %{level}") % { issue_code: issue.issue_code, level: level } end @severities[issue] = level @@ -153,7 +153,7 @@ def should_report? issue # def assert_issue issue unless issue.is_a? Issues::Issue - raise Puppet::DevError.new(_("Attempt to get validation severity for something that is not an Issue. (Got %{issue})") % { issue: issue.class }) + raise Puppet::DevError, _("Attempt to get validation severity for something that is not an Issue. (Got %{issue})") % { issue: issue.class } end end end diff --git a/lib/puppet/pops/visitor.rb b/lib/puppet/pops/visitor.rb index 7dc19375789..c9cbe424fa5 100644 --- a/lib/puppet/pops/visitor.rb +++ b/lib/puppet/pops/visitor.rb @@ -16,8 +16,8 @@ class Visitor attr_reader :receiver, :message, :min_args, :max_args, :cache def initialize(receiver, message, min_args = 0, max_args = nil) - raise ArgumentError.new("min_args must be >= 0") if min_args < 0 - raise ArgumentError.new("max_args must be >= min_args or nil") if max_args && max_args < min_args + raise ArgumentError, "min_args must be >= 0" if min_args < 0 + raise ArgumentError, "max_args must be >= min_args or nil" if max_args && max_args < min_args @receiver = receiver @message = message diff --git a/lib/puppet/property.rb b/lib/puppet/property.rb index a89aecc37e6..41dc566423d 100644 --- a/lib/puppet/property.rb +++ b/lib/puppet/property.rb @@ -203,21 +203,19 @@ def call_provider(value) # @raise [Puppet::DevError] if there were issues formatting the message # def change_to_s(current_value, newvalue) - begin - if current_value == :absent - return "defined '#{name}' as #{should_to_s(newvalue)}" - elsif newvalue == :absent or newvalue == [:absent] - return "undefined '#{name}' from #{is_to_s(current_value)}" - else - return "#{name} changed #{is_to_s(current_value)} to #{should_to_s(newvalue)}" - end - rescue Puppet::Error - raise - rescue => detail - message = _("Could not convert change '%{name}' to string: %{detail}") % { name: name, detail: detail } - Puppet.log_exception(detail, message) - raise Puppet::DevError, message, detail.backtrace + if current_value == :absent + return "defined '#{name}' as #{should_to_s(newvalue)}" + elsif newvalue == :absent or newvalue == [:absent] + return "undefined '#{name}' from #{is_to_s(current_value)}" + else + return "#{name} changed #{is_to_s(current_value)} to #{should_to_s(newvalue)}" end + rescue Puppet::Error + raise + rescue => detail + message = _("Could not convert change '%{name}' to string: %{detail}") % { name: name, detail: detail } + Puppet.log_exception(detail, message) + raise Puppet::DevError, message, detail.backtrace end # Produces the name of the event to use to describe a change of this property's value. diff --git a/lib/puppet/property/ensure.rb b/lib/puppet/property/ensure.rb index 4ca3a23469c..18c0a4fc04a 100644 --- a/lib/puppet/property/ensure.rb +++ b/lib/puppet/property/ensure.rb @@ -52,19 +52,17 @@ def self.inherited(sub) end def change_to_s(currentvalue, newvalue) - begin - if currentvalue == :absent || currentvalue.nil? - return _("created") - elsif newvalue == :absent - return _("removed") - else - return _('%{name} changed %{is} to %{should}') % { name: name, is: is_to_s(currentvalue), should: should_to_s(newvalue) } - end - rescue Puppet::Error - raise - rescue => detail - raise Puppet::DevError, _("Could not convert change %{name} to string: %{detail}") % { name: self.name, detail: detail }, detail.backtrace + if currentvalue == :absent || currentvalue.nil? + return _("created") + elsif newvalue == :absent + return _("removed") + else + return _('%{name} changed %{is} to %{should}') % { name: name, is: is_to_s(currentvalue), should: should_to_s(newvalue) } end + rescue Puppet::Error + raise + rescue => detail + raise Puppet::DevError, _("Could not convert change %{name} to string: %{detail}") % { name: self.name, detail: detail }, detail.backtrace end # Retrieves the _is_ value for the ensure property. diff --git a/lib/puppet/provider/file/posix.rb b/lib/puppet/provider/file/posix.rb index d33a4d6f75a..1fd5e1498d3 100644 --- a/lib/puppet/provider/file/posix.rb +++ b/lib/puppet/provider/file/posix.rb @@ -135,12 +135,10 @@ def mode end def mode=(value) - begin - File.chmod(value.to_i(8), resource[:path]) - rescue => detail - error = Puppet::Error.new(_("failed to set mode %{mode} on %{path}: %{message}") % { mode: mode, path: resource[:path], message: detail.message }) - error.set_backtrace detail.backtrace - raise error - end + File.chmod(value.to_i(8), resource[:path]) + rescue => detail + error = Puppet::Error.new(_("failed to set mode %{mode} on %{path}: %{message}") % { mode: mode, path: resource[:path], message: detail.message }) + error.set_backtrace detail.backtrace + raise error end end diff --git a/lib/puppet/provider/file/windows.rb b/lib/puppet/provider/file/windows.rb index 2aaccadd687..ce288675d22 100644 --- a/lib/puppet/provider/file/windows.rb +++ b/lib/puppet/provider/file/windows.rb @@ -43,11 +43,9 @@ def owner end def owner=(should) - begin - set_owner(should, resolved_path) - rescue => detail - raise Puppet::Error, _("Failed to set owner to '%{should}': %{detail}") % { should: should, detail: detail }, detail.backtrace - end + set_owner(should, resolved_path) + rescue => detail + raise Puppet::Error, _("Failed to set owner to '%{should}': %{detail}") % { should: should, detail: detail }, detail.backtrace end def group @@ -57,11 +55,9 @@ def group end def group=(should) - begin - set_group(should, resolved_path) - rescue => detail - raise Puppet::Error, _("Failed to set group to '%{should}': %{detail}") % { should: should, detail: detail }, detail.backtrace - end + set_group(should, resolved_path) + rescue => detail + raise Puppet::Error, _("Failed to set group to '%{should}': %{detail}") % { should: should, detail: detail }, detail.backtrace end def mode @@ -74,15 +70,13 @@ def mode end def mode=(value) - begin - managing_owner = !resource[:owner].nil? - managing_group = !resource[:group].nil? - set_mode(value.to_i(8), resource[:path], true, managing_owner, managing_group) - rescue => detail - error = Puppet::Error.new(_("failed to set mode %{mode} on %{path}: %{message}") % { mode: mode, path: resource[:path], message: detail.message }) - error.set_backtrace detail.backtrace - raise error - end + managing_owner = !resource[:owner].nil? + managing_group = !resource[:group].nil? + set_mode(value.to_i(8), resource[:path], true, managing_owner, managing_group) + rescue => detail + error = Puppet::Error.new(_("failed to set mode %{mode} on %{path}: %{message}") % { mode: mode, path: resource[:path], message: detail.message }) + error.set_backtrace detail.backtrace + raise error end def validate diff --git a/lib/puppet/provider/group/groupadd.rb b/lib/puppet/provider/group/groupadd.rb index b02d8983e54..457056008c1 100644 --- a/lib/puppet/provider/group/groupadd.rb +++ b/lib/puppet/provider/group/groupadd.rb @@ -144,7 +144,7 @@ def findgroup(key, value) unless @groups unless Puppet::FileSystem.exist?(group_file) - raise Puppet::Error.new("Forcelocal set for group resource '#{resource[:name]}', but #{group_file} does not exist") + raise Puppet::Error, "Forcelocal set for group resource '#{resource[:name]}', but #{group_file} does not exist" end @groups = [] diff --git a/lib/puppet/provider/nameservice/directoryservice.rb b/lib/puppet/provider/nameservice/directoryservice.rb index 5a7e1828134..c7bcf5efc21 100644 --- a/lib/puppet/provider/nameservice/directoryservice.rb +++ b/lib/puppet/provider/nameservice/directoryservice.rb @@ -264,8 +264,8 @@ def self.get_password(guid, username) # string. The password_hash provided as a resource attribute is a # hex value. We need to convert the Base64 encoded string to a # hex value and provide it back to Puppet. - password_hash = converted_hash_plist['SALTED-SHA512'].unpack("H*")[0] - password_hash + converted_hash_plist['SALTED-SHA512'].unpack("H*")[0] + end end end diff --git a/lib/puppet/provider/nameservice/objectadd.rb b/lib/puppet/provider/nameservice/objectadd.rb index da8fe0174f1..93a6d42237b 100644 --- a/lib/puppet/provider/nameservice/objectadd.rb +++ b/lib/puppet/provider/nameservice/objectadd.rb @@ -15,10 +15,8 @@ def flag(name) end def posixmethod(name) - name = name.intern if name.is_a? String - method = self.class.option(name, :method) || name - - method + name = name.intern if name.is_a? String + self.class.option(name, :method) || name end end end diff --git a/lib/puppet/provider/nameservice/pw.rb b/lib/puppet/provider/nameservice/pw.rb index 7e6ee8d395f..ae819efe67f 100644 --- a/lib/puppet/provider/nameservice/pw.rb +++ b/lib/puppet/provider/nameservice/pw.rb @@ -9,14 +9,13 @@ def deletecmd end def modifycmd(param, value) - cmd = [ + [ command(:pw), "#{@resource.class.name}mod", @resource[:name], flag(param), munge(param, value) ] - cmd end end end diff --git a/lib/puppet/provider/package/aptitude.rb b/lib/puppet/provider/package/aptitude.rb index c5b173b3c05..161e1cbd709 100644 --- a/lib/puppet/provider/package/aptitude.rb +++ b/lib/puppet/provider/package/aptitude.rb @@ -19,9 +19,7 @@ def aptget(*args) # Yay, stupid aptitude doesn't throw an error when the package is missing. if args.include?(:install) and output.to_s =~ /Couldn't find any package/ - raise Puppet::Error.new( - _("Could not find package %{name}") % { name: self.name } - ) + raise Puppet::Error, _("Could not find package %{name}") % { name: self.name } end end diff --git a/lib/puppet/provider/package/blastwave.rb b/lib/puppet/provider/package/blastwave.rb index 5941c93f44a..9847682dbec 100644 --- a/lib/puppet/provider/package/blastwave.rb +++ b/lib/puppet/provider/package/blastwave.rb @@ -95,11 +95,7 @@ def latest def query hash = self.class.blastlist(:justme => @resource[:name]) - if hash - hash - else - { :ensure => :absent } - end + hash || { :ensure => :absent } end # Remove the old package, and install the new one diff --git a/lib/puppet/provider/package/dpkg.rb b/lib/puppet/provider/package/dpkg.rb index b6b24b73683..dc77635ee2c 100644 --- a/lib/puppet/provider/package/dpkg.rb +++ b/lib/puppet/provider/package/dpkg.rb @@ -159,9 +159,7 @@ def query hash ||= { :ensure => :absent, :status => 'missing', :name => @resource[:name], :error => 'ok' } if hash[:error] != "ok" - raise Puppet::Error.new( - "Package #{hash[:name]}, version #{hash[:ensure]} is in error state: #{hash[:error]}" - ) + raise Puppet::Error, "Package #{hash[:name]}, version #{hash[:ensure]} is in error state: #{hash[:error]}" end hash diff --git a/lib/puppet/provider/package/gem.rb b/lib/puppet/provider/package/gem.rb index 6d8767dee40..c3521eaacc6 100644 --- a/lib/puppet/provider/package/gem.rb +++ b/lib/puppet/provider/package/gem.rb @@ -168,12 +168,10 @@ def insync?(is) end return is.any? do |version| - begin - should_range.include?(GEM_VERSION.parse(version)) - rescue GEM_VERSION::ValidationFailure - Puppet.debug("Cannot parse #{version} as a ruby gem version") - false - end + should_range.include?(GEM_VERSION.parse(version)) + rescue GEM_VERSION::ValidationFailure + Puppet.debug("Cannot parse #{version} as a ruby gem version") + false end end @@ -238,7 +236,7 @@ def install(useversion = true) command_options << uri.path when 'puppet' # we don't support puppet:// URLs (yet) - raise Puppet::Error.new(_("puppet:// URLs are not supported as gem sources")) + raise Puppet::Error, _("puppet:// URLs are not supported as gem sources") else # check whether it's an absolute file path to help Windows out if Puppet::Util.absolute_path?(source) diff --git a/lib/puppet/provider/package/pacman.rb b/lib/puppet/provider/package/pacman.rb index feb9a905732..282fa77b41e 100644 --- a/lib/puppet/provider/package/pacman.rb +++ b/lib/puppet/provider/package/pacman.rb @@ -28,12 +28,10 @@ def self.yaourt? # Checks if a given name is a group def self.group?(name) - begin - !pacman("-Sg", name).empty? - rescue Puppet::ExecutionFailure - # pacman returns an expected non-zero exit code when the name is not a group - false - end + !pacman("-Sg", name).empty? + rescue Puppet::ExecutionFailure + # pacman returns an expected non-zero exit code when the name is not a group + false end # Install a package using 'pacman', or 'yaourt' if available. @@ -74,24 +72,22 @@ def self.instances # returns a hash package => version of installed packages def self.get_installed_packages - begin - packages = {} - execpipe([command(:pacman), "-Q"]) do |pipe| - # pacman -Q output is 'packagename version-rel' - regex = %r{^(\S+)\s(\S+)} - pipe.each_line do |line| - match = regex.match(line) - if match - packages[match.captures[0]] = match.captures[1] - else - warning(_("Failed to match line '%{line}'") % { line: line }) - end + packages = {} + execpipe([command(:pacman), "-Q"]) do |pipe| + # pacman -Q output is 'packagename version-rel' + regex = %r{^(\S+)\s(\S+)} + pipe.each_line do |line| + match = regex.match(line) + if match + packages[match.captures[0]] = match.captures[1] + else + warning(_("Failed to match line '%{line}'") % { line: line }) end end - packages - rescue Puppet::ExecutionFailure - fail(_("Error getting installed packages")) end + packages + rescue Puppet::ExecutionFailure + fail(_("Error getting installed packages")) end # returns a hash of group => version of installed groups diff --git a/lib/puppet/provider/package/pip.rb b/lib/puppet/provider/package/pip.rb index d9b0af91b63..9319db1af48 100644 --- a/lib/puppet/provider/package/pip.rb +++ b/lib/puppet/provider/package/pip.rb @@ -133,12 +133,10 @@ def latest end def self.compare_pip_versions(x, y) - begin - Puppet::Util::Package::Version::Pip.compare(x, y) - rescue PIP_VERSION::ValidationFailure => ex - Puppet.debug("Cannot compare #{x} and #{y}. #{ex.message} Falling through default comparison mechanism.") - Puppet::Util::Package.versioncmp(x, y) - end + Puppet::Util::Package::Version::Pip.compare(x, y) + rescue PIP_VERSION::ValidationFailure => ex + Puppet.debug("Cannot compare #{x} and #{y}. #{ex.message} Falling through default comparison mechanism.") + Puppet::Util::Package.versioncmp(x, y) end # Use pip CLI to look up versions from PyPI repositories, diff --git a/lib/puppet/provider/package/pkgdmg.rb b/lib/puppet/provider/package/pkgdmg.rb index f35cea20dc4..0b8d45d8da9 100644 --- a/lib/puppet/provider/package/pkgdmg.rb +++ b/lib/puppet/provider/package/pkgdmg.rb @@ -77,7 +77,7 @@ def self.installpkgdmg(source, name) end unless source =~ /\.dmg$/i || source =~ /\.pkg$/i - raise Puppet::Error.new(_("Mac OS X PKG DMGs must specify a source string ending in .dmg or flat .pkg file")) + raise Puppet::Error, _("Mac OS X PKG DMGs must specify a source string ending in .dmg or flat .pkg file") end require 'open-uri' # Dead code; this is never used. The File.open call 20-ish lines south of here used to be Kernel.open but changed in '09. -NF @@ -107,7 +107,7 @@ def self.installpkgdmg(source, name) File.open(cached_source) do |dmg| xml_str = hdiutil "mount", "-plist", "-nobrowse", "-readonly", "-mountrandom", "/tmp", dmg.path hdiutil_info = Puppet::Util::Plist.parse_plist(xml_str) - raise Puppet::Error.new(_("No disk entities returned by mount at %{path}") % { path: dmg.path }) unless hdiutil_info.has_key?("system-entities") + raise Puppet::Error, _("No disk entities returned by mount at %{path}") % { path: dmg.path } unless hdiutil_info.has_key?("system-entities") mounts = hdiutil_info["system-entities"].filter_map { |entity| entity["mount-point"] @@ -146,12 +146,12 @@ def query def install source = @resource[:source] unless source - raise Puppet::Error.new(_("Mac OS X PKG DMGs must specify a package source.")) + raise Puppet::Error, _("Mac OS X PKG DMGs must specify a package source.") end name = @resource[:name] unless name - raise Puppet::Error.new(_("Mac OS X PKG DMGs must specify a package name.")) + raise Puppet::Error, _("Mac OS X PKG DMGs must specify a package name.") end self.class.installpkgdmg(source, name) diff --git a/lib/puppet/provider/package/pkgutil.rb b/lib/puppet/provider/package/pkgutil.rb index 2df2a4167a4..75aa5dd5090 100644 --- a/lib/puppet/provider/package/pkgutil.rb +++ b/lib/puppet/provider/package/pkgutil.rb @@ -172,11 +172,7 @@ def latest def query hash = pkgsingle(@resource) - if hash - hash - else - { :ensure => :absent } - end + hash || { :ensure => :absent } end def update diff --git a/lib/puppet/provider/package/portage.rb b/lib/puppet/provider/package/portage.rb index a7d2779a08f..79c024f1daa 100644 --- a/lib/puppet/provider/package/portage.rb +++ b/lib/puppet/provider/package/portage.rb @@ -60,7 +60,7 @@ def self.instances return packages rescue Puppet::ExecutionFailure => detail - raise Puppet::Error.new(detail) + raise Puppet::Error, detail end end @@ -138,7 +138,7 @@ def qatom end @atom = package_info rescue Puppet::ExecutionFailure => detail - raise Puppet::Error.new(detail) + raise Puppet::Error, detail end end @@ -155,9 +155,7 @@ def qatom_result_fields end def self.get_sets - @sets ||= begin - @sets = emerge(*(['--list-sets'])) - end + @sets ||= @sets = emerge(*(['--list-sets'])) end def query @@ -218,20 +216,20 @@ def query package[:ensure] = eix_get_version_for_slot(package[:installed_slots], qatom[:slot]) end - package[:ensure] = package[:ensure] ? package[:ensure] : :absent + package[:ensure] = package[:ensure] || :absent packages << package end case packages.size when 0 - raise Puppet::Error.new(_("No package found with the specified name [%{name}]") % { name: @resource[:name] }) + raise Puppet::Error, _("No package found with the specified name [%{name}]") % { name: @resource[:name] } when 1 @eix_result = packages[0] else - raise Puppet::Error.new(_("More than one package with the specified name [%{search_value}], please use the category parameter to disambiguate") % { search_value: search_value }) + raise Puppet::Error, _("More than one package with the specified name [%{search_value}], please use the category parameter to disambiguate") % { search_value: search_value } end rescue Puppet::ExecutionFailure => detail - raise Puppet::Error.new(detail) + raise Puppet::Error, detail end end diff --git a/lib/puppet/provider/package/puppetserver_gem.rb b/lib/puppet/provider/package/puppetserver_gem.rb index 88e30c32af9..7c0572f3c64 100644 --- a/lib/puppet/provider/package/puppetserver_gem.rb +++ b/lib/puppet/provider/package/puppetserver_gem.rb @@ -103,7 +103,7 @@ def install(useversion = true) command_options << uri.path when 'puppet' # we don't support puppet:// URLs (yet) - raise Puppet::Error.new(_('puppet:// URLs are not supported as gem sources')) + raise Puppet::Error, _('puppet:// URLs are not supported as gem sources') else # interpret it as a gem repository command_options << '--source' << "#{resource[:source]}" << resource[:name] diff --git a/lib/puppet/provider/package/rug.rb b/lib/puppet/provider/package/rug.rb index 23ef4c0fc79..80ed6ced128 100644 --- a/lib/puppet/provider/package/rug.rb +++ b/lib/puppet/provider/package/rug.rb @@ -26,9 +26,7 @@ def install rug "--quiet", :install, "-y", wanted unless self.query - raise Puppet::ExecutionFailure.new( - _("Could not find package %{name}") % { name: self.name } - ) + raise Puppet::ExecutionFailure, _("Could not find package %{name}") % { name: self.name } end end diff --git a/lib/puppet/provider/package/up2date.rb b/lib/puppet/provider/package/up2date.rb index 17303e44f24..bc7a719ef6d 100644 --- a/lib/puppet/provider/package/up2date.rb +++ b/lib/puppet/provider/package/up2date.rb @@ -15,9 +15,7 @@ def install up2date "-u", @resource[:name] unless self.query - raise Puppet::ExecutionFailure.new( - _("Could not find package %{name}") % { name: self.name } - ) + raise Puppet::ExecutionFailure, _("Could not find package %{name}") % { name: self.name } end end diff --git a/lib/puppet/provider/package/windows.rb b/lib/puppet/provider/package/windows.rb index 81b0a1a33f9..eacf459376c 100644 --- a/lib/puppet/provider/package/windows.rb +++ b/lib/puppet/provider/package/windows.rb @@ -39,11 +39,9 @@ class << self def self.post_resource_eval @paths.each do |path| - begin - Puppet::FileSystem.unlink(path) - rescue => detail - raise Puppet::Error.new(_("Error when unlinking %{path}: %{detail}") % { path: path, detail: detail.message }, detail) - end + Puppet::FileSystem.unlink(path) + rescue => detail + raise Puppet::Error.new(_("Error when unlinking %{path}: %{detail}") % { path: path, detail: detail.message }, detail) end if @paths end diff --git a/lib/puppet/provider/package/windows/exe_package.rb b/lib/puppet/provider/package/windows/exe_package.rb index b57da249e44..ef56a9800af 100644 --- a/lib/puppet/provider/package/windows/exe_package.rb +++ b/lib/puppet/provider/package/windows/exe_package.rb @@ -69,7 +69,7 @@ def self.install_command(resource) uri = URI(Puppet::Util.uri_encode(file_location)) client = Puppet.runtime[:http] client.get(uri, options: { include_system_store: true }) do |response| - raise Puppet::HTTP::ResponseError.new(response) unless response.success? + raise Puppet::HTTP::ResponseError, response unless response.success? File.open(tempfile.path, 'wb') do |file| response.read_body do |data| diff --git a/lib/puppet/provider/package/yum.rb b/lib/puppet/provider/package/yum.rb index 6f728d41298..0a2d25b52a6 100644 --- a/lib/puppet/provider/package/yum.rb +++ b/lib/puppet/provider/package/yum.rb @@ -59,12 +59,10 @@ def insync?(is) end is.split(self.class::MULTIVERSION_SEPARATOR).any? do |version| - begin - is_version = RPM_VERSION.parse(version) - should_version.include?(is_version) - rescue RPM_VERSION::ValidationFailure - Puppet.debug("Cannot parse #{is} as a RPM version") - end + is_version = RPM_VERSION.parse(version) + should_version.include?(is_version) + rescue RPM_VERSION::ValidationFailure + Puppet.debug("Cannot parse #{is} as a RPM version") end end end @@ -213,12 +211,10 @@ def best_version(should) end versions = [] available_versions(@resource[:name], disablerepo, enablerepo, disableexcludes).each do |version| - begin - rpm_version = RPM_VERSION.parse(version) - versions << rpm_version if should_range.include?(rpm_version) - rescue RPM_VERSION::ValidationFailure - Puppet.debug("Cannot parse #{version} as a RPM version") - end + rpm_version = RPM_VERSION.parse(version) + versions << rpm_version if should_range.include?(rpm_version) + rescue RPM_VERSION::ValidationFailure + Puppet.debug("Cannot parse #{version} as a RPM version") end version = versions.sort.last if versions.any? diff --git a/lib/puppet/provider/package/zypper.rb b/lib/puppet/provider/package/zypper.rb index 87c4debc950..1f8d5f6558b 100644 --- a/lib/puppet/provider/package/zypper.rb +++ b/lib/puppet/provider/package/zypper.rb @@ -145,9 +145,7 @@ def install zypper(*options) unless self.query - raise Puppet::ExecutionFailure.new( - _("Could not find package %{name}") % { name: self.name } - ) + raise Puppet::ExecutionFailure, _("Could not find package %{name}") % { name: self.name } end end diff --git a/lib/puppet/provider/service/daemontools.rb b/lib/puppet/provider/service/daemontools.rb index 469d1dec241..57b8357a564 100644 --- a/lib/puppet/provider/service/daemontools.rb +++ b/lib/puppet/provider/service/daemontools.rb @@ -110,7 +110,7 @@ def service # definition def daemon path = resource[:path] - raise Puppet::Error.new("#{self.class.name} must specify a path for daemon directory") unless path + raise Puppet::Error, "#{self.class.name} must specify a path for daemon directory" unless path File.join(path, resource[:name]) end diff --git a/lib/puppet/provider/service/launchd.rb b/lib/puppet/provider/service/launchd.rb index 3a085ce1a33..be8c83d2b4d 100644 --- a/lib/puppet/provider/service/launchd.rb +++ b/lib/puppet/provider/service/launchd.rb @@ -187,7 +187,7 @@ def self.job_list @job_list = Hash.new begin output = launchctl :list - raise Puppet::Error.new("launchctl list failed to return any data.") if output.nil? + raise Puppet::Error, "launchctl list failed to return any data." if output.nil? output.split("\n").each do |line| @job_list[line.split(/\s/).last] = :running @@ -212,7 +212,7 @@ def self.read_overrides Puppet.debug(_("Reading overrides plist, attempt %{i}") % { i: i }) if i > 1 overrides = read_plist(launchd_overrides) break unless overrides.nil? - raise Puppet::Error.new(_('Unable to read overrides plist, too many attempts')) if i == 20 + raise Puppet::Error, _('Unable to read overrides plist, too many attempts') if i == 20 Puppet.info(_('Overrides file could not be read, trying again.')) Kernel.sleep(0.1) @@ -239,7 +239,7 @@ def plist_from_label(label) if FileTest.file?(job_path) job_plist = self.class.read_plist(job_path) else - raise Puppet::Error.new("Unable to parse launchd plist at path: #{job_path}") + raise Puppet::Error, "Unable to parse launchd plist at path: #{job_path}" end [job_path, job_plist] end diff --git a/lib/puppet/provider/service/service.rb b/lib/puppet/provider/service/service.rb index b8accc7d6ae..72409e9911b 100644 --- a/lib/puppet/provider/service/service.rb +++ b/lib/puppet/provider/service/service.rb @@ -47,11 +47,9 @@ def ucommand(type, fof = true) # # @return [Puppet::Util::Execution::ProcessOutput] def service_execute(type, command, fof = true, squelch = false, combine = true) - begin - execute(command, :failonfail => fof, :override_locale => false, :squelch => squelch, :combine => combine) - rescue Puppet::ExecutionFailure => detail - @resource.fail Puppet::Error, "Could not #{type} #{@resource.ref}: #{detail}", detail - end + execute(command, :failonfail => fof, :override_locale => false, :squelch => squelch, :combine => combine) + rescue Puppet::ExecutionFailure => detail + @resource.fail Puppet::Error, "Could not #{type} #{@resource.ref}: #{detail}", detail end # Use either a specified command or the default for our provider. diff --git a/lib/puppet/provider/service/smf.rb b/lib/puppet/provider/service/smf.rb index f0dcadd65ef..2090b160ec3 100644 --- a/lib/puppet/provider/service/smf.rb +++ b/lib/puppet/provider/service/smf.rb @@ -175,7 +175,7 @@ def wait(*desired_states) end end rescue Timeout::Error - raise Puppet::Error.new("Timed out waiting for #{@resource[:name]} to transition states") + raise Puppet::Error, "Timed out waiting for #{@resource[:name]} to transition states" end def start diff --git a/lib/puppet/provider/service/src.rb b/lib/puppet/provider/service/src.rb index 2ca910d3941..b044a55a9f2 100644 --- a/lib/puppet/provider/service/src.rb +++ b/lib/puppet/provider/service/src.rb @@ -79,7 +79,7 @@ def wait(desired_state) end end rescue Timeout::Error - raise Puppet::Error.new("Timed out waiting for #{@resource[:name]} to transition states") + raise Puppet::Error, "Timed out waiting for #{@resource[:name]} to transition states" end def start diff --git a/lib/puppet/provider/service/systemd.rb b/lib/puppet/provider/service/systemd.rb index 8ab3391b0b3..7ca4e60f7a7 100644 --- a/lib/puppet/provider/service/systemd.rb +++ b/lib/puppet/provider/service/systemd.rb @@ -216,29 +216,23 @@ def statuscmd end def restart - begin - daemon_reload? - super - rescue Puppet::Error => e - raise Puppet::Error.new(prepare_error_message(@resource[:name], 'restart', e)) - end + daemon_reload? + super + rescue Puppet::Error => e + raise Puppet::Error, prepare_error_message(@resource[:name], 'restart', e) end def start - begin - daemon_reload? - super - rescue Puppet::Error => e - raise Puppet::Error.new(prepare_error_message(@resource[:name], 'start', e)) - end + daemon_reload? + super + rescue Puppet::Error => e + raise Puppet::Error, prepare_error_message(@resource[:name], 'start', e) end def stop - begin - super - rescue Puppet::Error => e - raise Puppet::Error.new(prepare_error_message(@resource[:name], 'stop', e)) - end + super + rescue Puppet::Error => e + raise Puppet::Error, prepare_error_message(@resource[:name], 'stop', e) end def prepare_error_message(name, action, exception) diff --git a/lib/puppet/provider/service/windows.rb b/lib/puppet/provider/service/windows.rb index 2dfc63e369b..bfdb82ce40c 100644 --- a/lib/puppet/provider/service/windows.rb +++ b/lib/puppet/provider/service/windows.rb @@ -56,7 +56,7 @@ def enabled? when :SERVICE_DISABLED :false else - raise Puppet::Error.new(_("Unknown start type: %{start_type}") % { start_type: start_type }) + raise Puppet::Error, _("Unknown start type: %{start_type}") % { start_type: start_type } end rescue => detail raise Puppet::Error.new(_("Cannot get start type %{resource_name}, error was: %{detail}") % { resource_name: @resource[:name], detail: detail }, detail) @@ -73,7 +73,7 @@ def start if enabled? == :false # If disabled and not managing enable, respect disabled and fail. if @resource[:enable].nil? - raise Puppet::Error.new(_("Will not start disabled service %{resource_name} without managing enable. Specify 'enable => false' to override.") % { resource_name: @resource[:name] }) + raise Puppet::Error, _("Will not start disabled service %{resource_name} without managing enable. Specify 'enable => false' to override.") % { resource_name: @resource[:name] } # Otherwise start. If enable => false, we will later sync enable and # disable the service again. elsif @resource[:enable] == :true @@ -101,7 +101,7 @@ def status 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] }) + raise Puppet::Error, _("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 @@ -164,16 +164,16 @@ def normalize_logonaccount def validate_logon_credentials unless Puppet::Util::Windows::User.localsystem?(@normalized_logon_account) - raise Puppet::Error.new("\"#{@normalized_logon_account}\" is not a valid account") unless @logonaccount_information && [:SidTypeUser, :SidTypeWellKnownGroup].include?(@logonaccount_information.account_type) + raise Puppet::Error, "\"#{@normalized_logon_account}\" is not a valid account" unless @logonaccount_information && [:SidTypeUser, :SidTypeWellKnownGroup].include?(@logonaccount_information.account_type) user_rights = Puppet::Util::Windows::User.get_rights(@logonaccount_information.domain_account) unless Puppet::Util::Windows::User.default_system_account?(@normalized_logon_account) - raise Puppet::Error.new("\"#{@normalized_logon_account}\" has the 'Log On As A Service' right set to denied.") if user_rights =~ /SeDenyServiceLogonRight/ - raise Puppet::Error.new("\"#{@normalized_logon_account}\" is missing the 'Log On As A Service' right.") unless user_rights.nil? || user_rights =~ /SeServiceLogonRight/ + raise Puppet::Error, "\"#{@normalized_logon_account}\" has the 'Log On As A Service' right set to denied." if user_rights =~ /SeDenyServiceLogonRight/ + raise Puppet::Error, "\"#{@normalized_logon_account}\" is missing the 'Log On As A Service' right." unless user_rights.nil? || user_rights =~ /SeServiceLogonRight/ end is_a_predefined_local_account = Puppet::Util::Windows::User.default_system_account?(@normalized_logon_account) || @normalized_logon_account == 'LocalSystem' account_info = @normalized_logon_account.split("\\") able_to_logon = Puppet::Util::Windows::User.password_is?(account_info[1], @resource[:logonpassword], account_info[0]) unless is_a_predefined_local_account - raise Puppet::Error.new("The given password is invalid for user '#{@normalized_logon_account}'.") unless is_a_predefined_local_account || able_to_logon + raise Puppet::Error, "The given password is invalid for user '#{@normalized_logon_account}'." unless is_a_predefined_local_account || able_to_logon end end diff --git a/lib/puppet/provider/user/directoryservice.rb b/lib/puppet/provider/user/directoryservice.rb index 8b9df1abae8..fc1920738bf 100644 --- a/lib/puppet/provider/user/directoryservice.rb +++ b/lib/puppet/provider/user/directoryservice.rb @@ -503,11 +503,9 @@ def merge_attribute_with_dscl(path, username, keyname, value) end def set_attribute_with_dscl(dscl_command, path, username, keyname, value) - begin - dscl '.', dscl_command, "/#{path}/#{username}", keyname, value - rescue Puppet::ExecutionFailure => detail - raise Puppet::Error, "Could not set the dscl #{keyname} key with value: #{value} - #{detail.inspect}", detail.backtrace - end + dscl '.', dscl_command, "/#{path}/#{username}", keyname, value + rescue Puppet::ExecutionFailure => detail + raise Puppet::Error, "Could not set the dscl #{keyname} key with value: #{value} - #{detail.inspect}", detail.backtrace end # Create the new user with dscl diff --git a/lib/puppet/provider/user/user_role_add.rb b/lib/puppet/provider/user/user_role_add.rb index 501f0a5dd10..a478fc1e926 100644 --- a/lib/puppet/provider/user/user_role_add.rb +++ b/lib/puppet/provider/user/user_role_add.rb @@ -221,25 +221,23 @@ def has_sensitive_data?(property = nil) # data, but it is still terrible. We still skip platform locking, so a # concurrent `vipw -s` session will have no idea we risk data loss. def password=(cryptopw) - begin - shadow = File.read(target_file_path) - - # Go Mifune loves the race here where we can lose data because - # /etc/shadow changed between reading it and writing it. - # --daniel 2012-02-05 - Puppet::Util.replace_file(target_file_path, 0o640) do |fh| - shadow.each_line do |line| - line_arr = line.split(':') - if line_arr[0] == @resource[:name] - line_arr[1] = cryptopw - line_arr[2] = (Date.today - Date.new(1970, 1, 1)).to_i.to_s - line = line_arr.join(':') - end - fh.print line + shadow = File.read(target_file_path) + + # Go Mifune loves the race here where we can lose data because + # /etc/shadow changed between reading it and writing it. + # --daniel 2012-02-05 + Puppet::Util.replace_file(target_file_path, 0o640) do |fh| + shadow.each_line do |line| + line_arr = line.split(':') + if line_arr[0] == @resource[:name] + line_arr[1] = cryptopw + line_arr[2] = (Date.today - Date.new(1970, 1, 1)).to_i.to_s + line = line_arr.join(':') end + fh.print line end - rescue => detail - self.fail Puppet::Error, "Could not write replace #{target_file_path}: #{detail}", detail end + rescue => detail + self.fail Puppet::Error, "Could not write replace #{target_file_path}: #{detail}", detail end end diff --git a/lib/puppet/provider/user/useradd.rb b/lib/puppet/provider/user/useradd.rb index 07c9f0724f3..05eb6dc3bb2 100644 --- a/lib/puppet/provider/user/useradd.rb +++ b/lib/puppet/provider/user/useradd.rb @@ -102,7 +102,7 @@ def finduser(key, value) unless @users unless Puppet::FileSystem.exist?(passwd_file) - raise Puppet::Error.new("Forcelocal set for user resource '#{resource[:name]}', but #{passwd_file} does not exist") + raise Puppet::Error, "Forcelocal set for user resource '#{resource[:name]}', but #{passwd_file} does not exist" end @users = [] @@ -163,7 +163,7 @@ def localgroups @groups_of[user] = [] unless Puppet::FileSystem.exist?(group_file) - raise Puppet::Error.new("Forcelocal set for user resource '#{user}', but #{group_file} does not exist") + raise Puppet::Error, "Forcelocal set for user resource '#{user}', but #{group_file} does not exist" end Puppet::FileSystem.each_line(group_file) do |line| diff --git a/lib/puppet/settings.rb b/lib/puppet/settings.rb index eb4fb3c6a39..e8d67e18f4f 100644 --- a/lib/puppet/settings.rb +++ b/lib/puppet/settings.rb @@ -429,13 +429,11 @@ def create_ancestors(dir) def call_hooks_deferred_to_application_initialization(options = {}) @hooks_to_call_on_application_initialization.each do |setting| - begin - setting.handle(self.value(setting.name)) - rescue InterpolationError => err - raise InterpolationError, err.message, err.backtrace unless options[:ignore_interpolation_dependency_errors] - # swallow. We're not concerned if we can't call hooks because dependencies don't exist yet - # we'll get another chance after application defaults are initialized - end + setting.handle(self.value(setting.name)) + rescue InterpolationError => err + raise InterpolationError, err.message, err.backtrace unless options[:ignore_interpolation_dependency_errors] + # swallow. We're not concerned if we can't call hooks because dependencies don't exist yet + # we'll get another chance after application defaults are initialized end end private :call_hooks_deferred_to_application_initialization @@ -783,9 +781,7 @@ def newsetting(hash) klass = StringSetting end hash[:settings] = self - setting = klass.new(hash) - - setting + klass.new(hash) end # This has to be private, because it doesn't add the settings to @config @@ -1410,14 +1406,13 @@ def explicit_config_file? # If they've specified neither, then the interpolation will fail and we'll # get an exception. # - begin - return true if self[:config] - rescue InterpolationError - # This means we failed to interpolate, which means that they didn't - # explicitly specify either :config or :confdir... so we'll fall out to - # the default value. - return false - end + + return true if self[:config] + rescue InterpolationError + # This means we failed to interpolate, which means that they didn't + # explicitly specify either :config or :confdir... so we'll fall out to + # the default value. + return false end private :explicit_config_file? diff --git a/lib/puppet/settings/alias_setting.rb b/lib/puppet/settings/alias_setting.rb index 38fd9b43af6..03e833ee458 100644 --- a/lib/puppet/settings/alias_setting.rb +++ b/lib/puppet/settings/alias_setting.rb @@ -26,11 +26,9 @@ def type end def method_missing(method, *args) - begin - alias_for.send(method, *args) - rescue => e - Puppet.log_exception(self.class, e.message) - end + alias_for.send(method, *args) + rescue => e + Puppet.log_exception(self.class, e.message) end private diff --git a/lib/puppet/settings/base_setting.rb b/lib/puppet/settings/base_setting.rb index d731e54e812..e509db51bae 100644 --- a/lib/puppet/settings/base_setting.rb +++ b/lib/puppet/settings/base_setting.rb @@ -89,7 +89,7 @@ def has_hook? def initialize(args = {}) @settings = args.delete(:settings) unless @settings - raise ArgumentError.new("You must refer to a settings object") + raise ArgumentError, "You must refer to a settings object" end # explicitly set name prior to calling other param= methods to provide meaningful feedback during diff --git a/lib/puppet/ssl/certificate_request.rb b/lib/puppet/ssl/certificate_request.rb index 5399e999ca1..b838e524e5a 100644 --- a/lib/puppet/ssl/certificate_request.rb +++ b/lib/puppet/ssl/certificate_request.rb @@ -214,19 +214,17 @@ def custom_attributes def add_csr_attributes(csr, csr_attributes) csr_attributes.each do |oid, value| - begin - if PRIVATE_CSR_ATTRIBUTES.include? oid - raise ArgumentError, _("Cannot specify CSR attribute %{oid}: conflicts with internally used CSR attribute") % { oid: oid } - end + if PRIVATE_CSR_ATTRIBUTES.include? oid + raise ArgumentError, _("Cannot specify CSR attribute %{oid}: conflicts with internally used CSR attribute") % { oid: oid } + end - encoded = OpenSSL::ASN1::PrintableString.new(value.to_s) + encoded = OpenSSL::ASN1::PrintableString.new(value.to_s) - attr_set = OpenSSL::ASN1::Set.new([encoded]) - csr.add_attribute(OpenSSL::X509::Attribute.new(oid, attr_set)) - Puppet.debug("Added csr attribute: #{oid} => #{attr_set.inspect}") - rescue OpenSSL::X509::AttributeError => e - raise Puppet::Error, _("Cannot create CSR with attribute %{oid}: %{message}") % { oid: oid, message: e.message }, e.backtrace - end + attr_set = OpenSSL::ASN1::Set.new([encoded]) + csr.add_attribute(OpenSSL::X509::Attribute.new(oid, attr_set)) + Puppet.debug("Added csr attribute: #{oid} => #{attr_set.inspect}") + rescue OpenSSL::X509::AttributeError => e + raise Puppet::Error, _("Cannot create CSR with attribute %{oid}: %{message}") % { oid: oid, message: e.message }, e.backtrace end end @@ -240,16 +238,14 @@ def extension_request_attribute(options) if options[:extension_requests] options[:extension_requests].each_pair do |oid, value| - begin - if PRIVATE_EXTENSIONS.include? oid - raise Puppet::Error, _("Cannot specify CSR extension request %{oid}: conflicts with internally used extension request") % { oid: oid } - end - - ext = OpenSSL::X509::Extension.new(oid, OpenSSL::ASN1::UTF8String.new(value.to_s).to_der, false) - extensions << ext - rescue OpenSSL::X509::ExtensionError => e - raise Puppet::Error, _("Cannot create CSR with extension request %{oid}: %{message}") % { oid: oid, message: e.message }, e.backtrace + if PRIVATE_EXTENSIONS.include? oid + raise Puppet::Error, _("Cannot specify CSR extension request %{oid}: conflicts with internally used extension request") % { oid: oid } end + + ext = OpenSSL::X509::Extension.new(oid, OpenSSL::ASN1::UTF8String.new(value.to_s).to_der, false) + extensions << ext + rescue OpenSSL::X509::ExtensionError => e + raise Puppet::Error, _("Cannot create CSR with extension request %{oid}: %{message}") % { oid: oid, message: e.message }, e.backtrace end end diff --git a/lib/puppet/syntax_checkers/base64.rb b/lib/puppet/syntax_checkers/base64.rb index c923533476f..0dcbb67f952 100644 --- a/lib/puppet/syntax_checkers/base64.rb +++ b/lib/puppet/syntax_checkers/base64.rb @@ -16,9 +16,9 @@ class Puppet::SyntaxCheckers::Base64 < Puppet::Plugins::SyntaxCheckers::SyntaxCh # @api public # def check(text, syntax, acceptor, source_pos) - raise ArgumentError.new(_("Base64 syntax checker: the text to check must be a String.")) unless text.is_a?(String) - raise ArgumentError.new(_("Base64 syntax checker: the syntax identifier must be a String, e.g. json, data+json")) unless syntax.is_a?(String) - raise ArgumentError.new(_("Base64 syntax checker: invalid Acceptor, got: '%{klass}'.") % { klass: acceptor.class.name }) unless acceptor.is_a?(Puppet::Pops::Validation::Acceptor) + raise ArgumentError, _("Base64 syntax checker: the text to check must be a String.") unless text.is_a?(String) + raise ArgumentError, _("Base64 syntax checker: the syntax identifier must be a String, e.g. json, data+json") unless syntax.is_a?(String) + raise ArgumentError, _("Base64 syntax checker: invalid Acceptor, got: '%{klass}'.") % { klass: acceptor.class.name } unless acceptor.is_a?(Puppet::Pops::Validation::Acceptor) cleaned_text = text.gsub(/[\r?\n[:blank:]]/, '') begin diff --git a/lib/puppet/syntax_checkers/epp.rb b/lib/puppet/syntax_checkers/epp.rb index 8f1255db65f..0081855bdf2 100644 --- a/lib/puppet/syntax_checkers/epp.rb +++ b/lib/puppet/syntax_checkers/epp.rb @@ -15,9 +15,9 @@ class Puppet::SyntaxCheckers::EPP < Puppet::Plugins::SyntaxCheckers::SyntaxCheck # @api public # def check(text, syntax, acceptor, source_pos) - raise ArgumentError.new(_("EPP syntax checker: the text to check must be a String.")) unless text.is_a?(String) - raise ArgumentError.new(_("EPP syntax checker: the syntax identifier must be a String, e.g. pp")) unless syntax == 'epp' - raise ArgumentError.new(_("EPP syntax checker: invalid Acceptor, got: '%{klass}'.") % { klass: acceptor.class.name }) unless acceptor.is_a?(Puppet::Pops::Validation::Acceptor) + raise ArgumentError, _("EPP syntax checker: the text to check must be a String.") unless text.is_a?(String) + raise ArgumentError, _("EPP syntax checker: the syntax identifier must be a String, e.g. pp") unless syntax == 'epp' + raise ArgumentError, _("EPP syntax checker: invalid Acceptor, got: '%{klass}'.") % { klass: acceptor.class.name } unless acceptor.is_a?(Puppet::Pops::Validation::Acceptor) begin Puppet::Pops::Parser::EvaluatingParser::EvaluatingEppParser.singleton.parse_string(text) diff --git a/lib/puppet/syntax_checkers/json.rb b/lib/puppet/syntax_checkers/json.rb index 78d517d50c0..5a56041e97b 100644 --- a/lib/puppet/syntax_checkers/json.rb +++ b/lib/puppet/syntax_checkers/json.rb @@ -15,9 +15,9 @@ class Puppet::SyntaxCheckers::Json < Puppet::Plugins::SyntaxCheckers::SyntaxChec # @api public # def check(text, syntax, acceptor, source_pos) - raise ArgumentError.new(_("Json syntax checker: the text to check must be a String.")) unless text.is_a?(String) - raise ArgumentError.new(_("Json syntax checker: the syntax identifier must be a String, e.g. json, data+json")) unless syntax.is_a?(String) - raise ArgumentError.new(_("Json syntax checker: invalid Acceptor, got: '%{klass}'.") % { klass: acceptor.class.name }) unless acceptor.is_a?(Puppet::Pops::Validation::Acceptor) + raise ArgumentError, _("Json syntax checker: the text to check must be a String.") unless text.is_a?(String) + raise ArgumentError, _("Json syntax checker: the syntax identifier must be a String, e.g. json, data+json") unless syntax.is_a?(String) + raise ArgumentError, _("Json syntax checker: invalid Acceptor, got: '%{klass}'.") % { klass: acceptor.class.name } unless acceptor.is_a?(Puppet::Pops::Validation::Acceptor) begin Puppet::Util::Json.load(text) diff --git a/lib/puppet/syntax_checkers/pp.rb b/lib/puppet/syntax_checkers/pp.rb index 3a63b9d9fb3..779237bddcc 100644 --- a/lib/puppet/syntax_checkers/pp.rb +++ b/lib/puppet/syntax_checkers/pp.rb @@ -15,9 +15,9 @@ class Puppet::SyntaxCheckers::PP < Puppet::Plugins::SyntaxCheckers::SyntaxChecke # @api public # def check(text, syntax, acceptor, source_pos) - raise ArgumentError.new(_("PP syntax checker: the text to check must be a String.")) unless text.is_a?(String) - raise ArgumentError.new(_("PP syntax checker: the syntax identifier must be a String, e.g. pp")) unless syntax == 'pp' - raise ArgumentError.new(_("PP syntax checker: invalid Acceptor, got: '%{klass}'.") % { klass: acceptor.class.name }) unless acceptor.is_a?(Puppet::Pops::Validation::Acceptor) + raise ArgumentError, _("PP syntax checker: the text to check must be a String.") unless text.is_a?(String) + raise ArgumentError, _("PP syntax checker: the syntax identifier must be a String, e.g. pp") unless syntax == 'pp' + raise ArgumentError, _("PP syntax checker: invalid Acceptor, got: '%{klass}'.") % { klass: acceptor.class.name } unless acceptor.is_a?(Puppet::Pops::Validation::Acceptor) begin Puppet::Pops::Parser::EvaluatingParser.singleton.parse_string(text) diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb index 05ca2aaf628..8dead10af2f 100644 --- a/lib/puppet/transaction.rb +++ b/lib/puppet/transaction.rb @@ -78,11 +78,9 @@ def perform_pre_run_checks prerun_errors = {} @catalog.vertices.each do |res| - begin - res.pre_run_check - rescue Puppet::Error => detail - prerun_errors[res] = detail - end + res.pre_run_check + rescue Puppet::Error => detail + prerun_errors[res] = detail end unless prerun_errors.empty? @@ -149,11 +147,9 @@ def evaluate(&block) end post_evalable_providers.each do |provider| - begin - provider.post_resource_eval - rescue => detail - Puppet.log_exception(detail, _("post_resource_eval failed for provider %{provider}") % { provider: provider }) - end + provider.post_resource_eval + rescue => detail + Puppet.log_exception(detail, _("post_resource_eval failed for provider %{provider}") % { provider: provider }) end persistence.save if persistence.enabled?(catalog) diff --git a/lib/puppet/transaction/persistence.rb b/lib/puppet/transaction/persistence.rb index 988133dfd6e..bb7903f6322 100644 --- a/lib/puppet/transaction/persistence.rb +++ b/lib/puppet/transaction/persistence.rb @@ -83,20 +83,18 @@ def load result = nil Puppet::Util.benchmark(:debug, _("Loaded transaction store file in %{seconds} seconds")) do + result = Puppet::Util::Yaml.safe_load_file(filename, self.class.allowed_classes) + rescue Puppet::Util::Yaml::YamlLoadError => detail + Puppet.log_exception(detail, _("Transaction store file %{filename} is corrupt (%{detail}); replacing") % { filename: filename, detail: detail }) + begin - result = Puppet::Util::Yaml.safe_load_file(filename, self.class.allowed_classes) - rescue Puppet::Util::Yaml::YamlLoadError => detail - Puppet.log_exception(detail, _("Transaction store file %{filename} is corrupt (%{detail}); replacing") % { filename: filename, detail: detail }) - - begin - File.rename(filename, filename + ".bad") - rescue => detail - Puppet.log_exception(detail, _("Unable to rename corrupt transaction store file: %{detail}") % { detail: detail }) - raise Puppet::Error, _("Could not rename corrupt transaction store file %{filename}; remove manually") % { filename: filename }, detail.backtrace - end - - result = {} + File.rename(filename, filename + ".bad") + rescue => detail + Puppet.log_exception(detail, _("Unable to rename corrupt transaction store file: %{detail}") % { detail: detail }) + raise Puppet::Error, _("Could not rename corrupt transaction store file %{filename}; remove manually") % { filename: filename }, detail.backtrace end + + result = {} end unless result.is_a?(Hash) diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb index fc445aba71d..f90e125d9c9 100644 --- a/lib/puppet/type.rb +++ b/lib/puppet/type.rb @@ -680,7 +680,7 @@ def []=(name, value) nv = name_var name = nv if nv end - raise Puppet::Error.new("Got nil value for #{name}") if value.nil? + raise Puppet::Error, "Got nil value for #{name}" if value.nil? property = self.newattr(name) @@ -710,7 +710,7 @@ def delete(attr) if @parameters.has_key?(attr) @parameters.delete(attr) else - raise Puppet::DevError.new(_("Undefined attribute '%{attribute}' in %{name}") % { attribute: attr, name: self }) + raise Puppet::DevError, _("Undefined attribute '%{attribute}' in %{name}") % { attribute: attr, name: self } end end @@ -1803,11 +1803,7 @@ def self.provide(name, options = {}, &block) pname else provider = self.provider(pname) - if provider - provider - else - raise Puppet::DevError, _("Could not find parent provider %{parent} of %{name}") % { parent: pname, name: name } - end + provider || raise(Puppet::DevError, _("Could not find parent provider %{parent} of %{name}") % { parent: pname, name: name }) end else Puppet::Provider @@ -1817,7 +1813,7 @@ def self.provide(name, options = {}, &block) self.providify - provider = genclass( + genclass( name, :parent => parent, :hash => provider_hash, @@ -1827,8 +1823,6 @@ def self.provide(name, options = {}, &block) :extend => feature_module, :attributes => options ) - - provider end # Ensures there is a `:provider` parameter defined. @@ -2363,13 +2357,11 @@ def initialize(resource) # # @return [void] def validate_resource - begin - self.validate if self.respond_to?(:validate) - rescue Puppet::Error, ArgumentError => detail - error = Puppet::ResourceError.new("Validation of #{ref} failed: #{detail}") - adderrorcontext(error, detail) - raise error - end + self.validate if self.respond_to?(:validate) + rescue Puppet::Error, ArgumentError => detail + error = Puppet::ResourceError.new("Validation of #{ref} failed: #{detail}") + adderrorcontext(error, detail) + raise error end protected @@ -2454,22 +2446,20 @@ def set_parameters(hash) # on invalid attributes. no_values = [] (self.class.allattrs + hash.keys).uniq.each do |attr| - begin - # Set any defaults immediately. This is mostly done so - # that the default provider is available for any other - # property validation. - if hash.has_key?(attr) - self[attr] = hash[attr] - else - no_values << attr - end - rescue ArgumentError, Puppet::Error, TypeError - raise - rescue => detail - error = Puppet::DevError.new(_("Could not set %{attribute} on %{class_name}: %{detail}") % { attribute: attr, class_name: self.class.name, detail: detail }) - error.set_backtrace(detail.backtrace) - raise error + # Set any defaults immediately. This is mostly done so + # that the default provider is available for any other + # property validation. + if hash.has_key?(attr) + self[attr] = hash[attr] + else + no_values << attr end + rescue ArgumentError, Puppet::Error, TypeError + raise + rescue => detail + error = Puppet::DevError.new(_("Could not set %{attribute} on %{class_name}: %{detail}") % { attribute: attr, class_name: self.class.name, detail: detail }) + error.set_backtrace(detail.backtrace) + raise error end no_values.each do |attr| set_default(attr) diff --git a/lib/puppet/type/file/ensure.rb b/lib/puppet/type/file/ensure.rb index 93a2b9cdfb1..38e86352022 100644 --- a/lib/puppet/type/file/ensure.rb +++ b/lib/puppet/type/file/ensure.rb @@ -188,9 +188,7 @@ def sync return :file_removed end - event = super - - event + super end end end diff --git a/lib/puppet/type/file/source.rb b/lib/puppet/type/file/source.rb index 669f4302ff5..d97bfd1226c 100644 --- a/lib/puppet/type/file/source.rb +++ b/lib/puppet/type/file/source.rb @@ -189,23 +189,21 @@ def metadata return nil unless value value.each do |source| - begin - options = { - :environment => resource.catalog.environment_instance, - :links => resource[:links], - :checksum_type => resource[:checksum], - :source_permissions => resource[:source_permissions] - } + options = { + :environment => resource.catalog.environment_instance, + :links => resource[:links], + :checksum_type => resource[:checksum], + :source_permissions => resource[:source_permissions] + } - data = Puppet::FileServing::Metadata.indirection.find(source, options) - if data - @metadata = data - @metadata.source = source - break - end - rescue => detail - self.fail Puppet::Error, "Could not retrieve file metadata for #{source}: #{detail}", detail + data = Puppet::FileServing::Metadata.indirection.find(source, options) + if data + @metadata = data + @metadata.source = source + break end + rescue => detail + self.fail Puppet::Error, "Could not retrieve file metadata for #{source}: #{detail}", detail end self.fail "Could not retrieve information from environment #{resource.catalog.environment} source(s) #{value.join(", ")}" unless @metadata @metadata @@ -317,7 +315,7 @@ def get_from_source_uri_source(url, &block) def get_from_http_source(url, &block) client = Puppet.runtime[:http] client.get(url, options: { include_system_store: true }) do |response| - raise Puppet::HTTP::ResponseError.new(response) unless response.success? + raise Puppet::HTTP::ResponseError, response unless response.success? response.read_body(&block) end diff --git a/lib/puppet/type/service.rb b/lib/puppet/type/service.rb index 6bd08482086..9f80621cf2d 100644 --- a/lib/puppet/type/service.rb +++ b/lib/puppet/type/service.rb @@ -283,14 +283,12 @@ def insync?(current) defaultto { provider.respond_to?(:default_timeout) ? provider.default_timeout : 10 } munge do |value| - begin - value = value.to_i - raise if value < 1 + value = value.to_i + raise if value < 1 - value - rescue - raise Puppet::Error.new(_("\"%{value}\" is not a positive integer: the timeout parameter must be specified as a positive integer") % { value: value }) - end + value + rescue + raise Puppet::Error, _("\"%{value}\" is not a positive integer: the timeout parameter must be specified as a positive integer") % { value: value } end end @@ -311,7 +309,7 @@ def self.needs_ensure_retrieved validate do if @parameters[:logonpassword] && @parameters[:logonaccount].nil? - raise Puppet::Error.new(_("The 'logonaccount' parameter is mandatory when setting 'logonpassword'.")) + raise Puppet::Error, _("The 'logonaccount' parameter is mandatory when setting 'logonpassword'.") end end end diff --git a/lib/puppet/type/tidy.rb b/lib/puppet/type/tidy.rb index 4e21c2d381f..c3f0d8fc490 100644 --- a/lib/puppet/type/tidy.rb +++ b/lib/puppet/type/tidy.rb @@ -368,15 +368,13 @@ def tidy?(path) end def stat(path) - begin - Puppet::FileSystem.lstat(path) - rescue Errno::ENOENT - debug _("File does not exist") - return nil - rescue Errno::EACCES - # TRANSLATORS "stat" is a program name and should not be translated - warning _("Could not stat; permission denied") - return nil - end + Puppet::FileSystem.lstat(path) + rescue Errno::ENOENT + debug _("File does not exist") + return nil + rescue Errno::EACCES + # TRANSLATORS "stat" is a program name and should not be translated + warning _("Could not stat; permission denied") + return nil end end diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb index f511036b30d..51930993c97 100644 --- a/lib/puppet/util.rb +++ b/lib/puppet/util.rb @@ -211,33 +211,31 @@ def which(bin) exts = ENV.fetch('PATHEXT', nil) exts = exts ? exts.split(File::PATH_SEPARATOR) : %w[.COM .EXE .BAT .CMD] ENV.fetch('PATH').split(File::PATH_SEPARATOR).each do |dir| - begin - dest = File.expand_path(File.join(dir, bin)) - rescue ArgumentError => e - # if the user's PATH contains a literal tilde (~) character and HOME is not set, we may get - # an ArgumentError here. Let's check to see if that is the case; if not, re-raise whatever error - # was thrown. - if e.to_s =~ /HOME/ and (ENV['HOME'].nil? || ENV.fetch('HOME', nil) == "") - # if we get here they have a tilde in their PATH. We'll issue a single warning about this and then - # ignore this path element and carry on with our lives. - # TRANSLATORS PATH and HOME are environment variables and should not be translated - Puppet::Util::Warnings.warnonce(_("PATH contains a ~ character, and HOME is not set; ignoring PATH element '%{dir}'.") % { dir: dir }) - elsif e.to_s =~ /doesn't exist|can't find user/ - # ...otherwise, we just skip the non-existent entry, and do nothing. - # TRANSLATORS PATH is an environment variable and should not be translated - Puppet::Util::Warnings.warnonce(_("Couldn't expand PATH containing a ~ character; ignoring PATH element '%{dir}'.") % { dir: dir }) - else - raise - end + dest = File.expand_path(File.join(dir, bin)) + rescue ArgumentError => e + # if the user's PATH contains a literal tilde (~) character and HOME is not set, we may get + # an ArgumentError here. Let's check to see if that is the case; if not, re-raise whatever error + # was thrown. + if e.to_s =~ /HOME/ and (ENV['HOME'].nil? || ENV.fetch('HOME', nil) == "") + # if we get here they have a tilde in their PATH. We'll issue a single warning about this and then + # ignore this path element and carry on with our lives. + # TRANSLATORS PATH and HOME are environment variables and should not be translated + Puppet::Util::Warnings.warnonce(_("PATH contains a ~ character, and HOME is not set; ignoring PATH element '%{dir}'.") % { dir: dir }) + elsif e.to_s =~ /doesn't exist|can't find user/ + # ...otherwise, we just skip the non-existent entry, and do nothing. + # TRANSLATORS PATH is an environment variable and should not be translated + Puppet::Util::Warnings.warnonce(_("Couldn't expand PATH containing a ~ character; ignoring PATH element '%{dir}'.") % { dir: dir }) else - if Puppet::Util::Platform.windows? && File.extname(dest).empty? - exts.each do |ext| - destext = File.expand_path(dest + ext) - return destext if FileTest.file? destext and FileTest.executable? destext - end + raise + end + else + if Puppet::Util::Platform.windows? && File.extname(dest).empty? + exts.each do |ext| + destext = File.expand_path(dest + ext) + return destext if FileTest.file? destext and FileTest.executable? destext end - return dest if FileTest.file? dest and FileTest.executable? dest end + return dest if FileTest.file? dest and FileTest.executable? dest end end nil @@ -407,7 +405,7 @@ def uri_query_encode(query_string) # query will encode + as %2B and space as %20 # fragment behaves like query def uri_encode(path, opts = { :allow_fragment => false }) - raise ArgumentError.new(_('path may not be nil')) if path.nil? + raise ArgumentError, _('path may not be nil') if path.nil? encoded = ''.dup @@ -470,7 +468,7 @@ def uri_unescape(str) module_function :uri_unescape def safe_posix_fork(stdin = $stdin, stdout = $stdout, stderr = $stderr, &block) - child_pid = Kernel.fork do + Kernel.fork do STDIN.reopen(stdin) STDOUT.reopen(stdout) STDERR.reopen(stderr) @@ -491,7 +489,6 @@ def safe_posix_fork(stdin = $stdin, stdout = $stdout, stderr = $stderr, &block) block.call if block end - child_pid end module_function :safe_posix_fork @@ -507,11 +504,9 @@ def symbolizehash(hash) # Just benchmark, with no logging. def thinmark - seconds = Benchmark.realtime { + Benchmark.realtime { yield } - - seconds end module_function :thinmark diff --git a/lib/puppet/util/at_fork/solaris.rb b/lib/puppet/util/at_fork/solaris.rb index 6cf1873ffe4..a36445174d7 100644 --- a/lib/puppet/util/at_fork/solaris.rb +++ b/lib/puppet/util/at_fork/solaris.rb @@ -63,27 +63,25 @@ def raise_if_error(&block) end def activate_new_contract_template - begin - tmpl = File.new(CTFS_PR_TEMPLATE, File::RDWR) - - begin - tmpl_fd = tmpl.fileno + tmpl = File.new(CTFS_PR_TEMPLATE, File::RDWR) - raise_if_error { ct_pr_tmpl_set_param(tmpl_fd, CT_PR_PGRPONLY) } - raise_if_error { ct_pr_tmpl_set_fatal(tmpl_fd, CT_PR_EV_HWERR) } - raise_if_error { ct_tmpl_set_critical(tmpl_fd, 0) } - raise_if_error { ct_tmpl_set_informative(tmpl_fd, CT_PR_EV_HWERR) } + begin + tmpl_fd = tmpl.fileno - raise_if_error { ct_tmpl_activate(tmpl_fd) } - rescue - tmpl.close - raise - end + raise_if_error { ct_pr_tmpl_set_param(tmpl_fd, CT_PR_PGRPONLY) } + raise_if_error { ct_pr_tmpl_set_fatal(tmpl_fd, CT_PR_EV_HWERR) } + raise_if_error { ct_tmpl_set_critical(tmpl_fd, 0) } + raise_if_error { ct_tmpl_set_informative(tmpl_fd, CT_PR_EV_HWERR) } - @tmpl = tmpl - rescue => detail - Puppet.log_exception(detail, _('Failed to activate a new process contract template')) + raise_if_error { ct_tmpl_activate(tmpl_fd) } + rescue + tmpl.close + raise end + + @tmpl = tmpl + rescue => detail + Puppet.log_exception(detail, _('Failed to activate a new process contract template')) end def deactivate_contract_template(parent) @@ -108,24 +106,22 @@ def deactivate_contract_template(parent) end def get_latest_child_contract_id - begin - stat = File.new(CTFS_PR_LATEST, File::RDONLY) + stat = File.new(CTFS_PR_LATEST, File::RDONLY) - begin - stathdl = Fiddle::Pointer.new(0) - - raise_if_error { ct_status_read(stat.fileno, CTD_COMMON, stathdl.ref) } - ctid = ct_status_get_id(stathdl) - ct_status_free(stathdl) - ensure - stat.close - end + begin + stathdl = Fiddle::Pointer.new(0) - ctid - rescue => detail - Puppet.log_exception(detail, _('Failed to get latest child process contract id')) - nil + raise_if_error { ct_status_read(stat.fileno, CTD_COMMON, stathdl.ref) } + ctid = ct_status_get_id(stathdl) + ct_status_free(stathdl) + ensure + stat.close end + + ctid + rescue => detail + Puppet.log_exception(detail, _('Failed to get latest child process contract id')) + nil end def abandon_latest_child_contract diff --git a/lib/puppet/util/command_line/puppet_option_parser.rb b/lib/puppet/util/command_line/puppet_option_parser.rb index 41f5128f624..8fff892cab4 100644 --- a/lib/puppet/util/command_line/puppet_option_parser.rb +++ b/lib/puppet/util/command_line/puppet_option_parser.rb @@ -63,7 +63,7 @@ def on(*args, &block) when :NONE options[:type] = :flag else - raise PuppetOptionError.new(_("Unsupported type: '%{type}'") % { type: type }) + raise PuppetOptionError, _("Unsupported type: '%{type}'") % { type: type } end @parser.opt long.sub("^--", "").intern, desc, options diff --git a/lib/puppet/util/command_line/trollop.rb b/lib/puppet/util/command_line/trollop.rb index 5e462cf8d25..54cf42fe8f8 100644 --- a/lib/puppet/util/command_line/trollop.rb +++ b/lib/puppet/util/command_line/trollop.rb @@ -337,7 +337,7 @@ def parse cmdline = ARGV when /^--no-([^-]\S*)$/ @long["[no-]#{::Regexp.last_match(1)}"] when /^--([^-]\S*)$/ - @long[::Regexp.last_match(1)] ? @long[::Regexp.last_match(1)] : @long["[no-]#{::Regexp.last_match(1)}"] + @long[::Regexp.last_match(1)] || @long["[no-]#{::Regexp.last_match(1)}"] else raise CommandlineError, _("invalid argument syntax: '%{arg}'") % { arg: arg } end @@ -454,15 +454,13 @@ def method_missing(m, *args) def parse_date_parameter param, arg # :nodoc: begin - begin - time = Chronic.parse(param) - rescue NameError - # chronic is not available - end - time ? Date.new(time.year, time.month, time.day) : Date.parse(param) - rescue ArgumentError - raise CommandlineError, _("option '%{arg}' needs a date") % { arg: arg }, $!.backtrace + time = Chronic.parse(param) + rescue NameError + # chronic is not available end + time ? Date.new(time.year, time.month, time.day) : Date.parse(param) + rescue ArgumentError + raise CommandlineError, _("option '%{arg}' needs a date") % { arg: arg }, $!.backtrace end ## Print the help message to +stream+. @@ -784,19 +782,17 @@ def options args = ARGV, *a, &b ## Requires passing in the parser object. def with_standard_exception_handling parser - begin - yield - rescue CommandlineError => e - $stderr.puts _("Error: %{value0}.") % { value0: e.message } - $stderr.puts _("Try --help for help.") - exit(-1) - rescue HelpNeeded - parser.educate - exit - rescue VersionNeeded - puts parser.version - exit - end + yield + rescue CommandlineError => e + $stderr.puts _("Error: %{value0}.") % { value0: e.message } + $stderr.puts _("Try --help for help.") + exit(-1) + rescue HelpNeeded + parser.educate + exit + rescue VersionNeeded + puts parser.version + exit end ## Informs the user that their usage of 'arg' was wrong, as detailed by diff --git a/lib/puppet/util/diff.rb b/lib/puppet/util/diff.rb index 190218a4f24..5e62ee08cb4 100644 --- a/lib/puppet/util/diff.rb +++ b/lib/puppet/util/diff.rb @@ -44,27 +44,25 @@ def lcs_diff(data_old, data_new, format = :unified, context_lines = 3) file_length_difference = 0 diffs.each do |piece| - begin - hunk = ::Diff::LCS::Hunk.new( - data_old, data_new, piece, - context_lines, - file_length_difference - ) - file_length_difference = hunk.file_length_difference - next unless oldhunk + hunk = ::Diff::LCS::Hunk.new( + data_old, data_new, piece, + context_lines, + file_length_difference + ) + file_length_difference = hunk.file_length_difference + next unless oldhunk - # Hunks may overlap, which is why we need to be careful when our - # diff includes lines of context. Otherwise, we might print - # redundant lines. - if (context_lines > 0) and hunk.overlaps?(oldhunk) - hunk.unshift(oldhunk) - else - output << oldhunk.diff(format) - end - ensure - oldhunk = hunk - output << "\n" + # Hunks may overlap, which is why we need to be careful when our + # diff includes lines of context. Otherwise, we might print + # redundant lines. + if (context_lines > 0) and hunk.overlaps?(oldhunk) + hunk.unshift(oldhunk) + else + output << oldhunk.diff(format) end + ensure + oldhunk = hunk + output << "\n" end # Handle the last remaining hunk diff --git a/lib/puppet/util/execution.rb b/lib/puppet/util/execution.rb index 67813e2b785..a5440c6272b 100644 --- a/lib/puppet/util/execution.rb +++ b/lib/puppet/util/execution.rb @@ -327,7 +327,7 @@ class << self # @api private # def self.execute_posix(command, options, stdin, stdout, stderr) - child_pid = Puppet::Util.safe_posix_fork(stdin, stdout, stderr) do + Puppet::Util.safe_posix_fork(stdin, stdout, stderr) do # We can't just call Array(command), and rely on it returning # things like ['foo'], when passed ['foo'], because # Array(command) will call command.to_a internally, which when @@ -371,7 +371,6 @@ def self.execute_posix(command, options, stdin, stdout, stderr) exit!(1) end end - child_pid end private_class_method :execute_posix diff --git a/lib/puppet/util/fileparsing.rb b/lib/puppet/util/fileparsing.rb index a5eda190f0e..4a294043ab5 100644 --- a/lib/puppet/util/fileparsing.rb +++ b/lib/puppet/util/fileparsing.rb @@ -42,7 +42,7 @@ class FileRecord def fields=(fields) @fields = fields.collect do |field| r = field.intern - raise ArgumentError.new(_("Cannot have fields named %{name}") % { name: r }) if INVALID_FIELDS.include?(r) + raise ArgumentError, _("Cannot have fields named %{name}") % { name: r } if INVALID_FIELDS.include?(r) r end diff --git a/lib/puppet/util/filetype.rb b/lib/puppet/util/filetype.rb index 1720f703018..616e608dc97 100644 --- a/lib/puppet/util/filetype.rb +++ b/lib/puppet/util/filetype.rb @@ -37,37 +37,33 @@ def self.newfiletype(name, &block) # Rename the read method define_method(:real_read, instance_method(:read)) define_method(:read) do - begin - val = real_read - @loaded = Time.now - if val - return val.gsub(/# HEADER.*\n/, '') - else - return "" - end - rescue Puppet::Error - raise - rescue => detail - message = _("%{klass} could not read %{path}: %{detail}") % { klass: self.class, path: @path, detail: detail } - Puppet.log_exception(detail, message) - raise Puppet::Error, message, detail.backtrace + val = real_read + @loaded = Time.now + if val + return val.gsub(/# HEADER.*\n/, '') + else + return "" end + rescue Puppet::Error + raise + rescue => detail + message = _("%{klass} could not read %{path}: %{detail}") % { klass: self.class, path: @path, detail: detail } + Puppet.log_exception(detail, message) + raise Puppet::Error, message, detail.backtrace end # And then the write method define_method(:real_write, instance_method(:write)) define_method(:write) do |text| - begin - val = real_write(text) - @synced = Time.now - return val - rescue Puppet::Error - raise - rescue => detail - message = _("%{klass} could not write %{path}: %{detail}") % { klass: self.class, path: @path, detail: detail } - Puppet.log_exception(detail, message) - raise Puppet::Error, message, detail.backtrace - end + val = real_write(text) + @synced = Time.now + return val + rescue Puppet::Error + raise + rescue => detail + message = _("%{klass} could not write %{path}: %{detail}") % { klass: self.class, path: @path, detail: detail } + Puppet.log_exception(detail, message) + raise Puppet::Error, message, detail.backtrace end end end @@ -82,7 +78,7 @@ def bucket end def initialize(path, default_mode = nil) - raise ArgumentError.new(_("Path is nil")) if path.nil? + raise ArgumentError, _("Path is nil") if path.nil? @path = path @default_mode = default_mode diff --git a/lib/puppet/util/inifile.rb b/lib/puppet/util/inifile.rb index b887ba4f3c9..f4eb7c84251 100644 --- a/lib/puppet/util/inifile.rb +++ b/lib/puppet/util/inifile.rb @@ -184,7 +184,7 @@ def parse(text) val = match[2].rstrip if section.nil? - raise IniParseError.new(_("Property with key %{key} outside of a section") % { key: key.inspect }) + raise IniParseError, _("Property with key %{key} outside of a section") % { key: key.inspect } end section[key] = val @@ -318,11 +318,7 @@ def add_section(name, file) # Return a file if it's already been defined, create a new file if it hasn't # been defined. def get_physical_file(file) - if @files[file] - @files[file] - else - new_physical_file(file) - end + @files[file] || new_physical_file(file) end # Create a new physical file and set required attributes on that file. diff --git a/lib/puppet/util/ldap/manager.rb b/lib/puppet/util/ldap/manager.rb index a643ceda273..3d7156aca00 100644 --- a/lib/puppet/util/ldap/manager.rb +++ b/lib/puppet/util/ldap/manager.rb @@ -116,14 +116,12 @@ def filter # 'dn', or nil if the entry cannot be found. def find(name) connect do |conn| - begin - conn.search2(dn(name), 0, "objectclass=*") do |result| - # Convert to puppet-appropriate attributes - return entry2provider(result) - end - rescue - return nil + conn.search2(dn(name), 0, "objectclass=*") do |result| + # Convert to puppet-appropriate attributes + return entry2provider(result) end + rescue + return nil end end diff --git a/lib/puppet/util/lockfile.rb b/lib/puppet/util/lockfile.rb index 3670ed2c26c..256c5cf2342 100644 --- a/lib/puppet/util/lockfile.rb +++ b/lib/puppet/util/lockfile.rb @@ -25,14 +25,12 @@ def initialize(file_path) # @return [boolean] true if lock is successfully acquired, false otherwise. def lock(lock_data = nil) - begin - Puppet::FileSystem.exclusive_create(@file_path, nil) do |fd| - fd.print(lock_data) - end - true - rescue Errno::EEXIST - false + Puppet::FileSystem.exclusive_create(@file_path, nil) do |fd| + fd.print(lock_data) end + true + rescue Errno::EEXIST + false end def unlock diff --git a/lib/puppet/util/log.rb b/lib/puppet/util/log.rb index e838b267e20..3e7aeac1715 100644 --- a/lib/puppet/util/log.rb +++ b/lib/puppet/util/log.rb @@ -64,7 +64,7 @@ def self.close_all close(dest) } # TRANSLATORS "Log.close_all" is a method name and should not be translated - raise Puppet::DevError.new(_("Log.close_all failed to close %{destinations}") % { destinations: @destinations.keys.inspect }) unless @destinations.empty? + raise Puppet::DevError, _("Log.close_all failed to close %{destinations}") % { destinations: @destinations.keys.inspect } unless @destinations.empty? end # Flush any log destinations that support such operations. diff --git a/lib/puppet/util/metric.rb b/lib/puppet/util/metric.rb index cdae471de27..21c522b249e 100644 --- a/lib/puppet/util/metric.rb +++ b/lib/puppet/util/metric.rb @@ -51,7 +51,7 @@ def initialize(name, label = nil) end def newvalue(name, value, label = nil) - raise ArgumentError.new("metric name #{name.inspect} is not a string") unless name.is_a? String + raise ArgumentError, "metric name #{name.inspect} is not a string" unless name.is_a? String label ||= self.class.labelize(name) @values.push [name, label, value] diff --git a/lib/puppet/util/monkey_patches.rb b/lib/puppet/util/monkey_patches.rb index eb88a2ff172..e4ad9e707c0 100644 --- a/lib/puppet/util/monkey_patches.rb +++ b/lib/puppet/util/monkey_patches.rb @@ -83,11 +83,9 @@ def set_default_paths @puppet_certs_loaded = true Puppet::Util::Windows::RootCerts.instance.to_a.uniq { |cert| cert.to_der }.each do |x509| - begin - add_cert(x509) - rescue OpenSSL::X509::StoreError - warn "Failed to add #{x509.subject.to_utf8}" - end + add_cert(x509) + rescue OpenSSL::X509::StoreError + warn "Failed to add #{x509.subject.to_utf8}" end end diff --git a/lib/puppet/util/plist.rb b/lib/puppet/util/plist.rb index 131391ea3c3..2dc06a6ef8f 100644 --- a/lib/puppet/util/plist.rb +++ b/lib/puppet/util/plist.rb @@ -138,20 +138,18 @@ def to_format(format) when :plain CFPropertyList::List::FORMAT_PLAIN else - raise FormatError.new "Unknown plist format #{format}" + raise FormatError, "Unknown plist format #{format}" end end # This method will write a plist file using a specified format (or XML # by default) def write_plist_file(plist, file_path, format = :xml) - begin - plist_to_save = CFPropertyList::List.new - plist_to_save.value = CFPropertyList.guess(plist) - plist_to_save.save(file_path, to_format(format), :formatted => true) - rescue IOError => e - Puppet.err(_("Unable to write the file %{file_path}. %{error}") % { file_path: file_path, error: e.inspect }) - end + plist_to_save = CFPropertyList::List.new + plist_to_save.value = CFPropertyList.guess(plist) + plist_to_save.save(file_path, to_format(format), :formatted => true) + rescue IOError => e + Puppet.err(_("Unable to write the file %{file_path}. %{error}") % { file_path: file_path, error: e.inspect }) end def dump_plist(plist_data, format = :xml) diff --git a/lib/puppet/util/posix.rb b/lib/puppet/util/posix.rb index 83edcc71ade..1f4f3570bb1 100644 --- a/lib/puppet/util/posix.rb +++ b/lib/puppet/util/posix.rb @@ -131,7 +131,7 @@ def idfield(space) when :gr, :group; return :gid when :pw, :user, :passwd; return :uid else - raise ArgumentError.new(_("Can only handle users and groups")) + raise ArgumentError, _("Can only handle users and groups") end end @@ -141,7 +141,7 @@ def methodbyid(space) when :gr, :group; return :getgrgid when :pw, :user, :passwd; return :getpwuid else - raise ArgumentError.new(_("Can only handle users and groups")) + raise ArgumentError, _("Can only handle users and groups") end end @@ -151,7 +151,7 @@ def methodbyname(space) when :gr, :group; return :getgrnam when :pw, :user, :passwd; return :getpwnam else - raise ArgumentError.new(_("Can only handle users and groups")) + raise ArgumentError, _("Can only handle users and groups") end end diff --git a/lib/puppet/util/storage.rb b/lib/puppet/util/storage.rb index 873c2b75ec9..9740185f231 100644 --- a/lib/puppet/util/storage.rb +++ b/lib/puppet/util/storage.rb @@ -55,16 +55,14 @@ def self.load return end Puppet::Util.benchmark(:debug, "Loaded state in %{seconds} seconds") do + @@state = Puppet::Util::Yaml.safe_load_file(filename, [Symbol, Time]) + rescue Puppet::Util::Yaml::YamlLoadError => detail + Puppet.err _("Checksumfile %{filename} is corrupt (%{detail}); replacing") % { filename: filename, detail: detail } + begin - @@state = Puppet::Util::Yaml.safe_load_file(filename, [Symbol, Time]) - rescue Puppet::Util::Yaml::YamlLoadError => detail - Puppet.err _("Checksumfile %{filename} is corrupt (%{detail}); replacing") % { filename: filename, detail: detail } - - begin - File.rename(filename, filename + ".bad") - rescue - raise Puppet::Error, _("Could not rename corrupt %{filename}; remove manually") % { filename: filename }, detail.backtrace - end + File.rename(filename, filename + ".bad") + rescue + raise Puppet::Error, _("Could not rename corrupt %{filename}; remove manually") % { filename: filename }, detail.backtrace end end diff --git a/lib/puppet/util/suidmanager.rb b/lib/puppet/util/suidmanager.rb index 60e29240857..a9f71f272c0 100644 --- a/lib/puppet/util/suidmanager.rb +++ b/lib/puppet/util/suidmanager.rb @@ -25,23 +25,21 @@ def osx_maj_ver module_function :osx_maj_ver def groups=(grouplist) - begin - Process.groups = grouplist - rescue Errno::EINVAL => e - # We catch Errno::EINVAL as some operating systems (OS X in particular) can - # cause troubles when using Process#groups= to change *this* user / process - # list of supplementary groups membership. This is done via Ruby's function - # "static VALUE proc_setgroups(VALUE obj, VALUE ary)" which is effectively - # a wrapper for "int setgroups(size_t size, const gid_t *list)" (part of SVr4 - # and 4.3BSD but not in POSIX.1-2001) that fails and sets errno to EINVAL. - # - # This does not appear to be a problem with Ruby but rather an issue on the - # operating system side. Therefore we catch the exception and look whether - # we run under OS X or not -- if so, then we acknowledge the problem and - # re-throw the exception otherwise. - if !osx_maj_ver || osx_maj_ver.empty? - raise e - end + Process.groups = grouplist + rescue Errno::EINVAL => e + # We catch Errno::EINVAL as some operating systems (OS X in particular) can + # cause troubles when using Process#groups= to change *this* user / process + # list of supplementary groups membership. This is done via Ruby's function + # "static VALUE proc_setgroups(VALUE obj, VALUE ary)" which is effectively + # a wrapper for "int setgroups(size_t size, const gid_t *list)" (part of SVr4 + # and 4.3BSD but not in POSIX.1-2001) that fails and sets errno to EINVAL. + # + # This does not appear to be a problem with Ruby but rather an issue on the + # operating system side. Therefore we catch the exception and look whether + # we run under OS X or not -- if so, then we acknowledge the problem and + # re-throw the exception otherwise. + if !osx_maj_ver || osx_maj_ver.empty? + raise e end end module_function :groups= diff --git a/lib/puppet/util/symbolic_file_mode.rb b/lib/puppet/util/symbolic_file_mode.rb index 0064bba260b..52421c4ba90 100644 --- a/lib/puppet/util/symbolic_file_mode.rb +++ b/lib/puppet/util/symbolic_file_mode.rb @@ -75,77 +75,75 @@ def symbolic_mode_to_int(modification, to_mode = 0, is_a_directory = false) } modification.split(/\s*,\s*/).each do |part| - begin - _, to, dsl = /^([ugoa]*)([-+=].*)$/.match(part).to_a - if dsl.nil? then raise Puppet::Error, _('Missing action') end - - to = "a" unless to and to.length > 0 - - # We want a snapshot of the mode before we start messing with it to - # make actions like 'a-g' atomic. Various parts of the DSL refer to - # the original mode, the final mode, or the current snapshot of the - # mode, for added fun. - snapshot_mode = {} - final_mode.each { |k, v| snapshot_mode[k] = v } - - to.gsub('a', 'ugo').split('').uniq.each do |who| - value = snapshot_mode[who] - - action = '!' - actions = { - '!' => ->(_, _) { raise Puppet::Error, _('Missing operation (-, =, or +)') }, - '=' => ->(m, v) { m | v }, - '+' => ->(m, v) { m | v }, - '-' => ->(m, v) { m & ~v }, - } - - dsl.split('').each do |op| - case op - when /[-+=]/ - action = op - # Clear all bits, if this is assignment - value = 0 if op == '=' - - when /[ugo]/ - value = actions[action].call(value, snapshot_mode[op]) - - when /[rwx]/ - value = actions[action].call(value, SymbolicMode[op]) - - when 'X' - # Only meaningful in combination with "set" actions. - if action != '+' - raise Puppet::Error, _("X only works with the '+' operator") - end - - # As per the BSD manual page, set if this is a directory, or if - # any execute bit is set on the original (unmodified) mode. - # Ignored otherwise; it is "add if", not "add or clear". - if is_a_directory or original_mode['any x?'] - value = actions[action].call(value, ExecBit) - end - - when /[st]/ - bit = SymbolicSpecialToBit[op][who] or fail _("internal error") - final_mode['s'] = actions[action].call(final_mode['s'], bit) - - else - raise Puppet::Error, _('Unknown operation') + _, to, dsl = /^([ugoa]*)([-+=].*)$/.match(part).to_a + if dsl.nil? then raise Puppet::Error, _('Missing action') end + + to = "a" unless to and to.length > 0 + + # We want a snapshot of the mode before we start messing with it to + # make actions like 'a-g' atomic. Various parts of the DSL refer to + # the original mode, the final mode, or the current snapshot of the + # mode, for added fun. + snapshot_mode = {} + final_mode.each { |k, v| snapshot_mode[k] = v } + + to.gsub('a', 'ugo').split('').uniq.each do |who| + value = snapshot_mode[who] + + action = '!' + actions = { + '!' => ->(_, _) { raise Puppet::Error, _('Missing operation (-, =, or +)') }, + '=' => ->(m, v) { m | v }, + '+' => ->(m, v) { m | v }, + '-' => ->(m, v) { m & ~v }, + } + + dsl.split('').each do |op| + case op + when /[-+=]/ + action = op + # Clear all bits, if this is assignment + value = 0 if op == '=' + + when /[ugo]/ + value = actions[action].call(value, snapshot_mode[op]) + + when /[rwx]/ + value = actions[action].call(value, SymbolicMode[op]) + + when 'X' + # Only meaningful in combination with "set" actions. + if action != '+' + raise Puppet::Error, _("X only works with the '+' operator") end - end - # Now, assign back the value. - final_mode[who] = value - end - rescue Puppet::Error => e - if part.inspect != modification.inspect - rest = " at #{part.inspect}" - else - rest = '' + # As per the BSD manual page, set if this is a directory, or if + # any execute bit is set on the original (unmodified) mode. + # Ignored otherwise; it is "add if", not "add or clear". + if is_a_directory or original_mode['any x?'] + value = actions[action].call(value, ExecBit) + end + + when /[st]/ + bit = SymbolicSpecialToBit[op][who] or fail _("internal error") + final_mode['s'] = actions[action].call(final_mode['s'], bit) + + else + raise Puppet::Error, _('Unknown operation') + end end - raise Puppet::Error, _("%{error}%{rest} in symbolic mode %{modification}") % { error: e, rest: rest, modification: modification.inspect }, e.backtrace + # Now, assign back the value. + final_mode[who] = value end + rescue Puppet::Error => e + if part.inspect != modification.inspect + rest = " at #{part.inspect}" + else + rest = '' + end + + raise Puppet::Error, _("%{error}%{rest} in symbolic mode %{modification}") % { error: e, rest: rest, modification: modification.inspect }, e.backtrace end result = diff --git a/lib/puppet/util/tagging.rb b/lib/puppet/util/tagging.rb index 1b519c83eda..152afe3d1d4 100644 --- a/lib/puppet/util/tagging.rb +++ b/lib/puppet/util/tagging.rb @@ -109,16 +109,14 @@ def tags=(tags) end def valid_tag?(maybe_tag) - begin - tag_enc = maybe_tag.encoding - if tag_enc == Encoding::UTF_8 || tag_enc == Encoding::ASCII - maybe_tag =~ ValidTagRegex - else - maybe_tag.encode(Encoding::UTF_8) =~ ValidTagRegex - end - rescue Encoding::UndefinedConversionError, Encoding::InvalidByteSequenceError - false + tag_enc = maybe_tag.encoding + if tag_enc == Encoding::UTF_8 || tag_enc == Encoding::ASCII + maybe_tag =~ ValidTagRegex + else + maybe_tag.encode(Encoding::UTF_8) =~ ValidTagRegex end + rescue Encoding::UndefinedConversionError, Encoding::InvalidByteSequenceError + false end private diff --git a/lib/puppet/util/watcher.rb b/lib/puppet/util/watcher.rb index 25eb4829fa0..289daa5f2fb 100644 --- a/lib/puppet/util/watcher.rb +++ b/lib/puppet/util/watcher.rb @@ -8,11 +8,9 @@ module Puppet::Util::Watcher module Common def self.file_ctime_change_watcher(filename) Puppet::Util::Watcher::ChangeWatcher.watch(lambda do - begin - Puppet::FileSystem.stat(filename).ctime - rescue Errno::ENOENT, Errno::ENOTDIR - :absent - end + Puppet::FileSystem.stat(filename).ctime + rescue Errno::ENOENT, Errno::ENOTDIR + :absent end) end end diff --git a/lib/puppet/util/windows/adsi.rb b/lib/puppet/util/windows/adsi.rb index 3cce8569df5..f4054196dcd 100644 --- a/lib/puppet/util/windows/adsi.rb +++ b/lib/puppet/util/windows/adsi.rb @@ -24,19 +24,15 @@ class << self extend FFI::Library def connectable?(uri) - begin - !!connect(uri) - rescue - false - end + !!connect(uri) + rescue + false end def connect(uri) - begin - WIN32OLE.connect(uri) - rescue WIN32OLERuntimeError => e - raise Puppet::Error.new(_("ADSI connection error: %{e}") % { e: e }, e) - end + WIN32OLE.connect(uri) + rescue WIN32OLERuntimeError => e + raise Puppet::Error.new(_("ADSI connection error: %{e}") % { e: e }, e) end def create(name, resource_type) @@ -58,7 +54,7 @@ def computer_name buffer_size.write_dword(max_length) # length in TCHARs if GetComputerNameW(buffer, buffer_size) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to get computer name")) + raise Puppet::Util::Windows::Error, _("Failed to get computer name") end @computer_name = buffer.read_wide_string(buffer_size.read_dword) @@ -97,7 +93,7 @@ def sid_uri_safe(sid) # used for IAdsGroup::Add / IAdsGroup::Remove. These URIs are not useable # to resolve an account with WIN32OLE.connect def sid_uri(sid) - raise Puppet::Error.new(_("Must use a valid SID::Principal")) unless sid.is_a?(Puppet::Util::Windows::SID::Principal) + raise Puppet::Error, _("Must use a valid SID::Principal") unless sid.is_a?(Puppet::Util::Windows::SID::Principal) "WinNT://#{sid.sid}" end @@ -162,7 +158,7 @@ def uri(name, host = '.') def parse_name(name) if name =~ /\// - raise Puppet::Error.new(_("Value must be in DOMAIN\\%{object_class} style syntax") % { object_class: @object_class }) + raise Puppet::Error, _("Value must be in DOMAIN\\%{object_class} style syntax") % { object_class: @object_class } end matches = name.scan(/((.*)\\)?(.*)/) @@ -195,7 +191,7 @@ def name_sid_hash(names, allow_unresolved = false) sids = names.map do |name| sid = Puppet::Util::Windows::SID.name_to_principal(name, allow_unresolved) - raise Puppet::Error.new(_("Could not resolve name: %{name}") % { name: name }) unless sid + raise Puppet::Error, _("Could not resolve name: %{name}") % { name: name } unless sid [sid.sid, sid] end @@ -292,9 +288,7 @@ def commit rescue WIN32OLERuntimeError => e # ERROR_BAD_USERNAME 2202L from winerror.h if e.message =~ /8007089A/m - raise Puppet::Error.new( - _("Puppet is not able to create/delete domain %{object_class} objects with the %{object_class} resource.") % { object_class: object_class }, - ) + raise Puppet::Error, _("Puppet is not able to create/delete domain %{object_class} objects with the %{object_class} resource.") % { object_class: object_class } end raise Puppet::Error.new(_("%{object_class} update failed: %{error}") % { object_class: object_class.capitalize, error: e }, e) @@ -323,7 +317,7 @@ def logon(name, password) def create(name) # Windows error 1379: The specified local group already exists. - raise Puppet::Error.new(_("Cannot create user if group '%{name}' exists.") % { name: name }) if Puppet::Util::Windows::ADSI::Group.exists? name + raise Puppet::Error, _("Cannot create user if group '%{name}' exists.") % { name: name } if Puppet::Util::Windows::ADSI::Group.exists? name new(name, Puppet::Util::Windows::ADSI.create(name, @object_class)) end @@ -508,7 +502,7 @@ def self.current_user_name buffer_size.write_dword(max_length) # length in TCHARs if GetUserNameW(buffer, buffer_size) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to get user name")) + raise Puppet::Util::Windows::Error, _("Failed to get user name") end # buffer_size includes trailing NULL @@ -583,17 +577,15 @@ def self.current_user_sid class UserProfile def self.delete(sid) - begin - Puppet::Util::Windows::ADSI.wmi_connection.Delete("Win32_UserProfile.SID='#{sid}'") - rescue WIN32OLERuntimeError => e - # https://social.technet.microsoft.com/Forums/en/ITCG/thread/0f190051-ac96-4bf1-a47f-6b864bfacee5 - # Prior to Vista SP1, there's no built-in way to programmatically - # delete user profiles (except for delprof.exe). So try to delete - # but warn if we fail - raise e unless e.message.include?('80041010') + Puppet::Util::Windows::ADSI.wmi_connection.Delete("Win32_UserProfile.SID='#{sid}'") + rescue WIN32OLERuntimeError => e + # https://social.technet.microsoft.com/Forums/en/ITCG/thread/0f190051-ac96-4bf1-a47f-6b864bfacee5 + # Prior to Vista SP1, there's no built-in way to programmatically + # delete user profiles (except for delprof.exe). So try to delete + # but warn if we fail + raise e unless e.message.include?('80041010') - Puppet.warning _("Cannot delete user profile for '%{sid}' prior to Vista SP1") % { sid: sid } - end + Puppet.warning _("Cannot delete user profile for '%{sid}' prior to Vista SP1") % { sid: sid } end end @@ -609,7 +601,7 @@ def list_all def create(name) # Windows error 2224: The account already exists. - raise Puppet::Error.new(_("Cannot create group if user '%{name}' exists.") % { name: name }) if Puppet::Util::Windows::ADSI::User.exists?(name) + raise Puppet::Error, _("Cannot create group if user '%{name}' exists.") % { name: name } if Puppet::Util::Windows::ADSI::User.exists?(name) new(name, Puppet::Util::Windows::ADSI.create(name, @object_class)) end diff --git a/lib/puppet/util/windows/daemon.rb b/lib/puppet/util/windows/daemon.rb index 853064241df..f0ecdf6dd3b 100644 --- a/lib/puppet/util/windows/daemon.rb +++ b/lib/puppet/util/windows/daemon.rb @@ -145,48 +145,46 @@ class Daemon # Called by the service control manager after the call to StartServiceCtrlDispatcher. Service_Main = FFI::Function.new(:void, [:ulong, :pointer], :blocking => false) do |dwArgc, lpszArgv| - begin - # Obtain the name of the service. - if lpszArgv.address != 0 - argv = lpszArgv.get_array_of_string(0, dwArgc) - lpszServiceName = argv[0] - else - lpszServiceName = '' - end + # Obtain the name of the service. + if lpszArgv.address != 0 + argv = lpszArgv.get_array_of_string(0, dwArgc) + lpszServiceName = argv[0] + else + lpszServiceName = '' + end - # Args passed to Service.start - if dwArgc > 1 - @@Argv = argv[1..] - else - @@Argv = nil - end + # Args passed to Service.start + if dwArgc > 1 + @@Argv = argv[1..] + else + @@Argv = nil + end - # Register the service ctrl handler. - @@ssh = RegisterServiceCtrlHandlerExW( - lpszServiceName, - Service_Ctrl_ex, - nil - ) + # Register the service ctrl handler. + @@ssh = RegisterServiceCtrlHandlerExW( + lpszServiceName, + Service_Ctrl_ex, + nil + ) - # No service to stop, no service handle to notify, nothing to do but exit. - break if @@ssh == 0 + # No service to stop, no service handle to notify, nothing to do but exit. + break if @@ssh == 0 - # The service has started. - SetTheServiceStatus.call(SERVICE_RUNNING, NO_ERROR, 0, 0) + # The service has started. + SetTheServiceStatus.call(SERVICE_RUNNING, NO_ERROR, 0, 0) - SetEvent(@@hStartEvent) + SetEvent(@@hStartEvent) - # Main loop for the service. - while WaitForSingleObject(@@hStopEvent, 1000) != WAIT_OBJECT_0 do - end + # Main loop for the service. + while WaitForSingleObject(@@hStopEvent, 1000) != WAIT_OBJECT_0 do + end - # Main loop for the service. - while WaitForSingleObject(@@hStopCompletedEvent, 1000) != WAIT_OBJECT_0 do - end - ensure - # Stop the service. - SetTheServiceStatus.call(SERVICE_STOPPED, NO_ERROR, 0, 0) + # Main loop for the service. + while WaitForSingleObject(@@hStopCompletedEvent, 1000) != WAIT_OBJECT_0 do end + ensure + # Stop the service. + SetTheServiceStatus.call(SERVICE_STOPPED, NO_ERROR, 0, 0) end ThreadProc = FFI::Function.new(:ulong, [:pointer]) do |lpParameter| @@ -280,36 +278,34 @@ def mainloop end thr = Thread.new do - begin - while WaitForSingleObject(@@hStopEvent, 1000) == WAIT_TIMEOUT - # Check to see if anything interesting has been signaled - case @@waiting_control_code - when SERVICE_CONTROL_PAUSE - service_pause() if respond_to?('service_pause') - when SERVICE_CONTROL_CONTINUE - service_resume() if respond_to?('service_resume') - when SERVICE_CONTROL_INTERROGATE - service_interrogate() if respond_to?('service_interrogate') - when SERVICE_CONTROL_SHUTDOWN - service_shutdown() if respond_to?('service_shutdown') - when SERVICE_CONTROL_PARAMCHANGE - service_paramchange() if respond_to?('service_paramchange') - when SERVICE_CONTROL_NETBINDADD - service_netbindadd() if respond_to?('service_netbindadd') - when SERVICE_CONTROL_NETBINDREMOVE - service_netbindremove() if respond_to?('service_netbindremove') - when SERVICE_CONTROL_NETBINDENABLE - service_netbindenable() if respond_to?('service_netbindenable') - when SERVICE_CONTROL_NETBINDDISABLE - service_netbinddisable() if respond_to?('service_netbinddisable') - end - @@waiting_control_code = IDLE_CONTROL_CODE + while WaitForSingleObject(@@hStopEvent, 1000) == WAIT_TIMEOUT + # Check to see if anything interesting has been signaled + case @@waiting_control_code + when SERVICE_CONTROL_PAUSE + service_pause() if respond_to?('service_pause') + when SERVICE_CONTROL_CONTINUE + service_resume() if respond_to?('service_resume') + when SERVICE_CONTROL_INTERROGATE + service_interrogate() if respond_to?('service_interrogate') + when SERVICE_CONTROL_SHUTDOWN + service_shutdown() if respond_to?('service_shutdown') + when SERVICE_CONTROL_PARAMCHANGE + service_paramchange() if respond_to?('service_paramchange') + when SERVICE_CONTROL_NETBINDADD + service_netbindadd() if respond_to?('service_netbindadd') + when SERVICE_CONTROL_NETBINDREMOVE + service_netbindremove() if respond_to?('service_netbindremove') + when SERVICE_CONTROL_NETBINDENABLE + service_netbindenable() if respond_to?('service_netbindenable') + when SERVICE_CONTROL_NETBINDDISABLE + service_netbinddisable() if respond_to?('service_netbinddisable') end - - service_stop() if respond_to?('service_stop') - ensure - SetEvent(@@hStopCompletedEvent) + @@waiting_control_code = IDLE_CONTROL_CODE end + + service_stop() if respond_to?('service_stop') + ensure + SetEvent(@@hStopCompletedEvent) end if respond_to?('service_main') diff --git a/lib/puppet/util/windows/error.rb b/lib/puppet/util/windows/error.rb index 3fbf7e49b63..ac9f6c88996 100644 --- a/lib/puppet/util/windows/error.rb +++ b/lib/puppet/util/windows/error.rb @@ -42,13 +42,13 @@ def self.format_error_code(code) if length == FFI::WIN32_FALSE # can't raise same error type here or potentially recurse infinitely - raise Puppet::Error.new(_("FormatMessageW could not format code %{code}") % { code: code }) + raise Puppet::Error, _("FormatMessageW could not format code %{code}") % { code: code } end # returns an FFI::Pointer with autorelease set to false, which is what we want buffer_ptr.read_win32_local_pointer do |wide_string_ptr| if wide_string_ptr.null? - raise Puppet::Error.new(_("FormatMessageW failed to allocate buffer for code %{code}") % { code: code }) + raise Puppet::Error, _("FormatMessageW failed to allocate buffer for code %{code}") % { code: code } end error_string = wide_string_ptr.read_wide_string(length) diff --git a/lib/puppet/util/windows/file.rb b/lib/puppet/util/windows/file.rb index 72ef2f0379b..46687befd8b 100644 --- a/lib/puppet/util/windows/file.rb +++ b/lib/puppet/util/windows/file.rb @@ -30,7 +30,7 @@ def replace_file(target, source) return true if result != FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new("ReplaceFile(#{target}, #{source})") + raise Puppet::Util::Windows::Error, "ReplaceFile(#{target}, #{source})" end module_function :replace_file @@ -41,8 +41,7 @@ def move_file_ex(source, target, flags = 0) return true if result != FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error - .new("MoveFileEx(#{source}, #{target}, #{flags.to_s(8)})") + raise Puppet::Util::Windows::Error, "MoveFileEx(#{source}, #{target}, #{flags.to_s(8)})" end module_function :move_file_ex @@ -52,9 +51,7 @@ def symlink(target, symlink) wide_string(target.to_s), flags) return true if result != FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new( - "CreateSymbolicLink(#{symlink}, #{target}, #{flags.to_s(8)})" - ) + raise Puppet::Util::Windows::Error, "CreateSymbolicLink(#{symlink}, #{target}, #{flags.to_s(8)})" end module_function :symlink @@ -93,7 +90,7 @@ def exist?(path) def get_attributes(file_name, raise_on_invalid = true) result = GetFileAttributesW(wide_string(file_name.to_s)) if raise_on_invalid && result == INVALID_FILE_ATTRIBUTES - raise Puppet::Util::Windows::Error.new("GetFileAttributes(#{file_name})") + raise Puppet::Util::Windows::Error, "GetFileAttributes(#{file_name})" end result @@ -120,7 +117,7 @@ def remove_attributes(path, flags) def set_attributes(path, flags) success = SetFileAttributesW(wide_string(path), flags) != FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to set file attributes")) unless success + raise Puppet::Util::Windows::Error, _("Failed to set file attributes") unless success success end @@ -137,11 +134,9 @@ def self.create_file(file_name, desired_access, share_mode, security_attributes, return result unless result == INVALID_HANDLE_VALUE - raise Puppet::Util::Windows::Error.new( - "CreateFile(#{file_name}, #{desired_access.to_s(8)}, #{share_mode.to_s(8)}, " \ - "#{security_attributes}, #{creation_disposition.to_s(8)}, " \ - "#{flags_and_attributes.to_s(8)}, #{template_file_handle})" - ) + raise Puppet::Util::Windows::Error, "CreateFile(#{file_name}, #{desired_access.to_s(8)}, #{share_mode.to_s(8)}, " \ + "#{security_attributes}, #{creation_disposition.to_s(8)}, " \ + "#{flags_and_attributes.to_s(8)}, #{template_file_handle})" end def self.get_reparse_point_data(handle, &block) @@ -156,10 +151,10 @@ def self.get_reparse_point_data(handle, &block) when IO_REPARSE_TAG_MOUNT_POINT MOUNT_POINT_REPARSE_DATA_BUFFER when IO_REPARSE_TAG_NFS - raise Puppet::Util::Windows::Error.new("Retrieving NFS reparse point data is unsupported") + raise Puppet::Util::Windows::Error, "Retrieving NFS reparse point data is unsupported" else - raise Puppet::Util::Windows::Error.new("DeviceIoControl(#{handle}, " \ - "FSCTL_GET_REPARSE_POINT) returned unknown tag 0x#{reparse_tag.to_s(16).upcase}") + raise Puppet::Util::Windows::Error, "DeviceIoControl(#{handle}, " \ + "FSCTL_GET_REPARSE_POINT) returned unknown tag 0x#{reparse_tag.to_s(16).upcase}" end yield buffer_type.new(reparse_data_buffer_ptr) @@ -185,7 +180,7 @@ def self.get_reparse_point_tag(handle) def self.device_io_control(handle, io_control_code, in_buffer = nil, out_buffer = nil) if out_buffer.nil? - raise Puppet::Util::Windows::Error.new(_("out_buffer is required")) + raise Puppet::Util::Windows::Error, _("out_buffer is required") end FFI::MemoryPointer.new(:dword, 1) do |bytes_returned_ptr| @@ -199,11 +194,9 @@ def self.device_io_control(handle, io_control_code, in_buffer = nil, out_buffer ) if result == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new( - "DeviceIoControl(#{handle}, #{io_control_code}, " \ - "#{in_buffer}, #{in_buffer ? in_buffer.size : ''}, " \ - "#{out_buffer}, #{out_buffer ? out_buffer.size : ''}" - ) + raise Puppet::Util::Windows::Error, "DeviceIoControl(#{handle}, #{io_control_code}, " \ + "#{in_buffer}, #{in_buffer ? in_buffer.size : ''}, " \ + "#{out_buffer}, #{out_buffer ? out_buffer.size : ''}" end end @@ -261,7 +254,7 @@ def get_long_pathname(path) buffer_size = GetLongPathNameW(path_ptr, FFI::Pointer::NULL, 0) FFI::MemoryPointer.new(:wchar, buffer_size) do |converted_ptr| if GetLongPathNameW(path_ptr, converted_ptr, buffer_size) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to call GetLongPathName")) + raise Puppet::Util::Windows::Error, _("Failed to call GetLongPathName") end converted = converted_ptr.read_wide_string(buffer_size - 1) @@ -279,7 +272,7 @@ def get_short_pathname(path) buffer_size = GetShortPathNameW(path_ptr, FFI::Pointer::NULL, 0) FFI::MemoryPointer.new(:wchar, buffer_size) do |converted_ptr| if GetShortPathNameW(path_ptr, converted_ptr, buffer_size) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new("Failed to call GetShortPathName") + raise Puppet::Util::Windows::Error, "Failed to call GetShortPathName" end converted = converted_ptr.read_wide_string(buffer_size - 1) diff --git a/lib/puppet/util/windows/principal.rb b/lib/puppet/util/windows/principal.rb index dbc7414601f..988b5739a71 100644 --- a/lib/puppet/util/windows/principal.rb +++ b/lib/puppet/util/windows/principal.rb @@ -73,7 +73,7 @@ def self.lookup_account_name(system_name = nil, sanitize = true, account_name) if LookupAccountNameW(system_name_ptr, account_name_ptr, sid_ptr, sid_length_ptr, domain_ptr, domain_length_ptr, name_use_enum_ptr) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_('Failed to call LookupAccountNameW with account: %{account_name}') % { account_name: account_name }) + raise Puppet::Util::Windows::Error, _('Failed to call LookupAccountNameW with account: %{account_name}') % { account_name: account_name } end # with a SID returned, loop back through lookup_account_sid to retrieve official name @@ -97,7 +97,7 @@ def self.lookup_account_sid(system_name = nil, sid_bytes) system_name_ptr = FFI::Pointer::NULL if sid_bytes.nil? || (!sid_bytes.is_a? Array) || (sid_bytes.length == 0) # TRANSLATORS `lookup_account_sid` is a variable name and should not be translated - raise Puppet::Util::Windows::Error.new(_('Byte array for lookup_account_sid must not be nil and must be at least 1 byte long')) + raise Puppet::Util::Windows::Error, _('Byte array for lookup_account_sid must not be nil and must be at least 1 byte long') end begin @@ -128,7 +128,7 @@ def self.lookup_account_sid(system_name = nil, sid_bytes) FFI::MemoryPointer.new(:lpwstr, domain_length_ptr.read_dword) do |domain_ptr| if LookupAccountSidW(system_name_ptr, sid_ptr, name_ptr, name_length_ptr, domain_ptr, domain_length_ptr, name_use_enum_ptr) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_('Failed to call LookupAccountSidW with bytes: %{sid_bytes}') % { sid_bytes: sid_bytes }) + raise Puppet::Util::Windows::Error, _('Failed to call LookupAccountSidW with bytes: %{sid_bytes}') % { sid_bytes: sid_bytes } end return new( diff --git a/lib/puppet/util/windows/process.rb b/lib/puppet/util/windows/process.rb index 0c327746ab7..6a58bc2ccb3 100644 --- a/lib/puppet/util/windows/process.rb +++ b/lib/puppet/util/windows/process.rb @@ -45,7 +45,7 @@ def wait_process(handle) exit_status = -1 FFI::MemoryPointer.new(:dword, 1) do |exit_status_ptr| if GetExitCodeProcess(handle, exit_status_ptr) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to get child process exit code")) + raise Puppet::Util::Windows::Error, _("Failed to get child process exit code") end exit_status = exit_status_ptr.read_dword @@ -73,9 +73,7 @@ def open_process(desired_access, inherit_handle, process_id, &block) begin phandle = OpenProcess(desired_access, inherit, process_id) if phandle == FFI::Pointer::NULL_HANDLE - raise Puppet::Util::Windows::Error.new( - "OpenProcess(#{desired_access.to_s(8)}, #{inherit}, #{process_id})" - ) + raise Puppet::Util::Windows::Error, "OpenProcess(#{desired_access.to_s(8)}, #{inherit}, #{process_id})" end yield phandle @@ -94,9 +92,7 @@ def open_process_token(handle, desired_access, &block) FFI::MemoryPointer.new(:handle, 1) do |token_handle_ptr| result = OpenProcessToken(handle, desired_access, token_handle_ptr) if result == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new( - "OpenProcessToken(#{handle}, #{desired_access.to_s(8)}, #{token_handle_ptr})" - ) + raise Puppet::Util::Windows::Error, "OpenProcessToken(#{handle}, #{desired_access.to_s(8)}, #{token_handle_ptr})" end yield token_handle = token_handle_ptr.read_handle @@ -137,10 +133,8 @@ def get_process_image_name_by_pid(pid) use_win32_path_format = 0 result = QueryFullProcessImageNameW(phandle, use_win32_path_format, exe_name_ptr, exe_name_length_ptr) if result == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new( - "QueryFullProcessImageNameW(phandle, #{use_win32_path_format}, " \ - "exe_name_ptr, #{max_chars}" - ) + raise Puppet::Util::Windows::Error, "QueryFullProcessImageNameW(phandle, #{use_win32_path_format}, " \ + "exe_name_ptr, #{max_chars}" end image_name = exe_name_ptr.read_wide_string(exe_name_length_ptr.read_dword) end @@ -161,9 +155,7 @@ def lookup_privilege_value(name, system_name = '', &block) ) if result == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new( - "LookupPrivilegeValue(#{system_name}, #{name}, #{luid_ptr})" - ) + raise Puppet::Util::Windows::Error, "LookupPrivilegeValue(#{system_name}, #{name}, #{luid_ptr})" end yield LUID.new(luid_ptr) @@ -181,9 +173,7 @@ def get_token_information(token_handle, token_information, &block) return_length = return_length_ptr.read_dword if return_length <= 0 - raise Puppet::Util::Windows::Error.new( - "GetTokenInformation(#{token_handle}, #{token_information}, nil, 0, #{return_length_ptr})" - ) + raise Puppet::Util::Windows::Error, "GetTokenInformation(#{token_handle}, #{token_information}, nil, 0, #{return_length_ptr})" end # re-call API with properly sized buffer for all results @@ -192,10 +182,8 @@ def get_token_information(token_handle, token_information, &block) token_information_buf, return_length, return_length_ptr) if result == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new( - "GetTokenInformation(#{token_handle}, #{token_information}, #{token_information_buf}, " \ - "#{return_length}, #{return_length_ptr})" - ) + raise Puppet::Util::Windows::Error, "GetTokenInformation(#{token_handle}, #{token_information}, #{token_information_buf}, " \ + "#{return_length}, #{return_length_ptr})" end yield token_information_buf @@ -292,7 +280,7 @@ def windows_major_version result = GetVersionExW(os_version_ptr) if result == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("GetVersionEx failed")) + raise Puppet::Util::Windows::Error, _("GetVersionEx failed") end ver = os_version[:dwMajorVersion] @@ -342,12 +330,12 @@ def set_environment_variable(name, val) FFI::MemoryPointer.from_string_to_wide_string(name) do |name_ptr| if val.nil? if SetEnvironmentVariableW(name_ptr, FFI::MemoryPointer::NULL) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to remove environment variable: %{name}") % { name: name }) + raise Puppet::Util::Windows::Error, _("Failed to remove environment variable: %{name}") % { name: name } end else FFI::MemoryPointer.from_string_to_wide_string(val) do |val_ptr| if SetEnvironmentVariableW(name_ptr, val_ptr) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to set environment variable: %{name}") % { name: name }) + raise Puppet::Util::Windows::Error, _("Failed to set environment variable: %{name}") % { name: name } end end end diff --git a/lib/puppet/util/windows/registry.rb b/lib/puppet/util/windows/registry.rb index 38aecd24bae..6c0d7579d63 100644 --- a/lib/puppet/util/windows/registry.rb +++ b/lib/puppet/util/windows/registry.rb @@ -81,12 +81,10 @@ def values_by_name(key, names) vals = {} names.each do |name| FFI::Pointer.from_string_to_wide_string(name) do |subkeyname_ptr| - begin - _, vals[name] = read(key, subkeyname_ptr) - rescue Puppet::Util::Windows::Error => e - # ignore missing names, but raise other errors - raise e unless e.code == Puppet::Util::Windows::Error::ERROR_FILE_NOT_FOUND - end + _, vals[name] = read(key, subkeyname_ptr) + rescue Puppet::Util::Windows::Error => e + # ignore missing names, but raise other errors + raise e unless e.code == Puppet::Util::Windows::Error::ERROR_FILE_NOT_FOUND end end vals diff --git a/lib/puppet/util/windows/security.rb b/lib/puppet/util/windows/security.rb index 69bf4bdcced..56d3c875a23 100644 --- a/lib/puppet/util/windows/security.rb +++ b/lib/puppet/util/windows/security.rb @@ -173,7 +173,7 @@ def supports_acl?(path) if GetVolumeInformationW(wide_string(root), FFI::Pointer::NULL, 0, FFI::Pointer::NULL, FFI::Pointer::NULL, flags_ptr, FFI::Pointer::NULL, 0) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to get volume information")) + raise Puppet::Util::Windows::Error, _("Failed to get volume information") end supported = flags_ptr.read_dword & FILE_PERSISTENT_ACLS == FILE_PERSISTENT_ACLS @@ -433,11 +433,11 @@ def add_access_allowed_ace(acl, mask, sid, inherit = nil) Puppet::Util::Windows::SID.string_to_sid_ptr(sid) do |sid_ptr| if Puppet::Util::Windows::SID.IsValidSid(sid_ptr) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Invalid SID")) + raise Puppet::Util::Windows::Error, _("Invalid SID") end if AddAccessAllowedAceEx(acl, ACL_REVISION, inherit, mask, sid_ptr) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to add access control entry")) + raise Puppet::Util::Windows::Error, _("Failed to add access control entry") end end @@ -450,11 +450,11 @@ def add_access_denied_ace(acl, mask, sid, inherit = nil) Puppet::Util::Windows::SID.string_to_sid_ptr(sid) do |sid_ptr| if Puppet::Util::Windows::SID.IsValidSid(sid_ptr) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Invalid SID")) + raise Puppet::Util::Windows::Error, _("Invalid SID") end if AddAccessDeniedAceEx(acl, ACL_REVISION, inherit, mask, sid_ptr) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to add access control entry")) + raise Puppet::Util::Windows::Error, _("Failed to add access control entry") end end @@ -465,7 +465,7 @@ def add_access_denied_ace(acl, mask, sid, inherit = nil) def parse_dacl(dacl_ptr) # REMIND: need to handle NULL DACL if IsValidAcl(dacl_ptr) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Invalid DACL")) + raise Puppet::Util::Windows::Error, _("Invalid DACL") end dacl_struct = ACL.new(dacl_ptr) @@ -522,7 +522,7 @@ def open_file(path, access, &block) ) # template if handle == Puppet::Util::Windows::File::INVALID_HANDLE_VALUE - raise Puppet::Util::Windows::Error.new(_("Failed to open '%{path}'") % { path: path }) + raise Puppet::Util::Windows::Error, _("Failed to open '%{path}'") % { path: path } end begin @@ -569,7 +569,7 @@ def set_privilege(privilege, enable) if AdjustTokenPrivileges(token, FFI::WIN32_FALSE, token_privileges, token_privileges.size, FFI::MemoryPointer::NULL, FFI::MemoryPointer::NULL) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to adjust process privileges")) + raise Puppet::Util::Windows::Error, _("Failed to adjust process privileges") end end end @@ -599,7 +599,7 @@ def get_security_descriptor(path) FFI::Pointer::NULL, # sacl sd_ptr_ptr ) # sec desc - raise Puppet::Util::Windows::Error.new(_("Failed to get security information")) if rv != FFI::ERROR_SUCCESS + raise Puppet::Util::Windows::Error, _("Failed to get security information") if rv != FFI::ERROR_SUCCESS # these 2 convenience params are not freed since they point inside sd_ptr owner = Puppet::Util::Windows::SID.sid_ptr_to_string(owner_sid_ptr_ptr.get_pointer(0)) @@ -609,7 +609,7 @@ def get_security_descriptor(path) FFI::MemoryPointer.new(:dword, 1) do |revision| sd_ptr_ptr.read_win32_local_pointer do |sd_ptr| if GetSecurityDescriptorControl(sd_ptr, control, revision) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to get security descriptor control")) + raise Puppet::Util::Windows::Error, _("Failed to get security descriptor control") end protect = (control.read_word & SE_DACL_PROTECTED) == SE_DACL_PROTECTED @@ -642,11 +642,11 @@ def get_max_generic_acl_size(ace_count) def set_security_descriptor(path, sd) FFI::MemoryPointer.new(:byte, get_max_generic_acl_size(sd.dacl.count)) do |acl_ptr| if InitializeAcl(acl_ptr, acl_ptr.size, ACL_REVISION) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to initialize ACL")) + raise Puppet::Util::Windows::Error, _("Failed to initialize ACL") end if IsValidAcl(acl_ptr) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Invalid DACL")) + raise Puppet::Util::Windows::Error, _("Invalid DACL") end with_privilege(SE_BACKUP_NAME) do @@ -681,7 +681,7 @@ def set_security_descriptor(path, sd) FFI::MemoryPointer::NULL) if rv != FFI::ERROR_SUCCESS - raise Puppet::Util::Windows::Error.new(_("Failed to set security information")) + raise Puppet::Util::Windows::Error, _("Failed to set security information") end end end diff --git a/lib/puppet/util/windows/service.rb b/lib/puppet/util/windows/service.rb index 329fb5d074c..cbccb438ce9 100644 --- a/lib/puppet/util/windows/service.rb +++ b/lib/puppet/util/windows/service.rb @@ -111,7 +111,7 @@ def service_state(service_name) end end if state.nil? - raise Puppet::Error.new(_("Unknown Service state '%{current_state}' for '%{service_name}'") % { current_state: state.to_s, service_name: service_name }) + raise Puppet::Error, _("Unknown Service state '%{current_state}' for '%{service_name}'") % { current_state: state.to_s, service_name: service_name } end state @@ -139,7 +139,7 @@ def service_start_type(service_name) end end if start_type.nil? - raise Puppet::Error.new(_("Unknown start type '%{start_type}' for '%{service_name}'") % { start_type: start_type.to_s, service_name: service_name }) + raise Puppet::Error, _("Unknown start type '%{start_type}' for '%{service_name}'") % { start_type: start_type.to_s, service_name: service_name } end start_type @@ -190,7 +190,7 @@ def set_startup_configuration(service_name, options: {}) FFI::Pointer::NULL # lpDisplayName ) if success == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to update service configuration")) + raise Puppet::Util::Windows::Error, _("Failed to update service configuration") end end @@ -255,7 +255,7 @@ def services FFI::Pointer::NULL ) if success == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to fetch services")) + raise Puppet::Util::Windows::Error, _("Failed to fetch services") end # Now that the buffer is populated with services @@ -305,7 +305,7 @@ def open_service(service_name, scm_access, service_access, &block) result = nil open_scm(scm_access) do |scm| service = OpenServiceW(scm, wide_string(service_name), service_access) - raise Puppet::Util::Windows::Error.new(_("Failed to open a handle to the service")) if service == FFI::Pointer::NULL_HANDLE + raise Puppet::Util::Windows::Error, _("Failed to open a handle to the service") if service == FFI::Pointer::NULL_HANDLE result = yield service end @@ -323,7 +323,7 @@ def open_service(service_name, scm_access, service_access, &block) # @param [Integer] scm_access code corresponding to the access type requested for the scm def open_scm(scm_access, &block) scm = OpenSCManagerW(FFI::Pointer::NULL, FFI::Pointer::NULL, scm_access) - raise Puppet::Util::Windows::Error.new(_("Failed to open a handle to the service control manager")) if scm == FFI::Pointer::NULL_HANDLE + raise Puppet::Util::Windows::Error, _("Failed to open a handle to the service control manager") if scm == FFI::Pointer::NULL_HANDLE yield scm ensure @@ -439,7 +439,7 @@ def query_status(service) bytes_pointer ) if success == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Service query failed")) + raise Puppet::Util::Windows::Error, _("Service query failed") end yield status @@ -476,7 +476,7 @@ def query_config(service, &block) bytes_pointer ) if success == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Service query failed")) + raise Puppet::Util::Windows::Error, _("Service query failed") end yield config @@ -520,7 +520,7 @@ def query_config2(service, info_level, &block) bytes_pointer ) if success == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Service query for %{parameter_name} failed") % { parameter_name: SERVICE_CONFIG_TYPES[info_level] }) + raise Puppet::Util::Windows::Error, _("Service query for %{parameter_name} failed") % { parameter_name: SERVICE_CONFIG_TYPES[info_level] } end yield config @@ -544,7 +544,7 @@ def set_optional_parameter(service_name, change, value) value, # lpInfo ) if success == FFI::WIN32_FALSE - raise Puppet::Util.windows::Error.new(_("Failed to update service %{change} configuration") % { change: change }) + raise Puppet::Util.windows::Error, _("Failed to update service %{change} configuration") % { change: change } end end end diff --git a/lib/puppet/util/windows/sid.rb b/lib/puppet/util/windows/sid.rb index bd601545d68..e3d06cc5466 100644 --- a/lib/puppet/util/windows/sid.rb +++ b/lib/puppet/util/windows/sid.rb @@ -99,7 +99,7 @@ class << self; alias name_to_sid_object name_to_principal; end # This method returns a SID::Principal with the account, domain, SID, etc def octet_string_to_principal(bytes) if !bytes || !bytes.respond_to?('pack') || bytes.empty? - raise Puppet::Util::Windows::Error.new(_("Octet string must be an array of bytes")) + raise Puppet::Util::Windows::Error, _("Octet string must be an array of bytes") end Principal.lookup_account_sid(bytes) @@ -116,7 +116,7 @@ class << self; alias octet_string_to_sid_object octet_string_to_principal; end def ads_to_principal(ads_object) if !ads_object || !ads_object.respond_to?(:ole_respond_to?) || !ads_object.ole_respond_to?(:objectSID) || !ads_object.ole_respond_to?(:Name) - raise Puppet::Error.new("ads_object must be an IAdsUser or IAdsGroup instance") + raise Puppet::Error, "ads_object must be an IAdsUser or IAdsGroup instance" end octet_string_to_principal(ads_object.objectSID) @@ -160,18 +160,18 @@ def sid_to_name(value) # Convert a SID pointer to a SID string, e.g. "S-1-5-32-544". def sid_ptr_to_string(psid) if !psid.is_a?(FFI::Pointer) || IsValidSid(psid) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Invalid SID")) + raise Puppet::Util::Windows::Error, _("Invalid SID") end sid_string = nil FFI::MemoryPointer.new(:pointer, 1) do |buffer_ptr| if ConvertSidToStringSidW(psid, buffer_ptr) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to convert binary SID")) + raise Puppet::Util::Windows::Error, _("Failed to convert binary SID") end buffer_ptr.read_win32_local_pointer do |wide_string_ptr| if wide_string_ptr.null? - raise Puppet::Error.new(_("ConvertSidToStringSidW failed to allocate buffer for sid")) + raise Puppet::Error, _("ConvertSidToStringSidW failed to allocate buffer for sid") end sid_string = wide_string_ptr.read_arbitrary_wide_string_up_to(MAXIMUM_SID_STRING_LENGTH) @@ -190,7 +190,7 @@ def string_to_sid_ptr(string_sid, &block) FFI::MemoryPointer.from_string_to_wide_string(string_sid) do |lpcwstr| FFI::MemoryPointer.new(:pointer, 1) do |sid_ptr_ptr| if ConvertStringSidToSidW(lpcwstr, sid_ptr_ptr) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to convert string SID: %{string_sid}") % { string_sid: string_sid }) + raise Puppet::Util::Windows::Error, _("Failed to convert string SID: %{string_sid}") % { string_sid: string_sid } end sid_ptr_ptr.read_win32_local_pointer do |sid_ptr| @@ -221,7 +221,7 @@ def valid_sid?(string_sid) def get_length_sid(sid_ptr) # MSDN states IsValidSid should be called on pointer first if !sid_ptr.is_a?(FFI::Pointer) || IsValidSid(sid_ptr) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Invalid SID")) + raise Puppet::Util::Windows::Error, _("Invalid SID") end GetLengthSid(sid_ptr) diff --git a/lib/puppet/util/windows/user.rb b/lib/puppet/util/windows/user.rb index 567074ba20f..a90a0cf5c41 100644 --- a/lib/puppet/util/windows/user.rb +++ b/lib/puppet/util/windows/user.rb @@ -52,17 +52,17 @@ def check_token_membership size_pointer.write_uint32(SECURITY_MAX_SID_SIZE) if CreateWellKnownSid(:WinBuiltinAdministratorsSid, FFI::Pointer::NULL, sid_pointer, size_pointer) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to create administrators SID")) + raise Puppet::Util::Windows::Error, _("Failed to create administrators SID") end end if IsValidSid(sid_pointer) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Invalid SID")) + raise Puppet::Util::Windows::Error, _("Invalid SID") end FFI::MemoryPointer.new(:win32_bool, 1) do |ismember_pointer| if CheckTokenMembership(FFI::Pointer::NULL_HANDLE, sid_pointer, ismember_pointer) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to check membership")) + raise Puppet::Util::Windows::Error, _("Failed to check membership") end # Is administrators SID enabled in calling thread's access token? @@ -75,18 +75,16 @@ def check_token_membership module_function :check_token_membership def password_is?(name, password, domain = '.') - begin - logon_user(name, password, domain) { |token| } - rescue Puppet::Util::Windows::Error => detail - authenticated_error_codes = Set[ - ERROR_ACCOUNT_RESTRICTION, - ERROR_INVALID_LOGON_HOURS, - ERROR_INVALID_WORKSTATION, - ERROR_ACCOUNT_DISABLED, - ] - - return authenticated_error_codes.include?(detail.code) - end + logon_user(name, password, domain) { |token| } + rescue Puppet::Util::Windows::Error => detail + authenticated_error_codes = Set[ + ERROR_ACCOUNT_RESTRICTION, + ERROR_INVALID_LOGON_HOURS, + ERROR_INVALID_WORKSTATION, + ERROR_ACCOUNT_DISABLED, + ] + + return authenticated_error_codes.include?(detail.code) end module_function :password_is? @@ -101,7 +99,7 @@ def logon_user(name, password, domain = '.', &block) # try logon using network else try logon using interactive mode if logon_user_by_logon_type(name, domain, password, fLOGON32_LOGON_NETWORK, fLOGON32_PROVIDER_DEFAULT, token_pointer) == FFI::WIN32_FALSE if logon_user_by_logon_type(name, domain, password, fLOGON32_LOGON_INTERACTIVE, fLOGON32_PROVIDER_DEFAULT, token_pointer) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to logon user %{name}") % { name: name.inspect }) + raise Puppet::Util::Windows::Error, _("Failed to logon user %{name}") % { name: name.inspect } end end @@ -132,13 +130,13 @@ def load_profile(user, password) # Load the profile. Since it doesn't exist, it will be created if LoadUserProfileW(token, pi.pointer) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to load user profile %{user}") % { user: user.inspect }) + raise Puppet::Util::Windows::Error, _("Failed to load user profile %{user}") % { user: user.inspect } end Puppet.debug("Loaded profile for #{user}") if UnloadUserProfile(token, pi[:hProfile]) == FFI::WIN32_FALSE - raise Puppet::Util::Windows::Error.new(_("Failed to unload user profile %{user}") % { user: user.inspect }) + raise Puppet::Util::Windows::Error, _("Failed to unload user profile %{user}") % { user: user.inspect } end end end @@ -260,7 +258,7 @@ def self.check_lsa_nt_status_and_raise_failures(status, method_name) "The RPC server is unavailable or given domain name is invalid." end - raise Puppet::Error.new("Calling `#{method_name}` returned 'Win32 Error Code 0x%08X'. #{error_reason}" % error_code) + raise Puppet::Error, "Calling `#{method_name}` returned 'Win32 Error Code 0x%08X'. #{error_reason}" % error_code end private_class_method :check_lsa_nt_status_and_raise_failures diff --git a/util/rspec_grouper b/util/rspec_grouper index 8f28fbfcf91..71721ee840a 100755 --- a/util/rspec_grouper +++ b/util/rspec_grouper @@ -97,10 +97,8 @@ if __FILE__ == $0 rescue Exception # Delete all files on an exception paths.each do |path| - begin - File.delete path - rescue Exception - end + File.delete path + rescue Exception end raise end