diff options
42 files changed, 1148 insertions, 561 deletions
diff --git a/install.rb b/install.rb index 6854363ca..6bf4f5587 100755 --- a/install.rb +++ b/install.rb @@ -80,8 +80,12 @@ def do_configs(configs, target, strip = 'conf/') Dir.mkdir(target) unless File.directory? target configs.each do |cf| ocf = File.join(InstallOptions.config_dir, cf.gsub(/#{strip}/, '')) - File.install(cf, ocf, 0644, true) - end + if $haveftools + File.install(cf, ocf, 0644, true) + else + FileUtils.install(cf, ocf, {:mode => 0644, :verbose => true}) + end + end end def do_bins(bins, target, strip = 's?bin/') diff --git a/lib/puppet/application.rb b/lib/puppet/application.rb index 7ef71bc81..57bd88877 100644 --- a/lib/puppet/application.rb +++ b/lib/puppet/application.rb @@ -1,4 +1,5 @@ require 'optparse' +require 'puppet/util/plugins' # This class handles all the aspects of a Puppet application/executable # * setting up options @@ -298,11 +299,11 @@ class Application # This is the main application entry point def run - exit_on_fail("initialize") { preinit } - exit_on_fail("parse options") { parse_options } + exit_on_fail("initialize") { hook('preinit') { preinit } } + exit_on_fail("parse options") { hook('parse_options') { parse_options } } exit_on_fail("parse configuration file") { Puppet.settings.parse } if should_parse_config? - exit_on_fail("prepare for execution") { setup } - exit_on_fail("run") { run_command } + exit_on_fail("prepare for execution") { hook('setup') { setup } } + exit_on_fail("run") { hook('run_command') { run_command } } end def main @@ -392,11 +393,18 @@ class Application private def exit_on_fail(message, code = 1) - yield + yield rescue RuntimeError, NotImplementedError => detail - puts detail.backtrace if Puppet[:trace] - $stderr.puts "Could not #{message}: #{detail}" - exit(code) + puts detail.backtrace if Puppet[:trace] + $stderr.puts "Could not #{message}: #{detail}" + exit(code) + end + + def hook(step,&block) + Puppet::Plugins.send("before_application_#{step}",:application_object => self) + x = yield + Puppet::Plugins.send("after_application_#{step}",:application_object => self, :return_value => x) + x end end end diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb index 72e387c64..cfeb73a1f 100644 --- a/lib/puppet/configurer.rb +++ b/lib/puppet/configurer.rb @@ -173,9 +173,7 @@ class Puppet::Configurer report.finalize_report if trans puts report.summary if Puppet[:summarize] save_last_run_summary(report) - if Puppet[:report] - Puppet::Transaction::Report.indirection.save(report) - end + Puppet::Transaction::Report.indirection.save(report) if Puppet[:report] rescue => detail puts detail.backtrace if Puppet[:trace] Puppet.err "Could not send report: #{detail}" diff --git a/lib/puppet/indirector/certificate_status.rb b/lib/puppet/indirector/certificate_status.rb new file mode 100644 index 000000000..47c3adcd4 --- /dev/null +++ b/lib/puppet/indirector/certificate_status.rb @@ -0,0 +1,4 @@ +require 'puppet/indirector' + +class Puppet::Indirector::CertificateStatus +end diff --git a/lib/puppet/indirector/certificate_status/file.rb b/lib/puppet/indirector/certificate_status/file.rb new file mode 100644 index 000000000..9061d9423 --- /dev/null +++ b/lib/puppet/indirector/certificate_status/file.rb @@ -0,0 +1,82 @@ +require 'puppet' +require 'puppet/indirector/certificate_status' +require 'puppet/ssl/certificate' +require 'puppet/ssl/certificate_authority' +require 'puppet/ssl/certificate_request' +require 'puppet/ssl/host' +require 'puppet/ssl/key' + +class Puppet::Indirector::CertificateStatus::File < Puppet::Indirector::Code + def ca + raise ArgumentError, "This process is not configured as a certificate authority" unless Puppet::SSL::CertificateAuthority.ca? + Puppet::SSL::CertificateAuthority.new + end + + def destroy(request) + deleted = [] + [ + Puppet::SSL::Certificate, + Puppet::SSL::CertificateRequest, + Puppet::SSL::Key, + ].collect do |part| + if part.indirection.destroy(request.key) + deleted << "#{part}" + end + end + + return "Nothing was deleted" if deleted.empty? + "Deleted for #{request.key}: #{deleted.join(", ")}" + end + + def save(request) + if request.instance.desired_state == "signed" + certificate_request = Puppet::SSL::CertificateRequest.indirection.find(request.key) + raise Puppet::Error, "Cannot sign for host #{request.key} without a certificate request" unless certificate_request + ca.sign(request.key) + elsif request.instance.desired_state == "revoked" + certificate = Puppet::SSL::Certificate.indirection.find(request.key) + raise Puppet::Error, "Cannot revoke host #{request.key} because has it doesn't have a signed certificate" unless certificate + ca.revoke(request.key) + else + raise Puppet::Error, "State #{request.instance.desired_state} invalid; Must specify desired state of 'signed' or 'revoked' for host #{request.key}" + end + + end + + def search(request) + # Support historic interface wherein users provide classes to filter + # the search. When used via the REST API, the arguments must be + # a Symbol or an Array containing Symbol objects. + klasses = case request.options[:for] + when Class + [request.options[:for]] + when nil + [ + Puppet::SSL::Certificate, + Puppet::SSL::CertificateRequest, + Puppet::SSL::Key, + ] + else + [request.options[:for]].flatten.map do |klassname| + indirection.class.model(klassname.to_sym) + end + end + + klasses.collect do |klass| + klass.indirection.search(request.key, request.options) + end.flatten.collect do |result| + result.name + end.uniq.collect &Puppet::SSL::Host.method(:new) + end + + def find(request) + ssl_host = Puppet::SSL::Host.new(request.key) + public_key = Puppet::SSL::Certificate.indirection.find(request.key) + + if ssl_host.certificate_request || public_key + ssl_host + else + nil + end + end +end diff --git a/lib/puppet/indirector/certificate_status/rest.rb b/lib/puppet/indirector/certificate_status/rest.rb new file mode 100644 index 000000000..c53b663b5 --- /dev/null +++ b/lib/puppet/indirector/certificate_status/rest.rb @@ -0,0 +1,10 @@ +require 'puppet/ssl/host' +require 'puppet/indirector/rest' +require 'puppet/indirector/certificate_status' + +class Puppet::Indirector::CertificateStatus::Rest < Puppet::Indirector::REST + desc "Sign, revoke, search for, or clean certificates & certificate requests over HTTP." + + use_server_setting(:ca_server) + use_port_setting(:ca_port) +end diff --git a/lib/puppet/network/http/api/v1.rb b/lib/puppet/network/http/api/v1.rb index dcb0e0a22..5fe143979 100644 --- a/lib/puppet/network/http/api/v1.rb +++ b/lib/puppet/network/http/api/v1.rb @@ -61,6 +61,7 @@ module Puppet::Network::HTTP::API::V1 # that leads to the fix being too long. return :singular if indirection == "facts" return :singular if indirection == "status" + return :singular if indirection == "certificate_status" return :plural if indirection == "inventory" result = (indirection =~ /s$|_search$/) ? :plural : :singular diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb index d50530618..dc631979e 100644 --- a/lib/puppet/node/environment.rb +++ b/lib/puppet/node/environment.rb @@ -79,7 +79,7 @@ class Puppet::Node::Environment # environment has changed do we delve deeper. Thread.current[:known_resource_types] = nil if (krt = Thread.current[:known_resource_types]) && krt.environment != self Thread.current[:known_resource_types] ||= synchronize { - if @known_resource_types.nil? or @known_resource_types.stale? + if @known_resource_types.nil? or @known_resource_types.require_reparse? @known_resource_types = Puppet::Resource::TypeCollection.new(self) @known_resource_types.import_ast(perform_initial_import, '') end @@ -160,6 +160,8 @@ class Puppet::Node::Environment end parser.parse rescue => detail + known_resource_types.parse_failed = true + msg = "Could not parse for environment #{self}: #{detail}" error = Puppet::Error.new(msg) error.set_backtrace(detail.backtrace) diff --git a/lib/puppet/provider/package/gem.rb b/lib/puppet/provider/package/gem.rb index 19414cec4..28731c849 100755 --- a/lib/puppet/provider/package/gem.rb +++ b/lib/puppet/provider/package/gem.rb @@ -22,7 +22,7 @@ Puppet::Type.type(:package).provide :gem, :parent => Puppet::Provider::Package d end if name = hash[:justme] - command << name + command << name + "$" end begin @@ -94,7 +94,7 @@ Puppet::Type.type(:package).provide :gem, :parent => Puppet::Provider::Package d command << "--source" << "#{source}" << resource[:name] end else - command << resource[:name] + command << "--no-rdoc" << "--no-ri" << resource[:name] end output = execute(command) diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb index a8668d844..98c29657e 100644 --- a/lib/puppet/resource/catalog.rb +++ b/lib/puppet/resource/catalog.rb @@ -61,36 +61,32 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph [$1, $2] end - # Add one or more resources to our graph and to our resource table. + # Add a resource to our graph and to our resource table. # This is actually a relatively complicated method, because it handles multiple # aspects of Catalog behaviour: # * Add the resource to the resource table # * Add the resource to the resource graph # * Add the resource to the relationship graph # * Add any aliases that make sense for the resource (e.g., name != title) - def add_resource(*resources) - resources.each do |resource| - raise ArgumentError, "Can only add objects that respond to :ref, not instances of #{resource.class}" unless resource.respond_to?(:ref) - end.each { |resource| fail_on_duplicate_type_and_title(resource) }.each do |resource| - title_key = title_key_for_ref(resource.ref) - - @transient_resources << resource if applying? - @resource_table[title_key] = resource - - # If the name and title differ, set up an alias - - if resource.respond_to?(:name) and resource.respond_to?(:title) and resource.respond_to?(:isomorphic?) and resource.name != resource.title - self.alias(resource, resource.uniqueness_key) if resource.isomorphic? - end - - resource.catalog = self if resource.respond_to?(:catalog=) - - add_vertex(resource) - - @relationship_graph.add_vertex(resource) if @relationship_graph - - yield(resource) if block_given? + def add_resource(*resource) + add_resource(*resource[0..-2]) if resource.length > 1 + resource = resource.pop + raise ArgumentError, "Can only add objects that respond to :ref, not instances of #{resource.class}" unless resource.respond_to?(:ref) + fail_on_duplicate_type_and_title(resource) + title_key = title_key_for_ref(resource.ref) + + @transient_resources << resource if applying? + @resource_table[title_key] = resource + + # If the name and title differ, set up an alias + + if resource.respond_to?(:name) and resource.respond_to?(:title) and resource.respond_to?(:isomorphic?) and resource.name != resource.title + self.alias(resource, resource.uniqueness_key) if resource.isomorphic? end + + resource.catalog = self if resource.respond_to?(:catalog=) + add_vertex(resource) + @relationship_graph.add_vertex(resource) if @relationship_graph end # Create an alias for a resource. @@ -335,13 +331,75 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph @relationship_graph.write_graph(:relationships) if host_config? # Then splice in the container information - @relationship_graph.splice!(self, Puppet::Type::Component) + splice!(@relationship_graph) @relationship_graph.write_graph(:expanded_relationships) if host_config? end @relationship_graph end + # Impose our container information on another graph by using it + # to replace any container vertices X with a pair of verticies + # { admissible_X and completed_X } such that that + # + # 0) completed_X depends on admissible_X + # 1) contents of X each depend on admissible_X + # 2) completed_X depends on each on the contents of X + # 3) everything which depended on X depens on completed_X + # 4) admissible_X depends on everything X depended on + # 5) the containers and their edges must be removed + # + # Note that this requires attention to the possible case of containers + # which contain or depend on other containers, but has the advantage + # that the number of new edges created scales linearly with the number + # of contained verticies regardless of how containers are related; + # alternatives such as replacing container-edges with content-edges + # scale as the product of the number of external dependences, which is + # to say geometrically in the case of nested / chained containers. + # + Default_label = { :callback => :refresh, :event => :ALL_EVENTS } + def splice!(other) + stage_class = Puppet::Type.type(:stage) + whit_class = Puppet::Type.type(:whit) + component_class = Puppet::Type.type(:component) + containers = vertices.find_all { |v| (v.is_a?(component_class) or v.is_a?(stage_class)) and vertex?(v) } + # + # These two hashes comprise the aforementioned attention to the possible + # case of containers that contain / depend on other containers; they map + # containers to their sentinals but pass other verticies through. Thus we + # can "do the right thing" for references to other verticies that may or + # may not be containers. + # + admissible = Hash.new { |h,k| k } + completed = Hash.new { |h,k| k } + containers.each { |x| + admissible[x] = whit_class.new(:name => "admissible_#{x.name}", :catalog => self) + completed[x] = whit_class.new(:name => "completed_#{x.name}", :catalog => self) + } + # + # Implement the six requierments listed above + # + containers.each { |x| + contents = adjacent(x, :direction => :out) + other.add_edge(admissible[x],completed[x]) if contents.empty? # (0) + contents.each { |v| + other.add_edge(admissible[x],admissible[v],Default_label) # (1) + other.add_edge(completed[v], completed[x], Default_label) # (2) + } + # (3) & (5) + other.adjacent(x,:direction => :in,:type => :edges).each { |e| + other.add_edge(completed[e.source],admissible[x],e.label) + other.remove_edge! e + } + # (4) & (5) + other.adjacent(x,:direction => :out,:type => :edges).each { |e| + other.add_edge(completed[x],admissible[e.target],e.label) + other.remove_edge! e + } + } + containers.each { |x| other.remove_vertex! x } # (5) + end + # Remove the resource from our catalog. Notice that we also call # 'remove' on the resource, at least until resource classes no longer maintain # references to the resource instances. diff --git a/lib/puppet/resource/type_collection.rb b/lib/puppet/resource/type_collection.rb index 6ab978f7c..9fe7cdd06 100644 --- a/lib/puppet/resource/type_collection.rb +++ b/lib/puppet/resource/type_collection.rb @@ -1,5 +1,6 @@ class Puppet::Resource::TypeCollection attr_reader :environment + attr_accessor :parse_failed def clear @hostclasses.clear @@ -120,6 +121,10 @@ class Puppet::Resource::TypeCollection end end + def require_reparse? + @parse_failed || stale? + end + def stale? @watched_files.values.detect { |file| file.changed? } end diff --git a/lib/puppet/simple_graph.rb b/lib/puppet/simple_graph.rb index 27e068e30..671eef150 100644 --- a/lib/puppet/simple_graph.rb +++ b/lib/puppet/simple_graph.rb @@ -19,7 +19,7 @@ class Puppet::SimpleGraph # # This class is intended to be used with DAGs. However, if the # graph has a cycle, it will not cause non-termination of any of the - # algorithms. The topsort method detects and reports cycles. + # algorithms. # def initialize @in_to = {} @@ -221,6 +221,7 @@ class Puppet::SimpleGraph def report_cycles_in_graph cycles = find_cycles_in_graph n = cycles.length # where is "pluralize"? --daniel 2011-01-22 + return if n == 0 s = n == 1 ? '' : 's' message = "Found #{n} dependency cycle#{s}:\n" @@ -262,36 +263,6 @@ class Puppet::SimpleGraph return filename end - # Provide a topological sort. - def topsort - degree = {} - zeros = [] - result = [] - - # Collect each of our vertices, with the number of in-edges each has. - vertices.each do |v| - edges = @in_to[v] - zeros << v if edges.empty? - degree[v] = edges.length - end - - # Iterate over each 0-degree vertex, decrementing the degree of - # each of its out-edges. - while v = zeros.pop - result << v - @out_from[v].each { |v2,es| - zeros << v2 if (degree[v2] -= 1) == 0 - } - end - - # If we have any vertices left with non-zero in-degrees, then we've found a cycle. - if cycles = degree.values.reject { |ns| ns == 0 } and cycles.length > 0 - report_cycles_in_graph - end - - result - end - # Add a new vertex to the graph. def add_vertex(vertex) @in_to[vertex] ||= {} @@ -368,52 +339,6 @@ class Puppet::SimpleGraph (options[:type] == :edges) ? ns.values.flatten : ns.keys end - # Take container information from another graph and use it - # to replace any container vertices with their respective leaves. - # This creates direct relationships where there were previously - # indirect relationships through the containers. - def splice!(other, type) - # We have to get the container list via a topological sort on the - # configuration graph, because otherwise containers that contain - # other containers will add those containers back into the - # graph. We could get a similar affect by only setting relationships - # to container leaves, but that would result in many more - # relationships. - stage_class = Puppet::Type.type(:stage) - whit_class = Puppet::Type.type(:whit) - containers = other.topsort.find_all { |v| (v.is_a?(type) or v.is_a?(stage_class)) and vertex?(v) } - containers.each do |container| - # Get the list of children from the other graph. - children = other.adjacent(container, :direction => :out) - - # MQR TODO: Luke suggests that it should be possible to refactor the system so that - # container nodes are retained, thus obviating the need for the whit. - children = [whit_class.new(:name => container.name, :catalog => other)] if children.empty? - - # First create new edges for each of the :in edges - [:in, :out].each do |dir| - edges = adjacent(container, :direction => dir, :type => :edges) - edges.each do |edge| - children.each do |child| - if dir == :in - s = edge.source - t = child - else - s = child - t = edge.target - end - - add_edge(s, t, edge.label) - end - - # Now get rid of the edge, so remove_vertex! works correctly. - remove_edge!(edge) - end - end - remove_vertex!(container) - end - end - # Just walk the tree and pass each edge. def walk(source, direction) # Use an iterative, breadth-first traversal of the graph. One could do @@ -453,6 +378,10 @@ class Puppet::SimpleGraph result end + def direct_dependents_of(v) + (@out_from[v] || {}).keys + end + def upstream_from_vertex(v) return @upstream_from[v] if @upstream_from[v] result = @upstream_from[v] = {} @@ -463,6 +392,36 @@ class Puppet::SimpleGraph result end + def direct_dependencies_of(v) + (@in_to[v] || {}).keys + end + + # Return an array of the edge-sets between a series of n+1 vertices (f=v0,v1,v2...t=vn) + # connecting the two given verticies. The ith edge set is an array containing all the + # edges between v(i) and v(i+1); these are (by definition) never empty. + # + # * if f == t, the list is empty + # * if they are adjacent the result is an array consisting of + # a single array (the edges from f to t) + # * and so on by induction on a vertex m between them + # * if there is no path from f to t, the result is nil + # + # This implementation is not particularly efficient; it's used in testing where clarity + # is more important than last-mile efficiency. + # + def path_between(f,t) + if f==t + [] + elsif direct_dependents_of(f).include?(t) + [edges_between(f,t)] + elsif dependents(f).include?(t) + m = (dependents(f) & direct_dependencies_of(t)).first + path_between(f,m) + path_between(m,t) + else + nil + end + end + # LAK:FIXME This is just a paste of the GRATR code with slight modifications. # Return a DOT::DOTDigraph for directed graphs or a DOT::DOTSubgraph for an diff --git a/lib/puppet/ssl/host.rb b/lib/puppet/ssl/host.rb index 7f71ced99..b9215effd 100644 --- a/lib/puppet/ssl/host.rb +++ b/lib/puppet/ssl/host.rb @@ -1,3 +1,4 @@ +require 'puppet/indirector' require 'puppet/ssl' require 'puppet/ssl/key' require 'puppet/ssl/certificate' @@ -15,11 +16,17 @@ class Puppet::SSL::Host CertificateRequest = Puppet::SSL::CertificateRequest CertificateRevocationList = Puppet::SSL::CertificateRevocationList + extend Puppet::Indirector + indirects :certificate_status, :terminus_class => :file + attr_reader :name attr_accessor :ca attr_writer :key, :certificate, :certificate_request + # This accessor is used in instances for indirector requests to hold desired state + attr_accessor :desired_state + class << self include Puppet::Util::Cacher @@ -47,6 +54,13 @@ class Puppet::SSL::Host CertificateRequest.indirection.terminus_class = terminus CertificateRevocationList.indirection.terminus_class = terminus + host_map = {:ca => :file, :file => nil, :rest => :rest} + if term = host_map[terminus] + self.indirection.terminus_class = term + else + self.indirection.reset_terminus_class + end + if cache # This is weird; we don't actually cache our keys, we # use what would otherwise be the cache as our normal @@ -85,30 +99,34 @@ class Puppet::SSL::Host # Specify how we expect to interact with our certificate authority. def self.ca_location=(mode) - raise ArgumentError, "CA Mode can only be #{CA_MODES.collect { |m| m.to_s }.join(", ")}" unless CA_MODES.include?(mode) + modes = CA_MODES.collect { |m, vals| m.to_s }.join(", ") + raise ArgumentError, "CA Mode can only be one of: #{modes}" unless CA_MODES.include?(mode) @ca_location = mode configure_indirection(*CA_MODES[@ca_location]) end - # Remove all traces of a given host + # Puppet::SSL::Host is actually indirected now so the original implementation + # has been moved into the certificate_status indirector. This method is in-use + # in `puppet cert -c <certname>`. def self.destroy(name) - [Key, Certificate, CertificateRequest].collect { |part| part.indirection.destroy(name) }.any? { |x| x } + indirection.destroy(name) end - # Search for more than one host, optionally only specifying - # an interest in hosts with a given file type. - # This just allows our non-indirected class to have one of - # indirection methods. - def self.search(options = {}) - classlist = [options[:for] || [Key, CertificateRequest, Certificate]].flatten - - # Collect the results from each class, flatten them, collect all of the names, make the name list unique, - # then create a Host instance for each one. - classlist.collect { |klass| klass.indirection.search }.flatten.collect { |r| r.name }.uniq.collect do |name| - new(name) + def self.from_pson(pson) + instance = new(pson["name"]) + if pson["desired_state"] + instance.desired_state = pson["desired_state"] end + instance + end + + # Puppet::SSL::Host is actually indirected now so the original implementation + # has been moved into the certificate_status indirector. This method does not + # appear to be in use in `puppet cert -l`. + def self.search(options = {}) + indirection.search("*", options) end # Is this a ca host, meaning that all of its files go in the CA location? @@ -221,6 +239,24 @@ class Puppet::SSL::Host @ssl_store end + def to_pson(*args) + my_cert = Puppet::SSL::Certificate.indirection.find(name) + pson_hash = { :name => name } + + my_state = state + + pson_hash[:state] = my_state + pson_hash[:desired_state] = desired_state if desired_state + + if my_state == 'requested' + pson_hash[:fingerprint] = certificate_request.fingerprint + else + pson_hash[:fingerprint] = my_cert.fingerprint + end + + pson_hash.to_pson(*args) + end + # Attempt to retrieve a cert, if we don't already have one. def wait_for_cert(time) begin @@ -257,6 +293,20 @@ class Puppet::SSL::Host end end end + + def state + my_cert = Puppet::SSL::Certificate.indirection.find(name) + if certificate_request + return 'requested' + end + + begin + Puppet::SSL::CertificateAuthority.new.verify(my_cert) + return 'signed' + rescue Puppet::SSL::CertificateAuthority::CertificateVerificationError + return 'revoked' + end + end end require 'puppet/ssl/certificate_authority' diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb index eba601cfe..0533273d9 100644 --- a/lib/puppet/transaction.rb +++ b/lib/puppet/transaction.rb @@ -4,6 +4,7 @@ require 'puppet' require 'puppet/util/tagging' require 'puppet/application' +require 'sha1' class Puppet::Transaction require 'puppet/transaction/event' @@ -12,7 +13,7 @@ class Puppet::Transaction require 'puppet/resource/status' attr_accessor :component, :catalog, :ignoreschedules - attr_accessor :sorted_resources, :configurator + attr_accessor :configurator # The report, once generated. attr_accessor :report @@ -47,7 +48,7 @@ class Puppet::Transaction def apply(resource, ancestor = nil) status = resource_harness.evaluate(resource) add_resource_status(status) - event_manager.queue_events(ancestor || resource, status.events) + event_manager.queue_events(ancestor || resource, status.events) unless status.failed? rescue => detail resource.err "Could not evaluate: #{detail}" end @@ -57,68 +58,36 @@ class Puppet::Transaction report.resource_statuses.values.find_all { |status| status.changed }.collect { |status| catalog.resource(status.resource) } end + # Find all of the applied resources (including failed attempts). + def applied_resources + report.resource_statuses.values.collect { |status| catalog.resource(status.resource) } + end + # Copy an important relationships from the parent to the newly-generated # child resource. - def make_parent_child_relationship(resource, children) - depthfirst = resource.depthfirst? - - children.each do |gen_child| - if depthfirst - edge = [gen_child, resource] - else - edge = [resource, gen_child] - end - relationship_graph.add_vertex(gen_child) - - unless relationship_graph.edge?(edge[1], edge[0]) - relationship_graph.add_edge(*edge) - else - resource.debug "Skipping automatic relationship to #{gen_child}" - end + def add_conditional_directed_dependency(parent, child, label=nil) + relationship_graph.add_vertex(child) + edge = parent.depthfirst? ? [child, parent] : [parent, child] + if relationship_graph.edge?(*edge.reverse) + parent.debug "Skipping automatic relationship to #{child}" + else + relationship_graph.add_edge(edge[0],edge[1],label) end end - # See if the resource generates new resources at evaluation time. - def eval_generate(resource) - generate_additional_resources(resource, :eval_generate) - end - # Evaluate a single resource. def eval_resource(resource, ancestor = nil) if skip?(resource) resource_status(resource).skipped = true else - eval_children_and_apply_resource(resource, ancestor) + resource_status(resource).scheduled = true + apply(resource, ancestor) end # Check to see if there are any events queued for this resource event_manager.process_events(resource) end - def eval_children_and_apply_resource(resource, ancestor = nil) - resource_status(resource).scheduled = true - - # We need to generate first regardless, because the recursive - # actions sometimes change how the top resource is applied. - children = eval_generate(resource) - - if ! children.empty? and resource.depthfirst? - children.each do |child| - # The child will never be skipped when the parent isn't - eval_resource(child, ancestor || resource) - end - end - - # Perform the actual changes - apply(resource, ancestor) - - if ! children.empty? and ! resource.depthfirst? - children.each do |child| - eval_resource(child, ancestor || resource) - end - end - end - # This method does all the actual work of running a transaction. It # collects all of the changes, executes them, and responds to any # necessary events. @@ -131,19 +100,13 @@ class Puppet::Transaction Puppet.info "Applying configuration version '#{catalog.version}'" if catalog.version begin - @sorted_resources.each do |resource| - next if stop_processing? + relationship_graph.traverse do |resource| if resource.is_a?(Puppet::Type::Component) Puppet.warning "Somehow left a component in the relationship graph" - next + else + seconds = thinmark { eval_resource(resource) } + resource.info "Evaluated in %0.2f seconds" % seconds if Puppet[:evaltrace] and @catalog.host_config? end - ret = nil - seconds = thinmark do - ret = eval_resource(resource) - end - - resource.info "Evaluated in %0.2f seconds" % seconds if Puppet[:evaltrace] and @catalog.host_config? - ret end ensure # And then close the transaction log. @@ -177,48 +140,66 @@ class Puppet::Transaction found_failed end + def eval_generate(resource) + raise Puppet::DevError,"Depthfirst resources are not supported by eval_generate" if resource.depthfirst? + begin + made = resource.eval_generate.uniq.reverse + rescue => detail + puts detail.backtrace if Puppet[:trace] + resource.err "Failed to generate additional resources using 'eval_generate: #{detail}" + return + end + made.each do |res| + begin + res.tag(*resource.tags) + @catalog.add_resource(res) + res.finish + rescue Puppet::Resource::Catalog::DuplicateResourceError + res.info "Duplicate generated resource; skipping" + end + end + sentinal = Puppet::Type::Whit.new(:name => "completed_#{resource.title}", :catalog => resource.catalog) + relationship_graph.adjacent(resource,:direction => :out,:type => :edges).each { |e| + add_conditional_directed_dependency(sentinal, e.target, e.label) + relationship_graph.remove_edge! e + } + default_label = Puppet::Resource::Catalog::Default_label + made.each do |res| + add_conditional_directed_dependency(made.find { |r| r != res && r.name == res.name[0,r.name.length]} || resource, res) + add_conditional_directed_dependency(res, sentinal, default_label) + end + add_conditional_directed_dependency(resource, sentinal, default_label) + end + # A general method for recursively generating new resources from a # resource. - def generate_additional_resources(resource, method) - return [] unless resource.respond_to?(method) + def generate_additional_resources(resource) + return unless resource.respond_to?(:generate) begin - made = resource.send(method) + made = resource.generate rescue => detail puts detail.backtrace if Puppet[:trace] - resource.err "Failed to generate additional resources using '#{method}': #{detail}" + resource.err "Failed to generate additional resources using 'generate': #{detail}" end - return [] unless made + return unless made made = [made] unless made.is_a?(Array) - made.uniq.find_all do |res| + made.uniq.each do |res| begin res.tag(*resource.tags) - @catalog.add_resource(res) do |r| - r.finish - make_parent_child_relationship(resource, [r]) - - # Call 'generate' recursively - generate_additional_resources(r, method) - end - true + @catalog.add_resource(res) + res.finish + add_conditional_directed_dependency(resource, res) + generate_additional_resources(res) rescue Puppet::Resource::Catalog::DuplicateResourceError res.info "Duplicate generated resource; skipping" - false end end end # Collect any dynamically generated resources. This method is called # before the transaction starts. - def generate - list = @catalog.vertices - newlist = [] - while ! list.empty? - list.each do |resource| - newlist += generate_additional_resources(resource, :generate) - end - list = newlist - newlist = [] - end + def xgenerate + @catalog.vertices.each { |resource| generate_additional_resources(resource) } end # Should we ignore tags? @@ -264,18 +245,75 @@ class Puppet::Transaction # Prepare to evaluate the resources in a transaction. def prepare # Now add any dynamically generated resources - generate + xgenerate # Then prefetch. It's important that we generate and then prefetch, # so that any generated resources also get prefetched. prefetch + end + - # This will throw an error if there are cycles in the graph. - @sorted_resources = relationship_graph.topsort + # We want to monitor changes in the relationship graph of our + # catalog but this is complicated by the fact that the catalog + # both is_a graph and has_a graph, by the fact that changes to + # the structure of the object can have adverse serialization + # effects, by threading issues, by order-of-initialization issues, + # etc. + # + # Since the proper lifetime/scope of the monitoring is a transaction + # and the transaction is already commiting a mild law-of-demeter + # transgression, we cut the Gordian knot here by simply wrapping the + # transaction's view of the resource graph to capture and maintain + # the information we need. Nothing outside the transaction needs + # this information, and nothing outside the transaction can see it + # except via the Transaction#relationship_graph + + class Relationship_graph_wrapper + attr_reader :real_graph,:transaction,:ready,:generated,:done,:unguessable_deterministic_key + def initialize(real_graph,transaction) + @real_graph = real_graph + @transaction = transaction + @ready = {} + @generated = {} + @done = {} + @unguessable_deterministic_key = Hash.new { |h,k| h[k] = Digest::SHA1.hexdigest("NaCl, MgSO4 (salts) and then #{k.title}") } + vertices.each { |v| check_if_now_ready(v) } + end + def method_missing(*args,&block) + real_graph.send(*args,&block) + end + def add_vertex(v) + real_graph.add_vertex(v) + check_if_now_ready(v) # ????????????????????????????????????????? + end + def add_edge(f,t,label=nil) + ready.delete(t) + real_graph.add_edge(f,t,label) + end + def check_if_now_ready(r) + ready[r] = true if direct_dependencies_of(r).all? { |r2| done[r2] } + end + def next_resource + ready.keys.sort_by { |r0| unguessable_deterministic_key[r0] }.first + end + def traverse(&block) + real_graph.report_cycles_in_graph + while (r = next_resource) && !transaction.stop_processing? + if !generated[r] && r.respond_to?(:eval_generate) + transaction.eval_generate(r) + generated[r] = true + else + ready.delete(r) + yield r + done[r] = true + direct_dependents_of(r).each { |v| check_if_now_ready(v) } + end + end + end end def relationship_graph - catalog.relationship_graph + @relationship_graph ||= Relationship_graph_wrapper.new(catalog.relationship_graph,self) end def add_resource_status(status) diff --git a/lib/puppet/transaction/event_manager.rb b/lib/puppet/transaction/event_manager.rb index 3ebb0a9d3..a21bbf892 100644 --- a/lib/puppet/transaction/event_manager.rb +++ b/lib/puppet/transaction/event_manager.rb @@ -31,7 +31,7 @@ class Puppet::Transaction::EventManager # Queue events for other resources to respond to. All of these events have # to be from the same resource. def queue_events(resource, events) - @events += events + #@events += events # Do some basic normalization so we're not doing so many # graph queries for large sets of events. @@ -47,12 +47,15 @@ class Puppet::Transaction::EventManager # Collect the targets of any subscriptions to those events. We pass # the parent resource in so it will override the source in the events, # since eval_generated children can't have direct relationships. + received = (event.name != :restarted) relationship_graph.matching_edges(event, resource).each do |edge| + received ||= true unless edge.target.is_a?(Puppet::Type::Whit) next unless method = edge.callback next unless edge.target.respond_to?(method) queue_events_for_resource(resource, edge.target, method, list) end + @events << event if received queue_events_for_resource(resource, resource, :refresh, [event]) if resource.self_refresh? and ! resource.deleting? end diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb index d24cc8554..5ecc430d4 100644 --- a/lib/puppet/type.rb +++ b/lib/puppet/type.rb @@ -598,14 +598,8 @@ class Type ############################### # Code related to the container behaviour. - # this is a retarded hack method to get around the difference between - # component children and file children - def self.depthfirst? - @depthfirst - end - def depthfirst? - self.class.depthfirst? + false end # Remove an object. The argument determines whether the object's diff --git a/lib/puppet/type/cron.rb b/lib/puppet/type/cron.rb index 4b18e71f9..4f6ea733c 100755 --- a/lib/puppet/type/cron.rb +++ b/lib/puppet/type/cron.rb @@ -226,7 +226,9 @@ Puppet::Type.newtype(:cron) do end newproperty(:special) do - desc "Special schedules" + desc "A special value such as 'reboot' or 'annually'. + Only available on supported systems such as Vixie Cron. + Overrides more specific time of day/week settings." def specials %w{reboot yearly annually monthly weekly daily midnight hourly} diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index 1a6d0c3ac..715836b22 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -122,7 +122,18 @@ Puppet::Type.newtype(:file) do newparam(:recurse) do desc "Whether and how deeply to do recursive - management." + management. Options are: + + * `inf,true` --- Regular style recursion on both remote and local + directory structure. + * `remote` --- Descends recursively into the remote directory + but not the local directory. Allows copying of + a few files into a directory containing many + unmanaged files without scanning all the local files. + * `false` --- Default of no recursion. + * `[0-9]+` --- Same as true, but limit recursion. Warning: this syntax + has been deprecated in favor of the `recurselimit` attribute. + " newvalues(:true, :false, :inf, :remote, /^[0-9]+$/) @@ -303,8 +314,6 @@ Puppet::Type.newtype(:file) do return self.new(:name => base, :recurse => true, :recurselimit => 1, :audit => :all).recurse_local.values end - @depthfirst = false - # Determine the user to write files as. def asuser if self.should(:owner) and ! self.should(:owner).is_a?(Symbol) @@ -464,8 +473,7 @@ Puppet::Type.newtype(:file) do # be used to copy remote files, manage local files, and/or make links # to map to another directory. def recurse - children = {} - children = recurse_local if self[:recurse] != :remote + children = (self[:recurse] == :remote) ? {} : recurse_local if self[:target] recurse_link(children) @@ -502,11 +510,7 @@ Puppet::Type.newtype(:file) do # A simple method for determining whether we should be recursing. def recurse? - return false unless @parameters.include?(:recurse) - - val = @parameters[:recurse].value - - !!(val and (val == true or val == :remote)) + self[:recurse] == true or self[:recurse] == :remote end # Recurse the target of the link. @@ -578,13 +582,10 @@ Puppet::Type.newtype(:file) do end def perform_recursion(path) - Puppet::FileServing::Metadata.indirection.search( - path, :links => self[:links], :recurse => (self[:recurse] == :remote ? true : self[:recurse]), - :recurselimit => self[:recurselimit], :ignore => self[:ignore], :checksum_type => (self[:source] || self[:content]) ? self[:checksum] : :none diff --git a/lib/puppet/type/group.rb b/lib/puppet/type/group.rb index aa96bd9c3..2573633f9 100755 --- a/lib/puppet/type/group.rb +++ b/lib/puppet/type/group.rb @@ -1,6 +1,7 @@ require 'etc' require 'facter' +require 'puppet/property/keyvalue' module Puppet newtype(:group) do diff --git a/lib/puppet/type/tidy.rb b/lib/puppet/type/tidy.rb index 146481fed..1a308e17d 100755 --- a/lib/puppet/type/tidy.rb +++ b/lib/puppet/type/tidy.rb @@ -209,7 +209,9 @@ Puppet::Type.newtype(:tidy) do [] end - @depthfirst = true + def depthfirst? + true + end def initialize(hash) super @@ -238,10 +240,6 @@ Puppet::Type.newtype(:tidy) do [] end - def eval_generate - [] - end - def generate return [] unless stat(self[:path]) diff --git a/lib/puppet/type/whit.rb b/lib/puppet/type/whit.rb index 55bfcfb46..55ed0386e 100644 --- a/lib/puppet/type/whit.rb +++ b/lib/puppet/type/whit.rb @@ -6,6 +6,12 @@ Puppet::Type.newtype(:whit) do end def to_s - "Class[#{name}]" + "(#{name})" + end + + def refresh + # We don't do anything with them, but we need this to + # show that we are "refresh aware" and not break the + # chain of propogation. end end diff --git a/lib/puppet/util/command_line.rb b/lib/puppet/util/command_line.rb index 7f74d266a..52b5f81ef 100644 --- a/lib/puppet/util/command_line.rb +++ b/lib/puppet/util/command_line.rb @@ -1,9 +1,10 @@ +require "puppet/util/plugins" + module Puppet module Util class CommandLine - LegacyName = Hash.new{|h,k| k}.update( - { + LegacyName = Hash.new{|h,k| k}.update( 'agent' => 'puppetd', 'cert' => 'puppetca', 'doc' => 'puppetdoc', @@ -13,9 +14,8 @@ module Puppet 'queue' => 'puppetqd', 'resource' => 'ralsh', 'kick' => 'puppetrun', - 'master' => 'puppetmasterd', - - }) + 'master' => 'puppetmasterd' + ) def initialize( zero = $0, argv = ARGV, stdin = STDIN ) @zero = zero @@ -23,6 +23,7 @@ module Puppet @stdin = stdin @subcommand_name, @args = subcommand_and_args( @zero, @argv, @stdin ) + Puppet::Plugins.on_commandline_initialization(:command_line_object => self) end attr :subcommand_name @@ -56,21 +57,23 @@ module Puppet puts usage_message elsif available_subcommands.include?(subcommand_name) #subcommand require_application subcommand_name - Puppet::Application.find(subcommand_name).new(self).run + app = Puppet::Application.find(subcommand_name).new(self) + Puppet::Plugins.on_application_initialization(:appliation_object => self) + app.run else abort "Error: Unknown command #{subcommand_name}.\n#{usage_message}" unless execute_external_subcommand end end def execute_external_subcommand - external_command = "puppet-#{subcommand_name}" + external_command = "puppet-#{subcommand_name}" - require 'puppet/util' - path_to_subcommand = Puppet::Util.which( external_command ) - return false unless path_to_subcommand + require 'puppet/util' + path_to_subcommand = Puppet::Util.which( external_command ) + return false unless path_to_subcommand - system( path_to_subcommand, *args ) - true + system( path_to_subcommand, *args ) + true end def legacy_executable_name diff --git a/lib/puppet/util/plugins.rb b/lib/puppet/util/plugins.rb new file mode 100644 index 000000000..105fdcd75 --- /dev/null +++ b/lib/puppet/util/plugins.rb @@ -0,0 +1,82 @@ +# +# This system manages an extensible set of metadata about plugins which it +# collects by searching for files named "plugin_init.rb" in a series of +# directories. Initially, these are simply the $LOAD_PATH. +# +# The contents of each file found is executed in the context of a Puppet::Plugins +# object (and thus scoped). An example file might contain: +# +# ------------------------------------------------------- +# @name = "Greet the CA" +# +# @description = %q{ +# This plugin causes a friendly greeting to print out on a master +# that is operating as the CA, after it has been set up but before +# it does anything. +# } +# +# def after_application_setup(options) +# if options[:application_object].is_a?(Puppet::Application::Master) && Puppet::SSL::CertificateAuthority.ca? +# puts "Hey, this is the CA!" +# end +# end +# ------------------------------------------------------- +# +# Note that the instance variables are local to this Puppet::Plugin (and so may be used +# for maintaining state, etc.) but the plugin system does not provide any thread safety +# assurances, so they may not be adequate for some complex use cases. +# +# +module Puppet + class Plugins + Paths = [] # Where we might find plugin initialization code + Loaded = [] # Code we have found (one-to-one with paths once searched) + # + # Return all the Puppet::Plugins we know about, searching any new paths + # + def self.known + Paths[Loaded.length...Paths.length].each { |path| + file = File.join(path,'plugin_init.rb') + Loaded << (File.exist?(file) && new(file)) + } + Loaded.compact + end + # + # Add more places to look for plugins without adding duplicates or changing the + # order of ones we've already found. + # + def self.look_in(*paths) + Paths.replace Paths | paths.flatten.collect { |path| File.expand_path(path) } + end + # + # Initially just look in $LOAD_PATH + # + look_in $LOAD_PATH + # + # Calling methods (hooks) on the class calls the method of the same name on + # all plugins that use that hook, passing in the same arguments to each + # and returning an array containing the results returned by each plugin as + # an array of [plugin_name,result] pairs. + # + def self.method_missing(hook,*args,&block) + known. + select { |p| p.respond_to? hook }. + collect { |p| [p.name,p.send(hook,*args,&block)] } + end + # + # + # + attr_reader :path,:name + def initialize(path) + @name = @path = path + class << self + private + def define_hooks + eval File.read(path),nil,path,1 + end + end + define_hooks + end + end +end + diff --git a/spec/integration/indirector/catalog/compiler_spec.rb b/spec/integration/indirector/catalog/compiler_spec.rb index 1146c20b0..dafa1af7c 100755 --- a/spec/integration/indirector/catalog/compiler_spec.rb +++ b/spec/integration/indirector/catalog/compiler_spec.rb @@ -10,11 +10,8 @@ describe Puppet::Resource::Catalog::Compiler do before do Facter.stubs(:value).returns "something" @catalog = Puppet::Resource::Catalog.new - - @one = Puppet::Resource.new(:file, "/one") - - @two = Puppet::Resource.new(:file, "/two") - @catalog.add_resource(@one, @two) + @catalog.add_resource(@one = Puppet::Resource.new(:file, "/one")) + @catalog.add_resource(@two = Puppet::Resource.new(:file, "/two")) end after { Puppet.settings.clear } diff --git a/spec/integration/transaction_spec.rb b/spec/integration/transaction_spec.rb index ff15e597d..e608faa27 100755 --- a/spec/integration/transaction_spec.rb +++ b/spec/integration/transaction_spec.rb @@ -135,33 +135,26 @@ describe Puppet::Transaction do it "should not let one failed refresh result in other refreshes failing" do path = tmpfile("path") newfile = tmpfile("file") - - file = Puppet::Type.type(:file).new( - + file = Puppet::Type.type(:file).new( :path => path, - :ensure => "file" ) - exec1 = Puppet::Type.type(:exec).new( - + exec1 = Puppet::Type.type(:exec).new( :path => ENV["PATH"], :command => "touch /this/cannot/possibly/exist", :logoutput => true, :refreshonly => true, :subscribe => file, - :title => "one" ) - exec2 = Puppet::Type.type(:exec).new( - + exec2 = Puppet::Type.type(:exec).new( :path => ENV["PATH"], :command => "touch #{newfile}", :logoutput => true, :refreshonly => true, :subscribe => [file, exec1], - :title => "two" ) @@ -178,22 +171,18 @@ describe Puppet::Transaction do Puppet[:ignoreschedules] = false - file = Puppet::Type.type(:file).new( - + file = Puppet::Type.type(:file).new( :name => tmpfile("file"), - :ensure => "file", :backup => false ) fname = tmpfile("exec") - exec = Puppet::Type.type(:exec).new( - + exec = Puppet::Type.type(:exec).new( :name => "touch #{fname}", :path => "/usr/bin:/bin", :schedule => "monthly", - :subscribe => Puppet::Resource.new("file", file.name) ) @@ -230,29 +219,21 @@ describe Puppet::Transaction do it "should not attempt to evaluate resources with failed dependencies" do - exec = Puppet::Type.type(:exec).new( - + exec = Puppet::Type.type(:exec).new( :command => "/bin/mkdir /this/path/cannot/possibly/exit", - :title => "mkdir" ) - - file1 = Puppet::Type.type(:file).new( - + file1 = Puppet::Type.type(:file).new( :title => "file1", :path => tmpfile("file1"), - :require => exec, :ensure => :file ) - - file2 = Puppet::Type.type(:file).new( - + file2 = Puppet::Type.type(:file).new( :title => "file2", :path => tmpfile("file2"), - :require => file1, :ensure => :file ) @@ -264,6 +245,32 @@ describe Puppet::Transaction do FileTest.should_not be_exists(file2[:path]) end + it "should not trigger subscribing resources on failure" do + file1 = tmpfile("file1") + file2 = tmpfile("file2") + + create_file1 = Puppet::Type.type(:exec).new( + :command => "/usr/bin/touch #{file1}" + ) + + exec = Puppet::Type.type(:exec).new( + :command => "/bin/mkdir /this/path/cannot/possibly/exit", + :title => "mkdir", + :notify => create_file1 + ) + + create_file2 = Puppet::Type.type(:exec).new( + :command => "/usr/bin/touch #{file2}", + :subscribe => exec + ) + + catalog = mk_catalog(exec, create_file1, create_file2) + catalog.apply + + FileTest.should_not be_exists(file1) + FileTest.should_not be_exists(file2) + end + # #801 -- resources only checked in noop should be rescheduled immediately. it "should immediately reschedule noop resources" do Puppet::Type.type(:schedule).mkdefaultschedules diff --git a/spec/integration/type/file_spec.rb b/spec/integration/type/file_spec.rb index 46900a0f1..513b96e41 100755 --- a/spec/integration/type/file_spec.rb +++ b/spec/integration/type/file_spec.rb @@ -28,7 +28,8 @@ describe Puppet::Type.type(:file) do bucket = Puppet::Type.type(:filebucket).new :path => tmpfile("filebucket"), :name => "mybucket" file = Puppet::Type.type(:file).new :path => tmpfile("bucket_backs"), :backup => "mybucket", :content => "foo" catalog = Puppet::Resource::Catalog.new - catalog.add_resource file, bucket + catalog.add_resource file + catalog.add_resource bucket File.open(file[:path], "w") { |f| f.puts "bar" } @@ -80,7 +81,8 @@ describe Puppet::Type.type(:file) do bucket = Puppet::Type.type(:filebucket).new :path => tmpfile("filebucket"), :name => "mybucket" file = Puppet::Type.type(:file).new :path => link, :target => dest2, :ensure => :link, :backup => "mybucket" catalog = Puppet::Resource::Catalog.new - catalog.add_resource file, bucket + catalog.add_resource file + catalog.add_resource bucket File.open(dest1, "w") { |f| f.puts "whatever" } File.symlink(dest1, link) @@ -113,7 +115,8 @@ describe Puppet::Type.type(:file) do bucket = Puppet::Type.type(:filebucket).new :path => tmpfile("filebucket"), :name => "mybucket" file = Puppet::Type.type(:file).new :path => tmpfile("bucket_backs"), :backup => "mybucket", :content => "foo", :force => true catalog = Puppet::Resource::Catalog.new - catalog.add_resource file, bucket + catalog.add_resource file + catalog.add_resource bucket Dir.mkdir(file[:path]) foofile = File.join(file[:path], "foo") @@ -337,10 +340,10 @@ describe Puppet::Type.type(:file) do it "should have an edge to each resource in the relationship graph" do @catalog.apply do |trans| one = @catalog.resource(:file, File.join(@dest, "one")) - @catalog.relationship_graph.should be_edge(@file, one) + @catalog.relationship_graph.edge?(@file, one).should be two = @catalog.resource(:file, File.join(@dest, "two")) - @catalog.relationship_graph.should be_edge(@file, two) + @catalog.relationship_graph.edge?(@file, two).should be end end end diff --git a/spec/unit/configurer_spec.rb b/spec/unit/configurer_spec.rb index f8acdd002..d21d86ecf 100755 --- a/spec/unit/configurer_spec.rb +++ b/spec/unit/configurer_spec.rb @@ -226,10 +226,12 @@ describe Puppet::Configurer, "when executing a catalog run" do end describe Puppet::Configurer, "when sending a report" do + include PuppetSpec::Files + before do Puppet.settings.stubs(:use).returns(true) @configurer = Puppet::Configurer.new - @configurer.stubs(:save_last_run_summary) + Puppet[:lastrunfile] = tmpfile('last_run_file') @report = Puppet::Transaction::Report.new("apply") @trans = stub 'transaction' @@ -277,10 +279,10 @@ describe Puppet::Configurer, "when sending a report" do @configurer.send_report(@report, nil) end - it "should not save the last run summary if reporting is disabled" do + it "should save the last run summary if reporting is disabled" do Puppet.settings[:report] = false - @configurer.expects(:save_last_run_summary).never + @configurer.expects(:save_last_run_summary).with(@report) @configurer.send_report(@report, nil) end diff --git a/spec/unit/indirector/certificate_status/file_spec.rb b/spec/unit/indirector/certificate_status/file_spec.rb new file mode 100644 index 000000000..6cc0bb547 --- /dev/null +++ b/spec/unit/indirector/certificate_status/file_spec.rb @@ -0,0 +1,188 @@ +#!/usr/bin/env ruby + +require File.expand_path(File.dirname(__FILE__) + '/../../../spec_helper.rb') +require 'puppet/ssl/host' +require 'puppet/indirector/certificate_status' +require 'tempfile' + +describe "Puppet::Indirector::CertificateStatus::File" do + include PuppetSpec::Files + + before do + Puppet::SSL::CertificateAuthority.stubs(:ca?).returns true + @terminus = Puppet::SSL::Host.indirection.terminus(:file) + + @tmpdir = tmpdir("certificate_status_ca_testing") + Puppet[:confdir] = @tmpdir + Puppet[:vardir] = @tmpdir + + # localcacert is where each client stores the CA certificate + # cacert is where the master stores the CA certificate + # Since we need to play the role of both for testing we need them to be the same and exist + Puppet[:cacert] = Puppet[:localcacert] + end + + def generate_csr(host) + host.generate_key + csr = Puppet::SSL::CertificateRequest.new(host.name) + csr.generate(host.key.content) + Puppet::SSL::CertificateRequest.indirection.save(csr) + end + + def sign_csr(host) + host.desired_state = "signed" + @terminus.save(Puppet::Indirector::Request.new(:certificate_status, :save, host.name, host)) + end + + def generate_signed_cert(host) + generate_csr(host) + sign_csr(host) + + @terminus.find(Puppet::Indirector::Request.new(:certificate_status, :find, host.name, host)) + end + + def generate_revoked_cert(host) + generate_signed_cert(host) + + host.desired_state = "revoked" + + @terminus.save(Puppet::Indirector::Request.new(:certificate_status, :save, host.name, host)) + end + + it "should be a terminus on SSL::Host" do + @terminus.should be_instance_of(Puppet::Indirector::CertificateStatus::File) + end + + it "should create a CA instance if none is present" do + @terminus.ca.should be_instance_of(Puppet::SSL::CertificateAuthority) + end + + describe "when creating the CA" do + it "should fail if it is not a valid CA" do + Puppet::SSL::CertificateAuthority.expects(:ca?).returns false + lambda { @terminus.ca }.should raise_error(ArgumentError, "This process is not configured as a certificate authority") + end + end + + it "should be indirected with the name 'certificate_status'" do + Puppet::SSL::Host.indirection.name.should == :certificate_status + end + + describe "when finding" do + before do + @host = Puppet::SSL::Host.new("foo") + Puppet.settings.use(:main) + end + + it "should return the Puppet::SSL::Host when a CSR exists for the host" do + generate_csr(@host) + request = Puppet::Indirector::Request.new(:certificate_status, :find, "foo", @host) + + retrieved_host = @terminus.find(request) + + retrieved_host.name.should == @host.name + retrieved_host.certificate_request.content.to_s.chomp.should == @host.certificate_request.content.to_s.chomp + end + + it "should return the Puppet::SSL::Host when a public key exist for the host" do + generate_signed_cert(@host) + request = Puppet::Indirector::Request.new(:certificate_status, :find, "foo", @host) + + retrieved_host = @terminus.find(request) + + retrieved_host.name.should == @host.name + retrieved_host.certificate.content.to_s.chomp.should == @host.certificate.content.to_s.chomp + end + + it "should return nil when neither a CSR nor public key exist for the host" do + request = Puppet::Indirector::Request.new(:certificate_status, :find, "foo", @host) + @terminus.find(request).should == nil + end + end + + describe "when saving" do + before do + @host = Puppet::SSL::Host.new("foobar") + Puppet.settings.use(:main) + end + + describe "when signing a cert" do + before do + @host.desired_state = "signed" + @request = Puppet::Indirector::Request.new(:certificate_status, :save, "foobar", @host) + end + + it "should fail if no CSR is on disk" do + lambda { @terminus.save(@request) }.should raise_error(Puppet::Error, /certificate request/) + end + + it "should sign the on-disk CSR when it is present" do + signed_host = generate_signed_cert(@host) + + signed_host.state.should == "signed" + Puppet::SSL::Certificate.indirection.find("foobar").should be_instance_of(Puppet::SSL::Certificate) + end + end + + describe "when revoking a cert" do + before do + @request = Puppet::Indirector::Request.new(:certificate_status, :save, "foobar", @host) + end + + it "should fail if no certificate is on disk" do + @host.desired_state = "revoked" + lambda { @terminus.save(@request) }.should raise_error(Puppet::Error, /Cannot revoke/) + end + + it "should revoke the certificate when it is present" do + generate_revoked_cert(@host) + + @host.state.should == 'revoked' + end + end + end + + describe "when deleting" do + before do + Puppet.settings.use(:main) + end + + it "should not delete anything if no certificate, request, or key is on disk" do + host = Puppet::SSL::Host.new("clean_me") + request = Puppet::Indirector::Request.new(:certificate_status, :delete, "clean_me", host) + @terminus.destroy(request).should == "Nothing was deleted" + end + + it "should clean certs, cert requests, keys" do + signed_host = Puppet::SSL::Host.new("clean_signed_cert") + generate_signed_cert(signed_host) + signed_request = Puppet::Indirector::Request.new(:certificate_status, :delete, "clean_signed_cert", signed_host) + @terminus.destroy(signed_request).should == "Deleted for clean_signed_cert: Puppet::SSL::Certificate, Puppet::SSL::Key" + + requested_host = Puppet::SSL::Host.new("clean_csr") + generate_csr(requested_host) + csr_request = Puppet::Indirector::Request.new(:certificate_status, :delete, "clean_csr", requested_host) + @terminus.destroy(csr_request).should == "Deleted for clean_csr: Puppet::SSL::CertificateRequest, Puppet::SSL::Key" + end + end + + describe "when searching" do + it "should return a list of all hosts with certificate requests, signed certs, or revoked certs" do + Puppet.settings.use(:main) + + signed_host = Puppet::SSL::Host.new("signed_host") + generate_signed_cert(signed_host) + + requested_host = Puppet::SSL::Host.new("requested_host") + generate_csr(requested_host) + + revoked_host = Puppet::SSL::Host.new("revoked_host") + generate_revoked_cert(revoked_host) + + retrieved_hosts = @terminus.search(Puppet::Indirector::Request.new(:certificate_status, :search, "all", signed_host)) + + results = retrieved_hosts.map {|h| [h.name, h.state]}.sort{ |h,i| h[0] <=> i[0] } + results.should == [["ca","signed"],["requested_host","requested"],["revoked_host","revoked"],["signed_host","signed"]] + end + end +end diff --git a/spec/unit/indirector/certificate_status/rest_spec.rb b/spec/unit/indirector/certificate_status/rest_spec.rb new file mode 100644 index 000000000..f44eac671 --- /dev/null +++ b/spec/unit/indirector/certificate_status/rest_spec.rb @@ -0,0 +1,15 @@ +#!/usr/bin/env ruby + +require File.expand_path(File.dirname(__FILE__) + '/../../../spec_helper.rb') +require 'puppet/ssl/host' +require 'puppet/indirector/certificate_status' + +describe "Puppet::CertificateStatus::Rest" do + before do + @terminus = Puppet::SSL::Host.indirection.terminus(:rest) + end + + it "should be a terminus on Puppet::SSL::Host" do + @terminus.should be_instance_of(Puppet::Indirector::CertificateStatus::Rest) + end +end diff --git a/spec/unit/node/environment_spec.rb b/spec/unit/node/environment_spec.rb index 05527e70f..d34bdb000 100755 --- a/spec/unit/node/environment_spec.rb +++ b/spec/unit/node/environment_spec.rb @@ -85,28 +85,29 @@ describe Puppet::Node::Environment do @env.known_resource_types.should equal(@collection) end - it "should give to all threads the same collection if it didn't change" do - Puppet::Resource::TypeCollection.expects(:new).with(@env).returns @collection - @env.known_resource_types + it "should give to all threads using the same environment the same collection if the collection isn't stale" do + original_thread_type_collection = Puppet::Resource::TypeCollection.new(@env) + Puppet::Resource::TypeCollection.expects(:new).with(@env).returns original_thread_type_collection + @env.known_resource_types.should equal(original_thread_type_collection) + + original_thread_type_collection.expects(:require_reparse?).returns(false) + Puppet::Resource::TypeCollection.stubs(:new).with(@env).returns @collection t = Thread.new { - @env.known_resource_types.should equal(@collection) + @env.known_resource_types.should equal(original_thread_type_collection) } t.join end - it "should give to new threads a new collection if it isn't stale" do - Puppet::Resource::TypeCollection.expects(:new).with(@env).returns @collection - @env.known_resource_types.expects(:stale?).returns(true) - - Puppet::Resource::TypeCollection.expects(:new).returns @collection + it "should generate a new TypeCollection if the current one requires reparsing" do + old_type_collection = @env.known_resource_types + old_type_collection.stubs(:require_reparse?).returns true + Thread.current[:known_resource_types] = nil + new_type_collection = @env.known_resource_types - t = Thread.new { - @env.known_resource_types.should equal(@collection) - } - t.join + new_type_collection.should be_a Puppet::Resource::TypeCollection + new_type_collection.should_not equal(old_type_collection) end - end [:modulepath, :manifestdir].each do |setting| @@ -277,7 +278,7 @@ describe Puppet::Node::Environment do describe "when performing initial import" do before do - @parser = stub 'parser' + @parser = Puppet::Parser::Parser.new("test") Puppet::Parser::Parser.stubs(:new).returns @parser @env = Puppet::Node::Environment.new("env") end @@ -309,6 +310,7 @@ describe Puppet::Node::Environment do it "should fail helpfully if there is an error importing" do File.stubs(:exist?).returns true + @env.stubs(:known_resource_types).returns Puppet::Resource::TypeCollection.new(@env) @parser.expects(:file=).once @parser.expects(:parse).raises ArgumentError lambda { @env.instance_eval { perform_initial_import } }.should raise_error(Puppet::Error) @@ -321,5 +323,13 @@ describe Puppet::Node::Environment do @parser.expects(:parse).never @env.instance_eval { perform_initial_import } end + + it "should mark the type collection as needing a reparse when there is an error parsing" do + @parser.expects(:parse).raises Puppet::ParseError.new("Syntax error at ...") + @env.stubs(:known_resource_types).returns Puppet::Resource::TypeCollection.new(@env) + + lambda { @env.instance_eval { perform_initial_import } }.should raise_error(Puppet::Error, /Syntax error at .../) + @env.known_resource_types.require_reparse?.should be_true + end end end diff --git a/spec/unit/parser/functions/create_resources_spec.rb b/spec/unit/parser/functions/create_resources_spec.rb index d4095b777..366fb536c 100755 --- a/spec/unit/parser/functions/create_resources_spec.rb +++ b/spec/unit/parser/functions/create_resources_spec.rb @@ -51,11 +51,12 @@ describe 'function for dynamically creating resources' do it 'should be able to add edges' do @scope.function_create_resources(['notify', {'foo'=>{'require' => 'Notify[test]'}}]) @scope.compiler.compile - edge = @scope.compiler.catalog.to_ral.relationship_graph.edges.detect do |edge| - edge.source.title == 'test' - end - edge.source.title.should == 'test' - edge.target.title.should == 'foo' + rg = @scope.compiler.catalog.to_ral.relationship_graph + test = rg.vertices.find { |v| v.title == 'test' } + foo = rg.vertices.find { |v| v.title == 'foo' } + test.should be + foo.should be + rg.path_between(test,foo).should be end end describe 'when dynamically creating resource types' do @@ -93,11 +94,13 @@ notify{test:} it 'should be able to add edges' do @scope.function_create_resources(['foo', {'blah'=>{'one'=>'two', 'require' => 'Notify[test]'}}]) @scope.compiler.compile - edge = @scope.compiler.catalog.to_ral.relationship_graph.edges.detect do |edge| - edge.source.title == 'test' - end - edge.source.title.should == 'test' - edge.target.title.should == 'blah' + rg = @scope.compiler.catalog.to_ral.relationship_graph + test = rg.vertices.find { |v| v.title == 'test' } + blah = rg.vertices.find { |v| v.title == 'blah' } + test.should be + blah.should be + # (Yoda speak like we do) + rg.path_between(test,blah).should be @compiler.catalog.resource(:notify, "blah")['message'].should == 'two' end end @@ -123,13 +126,12 @@ notify{tester:} it 'should be able to add edges' do @scope.function_create_resources(['class', {'bar'=>{'one'=>'two', 'require' => 'Notify[tester]'}}]) @scope.compiler.compile - edge = @scope.compiler.catalog.to_ral.relationship_graph.edges.detect do |e| - e.source.title == 'tester' - end - edge.source.title.should == 'tester' - edge.target.title.should == 'test' - #@compiler.catalog.resource(:notify, "blah")['message'].should == 'two' + rg = @scope.compiler.catalog.to_ral.relationship_graph + test = rg.vertices.find { |v| v.title == 'test' } + tester = rg.vertices.find { |v| v.title == 'tester' } + test.should be + tester.should be + rg.path_between(tester,test).should be end - end end diff --git a/spec/unit/provider/package/gem_spec.rb b/spec/unit/provider/package/gem_spec.rb index 7c0d34d00..c2bd3cc90 100644 --- a/spec/unit/provider/package/gem_spec.rb +++ b/spec/unit/provider/package/gem_spec.rb @@ -42,8 +42,18 @@ describe provider_class do @provider.install end + it "should specify that documentation should not be included" do + @provider.expects(:execute).with { |args| args[3] == "--no-rdoc" }.returns "" + @provider.install + end + + it "should specify that RI should not be included" do + @provider.expects(:execute).with { |args| args[4] == "--no-ri" }.returns "" + @provider.install + end + it "should specify the package name" do - @provider.expects(:execute).with { |args| args[3] == "myresource" }.returns "" + @provider.expects(:execute).with { |args| args[5] == "myresource" }.returns "" @provider.install end diff --git a/spec/unit/resource/catalog_spec.rb b/spec/unit/resource/catalog_spec.rb index 42850c23b..78d1b3223 100755 --- a/spec/unit/resource/catalog_spec.rb +++ b/spec/unit/resource/catalog_spec.rb @@ -398,12 +398,6 @@ describe Puppet::Resource::Catalog, "when compiling" do relgraph.should be_vertex(@one) end - it "should yield added resources if a block is provided" do - yielded = [] - @catalog.add_resource(@one, @two) { |r| yielded << r } - yielded.length.should == 2 - end - it "should set itself as the resource's catalog if it is not a relationship graph" do @one.expects(:catalog=).with(@catalog) @catalog.add_resource @one @@ -740,7 +734,7 @@ describe Puppet::Resource::Catalog, "when compiling" do end it "should copy component relationships to all contained resources" do - @relationships.edge?(@one, @two).should be_true + @relationships.path_between(@one, @two).should be end it "should add automatic relationships to the relationship graph" do diff --git a/spec/unit/simple_graph_spec.rb b/spec/unit/simple_graph_spec.rb index c106f550b..99db2a55c 100755 --- a/spec/unit/simple_graph_spec.rb +++ b/spec/unit/simple_graph_spec.rb @@ -267,7 +267,7 @@ describe Puppet::SimpleGraph do end end - describe "when sorting the graph" do + describe "when reporting cycles in the graph" do before do @graph = Puppet::SimpleGraph.new end @@ -278,29 +278,24 @@ describe Puppet::SimpleGraph do end end - it "should sort the graph topologically" do - add_edges :a => :b, :b => :c - @graph.topsort.should == [:a, :b, :c] - end - it "should fail on two-vertex loops" do add_edges :a => :b, :b => :a - proc { @graph.topsort }.should raise_error(Puppet::Error) + proc { @graph.report_cycles_in_graph }.should raise_error(Puppet::Error) end it "should fail on multi-vertex loops" do add_edges :a => :b, :b => :c, :c => :a - proc { @graph.topsort }.should raise_error(Puppet::Error) + proc { @graph.report_cycles_in_graph }.should raise_error(Puppet::Error) end it "should fail when a larger tree contains a small cycle" do add_edges :a => :b, :b => :a, :c => :a, :d => :c - proc { @graph.topsort }.should raise_error(Puppet::Error) + proc { @graph.report_cycles_in_graph }.should raise_error(Puppet::Error) end it "should succeed on trees with no cycles" do add_edges :a => :b, :b => :e, :c => :a, :d => :c - proc { @graph.topsort }.should_not raise_error + proc { @graph.report_cycles_in_graph }.should_not raise_error end it "should produce the correct relationship text" do @@ -308,7 +303,7 @@ describe Puppet::SimpleGraph do # cycle detection starts from a or b randomly # so we need to check for either ordering in the error message want = %r{Found 1 dependency cycle:\n\((a => b => a|b => a => b)\)\nTry} - expect { @graph.topsort }.to raise_error(Puppet::Error, want) + expect { @graph.report_cycles_in_graph }.to raise_error(Puppet::Error, want) end it "cycle discovery should be the minimum cycle for a simple graph" do @@ -405,17 +400,6 @@ describe Puppet::SimpleGraph do paths = @graph.paths_in_cycle(cycles.first, 21) paths.length.should be == 20 end - - # Our graph's add_edge method is smart enough not to add - # duplicate edges, so we use the objects, which it doesn't - # check. - it "should be able to sort graphs with duplicate edges" do - one = Puppet::Relationship.new(:a, :b) - @graph.add_edge(one) - two = Puppet::Relationship.new(:a, :b) - @graph.add_edge(two) - proc { @graph.topsort }.should_not raise_error - end end describe "when writing dot files" do @@ -521,7 +505,7 @@ describe Puppet::SimpleGraph do require 'puppet/util/graph' - class Container + class Container < Puppet::Type::Component include Puppet::Util::Graph include Enumerable attr_accessor :name @@ -543,6 +527,7 @@ describe Puppet::SimpleGraph do end end + require "puppet/resource/catalog" describe "when splicing the graph" do def container_graph @one = Container.new("one", %w{a b}) @@ -555,13 +540,21 @@ describe Puppet::SimpleGraph do @whit = Puppet::Type.type(:whit) @stage = Puppet::Type.type(:stage).new(:name => "foo") - @contgraph = @top.to_graph + @contgraph = @top.to_graph(Puppet::Resource::Catalog.new) # We have to add the container to the main graph, else it won't # be spliced in the dependency graph. @contgraph.add_vertex(@empty) end + def containers + @contgraph.vertices.select { |x| !x.is_a? String } + end + + def contents_of(x) + @contgraph.direct_dependents_of(x) + end + def dependency_graph @depgraph = Puppet::SimpleGraph.new @contgraph.vertices.each do |v| @@ -570,13 +563,34 @@ describe Puppet::SimpleGraph do # We have to specify a relationship to our empty container, else it # never makes it into the dep graph in the first place. - {@one => @two, "f" => "c", "h" => @middle, "c" => @empty}.each do |source, target| + @explicit_dependencies = {@one => @two, "f" => "c", "h" => @middle, "c" => @empty} + @explicit_dependencies.each do |source, target| @depgraph.add_edge(source, target, :callback => :refresh) end end def splice - @depgraph.splice!(@contgraph, Container) + @contgraph.splice!(@depgraph) + end + + def whit_called(name) + x = @depgraph.vertices.find { |v| v.is_a?(@whit) && v.name =~ /#{name}/ } + x.should_not be_nil + def x.to_s + "Whit[#{name}]" + end + def x.inspect + to_s + end + x + end + + def admissible_sentinal_of(x) + @depgraph.vertex?(x) ? x : whit_called("admissible_#{x.name}") + end + + def completed_sentinal_of(x) + @depgraph.vertex?(x) ? x : whit_called("completed_#{x.name}") end before do @@ -585,63 +599,87 @@ describe Puppet::SimpleGraph do splice end - # This is the real heart of splicing -- replacing all containers in - # our relationship and exploding their relationships so that each - # relationship to a container gets copied to all of its children. + # This is the real heart of splicing -- replacing all containers X in our + # relationship graph with a pair of whits { admissible_X and completed_X } + # such that that + # + # 0) completed_X depends on admissible_X + # 1) contents of X each depend on admissible_X + # 2) completed_X depends on each on the contents of X + # 3) everything which depended on X depends on completed_X + # 4) admissible_X depends on everything X depended on + # 5) the containers and their edges must be removed + # + # Note that this requires attention to the possible case of containers + # which contain or depend on other containers. + # + # Point by point: + + # 0) completed_X depends on admissible_X + # + it "every container's completed sentinal should depend on its admissible sentinal" do + containers.each { |container| + @depgraph.path_between(admissible_sentinal_of(container),completed_sentinal_of(container)).should be + } + end + + # 1) contents of X each depend on admissible_X + # + it "all contained objects should depend on their container's admissible sentinal" do + containers.each { |container| + contents_of(container).each { |leaf| + @depgraph.should be_edge(admissible_sentinal_of(container),admissible_sentinal_of(leaf)) + } + } + end + + # 2) completed_X depends on each on the contents of X + # + it "completed sentinals should depend on their container's contents" do + containers.each { |container| + contents_of(container).each { |leaf| + @depgraph.should be_edge(completed_sentinal_of(leaf),completed_sentinal_of(container)) + } + } + end + + # + # 3) everything which depended on X depends on completed_X + + # + # 4) admissible_X depends on everything X depended on + + # 5) the containers and their edges must be removed + # it "should remove all Container objects from the dependency graph" do @depgraph.vertices.find_all { |v| v.is_a?(Container) }.should be_empty end - # This is a bit hideous, but required to make stages work with relationships - they're - # the top of the graph. it "should remove all Stage resources from the dependency graph" do @depgraph.vertices.find_all { |v| v.is_a?(Puppet::Type.type(:stage)) }.should be_empty end - it "should add container relationships to contained objects" do - @contgraph.leaves(@middle).each do |leaf| - @depgraph.should be_edge("h", leaf) - end - end - - it "should explode container-to-container relationships, making edges between all respective contained objects" do - @one.each do |oobj| - @two.each do |tobj| - @depgraph.should be_edge(oobj, tobj) - end - end - end - - it "should contain a whit-resource to mark the place held by the empty container" do - @depgraph.vertices.find_all { |v| v.is_a?(@whit) }.length.should == 1 - end - - it "should replace edges to empty containers with edges to their residual whit" do - emptys_whit = @depgraph.vertices.find_all { |v| v.is_a?(@whit) }.first - @depgraph.should be_edge("c", emptys_whit) - end - it "should no longer contain anything but the non-container objects" do @depgraph.vertices.find_all { |v| ! v.is_a?(String) and ! v.is_a?(@whit)}.should be_empty end - it "should copy labels" do - @depgraph.edges.each do |edge| - edge.label.should == {:callback => :refresh} - end + it "should retain labels on non-containment edges" do + @explicit_dependencies.each { |f,t| + @depgraph.edges_between(completed_sentinal_of(f),admissible_sentinal_of(t))[0].label.should == {:callback => :refresh} + } end it "should not add labels to edges that have none" do @depgraph.add_edge(@two, @three) splice - @depgraph.edges_between("c", "i")[0].label.should == {} + @depgraph.path_between("c", "i").any? {|segment| segment.all? {|e| e.label == {} }}.should be end it "should copy labels over edges that have none" do @depgraph.add_edge("c", @three, {:callback => :refresh}) splice # And make sure the label got copied. - @depgraph.edges_between("c", "i")[0].label.should == {:callback => :refresh} + @depgraph.path_between("c", "i").flatten.select {|e| e.label == {:callback => :refresh} }.should_not be_empty end it "should not replace a label with a nil label" do @@ -649,7 +687,7 @@ describe Puppet::SimpleGraph do @depgraph.add_edge(@middle, @three) @depgraph.add_edge("c", @three, {:callback => :refresh}) splice - @depgraph.edges_between("c", "i")[0].label.should == {:callback => :refresh} + @depgraph.path_between("c","i").flatten.select {|e| e.label == {:callback => :refresh} }.should_not be_empty end it "should copy labels to all created edges" do @@ -658,8 +696,9 @@ describe Puppet::SimpleGraph do splice @three.each do |child| edge = Puppet::Relationship.new("c", child) - @depgraph.should be_edge(edge.source, edge.target) - @depgraph.edges_between(edge.source, edge.target)[0].label.should == {:callback => :refresh} + (path = @depgraph.path_between(edge.source, edge.target)).should be + path.should_not be_empty + path.flatten.select {|e| e.label == {:callback => :refresh} }.should_not be_empty end end end diff --git a/spec/unit/ssl/host_spec.rb b/spec/unit/ssl/host_spec.rb index d8f15e738..885bd45e2 100755 --- a/spec/unit/ssl/host_spec.rb +++ b/spec/unit/ssl/host_spec.rb @@ -3,16 +3,19 @@ require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper') require 'puppet/ssl/host' +require 'puppet/sslcertificates' +require 'puppet/sslcertificates/ca' describe Puppet::SSL::Host do before do - @class = Puppet::SSL::Host - @host = @class.new("myname") + Puppet::SSL::Host.indirection.terminus_class = :file + @host = Puppet::SSL::Host.new("myname") end after do # Cleaned out any cached localhost instance. Puppet::Util::Cacher.expire + Puppet::SSL::Host.ca_location = :none end it "should use any provided name as its name" do @@ -140,13 +143,6 @@ describe Puppet::SSL::Host do end describe "when specifying the CA location" do - before do - [Puppet::SSL::Key, Puppet::SSL::Certificate, Puppet::SSL::CertificateRequest, Puppet::SSL::CertificateRevocationList].each do |klass| - klass.indirection.stubs(:terminus_class=) - klass.indirection.stubs(:cache_class=) - end - end - it "should support the location ':local'" do lambda { Puppet::SSL::Host.ca_location = :local }.should_not raise_error end @@ -168,80 +164,88 @@ describe Puppet::SSL::Host do end describe "as 'local'" do - it "should set the cache class for Certificate, CertificateRevocationList, and CertificateRequest as :file" do - Puppet::SSL::Certificate.indirection.expects(:cache_class=).with :file - Puppet::SSL::CertificateRequest.indirection.expects(:cache_class=).with :file - Puppet::SSL::CertificateRevocationList.indirection.expects(:cache_class=).with :file - + before do Puppet::SSL::Host.ca_location = :local end - it "should set the terminus class for Key as :file" do - Puppet::SSL::Key.indirection.expects(:terminus_class=).with :file + it "should set the cache class for Certificate, CertificateRevocationList, and CertificateRequest as :file" do + Puppet::SSL::Certificate.indirection.cache_class.should == :file + Puppet::SSL::CertificateRequest.indirection.cache_class.should == :file + Puppet::SSL::CertificateRevocationList.indirection.cache_class.should == :file + end - Puppet::SSL::Host.ca_location = :local + it "should set the terminus class for Key and Host as :file" do + Puppet::SSL::Key.indirection.terminus_class.should == :file + Puppet::SSL::Host.indirection.terminus_class.should == :file end it "should set the terminus class for Certificate, CertificateRevocationList, and CertificateRequest as :ca" do - Puppet::SSL::Certificate.indirection.expects(:terminus_class=).with :ca - Puppet::SSL::CertificateRequest.indirection.expects(:terminus_class=).with :ca - Puppet::SSL::CertificateRevocationList.indirection.expects(:terminus_class=).with :ca - - Puppet::SSL::Host.ca_location = :local + Puppet::SSL::Certificate.indirection.terminus_class.should == :ca + Puppet::SSL::CertificateRequest.indirection.terminus_class.should == :ca + Puppet::SSL::CertificateRevocationList.indirection.terminus_class.should == :ca end end describe "as 'remote'" do - it "should set the cache class for Certificate, CertificateRevocationList, and CertificateRequest as :file" do - Puppet::SSL::Certificate.indirection.expects(:cache_class=).with :file - Puppet::SSL::CertificateRequest.indirection.expects(:cache_class=).with :file - Puppet::SSL::CertificateRevocationList.indirection.expects(:cache_class=).with :file - + before do Puppet::SSL::Host.ca_location = :remote end - it "should set the terminus class for Key as :file" do - Puppet::SSL::Key.indirection.expects(:terminus_class=).with :file - - Puppet::SSL::Host.ca_location = :remote + it "should set the cache class for Certificate, CertificateRevocationList, and CertificateRequest as :file" do + Puppet::SSL::Certificate.indirection.cache_class.should == :file + Puppet::SSL::CertificateRequest.indirection.cache_class.should == :file + Puppet::SSL::CertificateRevocationList.indirection.cache_class.should == :file end - it "should set the terminus class for Certificate, CertificateRevocationList, and CertificateRequest as :rest" do - Puppet::SSL::Certificate.indirection.expects(:terminus_class=).with :rest - Puppet::SSL::CertificateRequest.indirection.expects(:terminus_class=).with :rest - Puppet::SSL::CertificateRevocationList.indirection.expects(:terminus_class=).with :rest + it "should set the terminus class for Key as :file" do + Puppet::SSL::Key.indirection.terminus_class.should == :file + end - Puppet::SSL::Host.ca_location = :remote + it "should set the terminus class for Host, Certificate, CertificateRevocationList, and CertificateRequest as :rest" do + Puppet::SSL::Host.indirection.terminus_class.should == :rest + Puppet::SSL::Certificate.indirection.terminus_class.should == :rest + Puppet::SSL::CertificateRequest.indirection.terminus_class.should == :rest + Puppet::SSL::CertificateRevocationList.indirection.terminus_class.should == :rest end end describe "as 'only'" do - it "should set the terminus class for Key, Certificate, CertificateRevocationList, and CertificateRequest as :ca" do - Puppet::SSL::Key.indirection.expects(:terminus_class=).with :ca - Puppet::SSL::Certificate.indirection.expects(:terminus_class=).with :ca - Puppet::SSL::CertificateRequest.indirection.expects(:terminus_class=).with :ca - Puppet::SSL::CertificateRevocationList.indirection.expects(:terminus_class=).with :ca - + before do Puppet::SSL::Host.ca_location = :only end - it "should reset the cache class for Certificate, CertificateRevocationList, and CertificateRequest to nil" do - Puppet::SSL::Certificate.indirection.expects(:cache_class=).with nil - Puppet::SSL::CertificateRequest.indirection.expects(:cache_class=).with nil - Puppet::SSL::CertificateRevocationList.indirection.expects(:cache_class=).with nil + it "should set the terminus class for Key, Certificate, CertificateRevocationList, and CertificateRequest as :ca" do + Puppet::SSL::Key.indirection.terminus_class.should == :ca + Puppet::SSL::Certificate.indirection.terminus_class.should == :ca + Puppet::SSL::CertificateRequest.indirection.terminus_class.should == :ca + Puppet::SSL::CertificateRevocationList.indirection.terminus_class.should == :ca + end - Puppet::SSL::Host.ca_location = :only + it "should set the cache class for Certificate, CertificateRevocationList, and CertificateRequest to nil" do + Puppet::SSL::Certificate.indirection.cache_class.should be_nil + Puppet::SSL::CertificateRequest.indirection.cache_class.should be_nil + Puppet::SSL::CertificateRevocationList.indirection.cache_class.should be_nil + end + + it "should set the terminus class for Host to :file" do + Puppet::SSL::Host.indirection.terminus_class.should == :file end end describe "as 'none'" do + before do + Puppet::SSL::Host.ca_location = :none + end + it "should set the terminus class for Key, Certificate, CertificateRevocationList, and CertificateRequest as :file" do - Puppet::SSL::Key.indirection.expects(:terminus_class=).with :file - Puppet::SSL::Certificate.indirection.expects(:terminus_class=).with :file - Puppet::SSL::CertificateRequest.indirection.expects(:terminus_class=).with :file - Puppet::SSL::CertificateRevocationList.indirection.expects(:terminus_class=).with :file + Puppet::SSL::Key.indirection.terminus_class.should == :file + Puppet::SSL::Certificate.indirection.terminus_class.should == :file + Puppet::SSL::CertificateRequest.indirection.terminus_class.should == :file + Puppet::SSL::CertificateRevocationList.indirection.terminus_class.should == :file + end - Puppet::SSL::Host.ca_location = :none + it "should set the terminus class for Host to 'none'" do + lambda { Puppet::SSL::Host.indirection.terminus_class }.should raise_error(Puppet::DevError) end end end @@ -271,8 +275,8 @@ describe Puppet::SSL::Host do Puppet::SSL::Host.destroy("myhost").should be_true end - it "should return false if none of the classes returned true" do - Puppet::SSL::Host.destroy("myhost").should be_false + it "should report that nothing was deleted if none of the classes returned true" do + Puppet::SSL::Host.destroy("myhost").should == "Nothing was deleted" end end @@ -709,4 +713,84 @@ describe Puppet::SSL::Host do @host.wait_for_cert(1) end end + + describe "when handling PSON" do + include PuppetSpec::Files + + before do + Puppet[:vardir] = tmpdir("ssl_test_vardir") + Puppet[:ssldir] = tmpdir("ssl_test_ssldir") + Puppet::SSLCertificates::CA.new.mkrootcert + # localcacert is where each client stores the CA certificate + # cacert is where the master stores the CA certificate + # Since we need to play the role of both for testing we need them to be the same and exist + Puppet[:cacert] = Puppet[:localcacert] + + @ca=Puppet::SSL::CertificateAuthority.new + end + + describe "when converting to PSON" do + it "should be able to identify a host with an unsigned certificate request" do + host = Puppet::SSL::Host.new("bazinga") + host.generate_certificate_request + pson_hash = { + "fingerprint" => host.certificate_request.fingerprint, + "desired_state" => 'requested', + "name" => host.name + } + + result = PSON.parse(Puppet::SSL::Host.new(host.name).to_pson) + result["fingerprint"].should == pson_hash["fingerprint"] + result["name"].should == pson_hash["name"] + result["state"].should == pson_hash["desired_state"] + end + + it "should be able to identify a host with a signed certificate" do + host = Puppet::SSL::Host.new("bazinga") + host.generate_certificate_request + @ca.sign(host.name) + pson_hash = { + "fingerprint" => Puppet::SSL::Certificate.indirection.find(host.name).fingerprint, + "desired_state" => 'signed', + "name" => host.name, + } + + result = PSON.parse(Puppet::SSL::Host.new(host.name).to_pson) + result["fingerprint"].should == pson_hash["fingerprint"] + result["name"].should == pson_hash["name"] + result["state"].should == pson_hash["desired_state"] + end + + it "should be able to identify a host with a revoked certificate" do + host = Puppet::SSL::Host.new("bazinga") + host.generate_certificate_request + @ca.sign(host.name) + @ca.revoke(host.name) + pson_hash = { + "fingerprint" => Puppet::SSL::Certificate.indirection.find(host.name).fingerprint, + "desired_state" => 'revoked', + "name" => host.name, + } + + result = PSON.parse(Puppet::SSL::Host.new(host.name).to_pson) + result["fingerprint"].should == pson_hash["fingerprint"] + result["name"].should == pson_hash["name"] + result["state"].should == pson_hash["desired_state"] + end + end + + describe "when converting from PSON" do + it "should return a Puppet::SSL::Host object with the specified desired state" do + host = Puppet::SSL::Host.new("bazinga") + host.desired_state="signed" + pson_hash = { + "name" => host.name, + "desired_state" => host.desired_state, + } + generated_host = Puppet::SSL::Host.from_pson(pson_hash) + generated_host.desired_state.should == host.desired_state + generated_host.name.should == host.name + end + end + end end diff --git a/spec/unit/transaction_spec.rb b/spec/unit/transaction_spec.rb index 3677bfd87..ab76130e3 100755 --- a/spec/unit/transaction_spec.rb +++ b/spec/unit/transaction_spec.rb @@ -114,7 +114,6 @@ describe Puppet::Transaction do describe "when evaluating a resource" do before do @transaction = Puppet::Transaction.new(Puppet::Resource::Catalog.new) - @transaction.stubs(:eval_children_and_apply_resource) @transaction.stubs(:skip?).returns false @resource = Puppet::Type.type(:file).new :path => @basepath @@ -126,12 +125,6 @@ describe Puppet::Transaction do @transaction.eval_resource(@resource) end - it "should eval and apply children" do - @transaction.expects(:eval_children_and_apply_resource).with(@resource, nil) - - @transaction.eval_resource(@resource) - end - it "should process events" do @transaction.event_manager.expects(:process_events).with(@resource) @@ -197,7 +190,7 @@ describe Puppet::Transaction do second.expects(:generate).returns [third] third.expects(:generate) - @transaction.generate_additional_resources(first, :generate) + @transaction.generate_additional_resources(first) end it "should finish all resources" do @@ -213,7 +206,7 @@ describe Puppet::Transaction do resource.expects(:finish) - @transaction.generate_additional_resources(generator, :generate) + @transaction.generate_additional_resources(generator) end it "should skip generated resources that conflict with existing resources" do @@ -230,7 +223,7 @@ describe Puppet::Transaction do resource.expects(:finish).never resource.expects(:info) # log that it's skipped - @transaction.generate_additional_resources(generator, :generate).should be_empty + @transaction.generate_additional_resources(generator) end it "should copy all tags to the newly generated resources" do @@ -244,8 +237,10 @@ describe Puppet::Transaction do @catalog.stubs(:add_resource) child.expects(:tag).with("one", "two") + child.expects(:finish) + generator.expects(:depthfirst?) - @transaction.generate_additional_resources(generator, :generate) + @transaction.generate_additional_resources(generator) end end @@ -387,20 +382,19 @@ describe Puppet::Transaction do describe 'within an evaluate call' do before do - @resource = stub 'resource', :ref => 'some_ref' + @resource = Puppet::Type.type(:notify).new :title => "foobar" @catalog.add_resource @resource @transaction.stubs(:prepare) - @transaction.sorted_resources = [@resource] end it 'should stop processing if :stop_processing? is true' do - @transaction.expects(:stop_processing?).returns(true) + @transaction.stubs(:stop_processing?).returns(true) @transaction.expects(:eval_resource).never @transaction.evaluate end it 'should continue processing if :stop_processing? is false' do - @transaction.expects(:stop_processing?).returns(false) + @transaction.stubs(:stop_processing?).returns(false) @transaction.expects(:eval_resource).returns(nil) @transaction.evaluate end diff --git a/spec/unit/type/whit_spec.rb b/spec/unit/type/whit_spec.rb index 46eb0ab6e..0a3324afa 100644 --- a/spec/unit/type/whit_spec.rb +++ b/spec/unit/type/whit_spec.rb @@ -5,7 +5,7 @@ require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper') whit = Puppet::Type.type(:whit).new(:name => "Foo::Bar") describe whit do - it "should stringify as though it were the class it represents" do - whit.to_s.should == "Class[Foo::Bar]" + it "should stringify in a way that users will regognise" do + whit.to_s.should == "(Foo::Bar)" end end diff --git a/test/lib/puppettest/support/assertions.rb b/test/lib/puppettest/support/assertions.rb index 31fa3f1da..758c126ce 100644 --- a/test/lib/puppettest/support/assertions.rb +++ b/test/lib/puppettest/support/assertions.rb @@ -46,7 +46,12 @@ module PuppetTest config = resources2catalog(*resources) transaction = Puppet::Transaction.new(config) - run_events(:evaluate, transaction, events, msg) + transaction.evaluate + newevents = transaction.events. + reject { |e| ['failure', 'audit'].include? e.status }. + collect { |e| e.name } + + assert_equal(events, newevents, "Incorrect evaluate #{msg} events") transaction end diff --git a/test/lib/puppettest/support/utils.rb b/test/lib/puppettest/support/utils.rb index bca5d9634..4ecc3819e 100644 --- a/test/lib/puppettest/support/utils.rb +++ b/test/lib/puppettest/support/utils.rb @@ -82,25 +82,6 @@ module PuppetTest::Support::Utils @mygroup = group end - def run_events(type, trans, events, msg) - case type - when :evaluate, :rollback # things are hunky-dory - else - raise Puppet::DevError, "Incorrect run_events type" - end - - method = type - - trans.send(method) - newevents = trans.events.reject { |e| ['failure', 'audit'].include? e.status }.collect { |e| - e.name - } - - assert_equal(events, newevents, "Incorrect #{type} #{msg} events") - - trans - end - def fakefile(name) ary = [basedir, "test"] ary += name.split("/") diff --git a/test/other/relationships.rb b/test/other/relationships.rb index 717353c02..e36dcda71 100755 --- a/test/other/relationships.rb +++ b/test/other/relationships.rb @@ -14,11 +14,8 @@ class TestRelationships < Test::Unit::TestCase def newfile assert_nothing_raised { - - return Puppet::Type.type(:file).new( - + return Puppet::Type.type(:file).new( :path => tempfile, - :check => [:mode, :owner, :group] ) } @@ -58,18 +55,14 @@ class TestRelationships < Test::Unit::TestCase def test_autorequire # We know that execs autorequire their cwd, so we'll use that path = tempfile - - - file = Puppet::Type.type(:file).new( - :title => "myfile", :path => path, - - :ensure => :directory) - - exec = Puppet::Type.newexec( - :title => "myexec", :cwd => path, - - :command => "/bin/echo") - + file = Puppet::Type.type(:file).new( + :title => "myfile", :path => path, + :ensure => :directory + ) + exec = Puppet::Type.newexec( + :title => "myexec", :cwd => path, + :command => "/bin/echo" + ) catalog = mk_catalog(file, exec) reqs = nil assert_nothing_raised do @@ -82,7 +75,7 @@ class TestRelationships < Test::Unit::TestCase # Now make sure that these relationships are added to the # relationship graph catalog.apply do |trans| - assert(catalog.relationship_graph.edge?(file, exec), "autorequire edge was not created") + assert(catalog.relationship_graph.path_between(file, exec), "autorequire edge was not created") end end diff --git a/test/other/transactions.rb b/test/other/transactions.rb index be8cef483..812e519ab 100755 --- a/test/other/transactions.rb +++ b/test/other/transactions.rb @@ -114,39 +114,6 @@ class TestTransactions < Test::Unit::TestCase assert_equal({inst.title => inst}, $prefetched, "evaluate did not call prefetch") end - # We need to generate resources before we prefetch them, else generated - # resources that require prefetching don't work. - def test_generate_before_prefetch - config = mk_catalog - trans = Puppet::Transaction.new(config) - - generate = nil - prefetch = nil - trans.expects(:generate).with { |*args| generate = Time.now; true } - trans.expects(:prefetch).with { |*args| ! generate.nil? } - trans.prepare - return - - resource = Puppet::Type.type(:file).new :ensure => :present, :path => tempfile - other_resource = mock 'generated' - def resource.generate - [other_resource] - end - - - config = mk_catalog(yay, rah) - trans = Puppet::Transaction.new(config) - - assert_nothing_raised do - trans.generate - end - - %w{ya ra y r}.each do |name| - assert(trans.catalog.vertex?(Puppet::Type.type(:generator)[name]), "Generated #{name} was not a vertex") - assert($finished.include?(name), "#{name} was not finished") - end - end - def test_ignore_tags? config = Puppet::Resource::Catalog.new config.host_config = true @@ -235,7 +202,7 @@ class TestTransactions < Test::Unit::TestCase config = mk_catalog(one, two) trans = Puppet::Transaction.new(config) assert_raise(Puppet::Error) do - trans.prepare + trans.evaluate end end diff --git a/test/ral/type/file/target.rb b/test/ral/type/file/target.rb index 272128586..d778f2891 100755 --- a/test/ral/type/file/target.rb +++ b/test/ral/type/file/target.rb @@ -24,12 +24,9 @@ class TestFileTarget < Test::Unit::TestCase file = nil assert_nothing_raised { - - file = Puppet::Type.type(:file).new( - + file = Puppet::Type.type(:file).new( :title => "somethingelse", :ensure => path, - :path => link ) } @@ -102,12 +99,9 @@ class TestFileTarget < Test::Unit::TestCase link = nil assert_nothing_raised { - - link = Puppet::Type.type(:file).new( - + link = Puppet::Type.type(:file).new( :ensure => source, :path => dest, - :recurse => true ) } @@ -140,11 +134,8 @@ class TestFileTarget < Test::Unit::TestCase link = nil assert_nothing_raised { - - link = Puppet::Type.type(:file).new( - + link = Puppet::Type.type(:file).new( :path => dest, - :ensure => "source" ) } @@ -161,20 +152,16 @@ class TestFileTarget < Test::Unit::TestCase resources = [] - resources << Puppet::Type.type(:exec).new( - + resources << Puppet::Type.type(:exec).new( :command => "mkdir #{source}; touch #{source}/file", :title => "yay", - :path => ENV["PATH"] ) - resources << Puppet::Type.type(:file).new( - + resources << Puppet::Type.type(:file).new( :ensure => source, :path => dest, :recurse => true, - :require => resources[0] ) |