search

limetech / dynamix

14 stars 5 forks source link

Make Dynamix webGui plugin a system plugin #247

Closed bergware closed 8 years ago

bergware commented 8 years ago

This changes the installation of the Dynamix webGUI plugin to folder /boot/plugins. It will ensure that the Dynamix webGUI plugin is always loaded before any other plugins, this avoids possible conflicts or none functional plugins.

limetech commented 8 years ago

The system plugin idea is a safety hook put in the code in case no other way to solve a problem was possible or practical. In template.php when the pages are built using build_pages() it specifically adds the 'dynamix' pages first. Subsequently if there is a plugin which includes a different version of a 'dynamix' page (to change/augment stock functionality) they will be loaded next, thereby replacing the stock pages in the $site array [which I think used to be called $pages?].

In other words: when a plugin is installed which changes pages in 'dynamix' it does so by simply creating the same name page in it's own directory rather than overwriting those pages in the dynamix directory upon installation. Make sense?

Anyway, is there a specific issue that you have seen that is only resolved by making dynamix a 'system' plugin?

bergware commented 8 years ago

The problem is not the execution phase, true the webGui folder is always executed first, but the problem is in the plugin installation phase.

The webGui plugin completely erases the folder /usr/local/emhttp/plugins/dynamix before installing the new version. This means that a plugin which was installed before and had files added to this folder will be incomplete afterwards. There are a number of plugins doing this and some reports on the forum I have seen talking about broken plugin functionality (maybe not everyone understands why).

The easiest solution is to ensure that installation of the webGui plugin is ahead of any other plugin upon a system restart, hence my proposal to make it a system plugin (the only one allowed).

Ps. After upgrading the webGUI it should then be recommended to reboot the system.

limetech commented 8 years ago

Isn't it the "right" solution to fix the plugins that are installing pages in the 'dyanmix' directory? Plugins shouldn't be doing that.

bergware commented 8 years ago

Ideally yes, but at the same time no strict rule is given or enforced to forbid such behaviour. It depends on the developer how well the plugins 'behave'.

I make the Dynamix plugins as much as possible autonomous by design, but I do have one plugin "local master" which replaces a page file in the Dynamix folder.

I needed to check why I did replace the file and not have the new version of that page in its own folder, the reason is that the page file in the dynamix folder always gets preferred (maybe we need to investigate that).

bergware commented 8 years ago

I made a correction in template.php for the plugins pages build (didn't see/realize before that actually 'dynamix' is executed twice).

bergware commented 8 years ago

And looking at the remarks in the forum, it is better to include a change log in the PLG file rather than referring to github...

limetech commented 8 years ago

re: changelog in plg file, well sure if you want to add the extra work. What I've been doing with releases is going through the commit log and just including the text of commits that have relevance to a user and grouping them under "webGui changes". For exmaple, I don't include the commit descriptions for thinks like code cleanups, or other development-type comments.

bergware commented 8 years ago

ok, template.php is better readable now.

changelog proposal

version 2016.01.22

version 2016.xx.xx

Need to write something, cause I am getting the blame ;)

limetech commented 8 years ago

Do you still want 'dynamix' as a system plugin? Ok well here's my attitude on it: First of all, I really want to discourage plugins (as you know) - especially for 'applications', those should be docker containers. Plugins should only be used to add functionality to the webGui itself, but of course people are free to do whatever they want.

As a developer, if there are conflicts with other plugins because of order of install, to me that should be fixed by plugin authors. But from business perspective, if you tell me there are numerous existing plugins that break now, well then we can make it a system plugin.

As long as we're on the subject of plugin conflict, maybe we should change that GLOB_NOSORT to GLOB_SORT because that will provide opportunity in future of using plugin name ordering to control installation.

bergware commented 8 years ago

There are currently 64 plugins available, according to the list of CA. I have no idea how well all of these behave, i.e. something may get broken with a new GUI install. I know from the Dynamix V5 days it was an issue with several of the Phaze plugins, I expect some 'issues' to appear, though so far the number of issues reported on the forum seem limited.

I am also wondering what should be the best approach going forward when new unRAID versions are introduced. These will have the latest GUI included, and something must be made to remove obsolete packages. Is this best done by deleting a complete separate folder (/boot/plugins) or selective deletion of files in an existing folder (/boot/config/plugins/dynamix)?

limetech commented 8 years ago

what should be the best approach going forward when new unRAID versions are introduced. These will have the latest GUI included, and something must be made to remove obsolete packages. Is this best done by deleting a complete separate folder (/boot/plugins) or selective deletion of files in an existing folder (/boot/config/plugins/dynamix)?

By "obsolete packages", do you mean the .plg file in /boot/plugins (the previous webGui release)?

Also what do you think of this idea:

This solves some problems:

what do you think?

bergware commented 8 years ago

My preference is also a single location to put plugins. Since the introduction of the plugin manager things are written to /boot/config/plugins. That is perfectly alright as long as a mechanism exists which ensures that the webGUI plugin gets loaded and executed first. This emulates the same behaviour as when the webGUI is built in.

Getting rid of legacy stuff can be safely done in this case.

limetech commented 8 years ago

Ok this is what we'll do then. I'll go ahead and merge this PR but not the changes to dynamix.plg. In addition, I will be releasing 6.1.8 with updated kernel 4.1.6 because it has the keyring ref link fix:

KEYS: Fix keyring ref leak in join_session_keyring()

commit 23567fd052a9abb6d67fe8e7a9ccdd9800a540f2 upstream.

This fixes CVE-2016-0728.