Merge pull request #141 from github/kpaulisse-release

Next octocatalog-diff release

Author: kpaulisse

.travis.yml

@@ -3,6 +3,7 @@
 language: ruby
 install: true
 script: "script/cibuild"
+dist: precise
 
 matrix:
   include:

.version

@@ -1 +1 @@
-1.3.0
+1.4.0

config/puppet-versions.json

@@ -0,0 +1,23 @@
+[
+  {
+    "minimum_version": "3.0.0",
+    "maximum_version": "3.99.99",
+    "additional_gems": [
+      { "name": "safe_yaml", "version": "~> 1.0.4" },
+      { "name": "puppetdb-terminus", "version": "3.2.4" }
+    ]
+  },
+  {
+    "minimum_version": "4.0.0",
+    "maximum_version": "4.99.99",
+    "additional_gems": [
+      { "name": "puppetdb-terminus", "version": "3.2.4" }
+    ]
+  },
+  {
+    "minimum_version": "5.0.0",
+    "maximum_version": "5.99.99",
+    "additional_gems": [
+    ]
+  }
+]

doc/CHANGELOG.md

@@ -8,6 +8,15 @@
 </tr>
 </thead><tbody>
 <tr valign=top>
+<td>1.4.0</td>
+<td>2017-08-03</td>
+<td>
+<li><a href="https://github.com/github/octocatalog-diff/pull/135">#135</a>: (Enhancement) Puppet 5 compatibility</li>
+<li><a href="https://github.com/github/octocatalog-diff/pull/140">#140</a>: (Internal) Prefix tmpdirs with ocd-</li>
+<li><a href="https://github.com/github/octocatalog-diff/pull/138">#138</a>: (Internal) Refactor catalog class with proper inheritance</li>
+</td>
+</tr>
+<tr valign=top>
 <td>1.3.0</td>
 <td>2017-06-09</td>
 <td>

doc/advanced-catalog-validation.md

@@ -4,12 +4,14 @@
 
 Catalog validation features include:
 
-- Validate references: Ensure resources targeted by `before`, `notify`, `require`, and/or `subscribe` exist in the catalog
+- Validate references: Ensure resources targeted by `before`, `notify`, `require`, and/or `subscribe` exist in the catalog for Puppet 4 and below.
 
 ## Validate references
 
 `octocatalog-diff` includes the ability to validate references by ensuring resources targeted by `before`, `notify`, `require`, and/or `subscribe` parameters also exist in the catalog.
 
+Puppet 5 already has this checking built in, so the `--validate-references` option described in this section will be ignored if Puppet 5 is being used. The same exception (`OctocatalogDiff::Errors::CatalogError`) is raised for a missing reference, whether the problem was detected by octocatalog-diff or Puppet 5.
+
 Consider the following Puppet code:
 
 ```

doc/dev/api/v1/calls/catalog-diff.md

@@ -194,7 +194,7 @@ The following exceptions may occur during the compilation of a catalog within th
 
 - `OctocatalogDiff::Errors::CatalogError`
 
-  Catalog failed to compile. Please note that whenever possible, a `OctocatalogDiff::API::V1::Catalog` object is still constructed for a failed catalog, with `#valid?` returning false.
+  Catalog failed to compile. Please note that whenever possible, a `OctocatalogDiff::API::V1::Catalog` object is still constructed for a failed catalog, with `#valid?` returning false. It's also possible that the catalog contained broken references -- see [Catalog validation](/doc/advanced-catalog-validation.md).
 
 - `OctocatalogDiff::Errors::GitCheckoutError`
 
@@ -203,7 +203,3 @@ The following exceptions may occur during the compilation of a catalog within th
 - `OctocatalogDiff::Errors::PuppetVersionError`
 
   The version of Puppet could not be determined, generally because the Puppet binary was not found, or does not respond as expected to `puppet version`.
-
-- `OctocatalogDiff::Errors::ReferenceValidationError`
-
-  See [Catalog validation](/doc/advanced-catalog-validation.md).

doc/dev/api/v1/calls/catalog.md

@@ -100,7 +100,7 @@ The following exceptions may occur during the compilation of a catalog:
 
 - `OctocatalogDiff::Errors::CatalogError`
 
-  Catalog failed to compile. Please note that whenever possible, a `OctocatalogDiff::API::V1::Catalog` object is still constructed for a failed catalog, with `#valid?` returning false.
+  Catalog failed to compile. Please note that whenever possible, a `OctocatalogDiff::API::V1::Catalog` object is still constructed for a failed catalog, with `#valid?` returning false. It's also possible that the catalog contained broken references -- see [Catalog validation](/doc/advanced-catalog-validation.md).
 
 - `OctocatalogDiff::Errors::GitCheckoutError`
 
