A comment or two below. None of them are likely worth redoing the patch, but I'll let you decide that.
On Aug 14, 2009, at 7:45 PM, Markus Roberts wrote: > > > Signed-off-by: Markus Roberts <[email protected]> > --- > lib/puppet/network/authstore.rb | 4 +- > spec/unit/network/authstore.rb | 94 ++++++++++++++++++++++++++++++ > +++++++++ > 2 files changed, 97 insertions(+), 1 deletions(-) > create mode 100644 spec/unit/network/authstore.rb > > diff --git a/lib/puppet/network/authstore.rb b/lib/puppet/network/ > authstore.rb > index 306e1ba..4707f36 100755 > --- a/lib/puppet/network/authstore.rb > +++ b/lib/puppet/network/authstore.rb > @@ -249,7 +249,7 @@ module Puppet > > # Does the name match our pattern? > def matchname?(name) > - name = munge_name(name) unless @name == :opaque > + name = munge_name(name) > return true if self.pattern == name > > # If it's an exact match, then just return false, > since the > @@ -272,6 +272,7 @@ module Puppet > # Convert the name to a common pattern. > def munge_name(name) > # LAK:NOTE http://snurl.com/21zf8 [groups_google_com] > + # Change to x = name.downcase.split(".",-1).reverse > for FQDN support > x = name.downcase.split(".").reverse > end > > @@ -306,6 +307,7 @@ module Puppet > raise AuthStoreError, "Invalid IP address > pattern %s" % value > end > when /^([a-zA-Z][-\w]*\.)+[-\w]+$/ # a full hostname > + # Change to /^([a-zA-Z][-\w]*\.)+[-\w]+\.?$/ > for FQDN support > @name = :domain > @pattern = munge_name(value) > when /^\*(\.([a-zA-Z][-\w]*)){1,}$/ # *.domain.com > diff --git a/spec/unit/network/authstore.rb b/spec/unit/network/ > authstore.rb > new file mode 100644 > index 0000000..224d671 > --- /dev/null > +++ b/spec/unit/network/authstore.rb > @@ -0,0 +1,94 @@ > +#!/usr/bin/env ruby > + > +require File.dirname(__FILE__) + '/../../spec_helper' > + > +require 'puppet/network/authconfig' > + > +describe Puppet::Network::AuthStore::Declaration do > + > + describe "when the pattern is simple numeric IP" do > + before :each do > + @ip = '100.101.99.98' > + @declaration = > Puppet::Network::AuthStore::Declaration.new(:allow,@ip) > + end > + it "should match the specified IP" do > + @declaration.should be_match('www.testsite.org',@ip) > + end > + it "should not match other IPs" do > + @declaration.should_not > be_match('www.testsite.org','200.101.99.98') > + end > + end > + > + describe "when the pattern is a numeric IP with a back > reference" do > + before :each do > + @ip = '100.101.$1' > + @declaration = > Puppet > ::Network > ::AuthStore::Declaration.new(:allow,@ip).interpolate('12.34'.match / > (.*)/) > + end > + it "should match an IP with the apropriate interpolation" do > + @declaration.should > be_match('www.testsite.org',@ip.sub(/\$1/,'12.34')) > + end For these tests, the before block makes it quite less readable. This test would be much more readable with them. E.g., I think the above test would be: Puppet::Network::AuthStore::Declaration.new(:allow, "100.101.$1").interpolate('12.34'.match /(.*)/).should be_match('www.testsite.org , '100.101.12.34') It seems like the '.match' call could be skipped somehow, but I guess not. It took me a couple of readings to figure out wtf was going on, which I think is a bad sign in a test. Having to modify any variable set in a before block is code smell, IMO. > > + it "should not match other IPs" do > + @declaration.should_not > be_match('www.testsite.org',@ip.sub(/\$1/,'66.34')) > + end > > + end > + > + describe "when the pattern is a PQDN" do > + before :each do > + @host = 'spirit.mars.nasa.gov' > + @declaration = > Puppet::Network::AuthStore::Declaration.new(:allow,@host) > + end > + it "should match the specified PQDN" do > + pending "FQDN consensus" Sorry, I must have missed this part of the discussion. Where does consensus need to be reached? > > + @declaration.should be_match(@host,'200.101.99.98') > + end > + it "should not match a similar FQDN" do > + pending "FQDN consensus" > + @declaration.should_not be_match(@host > +'.','200.101.99.98') > + end > + end > + > + describe "when the pattern is a FQDN" do > + before :each do > + @host = 'spirit.mars.nasa.gov.' > + @declaration = > Puppet::Network::AuthStore::Declaration.new(:allow,@host) > + end > + it "should match the specified FQDN" do > + pending "FQDN consensus" > + @declaration.should be_match(@host,'200.101.99.98') > + end > + it "should not match a similar PQDN" do > + pending "FQDN consensus" > + @declaration.should_not > be_match(@host[0..-2],'200.101.99.98') > + end > + end > + > + > + describe "when the pattern is an opaque string with a back > reference" do > + before :each do > + @host = 'c216f41a-f902-4bfb-a222-850dd957bebb' > + @item = "/catalog/#...@host}" > + @pattern = %{^/catalog/([^/]+)$} > + @declaration = > Puppet::Network::AuthStore::Declaration.new(:allow,'$1') > + end > + it "should match an IP with the apropriate interpolation" do > + @declaration.interpolate(@item.match(@pattern)).should > be_match(@host,'10.0.0.5') > + end > + end > + > + describe "when comparing patterns" do > + before :each do > + @ip = > Puppet::Network::AuthStore::Declaration.new(:allow,'127.0.0.1') > + @host_name = > Puppet::Network::AuthStore::Declaration.new(:allow,'www.hard_knocks.edu > ') > + @opaque = > Puppet::Network::AuthStore::Declaration.new(:allow,'hey_dude') > + end > + it "should consider ip addresses before host names" do > + (@ip < @host_name).should be_true > + end > + it "should consider ip addresses before opaque strings" do > + (@ip < @opaque).should be_true > + end > + it "should consider host_names before opaque strings" do > + (@host_name < @opaque).should be_true > + end > + end > +end > -- > 1.6.0.4 > > > > -- You can't build a reputation on what you are going to do. -- Henry Ford --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
