On Aug 16, 2009, at 11:18 PM, Markus wrote:
>
> Luke --
>
>> It took me a couple of readings to figure out wtf was going on, which
>> I think is a bad sign in a test.
>
> Agreed.
>
> The whole thing is too complex IMHO, but I wouldn't recommend a full
> refactoring in what amounts to a pre-release code freeze, and so we
> should test as best we can for now.
>
> The root of the problem is that we really have multiple parties to an
> interpolated match--the value being tested, the template it is being
> tested against, and the MatchData object being used to interpolate the
> template. To add further complexity, the cleanest whay I know of to
> construct a MatchData object is by matching a String against a RegExp,
> further increasing the number of players in this little drama.
>
> Would you find something like this more readable?
>
>
> Declaration = Puppet::Network::AuthStore::Declaration
>
> describe "when the pattern is a numeric IP with a back reference" do
>
> before :each do
> constant_part = '100.101.'
> variable_part = '43.21'
> template = constant_part+'$1'
> match_data = variable_part.match /^(.*)$/
> @declaration =
> Declaration.new(:allow,template).interpolate(match_data)
> @valid_ip = constant_part+variable_part
> @other_ip = constant_part+variable_part+'1'
> end
>
> it "should match an IP with the appropriate interpolation" do
> @declaration.should be_match('www.testsite.org',@valid_ip)
> end
>
> it "should not match other IPs" do
> @declaration.should_not be_match('www.testsite.org',@other_ip)
> end
We discussed this in person, but just to follow up: I think using
literals in the calls whenever possible really helps clarify the
tests. The setup code is much longer than the test code here, for
only two tests, and I don't think it's worth it.
You could maybe improve readability by having a 'mkdeclaration' method
or something that allowed each test to declare all of the literals:
it "should match an IP with the appropriate interpolation" do
mkdeclaration(:allow, "100.101.$1").should be_match("www.testsite.org
", '101.101.43.21')
end
I think this is much more readable and the only real duplication is
the data, which I'm fine with.
>
> ------------------------------------------------------------
>
>
>>> + 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?
>
> I'd posted the question to the dev-list sometime last week:
>
> Somewhat afield from the previous discussion; I notice that the
> present code doesn't handle FQDNs; this may be correct
> behavior,
> or it might be a omission no one cares about. My assumption
> (when I had my test writing hat on) was that it should handle
> them but consider them distinct from the corresponding PQDNs to
> preclude the possibility of sub-domain naming attacks
> ("benign.domain.com.ha_ha.evil.com.", aka "benign.domain.com").
>
> Is this correct, or should they be precluded (as presently--you
> can't write an allow/deny rule with a terminal dot) or is
> this a
> bit of esoterica that nobody else cares about?
>
> When no one responded I assumed it was a non-issue but left the
> comments
> in the code in case it came up later.
Also discussed in person, but it should definitely support fqdns with
the '.'.
--
The optimist proclaims that we live in the best of all possible worlds,
and the pessimist fears that this is true.
-- James Branch Cabell 1879-1958
---------------------------------------------------------------------
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
-~----------~----~----~----~------~----~------~--~---