@@ -109,7 +109,3 @@ The following exceptions may occur during the compilation of a catalog:
 - `OctocatalogDiff::Errors::PuppetVersionError`
 
   The version of Puppet could not be determined, generally because the Puppet binary was not found, or does not respond as expected to `puppet version`.
-
-- `OctocatalogDiff::Errors::ReferenceValidationError`
-
-  See [Catalog validation](/doc/advanced-catalog-validation.md).

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

@@ -38,7 +38,7 @@ class BuildDir
       # @param options [Hash] Options for class; see above description
       def initialize(options = {}, logger = nil)
         @options = options.dup
-        @tempdir = Dir.mktmpdir
+        @tempdir = Dir.mktmpdir('ocd-builddir-')
         at_exit { FileUtils.rm_rf(@tempdir) if File.directory?(@tempdir) }
 
         @factdir = nil

lib/octocatalog-diff/catalog.rb

@@ -12,14 +12,42 @@
 require_relative 'errors'
 
 module OctocatalogDiff
-  # This class represents a catalog. Generation of the catalog is handled via one of the
+  # Basic methods for interacting with a catalog. Generation of the catalog is handled via one of the
   # supported backends listed above as 'require_relative'. Usually, the 'computed' backend
   # will build the catalog from the Puppet command.
   class Catalog
-    # Readable
-    attr_reader :built, :catalog, :catalog_json
+    attr_accessor :node
+    attr_reader :built, :catalog, :catalog_json, :options
 
     # Constructor
+    def initialize(options = {})
+      unless options.is_a?(Hash)
+        raise ArgumentError, "#{self.class}.initialize requires hash argument, not #{options.class}"
+      end
+      @options = options
+
+      # Basic settings
+      @node = options[:node]
+      @error_message = nil
+      @catalog = nil
+      @catalog_json = nil
+      @retries = nil
+
+      # The compilation directory can be overridden, e.g. when testing
+      @override_compilation_dir = options[:compilation_dir]
+
+      # Keep track of whether references have been validated yet. Allow this to be fudged for when we do
+      # not desire reference validation to happen (e.g., for the "from" catalog that is otherwise valid).
+      @references_validated = options[:references_validated] || false
+
+      # Keep track of whether file resources have been converted.
+      @file_resources_converted = false
+
+      # Keep track of whether it's built yet
+      @built = false
+    end
+
+    # Guess the backend from the input and return the appropriate catalog object.
     # @param :backend [Symbol] If set, this will force a backend
     # @param :json [String] JSON catalog content (will avoid running Puppet to compile catalog)
     # @param :puppetdb [Object] If set, pull the catalog from PuppetDB rather than building
@@ -30,62 +58,68 @@ class Catalog
     # @param :pass_env_vars [Array<String>] OPTIONAL: Additional environment vars to pass
     # @param :convert_file_resources [Boolean] OPTIONAL: Convert file resource source to content
     # @param :storeconfigs [Boolean] OPTIONAL: Pass the '-s' flag, for puppetdb (storeconfigs) integration
-    def initialize(options = {})
-      @options = options
-
-      # Call appropriate backend for catalog generation
-      @catalog_obj = backend(options)
+    # @return [OctocatalogDiff::Catalog::<?>] Catalog object from guessed backend
+    def self.create(options = {})
+      # Hard-coded backend
+      if options[:backend]
+        return OctocatalogDiff::Catalog::JSON.new(options) if options[:backend] == :json
+        return OctocatalogDiff::Catalog::PuppetDB.new(options) if options[:backend] == :puppetdb
+        return OctocatalogDiff::Catalog::PuppetMaster.new(options) if options[:backend] == :puppetmaster
+        return OctocatalogDiff::Catalog::Computed.new(options) if options[:backend] == :computed
+        return OctocatalogDiff::Catalog::Noop.new(options) if options[:backend] == :noop
+        raise ArgumentError, "Unknown backend :#{options[:backend]}"
+      end
 
-      # The catalog is not built yet, except if the backend has no build method
-      @built = false
-      build unless @catalog_obj.respond_to?(:build)
+      # Determine backend based on arguments
+      return OctocatalogDiff::Catalog::JSON.new(options) if options[:json]
+      return OctocatalogDiff::Catalog::PuppetDB.new(options) if options[:puppetdb]
+      return OctocatalogDiff::Catalog::PuppetMaster.new(options) if options[:puppet_master]
 
