Anshu, et al., I updated https://blueprints.launchpad.net/drizzle/+spec/plugin-standards and moved the spec to http://wiki.drizzle.org/Plugin_Standards because the wiki is easier to edit and has revisions. For example: who added the info on using the SYS table as a plugin data store?
I simplified the spec and specified some new things. I think this spec is important because we're about to start making all plugins dynamic, so we need a consistent approach for this. Please read it over and let me know what you think. Thanks, Daniel Le 27 avr. 2012 à 23:56, Daniel Nichter a écrit : > Anshu, > > My reply is quite late, but the conference and Drizzle Day were both > enlightening. I look forward to next year. > > As for updating files, I think there's a simpler, more direct approach: > > SET GLOBAL regex_policy_policy=@@regex_policy_policy; > > That sets the var to its current value which triggers the plugin's update > hook and since the new value == the old value, the result is simply to reload > the file. > > This avoid having two code paths: one for SET GLOBAL > regex_policy_policy="new-file" and another for SET GLOBAL > regex_policy_reload=1 (which would eventually call the same code as the first > statement). > > Thoughts on this (you and anyone else)? > > -Daniel > > Le 17 avr. 2012 à 15:09, Anshu Kumar a écrit : > >> Hi Daniel, >> How was the conference and drizzle day? >> Coming to the point, after brainstorming the possibilities that I have in >> the issue of making regex policy plugin dynamic, I think that the solution I >> proposed in my last mail is best for now. And using that we wont need any >> issue handling with if the policy file was not correct or change in the look >> and feel of some specific plugins. From coding point of view, we can do this >> by adding an option of reload variable which will default be set to false. >> Upon making it true, it will trigger a function which will create a separate >> instance of policy and will call loadfile on it. If the call is a success, >> we will remove all old policies and reload the system with new ones. And if >> the call gives some error message then do nothing. After both the cases, the >> function will change the reload variable to false. This way we wont need any >> separate threads also. >> Waiting for your review comments to start working on this. >> >> On Wed, Apr 4, 2012 at 2:55 PM, Anshu Kumar <ansharyan...@gmail.com> wrote: >> Hey Daniel, >> After reading your views, I too think now that auto reloading is not the >> ideal solution to this scenario. In this solution I have used the idea of >> thread for reloading the policy files. You put a point that what if admin is >> using a ext editor which auto saves the changes after some fixed time. In >> that case incorrect policies may be read. >> >> For this issue, instead of using a thread, we can come up with a solution in >> which the reloading won't be dynamic. So it would be like, if you are an >> admin and you want to change the policy file, do it. After all changes are >> done, just change the value of regex_policy_reload to true and policies will >> then be reloaded. After policies are reloaded, the value of >> regex_policy_reload will again be set to false from code itself. In this >> way, it can be ensured that only the correct policies are loaded there. It >> is true that here too anyone can change the status of regex_policy_reload to >> true, so either don't let him access the policy file or we will be needing >> an authorization system for changing global variables too. >> >> Brainstorming further for the most optimal solution. Comment and suggestions >> are welcome. >> >> About the name concern, I use Ansh as my nick. Both are correct. You can >> call me anything you like. However changing my sign to 'Anshu', so that it >> don't cause any further confusion to anyone. :) >> >> On Wed, Apr 4, 2012 at 1:40 PM, Henrik Ingo <henrik.i...@avoinelama.fi> >> wrote: >> Daniel >> >> FWIW, I think you might be correct. I originally thought that auto-reloading >> would be an easy way to implement a way to re-read the policy file. For >> instance, it could be implemented crudely without any new configuration >> options and no new threads needing to launch. However, as we've gone deeper >> into the subject it became clear that a good implementation will need those >> things, hence the "easy" attribute is no longer there. I support going back >> to explore your original idea. >> >> henrik >> >> >> On Wed, Apr 4, 2012 at 8:12 AM, Daniel Nichter <dan...@percona.com> wrote: >> Anshu, Henrik, Clint, >> >> First: Anshu: I like to use people's names correctly, so I noticed you sign >> your emails "Ansh" but we've been writing "Anshu". Is one or the other, or >> both, correct? >> >> Now on to business. First, thanks for the code. This is a good start and >> I'm happy to see that you've been able to jump into the code with apparent >> ease. >> >> Second, I disagree that the auto-reloading approach is the ideal solution. >> I think we all need to debate the merits of this approach further. So let's >> do that... >> >> In my humble opinion, this approach has the following drawbacks: >> >> 1) It's "gold plating" because in the real world the policy file >> won't/shouldn't change so frequently as to make autp-reloading a valuable >> feature. It's not too much to ask the user to execute one simple command, >> and chances are they will expect to do this. Thinking of Drizzle and MySQL, >> no auto-(re)loading features come immediately to my mind, so we won't be >> depriving the user of functionality they're used to. Of course, I'm all for >> new types of functionality when it's clear that the functionality will be a >> "big win" for users, but given the nature of this issue, I don't think >> auto-reloading is a big win and certainly not worth the extra engineering >> effort. >> >> 2) It's a bit of magic and magic usually leads to problems down the road. >> Sure, it's just one little thread that sleeps, checks, and maybe does >> something, but one immediate "problem" comes to mind: testing. Anything >> dependent on time or time-based is inherently more difficult to test. It's >> possible, of course, but (related to #1) is it worth the effort? Another >> problem comes to mind: what if the admin is editing the policy file and his >> editor auto-saves every N minutes, and auto-reload is on? Drizzle may read >> the policy file before the admin intends. Magic can be very difficult to >> wield safely. >> >> 3) It doesn't address the original security concern. Henrik raised the >> problem that anyone could execute the command to reload the policy file, but >> anyone could enable the auto-reload, too. The real solution lies in the >> authorization modules. Right now Drizzle has only very basic authorization: >> schema and table access (via this plugin nonetheless). What Drizzle needs >> is authorization for setting/changing global variables. I think we can make >> still make this dynamic and leave the authorization solution for another >> time/person/project. >> >> 4) It introduces a concept that will either become a norm or become an >> exception. Given the aforementioned points, I wouldn't like to see >> auto-reloading before a norm, and we certainly don't want more exceptions >> because one of our high-level goals is to make the "look and feel" of all >> plugins consistent. So if we do this here, then why not >> --auth-file.auto-reload too? But then we're neck deep in gold plating and >> magic. >> >> 5) It seems contrary to "the Drizzle way". Brian, Stewart, and the original >> core developers have more of a sense of what the Drizzle way is, but I'm >> pretty certain is centers on removing the extraneous and the gotchas. >> Drizzle prides itself it what it doesn't have: no views, no triggers, no >> timezones, etc. I think auto-reloading is extraneous (gold plating) and >> prone to becoming a gotcha. To modify my previous example: what if the >> senior admin enables auto-reloading but forgets to tell the junior admin who >> changes the policy file on Friday and plans to review the changes with the >> senior admin on Monday only for both of them to get a call during the >> weekend by the CTO because Sally in HR can't access the employees database >> and she's trying to do payroll so now everyone's paychecks will be late? >> Gotcha. >> >> So, in conclusion: your code is a good start, but I think we need another >> solution. Other opinions? >> >> -Daniel >> >> >> Le 3 avr. 2012 à 18:03, Anshu Kumar a écrit : >> >>> Hi Daniel, >>> For making all the plugins dynamic, as the earlier discussion for auth_file >>> could not come to a conclusion, I started off with making the regex_policy >>> plugin dynamic. Here is the complete process which am proposing for this >>> plugin >>> >>> 1. There would be a autoreload variable, default set to false. This >>> variable will determine if policy file needs to be reloaded. >>> >>> 2. Upon setting this variable to true (SET GLOBAL >>> regex_policy_autoreload=ON), a new thread will be created which will handle >>> the reload issues of policy file. >>> >>> 3. In the thread created, on every one minute, using the stat command to >>> find the last modified time and taking the difference from current time, it >>> will be checked that if the file was modified in last one minute. And if it >>> was, reload the file. If it wasn't modified, continue to the next poll. >>> >>> 4. This thing will go on till you again set the variable value to false >>> (SET GLOBAL regex_policy_autoreload=OFF) >>> >>> Scenario would be, if you want to change the policy file and want those >>> changes to be reflected, just change the variable to true, do your >>> modifications, and the changes will be reflected the next minute in the >>> system. >>> >>> I have implemented this whole thing. Here is the link of the branch. It >>> would be great if you can go through it and give your comments. >>> https://code.launchpad.net/~ansharyan015/drizzle/dynamic_regex_policy >>> >>> P.S. Thanks to Henrik for being the continuous help. :) >>> Comments and suggestions are welcome. >>> >>> >>> On Mon, Apr 2, 2012 at 11:01 PM, Henrik Ingo <henrik.i...@avoinelama.fi> >>> wrote: >>> Anshu: To create a background thread, you need to use class >>> drizzled::plugin::Daemon >>> >>> Look at plugin/json_server/json_server.cc (class JsonServer at the >>> end) for an example. >>> >>> henrik >>> >>> On Mon, Apr 2, 2012 at 4:16 PM, Henrik Ingo <henrik.i...@avoinelama.fi> >>> wrote: >>> > Anshu >>> > >>> > You are right. Creating a thread is more correct. I was just trying to >>> > avoid it since you originally asked for "low-hanging-fruit" bug. >>> > >>> > Functionally, ignoring the performance hit, you could do this also >>> > from the restrict* methods and then you don't need to create your own >>> > thread. >>> > >>> > henrik >>> > >>> > On Mon, Apr 2, 2012 at 3:45 PM, Anshu Kumar <ansharyan...@gmail.com> >>> > wrote: >>> >> Hey Henrik, >>> >> What I was thinking is we can have a system like this. When autoreload >>> >> variable is set to true, we can create a thread which will check either >>> >> by >>> >> inotify or stat() that the corresponding regex policy file is changed or >>> >> not. And it of does then only reload the file. This is just like >>> >> creating a >>> >> handler for change in policy file. The performance will be better than >>> >> polling for a min and reloading the policy file. >>> >> The implementation is actually the same in this, and the way you >>> >> suggested >>> >> by Policy::restrict methods. So did you want to say that when checking >>> >> for >>> >> regex policy rights (if access is allowed or denied), we can check if >>> >> file >>> >> needs to be refreshed? >>> >> >>> >> On Mon, Apr 2, 2012 at 6:02 PM, Henrik Ingo <henrik.i...@avoinelama.fi> >>> >> wrote: >>> >>> >>> >>> Anshu >>> >>> >>> >>> Actually, now that I re-read this, I don't know if it was a smart idea >>> >>> and maybe there needs to be a thread that does the polling, but my >>> >>> original idea was that you could reload the file during an >>> >>> authorization request. So basically when any of the >>> >>> Polixy::restrict... methods are called, you would first check if the >>> >>> file needs to be refreshed and then re-read it (or not). This way you >>> >>> don't need to have a loop or do any polling or such. >>> >>> >>> >>> This approach is not good because it would introduce a performance hit >>> >>> into random requests every N seconds. But you could still do it this >>> >>> way as a proof of concept, then we could work on making it a >>> >>> background thread later. >>> >>> >>> >>> henrik >>> >>> >>> >>> On Mon, Apr 2, 2012 at 2:51 PM, Anshu Kumar <ansharyan...@gmail.com> >>> >>> wrote: >>> >>> > Hey Guys, >>> >>> > I have tried to implement this dynamic thing to regex_policy plugin. >>> >>> > In reference to my talk with Henrik yesterday, we discussed that there >>> >>> > could >>> >>> > be a autoreload variable, and when made true it will continuously poll >>> >>> > for >>> >>> > changes in default regex policy file. And it it is made false, it will >>> >>> > stop >>> >>> > polling, checking for modifications. Sticking to the discussion, the >>> >>> > code I >>> >>> > wrote is >>> >>> > >>> >>> > void autoReload_Regex_Policy(Session *, sql_var_t) >>> >>> > { >>> >>> > if(policy->sysvar_autoreload) >>> >>> > { >>> >>> > while(1) >>> >>> > { >>> >>> > if (not policy->loadFile()) >>> >>> > { >>> >>> > errmsg_printf(error::ERROR, _("Could not >>> >>> > load >>> >>> > regex policy file: %s\n"), >>> >>> > (policy ? >>> >>> > policy->getError().str().c_str() : _("Unknown"))); >>> >>> > return; >>> >>> > } >>> >>> > sleep(60); >>> >>> > if(!policy->sysvar_autoreload) >>> >>> > break; >>> >>> > } >>> >>> > } >>> >>> > else >>> >>> > { >>> >>> > } >>> >>> > } >>> >>> > This function handles the case when you change the value of autoreload >>> >>> > from >>> >>> > drizzle client. The problem is as the function is using sleep >>> >>> > recursively, >>> >>> > when trying to change autoreload value by "SET GLOBAL", the client >>> >>> > hangs. >>> >>> > This is obviously due to function recursive structure. Initially, I >>> >>> > thought >>> >>> > that it would change the variable and then polls. >>> >>> > >>> >>> > Now, coming to the discussion earlier in this mail, even if I create a >>> >>> > pthread from this function which will check for modification using >>> >>> > stat() or >>> >>> > inotify(), this function won't exit untill its thread stop working. >>> >>> > And >>> >>> > as >>> >>> > thread will continuously polls for changes, it wont exit. >>> >>> > >>> >>> > Is there any solution for this scenario, except adding the refresh >>> >>> > command >>> >>> > for refreshing the policy file? >>> >>> > >>> >>> > >>> >>> > On Fri, Mar 30, 2012 at 10:34 PM, Clint Byrum <cl...@fewbar.com> >>> >>> > wrote: >>> >>> >> >>> >>> >> Excerpts from Henrik Ingo's message of Thu Mar 29 21:16:26 -0700 >>> >>> >> 2012: >>> >>> >> > Daniel: >>> >>> >> > >>> >>> >> > Have you thought about authorization for this? I mean we wouldn't >>> >>> >> > want >>> >>> >> > any old logged in user to be able to >>> >>> >> > >>> >>> >> > SET GLOBAL auth_file.users=/home/hingo/igivemyselfrootpowers.users >>> >>> >> > >>> >>> >> > (Making the plugin reload the existing file will be helpful. But it >>> >>> >> > might not be a good idea to allow to change that value.) >>> >>> >> > >>> >>> >> >>> >>> >> Agreed. I'd like to see plugins like auth_file and regex_policy given >>> >>> >> a generic way to "watch" their files. There are a number of ways to >>> >>> >> do >>> >>> >> this, but I don't think each plugin should implement its own method. >>> >>> >> >>> >>> >> Thoughts I've had on this: >>> >>> >> >>> >>> >> * A thread which uses either inotify or falls back to polling >>> >>> >> with stat(), and whenever there is a change, calls any registered >>> >>> >> code >>> >>> >> to update that file's effect. >>> >>> >> >>> >>> >> * An admin command like REFRESH '/etc/drizzle/regex.policy' which >>> >>> >> does >>> >>> >> the same thing as the thread without the inotify/polling. >>> >>> >> >>> >>> >> * Cache the stat() call on the file and periodically expire the cache >>> >>> >> and refresh the contents if stat() indicates that it has changed. >> >> >> >> -- >> henrik.i...@avoinelama.fi >> +358-40-8211286 skype: henrik.ingo irc: hingo >> www.openlife.cc >> >> My LinkedIn profile: http://www.linkedin.com/profile/view?id=9522559 >> >> >> >> -- >> Regards, >> Anshu >> >> >> >> >> -- >> Regards, >> Anshu >> > > _______________________________________________ > Mailing list: https://launchpad.net/~drizzle-discuss > Post to : drizzle-discuss@lists.launchpad.net > Unsubscribe : https://launchpad.net/~drizzle-discuss > More help : https://help.launchpad.net/ListHelp
_______________________________________________ Mailing list: https://launchpad.net/~drizzle-discuss Post to : drizzle-discuss@lists.launchpad.net Unsubscribe : https://launchpad.net/~drizzle-discuss More help : https://help.launchpad.net/ListHelp
