Moodle
  1. Moodle
  2. MDL-35238

Automatic plugin update deployment

    Details

    • Testing Instructions:
      Hide

      Testing difficulty: HARD (requires fake server response)

      1. Upload the attached fake.php into your wwwroot and add the following line into your config.php
        $CFG->alternativeupdateproviderurl = $CFG->wwwroot.'/fake.php';
        

        This will switch the Available updates notification feature so that it reads data from the file instead of the standard download.moodle.org REST API. You may want to modify the fake.php according the plugin you install in the next step.

      2. Install some outdated version of a plugin. This is tricky because there are no official plugins for 2.4 yet. So we use 2.3 plugins and we prepare the information in fake.php in a way that simulates an update for the 2.4. During the development, the Stamp collection module has been used. The plugin version 2.3.0 has been installed manually and the fake.php provides information about a newer version 2.3.1 available.
      3. There are three places where the new version of the plugin can be installed: 1) from the Plugins overview page, 2) from the Plugins check screen when Moodle itself is about to be upgraded and 3) from the Plugins check screen when some plugin is about to be upgraded (while the Moodle itself not). You may want to test all these three places.
      4. Press the "Check for available updates" button. That will read info from fake.php and should inform you about a new version of the plugin available.
      5. Press "Install this update" button.
      6. Make sure that a new version of the plugin is deployed and can be upgraded.

      Together with this feature, there are small usability improvements at the Plugins overview page. Plugins can be now filtered so you can see eg updateable plugins only etc. You may wish to test these improvements, too.

      Show
      Testing difficulty: HARD (requires fake server response) Upload the attached fake.php into your wwwroot and add the following line into your config.php $CFG->alternativeupdateproviderurl = $CFG->wwwroot.'/fake.php'; This will switch the Available updates notification feature so that it reads data from the file instead of the standard download.moodle.org REST API. You may want to modify the fake.php according the plugin you install in the next step. Install some outdated version of a plugin. This is tricky because there are no official plugins for 2.4 yet. So we use 2.3 plugins and we prepare the information in fake.php in a way that simulates an update for the 2.4. During the development, the Stamp collection module has been used. The plugin version 2.3.0 has been installed manually and the fake.php provides information about a newer version 2.3.1 available. There are three places where the new version of the plugin can be installed: 1) from the Plugins overview page, 2) from the Plugins check screen when Moodle itself is about to be upgraded and 3) from the Plugins check screen when some plugin is about to be upgraded (while the Moodle itself not). You may want to test all these three places. Press the "Check for available updates" button. That will read info from fake.php and should inform you about a new version of the plugin available. Press "Install this update" button. Make sure that a new version of the plugin is deployed and can be upgraded. Together with this feature, there are small usability improvements at the Plugins overview page. Plugins can be now filtered so you can see eg updateable plugins only etc. You may wish to test these improvements, too.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-35238-deployment
    • Rank:
      43887

      Description

      Since Moodle 2.3, admins are notified of available updates for the contributed plugins installed on their sites. It is now requested that these new plugin versions can be also downloaded and deployed automatically by Moodle itself.

      1. fake.php
        3 kB
        David Mudrak
      1. confirmation.png
        39 kB
      2. notwritable.png
        28 kB
      3. screenshot-2.png
        47 kB
      4. screenshot-3.png
        27 kB

        Issue Links

          Activity

          Hide
          David Mudrak added a comment -

          (09:12:49) mudrd8mz@jabber.cz/glum: Hi Dan
          (09:12:57) Dan: hey
          (09:13:55) mudrd8mz@jabber.cz/glum: I must admit I am sort of stuck with the Deployment project. So I decided to follow the "How to eat the elephant" method - piece after piece.
          (09:14:42) Dan: I am not surprised.. it seems a very tough problem
          (09:14:50) mudrd8mz@jabber.cz/glum: There was one conceptual question still raising to me
          (09:15:10) mudrd8mz@jabber.cz/glum: should the fetching and deploying code be independent on the rest of Moodle or not
          (09:15:15) mudrd8mz@jabber.cz/glum: there are pros and cons
          (09:15:24) mudrd8mz@jabber.cz/glum: my current idea is
          (09:17:09) mudrd8mz@jabber.cz/glum: 1) when a plugin is being updated from /admin/plugins.php (that is when the core itself is not a subject of update), it is safe to use standard Moodle libs, respect proxy setting in $CFG etc
          2) when a plugin is being updated during the core upgrade, an independent implementation of fetching, unzipping etc would be used.
          (09:17:45) mudrd8mz@jabber.cz/glum: Although I'm not sure if it worths it
          (09:17:58) Dan: hmm, interesting problem
          (09:18:14) Dan: (or should I say hard)
          (09:18:48) Dan: if we get to the core thing, then we are going to need something completely indepdenent anyway, right?
          (09:18:56) mudrd8mz@jabber.cz/glum: Yes
          (09:18:59) Dan: Or could we depend on the previous version
          (09:19:11) Dan: and have something that gets the updates, and then have an indepdenent installer
          (09:19:17) mudrd8mz@jabber.cz/glum: I would not do it.
          (09:19:37) mudrd8mz@jabber.cz/glum: Note that installer itself is out of scope - we just need to deploy PHP files and redirect to /admin
          (09:21:40) mudrd8mz@jabber.cz/glum: On one side, it seemed that re-writing independent fetching and unzipping is a lot of useless work. On the other hand, while standard libs are universal (in terms of fetching from any source, unzipping any ZIP etc), here we have pretty limited and well defined environment. We know, for example, how ZIPs were created which makes unzipping really simple.
          (09:21:54) Dan: yeah
          (09:21:59) Dan: I was just thinking that
          (09:22:24) mudrd8mz@jabber.cz/glum: Actually, I have like one screen long script that does it
          (09:22:32) Dan: doing a http request surely isn't that big, even with proxy support
          (09:22:52) Dan: and then the unziping etc similar
          (09:22:58) mudrd8mz@jabber.cz/glum: Yes.
          (09:23:22) mudrd8mz@jabber.cz/glum: So, to conclude, I tend to use independent fetch+unzipper only.
          (09:23:36) Dan: makes sense to me
          (09:23:49) mudrd8mz@jabber.cz/glum: Then there is this Manifest thing.
          (09:25:04) mudrd8mz@jabber.cz/glum: I started to ask, what will we do at the Day 0 (the first plugin update ever on the site)
          (09:27:31) mudrd8mz@jabber.cz/glum: Just a message "Unable to check and compare PHP files contents, continue at your own risk" sounds like the only option. Then replace the plugin dir with the new version, calculate MD5 hash of every file in it and store it (where?). Next time this plugin is updated, we can already check for local modifications.
          (09:28:55) Dan: Hmm, well I was thinking that /plugins/ could provide a manfiest
          (09:29:28) Dan: shipped with the plugin
          (09:29:38) Dan: i've not really thought about this at all
          (09:30:12) mudrd8mz@jabber.cz/glum: Right. And what would be such Manifest good for?
          (09:30:39) Dan: this is what i'm trying to understand in my head now
          (09:31:03) Dan: I guess my thought was that it was shipped with manifest and then you verify agains tthe shipped manifest, but whats the point
          (09:31:24) Dan: what is our goal here, heh
          (09:31:43) mudrd8mz@jabber.cz/glum: We already know that the ZIP is an update against what we have installed.
          (09:31:51) mudrd8mz@jabber.cz/glum: So we know we want to replace it.
          (09:32:21) mudrd8mz@jabber.cz/glum: The goal (as I understood it) was how to prevent admins from overwriting their local modification.
          (09:33:01) Dan: yes, I agree with your day 0 suggestion
          (09:33:11) mudrd8mz@jabber.cz/glum: I can't see much what added value would the shipped Manifest file provide.
          (09:34:04) Dan: nope
          (09:34:41) Dan: btw whilst I think of it, we really should get ssl on the download site and make sure to ensure valid ssl connection
          (09:34:57) mudrd8mz@jabber.cz/glum: Yes, but that's tricky.
          (09:35:09) mudrd8mz@jabber.cz/glum: It's MNet, you know
          (09:35:23) mudrd8mz@jabber.cz/glum: *it would be MNet then, you know
          (09:36:06) mudrd8mz@jabber.cz/glum: Unless we get an SSL certificate that we know is known to 95% of our sites.
          (09:36:06) Dan: huh? We control the download site, so easy for us to have a proper ssled download site
          (09:36:18) Dan: yes, surely we can do that?
          (09:36:36) mudrd8mz@jabber.cz/glum: Well yes, I guess so.
          (09:36:38) Dan: whats what various integrations depend on
          (09:36:42) Dan: google etc
          (09:37:13) mudrd8mz@jabber.cz/glum: Hmm. But does SSL protect against man-in-the-middle attack? I don't think so.
          (09:37:37) mudrd8mz@jabber.cz/glum: Hmm, it does.
          (09:37:43) Dan: yes it does, unless you have control of the certificate authories on the server
          (09:37:54) ***mudrd8mz@jabber.cz/glum notes he is hmmming to much
          (09:38:20) mudrd8mz@jabber.cz/glum: Right. SSL is the way to go. Going to create the subtask.
          (09:38:20) Dan: its the right solution for mnet replacement.. except that we can't depend on people being able to setup ssl themselves
          (09:42:54) mudrd8mz@jabber.cz/glum: OK.
          (10:00:51) mudrd8mz@jabber.cz/glum: Are you OK with me putting the transcript of this chat into the tracker for later reference.
          (10:00:58) Dan: of course

          Show
          David Mudrak added a comment - (09:12:49) mudrd8mz@jabber.cz/glum: Hi Dan (09:12:57) Dan: hey (09:13:55) mudrd8mz@jabber.cz/glum: I must admit I am sort of stuck with the Deployment project. So I decided to follow the "How to eat the elephant" method - piece after piece. (09:14:42) Dan: I am not surprised.. it seems a very tough problem (09:14:50) mudrd8mz@jabber.cz/glum: There was one conceptual question still raising to me (09:15:10) mudrd8mz@jabber.cz/glum: should the fetching and deploying code be independent on the rest of Moodle or not (09:15:15) mudrd8mz@jabber.cz/glum: there are pros and cons (09:15:24) mudrd8mz@jabber.cz/glum: my current idea is (09:17:09) mudrd8mz@jabber.cz/glum: 1) when a plugin is being updated from /admin/plugins.php (that is when the core itself is not a subject of update), it is safe to use standard Moodle libs, respect proxy setting in $CFG etc 2) when a plugin is being updated during the core upgrade, an independent implementation of fetching, unzipping etc would be used. (09:17:45) mudrd8mz@jabber.cz/glum: Although I'm not sure if it worths it (09:17:58) Dan: hmm, interesting problem (09:18:14) Dan: (or should I say hard) (09:18:48) Dan: if we get to the core thing, then we are going to need something completely indepdenent anyway, right? (09:18:56) mudrd8mz@jabber.cz/glum: Yes (09:18:59) Dan: Or could we depend on the previous version (09:19:11) Dan: and have something that gets the updates, and then have an indepdenent installer (09:19:17) mudrd8mz@jabber.cz/glum: I would not do it. (09:19:37) mudrd8mz@jabber.cz/glum: Note that installer itself is out of scope - we just need to deploy PHP files and redirect to /admin (09:21:40) mudrd8mz@jabber.cz/glum: On one side, it seemed that re-writing independent fetching and unzipping is a lot of useless work. On the other hand, while standard libs are universal (in terms of fetching from any source, unzipping any ZIP etc), here we have pretty limited and well defined environment. We know, for example, how ZIPs were created which makes unzipping really simple. (09:21:54) Dan: yeah (09:21:59) Dan: I was just thinking that (09:22:24) mudrd8mz@jabber.cz/glum: Actually, I have like one screen long script that does it (09:22:32) Dan: doing a http request surely isn't that big, even with proxy support (09:22:52) Dan: and then the unziping etc similar (09:22:58) mudrd8mz@jabber.cz/glum: Yes. (09:23:22) mudrd8mz@jabber.cz/glum: So, to conclude, I tend to use independent fetch+unzipper only. (09:23:36) Dan: makes sense to me (09:23:49) mudrd8mz@jabber.cz/glum: Then there is this Manifest thing. (09:25:04) mudrd8mz@jabber.cz/glum: I started to ask, what will we do at the Day 0 (the first plugin update ever on the site) (09:27:31) mudrd8mz@jabber.cz/glum: Just a message "Unable to check and compare PHP files contents, continue at your own risk" sounds like the only option. Then replace the plugin dir with the new version, calculate MD5 hash of every file in it and store it (where?). Next time this plugin is updated, we can already check for local modifications. (09:28:55) Dan: Hmm, well I was thinking that /plugins/ could provide a manfiest (09:29:28) Dan: shipped with the plugin (09:29:38) Dan: i've not really thought about this at all (09:30:12) mudrd8mz@jabber.cz/glum: Right. And what would be such Manifest good for? (09:30:39) Dan: this is what i'm trying to understand in my head now (09:31:03) Dan: I guess my thought was that it was shipped with manifest and then you verify agains tthe shipped manifest, but whats the point (09:31:24) Dan: what is our goal here, heh (09:31:43) mudrd8mz@jabber.cz/glum: We already know that the ZIP is an update against what we have installed. (09:31:51) mudrd8mz@jabber.cz/glum: So we know we want to replace it. (09:32:21) mudrd8mz@jabber.cz/glum: The goal (as I understood it) was how to prevent admins from overwriting their local modification. (09:33:01) Dan: yes, I agree with your day 0 suggestion (09:33:11) mudrd8mz@jabber.cz/glum: I can't see much what added value would the shipped Manifest file provide. (09:34:04) Dan: nope (09:34:41) Dan: btw whilst I think of it, we really should get ssl on the download site and make sure to ensure valid ssl connection (09:34:57) mudrd8mz@jabber.cz/glum: Yes, but that's tricky. (09:35:09) mudrd8mz@jabber.cz/glum: It's MNet, you know (09:35:23) mudrd8mz@jabber.cz/glum: *it would be MNet then, you know (09:36:06) mudrd8mz@jabber.cz/glum: Unless we get an SSL certificate that we know is known to 95% of our sites. (09:36:06) Dan: huh? We control the download site, so easy for us to have a proper ssled download site (09:36:18) Dan: yes, surely we can do that? (09:36:36) mudrd8mz@jabber.cz/glum: Well yes, I guess so. (09:36:38) Dan: whats what various integrations depend on (09:36:42) Dan: google etc (09:37:13) mudrd8mz@jabber.cz/glum: Hmm. But does SSL protect against man-in-the-middle attack? I don't think so. (09:37:37) mudrd8mz@jabber.cz/glum: Hmm, it does. (09:37:43) Dan: yes it does, unless you have control of the certificate authories on the server (09:37:54) ***mudrd8mz@jabber.cz/glum notes he is hmmming to much (09:38:20) mudrd8mz@jabber.cz/glum: Right. SSL is the way to go. Going to create the subtask. (09:38:20) Dan: its the right solution for mnet replacement.. except that we can't depend on people being able to setup ssl themselves (09:42:54) mudrd8mz@jabber.cz/glum: OK. (10:00:51) mudrd8mz@jabber.cz/glum: Are you OK with me putting the transcript of this chat into the tracker for later reference. (10:00:58) Dan: of course
          Hide
          David Mudrak added a comment -

          Example of the Plugins overview page with an available update for a contributed module.

          Show
          David Mudrak added a comment - Example of the Plugins overview page with an available update for a contributed module.
          Hide
          Aparup Banerjee added a comment -

          not sure if its been discussed but :

          i had just typed a post here asking if we're using ws (with some credential/ip<->download logging) as its good for analytics.. till i had to login .. then i saw the request lol. good to know its via plugins db will look on monday. Thanks guys

          ps: download.moodle.org/plugins is for ?

          ah i just had a glance thru the ws. it seems we don't pass in any data for logging purposes whereas the browser downloading one can. At this point i'm not sure but hmmm i have to check local_plugins code to see its logging from the download url being served or not.

          looking at local_plugins/download.php theres no logging ?
          anyway the served url can be adjusted to make it start logged if needed.

          in short, i think its good to collect this download data @ local_plugins. (or even url requests - to separate plugin upgraders from non). and i'm not sure its being done so will see soon.

          ciao

          Show
          Aparup Banerjee added a comment - not sure if its been discussed but : i had just typed a post here asking if we're using ws (with some credential/ip<->download logging) as its good for analytics.. till i had to login .. then i saw the request lol. good to know its via plugins db will look on monday. Thanks guys ps: download.moodle.org/plugins is for ? ah i just had a glance thru the ws. it seems we don't pass in any data for logging purposes whereas the browser downloading one can. At this point i'm not sure but hmmm i have to check local_plugins code to see its logging from the download url being served or not. looking at local_plugins/download.php theres no logging ? anyway the served url can be adjusted to make it start logged if needed. in short, i think its good to collect this download data @ local_plugins. (or even url requests - to separate plugin upgraders from non). and i'm not sure its being done so will see soon. ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi Apu,

          no, we don't perform any logging of requests to that (local_plugins) WS. Everything is (and will always) handled anonymously along the whole updates API.

          Once this issue and other friends meet production... surely we can imagine some ways to bring usage stats back to the local_plugins database and other reports, always aggregates, no way (by design) to get individual information.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Hi Apu, no, we don't perform any logging of requests to that (local_plugins) WS. Everything is (and will always) handled anonymously along the whole updates API. Once this issue and other friends meet production... surely we can imagine some ways to bring usage stats back to the local_plugins database and other reports, always aggregates, no way (by design) to get individual information. Ciao
          Hide
          Aparup Banerjee added a comment -

          Thanks for the info Eloy. I've merged that into upstream and its ready for updating at moodle.org/plugins.

          Matt: We need to update moodle.org/plugins to the latest code here.

          Show
          Aparup Banerjee added a comment - Thanks for the info Eloy. I've merged that into upstream and its ready for updating at moodle.org/plugins. Matt: We need to update moodle.org/plugins to the latest code here.
          Hide
          Matt Sharpe (Inactive) added a comment -

          Done, let me know if anything further is required.

          Show
          Matt Sharpe (Inactive) added a comment - Done, let me know if anything further is required.
          Hide
          Aparup Banerjee added a comment -

          Hm, not seeing the update effects (downloadmd5).

          Show
          Aparup Banerjee added a comment - Hm, not seeing the update effects (downloadmd5).
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi Apu,

          since some hours ago, the WS call to moodle.org is returning error. And that causes the information about available plugins not to being refreshed at all (the updates API continues working with previous information).

          After some debugging I've been able to trace down the problem (you should have received one email about it).

          Basically is that the pluginversion_form.php does not validate the value introduced in the 'version' field at all (PARAM_TEXT right now, surely should check for YYYMMDDXX[.YY]), and there are plugins with values like:

          "Moodle 1.9"
          "Moodle 2.0/2.1/2.2"

          that, clearly, lead to borked urls when that 'version' is used to construct the download url. Note the browsers are able to escape the space and somehow the urls apparently work... but they do not pass the PARAM_URL validation (that the WS uses, properly I'd say) at all.

          Take a look to http://moodle.org/plugins/pluginversions.php?plugin=mod_amvonetroom for example, and you'll see, in source html source, how bad the 'download' urls look like.

          So, while I can "relax" the WS to accept anything in the URL... I don't think it's the correct solution at all. IMO we should, instead, fix the plugins DB to, alternatively, one of these:

          A) Validate the 'version' field to be proper version (YYYMMDDXX.[YY]) and fix the current offenders.
          B) Change the behavior to calculate the 'download' url to guarantee it's impossible to produce borked URLs (spaces, slashes...).

          If I had to pick one... I think that A) is the way to go, with versions only allowed to contain exactly that, versions. And surely, escaping the resulting URL for safety (spaces).

          Although, at the same time... I think that current generated URLs are a bit imperfect too (if you base them into something "editable", like the 'version'... then later they won't help much if somebody edits manually such version (and will break existing URLs out there). Perhaps something should be invented to produce immutable URLs (one mini-hash of the version->id or whatever) or, alternatively... prevent edition of the fields involved with the URL generation.

          Surely this is one new issue to be created in the Tracker to be fixed asap, I just wanted to expose it here to know what route are you going to get, to know if I need to modify something in the WS or no (as said, I think that the WS current approach is correct, validating that URLs are PARAM_URLs).

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Hi Apu, since some hours ago, the WS call to moodle.org is returning error. And that causes the information about available plugins not to being refreshed at all (the updates API continues working with previous information). After some debugging I've been able to trace down the problem (you should have received one email about it). Basically is that the pluginversion_form.php does not validate the value introduced in the 'version' field at all (PARAM_TEXT right now, surely should check for YYYMMDDXX [.YY] ), and there are plugins with values like: "Moodle 1.9" "Moodle 2.0/2.1/2.2" that, clearly, lead to borked urls when that 'version' is used to construct the download url. Note the browsers are able to escape the space and somehow the urls apparently work... but they do not pass the PARAM_URL validation (that the WS uses, properly I'd say) at all. Take a look to http://moodle.org/plugins/pluginversions.php?plugin=mod_amvonetroom for example, and you'll see, in source html source, how bad the 'download' urls look like. So, while I can "relax" the WS to accept anything in the URL... I don't think it's the correct solution at all. IMO we should, instead, fix the plugins DB to, alternatively, one of these: A) Validate the 'version' field to be proper version (YYYMMDDXX. [YY] ) and fix the current offenders. B) Change the behavior to calculate the 'download' url to guarantee it's impossible to produce borked URLs (spaces, slashes...). If I had to pick one... I think that A) is the way to go, with versions only allowed to contain exactly that, versions. And surely, escaping the resulting URL for safety (spaces). Although, at the same time... I think that current generated URLs are a bit imperfect too (if you base them into something "editable", like the 'version'... then later they won't help much if somebody edits manually such version (and will break existing URLs out there). Perhaps something should be invented to produce immutable URLs (one mini-hash of the version->id or whatever) or, alternatively... prevent edition of the fields involved with the URL generation. Surely this is one new issue to be created in the Tracker to be fixed asap, I just wanted to expose it here to know what route are you going to get, to know if I need to modify something in the WS or no (as said, I think that the WS current approach is correct, validating that URLs are PARAM_URLs). Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Something like this seems to do its job controlling versions are correct, note the lang string is missing and that it does not fix current offenders, just validates the (edited) version:

          diff --git a/pluginversion_form.php b/pluginversion_form.php
          index eacdc8b..c3e3f0b 100644
          --- a/pluginversion_form.php
          +++ b/pluginversion_form.php
          @@ -48,6 +48,7 @@ class local_plugins_edit_version_form extends moodleform {
                   if ($mode == 'edit') {
                       $form->addRule($prefix. 'version', get_string('required'), 'required');
                   }
          +        $form->addRule($prefix. 'version', get_string('versionformaterror', 'local_plugins'), 'regex', '/^(\d{10}(\.\d{2})?)$/');
                   $form->setType($prefix. 'version', PARAM_TEXT);
                   $form->addHelpButton($prefix. 'version', 'versionnumber', 'local_plugins');
          
          Show
          Eloy Lafuente (stronk7) added a comment - Something like this seems to do its job controlling versions are correct, note the lang string is missing and that it does not fix current offenders, just validates the (edited) version: diff --git a/pluginversion_form.php b/pluginversion_form.php index eacdc8b..c3e3f0b 100644 --- a/pluginversion_form.php +++ b/pluginversion_form.php @@ -48,6 +48,7 @@ class local_plugins_edit_version_form extends moodleform { if ($mode == 'edit') { $form->addRule($prefix. 'version', get_string('required'), 'required'); } + $form->addRule($prefix. 'version', get_string('versionformaterror', 'local_plugins'), 'regex', '/^(\d{10}(\.\d{2})?)$/'); $form->setType($prefix. 'version', PARAM_TEXT); $form->addHelpButton($prefix. 'version', 'versionnumber', 'local_plugins');
          Hide
          Aparup Banerjee added a comment -

          Hi Eloy,
          i'm really not sure why 'version' is an editable field actually.
          according to form help
          "This is the version number of the plugin. It should be the version number set in the version.php file if the plugin has a version.php file, otherwise it should be todays date as string followed by two zeros in the format YYYYMMDD00 e.g. 2011051200 for 12th May 2011."

          so it seems at some point it was allowed to not have ->version or version.php !?
          in any case. i'm thinking route (A) too since we always want it now. (during approval too)

          What i'm concerned about is if this will stop/hinder any uploading via 'uploader frustration',... or perhaps a date picker. hm... actually reading this now..

          http://docs.moodle.org/dev/Plugin_validation#Exceptions - so i've created http://tracker.moodle.org/browse/MDLSITE-1937 (perhaps it should be MDL so feel free to move it)

          so as far as the simple fix for (A) it will have to be for only 2.4 supported plugins IF MDLSITE-1937 is agreed upon.

          Show
          Aparup Banerjee added a comment - Hi Eloy, i'm really not sure why 'version' is an editable field actually. according to form help "This is the version number of the plugin. It should be the version number set in the version.php file if the plugin has a version.php file, otherwise it should be todays date as string followed by two zeros in the format YYYYMMDD00 e.g. 2011051200 for 12th May 2011." so it seems at some point it was allowed to not have ->version or version.php !? in any case. i'm thinking route (A) too since we always want it now. (during approval too) What i'm concerned about is if this will stop/hinder any uploading via 'uploader frustration',... or perhaps a date picker. hm... actually reading this now.. http://docs.moodle.org/dev/Plugin_validation#Exceptions - so i've created http://tracker.moodle.org/browse/MDLSITE-1937 (perhaps it should be MDL so feel free to move it) so as far as the simple fix for (A) it will have to be for only 2.4 supported plugins IF MDLSITE-1937 is agreed upon.
          Hide
          Aparup Banerjee added a comment -

          as for 'current offender' plugins besides exception ones, i suppose the data can be fixed up in plugins DB and we can atleast add this check for non themes and course formats.

          Show
          Aparup Banerjee added a comment - as for 'current offender' plugins besides exception ones, i suppose the data can be fixed up in plugins DB and we can atleast add this check for non themes and course formats.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Well, I've not problem with versions being editable/fixed neither with them being mandatory/optional.

          More yet, in order to detect if there are new versions (23_STABLE and upwards)... some organization is required (no version means Moodle won't detect that there are new versions). Not critical.

          But for sure, we need to be producing correct URLs for downloads. And, due to the problem with editable (freely) versions... we are leading to invalid URLs. And that IS critical.

          So, as you want. I just was exposing the problem (free version contents) and the effect (incorrect URLs produced). If the fix is to change the URLs schema or to fix the version field is something I don't really know.

          All we need is to ensure that URLs are correct URLs.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Well, I've not problem with versions being editable/fixed neither with them being mandatory/optional. More yet, in order to detect if there are new versions (23_STABLE and upwards)... some organization is required (no version means Moodle won't detect that there are new versions). Not critical. But for sure, we need to be producing correct URLs for downloads. And, due to the problem with editable (freely) versions... we are leading to invalid URLs. And that IS critical. So, as you want. I just was exposing the problem (free version contents) and the effect (incorrect URLs produced). If the fix is to change the URLs schema or to fix the version field is something I don't really know. All we need is to ensure that URLs are correct URLs. Ciao
          Hide
          Aparup Banerjee added a comment -

          noting that a fix to urlencode the version bit in download url was deployed. atm slashes still a problem.

          Show
          Aparup Banerjee added a comment - noting that a fix to urlencode the version bit in download url was deployed. atm slashes still a problem.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Note: new change has landed to the plugins DB so now it produces "correct" URLs always.

          That has caused immediately to get the plugins information renewed @ download.moodle.org so now the 1.1 api is sending the new download and md5 information:

          (I think that's enough to allow us to continue here, big thanks Apu & Matt).

          Anyway, current encoding seems to lead to not-downloadeable URLs (not working in the plugins DB either!!). See for example:

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Note: new change has landed to the plugins DB so now it produces "correct" URLs always. That has caused immediately to get the plugins information renewed @ download.moodle.org so now the 1.1 api is sending the new download and md5 information: API: http://download.moodle.org/api/1.1/updates.php?branch=2.2&version=0&plugins=local_codechecker@0 Plugins DB: http://moodle.org/plugins/pluginversions.php?plugin=local_codechecker (I think that's enough to allow us to continue here, big thanks Apu & Matt). Anyway, current encoding seems to lead to not-downloadeable URLs (not working in the plugins DB either!!). See for example: API: http://download.moodle.org/api/1.1/updates.php?branch=2.2&version=0&plugins=mod_amvonetroom@0 Plugins DB: http://moodle.org/plugins/pluginversions.php?plugin=mod_amvonetroom Ciao
          Hide
          David Mudrak added a comment -

          Thanks all for making this landed. I still believe that the primary problem with mod_amvonetroom is that is should never pass input validation. Integer plugin versions are crucial for the whole module functionality and people just must follow the rules there.

          Show
          David Mudrak added a comment - Thanks all for making this landed. I still believe that the primary problem with mod_amvonetroom is that is should never pass input validation. Integer plugin versions are crucial for the whole module functionality and people just must follow the rules there.
          Hide
          Martin Dougiamas added a comment -

          Text polishing:

          string 1:
          You are about to install a new version of the plugin <strong>{$a->name}</strong>. A zip package with version {$a->version} of the plugin will be downloaded from <a href="{$a->url}">{$a->url}</a> and extracted to your Moodle installation so it can upgrade your installation.

          String 2:
          Please note that Moodle will not automatically make a backup of your database before the upgrade. We strongly recommend that make a full snapshot backup now, to cope with the rare case that the new code has bugs that make your site unavailable or even corrupts your database. Proceed at your own risk.

          Show
          Martin Dougiamas added a comment - Text polishing: string 1: You are about to install a new version of the plugin <strong>{$a->name}</strong>. A zip package with version {$a->version} of the plugin will be downloaded from <a href="{$a->url}">{$a->url}</a> and extracted to your Moodle installation so it can upgrade your installation. String 2: Please note that Moodle will not automatically make a backup of your database before the upgrade. We strongly recommend that make a full snapshot backup now, to cope with the rare case that the new code has bugs that make your site unavailable or even corrupts your database. Proceed at your own risk.
          Hide
          Jason Cole added a comment -

          Will this functionality allow a partner to point at their own internal repository? We'll need to pull from git repos other than Moodle core for a lot of our functionality.

          Show
          Jason Cole added a comment - Will this functionality allow a partner to point at their own internal repository? We'll need to pull from git repos other than Moodle core for a lot of our functionality.
          Hide
          Jason Cole added a comment -

          I've tagged this as a partner issue as it will cause problems for partners if we can't A) manage the repo it checks and B) turn it off if we need to for a given site. We manage a set of patches for all of our customers and a number of customizations for individual clients. If they are just pulling from core Moodle, then they will break their site the first time they run this. The initial use case makes me a bit nervous as the target audience is a cheap web host, not a managed partner site.

          Show
          Jason Cole added a comment - I've tagged this as a partner issue as it will cause problems for partners if we can't A) manage the repo it checks and B) turn it off if we need to for a given site. We manage a set of patches for all of our customers and a number of customizations for individual clients. If they are just pulling from core Moodle, then they will break their site the first time they run this. The initial use case makes me a bit nervous as the target audience is a cheap web host, not a managed partner site.
          Hide
          David Mudrak added a comment -

          Thanks for your feedback Jason.

          re A) You will be able to point the site to your own repository with a bit of hacking (which is IMHO acceptable for managed partner site). The URL to take the ZIP packages from is served as a part of Available update notifications. And you can redirect Moodle to fetch available updates from your own feed, which in turn contains download URLs to your own repo.
          re B) You will be able to hard-lock the feature via a line in config.php

          $CFG->disableupdateautodeploy = true;
          

          If such line is present, the feature can't be enabled via WWW admin UI.

          The main use case was intentionally aimed to cheap hosts as it was expected that big institutions and partners have established their own mechanisms of deployment.

          Show
          David Mudrak added a comment - Thanks for your feedback Jason. re A) You will be able to point the site to your own repository with a bit of hacking (which is IMHO acceptable for managed partner site). The URL to take the ZIP packages from is served as a part of Available update notifications. And you can redirect Moodle to fetch available updates from your own feed, which in turn contains download URLs to your own repo. re B) You will be able to hard-lock the feature via a line in config.php $CFG->disableupdateautodeploy = true ; If such line is present, the feature can't be enabled via WWW admin UI. The main use case was intentionally aimed to cheap hosts as it was expected that big institutions and partners have established their own mechanisms of deployment.
          Hide
          David Mudrak added a comment -

          Submitting the first (working) version for the peer-review. The simplified logic of the mechanism is:

          1. When the user clicks "Install this update" button at a screen displaying available updates notifications, the confirmation page appears.
          2. When confirmed, the browser is redirected to a standalone utility /mdeploy.php providing all necessary options to deploy the ZIP package.
          3. When mdeploy.php finishes its job, it redirects the browser to /admin/index.php to actually execute the upgrade.

          There are some things left to do that I am aware of:

          • Exceptions handling/display in mdeploy.php
          • Finish the CLI support for mdeploy.php
          • Look at the option to execute mdeploy.php via AJAX call from the Plugins overview page
          • Implement the MD5 hash checking of the ZIP contents
          Show
          David Mudrak added a comment - Submitting the first (working) version for the peer-review. The simplified logic of the mechanism is: 1. When the user clicks "Install this update" button at a screen displaying available updates notifications, the confirmation page appears. 2. When confirmed, the browser is redirected to a standalone utility /mdeploy.php providing all necessary options to deploy the ZIP package. 3. When mdeploy.php finishes its job, it redirects the browser to /admin/index.php to actually execute the upgrade. There are some things left to do that I am aware of: Exceptions handling/display in mdeploy.php Finish the CLI support for mdeploy.php Look at the option to execute mdeploy.php via AJAX call from the Plugins overview page Implement the MD5 hash checking of the ZIP contents
          Hide
          Aparup Banerjee added a comment -

          i'm just coming across these unhandled exceptions in mdeploy.php now hehe...

          Show
          Aparup Banerjee added a comment - i'm just coming across these unhandled exceptions in mdeploy.php now hehe...
          Hide
          Aparup Banerjee added a comment - - edited

          Hi David,
          ok i've looked at this over the past days and heres my notes :

          1) -https://github.com/mudrd8mz/moodle/commit/b44fbda1744336c6c8eb0466d879a624f8a24dcc#L0R431 - versionondisk sounds clearer but no biggie.

          2) "Showing contributions only" --> does this mean 'from plugins directory' or mean 'showing any non-core plugin found locally even if not registered in plugins db' ? (clarification might help here to show meaning of it being registered)

          3) phpunit testing, since we're creating separate phpunit tests codebase, the codebase could do with a pure convenience phpunits wrapper caller script just so we can include this

          4) unit test results:
          phpunit --no-configuration mdeploytest

          1 fail with > mdeploytest::test_cast_value with data set #16 ('file:///etc/passwd', 'url', '')
          invalid_option_exception: Not a valid URL

          /Users/aparup/Sites/i/mdeploy.php:279
          /Users/aparup/Sites/i/mdeploytest.php:76
          /Users/aparup/Sites/i/mdeploytest.php:107

          5)
          in the cli:
          sudo -u apache php mdeploy.php (could mention to replace apache with your web server's username which may vary)

          6)
          Fatal (unhanded) error on some cli calls..
          php mdeploy.php -u
          (i'm removed traces and other cli calls here after noting you are aware of them and the possibility of redisplaying the usage help with any *_option_exception)

          7)
          when using a contributed theme and updating the theme, is the theme affected i wonder.. (edit: this worked fine from gui)

          8)
          core update in fake.php has the https url to download.moodle.org as url. https://download.moodle.org is currently not redirecting. (not sure if this is just about testing tho)

          9)

          • i believe this is a critical point:
            in lib/pluginlib.php : prepare_request_url()
            update provider urls returned (or accepted) must be ensured to be ssl/trusted etc else they could be spoofed. (and down the drain goes any md5 plugin zip file protection etc)

          10)
          the display of available updates seems to be displayed in gui in returned array order, this could be sortable or simply sorted with the latest version on top.
          (also i do note that a person not updating for awhile or a very actively versioned plugin will show up in plugins overview 'updates only' page many many times… it could be truncated with a 'show more' perhaps?)

          11)
          "plugin update confirmation" page could provide link to the plugin (versions) entry at the plugins directory to help with confirmation of the download url etc.
          i'm not sure if we want to release a friendly tool like this with 'Proceed at your own risk.' within it.

          12)
          logging todo noted --> going into moodle log tables?

          13)
          cli:
          data root not from config.php… hmm
          default usage example in mdeploy.php code header missing 'typeroot', 'name'.
          'name' option is slightly vague for me but there is always help i suppose.
          As a result of not understanding name i couldn't use the cli to deploy a plugin but i guess cli work is still being finished up.

          ps: i've found it useful to create a fake.php entry with a future version but a download link to an older version (for downloading tests)

          The gui interface functions totally fine for me so i'm coming in to work to finally meet the great guy who wrote this.

          cheers

          Show
          Aparup Banerjee added a comment - - edited Hi David, ok i've looked at this over the past days and heres my notes : 1) - https://github.com/mudrd8mz/moodle/commit/b44fbda1744336c6c8eb0466d879a624f8a24dcc#L0R431 - versionondisk sounds clearer but no biggie. 2) "Showing contributions only" --> does this mean 'from plugins directory' or mean 'showing any non-core plugin found locally even if not registered in plugins db' ? (clarification might help here to show meaning of it being registered) 3) phpunit testing, since we're creating separate phpunit tests codebase, the codebase could do with a pure convenience phpunits wrapper caller script just so we can include this 4) unit test results: phpunit --no-configuration mdeploytest 1 fail with > mdeploytest::test_cast_value with data set #16 ('file:///etc/passwd', 'url', '') invalid_option_exception: Not a valid URL /Users/aparup/Sites/i/mdeploy.php:279 /Users/aparup/Sites/i/mdeploytest.php:76 /Users/aparup/Sites/i/mdeploytest.php:107 5) in the cli: sudo -u apache php mdeploy.php (could mention to replace apache with your web server's username which may vary) 6) Fatal (unhanded) error on some cli calls.. php mdeploy.php -u (i'm removed traces and other cli calls here after noting you are aware of them and the possibility of redisplaying the usage help with any *_option_exception) 7) when using a contributed theme and updating the theme, is the theme affected i wonder.. (edit: this worked fine from gui) 8) core update in fake.php has the https url to download.moodle.org as url. https://download.moodle.org is currently not redirecting. (not sure if this is just about testing tho) 9) i believe this is a critical point: in lib/pluginlib.php : prepare_request_url() update provider urls returned (or accepted) must be ensured to be ssl/trusted etc else they could be spoofed. (and down the drain goes any md5 plugin zip file protection etc) 10) the display of available updates seems to be displayed in gui in returned array order, this could be sortable or simply sorted with the latest version on top. (also i do note that a person not updating for awhile or a very actively versioned plugin will show up in plugins overview 'updates only' page many many times… it could be truncated with a 'show more' perhaps?) 11) "plugin update confirmation" page could provide link to the plugin (versions) entry at the plugins directory to help with confirmation of the download url etc. i'm not sure if we want to release a friendly tool like this with 'Proceed at your own risk.' within it. 12) logging todo noted --> going into moodle log tables? 13) cli: data root not from config.php… hmm default usage example in mdeploy.php code header missing 'typeroot', 'name'. 'name' option is slightly vague for me but there is always help i suppose. As a result of not understanding name i couldn't use the cli to deploy a plugin but i guess cli work is still being finished up. ps: i've found it useful to create a fake.php entry with a future version but a download link to an older version (for downloading tests) The gui interface functions totally fine for me so i'm coming in to work to finally meet the great guy who wrote this. cheers
          Hide
          Aparup Banerjee added a comment -

          ah i forgot to mention
          14) atm https://download.moodle.org (which is the more info example in fake.php for core updates) doesn't redirect. This may need to be fixed up at the server.

          Show
          Aparup Banerjee added a comment - ah i forgot to mention 14) atm https://download.moodle.org (which is the more info example in fake.php for core updates) doesn't redirect. This may need to be fixed up at the server.
          Hide
          Aparup Banerjee added a comment -

          spoke to David and just noting here:

          • optional cli argument to read moodle's config.php would be useful
          • the --name is frankenstyle, i had tried 'theme_bootstrap' during (13) but couldn't proceed due to following which i may as well paste here now. (related to unit test failure?) :
            $ php mdeploy.php --upgrade --package=https://moodle.org/plugins/download.php/1489/theme_bootstrap_moodle23_2012101200.zip --dataroot=~/Sitesdata/i/master/mysql --typeroot=theme --name theme_bootstrap
            PHP Fatal error:  Uncaught exception 'invalid_option_exception' with message 'Invalid plugin name' in /Users/aparup/Sites/i/mdeploy.php:284
            Stack trace:
            #0 /Users/aparup/Sites/i/mdeploy.php(426): input_manager->cast_value(false, 'plugin')
            #1 /Users/aparup/Sites/i/mdeploy.php(402): input_provider->populate_options()
            #2 /Users/aparup/Sites/i/mdeploy.php(75): input_provider->initialize()
            #3 /Users/aparup/Sites/i/mdeploy.php(302): singleton_pattern::instance()
            #4 /Users/aparup/Sites/i/mdeploy.php(75): input_manager->initialize()
            #5 /Users/aparup/Sites/i/mdeploy.php(699): singleton_pattern::instance()
            #6 /Users/aparup/Sites/i/mdeploy.php(75): worker->initialize()
            #7 /Users/aparup/Sites/i/mdeploy.php(1084): singleton_pattern::instance()
            #8 {main}
              thrown in /Users/aparup/Sites/i/mdeploy.php on line 284
            
            Fatal error: Uncaught exception 'invalid_option_exception' with message 'Invalid plugin name' in /Users/aparup/Sites/i/mdeploy.php on line 284
            
            invalid_option_exception: Invalid plugin name in /Users/aparup/Sites/i/mdeploy.php on line 284
            
            Call Stack:
                0.0013     467408   1. {main}() /Users/aparup/Sites/i/mdeploy.php:0
                0.0013     468136   2. singleton_pattern::instance() /Users/aparup/Sites/i/mdeploy.php:1084
                0.0014     468656   3. worker->initialize() /Users/aparup/Sites/i/mdeploy.php:75
                0.0014     468736   4. singleton_pattern::instance() /Users/aparup/Sites/i/mdeploy.php:699
                0.0014     469176   5. input_manager->initialize() /Users/aparup/Sites/i/mdeploy.php:75
                0.0014     469256   6. singleton_pattern::instance() /Users/aparup/Sites/i/mdeploy.php:302
                0.0014     469824   7. input_provider->initialize() /Users/aparup/Sites/i/mdeploy.php:75
                0.0014     469856   8. input_provider->populate_options() /Users/aparup/Sites/i/mdeploy.php:402
                0.0019     474064   9. input_manager->cast_value() /Users/aparup/Sites/i/mdeploy.php:426
            
          Show
          Aparup Banerjee added a comment - spoke to David and just noting here: optional cli argument to read moodle's config.php would be useful the --name is frankenstyle, i had tried 'theme_bootstrap' during (13) but couldn't proceed due to following which i may as well paste here now. (related to unit test failure?) : $ php mdeploy.php --upgrade -- package =https: //moodle.org/plugins/download.php/1489/theme_bootstrap_moodle23_2012101200.zip --dataroot=~/Sitesdata/i/master/mysql --typeroot=theme --name theme_bootstrap PHP Fatal error: Uncaught exception 'invalid_option_exception' with message 'Invalid plugin name' in /Users/aparup/Sites/i/mdeploy.php:284 Stack trace: #0 /Users/aparup/Sites/i/mdeploy.php(426): input_manager->cast_value( false , 'plugin') #1 /Users/aparup/Sites/i/mdeploy.php(402): input_provider->populate_options() #2 /Users/aparup/Sites/i/mdeploy.php(75): input_provider->initialize() #3 /Users/aparup/Sites/i/mdeploy.php(302): singleton_pattern::instance() #4 /Users/aparup/Sites/i/mdeploy.php(75): input_manager->initialize() #5 /Users/aparup/Sites/i/mdeploy.php(699): singleton_pattern::instance() #6 /Users/aparup/Sites/i/mdeploy.php(75): worker->initialize() #7 /Users/aparup/Sites/i/mdeploy.php(1084): singleton_pattern::instance() #8 {main} thrown in /Users/aparup/Sites/i/mdeploy.php on line 284 Fatal error: Uncaught exception 'invalid_option_exception' with message 'Invalid plugin name' in /Users/aparup/Sites/i/mdeploy.php on line 284 invalid_option_exception: Invalid plugin name in /Users/aparup/Sites/i/mdeploy.php on line 284 Call Stack: 0.0013 467408 1. {main}() /Users/aparup/Sites/i/mdeploy.php:0 0.0013 468136 2. singleton_pattern::instance() /Users/aparup/Sites/i/mdeploy.php:1084 0.0014 468656 3. worker->initialize() /Users/aparup/Sites/i/mdeploy.php:75 0.0014 468736 4. singleton_pattern::instance() /Users/aparup/Sites/i/mdeploy.php:699 0.0014 469176 5. input_manager->initialize() /Users/aparup/Sites/i/mdeploy.php:75 0.0014 469256 6. singleton_pattern::instance() /Users/aparup/Sites/i/mdeploy.php:302 0.0014 469824 7. input_provider->initialize() /Users/aparup/Sites/i/mdeploy.php:75 0.0014 469856 8. input_provider->populate_options() /Users/aparup/Sites/i/mdeploy.php:402 0.0019 474064 9. input_manager->cast_value() /Users/aparup/Sites/i/mdeploy.php:426
          Hide
          Aparup Banerjee added a comment -

          Ah David and i just sat down together, the cli works great if you use it right :-p

          php mdeploy.php --upgrade --package=https://moodle.org/plugins/download.php/1489/theme_bootstrap_moodle23_2012101200.zip --dataroot=/Users/aparup/Sitesdata/i_data/master/mysql --typeroot=/Users/aparup/Sites/i/theme --name=bootstrap

          from the above note that

          • dataroot and typeroot must be full paths. (i had even tried to use tilde hehe)
          • --name is really just the name of the folder of the plugin.. so its not frankenstyle there.. so 'name' makes sense more now.

          so in short cli does work.

          Show
          Aparup Banerjee added a comment - Ah David and i just sat down together, the cli works great if you use it right :-p php mdeploy.php --upgrade --package= https://moodle.org/plugins/download.php/1489/theme_bootstrap_moodle23_2012101200.zip --dataroot=/Users/aparup/Sitesdata/i_data/master/mysql --typeroot=/Users/aparup/Sites/i/theme --name=bootstrap from the above note that dataroot and typeroot must be full paths. (i had even tried to use tilde hehe) --name is really just the name of the folder of the plugin.. so its not frankenstyle there.. so 'name' makes sense more now. so in short cli does work.
          Hide
          Helen Foster added a comment -

          Just noting that this issue is very difficult for a non-developer to test, as it requires a fake environment, and so is only suitable for QA testing after 2.4 is released.

          Thus, as agreed in a chat with David, we're going to leave the qa_test_required label on this issue but not write a QA test for it until the next QA cycle.

          Show
          Helen Foster added a comment - Just noting that this issue is very difficult for a non-developer to test, as it requires a fake environment, and so is only suitable for QA testing after 2.4 is released. Thus, as agreed in a chat with David, we're going to leave the qa_test_required label on this issue but not write a QA test for it until the next QA cycle.
          Hide
          David Mudrak added a comment -

          Removing "Pull Master Diff URL". The branch is now rebased against integration/master and the URL is not valid any more.

          Show
          David Mudrak added a comment - Removing "Pull Master Diff URL". The branch is now rebased against integration/master and the URL is not valid any more.
          Hide
          David Mudrak added a comment -

          Submitting for the final integration after fixing some remaining todos and addressing issues that Apu raised during the review.

          DEAR INTEGRATORS,

          please note that the branch is now rebased against integration/master. It is expected to be merged into master only, no backport.

          Show
          David Mudrak added a comment - Submitting for the final integration after fixing some remaining todos and addressing issues that Apu raised during the review. DEAR INTEGRATORS, please note that the branch is now rebased against integration/master. It is expected to be merged into master only, no backport.
          Hide
          David Mudrak added a comment -

          For the record, I am attaching the output of the 'git request-pull' command.

          The following changes since commit f89a1a88765c5f5ec5ed3e6fad3ec4cda99c798e:
          
            Merge branch 'MDL-36197_master' of git://github.com/lazydaisy/moodle (2012-11-08 11:30:49 +1300)
          
          are available in the git repository at:
          
            git://github.com/mudrd8mz/moodle.git MDL-35238-deployment
          
          David Mudrák (29):
                MDL-34099 Report available updates for plugins at admin/index.php
                MDL-35238 Introduce a first version of the mdeploy.php script
                MDL-35238 Print help on the script usage in CLI mode
                MDL-35238 Fix getting input option value
                MDL-35238 Implement HTTP authorization based on passphrase matching
                MDL-35238 Allow filtering at the Plugins overview page
                MDL-35238 Remove the hide/show icon from the Plugins overview page
                MDL-35238 Distinguish the uninstall link from the settings link
                MDL-35238 Add a new admin setting to enable updates deployment
                MDL-35238 Make the auto-deploy feature lockable via config.php
                MDL-35238 Introduce available_update_deployer class
                MDL-35238 Display a button to install an available update
                MDL-35238 Handle the update installation request - display the confirmation page
                MDL-35238 Admin renderer method to display the plugin update confirmation page
                MDL-35238 Implement deployment authorization
                MDL-35238 Reorganise the worker::execute() flow
                MDL-35238 Fetch the package and store it in a temporary location
                MDL-35238 Backup existing plugin version before replacing it
                MDL-35238 Unzip the downloaded package and redirect to the upgrade page
                MDL-35238 Support deployment from yet another plugins check page too
                MDL-35238 Warn the admin if they are about to overwrite a SCM checkout
                MDL-35238 Fetch available updates using the 1.1 version of the API
                MDL-35238 Compare the ZIP package content hash with the expected value
                MDL-35238 Remove the ZIP file after successful decompression
                MDL-35238 Fix the unit test for invalid input_manager::TYPE_URL values
                MDL-35238 Add simple logging of mdeploy.php execution
                MDL-35238 Be more verbose if the tilde character is used in TYPE_PATH script options
                MDL-35238 Improve exceptions handling
                MDL-35238 Add support for explicit singleton reset
          
           admin/index.php              |   46 ++-
           admin/plugins.php            |   25 +-
           admin/renderer.php           |  187 ++++++-
           admin/settings/server.php    |    4 +
           lang/en/admin.php            |    5 +
           lang/en/plugin.php           |   10 +
           lib/adminlib.php             |   74 +++
           lib/phpunit/classes/util.php |    3 +
           lib/pluginlib.php            |  436 ++++++++++++++-
           lib/tests/pluginlib_test.php |   36 ++
           mdeploy.php                  | 1313 ++++++++++++++++++++++++++++++++++++++++++
           mdeploytest.php              |  215 +++++++
           theme/base/style/admin.css   |   10 +
           13 files changed, 2342 insertions(+), 22 deletions(-)
           create mode 100644 mdeploy.php
           create mode 100644 mdeploytest.php
          
          Show
          David Mudrak added a comment - For the record, I am attaching the output of the 'git request-pull' command. The following changes since commit f89a1a88765c5f5ec5ed3e6fad3ec4cda99c798e: Merge branch 'MDL-36197_master' of git: //github.com/lazydaisy/moodle (2012-11-08 11:30:49 +1300) are available in the git repository at: git: //github.com/mudrd8mz/moodle.git MDL-35238-deployment David Mudrák (29): MDL-34099 Report available updates for plugins at admin/index.php MDL-35238 Introduce a first version of the mdeploy.php script MDL-35238 Print help on the script usage in CLI mode MDL-35238 Fix getting input option value MDL-35238 Implement HTTP authorization based on passphrase matching MDL-35238 Allow filtering at the Plugins overview page MDL-35238 Remove the hide/show icon from the Plugins overview page MDL-35238 Distinguish the uninstall link from the settings link MDL-35238 Add a new admin setting to enable updates deployment MDL-35238 Make the auto-deploy feature lockable via config.php MDL-35238 Introduce available_update_deployer class MDL-35238 Display a button to install an available update MDL-35238 Handle the update installation request - display the confirmation page MDL-35238 Admin renderer method to display the plugin update confirmation page MDL-35238 Implement deployment authorization MDL-35238 Reorganise the worker::execute() flow MDL-35238 Fetch the package and store it in a temporary location MDL-35238 Backup existing plugin version before replacing it MDL-35238 Unzip the downloaded package and redirect to the upgrade page MDL-35238 Support deployment from yet another plugins check page too MDL-35238 Warn the admin if they are about to overwrite a SCM checkout MDL-35238 Fetch available updates using the 1.1 version of the API MDL-35238 Compare the ZIP package content hash with the expected value MDL-35238 Remove the ZIP file after successful decompression MDL-35238 Fix the unit test for invalid input_manager::TYPE_URL values MDL-35238 Add simple logging of mdeploy.php execution MDL-35238 Be more verbose if the tilde character is used in TYPE_PATH script options MDL-35238 Improve exceptions handling MDL-35238 Add support for explicit singleton reset admin/index.php | 46 ++- admin/plugins.php | 25 +- admin/renderer.php | 187 ++++++- admin/settings/server.php | 4 + lang/en/admin.php | 5 + lang/en/plugin.php | 10 + lib/adminlib.php | 74 +++ lib/phpunit/classes/util.php | 3 + lib/pluginlib.php | 436 ++++++++++++++- lib/tests/pluginlib_test.php | 36 ++ mdeploy.php | 1313 ++++++++++++++++++++++++++++++++++++++++++ mdeploytest.php | 215 +++++++ theme/base/style/admin.css | 10 + 13 files changed, 2342 insertions(+), 22 deletions(-) create mode 100644 mdeploy.php create mode 100644 mdeploytest.php
          Hide
          Aparup Banerjee added a comment -

          Hi David,
          i've just noted here that pluginlib.php's prepare_request_url() is still non-ssl urls. ref point (9). perhaps at least the default should be ssl ?
          MDLSITE-1931 seems for moodle.org , not sure about download.moodle.org/api/* not being ssl. This is where the zip's urls are being cached.

          Show
          Aparup Banerjee added a comment - Hi David, i've just noted here that pluginlib.php's prepare_request_url() is still non-ssl urls. ref point (9) . perhaps at least the default should be ssl ? MDLSITE-1931 seems for moodle.org , not sure about download.moodle.org/api/* not being ssl. This is where the zip's urls are being cached.
          Hide
          Dan Poltawski added a comment -

          Review WIP, but two quick comments:

          • Whilst its a convenience for testing, now we are doing automatic deployments, I have major concerns about the having config option $CFG->alternativeupdateproviderurl. I think we should be hardcoding this and not providing a way to change it. (Sucks to be us dev/testers only).
            That way if someone has access to the database, they are not able to do UPDATE config SET value = 'http://my.dodgy.site/updates.php' WHERE name = 'alternativeupdateproviderurl' and use this as a vector to install malicious code on the server. Also, as Apu says, that url should be https'ed before release IMO.
          • download_file() doesn't seem to have proxy support, it'll not work in a lot of environments
          Show
          Dan Poltawski added a comment - Review WIP, but two quick comments: Whilst its a convenience for testing, now we are doing automatic deployments, I have major concerns about the having config option $CFG->alternativeupdateproviderurl. I think we should be hardcoding this and not providing a way to change it. (Sucks to be us dev/testers only). That way if someone has access to the database, they are not able to do UPDATE config SET value = 'http://my.dodgy.site/updates.php' WHERE name = 'alternativeupdateproviderurl' and use this as a vector to install malicious code on the server. Also, as Apu says, that url should be https'ed before release IMO. download_file() doesn't seem to have proxy support, it'll not work in a lot of environments
          Hide
          Aparup Banerjee added a comment -

          just adding Nadav and Mary here for very possible RTL follow ups needed.

          Show
          Aparup Banerjee added a comment - just adding Nadav and Mary here for very possible RTL follow ups needed.
          Hide
          David Mudrak added a comment -

          Thanks guys for your comments and sorry for the late submission. I kind of expected this would be integrated by Apu (as was agreed on https://moodle.org/local/chatlogs/index.php?conversationid=11217#c386840) who has already reviewed it. Thence I paid attention to other issues and did not count with any significant time needed for the integration. However, I'm happy that Dan will look at this, too (of course). I mean it as an excuse for the "quarter to release o'clock" submission.

          Re SSL: This was discussed today https://moodle.org/local/chatlogs/index.php?conversationid=11427#c393551 with the conclusion that HQ will buy a commercial certificate for the server. Once it works, I'll put https://download.moodle.org/api/1.1/updates.php as the default updates info provider.

          Re alternativeupdateproviderurl: I modified the code so that it exclusively reads this setting from config.php only, not from the mdl_config table.

          Re proxy: Correct, this is a known missing feature and should be fixed on 2.4 STABLE.

          Re RTL: Sorry I can't see how is this related to right-to-left languages? Can you please elaborate?

          Show
          David Mudrak added a comment - Thanks guys for your comments and sorry for the late submission. I kind of expected this would be integrated by Apu (as was agreed on https://moodle.org/local/chatlogs/index.php?conversationid=11217#c386840 ) who has already reviewed it. Thence I paid attention to other issues and did not count with any significant time needed for the integration. However, I'm happy that Dan will look at this, too (of course). I mean it as an excuse for the "quarter to release o'clock" submission. Re SSL: This was discussed today https://moodle.org/local/chatlogs/index.php?conversationid=11427#c393551 with the conclusion that HQ will buy a commercial certificate for the server. Once it works, I'll put https://download.moodle.org/api/1.1/updates.php as the default updates info provider. Re alternativeupdateproviderurl: I modified the code so that it exclusively reads this setting from config.php only, not from the mdl_config table. Re proxy: Correct, this is a known missing feature and should be fixed on 2.4 STABLE. Re RTL: Sorry I can't see how is this related to right-to-left languages? Can you please elaborate?
          Hide
          Nadav Kavalerchik added a comment -

          David,
          I think Aparup wanted to let us (RTL team) know of the new UI changes so we could handle the RTL CSS modification needed for that UI when viewed from RTL. (Please correct me if I am wrong)
          I checked it, and it looks oK. After this (beautiful) code is integrated I will have another go over the UI to see that is cleared and correctly aligned to the RTL folks too

          Show
          Nadav Kavalerchik added a comment - David, I think Aparup wanted to let us (RTL team) know of the new UI changes so we could handle the RTL CSS modification needed for that UI when viewed from RTL. (Please correct me if I am wrong) I checked it, and it looks oK. After this (beautiful) code is integrated I will have another go over the UI to see that is cleared and correctly aligned to the RTL folks too
          Hide
          David Mudrak added a comment -

          Attaching a screenshot of the situation when the web server process does not have write access to the plugin location.

          Show
          David Mudrak added a comment - Attaching a screenshot of the situation when the web server process does not have write access to the plugin location.
          Hide
          David Mudrak added a comment -

          Attaching a screenshot of the confirmation page. In this situation, the current code has been obtained via a SCM checkout. So installing the update would replace it with a plain copy of the ZIP content. This may and may not be what the admin intents so we better warn them.

          Show
          David Mudrak added a comment - Attaching a screenshot of the confirmation page. In this situation, the current code has been obtained via a SCM checkout. So installing the update would replace it with a plain copy of the ZIP content. This may and may not be what the admin intents so we better warn them.
          Hide
          David Mudrak added a comment -

          DEAR INTEGRATORS,

          I have added three more commits on top of the branch that implement Dan's suggestions discussed today. I have also rebased the branch against integration/master again - sorry if that causes any troubles to you (please advise correct procedure for hot fixes like this).

          These are the improvements addressed:

          • $CFG->alternativeupdateproviderurl issue described above
          • Moodle does not check any more for write permission at the moment when this feature is being enabled.
          • If the feature is enabled but the deployment can not happen due to insufficient write permissions, a help link is displayed where the "Install this update" button could be (see the attached screenshot).
          Show
          David Mudrak added a comment - DEAR INTEGRATORS, I have added three more commits on top of the branch that implement Dan's suggestions discussed today. I have also rebased the branch against integration/master again - sorry if that causes any troubles to you (please advise correct procedure for hot fixes like this). These are the improvements addressed: $CFG->alternativeupdateproviderurl issue described above Moodle does not check any more for write permission at the moment when this feature is being enabled. If the feature is enabled but the deployment can not happen due to insufficient write permissions, a help link is displayed where the "Install this update" button could be (see the attached screenshot).
          Hide
          Dan Poltawski added a comment -

          Thanks a lot David, i've integrated this now.

          Just to note that we should add to the testing steps with 'modified plugins' (i.e. standard plugin modified so its an unknown md5) and also CVS/svn/git checkouts since it looks like you are detecting it.

          I had one remaining thought regarding the 'security' of this deployment script via the web, which was the idea that we should be validating the authenticity of the parameters passed when be called on the web. In particularly the 'dirroot' (because we validate the dataroot is correct due to the pass file, but not necessarily relationship with the dirroot).

          However, I can't think of a reason why this makes the solution more robust or provides a security advantage, so no doubt i'm overengineering. But I thought i'd record the thought anyway.

          Show
          Dan Poltawski added a comment - Thanks a lot David, i've integrated this now. Just to note that we should add to the testing steps with 'modified plugins' (i.e. standard plugin modified so its an unknown md5) and also CVS/svn/git checkouts since it looks like you are detecting it. I had one remaining thought regarding the 'security' of this deployment script via the web, which was the idea that we should be validating the authenticity of the parameters passed when be called on the web. In particularly the 'dirroot' (because we validate the dataroot is correct due to the pass file, but not necessarily relationship with the dirroot). However, I can't think of a reason why this makes the solution more robust or provides a security advantage, so no doubt i'm overengineering. But I thought i'd record the thought anyway.
          Hide
          Aparup Banerjee added a comment -

          Thanks David.

          yes Nadav you're right, thanks for looking thru the UI for RTL goodness

          Yay ssl!

          Show
          Aparup Banerjee added a comment - Thanks David. yes Nadav you're right, thanks for looking thru the UI for RTL goodness Yay ssl!
          Hide
          David Monllaó added a comment -

          It passes.

          I've installed the 1.5 local_flavours version and I've upgraded to the latest version of the plugin:

          • From the plugins overview page
          • When installing the 1.5 version (I've been asked for installing the latest version)
          • When upgrading moodle to a false higher version (changing version.php) I've been asked to also upgrade the plugin

          After each deployment I've checked local/flavours/version.php to ensure is the 1.6 version

          I also been able to test the filters between contrib. plugins and core when upgrading from the plugins overview page.

          Show
          David Monllaó added a comment - It passes. I've installed the 1.5 local_flavours version and I've upgraded to the latest version of the plugin: From the plugins overview page When installing the 1.5 version (I've been asked for installing the latest version) When upgrading moodle to a false higher version (changing version.php) I've been asked to also upgrade the plugin After each deployment I've checked local/flavours/version.php to ensure is the 1.6 version I also been able to test the filters between contrib. plugins and core when upgrading from the plugins overview page.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Amazed. Inspired. Grateful. That’s how your generosity makes me feel.

          (not really)

          Closing, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Amazed. Inspired. Grateful. That’s how your generosity makes me feel. (not really) Closing, thanks!
          Hide
          Helen Foster added a comment -

          This fabulous new feature is now documented in http://docs.moodle.org/24/en/Installing_plugins with a mention http://docs.moodle.org/24/en/Notifications so I'm removing the docs_required label from this issue.

          Anyone please feel free to add to or amend the documentation as necessary.

          Show
          Helen Foster added a comment - This fabulous new feature is now documented in http://docs.moodle.org/24/en/Installing_plugins with a mention http://docs.moodle.org/24/en/Notifications so I'm removing the docs_required label from this issue. Anyone please feel free to add to or amend the documentation as necessary.

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: