Skip to content

Commit

Permalink
(PUP-7260) Remove historical_value field from the report
Browse files Browse the repository at this point in the history
This field was used to track previous values when auditing. Since
auditing is being disabled, this field is unnecessary. This commit
removes the field from the report schema, serialization code, and tests.
  • Loading branch information
Magisus committed Jun 7, 2017
1 parent c2e1c14 commit 50f2b6a
Show file tree
Hide file tree
Showing 6 changed files with 6 additions and 28 deletions.
10 changes: 0 additions & 10 deletions api/schemas/report.json
Original file line number Diff line number Diff line change
Expand Up @@ -581,16 +581,6 @@
]
},

"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"
Expand Down
4 changes: 1 addition & 3 deletions lib/puppet/transaction/event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class Puppet::Transaction::Event
include Puppet::Util::Logging
include Puppet::Network::FormatSupport

ATTRIBUTES = [:name, :resource, :property, :previous_value, :desired_value, :historical_value, :status, :message, :file, :line, :source_description, :invalidate_refreshes, :redacted, :corrective_change]
ATTRIBUTES = [:name, :resource, :property, :previous_value, :desired_value, :status, :message, :file, :line, :source_description, :invalidate_refreshes, :redacted, :corrective_change]
attr_accessor *ATTRIBUTES
attr_accessor :time
attr_reader :default_log_level
Expand Down Expand Up @@ -42,7 +42,6 @@ def initialize_from_hash(data)
@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']
Expand All @@ -57,7 +56,6 @@ def to_data_hash
'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,
Expand Down
13 changes: 4 additions & 9 deletions lib/puppet/transaction/resource_harness.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,11 @@ 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]

begin
if param.should && !param.safe_insync?(current_value)
event = create_change_event(param, current_value, historical_value)
event = create_change_event(param, current_value)

if param.noop
noop(event, param, current_value)
Expand All @@ -130,15 +129,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, historical_value)
event = create_change_event(param, current_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, historical_value)
event = create_change_event(param, current_value)
event.status = "failure"
event.message = param.format(_("change from %s to %s failed: "),
param.is_to_s(current_value),
Expand All @@ -154,18 +153,16 @@ def sync_if_needed(param, context)
end
end

def create_change_event(property, current_value, historical_value)
def create_change_event(property, current_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)
Expand Down Expand Up @@ -219,14 +216,12 @@ def new_system_value(property, event, old_system_value)
# @api private
ResourceApplicationContext = Struct.new(:resource,
:current_values,
:historical_values,
: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,
[],
status,
resource.parameters.select { |n,p| p.is_a?(Puppet::Property) && !p.sensitive })
Expand Down
1 change: 0 additions & 1 deletion spec/unit/resource/status_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,6 @@ def events_as_hashes(report)
:property => e.property,
:previous_value => e.previous_value,
:desired_value => e.desired_value,
:historical_value => e.historical_value,
:message => e.message,
:name => e.name,
:status => e.status,
Expand Down
5 changes: 1 addition & 4 deletions spec/unit/transaction/event_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def [](v)
let(:event) do
Puppet::Transaction::Event.new(:source_description => "/my/param", :resource => resource,
:file => "/foo.rb", :line => 27, :tags => %w{one two},
:desired_value => 7, :historical_value => 'Brazil',
:desired_value => 7,
:message => "Help I'm trapped in a spec test",
:name => :mode_changed, :previous_value => 6, :property => :mode,
:status => 'success',
Expand All @@ -141,7 +141,6 @@ def [](v)
:line => 27,
:tags => %w{one two},
:desired_value => 7,
:historical_value => 'Brazil',
:message => "Help I'm trapped in a spec test",
:name => :mode_changed,
:previous_value => 6,
Expand All @@ -153,7 +152,6 @@ def [](v)
expect(tripped.property).to eq(event.property)
expect(tripped.previous_value).to eq(event.previous_value)
expect(tripped.desired_value).to eq(event.desired_value)
expect(tripped.historical_value).to eq(event.historical_value)
expect(tripped.message).to eq(event.message)
expect(tripped.name).to eq(event.name)
expect(tripped.status).to eq(event.status)
Expand All @@ -176,7 +174,6 @@ def [](v)
tripped = Puppet::Transaction::Event.from_data_hash(PSON.parse(event.to_pson))

expect(tripped.desired_value).to be_nil
expect(tripped.historical_value).to be_nil
expect(tripped.name).to be_nil

expect(tripped.property).to eq(event.property)
Expand Down
1 change: 0 additions & 1 deletion spec/unit/transaction/report_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,6 @@ def generate_report
:property => 'message',
:previous_value => SemanticPuppet::VersionRange.parse('>=1.0.0'),
:desired_value => SemanticPuppet::VersionRange.parse('>=1.2.0'),
:historical_value => nil,
:message => "defined 'message' as 'a resource'",
:name => :message_changed,
:status => 'success',
Expand Down

0 comments on commit 50f2b6a

Please sign in to comment.