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
