Open timodwhit opened 10 years ago
I'm interested in this functionality also, but yes, it's going to take a lot of effort and planning to do properly. After "Drupalgeddon" with the SQL injection attack hijacking sites within 7 hours of release, it's a great example of how manual updates just don't cut it any more with large pieces of web software, any more than attacks against popular browsers or operating systems.
There's a bit of relevant discussion over in https://github.com/backdrop/backdrop-issues/issues/395, but only loosely.
I'd love to take a look at the Wordpress auto-update model and how well that is working for them.
I'm conflicted about this. I like the idea of auto-updates, but also worry about 1) php being able to write it's own files (isn't that a huge risk?) and 2) breaking things in contrib by updating core.
2 will be a bigger risk for us than with Drupal 7 since we may be doing things like adding modules to core. If you had link module from contrib and are running 1.0.0 and in 1.2.0 we added link module to core, and in 1.2.2 we had a major security fix, auto updating you to 1.2.2 might cause a ton of problems. Would we also need to maintain a 1.0.x with the same security fix as 1.2.2 to prevent this from happening? Possible, but more work.
I'm conflicted as well about it, but at the same time, it is helpful for those smaller sites that may not be maintained as regularly as other large sites. Over at d.o there are different thoughts and ideas being kicked around: https://www.drupal.org/node/2367319
As far as the core bringing in modules, I could foreseeably see us saying that we support up to three minor versions pack with security patches or something similar to that. And then each site would have the ability to chose how they update like gems and bundler. You can select whether to update to major, minor, or patch version through a module_version
and module_version.lock
file type approach. By default we could say modules update to patch versions, but minor we leave alone unless you decide to do that. This could easily be done with a UI of check boxes, like the permissions tab or through files.
I think the level of security issue something like the "drupalgeddon" bug would warrant a patch to minor versions, lets say 3 minor versions back, which seems like it would be quite significant.
As far as PHP writing files, I think it depends on the host you are on, doesn't the update module allow for this currently?
@quicksketch, @jenlampton and I talked for a while about this today at Twin Cities Drupal Camp sprints. Its a problem I've been working on, on and off, since I was introduced to Backdrop's philosophy at last year's camp. Especially when coupled with security-only releases of older Backdrop versions, making the technology seemed worthwhile.
My approach has been to create a mostly-standalone web application that updates other web applications, vs. simply modifying/extending similar functionality already in Backdrop that installs and updates modules. The advantages of this are that it avoids potential complications with the updater being dependent on and loading things while they're getting updated, that if a given update totally broke a given site, the updater application itself is still usable, and that it becomes simpler to make the work applicable to other php applications.
The work so far can be found at https://github.com/curator-wik. I call the app Curator.
To clarify, the first problem I'm trying to solve is in-browser, user attended core updates, with automatic updates being a progression from there. But, some takeaways from today's discussion that ought to get noted on here:
Nate also brought up a bit of a thought experiment about what it would look like for Curator to self-update, especially if it was shipped as part of core and thus in some releases would be responsible for updating itself. Since Curator will be distributed as a single .phar file, it should be fairly simple to rapidly and cleanly switch from an old version to a new one. However, there was consensus that there should probably be a little special handling when a core update includes a Curator update, to ensure that the updater updates itself first, before attempting to apply the update to Backdrop.
We talked quite a bit about what the input to Curator should be, e.g. a full .tgz of the new release vs some paired-down format that only contains the things that changed from one Backdrop release to another. The advantage of updating from a full .tgz is that there is less new infrastructure/release generation work necessary. The advantage of a paired-down format are that applying an update can be done more quickly by each site, by executing just the file overwrites/deletes necessary to make the old release source tree match the new release source tree rather than deleting the entire old core and then installing an entirely new one. Curator supports a paired-down format. I think I was at least reasonably successful at bringing Jen and Nate around to my way of thinking that incorporating some automation to their release process that generates these paired-down packages might be worthwhile :)
The concept of self or auto updates is obviously important and the approach sounds practical. Apart from that though I cannot comment on the plan or code: a bit high level for me. Will be happy to test when it is ready, and if it is already testable, will need instructions.
Thank you @mbaynton for all the time and energy spent on this 👍 Basically what @docwilmot said: way beyond me, but do let me know when you need help with testing.
For reference, Drupal is beginning to discuss a similar idea: https://www.drupal.org/project/ideas/issues/2940731
The goal is to implement a secure system for automatically installing updates in Drupal, lowering the total cost of ownership of maintaining a Drupal site, improving the security of Drupal sites in the wild, and lowering the barrier to entry to using Drupal.
Moreover, content of such sites with low cost of ownership and maintaining also can be added automatically, or at least this server can generate(mining) some (any buzzwords can be placed here)coins.
I wonder how this issue here fits into #2018 and its child issues. Is this also a sub-issue of the META, or should it be closed as duplicate?
@klonos I think this is the automatic part of #2018 and therefore should be the last part of that meta.
Could use feedback on UX and language:
in the top right I would like to put date pickers so people can restrict the updates to a time window (like between 2AM and 5AM).
current language:
A Security release is a release that pathes a security vulnerability. A Patch release is a security and/or bug fix releases, i.e. 1.10.1 to 1.10.2 is a patch release. A Minor release is feature release (new functionality no API changes), i.e. 1.10.2 to 1.11.0 is a minor release.
I like @olafgrabienski's proposal. My suggestion based on that:
I kinda liked the "i.e." examples, but we could omit them if people find them too "technical". ...although this part of the admin UI is targeted towards technically-savvy people I guess.
@serundeputy do you have a PR for this?
@klonos PR is fortcoming. I have code but needs attention and <3.
PR: https://github.com/backdrop/backdrop/pull/2307
core
from backdrop.zip to BACKDROP_ROOT/core
TODO:
installer_cron
core
dir from backdrop.zip
over the currently installed core
, but for example we moved node_path_info()
to a new file, but with the copy it is in both files and backdrop explodes; ( i think this would fail on updating via UI too on an update from 1.10.2
to 1.11.0
)
So it leaves files that are no longer in the new version? If that's true then I guess we do need to remove core first somehow.
I wonder how WP does it? I also seem to recall it has ability to roll back at some point (before db updates i imagine).
I know lots of work has gone into this direction for doing your updates already, but this talk about deleting files and renaming files and needing to remove core have sucked me in :) You'd have to spin up a Drupal 7 site :scream: but you might want to take a gander at
Although the thing you can actually test drive today is Drupal 7, the entirety of the D7 specific code is like 150 lines; the rest lives in a .phar file that is application agnostic. It should be pretty easy to port to Backdrop. I'd do it myself but I was actually working on adding rollback capability as I saw these comments, so I'm otherwise engaged at the moment :wink:
Oh! I also has docs and resources and goodies:
@mbaynton thanks; I took a brief glance at the things you have going on there, but I could not get quickly up to speed on it (a lot to digest)
My current thinking is that i'm close with the api that already exists in Backdrop ... so it seems simpler to me to implement that way. I'm going to continue on that path ... others should give their opinions as well.
The current PR is not ready for general testing (it doesn't quite work yet), but dropping this script here that eases some of the steps needed to get to a place that one can test:
#!/usr/bin/env.bash¬
set -ex¬
git clone git@github.com:backdrop/backdrop.git 414automaticUpdates
cp 2289.patch 414automaticUpdates/
cp 2307.patch 414automaticUpdates/
cp .lando.yml 414automaticUpdates/
cd 414automaticUpdates/
git checkout 1.10.2
git apply 2289.patch
git apply 2307.patch
lando.dev start && lando.dev drush si --db-url=mysql://backdrop:backdrop@database/backdrop
lando.dev drush uli
Run this in a clean directory and it will set up a testable backdrop environment that has the right patches and is ready to upgrade from 1.10.2
to 1.11.0
.
When you are done with that environment (like maybe you want to test again) do a lando destroy -y
and rm -rf 414automaticUpdates
, then you can run the script again and start over.
I have just tested upgrading 1.11.0 to 1.11.1 and it run smoothly 👍. Notes:
/admin/reports/updates/settings
)"Check if you want the ability...": I usually prefer the words "enable" (formal) or "tick" (informal). It helps avoid ambiguations (check = test/verify).
I would like to propose to remove the checkbox for core updates altogether. We can then change the label of the dropdown to "Core updates method", and also change its options to these:
...I would in fact argue that the "Disabled" option should not exist at all (since we have checkboxes which allow people to include/exclude individual projects during the update wizard). This, combined with changing the dropdown to a set of radio options would allow to place the respective help text under each method. Here's what it would look like:
...so no need for custom #states
/css magic to show/hide any options + single click to make a choice = UX+
Also... notice how in the help text for the manual option in my mockup I have a ???
placeholder for the update wizard page? Well, how weird is it now that the core updates are available under the "Update modules" page (/admin/modules/update
) & the "Update themes" page (/admin/themes/update
) & the "Update layouts" page (/admin/layouts/update
), huh? 😄 ...this is me shamelessly plugging #3039
Perhaps "Back up your database and site before you continue. Learn how." (/admin/update/ready
) should be a warning
message instead of plain markup text. Before:
...after (mockup):
...not sure about "Updating modules, themes, and layouts requires FTP access to your server. ...". It feels like an info
message.
"Updating modules, themes, and layouts, as well as Backdrop core requires..."
During next step, if ftp is the only method, then why offer a dropdown menu with options?
...or are we removing that altogether (#3208)?
...another question: I know that this ticket is about the backend work required to get automatic updates working, but why restrict the security updates only to core? A security update is a security update no matter what.
Anyway, in the case we do want to enable automatic security updates for contrib projects too, then I think that we should:
@klonos Above, you mentioned the page admin/update/ready
with the message "Back up your database and site before you continue. Learn how."
I just noticed that the "Learn how" link points to the page "Backing up a site" on drupal.org. Indeed, I didn't find a similar page in the Backdrop documentation. Are you aware of such a page? (If not, I'd open a ticket with the suggestion to add one.)
@olafgrabienski I could not find anything in https://backdropcms.org/user-guide, so by all means create a separate documentation issue. We will need to link to it from other pages too, like in update.php, where we say:
... Create backups. This update utility will alter your database and config files. In case of an emergency you may need to revert to a recent backup; make sure you have one. ...
...in fact there are still many places in our codebase where we still link to d.org. We cannot possibly match the accumulated decades of documentation that's there.
I added a screenshot of the proposed UI from the PR into the parent issue here. It looks like there are a bunch of assumptions there (like the ability to update to not-the-latest-version) that we haven't yet determined are possible.
I also have some suggestions for language improvements, but I'm going to start on the single checkbox for "Enable core updates" and put those language changes in the PR for https://github.com/backdrop/backdrop-issues/issues/3271.
While working on https://github.com/backdrop/backdrop-issues/issues/3271 It occurred to me that we should add a few other settings for the automatic updates (and these may qualify as feature additions on their own)
cross-posting from https://github.com/backdrop/backdrop-issues/issues/3271#issuecomment-445130885
...wondering if it would be a safer option to be renaming the old core and then extracting the update in its place. We'd then let the end user manually delete the backup of the old core.
If they don't delete the old backups, worse that can happen is to run out of disk space (and given the size of core and the space offerings by hosting providers nowadays, that'd be rare). That's way safer than destructively deleting things.
I believe it relies on the updater classes https://github.com/backdrop/backdrop/blob/1.x/core/includes/updater.inc. There is a backup function makeBackup
but it currently does nothing.
This PR probably needs to be updated to work with the code that was merged in https://github.com/backdrop/backdrop-issues/issues/3271. We added the UI to enable manual updates, so this issue can focus just on adding the automatic part.
Just subscribing to this nice discussion to include it under my radar.
It still needs work and further feedback but I started reworking @serundeputy's PR from ~3 years ago here: https://github.com/backdrop/backdrop/pull/3753
Please help if interested :sweat_smile:
An example of an email that Wordpress sends after an auto-update, for reference:
SUBJECT:
[Website Name] Some plugins were automatically updated
BODY:
Howdy! Some plugins have automatically updated to their latest versions on your site at https://example.org. No further action is needed on your part.
These plugins are now up to date:
- Yoast SEO (from version 17.2 to 17.2.1)
If you experience any issues or need support, the volunteers in the WordPress.org support forums may be able to help. https://wordpress.org/support/forums/
The WordPress Team
If we're doing auto-updates for sites, then I would be more comfortable if we also auto-backed up their db as well as part of the process. So I would like this email to also mention something along the lines of:
A backup of your database has been created under [path-to-private-dir]
@laryn how do we test this? Is it for core only, or contrib projects as well?
If would expect this to be a scenario to test:
What about core? How do we test that? Is this a scenario that should work?:
We've got a whole meta issue on things that we need before we can turn this on, including digital signatures on packages: https://github.com/backdrop/backdrop-issues/issues/2018. (I think the signatures is getting close but it has stalled as well.)
Yes, I didn't mean to imply that this was ready to go (or to take it over), just trying to keep it up to speed with core changes and hopefully keep it on the radar.
@klonos I haven't changed any of the functionality from the initial PR and I think a lot of those questions are still to be answered. Right now it's in a very early form with few options or checks in place. As far as testing what is there now, here's a quick way:
1.21.x-dev
core/includes/bootstrap.inc
and set the version number to 1.19.2
(or whatever earlier version)1.20.0
currently)I think doing this on an actual 1.19.2
install will raise the issue of database updates required -- which the PR still marks as a @todo
.
I would like to propose to remove the checkbox for core updates altogether. We can then change the label of the dropdown to "Core updates method", and also change its options to these:
- Disabled
- Manual
- Automatic - Security releases only
- Automatic - Minor releases
- Automatic - Patch releases
FTR, this is what the respective UI looks like in D9:
To me, "Disabled" implies "Manual" so that could probably be consolidated.
I would like to see 3 options personally, 4 at the most.
"Automatic - Patch releases" doesn't mean anything to me so I would never use it. I can only guess a patch is a major bug fix that can't wait for the next release (a bug got through review and broke stuff for example), which I would personally lump in to run with security updates which are technically patches if I'm thinking about it right.
One thought I've pondered for a while is if it's worth doing a diff on core files before an update?
A long time ago, I played around with PHPBB and when updating their platform, it would scan all of the core files and tell you if any core files had manual changes performed and told you exactly which files did not match the old or new meaning they were modified by the user. It would even display the line mismatches in the GUI. There were several options to deal with those changes, but it's made me wonder if it would be worth it to have the updater (whether auto or manual) do a diff check to see if a core file has been modified by the end-user and then ask what to do about it before proceeding. This would mainly benefit those who add patches to core files but it adds another layer of break-proofing things. For what it's worth.
Finally, what would it take to push updates instead of each site pinging for an update? I would think a site should still ping occasionally if it hasn't received anything from the server in a while, but if there is a major security update that rolls through (like the Drupal SQL one), sites will be notified of the update and instructed to updated instead of requesting if there is an update at pre-defined times that might open the window for exploitation. It would take some thought on the load-balancing side of things to offset how much is being pushed to prevent server load etc but I feel this would be a better solution long term. I would be OK if this route was only applied to security and "patch" updates.
Not sure if there is an issue about this, but I think something strong that backdrop-issue could offer is auto updates for security issues etc.
Since there is semantic versioning, this could be used as a check.
There are obviously a lot of issues that are needing to be addressed here, but thought I would start the issue.
~PR by @serundeputy: https://github.com/backdrop/backdrop/pull/2307~ Revised by @laryn: https://github.com/backdrop/backdrop/pull/3753
Part of META issue: https://github.com/backdrop/backdrop-issues/issues/2018