Merge pull request #5983 from Magisus/re-add-audit

(PUP-7620) Only remove `inspect` command, not `audit` metaparameter

api/schemas/report.json

@@ -362,6 +362,7 @@
                                 "enum": [
                                     "success",
                                     "failure",
+                                    "audit",
                                     "noop",
                                     "total"
                                 ]
@@ -371,6 +372,7 @@
                                 "enum":  [
                                     "Success",
                                     "Failure",
+                                    "Audit",
                                     "Noop",
                                     "Total"
                                 ]
@@ -553,6 +555,11 @@
             "type":        "object",
             "properties": {
 
+                "audited": {
+                    "description": "True if this property is being audited, otherwise false. True in report of `inspect` kind.",
+                    "type":        "boolean"
+                },
+
                 "property": {
                     "description": "The property for which the event occurred.",
                     "oneOf": [
@@ -581,6 +588,16 @@
                     ]
                 },
 
+                "historical_value": {
+                    "description": "The audited value from a previous run of Puppet, if known. Absent in reports of `inspect` kind.",
+                    "oneOf": [
+                        { "type": "string" },
+                        { "type": "array" },
+                        { "type": "object" },
+                        { "type": "null" }
+                    ]
+                },
+
                 "message": {
                     "description": "The log message generated by this event.",
                     "type":        "string"
@@ -597,11 +614,12 @@
                 },
 
                 "status": {
-                    "description": "One of the following strings:\n\n* `success` - property was out of sync, and was successfully changed to be in sync.\n* `failure`- property was out of sync, and couldn't be changed to be in sync due to an error.\n* `noop` - property was out of sync, and wasn't changed due to noop mode.\n\ndepending on the type of the event.\n",
+                    "description": "One of the following strings:\n\n* `success` - property was out of sync, and was successfully changed to be in sync.\n* `failure`- property was out of sync, and couldn't be changed to be in sync due to an error.\n* `noop` - property was out of sync, and wasn't changed due to noop mode.\n* `audit` - property was in sync, and was being audited.\n\ndepending on the type of the event. Always `audit` in reports of `inspect` kind.\n",
                     "enum": [
                         "success",
                         "failure",
-                        "noop"
+                        "noop",
+                        "audit"
                     ]
                 },
                 "time": {
@@ -615,6 +633,7 @@
                 }
             },
             "required": [
+                "audited",
                 "property",
                 "message",
                 "name",

lib/puppet/indirector/resource/ral.rb

@@ -12,13 +12,7 @@ def allow_remote_requests?
   def find( request )
     # find by name
     res   = type(request).instances.find { |o| o.name == resource_name(request) }
-    if !res
-      res = type(request).new(:name => resource_name(request))
-      # Register all of the properties for data collection
-      type(request).properties.collect do |s|
-        res.newattr(s.name.to_sym)
-      end
-    end
+    res ||= type(request).new(:name => resource_name(request), :audit => type(request).properties.collect { |s| s.name })
 
     res.to_resource
   end

lib/puppet/pops/resource/resource_type_impl.rb

@@ -79,6 +79,7 @@ def <=>(other)
   METAPARAMS = [
     :noop,
     :schedule,
+    :audit,
     :loglevel,
     :alias,
     :tag,

lib/puppet/resource/status.rb

@@ -64,7 +64,7 @@ class Status
       attr_reader :change_count
 
       # @!attribute [r] out_of_sync_count
-      #   @return [Integer] A count of the changes made while
+      #   @return [Integer] A count of the audited changes made while
       #     evaluating `@real_resource`.
       attr_reader :out_of_sync_count
 
@@ -122,8 +122,10 @@ def add_event(event)
           @change_count += 1
           @changed = true
         end
-        @out_of_sync_count += 1
-        @out_of_sync = true
+        if event.status != 'audit'
+          @out_of_sync_count += 1
+          @out_of_sync = true
+        end
         if event.corrective_change
           @corrective_change = true
         end

lib/puppet/settings.rb

@@ -787,7 +787,7 @@ def service_user_available?
     return @service_user_available if defined?(@service_user_available)
 
     if self[:user]
-      user = Puppet::Type.type(:user).new :name => self[:user]
+      user = Puppet::Type.type(:user).new :name => self[:user], :audit => :ensure
 
       @service_user_available = user.exists?
     else
@@ -799,7 +799,7 @@ def service_group_available?
     return @service_group_available if defined?(@service_group_available)
 
     if self[:group]
-      group = Puppet::Type.type(:group).new :name => self[:group]
+      group = Puppet::Type.type(:group).new :name => self[:group], :audit => :ensure
 
       @service_group_available = group.exists?
     else

lib/puppet/transaction/event.rb

@@ -11,12 +11,12 @@ class Puppet::Transaction::Event
   include Puppet::Util::Logging
   include Puppet::Network::FormatSupport
 
-  ATTRIBUTES = [:name, :resource, :property, :previous_value, :desired_value, :status, :message, :file, :line, :source_description, :invalidate_refreshes, :redacted, :corrective_change]
+  ATTRIBUTES = [:name, :resource, :property, :previous_value, :desired_value, :historical_value, :status, :message, :file, :line, :source_description, :audited, :invalidate_refreshes, :redacted, :corrective_change]
   attr_accessor *ATTRIBUTES
   attr_accessor :time
   attr_reader :default_log_level
 
-  EVENT_STATUSES = %w{noop success failure }
+  EVENT_STATUSES = %w{noop success failure audit}
 
   def self.from_data_hash(data)
     obj = self.allocate
@@ -25,6 +25,7 @@ def self.from_data_hash(data)
   end
 
   def initialize(options = {})
+    @audited = false
     @redacted = false
     @corrective_change = false
 
@@ -42,9 +43,11 @@ def initialize_from_hash(data)
       :allow_unresolved => true,
       :loader => Puppet::Pops::Loaders.static_loader
     })
