Horray! I am excited to test out the list of signed certificates.
What serialization format would the list of certs be returned as?
(PSON, YAML)? Can I have a choice by passing in the format I desire?
Corey
On Apr 5, 10:47 am, Jacob Helwig <ja...@puppetlabs.com> wrote:
> On Mon, 04 Apr 2011 17:13:02 -0700, Max Martin wrote:
>
> > This code has not actually been merged yet, but is currently being reviewed
> > by Jacob Helwig. Matt and I decided to go ahead and send out this e-mail to
> > see if anyone has comments/suggestions/issues, since this is a pretty big
> > feature and a lot of people have touched the code, so please reply if
> > anything stands out to you.
>
> I already gave these comments to Matt & Max, but just to keep everyone
> else in the loop:
>
> The original form ended up generating a lot of text to wade through, so
> a form like this might be easier to read.
>
> diff --git a/lib/puppet/indirector/certificate_status/file.rb
> b/lib/puppet/indirector/certificate_status/file.rb
> index e44de1c..092747e 100644
> --- a/lib/puppet/indirector/certificate_status/file.rb
> +++ b/lib/puppet/indirector/certificate_status/file.rb
> @@ -20,12 +20,12 @@ class Puppet::Indirector::CertificateStatus::File <
> Puppet::Indirector::Code
> Puppet::SSL::Key,
> ].collect do |part|
> if part.indirection.destroy(request.key)
> - deleted << "#{part} for host #{request.key} was deleted"
> + deleted << "#{part}"
> end
> end
>
> return "Nothing was deleted" if deleted.empty?
> - deleted.join(", ")
> + "Deleted for '#{request.key}': #{deleted.join(", ")}"
> end
>
> def save(request)
>
> Minor grammar nit, details isn't used, also the 'invalid' status
> functionality appears to have been lost from an earlier iteration. From
> talking with Matt & Max, the code has changed enough that the earlier
> approach no longer works. Matt said he'd open a ticket to add the
> 'invalid' status support.
>
> diff --git a/lib/puppet/ssl/host.rb b/lib/puppet/ssl/host.rb
> index d00efd9..d43ed1d 100644
> --- a/lib/puppet/ssl/host.rb
> +++ b/lib/puppet/ssl/host.rb
> @@ -100,7 +100,7 @@ class Puppet::SSL::Host
> # Specify how we expect to interact with our certificate authority.
> def self.ca_location=(mode)
> modes = CA_MODES.collect { |m, vals| m.to_s }.join(", ")
> - raise ArgumentError, "CA Mode can only be #{modes}" unless
> CA_MODES.include?(mode)
> + raise ArgumentError, "CA Mode can only be one of: #{modes}" unless
> CA_MODES.include?(mode)
>
> @ca_location = mode
>
> @@ -302,11 +302,11 @@ class Puppet::SSL::Host
> begin
> Puppet::SSL::CertificateAuthority.new.verify(my_cert)
> return 'signed'
> - rescue Puppet::SSL::CertificateAuthority::CertificateVerificationError
> => details
> + # Couldn't this also mean that it's invalid?
> + rescue Puppet::SSL::CertificateAuthority::CertificateVerificationError
> return 'revoked'
> end
> end
> -
> end
>
> require 'puppet/ssl/certificate_authority'
>
> There's an easier way to create a temp directory, updating tests to
> reflect the message change earlier, and using the two argument form of
> '.should raise_error'.
>
> diff --git a/spec/unit/indirector/certificate_status/file_spec.rb
> b/spec/unit/indirector/certificate_status/file_spec.rb
> index 0a090cf..ab10343 100644
> --- a/spec/unit/indirector/certificate_status/file_spec.rb
> +++ b/spec/unit/indirector/certificate_status/file_spec.rb
> @@ -6,16 +6,15 @@ 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 = Tempfile.new("certificate_status_ca_testing")
> - @tmpdir.close
> - File.unlink(@tmpdir.path)
> - Dir.mkdir(@tmpdir.path)
> - Puppet[:confdir] = @tmpdir.path
> - Puppet[:vardir] = @tmpdir.path
> + @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
> @@ -61,7 +60,7 @@ describe "Puppet::Indirector::CertificateStatus::File" do
> 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)
> + lambda { @terminus.ca }.should raise_error(ArgumentError, "This
> process is not configured as a certificate authority")
> end
> end
>
> @@ -158,12 +157,12 @@ describe "Puppet::Indirector::CertificateStatus::File"
> 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 == "Puppet::SSL::Certificate
> for host clean_signed_cert was deleted, Puppet::SSL::Key for host
> clean_signed_cert was deleted"
> + @terminus.destroy(signed_request).should == "Deleted for host
> 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 ==
> "Puppet::SSL::CertificateRequest for host clean_csr was deleted,
> Puppet::SSL::Key for host clean_csr was deleted"
> + @terminus.destroy(csr_request).should == "Deleted for host
> clean_signed_cert: Puppet::SSL::CertificateRequest, Puppet::SSL::Key"
> end
> end
>
> @@ -186,5 +185,4 @@ describe "Puppet::Indirector::CertificateStatus::File" do
> results.should ==
> [["ca","signed"],["requested_host","requested"],["revoked_host","revoked"],["signed_host","signed"]]
> end
> end
> -
> end
>
> --
> Jacob Helwig
>
> signature.asc
> < 1KViewDownload
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to puppet-dev@googlegroups.com.
To unsubscribe from this group, send email to
puppet-dev+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
