search

virtualmin / virtualmin-gpl

Virtualmin web hosting control panel for Webmin
https://www.virtualmin.com
GNU General Public License v3.0
331 stars 102 forks source link

PostgreSQL offered in 'Features and Plugins' even if not installed/configured #109

Open PitWenkin opened 5 years ago

PitWenkin commented 5 years ago

I've never installed PostgreSQL, however PostgreSQL database is listed among Features and Plugins. PostgreSQL-1

The whole PostgreSQL Database Server-module is unused, and shows the following: PostgreSQL-2

I checked if some PostgreSQL-packages where installed but the only package matching 'postgre' is postgrey from Postfix

Operating system Ubuntu Linux 18.04.3 Webmin version 1.930 Usermin version 1.780 Virtualmin version 6.07 Authentic theme version 19.39-2

https://github.com/virtualmin/virtualmin-gpl/blob/1a70293487dba271845f6782c91f03ad31291738/edit_newfeatures.cgi#L20

PitWenkin commented 3 years ago

Still the case on a newly installed system:

Webmin version | 1.973 Usermin version | 1.823 Virtualmin version | 6.16

iliajie commented 3 years ago

Hi,

Do you expect it to be hidden on Features and Plugins page if PostgreSQL package isn't installed?

iliajie commented 3 years ago

What would you expect? Would you like having features that are unused being hidden? @jcameron Jamie, what do you think of hiding core features that cannot be used, like in case of PostgreSQL, when system package is not installed? Do you mind hiding them?

PitWenkin commented 3 years ago

It's a bit strange to see features listed I cannot use.

If at least they were somehow marked as not installed …?

iliajie commented 3 years ago

If at least they were somehow marked as not installed …?

Yes, I think we could do it, unless Jamie thinks that hiding it completely would be a better option.

swelljoe commented 3 years ago

This shouldn't be a theme-specific implementation. Better if it goes into Virtualmin proper, if we're doing it. But, I also am not sure if this is the right thing. Features and Plugins is a root-user page, and it's the source of truth on what features are available in the UI elsewhere. It's the one place where I don't think we should be trying to hide everything.

It might be worth breaking the databases out into their own plugin modules, so that uninstalling e.g. virtualmin-postgresql would make it disappear from Features and Plugins? But, I'm hesitant to make it a separate hiding process.

iliajie commented 3 years ago

This shouldn't be a theme-specific implementation. Better if it goes into Virtualmin proper, if we're doing it.

Yes, certainly, never meant to make it a theme based feature.

It might be worth breaking the databases out into their own plugin modules, so that uninstalling e.g. virtualmin-postgresql

That would be great but due to the design it wouldn't be possible. I wonder at the moment, if disabling a module in acls actually would remove it from this page. That would be reasonable and a right thing to do. I'll have a look tomorrow.

But, I also am not sure if this is the right thing. Features and Plugins is a root-user page, and it's the source of truth on what features are available in the UI elsewhere.

Understandable. However specifically applied to databases I personally wouldn't mind making it disabled (like first row with home) or even removing it (to avoid confusion) from the list (hiding it) if only either or neither is installed. The whole implementation would be is to check for mysql and postgresql command existence. Although I like the idea of hiding it if the module is not available in acls.

swelljoe commented 3 years ago

That would be great but due to the design it wouldn't be possible.

That's not true at all. We already have database plugins (Oracle, SQLite, maybe others?) that are independent of Virtualmin.

I wonder at the moment, if disabling a module in acls actually would remove it from this page.

Again, this is for the root user. ACLs aren't a thing that is reasonable to use for the root user.

Greying it out if postgresql isn't installed/configured/enabled would be fine. I don't think it needs to be disappeared.

iliajie commented 3 years ago

That's not true at all. We already have database plugins (Oracle, SQLite, maybe others?) that are independent of Virtualmin.

Well, it seemed to me that there were only one feature-sql that was abstracted for both MySQL and PostgreSQL, so I doubled checked, and it appeared that there is feature-mysql andfeature-postgresql as well. I also remembered the patches I made specifically for MySQL were on that file solely. You're right! I assume it should be doable then (separate it to installable package).

Again, this is for the root user. ACLs aren't a thing that is reasonable to use for the root user.

Why not if an admin wanted to remove something out of sight, by unchecking it from Available Webmin Modules in Webmin Users, I would expect the given feature to be removed from the list as well, just the way module's link is getting removed from the navigation menu. Common practice to disable all modules you don't use for clarity of the UI.

Greying it out if postgresql isn't installed/configured/enabled would be fine. I don't think it needs to be disappeared.

Perhaps if it would be greyed out on the UI, without giving particular info could lead to more questions?

iliajie commented 3 years ago

@jcameron Jamie, do you think it would be reasonable not to show PostgreSQL database feature on System Settings ⇾ Features and Plugins page if PostgreSQL module is disabled on System Settings ⇾ Features and Plugins: Edit Webmin User page?

At the moment it's not happening, while I think it should, as feature-postgresql is dependent on the Webmin postgresql module at the first place.

swelljoe commented 3 years ago

Why not if an admin wanted to remove something out of sight, by unchecking it from Available Webmin Modules in Webmin Users, I would expect the given feature to be removed from the list as well

That's Virtualmin domain owner users. It doesn't make sense for that page to limit what root can see or do. I don't feel strongly about providing a way to hide it, but I feel really strongly it shouldn't be this.

Edit: Oh, wait, you mean in Webmin, not Virtualmin. I guess that's fine.

iliajie commented 3 years ago

I don't feel strongly about providing a way to hide it, but I feel really strongly it shouldn't be this.

Yes, it could be greyed out (as you suggested) earlier. The problem is that making it "disabled" by removing checkboxes make it impossible for an admin to try to enable it and get a clear error message, like:

image

Edit: Oh, wait, you mean in Webmin, not Virtualmin. I guess that's fine.

Yes, I meant cosmetics in this case. Features and Plugins page is admin like page. MySQL and PostgreSQL libs in Virtualmin they don't check for module availability (i.e. if module is opt-out or opt-in in Webmin Usesr module (acl)), and Virtualmin libs (feature-mysql and feature-postgresql) just require the module directory, as otherwise it wouldn't work for other users.

But if picky admin feels like he doesn't wanna see it on the features and plugins list in Virtualmin, then they would be able to use Webmin Users module to remove it from acls. By doing so it will disappear from the left menu in Webmin and from Features and Plugins page. Lemmi see ..

iliajie commented 3 years ago

There, Joe, check this PR out - https://github.com/virtualmin/virtualmin-gpl/pull/276