GUACAMOLE-374: Generalize Docker image to automatically map environment variables to properties. #979
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I believe this rewrite maintains parity with the features of the existing image, except that fewer sanity checks are performed. This revised process instead relies on the webapp itself to appropriately log errors related to configuration. |
necouchman
reviewed
Apr 23, 2024
necouchman
requested changes
Apr 25, 2024
There was a problem hiding this comment.
A couple more minor things, but looks pretty good otherwise.
@jmuehlner Anything from you on this one?
… "*_FILE" environment variables.
…r configuring RemoteIpValve.
…ted with each enum value for EnumGuacamoleProperty.
mike-jumper
force-pushed
the
generalize-docker
branch
from
c16ef15
to
1b271a2
Compare
necouchman
approved these changes
Apr 25, 2024
Nope, seems like a reasonable change. If you're good with it, so am I. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change strips away much of the manual mapping of environment variables to properties, instead relying on automatic mapping of properties using
enable-environment-properties.*_FILEenvironment variables are also automatically mapped.Additionally:
log-levelproperty that maps to theLOG_LEVELenvironment variable.REMOTE_IP_VALVE_*environment variables have been renamed to align with the attributes being configured.GUACAMOLE_HOMEis now transient and randomly generated to allow users to use the familiar/etc/guacamolelocation for their configuration template without potential collisions. Users can even use/etc/guacamoledirectly without overridingGUACAMOLE_HOMEand rely on the image to combine what they've provided with anything the image has configured via environment variables.As of these changes, there are only two mappings that must be manually maintained (and then only when a new extension is introduced):
.jarfiles. This is handled byguacamole-docker/build.d/010-map-guacamole-extensions.sh..jarfiles (ie: JDBC drivers). This is handled byguacamole-docker/build.d/020-download-drivers.sh.Both of the above mappings/scripts organize things within a directory hierarchy that allows the entrypoint to easily link extensions, drivers, etc. into place based on which environment variables are set. Once the extensions are in place, everything else is implicit via
enable-environment-properties.In cases where environment variables are changing names, compatibility with the old naming is maintained with
guacamole-docker/entrypoint.d/000-migrate-legacy-variables.sh. Old variables are automatically migrated to the new ones and a warning regarding deprecation is logged.