+    @audited = data['audited']
     @property = data['property']
     @previous_value = data['previous_value']
     @desired_value = data['desired_value']
+    @historical_value = data['historical_value']
     @message = data['message']
     @name = data['name'].intern if data['name']
     @status = data['status']
@@ -56,9 +59,11 @@ def initialize_from_hash(data)
 
   def to_data_hash
     hash = {
+      'audited' => @audited,
       'property' => @property,
       'previous_value' => @previous_value,
       'desired_value' => @desired_value,
+      'historical_value' => @historical_value,
       'message' => @message,
       'name' => @name.nil? ? nil : @name.to_s,
       'status' => @status,

lib/puppet/transaction/resource_harness.rb

@@ -55,12 +55,12 @@ def schedule(resource)
     resource.catalog.resource(:schedule, name) || resource.fail(_("Could not find schedule %{name}") % { name: name })
   end
 
-  # Used mostly for scheduling at this point.
+  # Used mostly for scheduling and auditing at this point.
   def cached(resource, name)
     Puppet::Util::Storage.cache(resource)[name]
   end
 
-  # Used mostly for scheduling at this point.
+  # Used mostly for scheduling and auditing at this point.
   def cache(resource, name, value)
     Puppet::Util::Storage.cache(resource)[name] = value
   end
@@ -70,6 +70,11 @@ def cache(resource, name, value)
   def perform_changes(resource, context)
     cache(resource, :checked, Time.now)
 
+    # Record the current state in state.yml.
+    context.audited_params.each do |param|
+      cache(resource, param, context.current_values[param])
+    end
+
     ensure_param = resource.parameter(:ensure)
     if ensure_param && ensure_param.should
       ensure_event = sync_if_needed(ensure_param, context)
@@ -87,6 +92,7 @@ def perform_changes(resource, context)
       end
     end
 
+    capture_audit_events(resource, context)
     persist_system_values(resource, context)
   end
 
@@ -109,16 +115,23 @@ def persist_system_values(resource, context)
   end
 
   def sync_if_needed(param, context)
+    historical_value = context.historical_values[param.name]
     current_value = context.current_values[param.name]
+    do_audit = context.audited_params.include?(param.name)
 
     begin
       if param.should && !param.safe_insync?(current_value)
-        event = create_change_event(param, current_value)
+        event = create_change_event(param, current_value, historical_value)
+        if do_audit
+          event = audit_event(event, param, context)
+        end
+
+        brief_audit_message = audit_message(param, do_audit, historical_value, current_value)
 
         if param.noop
-          noop(event, param, current_value)
+          noop(event, param, current_value, brief_audit_message)
         else
-          sync(event, param, current_value)
+          sync(event, param, current_value, brief_audit_message)
         end
 
         event
@@ -129,15 +142,15 @@ def sync_if_needed(param, context)
       # Execution will continue on StandardErrors, just store the event
       Puppet.log_exception(detail)
 
-      event = create_change_event(param, current_value)
+      event = create_change_event(param, current_value, historical_value)
       event.status = "failure"
       event.message = param.format(_("change from %s to %s failed: "),
                                    param.is_to_s(current_value),
                                    param.should_to_s(param.should)) + detail.to_s
       event
     rescue Exception => detail
       # Execution will halt on Exceptions, they get raised to the application
-      event = create_change_event(param, current_value)
+      event = create_change_event(param, current_value, historical_value)
       event.status = "failure"
       event.message = param.format(_("change from %s to %s failed: "),
                                    param.is_to_s(current_value),
@@ -153,40 +166,103 @@ def sync_if_needed(param, context)
     end
   end
 
-  def create_change_event(property, current_value)
+  def create_change_event(property, current_value, historical_value)
     options = {}
     should = property.should
 
     if property.sensitive
       options[:previous_value] = current_value.nil? ? nil : '[redacted]'
       options[:desired_value] = should.nil? ? nil : '[redacted]'
+      options[:historical_value] = historical_value.nil? ? nil : '[redacted]'
     else
       options[:previous_value] = current_value
       options[:desired_value] = should
+      options[:historical_value] = historical_value
     end
 
     property.event(options)
   end
 
-  def noop(event, param, current_value)
+  # This method is an ugly hack because, given a Time object with nanosecond
+  # resolution, roundtripped through YAML serialization, the Time object will
+  # be truncated to microseconds.
+  # For audit purposes, this code special cases this comparison, and compares
+  # the two objects by their second and microsecond components. tv_sec is the
+  # number of seconds since the epoch, and tv_usec is only the microsecond
+  # portion of time.
+  def are_audited_values_equal(a, b)
+    a == b || (a.is_a?(Time) && b.is_a?(Time) && a.tv_sec == b.tv_sec && a.tv_usec == b.tv_usec)
+  end
+  private :are_audited_values_equal
+
+  # Populate an existing event with audit information.
+  #
+  # @param event [Puppet::Transaction::Event] The event to be populated.
+  # @param property [Puppet::Property] The property being audited.
+  # @param context [ResourceApplicationContext]
+  #
+  # @return [Puppet::Transaction::Event] The given event, populated with the audit information.
+  def audit_event(event, property, context)
+    event.audited = true
+    event.status = "audit"
+
+    # The event we've been provided might have been redacted so we need to use the state stored within
+    # the resource application context to see if an event was actually generated.
+    if !are_audited_values_equal(context.historical_values[property.name], context.current_values[property.name])
+      event.message = property.format(_("audit change: previously recorded value %s has been changed to %s"),
+                 property.is_to_s(event.historical_value),
+                 property.is_to_s(event.previous_value))
+    end
+
+    event
+  end
+
+  def audit_message(param, do_audit, historical_value, current_value)
+    if do_audit && historical_value && !are_audited_values_equal(historical_value, current_value)
+      param.format(_(" (previously recorded value was %s)"), param.is_to_s(historical_value))
+    else
+      ""
+    end
+  end
+
+  def noop(event, param, current_value, audit_message)
     event.message = param.format(_("current_value %s, should be %s (noop)"),
                                  param.is_to_s(current_value),
-                                 param.should_to_s(param.should))
+                                 param.should_to_s(param.should)) + audit_message.to_s
     event.status = "noop"
   end
 
-  def sync(event, param, current_value)
+  def sync(event, param, current_value, audit_message)
     param.sync
     if param.sensitive
       event.message = param.format(_("changed %s to %s"),
                                    param.is_to_s(current_value),
-                                   param.should_to_s(param.should))
+                                   param.should_to_s(param.should)) + audit_message.to_s
     else
