From 7e5ca648112a2703ba827f96f36fe2a5f7d1c751 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Thu, 7 Jul 2011 00:03:51 -0700 Subject: Making Fact json handling more resilient We were failing if any values were nil, and values were often nil, at least in testing. We now only include non-nil values, and we handle nil values just fine. Signed-off-by: Luke Kanies Reviewed-by: Nick Lewis --- lib/puppet/node/facts.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'lib/puppet/node') diff --git a/lib/puppet/node/facts.rb b/lib/puppet/node/facts.rb index 577b62b62..8d0a03474 100755 --- a/lib/puppet/node/facts.rb +++ b/lib/puppet/node/facts.rb @@ -61,18 +61,21 @@ class Puppet::Node::Facts def self.from_pson(data) result = new(data['name'], data['values']) - result.timestamp = Time.parse(data['timestamp']) - result.expiration = Time.parse(data['expiration']) + result.timestamp = Time.parse(data['timestamp']) if data['timestamp'] + result.expiration = Time.parse(data['expiration']) if data['expiration'] result end def to_pson(*args) - { - 'expiration' => expiration, + result = { 'name' => name, - 'timestamp' => timestamp, 'values' => strip_internal, - }.to_pson(*args) + } + + result['timestamp'] = timestamp if timestamp + result['expiration'] = expiration if expiration + + result.to_pson(*args) end # Add internal data to the facts for storage. -- cgit From 45ae5b4a9ced26dfcd3e324391f9a26cb02bf93d Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Mon, 18 Jul 2011 22:44:10 -0700 Subject: (#8268) Require windows drive letters in absolute file paths When testing whether a file path is absolute, the regexp was only handling POSIX style file paths. This commit requires Windows style file paths to start with a drive letter. A future commit will refacter the various places we do path validation to support both Windows drive letters and UNC paths. Reviewed-by: Jacob Helwig --- lib/puppet/node/environment.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'lib/puppet/node') diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb index dc631979e..96fdc3c1e 100644 --- a/lib/puppet/node/environment.rb +++ b/lib/puppet/node/environment.rb @@ -136,14 +136,15 @@ class Puppet::Node::Environment end def validate_dirs(dirs) + dir_regex = Puppet.features.microsoft_windows? ? /^[A-Za-z]:#{File::SEPARATOR}/ : /^#{File::SEPARATOR}/ dirs.collect do |dir| - if dir !~ /^#{File::SEPARATOR}/ + if dir !~ dir_regex File.join(Dir.getwd, dir) else dir end end.find_all do |p| - p =~ /^#{File::SEPARATOR}/ && FileTest.directory?(p) + p =~ dir_regex && FileTest.directory?(p) end end -- cgit From d198fedf65e472b384666fc9ae3bef487852068a Mon Sep 17 00:00:00 2001 From: Nick Lewis Date: Thu, 21 Jul 2011 11:53:06 -0700 Subject: Rework Puppet::Util::Cacher to only expire using TTLs We have removed every usage of cached_attr in which the attribute needs to be manually expired. Thus, the only meaningful behavior provided by Puppet::Util::Cacher is expiration based on TTLs. This commit reworks the cacher to only support that behavior. Rather than accepting an options hash, of which :ttl is the only available option, cached_attr now requires a second argument, which is the TTL. TTLs are now used to compute expirations, which are stored and used for expiring values. Previously, we stored a timestamp and used it and the TTL to determine whether the attribute was expired. This had the potentially undesirable side effect that the lifetime of a cached attribute could be extended after its insertion by modifying the TTL setting for the cache. Now, the lifetime of an attribute is determined when it is set, and is thereafter immutable, aside from deliberately re-setting the expiration for that particular attribute. Reviewed-By: Jacob Helwig --- lib/puppet/node/environment.rb | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) (limited to 'lib/puppet/node') diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb index 96fdc3c1e..f25bb65a9 100644 --- a/lib/puppet/node/environment.rb +++ b/lib/puppet/node/environment.rb @@ -95,7 +95,7 @@ class Puppet::Node::Environment # Cache the modulepath, so that we aren't searching through # all known directories all the time. - cached_attr(:modulepath, :ttl => Puppet[:filetimeout]) do + cached_attr(:modulepath, Puppet[:filetimeout]) do dirs = self[:modulepath].split(File::PATH_SEPARATOR) dirs = ENV["PUPPETLIB"].split(File::PATH_SEPARATOR) + dirs if ENV["PUPPETLIB"] validate_dirs(dirs) @@ -103,7 +103,7 @@ class Puppet::Node::Environment # Return all modules from this environment. # Cache the list, because it can be expensive to create. - cached_attr(:modules, :ttl => Puppet[:filetimeout]) do + cached_attr(:modules, Puppet[:filetimeout]) do module_names = modulepath.collect { |path| Dir.entries(path) }.flatten.uniq module_names.collect do |path| begin @@ -114,12 +114,6 @@ class Puppet::Node::Environment end.compact end - # Cache the manifestdir, so that we aren't searching through - # all known directories all the time. - cached_attr(:manifestdir, :ttl => Puppet[:filetimeout]) do - validate_dirs(self[:manifestdir].split(File::PATH_SEPARATOR)) - end - def to_s name.to_s end -- cgit From 9279d0954eb20d75e18a666fd572b5492e157608 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 22 Jul 2011 12:44:15 -0700 Subject: Fix issue with forward and backslashes in Windows paths The environment validates its modulepath and manifestdir settings, but it uses Dir.getwd to convert a relative path into an absolute path. The problem is that on Windows, Dir.getwd returns a path with backslashes. (Interestingly this only happens when puppet is loaded, not in irb for example.) And since we do not yet support backslashes in Windows paths or UNC paths, the directory is not included in the environment. For the time being, I am using File.expand_path to normalize the path. It has the side-effect of converting backslashes to forward slashes. This is sufficient to work around backslashes in Dir.getwd. In the near future, I will be refactoring how paths are split, validated, tested, etc, and I have a REMIND in place to fix the environment. But as a result of this change it exposed a bug in our rdoc parser dealing with the finding the root of a path. The parser assumed that the root was '/', but caused an infinite loop when passed a Windows path. I added a test for this case, which is only run on Windows, because on Unix File.dirname("C:/") == '.'. After all of that, I had to disable one of the rdoc spec tests, because it attempted to reproduce a specific bug, which caused rdoc to try to create a directory of the form: C:/.../files/C:/.... Of course, this fails because ':' is not a valid filename character on Windows. Paired-with: Nick Lewis Reviewed-by: Jacob Helwig --- lib/puppet/node/environment.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'lib/puppet/node') diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb index f25bb65a9..4fc314a6a 100644 --- a/lib/puppet/node/environment.rb +++ b/lib/puppet/node/environment.rb @@ -131,9 +131,12 @@ class Puppet::Node::Environment def validate_dirs(dirs) dir_regex = Puppet.features.microsoft_windows? ? /^[A-Za-z]:#{File::SEPARATOR}/ : /^#{File::SEPARATOR}/ + # REMIND: Dir.getwd on windows returns a path containing backslashes, which when joined with + # dir containing forward slashes, breaks our regex matching. In general, path validation needs + # to be refactored which will be handled in a future commit. dirs.collect do |dir| if dir !~ dir_regex - File.join(Dir.getwd, dir) + File.expand_path(File.join(Dir.getwd, dir)) else dir end -- cgit