-      # The compilation directory can be overridden, e.g. when testing
-      @override_compilation_dir = nil
+      # Default is to build catalog ourselves
+      OctocatalogDiff::Catalog::Computed.new(options)
     end
 
     # Build catalog - this method needs to be called to build the catalog. It is separate due to
     # the serialization of the logger object -- the parallel gem cannot serialize/deserialize a logger
     # object so it cannot be part of any object that is passed around.
     # @param logger [Logger] Logger object, initialized to a default throwaway value
     def build(logger = Logger.new(StringIO.new))
-      # Only build once
+      # If already built, don't build again
       return if @built
       @built = true
 
-      # Call catalog's build method.
-      if @catalog_obj.respond_to?(:build)
-        logger.debug "Calling build for object #{@catalog_obj.class}"
-        @catalog_obj.build(logger)
-      end
-
-      # These methods must exist in all backends
-      @catalog = @catalog_obj.catalog
-      @catalog_json = @catalog_obj.catalog_json
-      @error_message = @catalog_obj.error_message
-
       # The resource hash is computed the first time it's needed. For now initialize it as nil.
       @resource_hash = nil
 
+      # Invoke the backend's build method, if there is one. There's a stub below in case there's not.
+      logger.debug "Calling build for object #{self.class}"
+      build_catalog(logger)
+
       # Perform post-generation processing of the catalog
       return unless valid?
-      unless @catalog_obj.respond_to?(:convert_file_resources) && @catalog_obj.convert_file_resources == false
-        if @options.fetch(:compare_file_text, false)
-          OctocatalogDiff::CatalogUtil::FileResources.convert_file_resources(self, environment)
-        end
-      end
+
+      validate_references
+      return unless valid?
+
+      convert_file_resources if @options[:compare_file_text]
+
+      true
+    end
+
+    # Stub method if the backend does not contain a build method.
+    def build_catalog(_logger)
     end
 
     # Compilation environment
     # @return [String] Compilation environment (if set), else 'production' by default
     def environment
-      @catalog_obj.respond_to?(:environment) ? @catalog_obj.environment : 'production'
+      @environment ||= 'production'
     end
 
     # For logging we may wish to know the backend being used
     # @return [String] Class of backend used
     def builder
-      @catalog_obj.class.to_s
+      self.class.to_s
     end
 
     # Set the catalog JSON
@@ -98,8 +132,7 @@ def catalog_json=(str)
     # This retrieves the compilation directory from the catalog, or otherwise the passed-in directory.
     # @return [String] Compilation directory
     def compilation_dir
-      return @override_compilation_dir if @override_compilation_dir
-      @catalog_obj.respond_to?(:compilation_dir) ? @catalog_obj.compilation_dir : @options[:basedir]
+      @override_compilation_dir || @options[:basedir]
     end
 
     # The compilation directory can be overridden, e.g. during testing.
@@ -108,16 +141,16 @@ def compilation_dir=(dir)
       @override_compilation_dir = dir
     end
 
-    # Determine whether the underlying catalog object supports :compare_file_text
-    # @return [Boolean] Whether underlying catalog object supports :compare_file_text
-    def convert_file_resources
-      return true unless @catalog_obj.respond_to?(:convert_file_resources)
-      @catalog_obj.convert_file_resources
+    # Stub method for "convert_file_resources" -- returns false because if the underlying class does
+    # not implement this method, it's not supported.
+    def convert_file_resources(_dry_run = false)
+      false
     end
 
     # Retrieve the error message.
     # @return [String] Error message (maximum 20,000 characters) - nil if no error.
     def error_message
+      build
       return nil if @error_message.nil? || !@error_message.is_a?(String)
       @error_message[0, 20_000]
     end
@@ -133,12 +166,11 @@ def error_message=(error)
       @resource_hash = nil
     end
 
-    # This retrieves the version of Puppet used to compile a catalog. If the underlying catalog was not
-    # compiled by running Puppet (e.g., it was read in from JSON or puppetdb), then this returns the
-    # puppet version optionally passed in to the constructor. This can also be nil.
+    # Stub method to return the puppet version if the back end doesn't support this.
     # @return [String] Puppet version
     def puppet_version
-      @catalog_obj.respond_to?(:puppet_version) ? @catalog_obj.puppet_version : @options[:puppet_version]
+      build
+      @options[:puppet_version]
     end
 
     # This allows retrieving a resource by type and title. This is intended for use when a O(1) lookup is required.