-      event.message = "#{param.change_to_s(current_value, param.should)}"
+      event.message = "#{param.change_to_s(current_value, param.should)}#{audit_message}"
     end
     event.status = "success"
   end
 
+  def capture_audit_events(resource, context)
+    context.audited_params.each do |param_name|
+      if context.historical_values.include?(param_name)
+        if !are_audited_values_equal(context.historical_values[param_name], context.current_values[param_name]) && !context.synced_params.include?(param_name)
+          parameter = resource.parameter(param_name)
+          event = audit_event(create_change_event(parameter,
+                                                  context.current_values[param_name],
+                                                  context.historical_values[param_name]),
+                              parameter, context)
+          event.send_log
+          context.record(event)
+        end
+      else
+        property = resource.property(param_name)
+        property.notice(property.format(_("audit change: newly-recorded value %s"), context.current_values[param_name]))
+      end
+    end
+  end
+
   # Given an event and its property, calculate the system_value to persist
   # for future calculations.
   # @param [Puppet::Transaction::Event] event event to use for processing
@@ -216,12 +292,16 @@ def new_system_value(property, event, old_system_value)
   # @api private
   ResourceApplicationContext = Struct.new(:resource,
                                           :current_values,
+                                          :historical_values,
+                                          :audited_params,
                                           :synced_params,
                                           :status,
                                           :system_value_params) do
     def self.from_resource(resource, status)
       ResourceApplicationContext.new(resource,
                                      resource.retrieve_resource.to_hash,
+                                     Puppet::Util::Storage.cache(resource).dup,
+                                     (resource[:audit] || []).map { |p| p.to_sym },
                                      [],
                                      status,
                                      resource.parameters.select { |n,p| p.is_a?(Puppet::Property) && !p.sensitive })