Ruby 2.4 compatibility

.travis.yml

@@ -7,9 +7,11 @@ script: "script/cibuild"
 matrix:
   include:
     # Build with latest ruby
-    - rvm: 2.3.1
+    - rvm: 2.4.1
       env: RUBOCOP_TEST="true" RSPEC_TEST="true"
     # Build with older ruby versions
+    - rvm: 2.3.4
+      env: RUBOCOP_TEST="false" RSPEC_TEST="true"
     - rvm: 2.2
       env: RUBOCOP_TEST="false" RSPEC_TEST="true"
     - rvm: 2.1

doc/dev/integration-tests.md

@@ -22,7 +22,7 @@ describe 'whatever behavior' do
     # @result[:logs] is a String containing everything printed to STDERR (Logger)
     # @result[:output] is a String containing everything printed to STDOUT
     # @result[:diffs] is an Array of differences
-    # @result[:exitcode] is a Fixnum representing the exit code: 0 = no changes, 1 = failure, 2 = success, with changes
+    # @result[:exitcode] is an Integer representing the exit code: 0 = no changes, 1 = failure, 2 = success, with changes
     # @result[:exception] contains any exception that was thrown
   end
 
@@ -46,7 +46,7 @@ describe 'whatever behavior' do
     # @result[:logs] is a String containing everything printed to STDERR (Logger)
     # @result[:output] is a String containing everything printed to STDOUT
     # @result[:diffs] is an Array of differences
-    # @result[:exitcode] is a Fixnum representing the exit code: 0 = no changes, 1 = failure, 2 = success, with changes
+    # @result[:exitcode] is an Integer representing the exit code: 0 = no changes, 1 = failure, 2 = success, with changes
     # @result[:exception] contains any exception that was thrown
   end
 

doc/requirements.md

@@ -2,10 +2,10 @@
 
 To run `octocatalog-diff` you will need these basics:
 
