The current code only returns PSON correctly. I'm putting in a ticket
today for fixing problems with YAML and other supported formats.
On Tue, Apr 5, 2011 at 12:11 PM, Corey Osman <co...@logicminds.biz> wrote:
> 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.
>
>
--
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.
