On Fri, May 6, 2011 at 04:21, Brice Figureau
<[email protected]> wrote:

> One small comment,

That is a really great catch!  Good eye.

> On Wed, 2011-05-04 at 16:45 -0700, Pieter van de Bruggen wrote:
>> By default, it is useful to permit an individual node to query
>> information about itself, and there is no good reason to reject
>> this by default.

>> diff --git a/spec/unit/network/rest_authconfig_spec.rb 
>> b/spec/unit/network/rest_authconfig_spec.rb
>> index 499a14b..e140399 100755
>> --- a/spec/unit/network/rest_authconfig_spec.rb
>> +++ b/spec/unit/network/rest_authconfig_spec.rb
>> @@ -5,18 +5,7 @@ require 'puppet/network/rest_authconfig'
>>
>>  describe Puppet::Network::RestAuthConfig do
>>
>> -  DEFAULT_ACL = [
>> -    { :acl => "~ ^\/catalog\/([^\/]+)$", :method => :find, :allow => '$1', 
>> :authenticated => true },
>> -    # this one will allow all file access, and thus delegate
>> -    # to fileserver.conf
>> -    { :acl => "/file" },
>> -    { :acl => "/certificate_revocation_list/ca", :method => :find, 
>> :authenticated => true },
>> -    { :acl => "/report", :method => :save, :authenticated => true },
>> -    { :acl => "/certificate/ca", :method => :find, :authenticated => false 
>> },
>> -    { :acl => "/certificate/", :method => :find, :authenticated => false },
>> -    { :acl => "/certificate_request", :method => [:find, :save], 
>> :authenticated => false },
>> -    { :acl => "/status", :method => [:find], :authenticated => true },
>> -  ]
>> +  DEFAULT_ACL = Puppet::Network::RestAuthConfig::DEFAULT_ACL
>
> While I understand why you're doing this, the problem is that you're
> losing some test rationales: testing that the above access control
> entries are still correctly applied.
> If someone goes to change the original catalog acl with an invalid rule
> the new test won't fail.

Those are not actually used to test anything but "can we inject
defaults correctly" against the auth.conf file content.  We *should*
be testing more than that, but this change doesn't actually do that.
It uses this to drive a table of tests to make sure our default
matching rules are working as designed.

So, after talking about this internally we are comfortable that this
is the right thing to do in the context.  Are you happy with that
explanation?

Daniel
-- 
⎋ Puppet Labs Developer – http://puppetlabs.com
✉ Daniel Pittman <[email protected]>
✆ Contact me via gtalk, email, or phone: +1 (877) 575-9775
♲ Made with 100 percent post-consumer electrons

-- 
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.

Reply via email to