Comments below. On Jul 20, 2011, at 3:00 PM, Daniel Pittman wrote:
> The Puppet::SSL::CertificateAuthority::Interface class was an early prototype > heading toward building out a system like Faces. Now that we have done that, > this changeset ports the early code to a new face. > > Reviewed-By: Pieter van de Bruggen <[email protected]> > --- > lib/puppet/application/ca.rb | 5 + > lib/puppet/face/ca.rb | 233 +++++++++++++++++++++++++++ > spec/unit/face/ca_spec.rb | 355 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 593 insertions(+), 0 deletions(-) > create mode 100644 lib/puppet/application/ca.rb > create mode 100644 lib/puppet/face/ca.rb > create mode 100755 spec/unit/face/ca_spec.rb > > diff --git a/lib/puppet/application/ca.rb b/lib/puppet/application/ca.rb > new file mode 100644 > index 0000000..d1ec250 > --- /dev/null > +++ b/lib/puppet/application/ca.rb > @@ -0,0 +1,5 @@ > +require 'puppet/application/face_base' > + > +class Puppet::Application::Ca < Puppet::Application::FaceBase > + run_mode :master > +end > diff --git a/lib/puppet/face/ca.rb b/lib/puppet/face/ca.rb > new file mode 100644 > index 0000000..e643530 > --- /dev/null > +++ b/lib/puppet/face/ca.rb > @@ -0,0 +1,233 @@ > +require 'puppet/face' > + > +Puppet::Face.define(:ca, '0.1.0') do > + copyright "Puppet Labs", 2011 > + license "Apache 2 license; see COPYING" > + > + summary "Local Puppet Certificate Authority management." > + > + description <<TEXT > +This provides local management of the Puppet Certificate Authority. > + > +You can use this subcommand to sign outstanding certificate requests, list > +and manage local certificates, and inspect the state of the CA. > +TEXT > + > + action :list do > + summary "List certificates and/or certificate requests." > + > + description <<-end > +This will list the current certificates and certificate signing requests > +in the Puppet CA. You will also get the fingerprint, and any certificate > +verification failure reported. > + end > + > + option "--[no-]all" do > + summary "Include all certificates and requests." > + end > + > + option "--[no-]pending" do > + summary "Include pending certificate signing requests." > + end > + > + option "--[no-]signed" do > + summary "Include signed certificates." > + end > + > + option "--subject PATTERN" do > + summary "Only list if the subject matches PATTERN." > + > + description <<TEXT > +Only include certificates or requests where subject matches PATTERN. > + > +PATTERN is interpreted as a regular expression, allowing complex > +filtering of the content. > +TEXT > + end These seem to work better as plain arguments, rather than options. E.g., something like: $ puppet ca list all rather than: $ puppet ca list --all This method of using options made sense in the previous style of applications, but it doesn't work as well in the faces-style applications, IMO. I might also recommend renaming 'list' to be 'search', just to be more consistent, but I could go either way on that, really. > + when_invoked do |options| > + raise "Not a CA" unless Puppet::SSL::CertificateAuthority.ca? > + unless ca = Puppet::SSL::CertificateAuthority.instance > + raise "Unable to fetch the CA" > + end > + > + pattern = options[:subject].nil? ? nil : > + Regexp.new(options[:subject], Regexp::IGNORECASE) > + > + pending = options[:pending].nil? ? options[:all] : options[:pending] > + signed = options[:signed].nil? ? options[:all] : options[:signed] > + > + # By default we list pending, so if nothing at all was requested... > + unless pending or signed then pending = true end > + > + hosts = [] > + > + pending and hosts += ca.waiting? > + signed and hosts += ca.list > + > + pattern and hosts = hosts.select {|hostname| pattern.match hostname } > + > + hosts.sort.map {|host| Puppet::SSL::Host.new(host) } > + end > + > + when_rendering :console do |hosts| > + unless ca = Puppet::SSL::CertificateAuthority.instance > + raise "Unable to fetch the CA" > + end Shouldn't this be in the setup code or something? I expect this is part of every action. > + length = hosts.map{|x| x.name.length }.max + 1 > + > + hosts.map do |host| > + name = host.name.ljust(length) > + if host.certificate_request then > + " #{name} (#{host.certificate_request.fingerprint})" > + else > + begin > + ca.verify(host.certificate) > + "+ #{name} (#{host.certificate.fingerprint})" > + rescue > Puppet::SSL::CertificateAuthority::CertificateVerificationError => e > + "- #{name} (#{host.certificate.fingerprint}) (#{e.to_s})" > + end > + end > + end.join("\n") > + end > + end > + > + action :destroy do > + when_invoked do |host, options| > + raise "Not a CA" unless Puppet::SSL::CertificateAuthority.ca? > + unless ca = Puppet::SSL::CertificateAuthority.instance > + raise "Unable to fetch the CA" > + end > + > + ca.destroy host > + end > + end > + > + action :revoke do > + when_invoked do |host, options| > + raise "Not a CA" unless Puppet::SSL::CertificateAuthority.ca? > + unless ca = Puppet::SSL::CertificateAuthority.instance > + raise "Unable to fetch the CA" > + end > + > + begin > + ca.revoke host > + rescue ArgumentError => e > + # This is a bit naff, but it makes the behaviour consistent with the > + # destroy action. The underlying tools could be nicer for that sort > + # of thing; they have fairly inconsistent reporting of failures. > + raise unless e.to_s =~ /Could not find a serial number for / > + "Nothing was revoked" > + end > + end > + end > + > + action :generate do > + when_invoked do |host, options| > + raise "Not a CA" unless Puppet::SSL::CertificateAuthority.ca? > + unless ca = Puppet::SSL::CertificateAuthority.instance > + raise "Unable to fetch the CA" > + end > + > + begin > + ca.generate host > + rescue RuntimeError => e > + if e.to_s =~ /already has a requested certificate/ > + "#{host} already has a certificate request; use sign instead" > + else > + raise > + end > + rescue ArgumentError => e > + if e.to_s =~ /A Certificate already exists for / > + "#{host} already has a certificate" > + else > + raise > + end > + end > + end > + end > + > + action :sign do > + when_invoked do |host, options| > + raise "Not a CA" unless Puppet::SSL::CertificateAuthority.ca? > + unless ca = Puppet::SSL::CertificateAuthority.instance > + raise "Unable to fetch the CA" > + end > + > + begin > + ca.sign host > + rescue ArgumentError => e > + if e.to_s =~ /Could not find certificate request/ > + e.to_s > + else > + raise > + end > + end > + end > + end > + > + action :print do > + when_invoked do |host, options| > + raise "Not a CA" unless Puppet::SSL::CertificateAuthority.ca? > + unless ca = Puppet::SSL::CertificateAuthority.instance > + raise "Unable to fetch the CA" > + end > + > + ca.print host > + end > + end > + > + action :fingerprint do > + option "--digest ALGORITHM" do > + summary "The hash algorithm to use when displaying the fingerprint" > + end > + > + when_invoked do |host, options| > + raise "Not a CA" unless Puppet::SSL::CertificateAuthority.ca? > + unless ca = Puppet::SSL::CertificateAuthority.instance > + raise "Unable to fetch the CA" > + end > + > + begin > + # I want the default from the CA, not to duplicate it, but passing > + # 'nil' explicitly means that we don't get that. This works... > + if options.has_key? :digest > + ca.fingerprint host, options[:digest] > + else > + ca.fingerprint host > + end > + rescue ArgumentError => e > + raise unless e.to_s =~ /Could not find a certificate or csr for/ > + nil > + end > + end > + end > + > + action :verify do > + when_invoked do |host, options| > + raise "Not a CA" unless Puppet::SSL::CertificateAuthority.ca? > + unless ca = Puppet::SSL::CertificateAuthority.instance > + raise "Unable to fetch the CA" > + end > + > + begin > + ca.verify host > + { :host => host, :valid => true } > + rescue ArgumentError => e > + raise unless e.to_s =~ /Could not find a certificate for/ > + { :host => host, :valid => false, :error => e.to_s } > + rescue Puppet::SSL::CertificateAuthority::CertificateVerificationError > => e > + { :host => host, :valid => false, :error => e.to_s } > + end > + end > + > + when_rendering :console do |value| > + if value[:valid] > + nil > + else > + "Could not verify #{value[:host]}: #{value[:error]}" > + end > + end > + end > +end > diff --git a/spec/unit/face/ca_spec.rb b/spec/unit/face/ca_spec.rb > new file mode 100755 > index 0000000..8476960 > --- /dev/null > +++ b/spec/unit/face/ca_spec.rb > @@ -0,0 +1,355 @@ > +#!/usr/bin/env rspec > +require 'spec_helper' > +require 'puppet/face' > + > +describe Puppet::Face[:ca, '0.1.0'] do > + include PuppetSpec::Files > + > + before :each do > + Puppet.run_mode.stubs(:master?).returns(true) > + Puppet[:ca] = true > + Puppet[:ssldir] = tmpdir("face-ca-ssldir") > + > + Puppet::SSL::Host.ca_location = :only > + Puppet[:certificate_revocation] = true > + > + # This is way more intimate than I want to be with the implementation, > but > + # there doesn't seem any other way to test this. --daniel 2011-07-18 > + Puppet::SSL::CertificateAuthority.stubs(:instance).returns( > + # ...and this actually does the directory creation, etc. > + Puppet::SSL::CertificateAuthority.new > + ) > + end > + > + def make_certs(csr_names, crt_names) > + Array(csr_names).map do |name| > + Puppet::SSL::Host.new(name).generate_certificate_request > + end > + > + Array(crt_names).map do |name| > + Puppet::SSL::Host.new(name).generate > + end > + end > + > + context "#verify" do > + let :action do Puppet::Face[:ca, '0.1.0'].get_action(:verify) end > + > + it "should not explode if there is no certificate" do > + expect { > + subject.verify('random-host').should == { > + :host => 'random-host', :valid => false, > + :error => 'Could not find a certificate for random-host' > + } > + }.should_not raise_error > + end > + > + it "should not explode if there is only a CSR" do > + make_certs('random-host', []) > + expect { > + subject.verify('random-host').should == { > + :host => 'random-host', :valid => false, > + :error => 'Could not find a certificate for random-host' > + } > + }.should_not raise_error > + end > + > + it "should verify a signed certificate" do > + make_certs([], 'random-host') > + subject.verify('random-host').should == { > + :host => 'random-host', :valid => true > + } > + end > + > + it "should not verify a revoked certificate" do > + make_certs([], 'random-host') > + subject.revoke('random-host') > + > + expect { > + subject.verify('random-host').should == { > + :host => 'random-host', :valid => false, > + :error => 'certificate revoked' > + } > + }.should_not raise_error > + end > + > + it "should verify a revoked certificate if CRL use was turned off" do > + make_certs([], 'random-host') > + subject.revoke('random-host') > + > + Puppet[:certificate_revocation] = false > + subject.verify('random-host').should == { > + :host => 'random-host', :valid => true > + } > + end > + end > + > + context "#fingerprint" do > + let :action do Puppet::Face[:ca, '0.1.0'].get_action(:fingerprint) end > + > + it "should have a 'digest' option" do > + action.should be_option :digest > + end > + > + it "should not explode if there is no certificate" do > + expect { > + subject.fingerprint('random-host').should be_nil > + }.should_not raise_error > + end > + > + it "should fingerprint a CSR" do > + make_certs('random-host', []) > + expect { > + subject.fingerprint('random-host').should =~ /^[0-9A-F:]+$/ > + }.should_not raise_error > + end > + > + it "should fingerprint a certificate" do > + make_certs([], 'random-host') > + subject.fingerprint('random-host').should =~ /^[0-9A-F:]+$/ > + end > + > + %w{md5 MD5 sha1 ShA1 SHA1 RIPEMD160 sha256 sha512}.each do |digest| > + it "should fingerprint with #{digest.inspect}" do > + make_certs([], 'random-host') > + subject.fingerprint('random-host', :digest => digest).should =~ > /^[0-9A-F:]+$/ > + end > + > + it "should fingerprint with #{digest.to_sym} as a symbol" do > + make_certs([], 'random-host') > + subject.fingerprint('random-host', :digest => digest.to_sym). > + should =~ /^[0-9A-F:]+$/ > + end > + end > + end > + > + context "#print" do > + let :action do Puppet::Face[:ca, '0.1.0'].get_action(:print) end > + > + it "should not explode if there is no certificate" do > + expect { > + subject.print('random-host').should be_nil > + }.should_not raise_error > + end > + > + it "should return nothing if there is only a CSR" do > + make_certs('random-host', []) > + expect { > + subject.print('random-host').should be_nil > + }.should_not raise_error > + end > + > + it "should return the certificate content if there is a cert" do > + make_certs([], 'random-host') > + text = subject.print('random-host') > + text.should be_an_instance_of String > + text.should =~ /^Certificate:/ > + text.should =~ /Issuer: CN=Puppet CA: / > + text.should =~ /Subject: CN=random-host$/ > + end > + end > + > + context "#sign" do > + let :action do Puppet::Face[:ca, '0.1.0'].get_action(:sign) end > + > + it "should not explode if there is no CSR" do > + expect { > + subject.sign('random-host'). > + should == 'Could not find certificate request for random-host' > + }.should_not raise_error > + end > + > + it "should not explode if there is a signed cert" do > + make_certs([], 'random-host') > + expect { > + subject.sign('random-host'). > + should == 'Could not find certificate request for random-host' > + }.should_not raise_error > + end > + > + it "should sign a CSR if one exists" do > + make_certs('random-host', []) > + subject.sign('random-host').should be_an_instance_of > Puppet::SSL::Certificate > + > + list = subject.list(:signed => true) > + list.count.should == 1 > + list.first.name.should == 'random-host' > + end > + end > + > + context "#generate" do > + let :action do Puppet::Face[:ca, '0.1.0'].get_action(:generate) end > + > + it "should generate a certificate if requested" do > + subject.list(:all => true).should == [] > + > + subject.generate('random-host') > + > + list = subject.list(:signed => true) > + list.count.should == 1 > + list.first.name.should == 'random-host' > + end > + > + it "should not explode if a CSR with that name already exists" do > + make_certs('random-host', []) > + expect { > + subject.generate('random-host').should =~ /already has a certificate > request/ > + }.should_not raise_error > + end > + > + it "should not explode if the certificate with that name already exists" > do > + make_certs([], 'random-host') > + expect { > + subject.generate('random-host').should =~ /already has a certificate/ > + }.should_not raise_error > + end > + end > + > + context "#revoke" do > + let :action do Puppet::Face[:ca, '0.1.0'].get_action(:revoke) end > + > + it "should not explode when asked to revoke something that doesn't > exist" do > + expect { subject.revoke('nonesuch') }.should_not raise_error > + end > + > + it "should let the user know what went wrong" do > + subject.revoke('nonesuch').should == 'Nothing was revoked' > + end > + > + it "should revoke a certificate" do > + make_certs([], 'random-host') > + found = subject.list(:all => true, :subject => 'random-host') > + subject.get_action(:list).when_rendering(:console).call(found). > + should =~ /^\+ random-host/ > + > + subject.revoke('random-host') > + > + found = subject.list(:all => true, :subject => 'random-host') > + subject.get_action(:list).when_rendering(:console).call(found). > + should =~ /^- random-host \([:0-9A-F]+\) \(certificate revoked\)/ > + end > + end > + > + context "#destroy" do > + let :action do Puppet::Face[:ca, '0.1.0'].get_action(:destroy) end > + > + it "should not explode when asked to delete something that doesn't > exist" do > + expect { subject.destroy('nonesuch') }.should_not raise_error > + end > + > + it "should let the user know if nothing was deleted" do > + subject.destroy('nonesuch').should == "Nothing was deleted" > + end > + > + it "should destroy a CSR, if we have one" do > + make_certs('random-host', []) > + subject.list(:pending => true, :subject => 'random-host').should_not > == [] > + > + subject.destroy('random-host') > + > + subject.list(:pending => true, :subject => 'random-host').should == [] > + end > + > + it "should destroy a certificate, if we have one" do > + make_certs([], 'random-host') > + subject.list(:signed => true, :subject => 'random-host').should_not == > [] > + > + subject.destroy('random-host') > + > + subject.list(:signed => true, :subject => 'random-host').should == [] > + end > + > + it "should tell the user something was deleted" do > + make_certs([], 'random-host') > + subject.list(:signed => true, :subject => 'random-host').should_not == > [] > + subject.destroy('random-host'). > + should == "Deleted for random-host: Puppet::SSL::Certificate, > Puppet::SSL::Key" > + end > + end > + > + context "#list" do > + let :action do Puppet::Face[:ca, '0.1.0'].get_action(:list) end > + > + context "options" do > + subject { Puppet::Face[:ca, '0.1.0'].get_action(:list) } > + it { should be_option :pending } > + it { should be_option :signed } > + it { should be_option :all } > + it { should be_option :subject } > + end > + > + context "with no hosts in CA" do > + [:pending, :signed, :all].each do |type| > + it "should return nothing for #{type}" do > + subject.list(type => true).should == [] > + end > + > + it "should not fail when a matcher is passed" do > + expect { > + subject.list(type => true, :subject => '.').should == [] > + }.should_not raise_error > + end > + end > + end > + > + context "with some hosts" do > + csr_names = (1..3).map {|n| "csr-#{n}" } > + crt_names = (1..3).map {|n| "crt-#{n}" } > + all_names = csr_names + crt_names > + > + { > + {} => csr_names, > + { :pending => true } => csr_names, > + > + { :signed => true } => crt_names, > + > + { :all => true } => all_names, > + { :pending => true, :signed => true } => all_names, > + }.each do |input, expect| > + it "should map #{input.inspect} to #{expect.inspect}" do > + make_certs(csr_names, crt_names) > + subject.list(input).map(&:name).should =~ expect > + end > + > + ['', '.', '2', 'none'].each do |pattern| > + filtered = expect.select {|x| Regexp.new(pattern).match(x) } > + > + it "should filter all hosts matching #{pattern.inspect} to > #{filtered.inspect}" do > + make_certs(csr_names, crt_names) > + subject.list(input.merge :subject => pattern).map(&:name).should > =~ filtered > + end > + end > + end > + > + context "when_rendering :console" do > + { [["csr1.local"], []] => '^ csr1.local ', > + [[], ["crt1.local"]] => '^\+ crt1.local ', > + [["csr2"], ["crt2"]] => ['^ csr2 ', '^\+ crt2 '] > + }.each do |input, pattern| > + it "should render #{input.inspect} to match #{pattern.inspect}" do > + make_certs(*input) > + text = action.when_rendering(:console).call(subject.list(:all => > true)) > + Array(pattern).each do |item| > + text.should =~ Regexp.new(item) > + end > + end > + end > + end > + end > + end > + > + actions = %w{destroy list revoke generate sign print verify fingerprint} > + actions.each do |action| > + it { should be_action action } > + it "should fail #{action} when not a CA" do > + Puppet[:ca] = false > + expect { > + case subject.method(action).arity > + when -1 then subject.send(action) > + when -2 then subject.send(action, 'dummy') > + else > + raise "#{action} has arity #{subject.method(action).arity}" > + end > + }.should raise_error(/Not a CA/) > + end > + end > +end > -- > 1.7.6 > > -- > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]. > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en. > -- Think twice before you speak, and then you may be able to say something more insulting than if you spoke right out at once. -- Evan Esar --------------------------------------------------------------------- Luke Kanies | http://puppetlabs.com | http://about.me/lak Join us in PDX for PuppetConf: http://bit.ly/puppetconfsig -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