-- Ruby 2.0 or higher
+- Ruby 2.0 through 2.4 (we test octocatalog-diff with Ruby 2.0, 2.1, 2.2, 2.3, and 2.4)
 - Mac OS, Linux, or other Unix-line operating system (Windows is not supported)
 - Ability to install gems, e.g. with [rbenv](https://github.com/rbenv/rbenv) or [rvm](https://rvm.io/), or root privileges to install into the system Ruby
-- Puppet agent for [Linux](https://docs.puppet.com/puppet/latest/reference/install_linux.html) or [Mac OS X](https://docs.puppet.com/puppet/latest/reference/install_osx.html), or installed as a gem
+- Puppet agent for [Linux](https://docs.puppet.com/puppet/latest/reference/install_linux.html) or [Mac OS X](https://docs.puppet.com/puppet/latest/reference/install_osx.html), or installed as a gem (we support Puppet 3.8.7 and all versions of Puppet 4.x)
 
 We recommend that you also have the following to get the most out of `octocatalog-diff`, but these are not absolute requirements:
 

lib/octocatalog-diff/api/v1/override.rb

@@ -72,9 +72,9 @@ def convert_to_data_type(datatype, value)
           return value if datatype == 'string'
           return parse_json(value) if datatype == 'json'
           return nil if datatype == 'nil'
-          if datatype == 'fixnum'
+          if datatype == 'fixnum' || datatype == 'integer'
             return Regexp.last_match(1).to_i if value =~ /^(-?\d+)$/
-            raise ArgumentError, "Illegal fixnum '#{value}'"
+            raise ArgumentError, "Illegal integer '#{value}'"
           end
           if datatype == 'float'
             return Regexp.last_match(1).to_f if value =~ /^(-?\d*\.\d+)$/

lib/octocatalog-diff/bootstrap.rb

@@ -10,7 +10,7 @@ class Bootstrap
     # @param options [Hash] Options hash:
     #        :path [String] => Directory to bootstrap
     #        :bootstrap_script [String] => Bootstrap script, relative to directory
-    # @return [Hash] => [Fixnum] :status_code, [String] :output
+    # @return [Hash] => [Integer] :status_code, [String] :output
     def self.bootstrap(options = {})
       # Options validation
       unless options[:path].is_a?(String)

lib/octocatalog-diff/catalog-diff/differ.rb

@@ -530,7 +530,7 @@ def hashdiff_initial(catalog1_in, catalog2_in)
 
           # Added a new key that points to some kind of data structure that we know how
           # to handle.
-          if obj[1] =~ /^(.+)\f([^\f]+)$/ && [String, Fixnum, Float, TrueClass, FalseClass, Array, Hash].include?(obj[2].class)
+          if obj[1] =~ /^(.+)\f([^\f]+)$/ && [String, Integer, Float, TrueClass, FalseClass, Array, Hash].include?(obj[2].class)
             hashdiff_add_remove.add(obj[1])
             next
           end

lib/octocatalog-diff/catalog-diff/display/text.rb

@@ -255,7 +255,7 @@ def self.loc_string(loc, compilation_dir, logger)
         # Get the diff of two long strings. Call the 'diffy' gem for this.
         # @param string1 [String] First string (-)
         # @param string2 [String] Second string (+)
-        # @param depth [Fixnum] Depth, for correct indentation
+        # @param depth [Integer] Depth, for correct indentation
         # @return Array<String> Displayable result
         def self.diff_two_strings_with_diffy(string1, string2, depth)
           # Single line strings?
@@ -324,8 +324,8 @@ def self.make_trailing_whitespace_visible(string_in)
         # Get the diff of two hashes. Call the 'diffy' gem for this.
         # @param hash1 [Hash] First hash (-)
         # @param hash1 [Hash] Second hash (+)
-        # @param depth [Fixnum] Depth, for correct indentation
-        # @param limit [Fixnum] Maximum string length
+        # @param depth [Integer] Depth, for correct indentation
+        # @param limit [Integer] Maximum string length
         # @param strip_diff [Boolean] Strip leading +/-/" "
         # @return [Array<String>] Displayable result
         def self.diff_two_hashes_with_diffy(opts = {})
@@ -358,7 +358,7 @@ def self.diff_two_hashes_with_diffy(opts = {})
         end
 
         # Special case: addition only, no truncation
-        # @param depth [Fixnum] Depth, for correct indentation
+        # @param depth [Integer] Depth, for correct indentation
         # @param hash [Hash] Added object
         # @return [Array<String>] Displayable result
         def self.addition_only_no_truncation(depth, hash)
@@ -383,7 +383,7 @@ def self.addition_only_no_truncation(depth, hash)
 
         # Limit length of a string
         # @param str [String] String
-        # @param limit [Fixnum] Limit (0=unlimited)
+        # @param limit [Integer] Limit (0=unlimited)
         # @return [String] Truncated string
         def self.truncate_string(str, limit)
           return str if limit.nil? || str.length <= limit
@@ -392,7 +392,7 @@ def self.truncate_string(str, limit)
 
         # Get the diff between two hashes. This is recursive-aware.
         # @param obj [diff object] diff object
-        # @param depth [Fixnum] Depth of nesting, used for indentation
+        # @param depth [Integer] Depth of nesting, used for indentation
         # @return Array<String> Printable diff outputs
         def self.hash_diff(obj, depth, key_in, nested = false)
           result = []
@@ -417,7 +417,7 @@ def self.hash_diff(obj, depth, key_in, nested = false)
         end
 
         # Get the diff between two arbitrary objects
-        # @param depth [Fixnum] Depth of nesting, used for indentation
+        # @param depth [Integer] Depth of nesting, used for indentation
         # @param old_obj [?] Old object
         # @param new_obj [?] New object
         # @return Array<String> Diff output
@@ -432,23 +432,42 @@ def self.diff_at_depth(depth, old_obj, new_obj)
 
         # Utility Method!
         # Indent a given text string with a certain number of spaces
-        # @param spaces [Fixnum] Number of spaces
+        # @param spaces [Integer] Number of spaces
         # @param text [String] Text
         def self.left_pad(spaces, text = '')
           [' ' * spaces, text].join('')
         end
 
+        # Utility Method!
+        # Harmonize equivalent class names for comparison purposes.
+        # @param class_name [String] Class name as input
+        # @return [String] Class name as output
+        def self.class_name_for_diffy(class_name)
+          return 'Integer' if class_name == 'Fixnum'
+          class_name
+        end
+
+        # Utility Method!
+        # `is_a?(class)` only allows one method, but this uses an array
+        # @param object [?] Object to consider
+        # @param classes [Array] Classes to determine if object is a member of
+        # @return [Boolean] True if object is_a any of the classes, false otherwise
+        def self.object_is_any_of?(object, classes)
+          classes.each { |clazz| return true if object.is_a? clazz }
+          false
+        end
+
         # Utility Method!
         # Given an arbitrary object, convert it into a string for use by 'diffy'.
         # This basically exists so we can do something prettier than just calling .inspect or .to_s
         # on object types we anticipate seeing, while not failing entirely on other object types.
         # @param obj [?] Object to be stringified
         # @return [String] String representation of object for diffy
         def self.stringify_for_diffy(obj)
-          return JSON.pretty_generate(obj) if [Hash, Array].include?(obj.class)
+          return JSON.pretty_generate(obj) if object_is_any_of?(obj, [Hash, Array])
           return '""' if obj.is_a?(String) && obj == ''
-          return obj if [String, Fixnum, Float].include?(obj.class)
-          "#{obj.class}: #{obj.inspect}"
+          return obj if object_is_any_of?(obj, [String, Fixnum, Integer, Float])
+          "#{class_name_for_diffy(obj.class)}: #{obj.inspect}"
         end
 
         # Utility Method!
@@ -512,8 +531,8 @@ def self._adjust_for_display_datatype(obj1, obj2, option, logger)
           return ['""', 'undef'] if obj2.nil?
 
           # If one is an integer and the other is a string
-          return [obj1, "\"#{obj2}\""] if obj1.is_a?(Fixnum) && obj2.is_a?(String)
-          return ["\"#{obj1}\"", obj2] if obj1.is_a?(String) && obj2.is_a?(Fixnum)
+          return [obj1, "\"#{obj2}\""] if obj1.is_a?(Integer) && obj2.is_a?(String)
+          return ["\"#{obj1}\"", obj2] if obj1.is_a?(String) && obj2.is_a?(Integer)
 
           # True and false
           return [obj1, "\"#{obj2}\""] if obj1.is_a?(TrueClass) && obj2.is_a?(String)

lib/octocatalog-diff/catalog-util/builddir.rb

@@ -21,7 +21,7 @@ class BuildDir
       # Constructor
       # Options for constructor:
       # :puppetdb_url [String] PuppetDB Server URLs
-      # :puppetdb_server_url_timeout [Fixnum] Timeout (seconds) for puppetdb.conf
+      # :puppetdb_server_url_timeout [Integer] Timeout (seconds) for puppetdb.conf
       # :facts [OctocatalogDiff::Facts] Facts object
       # :fact_file [String] File from which to read facts
       # :node [String] Node name
@@ -99,14 +99,14 @@ def create_symlinks(logger = nil)
 
       # Install puppetdb.conf file in temporary directory
       # @param server_urls [String] String for server_urls in puppetdb.conf
-      # @param server_url_timeout [Fixnum] Value for server_url_timeout in puppetdb.conf
+      # @param server_url_timeout [Integer] Value for server_url_timeout in puppetdb.conf
       def install_puppetdb_conf(logger, server_urls, server_url_timeout = 30)
         unless server_urls.is_a?(String)
           raise ArgumentError, "server_urls must be a string, got a: #{server_urls.class}"
         end
 
         server_url_timeout ||= 30 # If called with nil argument, supply default
-        unless server_url_timeout.is_a?(Fixnum)
+        unless server_url_timeout.is_a?(Integer)
           raise ArgumentError, "server_url_timeout must be a fixnum, got a: #{server_url_timeout.class}"
         end
 

lib/octocatalog-diff/catalog-util/command.rb

@@ -167,7 +167,7 @@ def override_and_append_commandline_with_user_supplied_arguments(cmdline)
       # the index.
       # @param cmdline [Array] Existing command line
       # @param key [String] Key to look up
-      # @return [Fixnum] Index of where key is defined (nil if undefined)
+      # @return [Integer] Index of where key is defined (nil if undefined)
       def key_position(cmdline, key)
         cmdline.index { |x| x == "--#{key}" || x =~ /\A--#{key}=/ }
       end

lib/octocatalog-diff/catalog/computed.rb

@@ -22,7 +22,7 @@ class Computed
       # @param :node [String] REQUIRED: Node name
       # @param :basedir [String] Directory in which to compile the catalog
       # @param :pass_env_vars [Array<String>] Environment variables to pass when compiling catalog
-      # @param :retry_failed_catalog [Fixnum] Number of retries if a catalog compilation fails
+      # @param :retry_failed_catalog [Integer] Number of retries if a catalog compilation fails
       # @param :tag [String] For display purposes, the catalog being compiled
       # @param :puppet_binary [String] Full path to Puppet
       # @param :puppet_version [String] Puppet version (optional; if not supplied, it is calculated)

lib/octocatalog-diff/catalog/puppetdb.rb

@@ -15,7 +15,7 @@ class PuppetDB
 
       # Constructor - See OctocatalogDiff::PuppetDB for additional parameters
       # @param :node [String] Node name
-      # @param :retry [Fixnum] Number of retries, if fetch fails
+      # @param :retry [Integer] Number of retries, if fetch fails
       def initialize(options)
         raise ArgumentError, 'Hash of options must be passed to OctocatalogDiff::Catalog::PuppetDB' unless options.is_a?(Hash)
         raise ArgumentError, 'node must be a non-empty string' unless options[:node].is_a?(String) && options[:node] != ''

lib/octocatalog-diff/catalog/puppetmaster.rb

@@ -22,17 +22,17 @@ class PuppetMaster
 
       # Constructor
       # @param :node [String] Node name
-      # @param :retry_failed_catalog [Fixnum] Number of retries, if fetch fails
+      # @param :retry_failed_catalog [Integer] Number of retries, if fetch fails
       # @param :branch [String] Environment to fetch from Puppet Master
       # @param :puppet_master [String] Puppet server and port number (assumed to be DEFAULT_PUPPET_PORT_NUMBER if not given)
-      # @param :puppet_master_api_version [Fixnum] Puppet server API (default DEFAULT_PUPPET_SERVER_API)
+      # @param :puppet_master_api_version [Integer] Puppet server API (default DEFAULT_PUPPET_SERVER_API)
       # @param :puppet_master_ssl_ca [String] Path to file used to sign puppet master's certificate
       # @param :puppet_master_ssl_verify [Boolean] Override the CA verification setting guessed from parameters
       # @param :puppet_master_ssl_client_pem [String] PEM-encoded client key and certificate
       # @param :puppet_master_ssl_client_p12 [String] pkcs12-encoded client key and certificate
       # @param :puppet_master_ssl_client_password [String] Path to file containing password for SSL client key (any format)
       # @param :puppet_master_ssl_client_auth [Boolean] Override the client-auth that is guessed from parameters
-      # @param :timeout [Fixnum] Connection timeout for Puppet master (default=PUPPET_MASTER_TIMEOUT seconds)
+      # @param :timeout [Integer] Connection timeout for Puppet master (default=PUPPET_MASTER_TIMEOUT seconds)
       def initialize(options)
         raise ArgumentError, 'Hash of options must be passed to OctocatalogDiff::Catalog::PuppetMaster' unless options.is_a?(Hash)
         raise ArgumentError, 'node must be a non-empty string' unless options[:node].is_a?(String) && options[:node] != ''

lib/octocatalog-diff/cli.rb

@@ -50,7 +50,7 @@ class Cli
     # @param argv [Array] Use specified arguments (defaults to ARGV)
     # @param logger [Logger] Logger object
     # @param opts [Hash] Additional options
-    # @return [Fixnum] Exit code: 0=no diffs, 1=something went wrong, 2=worked but there are diffs
+    # @return [Integer] Exit code: 0=no diffs, 1=something went wrong, 2=worked but there are diffs
     def self.cli(argv = ARGV, logger = Logger.new(STDERR), opts = {})
       # Save a copy of argv to print out later in debugging
       argv_save = argv.dup

lib/octocatalog-diff/cli/options/puppet_master_timeout.rb

@@ -14,7 +14,7 @@ def parse(parser, options)
       cli_name: 'puppet-master-timeout',
       option_name: 'puppet_master_timeout',
       desc: 'Puppet Master catalog retrieval timeout in seconds',
-      validator: ->(x) { x.to_i > 0 || raise(ArgumentError, 'Specify timeout as a integer greater than 0') },
+      validator: ->(x) { x.to_i > 0 || raise(ArgumentError, 'Specify timeout as an integer greater than 0') },
       translator: ->(x) { x.to_i }
     )
   end

lib/octocatalog-diff/facts/puppetdb.rb

@@ -17,7 +17,7 @@ class PuppetDB
 
       # Retrieve facts from PuppetDB for a specified node.
       # @param :puppetdb_url [String|Array] => URL to PuppetDB
-      # @param :retry [Fixnum] => Retry after timeout (default 0 retries, can be more)
+      # @param :retry [Integer] => Retry after timeout (default 0 retries, can be more)
       # @param node [String] Node name. (REQUIRED for PuppetDB fact source)
       # @return [Hash] Facts
       def self.fact_retriever(options = {}, node)

lib/octocatalog-diff/puppetdb.rb

@@ -38,15 +38,15 @@ class PuppetDB
     # Supported arguments:
     # @param :puppetdb_url [String or Array<String>] PuppetDB URL(s) to try in random order
     # @param :puppetdb_host [String] PuppetDB hostname, when constructing a URL
-    # @param :puppetdb_port [Fixnum] Port number, defaults to 8080 (non-SSL) or 8081 (SSL)
+    # @param :puppetdb_port [Integer] Port number, defaults to 8080 (non-SSL) or 8081 (SSL)
     # @param :puppetdb_ssl [Boolean] defaults to true, because you should use SSL
     # @param :puppetdb_ssl_ca [String] Path to file containing CA certificate
     # @param :puppetdb_ssl_verify [Boolean] Override the CA verification setting guessed from parameters
     # @param :puppetdb_ssl_client_pem [String] PEM-encoded client key and certificate
     # @param :puppetdb_ssl_client_p12 [String] pkcs12-encoded client key and certificate
     # @param :puppetdb_ssl_client_password [String] Path to file containing password for SSL client key (any format)
     # @param :puppetdb_ssl_client_auth [Boolean] Override the client-auth that is guessed from parameters
-    # @param :timeout [Fixnum] Connection timeout for PuppetDB (default=10)
+    # @param :timeout [Integer] Connection timeout for PuppetDB (default=10)
     def initialize(options = {})
       @connections =
         if options.key?(:puppetdb_url)
@@ -149,7 +149,7 @@ def _get(path)
 
     # Parse a URL to determine hostname, port number, and whether or not SSL is used.
     # @param url [String] URL to parse
-    # @return [Hash] { ssl: true/false, host: <String>, port: <Fixnum> }
+    # @return [Hash] { ssl: true/false, host: <String>, port: <Integer> }
     def parse_url(url)
       uri = URI(url)
       raise ArgumentError, "URL #{url} has invalid scheme" unless uri.scheme =~ /^https?$/

lib/octocatalog-diff/util/catalogs.rb

@@ -220,7 +220,7 @@ def build_catalog(opts, logger = @logger)
         time_start = Time.now
         catalog.build(logger)
         time_it_took = Time.now - time_start
-        retries_str = " retries = #{catalog.retries}" if catalog.retries.is_a?(Fixnum)
+        retries_str = " retries = #{catalog.retries}" if catalog.retries.is_a?(Integer)
         time_str = "in #{time_it_took} seconds#{retries_str}"
         status_str = catalog.valid? ? 'successfully built' : 'failed'
         logger.debug "Catalog for #{opts[:branch]} #{status_str} with #{catalog.builder} #{time_str}"

lib/octocatalog-diff/util/httparty.rb

@@ -118,12 +118,20 @@ def self.ssl_options(options)
           else
             raise ArgumentError, 'SSL client auth enabled but no client keypair specified'
           end
-          if result[:pem]
-            result[:pem_password] = options[:ssl_client_password] if options[:ssl_client_password]
-            # Make sure there's not a password required, or that if the password is given, it is correct.
-            # We do not want to wait on STDIN.
-            # This will raise OpenSSL::PKey::RSAError if the key needs a password.
-            OpenSSL::PKey::RSA.new(result[:pem], result[:pem_password] || '')
+
+          # Make sure there's not a password required, or that if the password is given, it is correct.
+          # This will raise OpenSSL::PKey::RSAError if the key needs a password.
+          if result[:pem] && options[:ssl_client_password]
+            result[:pem_password] = options[:ssl_client_password]
+            _trash = OpenSSL::PKey::RSA.new(result[:pem], result[:pem_password])
+          elsif result[:pem]
+            # Ruby 2.4 requires a minimum password length of 4. If no password is needed for
+            # the certificate, the specified password here is effectively ignored.
+            # We do not want to wait on STDIN, so a password-protected certificate without a
+            # password will cause this to raise an error. There are two checks here, to exclude
+            # an edge case where somebody did actually put '1234' as their password.
+            _trash = OpenSSL::PKey::RSA.new(result[:pem], '1234')
+            _trash = OpenSSL::PKey::RSA.new(result[:pem], '5678')
           end
         end