[
https://issues.apache.org/jira/browse/GUACAMOLE-32?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15305799#comment-15305799
]
ASF GitHub Bot commented on GUACAMOLE-32:
-----------------------------------------
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/10#discussion_r64998689
--- Diff: guacamole-docker/bin/start.sh ---
@@ -90,15 +90,21 @@ set_optional_property() {
## instead of a linked container.
##
associate_mysql() {
-
- # Use linked container if specified
- if [ -n "$MYSQL_NAME" ]; then
- MYSQL_HOSTNAME="$MYSQL_PORT_3306_TCP_ADDR"
- MYSQL_PORT="$MYSQL_PORT_3306_TCP_PORT"
- fi
-
- # Use default port if none specified
- MYSQL_PORT="${MYSQL_PORT-3306}"
+ : ${MYSQL_HOSTNAME:=mysql}
+ # overwrite "MYSQL_PORT" with default port if using linked container
+ if [ -n "$MYSQL_ENV_MYSQL_DATABASE" ]; then
+ MYSQL_PORT=3306
+ fi
+ # Use default port if none specified
+ : ${MYSQL_PORT:=3306}
+
+ # if we're linked to MySQL and thus have credentials already, let's use
them
+ : ${MYSQL_USER:=${MYSQL_ENV_MYSQL_USER:-root}}
--- End diff --
Defaulting the user (and password) to the root credentials seems awfully
scary. The documentation covers creating a restricted-privilege user with only
the necessary permissions to work with the Guacamole database.
I'd think it'd be far better to refuse to start the image and kick back an
error noting that required environment variables are missing.
> Extended variable support in guacamole-docker image for linked databases
> ------------------------------------------------------------------------
>
> Key: GUACAMOLE-32
> URL: https://issues.apache.org/jira/browse/GUACAMOLE-32
> Project: Guacamole
> Issue Type: Improvement
> Reporter: Michael Jumper
> Priority: Minor
>
> {panel:bgColor=#FFFFEE}
> *The description of this issue was copied from
> [GUAC-1545|https://glyptodon.org/jira/browse/GUAC-1545], an issue in the JIRA
> instance used by the Guacamole project prior to its acceptance into the
> Apache Incubator.*
> Comments, attachments, related issues, and history from prior to acceptance
> *have not been copied* and can be found instead at the original issue.
> {panel}
> The current Guacamole Docker image has some issues related to environment
> variables:
> # The link environmental variables, like {{MYSQL_PORT_3306_TCP_ADDR}}, are
> deprecated (see the [link environment variables
> reference|https://docs.docker.com/compose/link-env-deprecated/]). The name of
> the service, or alias, should be used to connect instead.
> # The current docker image is not leveraging all of the environmental
> variables of the linked database container. (e.g. {{MYSQL_ENV_MYSQL_USER}})
> # The current name of the environmental variables {{MYSQL_PORT}} and
> {{POSTGRES_PORT}} are not the best choice, because they are also used by
> Docker (though deprecated) for specifying the full connection URL to the
> linked container. e.g. {{tcp://172.17.0.5:5432}}
> I've created a pull request implementing the following changes:
> - Adding support for leveraging {{MYSQL_ENV_MYSQL_USER}},
> {{MYSQL_ENV_MYSQL_PASSWORD}}, {{MYSQL_ENV_MYSQL_DATABASE}} and
> {{MYSQL_ENV_MYSQL_ROOT_PASSWORD}} for linked
> - Removed deprecated environmental variables {{MYSQL_NAME}},
> {{MYSQL_PORT_3306_TCP_ADDR}} and {{MYSQL_PORT_3306_TCP_PORT}}
> - Adding support for leveraging {{POSTGRES_ENV_POSTGRES_USER}},
> {{POSTGRES_ENV_POSTGRES_PASSWORD}} and {{POSTGRES_ENV_POSTGRES_DB}} for linked
> - Removed deprecated environmental variables {{POSTGRES_NAME}},
> {{POSTGRES_PORT_5432_TCP_ADDR}} and {{POSTGRES_PORT_5432_TCP_PORT}}
> The modified branch can be found here:
> https://github.com/vchrisb/guacamole-docker/tree/linked-db-envs
> The modified Docker image is {{vchrisb/guacamole:linked-db-envs}}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
