On Aug 13, 2009, at 3:48 PM, Nigel Kersten wrote: > > > On Thu, Aug 13, 2009 at 3:39 PM, Markus <[email protected]> wrote: > > -1 > > Other branches of this case statement run the value through > munge_name() > before storing it in @pattern, and that function splits on ".", > resulting in an array. Thus the proposed change would leave @pattern > sometimes a String and sometimes an Array thus (I suspect) just moving > the problem around rather than fixing it. > > I agree in general that what is there is goofy/wrong, but I'm not sold > that this is an adequate fix. > > I'm not sure either. That's why I sent it to the list :) > > The thing is that opaque strings *are* different to IP addresses or > hostnames, which are being decomposed into their components > precisely so that we can have partial matching on some components. > > This simply isn't possible with an opaque string. > > The equality test is simply never going to work with an array and a > string, which leads me to think that is there entirely for opaque > strings and (perhaps?) explicit auth settings with no regexp/ > backreferences. (and it will fail in both cases) > > The .zip method is always being used as is. > > Although I'm not convinced, I can see some merit in demanding that > pattern is always an array, so I see these options: > > a) the patch as is > b) keep pattern an array for opaque strings, modify matchname? to > convert pattern to a string for equality testing *if* the value is > opaque. > c) keep pattern an array for opaque strings, modify matchname? to > convert pattern to a string for equality testing in all cases before > falling through to pattern.zip > > I still really don't see any reason why matchname? needs to call > munge_name on non-opaque values given that they've all already run > through it already ....
I am pretty sure the 'match?' is being called from outside, so it's really 'does this rule match the provided name/ip combo?'. An overall refactor has been mentioned multiple times, but what would that refactor look like? It seems to me we just need our matchname? method to be a bit more state-based - match one way when opaque and another when not. This could be done pretty easily with a branch to two separate methods, couldn't it? That seems like an appropriate action to me. -- Every great advance in natural knowledge has involved the absolute rejection of authority. --Thomas H. Huxley --------------------------------------------------------------------- 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 -~----------~----~----~----~------~----~------~--~---
