diff --git a/lib/puppet/node/facts.rb b/lib/puppet/node/facts.rb index aaa8932fa90..20288cb0091 100644 --- a/lib/puppet/node/facts.rb +++ b/lib/puppet/node/facts.rb @@ -21,7 +21,7 @@ def save(instance, key = nil, options={}) indirects :facts, :terminus_setting => :facts_terminus, :extend => NodeExpirer - attr_accessor :name, :values + attr_accessor :name, :values, :timestamp def add_local_facts values["clientcert"] = Puppet.settings[:certname] @@ -39,16 +39,16 @@ def initialize(name, values = {}) def initialize_from_hash(data) @name = data['name'] @values = data['values'] - # Timestamp will be here in YAML - timestamp = data['values']['_timestamp'] - @values.delete_if do |key, val| - key =~ /^_/ - end - - #Timestamp will be here in pson + # Timestamp will be here in YAML, e.g. when reading old reports + timestamp = @values.delete('_timestamp') + # Timestamp will be here in JSON timestamp ||= data['timestamp'] - timestamp = Time.parse(timestamp) if timestamp.is_a? String - self.timestamp = timestamp + + if timestamp.is_a? String + @timestamp = Time.parse(timestamp) + else + @timestamp = timestamp + end self.expiration = data['expiration'] if expiration.is_a? String @@ -66,7 +66,7 @@ def sanitize def ==(other) return false unless self.name == other.name - strip_internal == other.send(:strip_internal) + values == other.values end def self.from_data_hash(data) @@ -83,14 +83,14 @@ def self.from_pson(data) def to_data_hash result = { 'name' => name, - 'values' => strip_internal, + 'values' => values } - if timestamp - if timestamp.is_a? Time - result['timestamp'] = timestamp.iso8601(9) + if @timestamp + if @timestamp.is_a? Time + result['timestamp'] = @timestamp.iso8601(9) else - result['timestamp'] = timestamp + result['timestamp'] = @timestamp end end @@ -105,24 +105,13 @@ def to_data_hash result end - # Add internal data to the facts for storage. def add_timestamp - self.timestamp = Time.now - end - - def timestamp=(time) - self.values['_timestamp'] = time - end - - def timestamp - self.values['_timestamp'] + @timestamp = Time.now end - # Strip out that internal data. + # @deprecated Use {#values} instead of this method. def strip_internal - newvals = values.dup - newvals.find_all { |name, value| name.to_s =~ /^_/ }.each { |name, value| newvals.delete(name) } - newvals + values end private diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb index c8cfa6060b3..6d8b49edcda 100644 --- a/lib/puppet/parser/scope.rb +++ b/lib/puppet/parser/scope.rb @@ -655,10 +655,6 @@ def set_trusted(hash) end def set_facts(hash) - # Remove _timestamp (it has an illegal datatype). It is not allowed to mutate the given hash - # since it contains the facts. - hash = hash.dup - hash.delete('_timestamp') setvar('facts', deep_freeze(hash), :privileged => true) end diff --git a/spec/unit/node/facts_spec.rb b/spec/unit/node/facts_spec.rb index b659434590f..20e87435f8b 100755 --- a/spec/unit/node/facts_spec.rb +++ b/spec/unit/node/facts_spec.rb @@ -112,9 +112,51 @@ end describe "when storing and retrieving" do - it "should add metadata to the facts" do - facts = Puppet::Node::Facts.new("me", "one" => "two", "three" => "four") - facts.values['_timestamp'].should be_instance_of(Time) + it "doesn't manufacture a `_timestamp` fact value" do + values = {"one" => "two", "three" => "four"} + facts = Puppet::Node::Facts.new("mynode", values) + + expect(facts.values).to eq(values) + end + + describe "when deserializing from yaml" do + let(:timestamp) { Time.parse("Thu Oct 28 11:16:31 -0700 2010") } + let(:expiration) { Time.parse("Thu Oct 28 11:21:31 -0700 2010") } + + def create_facts(values = {}) + Puppet::Node::Facts.new('mynode', values) + end + + def deserialize_yaml_facts(facts) + format = Puppet::Network::FormatHandler.format('yaml') + format.intern(Puppet::Node::Facts, facts.to_yaml) + end + + it 'preserves `_timestamp` value' do + facts = deserialize_yaml_facts(create_facts('_timestamp' => timestamp)) + + expect(facts.timestamp).to eq(timestamp) + end + + it "doesn't preserve the `_timestamp` fact" do + facts = deserialize_yaml_facts(create_facts('_timestamp' => timestamp)) + + expect(facts.values['_timestamp']).to be_nil + end + + it 'preserves expiration time if present' do + old_facts = create_facts + old_facts.expiration = expiration + facts = deserialize_yaml_facts(old_facts) + + expect(facts.expiration).to eq(expiration) + end + + it 'ignores expiration time if absent' do + facts = deserialize_yaml_facts(create_facts) + + expect(facts.expiration).to be_nil + end end describe "using pson" do @@ -129,7 +171,8 @@ facts = format.intern(Puppet::Node::Facts,pson) facts.name.should == 'foo' facts.expiration.should == @expiration - facts.values.should == {'a' => '1', 'b' => '2', 'c' => '3', '_timestamp' => @timestamp} + facts.timestamp.should == @timestamp + facts.values.should == {'a' => '1', 'b' => '2', 'c' => '3'} end it "should generate properly formatted pson" do @@ -138,7 +181,7 @@ facts.expiration = @expiration result = PSON.parse(facts.to_pson) result['name'].should == facts.name - result['values'].should == facts.values.reject { |key, value| key.to_s =~ /_/ } + result['values'].should == facts.values result['timestamp'].should == facts.timestamp.iso8601(9) result['expiration'].should == facts.expiration.iso8601(9) end