GEOS-11555: fix installation of extensions in various configurations

[spike83]

With this PR, I want to address the issues GEOS-11555 and GEOS-11258 about some inabilities of the extension installer script.

In my opinion, it should be possible to add community modules also to official images. So, I added a warning message instead of blocking it.

Also, I tried to get rid of the confusion in the naming of the variable INSTALL_EXTENSIONS which was controlling the download behavior. Thus, I introduced a variable DOWNLOAD_EXTENSIONS which controls this and made the INSTALL_EXTENSIONS to really control the installation of extensions.

On one point, I'm not sure: Should we fail the startup when we are unable to download extensions?
On one hand, the full functionality might not he available and cannot be detected with any sort of health check and on the other hand, maybe it's better to run without an extension than not run at all.

[reinout]

I think it is clearer to end both URLs in the same way (with or without slash). Without slash is visually clearer (and matches the original COMMUNITY_PLUGIN_URL).

Suggested change

	| COMMUNITY_PLUGIN_BASE_URL | Specifies the **base** URL for downloading the latest community-contributed GeoServer plugins. | `https://build.geoserver.org/geoserver/` | |
	| COMMUNITY_PLUGIN_URL | Specifies the URL for downloading the latest community-contributed GeoServer plugins | `${COMMUNITY_PLUGIN_BASE_URL}2.25.x/community-latest` |
	| COMMUNITY_PLUGIN_BASE_URL | Specifies the **base** URL for downloading the latest community-contributed GeoServer plugins. | `https://build.geoserver.org/geoserver` | |
	| COMMUNITY_PLUGIN_URL | Specifies the URL for downloading the latest community-contributed GeoServer plugins | `${COMMUNITY_PLUGIN_BASE_URL}/2.25.x/community-latest` |

[reinout]

Although... I now notice that ADDITIONAL_LIBS_DIR seems to include the trailing slash. In that case, end both of these URLs with a slash?

[reinout]

Wasn't it your intention to remove this --no-check-certificate? (Which is a good idea to remove in my opinion :-) )

[spike83]

Yes, it is removed by this PR, but there is the variable WGET_OPTS in case you would for some reason need to set some options for wget. By default, this is empty, just mentioning a sample value here.

[reinout]

Sorry for the multiple comments, I should have done it properly as a single review. I thought I had only one quick suggestion :-)

Fixing up the if/else errors and the community plugin installs in the install script seems like a good idea.

One thing that worries me a bit are the extra environment variables. There are a lot already. Could your fix work without the extra environment variables?

• DOWNLOAD_EXTENSIONS: if you specify an extension, it should be fine to download it if missing. Why the extra check?
• COMMUNITY_PLUGIN_BASE_URL: a variable in the bash script is probably handy, but I don't think it needs to be configurable?

[spike83]

DOWNLOAD_EXTENSIONS is basically the functionality from the former INSTALL_EXTENSIONS variable to disable the download. This will prevent some errors when run in an environment where the geoserver has no internet access.

COMMUNITY_PLUGIN_BASE_URL allows changing the server for the build artifacts but keep the logic of the structure underneath it.

I BTW missed one ;-))

It could make sense to split them into multiple tables to get some clarity and better overview. Suggesting Geoserver, Tomcat, Extensions

[jodygarnett]

I would be very unhappy with installing community modules into a release intended for production - the point of them is to get feedback on snapshot / nightly build releases to support development. If people want to use extensions in production - stop messing around and fund / complete them.

Really we are already pushing our luck distributing these as nightly builds as they have not been given to the project steering committee for review and publication yet.