From a215abaef97ea1fb0187f46c5e6a880ff1d29036 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Thu, 21 Aug 2008 23:09:27 -0500 Subject: Dividing server/port configuration responsibility between the REST terminus and the indirection request. Previously, the REST terminus did all of the configuration, but it required rewriting the request key if it was a URI because you can't have a uri in a uri (i.e., you can't use http://host/puppet://host/dist/file). Now the request parses the URI and sets host/port/key/protocol appropriately, and the REST terminus has its own overrides and defaults so that subclasses like certificate classes can provide specific values. Signed-off-by: Luke Kanies --- lib/puppet/indirector/request.rb | 36 +++++++++++++++++++++++++++++++++++- lib/puppet/indirector/rest.rb | 40 ++++++++++++++++++++++++++-------------- 2 files changed, 61 insertions(+), 15 deletions(-) (limited to 'lib') diff --git a/lib/puppet/indirector/request.rb b/lib/puppet/indirector/request.rb index 98fa38885..f1d02cead 100644 --- a/lib/puppet/indirector/request.rb +++ b/lib/puppet/indirector/request.rb @@ -5,6 +5,8 @@ require 'puppet/indirector' class Puppet::Indirector::Request attr_accessor :indirection_name, :key, :method, :options, :instance, :node, :ip, :authenticated + attr_accessor :server, :port, :uri, :protocol + # Is this an authenticated request? def authenticated? # Double negative, so we just get true or false @@ -28,7 +30,15 @@ class Puppet::Indirector::Request end if key.is_a?(String) or key.is_a?(Symbol) - @key = key + # If the request key is a URI, then we need to treat it specially, + # because it rewrites the key. We could otherwise strip server/port/etc + # info out in the REST class, but it seemed bad design for the REST + # class to rewrite the key. + if key.to_s =~ /^\w+:\/\// # it's a URI + set_uri_key(key) + else + @key = key + end else @instance = key @key = @instance.name @@ -39,4 +49,28 @@ class Puppet::Indirector::Request def indirection Puppet::Indirector::Indirection.instance(@indirection_name) end + + private + + # Parse the key as a URI, setting attributes appropriately. + def set_uri_key(key) + @uri = key + begin + uri = URI.parse(URI.escape(key)) + rescue => detail + raise ArgumentError, "Could not understand URL %s: %s" % [source, detail.to_s] + end + @server = uri.host if uri.host + + # If the URI class can look up the scheme, it will provide a port, + # otherwise it will default to '0'. + if uri.port.to_i == 0 and uri.scheme == "puppet" + @port = Puppet.settings[:masterport].to_i + else + @port = uri.port.to_i + end + + @protocol = uri.scheme + @key = uri.path.sub(/^\//, '') + end end diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index 4389dfb7e..a2bfc5d9a 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -1,8 +1,33 @@ require 'net/http' require 'uri' +require 'puppet/network/http_pool' + # Access objects via REST class Puppet::Indirector::REST < Puppet::Indirector::Terminus + + class << self + attr_reader :server_setting, :port_setting + end + + # Specify the setting that we should use to get the server name. + def self.use_server_setting(setting) + @server_setting = setting + end + + def self.server + return Puppet.settings[server_setting || :server] + end + + # Specify the setting that we should use to get the port. + def self.use_port_setting(setting) + @port_setting = setting + end + + def self.port + return Puppet.settings[port_setting || :masterport].to_i + end + # Figure out the content type, turn that into a format, and use the format # to extract the body of the response. def deserialize(response, multiple = false) @@ -33,20 +58,7 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus end def network(request) - if request.key =~ /^\w+:\/\// # it looks like a URI - begin - uri = URI.parse(URI.escape(request.key)) - rescue => detail - raise ArgumentError, "Could not understand URL %s: %s" % [source, detail.to_s] - end - server = uri.host || Puppet[:server] - port = uri.port.to_i == 0 ? Puppet[:masterport].to_i : uri.port.to_i - else - server = Puppet[:server] - port = Puppet[:masterport].to_i - end - - Puppet::Network::HttpPool.http_instance(server, port) + Puppet::Network::HttpPool.http_instance(request.server || self.class.server, request.port || self.class.port) end def find(request) -- cgit From f5ba99fd24ce2e7cdba7c81153c46df14811d193 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sat, 23 Aug 2008 18:15:56 -0500 Subject: Special-casing 'file' URIs in the indirection requests. These just get converted to full file paths, since we know they will never pass over the wire. Signed-off-by: Luke Kanies --- lib/puppet/indirector/request.rb | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'lib') diff --git a/lib/puppet/indirector/request.rb b/lib/puppet/indirector/request.rb index f1d02cead..194b9e031 100644 --- a/lib/puppet/indirector/request.rb +++ b/lib/puppet/indirector/request.rb @@ -60,6 +60,13 @@ class Puppet::Indirector::Request rescue => detail raise ArgumentError, "Could not understand URL %s: %s" % [source, detail.to_s] end + + # Just short-circuit these to full paths + if uri.scheme == "file" + @key = uri.path + return + end + @server = uri.host if uri.host # If the URI class can look up the scheme, it will provide a port, -- cgit From 91b8252755c78dde752c2e06cf13ab0d757aad42 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sat, 23 Aug 2008 18:16:38 -0500 Subject: Fixing the fileserving terminus selection hook. It now uses the fact that the indirection request does URI parsing, rather than doing the parsing on its own. Signed-off-by: Luke Kanies --- lib/puppet/file_serving/indirection_hooks.rb | 45 ++++++++++++++-------------- 1 file changed, 23 insertions(+), 22 deletions(-) (limited to 'lib') diff --git a/lib/puppet/file_serving/indirection_hooks.rb b/lib/puppet/file_serving/indirection_hooks.rb index 66ed169dc..15564cf3d 100644 --- a/lib/puppet/file_serving/indirection_hooks.rb +++ b/lib/puppet/file_serving/indirection_hooks.rb @@ -9,36 +9,37 @@ require 'puppet/file_serving' # in file-serving indirections. This is necessary because # the terminus varies based on the URI asked for. module Puppet::FileServing::IndirectionHooks - PROTOCOL_MAP = {"puppet" => :rest, "file" => :file, "puppetmounts" => :file_server} + PROTOCOL_MAP = {"puppet" => :rest, "file" => :file} # Pick an appropriate terminus based on the protocol. def select_terminus(request) - full_uri = request.key - # Short-circuit to :file if it's a fully-qualified path. - return PROTOCOL_MAP["file"] if full_uri =~ /^#{::File::SEPARATOR}/ - begin - uri = URI.parse(URI.escape(full_uri)) - rescue => detail - raise ArgumentError, "Could not understand URI %s: %s" % [full_uri, detail.to_s] - end + # We rely on the request's parsing of the URI. - terminus = PROTOCOL_MAP[uri.scheme] || raise(ArgumentError, "URI protocol '%s' is not supported for file serving" % uri.scheme) + # Short-circuit to :file if it's a fully-qualified path or specifies a 'file' protocol. + return PROTOCOL_MAP["file"] if request.key =~ /^#{::File::SEPARATOR}/ + return PROTOCOL_MAP["file"] if request.protocol == "file" - # This provides a convenient mechanism for people to write configurations work - # well in both a networked and local setting. - if uri.host.nil? and uri.scheme == "puppet" and Puppet.settings[:name] == "puppet" - terminus = :file_server + # We're heading over the wire the protocol is 'puppet' and we've got a server name or we're not named 'puppet' + if request.protocol == "puppet" and (request.server or Puppet.settings[:name] != "puppet") + return PROTOCOL_MAP["puppet"] + end + + if request.protocol and PROTOCOL_MAP[request.protocol].nil? + raise(ArgumentError, "URI protocol '%s' is not currently supported for file serving" % request.protocol) end + # If we're still here, we're using the file_server or modules. + # This is the backward-compatible module terminus. - if terminus == :file_server and uri.path =~ %r{^/([^/]+)\b} - modname = $1 - if modname == "modules" - terminus = :modules - elsif terminus(:modules).find_module(modname, request.options[:node]) - Puppet.warning "DEPRECATION NOTICE: Found file '%s' in module without using the 'modules' mount; please prefix path with '/modules'" % uri.path - terminus = :modules - end + modname = request.key.split("/")[0] + + if modname == "modules" + terminus = :modules + elsif terminus(:modules).find_module(modname, request.options[:node]) + Puppet.warning "DEPRECATION NOTICE: Found file '%s' in module without using the 'modules' mount; please prefix path with 'modules/'" % request.key + terminus = :modules + else + terminus = :file_server end return terminus -- cgit From 0a05720e6c4bd0875fc03b53263ff26c6fe14de2 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sat, 23 Aug 2008 18:25:18 -0500 Subject: FileServing Configurations now expect unqualified files. This fits in with the fact that the indirection requests split URIs and set the request key to an unqualified path rather than a fully-qualified path. The whole system is unqualified end-to-end, now, except when you're specifically asking for a full, local file name. Signed-off-by: Luke Kanies --- lib/puppet/file_serving/configuration.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/puppet/file_serving/configuration.rb b/lib/puppet/file_serving/configuration.rb index 9c38aaa19..bceecc30c 100644 --- a/lib/puppet/file_serving/configuration.rb +++ b/lib/puppet/file_serving/configuration.rb @@ -98,13 +98,14 @@ class Puppet::FileServing::Configuration # Reparse the configuration if necessary. readconfig - raise(ArgumentError, "Cannot find file: Invalid path '%s'" % uri) unless uri =~ %r{/([-\w]+)/?} + raise(ArgumentError, "Cannot find file: Invalid path '%s'" % uri) unless uri =~ %r{^([-\w]+)(/|$)} # the dir is based on one of the mounts # so first retrieve the mount path mount = path = nil + # Strip off the mount name. - mount_name, path = uri.sub(%r{^/}, '').split(File::Separator, 2) + mount_name, path = uri.split(File::Separator, 2) return nil unless mount = @mounts[mount_name] -- cgit From bcd40fb58338e44d46452c47c0ef150a96a5f829 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sat, 23 Aug 2008 18:36:03 -0500 Subject: Cleaning up an exception. Only adding option information when options are present. Signed-off-by: Luke Kanies --- lib/puppet/indirector/indirection.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb index a9fff75b8..e6068a6aa 100644 --- a/lib/puppet/indirector/indirection.rb +++ b/lib/puppet/indirector/indirection.rb @@ -262,7 +262,11 @@ class Puppet::Indirector::Indirection return unless terminus.respond_to?(:authorized?) unless terminus.authorized?(request) - raise ArgumentError, "Not authorized to call %s on %s with %s" % [request.method, request.key, request.options.inspect] + msg = "Not authorized to call %s on %s" % [request.method, request.key] + unless request.options.empty? + msg += " with %s" % request.options.inspect + end + raise ArgumentError, msg end end -- cgit From 2f224c9fc97cf632ee8a551180aaa08e263d77df Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sat, 23 Aug 2008 19:02:43 -0500 Subject: Spell-correcting a comment Signed-off-by: Luke Kanies --- lib/puppet/indirector/rest.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index a2bfc5d9a..5ac25f02d 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -89,7 +89,7 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus private - # Create the qurey string, if options are present. + # Create the query string, if options are present. def query_string(request) return "" unless request.options and ! request.options.empty? "?" + request.options.collect { |key, value| "%s=%s" % [key, value] }.join("&") -- cgit From 6335b143a312481aaa200f71cd25dffd4f88c8ae Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sat, 23 Aug 2008 21:59:14 -0500 Subject: Causing the Indirection to fail if a terminus selection hook does not return a value. Signed-off-by: Luke Kanies --- lib/puppet/indirector/indirection.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb index e6068a6aa..04c3aed23 100644 --- a/lib/puppet/indirector/indirection.rb +++ b/lib/puppet/indirector/indirection.rb @@ -274,7 +274,9 @@ class Puppet::Indirector::Indirection def prepare(request) # Pick our terminus. if respond_to?(:select_terminus) - terminus_name = select_terminus(request) + unless terminus_name = select_terminus(request) + raise ArgumentError, "Could not determine appropriate terminus for %s" % request + end else terminus_name = terminus_class end -- cgit From a0bda8532f5e1e9f5bb29eb92f389383ae0857d5 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sat, 23 Aug 2008 22:00:19 -0500 Subject: Removing the yaml conversion code from FileContent. Also fixing some integration tests that were failing because of the change to the terminus selection code for file serving. Signed-off-by: Luke Kanies --- lib/puppet/file_serving/content.rb | 8 -------- 1 file changed, 8 deletions(-) (limited to 'lib') diff --git a/lib/puppet/file_serving/content.rb b/lib/puppet/file_serving/content.rb index 9398513e7..1114829f1 100644 --- a/lib/puppet/file_serving/content.rb +++ b/lib/puppet/file_serving/content.rb @@ -23,12 +23,4 @@ class Puppet::FileServing::Content < Puppet::FileServing::FileBase ::File.read(full_path()) end - - # Just return the file contents as the yaml. This allows us to - # avoid escaping or any such thing. LAK:NOTE Not really sure how - # this will behave if the file contains yaml... I think the far - # side needs to understand that it's a plain string. - def to_yaml - content - end end -- cgit From 89a3738d2b0678ae8fb1e7191e01caf6c5ece1f9 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sun, 24 Aug 2008 12:18:42 -0500 Subject: Adding suitability as a requirement for a format being supported. Signed-off-by: Luke Kanies --- lib/puppet/network/format.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/network/format.rb b/lib/puppet/network/format.rb index 5f259fa49..3e0a7d8aa 100644 --- a/lib/puppet/network/format.rb +++ b/lib/puppet/network/format.rb @@ -55,7 +55,8 @@ class Puppet::Network::Format end def supported?(klass) - klass.respond_to?(intern_method) and + suitable? and + klass.respond_to?(intern_method) and klass.respond_to?(intern_multiple_method) and klass.respond_to?(render_multiple_method) and klass.instance_methods.include?(render_method) -- cgit From 3101ea23e556081fe38502218034f02aafe0c5bf Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sun, 24 Aug 2008 13:02:16 -0500 Subject: Adding a hackish raw format. As the comment in the file says, we don't really have enough data to know what a good design would look like, and I think this format will be a bit of a one-off, so I'm just throwing up some barriers to keep people from doing silly things with it. Signed-off-by: Luke Kanies --- lib/puppet/network/formats.rb | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/network/formats.rb b/lib/puppet/network/formats.rb index 8e4c59fb3..c2a8b7ab6 100644 --- a/lib/puppet/network/formats.rb +++ b/lib/puppet/network/formats.rb @@ -42,7 +42,7 @@ Puppet::Network::FormatHandler.create(:marshal, :mime => "text/marshal") do Marshal.dump(instance) end - # Yaml monkey-patches Array, so this works. + # Marshal monkey-patches Array, so this works. def render_multiple(instances) Marshal.dump(instances) end @@ -54,3 +54,23 @@ Puppet::Network::FormatHandler.create(:marshal, :mime => "text/marshal") do end Puppet::Network::FormatHandler.create(:s, :mime => "text/plain") + +Puppet::Network::FormatHandler.create(:raw, :mime => "application/x-raw") do + def intern_multiple(klass, text) + raise NotImplementedError + end + + def render_multiple(instances) + raise NotImplementedError + end + + # LAK:NOTE The format system isn't currently flexible enough to handle + # what I need to support raw formats just for individual instances (rather + # than both individual and collections), but we don't yet have enough data + # to make a "correct" design. + # So, we hack it so it works for singular but fail if someone tries it + # on plurals. + def supported?(klass) + true + end +end -- cgit From 5a195e0c06daa2bfa008cfd94c660e50b9d0ae56 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sun, 24 Aug 2008 13:32:33 -0500 Subject: Renaming FileServing::FileBase to FileServing::Base. Also fixing a set of tests I broke last night. I'm looking at replacing autotest with rspactor, because my FSEvents hack to autotest means it's harder for me to rerun autotest. Signed-off-by: Luke Kanies --- lib/puppet/file_serving/content.rb | 17 +++++--- lib/puppet/file_serving/file_base.rb | 76 ------------------------------------ lib/puppet/file_serving/metadata.rb | 4 +- 3 files changed, 14 insertions(+), 83 deletions(-) delete mode 100644 lib/puppet/file_serving/file_base.rb (limited to 'lib') diff --git a/lib/puppet/file_serving/content.rb b/lib/puppet/file_serving/content.rb index 1114829f1..64f000eaa 100644 --- a/lib/puppet/file_serving/content.rb +++ b/lib/puppet/file_serving/content.rb @@ -4,23 +4,30 @@ require 'puppet/indirector' require 'puppet/file_serving' -require 'puppet/file_serving/file_base' +require 'puppet/file_serving/base' require 'puppet/file_serving/indirection_hooks' # A class that handles retrieving file contents. # It only reads the file when its content is specifically # asked for. -class Puppet::FileServing::Content < Puppet::FileServing::FileBase +class Puppet::FileServing::Content < Puppet::FileServing::Base extend Puppet::Indirector indirects :file_content, :extend => Puppet::FileServing::IndirectionHooks attr_reader :path + def collect + end + # Read the content of our file in. def content - # This stat can raise an exception, too. - raise(ArgumentError, "Cannot read the contents of links unless following links") if stat().ftype == "symlink" + unless defined?(@content) and @content + # This stat can raise an exception, too. + raise(ArgumentError, "Cannot read the contents of links unless following links") if stat().ftype == "symlink" - ::File.read(full_path()) + p full_path + @content = ::File.read(full_path()) + end + @content end end diff --git a/lib/puppet/file_serving/file_base.rb b/lib/puppet/file_serving/file_base.rb deleted file mode 100644 index e87d683aa..000000000 --- a/lib/puppet/file_serving/file_base.rb +++ /dev/null @@ -1,76 +0,0 @@ -# -# Created by Luke Kanies on 2007-10-22. -# Copyright (c) 2007. All rights reserved. - -require 'puppet/file_serving' - -# The base class for Content and Metadata; provides common -# functionality like the behaviour around links. -class Puppet::FileServing::FileBase - attr_accessor :key - - # Does our file exist? - def exist? - begin - stat - return true - rescue => detail - return false - end - end - - # Return the full path to our file. Fails if there's no path set. - def full_path - raise(ArgumentError, "You must set a path to get a file's path") unless self.path - - if relative_path.nil? or relative_path == "" - path - else - File.join(path, relative_path) - end - end - - def initialize(key, options = {}) - @key = key - @links = :manage - - options.each do |param, value| - begin - send param.to_s + "=", value - rescue NoMethodError - raise ArgumentError, "Invalid option %s for %s" % [param, self.class] - end - end - end - - # Determine how we deal with links. - attr_reader :links - def links=(value) - value = :manage if value == :ignore - raise(ArgumentError, ":links can only be set to :manage or :follow") unless [:manage, :follow].include?(value) - @links = value - end - - # Set our base path. - attr_reader :path - def path=(path) - raise ArgumentError.new("Paths must be fully qualified") unless path =~ /^#{::File::SEPARATOR}/ - @path = path - end - - # Set a relative path; this is used for recursion, and sets - # the file's path relative to the initial recursion point. - attr_reader :relative_path - def relative_path=(path) - raise ArgumentError.new("Relative paths must not be fully qualified") if path =~ /^#{::File::SEPARATOR}/ - @relative_path = path - end - - # Stat our file, using the appropriate link-sensitive method. - def stat - unless defined?(@stat_method) - @stat_method = self.links == :manage ? :lstat : :stat - end - File.send(@stat_method, full_path()) - end -end diff --git a/lib/puppet/file_serving/metadata.rb b/lib/puppet/file_serving/metadata.rb index beecaef48..a1265dd8b 100644 --- a/lib/puppet/file_serving/metadata.rb +++ b/lib/puppet/file_serving/metadata.rb @@ -5,12 +5,12 @@ require 'puppet' require 'puppet/indirector' require 'puppet/file_serving' -require 'puppet/file_serving/file_base' +require 'puppet/file_serving/base' require 'puppet/util/checksums' require 'puppet/file_serving/indirection_hooks' # A class that handles retrieving file metadata. -class Puppet::FileServing::Metadata < Puppet::FileServing::FileBase +class Puppet::FileServing::Metadata < Puppet::FileServing::Base include Puppet::Util::Checksums -- cgit From 90e70227b0bb7cfd104ae34de8f7c2b7250edb09 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sun, 24 Aug 2008 14:10:39 -0500 Subject: Adding weights to network formats, and sorting them based on the weight. This way the new hackish RAW format will only ever be used if it's specifically chosen. Signed-off-by: Luke Kanies --- lib/puppet/network/format.rb | 9 ++++++++- lib/puppet/network/format_handler.rb | 5 ++++- lib/puppet/network/formats.rb | 3 ++- 3 files changed, 14 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/puppet/network/format.rb b/lib/puppet/network/format.rb index 3e0a7d8aa..21aead7cc 100644 --- a/lib/puppet/network/format.rb +++ b/lib/puppet/network/format.rb @@ -5,7 +5,7 @@ require 'puppet/provider/confiner' class Puppet::Network::Format include Puppet::Provider::Confiner - attr_reader :name, :mime + attr_reader :name, :mime, :weight def initialize(name, options = {}, &block) @name = name.to_s.downcase.intern @@ -17,6 +17,13 @@ class Puppet::Network::Format self.mime = "text/%s" % name end + if weight = options[:weight] + @weight = weight + options.delete(:weight) + else + @weight = 5 + end + unless options.empty? raise ArgumentError, "Unsupported option(s) %s" % options.keys end diff --git a/lib/puppet/network/format_handler.rb b/lib/puppet/network/format_handler.rb index 4c9f4e59e..f3c3380e1 100644 --- a/lib/puppet/network/format_handler.rb +++ b/lib/puppet/network/format_handler.rb @@ -61,7 +61,10 @@ module Puppet::Network::FormatHandler end def supported_formats - format_handler.formats.collect { |f| format_handler.format(f) }.find_all { |f| f.supported?(self) }.collect { |f| f.name } + format_handler.formats.collect { |f| format_handler.format(f) }.find_all { |f| f.supported?(self) }.collect { |f| f.name }.sort do |a, b| + # It's an inverse sort -- higher weight formats go first. + format_handler.format(b).weight <=> format_handler.format(a).weight + end end end diff --git a/lib/puppet/network/formats.rb b/lib/puppet/network/formats.rb index c2a8b7ab6..85e8ce6f8 100644 --- a/lib/puppet/network/formats.rb +++ b/lib/puppet/network/formats.rb @@ -55,7 +55,8 @@ end Puppet::Network::FormatHandler.create(:s, :mime => "text/plain") -Puppet::Network::FormatHandler.create(:raw, :mime => "application/x-raw") do +# A very low-weight format so it'll never get chosen automatically. +Puppet::Network::FormatHandler.create(:raw, :mime => "application/x-raw", :weight => 1) do def intern_multiple(klass, text) raise NotImplementedError end -- cgit From 550e3d6ad5aadfe99fc1e10efa77cc193d3a9df3 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sun, 24 Aug 2008 14:11:48 -0500 Subject: Finishing the rename of FileBase => Base. Git did something really strange, in that it apparently didn't add the new base.rb files even though I used 'git mv'. Also fixing some other failing tests I hadn't previously tracked down because of the magical tuple of autotest's suckiness and my laziness. Signed-off-by: Luke Kanies --- lib/puppet/file_serving/base.rb | 76 +++++++++++++++++++++++++++++++++++ lib/puppet/indirector/module_files.rb | 7 ++-- 2 files changed, 79 insertions(+), 4 deletions(-) create mode 100644 lib/puppet/file_serving/base.rb (limited to 'lib') diff --git a/lib/puppet/file_serving/base.rb b/lib/puppet/file_serving/base.rb new file mode 100644 index 000000000..1cfd0bc4c --- /dev/null +++ b/lib/puppet/file_serving/base.rb @@ -0,0 +1,76 @@ +# +# Created by Luke Kanies on 2007-10-22. +# Copyright (c) 2007. All rights reserved. + +require 'puppet/file_serving' + +# The base class for Content and Metadata; provides common +# functionality like the behaviour around links. +class Puppet::FileServing::Base + attr_accessor :key + + # Does our file exist? + def exist? + begin + stat + return true + rescue => detail + return false + end + end + + # Return the full path to our file. Fails if there's no path set. + def full_path + raise(ArgumentError, "You must set a path to get a file's path") unless self.path + + if relative_path.nil? or relative_path == "" + path + else + File.join(path, relative_path) + end + end + + def initialize(key, options = {}) + @key = key + @links = :manage + + options.each do |param, value| + begin + send param.to_s + "=", value + rescue NoMethodError + raise ArgumentError, "Invalid option %s for %s" % [param, self.class] + end + end + end + + # Determine how we deal with links. + attr_reader :links + def links=(value) + value = :manage if value == :ignore + raise(ArgumentError, ":links can only be set to :manage or :follow") unless [:manage, :follow].include?(value) + @links = value + end + + # Set our base path. + attr_reader :path + def path=(path) + raise ArgumentError.new("Paths must be fully qualified") unless path =~ /^#{::File::SEPARATOR}/ + @path = path + end + + # Set a relative path; this is used for recursion, and sets + # the file's path relative to the initial recursion point. + attr_reader :relative_path + def relative_path=(path) + raise ArgumentError.new("Relative paths must not be fully qualified") if path =~ /^#{::File::SEPARATOR}/ + @relative_path = path + end + + # Stat our file, using the appropriate link-sensitive method. + def stat + unless defined?(@stat_method) + @stat_method = self.links == :manage ? :lstat : :stat + end + File.send(@stat_method, full_path()) + end +end diff --git a/lib/puppet/indirector/module_files.rb b/lib/puppet/indirector/module_files.rb index cf5c29cab..40dd06941 100644 --- a/lib/puppet/indirector/module_files.rb +++ b/lib/puppet/indirector/module_files.rb @@ -21,7 +21,7 @@ class Puppet::Indirector::ModuleFiles < Puppet::Indirector::Terminus # Make sure our file path starts with /modules, so that we authorize # against the 'modules' mount. - path = uri.path =~ /^\/modules/ ? uri.path : "/modules" + uri.path + path = uri.path =~ /^modules\// ? uri.path : "modules/" + uri.path configuration.authorized?(path, :node => request.node, :ipaddress => request.ip) end @@ -66,9 +66,8 @@ class Puppet::Indirector::ModuleFiles < Puppet::Indirector::Terminus def find_path(request) uri = key2uri(request.key) - # Strip off /modules if it's there -- that's how requests get routed to this terminus. - # Also, strip off the leading slash if present. - module_name, relative_path = uri.path.sub(/^\/modules\b/, '').sub(%r{^/}, '').split(File::Separator, 2) + # Strip off modules/ if it's there -- that's how requests get routed to this terminus. + module_name, relative_path = uri.path.sub(/^modules\//, '').sub(%r{^/}, '').split(File::Separator, 2) # And use the environment to look up the module. return nil unless mod = find_module(module_name, request.node) -- cgit From 8ea25efd90b4d2281db12076cbaab3f766cac8b4 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sun, 24 Aug 2008 14:42:48 -0500 Subject: Refactoring how files in FileServing are named. Previously, they retained some concept of the URI used to find them, and this uri was the primary key for the FileServing instances. This key was unfortunately completely useless, as evidenced by the fact that it was never used except to test that it worked. I've modified the FileServing instances (through modifying the Base class) to use their local path as their key, and they no longer care about the URI at all. This commit is mostly about fixing the code that interacts with the instances to use this new API. Signed-off-by: Luke Kanies --- lib/puppet/file_serving/base.rb | 8 ++------ lib/puppet/file_serving/content.rb | 1 - lib/puppet/file_serving/terminus_helper.rb | 2 +- lib/puppet/indirector/direct_file_server.rb | 10 ++++------ lib/puppet/indirector/file_server.rb | 2 +- lib/puppet/indirector/module_files.rb | 2 +- 6 files changed, 9 insertions(+), 16 deletions(-) (limited to 'lib') diff --git a/lib/puppet/file_serving/base.rb b/lib/puppet/file_serving/base.rb index 1cfd0bc4c..94e337b99 100644 --- a/lib/puppet/file_serving/base.rb +++ b/lib/puppet/file_serving/base.rb @@ -7,8 +7,6 @@ require 'puppet/file_serving' # The base class for Content and Metadata; provides common # functionality like the behaviour around links. class Puppet::FileServing::Base - attr_accessor :key - # Does our file exist? def exist? begin @@ -21,8 +19,6 @@ class Puppet::FileServing::Base # Return the full path to our file. Fails if there's no path set. def full_path - raise(ArgumentError, "You must set a path to get a file's path") unless self.path - if relative_path.nil? or relative_path == "" path else @@ -30,8 +26,8 @@ class Puppet::FileServing::Base end end - def initialize(key, options = {}) - @key = key + def initialize(path, options = {}) + self.path = path @links = :manage options.each do |param, value| diff --git a/lib/puppet/file_serving/content.rb b/lib/puppet/file_serving/content.rb index 64f000eaa..f13fcaa72 100644 --- a/lib/puppet/file_serving/content.rb +++ b/lib/puppet/file_serving/content.rb @@ -25,7 +25,6 @@ class Puppet::FileServing::Content < Puppet::FileServing::Base # This stat can raise an exception, too. raise(ArgumentError, "Cannot read the contents of links unless following links") if stat().ftype == "symlink" - p full_path @content = ::File.read(full_path()) end @content diff --git a/lib/puppet/file_serving/terminus_helper.rb b/lib/puppet/file_serving/terminus_helper.rb index e5da0e29f..bde0bd389 100644 --- a/lib/puppet/file_serving/terminus_helper.rb +++ b/lib/puppet/file_serving/terminus_helper.rb @@ -11,7 +11,7 @@ module Puppet::FileServing::TerminusHelper def path2instances(request, path) args = [:links, :ignore, :recurse].inject({}) { |hash, param| hash[param] = request.options[param] if request.options[param]; hash } Puppet::FileServing::Fileset.new(path, args).files.collect do |file| - inst = model.new(File.join(request.key, file), :path => path, :relative_path => file) + inst = model.new(path, :relative_path => file) inst.links = request.options[:links] if request.options[:links] inst end diff --git a/lib/puppet/indirector/direct_file_server.rb b/lib/puppet/indirector/direct_file_server.rb index b3b4886f3..bcda92366 100644 --- a/lib/puppet/indirector/direct_file_server.rb +++ b/lib/puppet/indirector/direct_file_server.rb @@ -12,16 +12,14 @@ class Puppet::Indirector::DirectFileServer < Puppet::Indirector::Terminus include Puppet::FileServing::TerminusHelper def find(request) - uri = key2uri(request.key) - return nil unless FileTest.exists?(uri.path) - instance = model.new(request.key, :path => uri.path) + return nil unless FileTest.exists?(request.key) + instance = model.new(request.key) instance.links = request.options[:links] if request.options[:links] return instance end def search(request) - uri = key2uri(request.key) - return nil unless FileTest.exists?(uri.path) - path2instances(request, uri.path) + return nil unless FileTest.exists?(request.key) + path2instances(request, request.key) end end diff --git a/lib/puppet/indirector/file_server.rb b/lib/puppet/indirector/file_server.rb index b0df7ff5d..476fc5b23 100644 --- a/lib/puppet/indirector/file_server.rb +++ b/lib/puppet/indirector/file_server.rb @@ -25,7 +25,7 @@ class Puppet::Indirector::FileServer < Puppet::Indirector::Terminus # Find our key using the fileserver. def find(request) return nil unless path = find_path(request) - result = model.new(request.key, :path => path) + result = model.new(path) result.links = request.options[:links] if request.options[:links] return result end diff --git a/lib/puppet/indirector/module_files.rb b/lib/puppet/indirector/module_files.rb index 40dd06941..7c5cf278f 100644 --- a/lib/puppet/indirector/module_files.rb +++ b/lib/puppet/indirector/module_files.rb @@ -30,7 +30,7 @@ class Puppet::Indirector::ModuleFiles < Puppet::Indirector::Terminus def find(request) return nil unless path = find_path(request) - result = model.new(request.key, :path => path) + result = model.new(path) result.links = request.options[:links] if request.options[:links] return result end -- cgit From 40e76fb83ef466425fec736abbf1913a6426bf01 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sun, 24 Aug 2008 20:53:25 -0500 Subject: Fixing the rest backends for webrick and mongrel so the get the whole request key. Also adding the Content work necessary to demonstrate that this is actually required. Signed-off-by: Luke Kanies --- lib/puppet/file_serving/content.rb | 16 +++++++++++++++- lib/puppet/network/http/mongrel/rest.rb | 2 +- lib/puppet/network/http/webrick/rest.rb | 2 +- 3 files changed, 17 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/puppet/file_serving/content.rb b/lib/puppet/file_serving/content.rb index f13fcaa72..b30070dd6 100644 --- a/lib/puppet/file_serving/content.rb +++ b/lib/puppet/file_serving/content.rb @@ -14,9 +14,19 @@ class Puppet::FileServing::Content < Puppet::FileServing::Base extend Puppet::Indirector indirects :file_content, :extend => Puppet::FileServing::IndirectionHooks - attr_reader :path + def self.supported_formats + [:raw] + end + + def self.from_raw(content) + instance = new("eh") + instance.content = content + instance + end + # Collect our data. def collect + content end # Read the content of our file in. @@ -29,4 +39,8 @@ class Puppet::FileServing::Content < Puppet::FileServing::Base end @content end + + def to_raw + content + end end diff --git a/lib/puppet/network/http/mongrel/rest.rb b/lib/puppet/network/http/mongrel/rest.rb index d265dde86..45d21ea62 100644 --- a/lib/puppet/network/http/mongrel/rest.rb +++ b/lib/puppet/network/http/mongrel/rest.rb @@ -35,7 +35,7 @@ class Puppet::Network::HTTP::MongrelREST < Mongrel::HttpHandler # return the key included in the request path def request_key(request) # LAK:NOTE See http://snurl.com/21zf8 [groups_google_com] - x = request.params[Mongrel::Const::REQUEST_PATH].split('/')[2] + x = request.params[Mongrel::Const::REQUEST_PATH].split('/', 3)[2] end # return the request body diff --git a/lib/puppet/network/http/webrick/rest.rb b/lib/puppet/network/http/webrick/rest.rb index 13f795fb2..f06914365 100644 --- a/lib/puppet/network/http/webrick/rest.rb +++ b/lib/puppet/network/http/webrick/rest.rb @@ -36,7 +36,7 @@ class Puppet::Network::HTTP::WEBrickREST < WEBrick::HTTPServlet::AbstractServlet def request_key(request) # LAK:NOTE See http://snurl.com/21zf8 [groups_google_com] - x = request.path.split('/')[2] + x = request.path.split('/', 3)[2] end def body(request) -- cgit From 30dea6839b0360e2fabbeb833e6c2b8658d3f53c Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sun, 24 Aug 2008 20:54:47 -0500 Subject: Adding a 'plural?' method to the Indirection::Request class. I'm in the process of creating a new service for handling all of the http calls, including generation of the RESTian URL. This service obviously needs to know whether the url should be plural or singular, and the request knows the method in use, so it can easily answer the question. Signed-off-by: Luke Kanies --- lib/puppet/indirector/request.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/puppet/indirector/request.rb b/lib/puppet/indirector/request.rb index 194b9e031..49cc01aab 100644 --- a/lib/puppet/indirector/request.rb +++ b/lib/puppet/indirector/request.rb @@ -1,7 +1,8 @@ require 'puppet/indirector' -# Provide any attributes or functionality needed for indirected -# instances. +# This class encapsulates all of the information you need to make an +# Indirection call, and as a a result also handles REST calls. It's somewhat +# analogous to an HTTP Request object, except tuned for our Indirector. class Puppet::Indirector::Request attr_accessor :indirection_name, :key, :method, :options, :instance, :node, :ip, :authenticated @@ -50,6 +51,11 @@ class Puppet::Indirector::Request Puppet::Indirector::Indirection.instance(@indirection_name) end + # Are we trying to interact with multiple resources, or just one? + def plural? + method == :search + end + private # Parse the key as a URI, setting attributes appropriately. -- cgit From 151a54ff7ac69aa2fa1708188ad75e444158e8a2 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sun, 24 Aug 2008 21:17:15 -0500 Subject: Causing format selection to fail intelligently if no suitable format can be picked. Signed-off-by: Luke Kanies --- lib/puppet/network/http/handler.rb | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb index 6f5117b16..7c7abccf5 100644 --- a/lib/puppet/network/http/handler.rb +++ b/lib/puppet/network/http/handler.rb @@ -12,7 +12,18 @@ module Puppet::Network::HTTP::Handler # Which format to use when serializing our response. Just picks # the first value in the accept header, at this point. def format_to_use(request) - accept_header(request).split(/,\s*/)[0] + unless header = accept_header(request) + raise ArgumentError, "An Accept header must be provided to pick the right format" + end + + format = nil + header.split(/,\s*/).each do |name| + next unless format = Puppet::Network::FormatHandler.format(name) + next unless format.suitable? + return name + end + + raise "No specified acceptable formats (%s) are functional on this machine" % header end def initialize_for_puppet(args = {}) -- cgit From 6ed8dfaf7c0cf091dca0374de310f524b0a033cc Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Mon, 25 Aug 2008 17:43:35 -0500 Subject: Adding the content writer to the content class. Also choosing a fully qualified fake name when creating content instances from raw content. Signed-off-by: Luke Kanies --- lib/puppet/file_serving/content.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/file_serving/content.rb b/lib/puppet/file_serving/content.rb index b30070dd6..0f169c28b 100644 --- a/lib/puppet/file_serving/content.rb +++ b/lib/puppet/file_serving/content.rb @@ -14,12 +14,14 @@ class Puppet::FileServing::Content < Puppet::FileServing::Base extend Puppet::Indirector indirects :file_content, :extend => Puppet::FileServing::IndirectionHooks + attr_writer :content + def self.supported_formats [:raw] end def self.from_raw(content) - instance = new("eh") + instance = new("/this/is/a/fake/path") instance.content = content instance end -- cgit From 8b45d13ab28837caf3bb09cc1c90ab61974bf4db Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Mon, 25 Aug 2008 18:00:42 -0700 Subject: Adding automatic attribute collection to the new fileserving code. Basically, this just includes a consistent method for collecting info (either content or metadata) and then calls that method when returning instances via the indirector. It's such a large commit mostly because of small changes in the normal code and large changes in the testing to accomodate those small changes. Signed-off-by: Luke Kanies --- lib/puppet/file_serving/content.rb | 1 + lib/puppet/file_serving/fileset.rb | 2 +- lib/puppet/file_serving/metadata.rb | 2 +- lib/puppet/file_serving/terminus_helper.rb | 1 + lib/puppet/indirector/file_metadata/file.rb | 4 ++-- lib/puppet/indirector/file_metadata/modules.rb | 2 +- lib/puppet/indirector/file_server.rb | 1 + lib/puppet/network/handler/fileserver.rb | 2 +- 8 files changed, 9 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/puppet/file_serving/content.rb b/lib/puppet/file_serving/content.rb index 0f169c28b..c1ecff749 100644 --- a/lib/puppet/file_serving/content.rb +++ b/lib/puppet/file_serving/content.rb @@ -28,6 +28,7 @@ class Puppet::FileServing::Content < Puppet::FileServing::Base # Collect our data. def collect + return if stat.ftype == "directory" content end diff --git a/lib/puppet/file_serving/fileset.rb b/lib/puppet/file_serving/fileset.rb index fe54350b1..80a718c68 100644 --- a/lib/puppet/file_serving/fileset.rb +++ b/lib/puppet/file_serving/fileset.rb @@ -20,7 +20,7 @@ class Puppet::FileServing::Fileset # Now strip off the leading path, so each file becomes relative, and remove # any slashes that might end up at the beginning of the path. - result = files.collect { |file| file.sub(%r{^#{@path}/*}, '') } + result = files.collect { |file| file.sub(@path, '').sub(%r{^/},'') } # And add the path itself. result.unshift(".") diff --git a/lib/puppet/file_serving/metadata.rb b/lib/puppet/file_serving/metadata.rb index a1265dd8b..1cc3fa355 100644 --- a/lib/puppet/file_serving/metadata.rb +++ b/lib/puppet/file_serving/metadata.rb @@ -47,7 +47,7 @@ class Puppet::FileServing::Metadata < Puppet::FileServing::Base # Retrieve the attributes for this file, relative to a base directory. # Note that File.stat raises Errno::ENOENT if the file is absent and this # method does not catch that exception. - def collect_attributes + def collect real_path = full_path() stat = stat() @owner = stat.uid diff --git a/lib/puppet/file_serving/terminus_helper.rb b/lib/puppet/file_serving/terminus_helper.rb index bde0bd389..598a5007a 100644 --- a/lib/puppet/file_serving/terminus_helper.rb +++ b/lib/puppet/file_serving/terminus_helper.rb @@ -13,6 +13,7 @@ module Puppet::FileServing::TerminusHelper Puppet::FileServing::Fileset.new(path, args).files.collect do |file| inst = model.new(path, :relative_path => file) inst.links = request.options[:links] if request.options[:links] + inst.collect inst end end diff --git a/lib/puppet/indirector/file_metadata/file.rb b/lib/puppet/indirector/file_metadata/file.rb index c46015c38..bb586489d 100644 --- a/lib/puppet/indirector/file_metadata/file.rb +++ b/lib/puppet/indirector/file_metadata/file.rb @@ -11,7 +11,7 @@ class Puppet::Indirector::FileMetadata::File < Puppet::Indirector::DirectFileSer def find(request) return unless data = super - data.collect_attributes + data.collect return data end @@ -19,7 +19,7 @@ class Puppet::Indirector::FileMetadata::File < Puppet::Indirector::DirectFileSer def search(request) return unless result = super - result.each { |instance| instance.collect_attributes } + result.each { |instance| instance.collect } return result end diff --git a/lib/puppet/indirector/file_metadata/modules.rb b/lib/puppet/indirector/file_metadata/modules.rb index 5ed7a8a45..4598c2175 100644 --- a/lib/puppet/indirector/file_metadata/modules.rb +++ b/lib/puppet/indirector/file_metadata/modules.rb @@ -11,7 +11,7 @@ class Puppet::Indirector::FileMetadata::Modules < Puppet::Indirector::ModuleFile def find(*args) return unless instance = super - instance.collect_attributes + instance.collect instance end end diff --git a/lib/puppet/indirector/file_server.rb b/lib/puppet/indirector/file_server.rb index 476fc5b23..46a590f9c 100644 --- a/lib/puppet/indirector/file_server.rb +++ b/lib/puppet/indirector/file_server.rb @@ -27,6 +27,7 @@ class Puppet::Indirector::FileServer < Puppet::Indirector::Terminus return nil unless path = find_path(request) result = model.new(path) result.links = request.options[:links] if request.options[:links] + result.collect return result end diff --git a/lib/puppet/network/handler/fileserver.rb b/lib/puppet/network/handler/fileserver.rb index 183979429..14319ef96 100755 --- a/lib/puppet/network/handler/fileserver.rb +++ b/lib/puppet/network/handler/fileserver.rb @@ -75,7 +75,7 @@ class Puppet::Network::Handler return "" unless metadata.exist? begin - metadata.collect_attributes + metadata.collect rescue => detail puts detail.backtrace if Puppet[:trace] Puppet.err detail -- cgit From 98ac24a9e155b1d3c2358da3e94610071b0e3cfb Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Mon, 25 Aug 2008 21:50:13 -0700 Subject: Adding a "source" attribute to fileserving instances. This will be used to cache the source that was used to retrieve the instance. Signed-off-by: Luke Kanies --- lib/puppet/file_serving/base.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'lib') diff --git a/lib/puppet/file_serving/base.rb b/lib/puppet/file_serving/base.rb index 94e337b99..9a50833de 100644 --- a/lib/puppet/file_serving/base.rb +++ b/lib/puppet/file_serving/base.rb @@ -7,6 +7,10 @@ require 'puppet/file_serving' # The base class for Content and Metadata; provides common # functionality like the behaviour around links. class Puppet::FileServing::Base + # This is for external consumers to store the source that was used + # to retrieve the metadata. + attr_accessor :source + # Does our file exist? def exist? begin -- cgit From 82714246b913087292f04190e03a885c99723f52 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Tue, 26 Aug 2008 22:06:02 -0700 Subject: One third done refactoring file[:source] -- retrieve() is done. It now uses the FileServing::Metadata indirection and is about 100x cleaner to boot. Signed-off-by: Luke Kanies --- lib/puppet/type/file/source.rb | 174 +++++++++++++++-------------------------- 1 file changed, 62 insertions(+), 112 deletions(-) (limited to 'lib') diff --git a/lib/puppet/type/file/source.rb b/lib/puppet/type/file/source.rb index 2514d3d1e..e8db13cfa 100755 --- a/lib/puppet/type/file/source.rb +++ b/lib/puppet/type/file/source.rb @@ -1,3 +1,7 @@ + +require 'puppet/file_serving/content' +require 'puppet/file_serving/metadata' + module Puppet # Copy files from a local or remote source. This state *only* does any work # when the remote file is an actual file; in that case, this state copies @@ -77,61 +81,48 @@ module Puppet end def change_to_s(currentvalue, newvalue) - # newvalue = "{md5}" + @stats[:checksum] + # newvalue = "{md5}" + @metadata.checksum if @resource.property(:ensure).retrieve == :absent - return "creating from source %s with contents %s" % [@source, @stats[:checksum]] + return "creating from source %s with contents %s" % [@source, @metadata.checksum] else - return "replacing from source %s with contents %s" % [@source, @stats[:checksum]] + return "replacing from source %s with contents %s" % [@source, @metadata.checksum] end end def checksum - if defined?(@stats) - @stats[:checksum] + if defined?(@metadata) + @metadata.checksum else nil end end - # Ask the file server to describe our file. - def describe(source) - sourceobj, path = @resource.uri2obj(source) - server = sourceobj.server + # Copy the values from the source to the resource. Yay. + def copy_source_values + devfail "Somehow got asked to copy source values without any metadata" unless metadata - begin - desc = server.describe(path, @resource[:links]) - rescue Puppet::Network::XMLRPCClientError => detail - fail detail, "Could not describe %s: %s" % [path, detail] + # Take each of the stats and set them as states on the local file + # if a value has not already been provided. + [:owner, :mode, :group].each do |param| + @resource[param] ||= metadata.send(param) end - return nil if desc == "" - - # Collect everything except the checksum - values = desc.split("\t") - other = values.pop - args = {} - pinparams.zip(values).each { |param, value| - if value =~ /^[0-9]+$/ - value = value.to_i - end - unless value.nil? - args[param] = value - end - } + unless @resource.deleting? + @resource[:ensure] = metadata.ftype + end - # Now decide whether we're doing checksums or symlinks - if args[:type] == "link" - args[:target] = other - else - args[:checksum] = other + if metadata.ftype == "link" + @resource[:target] = metadata.destination end + end - # we can't manage ownership unless we're root, so don't even try - unless Puppet::Util::SUIDManager.uid == 0 - args.delete(:owner) + # Ask the file server to describe our file. + def describe(source) + begin + Puppet::FileServing::Metadata.find source + rescue => detail + fail detail, "Could not retrieve file metadata for %s: %s" % [path, detail] end - - return args end # Use the info we get from describe() to check if we're in sync. @@ -142,7 +133,7 @@ module Puppet # the only thing this actual state can do is copy files around. Therefore, # only pay attention if the remote is a file. - unless @stats[:type] == "file" + unless @metadata.ftype == "file" return true end @@ -153,15 +144,15 @@ module Puppet end # Now, we just check to see if the checksums are the same parentchecksum = @resource.property(:checksum).retrieve - result = (!parentchecksum.nil? and (parentchecksum == @stats[:checksum])) + result = (!parentchecksum.nil? and (parentchecksum == @metadata.checksum)) # Diff the contents if they ask it. This is quite annoying -- we need to do this in # 'insync?' because they might be in noop mode, but we don't want to do the file # retrieval twice, so we cache the value. - if ! result and Puppet[:show_diff] and File.exists?(@resource[:path]) and ! @stats[:_diffed] - @stats[:_remote_content] = get_remote_content - string_file_diff(@resource[:path], @stats[:_remote_content]) - @stats[:_diffed] = true + if ! result and Puppet[:show_diff] and File.exists?(@resource[:path]) and ! @metadata._diffed + @metadata._remote_content = get_remote_content + string_file_diff(@resource[:path], @metadata._remote_content) + @metadata._diffed = true end return result end @@ -171,55 +162,34 @@ module Puppet end def found? - ! (@stats.nil? or @stats[:type].nil?) + ! (@metadata.nil? or @metadata.ftype.nil?) end - # This basically calls describe() on our file, and then sets all - # of the local states appropriately. If the remote file is a normal - # file then we set it to copy; if it's a directory, then we just mark - # that the local directory should be created. - def retrieve(remote = true) - sum = nil - @source = nil - - # This is set to false by the File#retrieve function on the second - # retrieve, so that we do not do two describes. - if remote - # Find the first source that exists. @shouldorig contains - # the sources as specified by the user. - @should.each { |source| - if @stats = self.describe(source) - @source = source - break + # Provide, and retrieve if necessary, the metadata for this file. Fail + # if we can't find data about this host, and fail if there are any + # problems in our query. + attr_writer :metadata + def metadata + unless defined?(@metadata) and @metadata + return @metadata = nil unless should + should.each do |source| + begin + if data = Puppet::FileServing::Metadata.find(source) + @metadata = data + @metadata.source = source + break + end + rescue => detail + fail detail, "Could not retrieve file metadata for %s: %s" % [source, detail] end - } - end - - if !found? - raise Puppet::Error, "No specified source was found from" + @should.inject("") { |s, source| s + " #{source},"}.gsub(/,$/,"") - end - - case @stats[:type] - when "directory", "file", "link": - @resource[:ensure] = @stats[:type] unless @resource.deleting? - else - self.info @stats.inspect - self.err "Cannot use files of type %s as sources" % @stats[:type] - return :nocopy + end + fail "Could not retrieve information from source(s) %s" % @should.join(", ") unless @metadata end + return @metadata + end - # Take each of the stats and set them as states on the local file - # if a value has not already been provided. - @stats.each { |stat, value| - next if stat == :checksum - next if stat == :type - - # was the stat already specified, or should the value - # be inherited from the source? - @resource[stat] = value unless @resource.argument?(stat) - } - - return @stats[:checksum] + def retrieve + copy_source_values end def should @@ -238,11 +208,13 @@ module Puppet end def sync - contents = @stats[:_remote_content] || get_remote_content() - exists = File.exists?(@resource[:path]) - @resource.write(contents, :source, @stats[:checksum]) + if content = Puppet::FileServing::Content.find(@metadata.source) + @resource.write(content.content, :source, @metadata.checksum) + else + raise "Could not retrieve content" + end if exists return :file_changed @@ -250,27 +222,5 @@ module Puppet return :file_created end end - - private - - def get_remote_content - raise Puppet::DevError, "Got told to copy non-file %s" % @resource[:path] unless @stats[:type] == "file" - - sourceobj, path = @resource.uri2obj(@source) - - begin - contents = sourceobj.server.retrieve(path, @resource[:links]) - rescue => detail - self.fail "Could not retrieve %s: %s" % [path, detail] - end - - contents = CGI.unescape(contents) unless sourceobj.server.local - - if contents == "" - self.notice "Could not retrieve contents for %s" % @source - end - - return contents - end end end -- cgit From be4c0e7fbe5e652ec1d49eab2fdc7a5fbbc486f3 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Tue, 26 Aug 2008 23:12:03 -0700 Subject: The file source is now refactored and uses REST. Signed-off-by: Luke Kanies --- lib/puppet/type/file.rb | 7 ++++++ lib/puppet/type/file/source.rb | 54 +++++++++++++++++++++++------------------- 2 files changed, 36 insertions(+), 25 deletions(-) (limited to 'lib') diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index ef010efda..bc2e53c9f 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -332,6 +332,13 @@ module Puppet recurse() end + def flush + # We want to make sure we retrieve metadata anew on each transaction. + @parameters.each do |name, param| + param.flush if param.respond_to?(:flush) + end + end + # Deal with backups. def handlebackup(file = nil) # let the path be specified diff --git a/lib/puppet/type/file/source.rb b/lib/puppet/type/file/source.rb index e8db13cfa..5eefb1dcb 100755 --- a/lib/puppet/type/file/source.rb +++ b/lib/puppet/type/file/source.rb @@ -83,9 +83,9 @@ module Puppet def change_to_s(currentvalue, newvalue) # newvalue = "{md5}" + @metadata.checksum if @resource.property(:ensure).retrieve == :absent - return "creating from source %s with contents %s" % [@source, @metadata.checksum] + return "creating from source %s with contents %s" % [metadata.source, @metadata.checksum] else - return "replacing from source %s with contents %s" % [@source, @metadata.checksum] + return "replacing from source %s with contents %s" % [metadata.source, @metadata.checksum] end end @@ -97,6 +97,19 @@ module Puppet end end + # Look up (if necessary) and return remote content. + def content + raise Puppet::DevError, "No source for content was stored with the metadata" unless metadata.source + + unless defined?(@content) and @content + unless tmp = Puppet::FileServing::Content.find(@metadata.source) + fail "Could not find any content at %s" % @metadata.source + end + @content = tmp.content + end + @content + end + # Copy the values from the source to the resource. Yay. def copy_source_values devfail "Somehow got asked to copy source values without any metadata" unless metadata @@ -116,21 +129,15 @@ module Puppet end end - # Ask the file server to describe our file. - def describe(source) - begin - Puppet::FileServing::Metadata.find source - rescue => detail - fail detail, "Could not retrieve file metadata for %s: %s" % [path, detail] - end + # Remove any temporary attributes we manage. + def flush + @metadata = nil + @content = nil end - - # Use the info we get from describe() to check if we're in sync. + + # Use the remote metadata to see if we're in sync. + # LAK:NOTE This method should still get refactored. def insync?(currentvalue) - if currentvalue == :nocopy - return true - end - # the only thing this actual state can do is copy files around. Therefore, # only pay attention if the remote is a file. unless @metadata.ftype == "file" @@ -149,10 +156,8 @@ module Puppet # Diff the contents if they ask it. This is quite annoying -- we need to do this in # 'insync?' because they might be in noop mode, but we don't want to do the file # retrieval twice, so we cache the value. - if ! result and Puppet[:show_diff] and File.exists?(@resource[:path]) and ! @metadata._diffed - @metadata._remote_content = get_remote_content - string_file_diff(@resource[:path], @metadata._remote_content) - @metadata._diffed = true + if ! result and Puppet[:show_diff] and File.exists?(@resource[:path]) + string_file_diff(@resource[:path], content) end return result end @@ -188,10 +193,13 @@ module Puppet return @metadata end + # Just call out to our copy method. Hopefully we'll refactor 'source' to + # be a parameter soon, in which case 'retrieve' is unnecessary. def retrieve copy_source_values end + # Return the whole array, rather than the first item. def should @should end @@ -208,13 +216,9 @@ module Puppet end def sync - exists = File.exists?(@resource[:path]) + exists = FileTest.exist?(@resource[:path]) - if content = Puppet::FileServing::Content.find(@metadata.source) - @resource.write(content.content, :source, @metadata.checksum) - else - raise "Could not retrieve content" - end + @resource.write(content, :source, @metadata.checksum) if exists return :file_changed -- cgit From ac419872e273dc31635f042bb1a23c7785dc227a Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Tue, 26 Aug 2008 23:29:04 -0700 Subject: Fixing the terminus helper so it correctly catches options passed from clients via REST. Signed-off-by: Luke Kanies --- lib/puppet/file_serving/terminus_helper.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/file_serving/terminus_helper.rb b/lib/puppet/file_serving/terminus_helper.rb index 598a5007a..b51e27297 100644 --- a/lib/puppet/file_serving/terminus_helper.rb +++ b/lib/puppet/file_serving/terminus_helper.rb @@ -9,7 +9,16 @@ require 'puppet/file_serving/fileset' module Puppet::FileServing::TerminusHelper # Create model instances for all files in a fileset. def path2instances(request, path) - args = [:links, :ignore, :recurse].inject({}) { |hash, param| hash[param] = request.options[param] if request.options[param]; hash } + args = [:links, :ignore, :recurse].inject({}) do |hash, param| + if request.options.include?(param) # use 'include?' so the values can be false + hash[param] = request.options[param] + elsif request.options.include?(param.to_s) + hash[param] = request.options[param.to_s] + end + hash[param] = true if hash[param] == "true" + hash[param] = false if hash[param] == "false" + hash + end Puppet::FileServing::Fileset.new(path, args).files.collect do |file| inst = model.new(path, :relative_path => file) inst.links = request.options[:links] if request.options[:links] -- cgit From 7c68fdb46802dbd3a57f5f7be3333ed6feacad45 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Wed, 27 Aug 2008 21:53:00 -0700 Subject: Fixing FileServing::Base so that it can recurse on a single file. It was throwing exceptions if you tried to use it on a file instead of a directory. Signed-off-by: Luke Kanies --- lib/puppet/file_serving/base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/file_serving/base.rb b/lib/puppet/file_serving/base.rb index 9a50833de..c59a54786 100644 --- a/lib/puppet/file_serving/base.rb +++ b/lib/puppet/file_serving/base.rb @@ -23,7 +23,7 @@ class Puppet::FileServing::Base # Return the full path to our file. Fails if there's no path set. def full_path - if relative_path.nil? or relative_path == "" + if relative_path.nil? or relative_path == "" or relative_path == "." path else File.join(path, relative_path) -- cgit From ee1a85d61bb6ee9b31ae4881adc0c136a99e42ed Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Wed, 27 Aug 2008 23:27:22 -0700 Subject: Mostly finishing refactoring file recursion to use REST. We have the majority of the work done (and it's a *lot* less code). We just have six more tests we need to implement the code for. Signed-off-by: Luke Kanies --- lib/puppet/type/file.rb | 57 +++++++++++++++++++++++++++++++----------- lib/puppet/type/file/source.rb | 10 ++++++-- 2 files changed, 50 insertions(+), 17 deletions(-) (limited to 'lib') diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index bc2e53c9f..2e3a585c1 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -329,7 +329,7 @@ module Puppet # Create any children via recursion or whatever. def eval_generate - recurse() + recurse() if self.recurse? end def flush @@ -679,27 +679,54 @@ module Puppet @parameters.include?(:purge) and (self[:purge] == :true or self[:purge] == "true") end + # Recurse the target of the link. + def recurse_link + perform_recursion(self[:target]) + end + + # Recurse the file itself, returning a Metadata instance for every found file. + def recurse_local + perform_recursion(self[:path]) + end + + # Recurse against our remote file. + def recurse_remote + total = self[:source].collect do |source| + next unless result = perform_recursion(source) + result.each { |data| data.source = "%s/%s" % [source, data.relative_path] } + return result if result and ! result.empty? and self[:sourceselect] == :first + result + end.flatten + + # This only happens if we have sourceselect == :all + found = [] + total.find_all do |data| + result = ! found.include?(data.relative_path) + found << data.relative_path unless found.include?(data.relative_path) + result + end + end + + def perform_recursion(path) + Puppet::FileServing::Metadata.search(self[:path], :links => self[:links], :recurse => self[:recurse], :ignore => self[:ignore]) + end + # Recurse into the directory. This basically just calls 'localrecurse' # and maybe 'sourcerecurse', returning the collection of generated # files. def recurse - # are we at the end of the recursion? - return unless self.recurse? - - recurse = self[:recurse] - # we might have a string, rather than a number - if recurse.is_a?(String) - if recurse =~ /^[0-9]+$/ - recurse = Integer(recurse) - else # anything else is infinite recursion - recurse = true - end + children = recurse_local + + if self[:target] + children += recurse_link end - if recurse.is_a?(Integer) - recurse -= 1 + if self[:source] + recurse_remote end - + + return children.collect { |child| newchild(child.relative_path) }.reject { |child| child.nil? } + children = [] # We want to do link-recursing before normal recursion so that all diff --git a/lib/puppet/type/file/source.rb b/lib/puppet/type/file/source.rb index 5eefb1dcb..03cc311b4 100755 --- a/lib/puppet/type/file/source.rb +++ b/lib/puppet/type/file/source.rb @@ -66,8 +66,14 @@ module Puppet uncheckable validate do |source| - unless @resource.uri2obj(source) - raise Puppet::Error, "Invalid source %s" % source + begin + uri = URI.parse(URI.escape(source)) + rescue => detail + self.fail "Could not understand source %s: %s" % [source, detail.to_s] + end + + unless uri.scheme.nil? or %w{file puppet}.include?(uri.scheme) + self.fail "Cannot use URLs of type '%s' as source for fileserving" % [uri.scheme] end end -- cgit From 5da26067cc76ad318359d9287ab1267d7a6c5b0b Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Thu, 28 Aug 2008 22:48:01 -0700 Subject: Recursion using REST seems to almost work. I think this is the bulk of the work, I just need to write some integration tests to hunt down a couple of small issues. Signed-off-by: Luke Kanies --- lib/puppet/type/file.rb | 246 ++++++++++++++++-------------------------------- 1 file changed, 83 insertions(+), 163 deletions(-) (limited to 'lib') diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index 2e3a585c1..2ae1e61b7 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -155,8 +155,6 @@ module Puppet engine, so shell metacharacters are fully supported, e.g. ``[a-z]*``. Matches that would descend into the directory structure are ignored, e.g., ``*/*``." - - defaultto false validate do |value| unless value.is_a?(Array) or value.is_a?(String) or value == false @@ -277,11 +275,6 @@ module Puppet @depthfirst = false - - def argument?(arg) - @arghash.include?(arg) - end - # Determine the user to write files as. def asuser if self.should(:owner) and ! self.should(:owner).is_a?(Symbol) @@ -329,7 +322,12 @@ module Puppet # Create any children via recursion or whatever. def eval_generate - recurse() if self.recurse? + raise(Puppet::DevError, "File recursion cannot happen without a catalog") unless catalog + + return nil unless self.recurse? + recurse.reject { |resource| catalog.resource(:file, resource[:path]) }.each do |child| + catalog.relationship_graph.add_edge self, child + end end def flush @@ -462,17 +460,6 @@ module Puppet @title.sub!(/\/$/, "") unless @title == "/" - # Clean out as many references to any file paths as possible. - # This was the source of many, many bugs. - @arghash = tmphash - @arghash.delete(self.class.namevar) - - [:source, :parent].each do |param| - if @arghash.include?(param) - @arghash.delete(param) - end - end - @stat = nil end @@ -568,88 +555,15 @@ module Puppet # Create a new file or directory object as a child to the current # object. - def newchild(path, local, hash = {}) - raise(Puppet::DevError, "File recursion cannot happen without a catalog") unless catalog - - # make local copy of arguments - args = symbolize_options(@arghash) - - # There's probably a better way to do this, but we don't want - # to pass this info on. - if v = args[:ensure] - v = symbolize(v) - args.delete(:ensure) - end - - if path =~ %r{^#{File::SEPARATOR}} - self.devfail( - "Must pass relative paths to PFile#newchild()" - ) - else - path = File.join(self[:path], path) - end - - args[:path] = path - - unless hash.include?(:recurse) - if args.include?(:recurse) - if args[:recurse].is_a?(Integer) - args[:recurse] -= 1 # reduce the level of recursion - end - end - - end - - hash.each { |key,value| - args[key] = value - } + def newchild(path) + full_path = File.join(self[:path], path) - child = nil - - # The child might already exist because 'localrecurse' runs - # before 'sourcerecurse'. I could push the override stuff into - # a separate method or something, but the work is the same other - # than this last bit, so it doesn't really make sense. - if child = catalog.resource(:file, path) - unless child.parent.object_id == self.object_id - self.debug "Not managing more explicit file %s" % - path - return nil - end + # the right-side hash wins in the merge. + options = to_hash.merge(:path => full_path) + options.delete(:parent) if options.include?(:parent) + options.delete(:recurse) if options.include?(:recurse) - # This is only necessary for sourcerecurse, because we might have - # created the object with different 'should' values than are - # set remotely. - unless local - args.each { |var,value| - next if var == :path - next if var == :name - - # behave idempotently - unless child.should(var) == value - child[var] = value - end - } - end - return nil - else # create it anew - #notice "Creating new file with args %s" % args.inspect - args[:parent] = self - begin - # This method is used by subclasses of :file, so use the class name rather than hard-coding - # :file. - return nil unless child = catalog.create_implicit_resource(self.class.name, args) - rescue => detail - self.notice "Cannot manage: %s" % [detail] - return nil - end - end - - # LAK:FIXME This shouldn't be necessary, but as long as we're - # modeling the relationship graph specifically, it is. - catalog.relationship_graph.add_edge self, child - - return child + return catalog.create_implicit_resource(self.class.name, options) end # Files handle paths specially, because they just lengthen their @@ -679,97 +593,103 @@ module Puppet @parameters.include?(:purge) and (self[:purge] == :true or self[:purge] == "true") end + def make_children(metadata) + metadata.collect { |meta| newchild(meta.relative_path) } + end + + # Recursively generate a list of file resources, which will + # be used to copy remote files, manage local files, and/or make links + # to map to another directory. + def recurse + children = recurse_local + + if self[:target] + recurse_link(children) + elsif self[:source] + recurse_remote(children) + end + + return children.values.sort { |a, b| a[:path] <=> b[:path] } + end + + # A simple method for determining whether we should be recursing. + def recurse? + return false unless @parameters.include?(:recurse) + + val = @parameters[:recurse].value + + if val and (val == true or val > 0) + return true + else + return false + end + end + # Recurse the target of the link. - def recurse_link - perform_recursion(self[:target]) + def recurse_link(children) + perform_recursion(self[:target]).each do |meta| + children[meta.relative_path] ||= newchild(meta.relative_path) + if meta.ftype == "directory" + children[meta.relative_path][:ensure] = :directory + else + children[meta.relative_path][:ensure] = :link + children[meta.relative_path][:target] = meta.full_path + end + end + children end # Recurse the file itself, returning a Metadata instance for every found file. def recurse_local - perform_recursion(self[:path]) + perform_recursion(self[:path]).inject({}) { |hash, meta| hash[meta.relative_path] = newchild(meta.relative_path); hash } end # Recurse against our remote file. - def recurse_remote + def recurse_remote(children) + sourceselect = self[:sourceselect] + total = self[:source].collect do |source| next unless result = perform_recursion(source) result.each { |data| data.source = "%s/%s" % [source, data.relative_path] } - return result if result and ! result.empty? and self[:sourceselect] == :first + break result if result and ! result.empty? and sourceselect == :first result end.flatten # This only happens if we have sourceselect == :all - found = [] - total.find_all do |data| - result = ! found.include?(data.relative_path) - found << data.relative_path unless found.include?(data.relative_path) - result - end - end - - def perform_recursion(path) - Puppet::FileServing::Metadata.search(self[:path], :links => self[:links], :recurse => self[:recurse], :ignore => self[:ignore]) - end - - # Recurse into the directory. This basically just calls 'localrecurse' - # and maybe 'sourcerecurse', returning the collection of generated - # files. - def recurse - children = recurse_local - - if self[:target] - children += recurse_link - end - - if self[:source] - recurse_remote - end - - return children.collect { |child| newchild(child.relative_path) }.reject { |child| child.nil? } - - children = [] - - # We want to do link-recursing before normal recursion so that all - # of the target stuff gets copied over correctly. - if @parameters.include? :target and ret = self.linkrecurse(recurse) - children += ret - end - if ret = self.localrecurse(recurse) - children += ret + unless sourceselect == :first + found = [] + total.reject! do |data| + result = found.include?(data.relative_path) + found << data.relative_path unless found.include?(data.relative_path) + result + end end - # These will be files pulled in by the file source - sourced = false - if @parameters.include?(:source) - ret, sourced = self.sourcerecurse(recurse) - if ret - children += ret - end + total.each do |meta| + children[meta.relative_path] ||= newchild(meta.relative_path) + children[meta.relative_path][:source] = meta.source + children[meta.relative_path].property(:source).metadata = meta end - # The purge check needs to happen after all of the other recursion. + # If we're purging resources, then delete any resource that isn't on the + # remote system. if self.purge? - children.each do |child| - if (sourced and ! sourced.include?(child[:path])) or ! child.managed? + # Make a hash of all of the resources we found remotely -- all we need is the + # fast lookup, the values don't matter. + remotes = total.inject({}) { |hash, meta| hash[meta.relative_path] = true; hash } + + children.each do |name, child| + unless remotes.include?(name) child[:ensure] = :absent end end end - + children end - # A simple method for determining whether we should be recursing. - def recurse? - return false unless @parameters.include?(:recurse) - - val = @parameters[:recurse].value - - if val and (val == true or val > 0) - return true - else - return false - end + def perform_recursion(path) + Puppet::FileServing::Metadata.search(path, :links => self[:links], :recurse => self[:recurse], :ignore => self[:ignore]) end # Remove the old backup. -- cgit From bd1163a339ff66dbb9a50a1cb13f6320cb056cc3 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Thu, 28 Aug 2008 23:09:52 -0700 Subject: Fixing filesets to allow nil ignore values. Signed-off-by: Luke Kanies --- lib/puppet/file_serving/fileset.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'lib') diff --git a/lib/puppet/file_serving/fileset.rb b/lib/puppet/file_serving/fileset.rb index 80a718c68..a90734a2b 100644 --- a/lib/puppet/file_serving/fileset.rb +++ b/lib/puppet/file_serving/fileset.rb @@ -30,6 +30,8 @@ class Puppet::FileServing::Fileset # Should we ignore this path? def ignore?(path) + return false if @ignore == [nil] + # 'detect' normally returns the found result, whereas we just want true/false. ! @ignore.detect { |pattern| File.fnmatch?(pattern, path) }.nil? end -- cgit From 93fc1139550bd97a11529b812e77ac0fc00c6079 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Thu, 28 Aug 2008 23:28:22 -0700 Subject: Files now use the Indirector to recurse locally. I don't yet have integration tests for remote recursion or link recursion, but we're nearly there. Signed-off-by: Luke Kanies --- lib/puppet/type/file.rb | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index 2ae1e61b7..a59192af3 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -322,10 +322,14 @@ module Puppet # Create any children via recursion or whatever. def eval_generate - raise(Puppet::DevError, "File recursion cannot happen without a catalog") unless catalog - return nil unless self.recurse? - recurse.reject { |resource| catalog.resource(:file, resource[:path]) }.each do |child| + + raise(Puppet::DevError, "Cannot generate resources for recursion without a catalog") unless catalog + + recurse.reject do |resource| + catalog.resource(:file, resource[:path]) + end.each do |child| + catalog.add_resource child catalog.relationship_graph.add_edge self, child end end @@ -559,11 +563,11 @@ module Puppet full_path = File.join(self[:path], path) # the right-side hash wins in the merge. - options = to_hash.merge(:path => full_path) + options = to_hash.merge(:path => full_path, :implicit => true) options.delete(:parent) if options.include?(:parent) options.delete(:recurse) if options.include?(:recurse) - return catalog.create_implicit_resource(self.class.name, options) + return self.class.create(options) end # Files handle paths specially, because they just lengthen their @@ -641,7 +645,12 @@ module Puppet # Recurse the file itself, returning a Metadata instance for every found file. def recurse_local - perform_recursion(self[:path]).inject({}) { |hash, meta| hash[meta.relative_path] = newchild(meta.relative_path); hash } + perform_recursion(self[:path]).inject({}) do |hash, meta| + next hash if meta.relative_path == "." + + hash[meta.relative_path] = newchild(meta.relative_path) + hash + end end # Recurse against our remote file. -- cgit From 45f465bc98aa87e1066a9d748dbb6bfaaef61476 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Fri, 29 Aug 2008 00:48:40 -0700 Subject: Source recursion is nearly working. It works, but you have to run it multiple times, and there are still a couple of strangenesses with the parameter values, such as the mode not getting set on the first run. Signed-off-by: Luke Kanies --- lib/puppet/type/file.rb | 29 ++++++++++++++++++++++------- lib/puppet/type/file/source.rb | 11 ++++++----- 2 files changed, 28 insertions(+), 12 deletions(-) (limited to 'lib') diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index a59192af3..ce1df1c35 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -564,8 +564,9 @@ module Puppet # the right-side hash wins in the merge. options = to_hash.merge(:path => full_path, :implicit => true) - options.delete(:parent) if options.include?(:parent) - options.delete(:recurse) if options.include?(:recurse) + [:parent, :recurse, :target].each do |param| + options.delete(param) if options.include?(param) + end return self.class.create(options) end @@ -632,6 +633,11 @@ module Puppet # Recurse the target of the link. def recurse_link(children) perform_recursion(self[:target]).each do |meta| + if meta.relative_path == "." + self[:ensure] = :directory + next + end + children[meta.relative_path] ||= newchild(meta.relative_path) if meta.ftype == "directory" children[meta.relative_path][:ensure] = :directory @@ -645,7 +651,9 @@ module Puppet # Recurse the file itself, returning a Metadata instance for every found file. def recurse_local - perform_recursion(self[:path]).inject({}) do |hash, meta| + result = perform_recursion(self[:path]) + return {} unless result + result.inject({}) do |hash, meta| next hash if meta.relative_path == "." hash[meta.relative_path] = newchild(meta.relative_path) @@ -675,8 +683,14 @@ module Puppet end total.each do |meta| + if meta.relative_path == "." + property(:source).metadata = meta + next + end children[meta.relative_path] ||= newchild(meta.relative_path) children[meta.relative_path][:source] = meta.source + children[meta.relative_path][:checksum] = :md5 if meta.ftype == "file" + children[meta.relative_path].property(:source).metadata = meta end @@ -759,21 +773,22 @@ module Puppet # a wrapper method to make sure the file exists before doing anything def retrieve unless stat = self.stat(true) - # If the file doesn't exist but we have a source, then call - # retrieve on that property propertyvalues = properties().inject({}) { |hash, property| hash[property] = :absent hash } + # If the file doesn't exist but we have a source, then call + # retrieve on the source property so it will set the 'should' + # values all around. if @parameters.include?(:source) - propertyvalues[:source] = @parameters[:source].retrieve + @parameters[:source].copy_source_values end return propertyvalues end - return currentpropvalues() + currentpropvalues() end # This recurses against the remote source and makes sure the local diff --git a/lib/puppet/type/file/source.rb b/lib/puppet/type/file/source.rb index 03cc311b4..e43706051 100755 --- a/lib/puppet/type/file/source.rb +++ b/lib/puppet/type/file/source.rb @@ -122,13 +122,14 @@ module Puppet # Take each of the stats and set them as states on the local file # if a value has not already been provided. - [:owner, :mode, :group].each do |param| - @resource[param] ||= metadata.send(param) + [:owner, :mode, :group, :checksum].each do |param| + next if param == :owner and Puppet::Util::SUIDManager.uid != 0 + unless value = @resource[param] and value != :absent + @resource[param] = metadata.send(param) + end end - unless @resource.deleting? - @resource[:ensure] = metadata.ftype - end + @resource[:ensure] = metadata.ftype if metadata.ftype == "link" @resource[:target] = metadata.destination -- cgit From b69c50ccd3a491b6c4a8d456af2fe6f9cac45eae Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Fri, 29 Aug 2008 01:25:17 -0700 Subject: Removing insanely stupid default property behaviour. Basically, I had the [] method on resources returning the 'should' value if one was available, but if one wasn't available, it would retrieve the current value from the resource. This resulted in all kinds of completely ridiculous behaviours. Signed-off-by: Luke Kanies --- lib/puppet/parameter.rb | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) (limited to 'lib') diff --git a/lib/puppet/parameter.rb b/lib/puppet/parameter.rb index 31e009af5..e8e962931 100644 --- a/lib/puppet/parameter.rb +++ b/lib/puppet/parameter.rb @@ -417,17 +417,7 @@ class Puppet::Parameter # it possible to call for properties, too. def value if self.is_a?(Puppet::Property) - # We should return the 'is' value if there's not 'should' - # value. This might be bad, though, because the 'should' - # method knows whether to return an array or not and that info - # is not exposed, and the 'is' value could be a symbol. I - # can't seem to create a test in which this is a problem, but - # that doesn't mean it's not one. - if self.should - return self.should - else - return self.retrieve - end + self.should else if defined? @value return @value -- cgit From a9b7f0881aed04fbbca59947cab0ffeedda6d2f8 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Fri, 29 Aug 2008 01:29:20 -0700 Subject: As far as I can tell, recursion is working entirely. W00t! Signed-off-by: Luke Kanies --- lib/puppet/type/file.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index ce1df1c35..543b0710e 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -563,7 +563,7 @@ module Puppet full_path = File.join(self[:path], path) # the right-side hash wins in the merge. - options = to_hash.merge(:path => full_path, :implicit => true) + options = to_hash.merge(:path => full_path, :implicit => true).reject { |param, value| value.nil? } [:parent, :recurse, :target].each do |param| options.delete(param) if options.include?(param) end -- cgit From ac5db5ec115455e54090542870847820357739a2 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Fri, 29 Aug 2008 01:40:47 -0700 Subject: Removing the old, obsolete recursion methods. Signed-off-by: Luke Kanies --- lib/puppet/type/file.rb | 168 ------------------------------------------------ 1 file changed, 168 deletions(-) (limited to 'lib') diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index 543b0710e..370ce1b4f 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -466,96 +466,6 @@ module Puppet @stat = nil end - - # Build a recursive map of a link source - def linkrecurse(recurse) - target = @parameters[:target].should - - method = :lstat - if self[:links] == :follow - method = :stat - end - - targetstat = nil - unless FileTest.exist?(target) - return - end - # Now stat our target - targetstat = File.send(method, target) - unless targetstat.ftype == "directory" - return - end - - # Now that we know our corresponding target is a directory, - # change our type - self[:ensure] = :directory - - unless FileTest.readable? target - self.notice "Cannot manage %s: permission denied" % self.name - return - end - - children = Dir.entries(target).reject { |d| d =~ /^\.+$/ } - - # Get rid of ignored children - if @parameters.include?(:ignore) - children = handleignore(children) - end - - added = [] - children.each do |file| - Dir.chdir(target) do - longname = File.join(target, file) - - # Files know to create directories when recursion - # is enabled and we're making links - args = { - :recurse => recurse, - :ensure => longname - } - - if child = self.newchild(file, true, args) - added << child - end - end - end - - added - end - - # Build up a recursive map of what's around right now - def localrecurse(recurse) - unless FileTest.exist?(self[:path]) and self.stat.directory? - #self.info "%s is not a directory; not recursing" % - # self[:path] - return - end - - unless FileTest.readable? self[:path] - self.notice "Cannot manage %s: permission denied" % self.name - return - end - - children = Dir.entries(self[:path]) - - #Get rid of ignored children - if @parameters.include?(:ignore) - children = handleignore(children) - end - - added = [] - children.each { |file| - file = File.basename(file) - next if file =~ /^\.\.?$/ # skip . and .. - options = {:recurse => recurse} - - if child = self.newchild(file, true, options) - added << child - end - } - - added - end # Create a new file or directory object as a child to the current # object. @@ -791,84 +701,6 @@ module Puppet currentpropvalues() end - # This recurses against the remote source and makes sure the local - # and remote structures match. It's run after 'localrecurse'. This - # method only does anything when its corresponding remote entry is - # a directory; in that case, this method creates file objects that - # correspond to any contained remote files. - def sourcerecurse(recurse) - # we'll set this manually as necessary - if @arghash.include?(:ensure) - @arghash.delete(:ensure) - end - - r = false - if recurse - unless recurse == 0 - r = 1 - end - end - - ignore = self[:ignore] - - result = [] - found = [] - - # Keep track of all the files we found in the source, so we can purge - # appropriately. - sourced = [] - - @parameters[:source].should.each do |source| - sourceobj, path = uri2obj(source) - - # okay, we've got our source object; now we need to - # build up a local file structure to match the remote - # one - - server = sourceobj.server - - desc = server.list(path, self[:links], r, ignore) - if desc == "" - next - end - - # Now create a new child for every file returned in the list. - result += desc.split("\n").collect { |line| - file, type = line.split("\t") - next if file == "/" # skip the listing object - name = file.sub(/^\//, '') - - # This makes sure that the first source *always* wins - # for conflicting files. - next if found.include?(name) - - # For directories, keep all of the sources, so that - # sourceselect still works as planned. - if type == "directory" - newsource = @parameters[:source].should.collect do |tmpsource| - tmpsource + file - end - else - newsource = source + file - end - args = {:source => newsource} - if type == file - args[:recurse] = nil - end - - found << name - sourced << File.join(self[:path], name) - - self.newchild(name, false, args) - }.reject {|c| c.nil? } - - if self[:sourceselect] == :first - return [result, sourced] - end - end - return [result, sourced] - end - # Set the checksum, from another property. There are multiple # properties that modify the contents of a file, and they need the # ability to make sure that the checksum value is in sync. -- cgit