You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by WojoInc <gi...@git.apache.org> on 2018/09/21 02:21:40 UTC
[GitHub] guacamole-client pull request #320: Guacamole-626 - Add support for Docker s...
GitHub user WojoInc opened a pull request:
https://github.com/apache/guacamole-client/pull/320
Guacamole-626 - Add support for Docker secrets to startup.sh
Did my best to follow the CONTRIBUTING, as I meant to submit these changes a while back.
JIRA: https://issues.apache.org/jira/browse/GUACAMOLE-626
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/WojoInc/guacamole-client guacamole-626
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/guacamole-client/pull/320.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #320
----
commit 106c030e52c406301ec2abf49de8b4767a0bc62b
Author: Thomas John Wesolowski <wo...@...>
Date: 2018-08-10T03:06:24Z
Guacamole-626: Add Docker secret support for MySQL and Postgres
Add support for reading from docker secret files. New script prefers environment variables ending with _FILE over normal variables, meaning that Docker secrets will take precedence. You can, however, mix variable types, ex. MYSQL_USER uses a normal environment variable, while MYSQL_PASSWORD uses a secret.
commit 0471c989dee19e0ed7fe0ed7f6a5193280a2f9c5
Author: Thomas John Wesolowski <wo...@...>
Date: 2018-08-10T03:25:23Z
Guacamole-626 Few additional changes to add secret support
Remove bug causing Docker secret for database file to prevent the script from completing successfully.
commit f9154a04b81561b843dd4a1f4380020b5485f1f5
Author: Thomas John Wesolowski <wo...@...>
Date: 2018-09-21T01:54:05Z
Guacamole-626 Update README.md
Add appropriate documentation for usage of Docker secrets
----
---
[GitHub] guacamole-client pull request #320: Guacamole-626 - Add support for Docker s...
Posted by necouchman <gi...@git.apache.org>.
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/320#discussion_r219652862
--- Diff: guacamole-docker/README.md ---
@@ -90,6 +107,9 @@ the image will stop:
1. `MYSQL_DATABASE` - The name of the database to use for Guacamole authentication.
2. `MYSQL_USER` - The user that Guacamole will use to connect to MySQL.
3. `MYSQL_PASSWORD` - The password that Guacamole will provide when connecting to MySQL as `MYSQL_USER`.
+4. `MYSQL_DATABASE_FILE` - The path of the docker secret containing the name of database to use for Guacamole authentication.
+5. `MYSQL_USER` - The path of the docker secret containing the name of the user that Guacamole will use to connect to MySQL.
--- End diff --
`MYSQL_USER` -> `MYSQL_USER_FILE`
---
[GitHub] guacamole-client pull request #320: Guacamole-626 - Add support for Docker s...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/320#discussion_r219370018
--- Diff: guacamole-docker/bin/start.sh ---
@@ -141,16 +141,39 @@ environment variables:
MYSQL_DATABASE The name of the MySQL database to use for Guacamole
authentication.
-END
+END`
--- End diff --
I believe that trailing backtick here will result in the `END` failing to match the `<<END`, as the shell will be searching for a line that contains nothing but `END`. Can you confirm whether this works as expected?
---
[GitHub] guacamole-client pull request #320: Guacamole-626 - Add support for Docker s...
Posted by necouchman <gi...@git.apache.org>.
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/320#discussion_r219652808
--- Diff: guacamole-docker/README.md ---
@@ -45,6 +56,9 @@ the image will stop:
1. `POSTGRES_DATABASE` - The name of the database to use for Guacamole authentication.
2. `POSTGRES_USER` - The user that Guacamole will use to connect to PostgreSQL.
3. `POSTGRES_PASSWORD` - The password that Guacamole will provide when connecting to PostgreSQL as `POSTGRES_USER`.
+4. `POSTGRES_DATABASE_FILE` - The path of the docker secret containing the name of database to use for Guacamole authentication.
+5. `POSTGRES_USER` - The path of the docker secret containing the name of the user that Guacamole will use to connect to PostgreSQL.
--- End diff --
This should probably be `POSTGRES_USER_FILE` if it's the "path of the docker secret," no?
---
[GitHub] guacamole-client pull request #320: Guacamole-626 - Add support for Docker s...
Posted by necouchman <gi...@git.apache.org>.
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/320#discussion_r219652878
--- Diff: guacamole-docker/README.md ---
@@ -90,6 +107,9 @@ the image will stop:
1. `MYSQL_DATABASE` - The name of the database to use for Guacamole authentication.
2. `MYSQL_USER` - The user that Guacamole will use to connect to MySQL.
3. `MYSQL_PASSWORD` - The password that Guacamole will provide when connecting to MySQL as `MYSQL_USER`.
+4. `MYSQL_DATABASE_FILE` - The path of the docker secret containing the name of database to use for Guacamole authentication.
+5. `MYSQL_USER` - The path of the docker secret containing the name of the user that Guacamole will use to connect to MySQL.
+6. `MYSQL_PASSWORD` - The path of the docker secret containing the password that Guacamole will provide when connecting to MySQL as `MYSQL_USER`.
--- End diff --
`MYSQL_PASSWORD` -> `MYSQL_PASSWORD_FILE`
---
[GitHub] guacamole-client pull request #320: Guacamole-626 - Add support for Docker s...
Posted by WojoInc <gi...@git.apache.org>.
Github user WojoInc commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/320#discussion_r219374952
--- Diff: guacamole-docker/bin/start.sh ---
@@ -141,16 +141,39 @@ environment variables:
MYSQL_DATABASE The name of the MySQL database to use for Guacamole
authentication.
-END
+END`
--- End diff --
My very rough example compose file for a guacamole stack:
```yaml
version: "3.1"
services:
daemon:
image: guacamole/guacd:latest
networks:
- guac
client:
image: wojoinc/guac-test:latest
networks:
- guac
- db
ports:
- "8081:8080"
environment:
GUACD_HOSTNAME: "daemon"
GUACD_PORT: "4822"
MYSQL_HOSTNAME: "db"
#MYSQL_DATABASE_FILE: "/run/secrets/guac-mysql-db"
MYSQL_PORT: "3306"
MYSQL_USER_FILE: "/run/secrets/guac-mysql-user"
MYSQL_PASSWORD_FILE: "/run/secrets/guac-mysql-password"
secrets:
- guac-mysql-db
- guac-mysql-user
- guac-mysql-password
db:
image: mariadb:latest
networks:
- db
environment:
MYSQL_ROOT_PASSWORD_FILE: "/run/secrets/guac-mysql-root-password"
MYSQL_DATABASE: "guacamole"
MYSQL_USER_FILE: "/run/secrets/guac-mysql-user"
MYSQL_PASSWORD: "/run/secrets/guac-mysql-password"
MYSQL_ROOT_HOST: "localhost"
volumes:
- "db_data:/var/lib/mysql"
- "./initdb.sql:/docker-entrypoint-initdb.d/dump.sql"
secrets:
- guac-mysql-root-password
- guac-mysql-db
- guac-mysql-user
- guac-mysql-password
networks:
guac:
db:
secrets:
guac-mysql-root-password:
external: true
guac-mysql-db:
external: true
guac-mysql-user:
external: true
guac-mysql-password:
external: true
volumes:
db_data:
```
When omitting MYSQL_DATABASE or MYSQL_DATABASE_FILE, this causes the container to fail to spin up, and the logs present the error message from before calling associate_mysql() located at the bottom of the file,
```
FATAL: No authentication configured
-------------------------------------------------------------------------------
The Guacamole Docker container needs at least one authentication mechanism in
order to function, such as a MySQL database, PostgreSQL database, or LDAP
directory. Please specify at least the MYSQL_DATABASE or POSTGRES_DATABASE
environment variables, or check Guacamole's Docker documentation regarding
configuring LDAP and/or custom extensions.
```
If you omit MYSQL_USER for example however, it does appear to read beyond the trailing backtick. I made the change to remove it, and I am rebuilding a test image now to see if it still works with that back tick removed. Thanks for letting me know!
---
[GitHub] guacamole-client pull request #320: Guacamole-626 - Add support for Docker s...
Posted by necouchman <gi...@git.apache.org>.
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/320#discussion_r219650390
--- Diff: guacamole-docker/README.md ---
@@ -28,14 +28,25 @@ Once the Guacamole image is running, Guacamole will be accessible at
`-p 8080:8080` option to expose this port at the level of the machine hosting
Docker, as well.
+Docker Secrets
+==============
+The string `_FILE` may be appended to some of the environment variables listed below if you are using MySQL or PostgreSQL authentication. This will cause the startup script to load the values for those variables from files within in the container. This is useful for specifying sensitive info, ie. passwords for the database, in secured files instead of plaintext environment variables, and is generally used for loading values from [Docker secrets](https://docs.docker.com/engine/swarm/secrets/#read-more-about-docker-secret-commands), which are stored in `/run/secrets/<secret_name>` within the container.
+
--- End diff --
A couple of issues, here:
- In the rest of the README.md file, here, lines are formatted with a roughly similar maximum column size. It looks like you've got a single very long line, here - please reformat these lines (and the others you've added below) to match the style used throughout the rest of the file.
- There is a mistake in the line "for those variables from files within in the" should be "for those variables from files within the".
---
[GitHub] guacamole-client pull request #320: Guacamole-626 - Add support for Docker s...
Posted by necouchman <gi...@git.apache.org>.
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/320#discussion_r219652842
--- Diff: guacamole-docker/README.md ---
@@ -45,6 +56,9 @@ the image will stop:
1. `POSTGRES_DATABASE` - The name of the database to use for Guacamole authentication.
2. `POSTGRES_USER` - The user that Guacamole will use to connect to PostgreSQL.
3. `POSTGRES_PASSWORD` - The password that Guacamole will provide when connecting to PostgreSQL as `POSTGRES_USER`.
+4. `POSTGRES_DATABASE_FILE` - The path of the docker secret containing the name of database to use for Guacamole authentication.
+5. `POSTGRES_USER` - The path of the docker secret containing the name of the user that Guacamole will use to connect to PostgreSQL.
+6. `POSTGRES_PASSWORD` - The path of the docker secret containing the password that Guacamole will provide when connecting to PostgreSQL as `POSTGRES_USER`.
--- End diff --
This should probably be `POSTGRES_PASSWORD_FILE` if it's the "path of the docker secret," no?
---