@@ -147,6 +179,7 @@ def puppet_version
     # @return [Hash] Resource item
     def resource(opts = {})
       raise ArgumentError, ':type and :title are required' unless opts[:type] && opts[:title]
+      build
       build_resource_hash if @resource_hash.nil?
       return nil unless @resource_hash[opts[:type]].is_a?(Hash)
       @resource_hash[opts[:type]][opts[:title]]
@@ -155,6 +188,7 @@ def resource(opts = {})
     # This is a compatibility layer for the resources, which are in a different place in Puppet 3.x and Puppet 4.x
     # @return [Array] Resource array
     def resources
+      build
       raise OctocatalogDiff::Errors::CatalogError, 'Catalog does not appear to have been built' if !valid? && error_message.nil?
       raise OctocatalogDiff::Errors::CatalogError, error_message unless valid?
       return @catalog['data']['resources'] if @catalog['data'].is_a?(Hash) && @catalog['data']['resources'].is_a?(Array)
@@ -165,28 +199,37 @@ def resources
       # :nocov:
     end
 
-    # This retrieves the number of retries necessary to compile the catalog. If the underlying catalog
+    # Stub method of the the number of retries necessary to compile the catalog. If the underlying catalog
     # generation backend does not support retries, nil is returned.
     # @return [Integer] Retry count
     def retries
-      @retries = @catalog_obj.respond_to?(:retries) ? @catalog_obj.retries : nil
+      nil
     end
 
     # Determine if the catalog build was successful.
     # @return [Boolean] Whether the catalog is valid
     def valid?
+      build
       !@catalog.nil?
     end
 
     # Determine if all of the (before, notify, require, subscribe) targets are actually in the catalog.
     # Raise a OctocatalogDiff::Errors::ReferenceValidationError for any found to be missing.
     # Uses @options[:validate_references] to influence which references are checked.
     def validate_references
+      # If we've already done the validation, don't do it again
+      return if @references_validated
+      @references_validated = true
+
       # Skip out early if no reference validation has been requested.
       unless @options[:validate_references].is_a?(Array) && @options[:validate_references].any?
         return
       end
 
+      # Puppet 5 has reference validation built-in and enabled, so there won't even be a valid catalog if
+      # there were invalid references. It's pointless to perform validation of our own.
+      return if puppet_version && puppet_version >= '5.0.0'
+
       # Iterate over all the resources and check each one that has one of the attributes being checked.
       # Keep track of all references that are missing for ultimate inclusion in the error message.
       missing = []
@@ -204,7 +247,7 @@ def validate_references
       # At this point there is at least one broken/missing reference. Format an error message and raise.
       errors = format_missing_references(missing)
       plural = errors =~ /;/ ? 's' : ''
-      raise OctocatalogDiff::Errors::ReferenceValidationError, "Catalog has broken reference#{plural}: #{errors}"
+      self.error_message = "Catalog has broken reference#{plural}: #{errors}"
     end
 
     private
@@ -265,29 +308,6 @@ def resources_missing_from_catalog(resources_to_check)
       end
     end
 
-    # Private method: Choose backend based on passed-in options
-    # @param options [Hash] Options passed into constructor
-    # @return [Object] OctocatalogDiff::Catalog::<whatever> object
-    def backend(options)
-      # Hard-coded backend
-      if options[:backend]
-        return OctocatalogDiff::Catalog::JSON.new(options) if options[:backend] == :json
-        return OctocatalogDiff::Catalog::PuppetDB.new(options) if options[:backend] == :puppetdb
-        return OctocatalogDiff::Catalog::PuppetMaster.new(options) if options[:backend] == :puppetmaster
-        return OctocatalogDiff::Catalog::Computed.new(options) if options[:backend] == :computed
-        return OctocatalogDiff::Catalog::Noop.new(options) if options[:backend] == :noop
-        raise ArgumentError, "Unknown backend :#{options[:backend]}"
-      end
-
-      # Determine backend based on arguments
-      return OctocatalogDiff::Catalog::JSON.new(options) if options[:json]
-      return OctocatalogDiff::Catalog::PuppetDB.new(options) if options[:puppetdb]
-      return OctocatalogDiff::Catalog::PuppetMaster.new(options) if options[:puppet_master]
-
-      # Default is to build catalog ourselves
-      OctocatalogDiff::Catalog::Computed.new(options)
-    end
-
     # Private method: Build the resource hash to be used used for O(1) lookups by type and title.
     # This method is called the first time the resource hash is accessed.
     def build_resource_hash