On 2.9.2016 04:19, Ben Lipton wrote: > On 07/27/2016 02:42 PM, Ben Lipton wrote: >> On 07/21/2016 11:43 AM, Petr Spacek wrote: >>> Besides this nit, >>> http://www.freeipa.org/page/V4/Automatic_Certificate_Request_Generation/Mapping_Rules#Planned_implementation >>> >>> sounds reasonable. I like how it prevents bad data from template-injection. >> >> That's what I like about it, too. It does turn out to make things a little >> tricky when it comes to writing rules that won't render if the data they >> depend on is unavailable. (Because instead of rendering individual rules >> which we can drop if they're missing data, we build one big template that >> has to handle missing data correctly on its own.) I think it's probably >> still worth it, though. I added this to the "Alternatives considered" >> section of the above document. > > By the way, I just wrote a followup blog post on this subject: describing the > challenges I've had with suppressing rules when the data isn't available, and > wondering if it's worth it. The post is here: > http://blog.benjaminlipton.com/2016/09/01/rule-suppression.html. It might be a > bit of a dense read, but I wanted to have the considerations documented at > least. As always, please let me know if there's anything I can clarify. And if > you do happen to read it and it makes you prefer one solution over the others, > I'd love to hear your opinion.
Hello Ben, my comments are in-line (text copied from the blog post): > Conclusions > > The current implementation is working ok, but the “Declaring data > dependencies” solution is also appealing. Recording in data rules what data > they depend on is only slightly more involved than wrapping that reference in > ipa.datafield(), and could also be useful for other purposes. I agree that syntax with explicit "if"s is little bit more elaborate. On the other hand, the explicit condition is easier to read (for me) because I can see what it is doing directly - I do not remember meaning of magical IPA macros. I.e. I like version with explicit "if"s more. > Plus, it would get rid of the empty sections in openssl configs, as well as some of the complex macros. +1 > The extra templating and new tags required to get rid of extra commas and > newlines don’t seem worth it to me, unless we discover a version of openssl > or certutil that can’t consume the current output. I definitely agree. > Finally, I think the number of hoops needing to be jumped through to > fine-tune the output format hint at this “template interpolation” approach > being less successful than originally expected. While it was expected that > inserting data rule templates into syntax rule templates and rendering the > whole thing would produce similar results to rendering data rules first and > inserting the output into syntax rules, that is not turning out to be the > case. It might be wise to reconsider the simpler option - it may be easier to > implement reliable jinja2 template markup escaping than to build templates > smart enough to handle any combination of data that’s available. This is certainly something to think about. Personally I think that version with explicit "if"s is easy to understandad, write, and also it has no risk of data injection (AFAIK). Explicit escaping is usually very error prone... but I'm not in position to judge how user-friendly it would be when compared with other solutions. After all, goal of the feature is to make life of an average admin easier :-) I hope this brain-dump will help you somehow :-) Have a nice day! -- Petr^2 Spacek P.S. I will not be available in next two weeks, sorry! -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code