You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2021/09/16 08:42:27 UTC

[GitHub] [guacamole-client] jbpaux opened a new pull request #643: GUACAMOLE-1418: Add support of SQL Server JDBC plugin in Docker Image

jbpaux opened a new pull request #643:
URL: https://github.com/apache/guacamole-client/pull/643


   Implement SQL Server support in Docker Image.
   According to JIRA issue, need to check if it's legally allowed (saw other ASF software are using it so I guess it could be)
   Have a good review :)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] necouchman commented on pull request #643: GUACAMOLE-1418: Add support of SQL Server JDBC plugin in Docker Image

Posted by GitBox <gi...@apache.org>.
necouchman commented on pull request #643:
URL: https://github.com/apache/guacamole-client/pull/643#issuecomment-920836059


   > saw other ASF software are using it so I guess it could be
   
   Can you provide these examples?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] jbpaux edited a comment on pull request #643: GUACAMOLE-1418: Add support of SQL Server JDBC plugin in Docker Image

Posted by GitBox <gi...@apache.org>.
jbpaux edited a comment on pull request #643:
URL: https://github.com/apache/guacamole-client/pull/643#issuecomment-920837352


   Here for example: https://github.com/apache/skywalking-java/blob/e6447d517558031f49cc4450467e41b644320071/apm-sniffer/apm-sdk-plugin/mssql-jdbc-plugin/pom.xml#L53 (pom dependency)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] jbpaux commented on pull request #643: GUACAMOLE-1418: Add support of SQL Server JDBC plugin in Docker Image

Posted by GitBox <gi...@apache.org>.
jbpaux commented on pull request #643:
URL: https://github.com/apache/guacamole-client/pull/643#issuecomment-920837352


   Here for example: https://github.com/apache/skywalking-java/blob/e6447d517558031f49cc4450467e41b644320071/apm-sniffer/apm-sdk-plugin/mssql-jdbc-plugin/pom.xml (pom dependency)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] jbpaux commented on pull request #643: GUACAMOLE-1418: Add support of SQL Server JDBC plugin in Docker Image

Posted by GitBox <gi...@apache.org>.
jbpaux commented on pull request #643:
URL: https://github.com/apache/guacamole-client/pull/643#issuecomment-920849169


   Ok, no pb, here then : https://github.com/apache/juddi/blob/5988d077548d54da86c982295e48df3e771ae53e/juddi-tomcat/pom.xml#L88 no provided I think (I'm not java expert so maybe I guess wrong 😄 )


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] necouchman commented on pull request #643: GUACAMOLE-1418: Add support of SQL Server JDBC plugin in Docker Image

Posted by GitBox <gi...@apache.org>.
necouchman commented on pull request #643:
URL: https://github.com/apache/guacamole-client/pull/643#issuecomment-920847262


   @jbpaux In the case you mentioned, the pom.xml file is actually _not_ pulling in the download - it lists it as a dependency, but the `<scope>provided</scope>` tag on that dependency means that Maven will proceed building without actually pulling in those files, and will rely on them being provided by the JDK/JRE at runtime. See the `provided` description in the following section:
   
   https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Dependency_Scope


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] jbpaux commented on a change in pull request #643: GUACAMOLE-1418: Add support of SQL Server JDBC plugin in Docker Image

Posted by GitBox <gi...@apache.org>.
jbpaux commented on a change in pull request #643:
URL: https://github.com/apache/guacamole-client/pull/643#discussion_r775050134



##########
File path: guacamole-docker/bin/start.sh
##########
@@ -403,6 +403,142 @@ END
 
 }
 
+# Print error message regarding missing required variables for SQLServer authentication
+sqlserver_missing_vars() {
+    cat <<END
+FATAL: Missing required environment variables
+-------------------------------------------------------------------------------
+If using a SQLServer database, you must provide each of the following
+environment variables or their corresponding Docker secrets by appending _FILE
+to the environment variable, and setting the value to the path of the
+corresponding secret:
+
+    SQLSERVER_USER     The user to authenticate as when connecting to
+                       SQLServer.
+
+    SQLSERVER_PASSWORD The password to use when authenticating with SQLServer
+                       as SQLSERVER_USER.
+
+    SQLSERVER_DATABASE The name of the SQLServer database to use for Guacamole
+                       authentication.

Review comment:
       Updated too, hopefully it's ok.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] necouchman commented on a change in pull request #643: GUACAMOLE-1418: Add support of SQL Server JDBC plugin in Docker Image

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #643:
URL: https://github.com/apache/guacamole-client/pull/643#discussion_r775018945



##########
File path: guacamole-docker/bin/start.sh
##########
@@ -403,6 +403,142 @@ END
 
 }
 
+# Print error message regarding missing required variables for SQLServer authentication
+sqlserver_missing_vars() {
+    cat <<END
+FATAL: Missing required environment variables
+-------------------------------------------------------------------------------
+If using a SQLServer database, you must provide each of the following
+environment variables or their corresponding Docker secrets by appending _FILE
+to the environment variable, and setting the value to the path of the
+corresponding secret:
+
+    SQLSERVER_USER     The user to authenticate as when connecting to
+                       SQLServer.
+
+    SQLSERVER_PASSWORD The password to use when authenticating with SQLServer
+                       as SQLSERVER_USER.
+
+    SQLSERVER_DATABASE The name of the SQLServer database to use for Guacamole
+                       authentication.

Review comment:
       Similar to the documentation section, above, this should also probably be reworked to take into account using either these three variables or the `_FILE` versions for Docker secret support. Again, I know it matches the Postgres and MySQL versions, so those should be reworked later, but since we're adding these, here, I think it makes sense to do it correctly.

##########
File path: guacamole-docker/README.md
##########
@@ -163,6 +163,63 @@ The process for doing this via the `mysql` utility included with MySQL is
 documented in
 [the Guacamole manual](http://guacamole.apache.org/doc/gug/jdbc-auth.html#jdbc-auth-mysql).
 
+Deploying Guacamole with SQLServer authentication
+--------------------------------------------------
+
+    docker run --name some-guacamole --link some-guacd:guacd \
+        --link some-sqlserver:sqlserver      \
+        -e SQLSERVER_DATABASE=guacamole_db  \
+        -e SQLSERVER_USER=guacamole_user    \
+        -e SQLSERVER_PASSWORD=some_password \
+        -e SQLSERVER_DATABASE_FILE=/run/secrets/<secret_name> \
+        -e SQLSERVER_USER_FILE=/run/secrets/<secret_name> \
+        -e SQLSERVER_PASSWORD_FILE=/run/secrets/<secret_name> \
+        -d -p 8080:8080 guacamole/guacamole
+
+Linking Guacamole to SQLServer requires three environment variables. If any of
+these environment variables are omitted, you will receive an error message, and
+the image will stop:
+
+1. `SQLSERVER_DATABASE` - The name of the database to use for Guacamole
+   authentication.
+2. `SQLSERVER_USER` - The user that Guacamole will use to connect to SQLServer.
+3. `SQLSERVER_PASSWORD` - The password that Guacamole will provide when
+   connecting to SQLServer as `SQLSERVER_USER`.
+4. `SQLSERVER_DATABASE_FILE` - The path of the docker secret containing the name
+   of database to use for Guacamole authentication.
+5. `SQLSERVER_USER_FILE` - The path of the docker secret containing the name of
+   the user that Guacamole will use to connect to SQLServer.
+6. `SQLSERVER_PASSWORD_FILE` - The path of the docker secret containing the
+   password that Guacamole will provide when connecting to SQLServer as
+   `SQLSERVER_USER.

Review comment:
       I think this section is a bit confusing, as it says that SQLServer authentication "requires three variable" and goes on to list 6. I know you copied the sections above, for MySQL and PostgreSQL, and those need to be separately reviewed and reworked, but it seems like it may make more sense to leave lines 179-187 as-is, and then put a break, and say something like, "Alternatively, if you want to store database credentials using Docker secrets, the following three variables are required and replace the previous three:", and then list the three _FILE variables.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] mike-jumper commented on pull request #643: GUACAMOLE-1418: Add support of SQL Server JDBC plugin in Docker Image

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on pull request #643:
URL: https://github.com/apache/guacamole-client/pull/643#issuecomment-921124651


   The JDBC driver provided by Microsoft for SQL Server is MIT-licensed and should be fine to bundle in the Docker image: https://github.com/microsoft/mssql-jdbc


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] jbpaux edited a comment on pull request #643: GUACAMOLE-1418: Add support of SQL Server JDBC plugin in Docker Image

Posted by GitBox <gi...@apache.org>.
jbpaux edited a comment on pull request #643:
URL: https://github.com/apache/guacamole-client/pull/643#issuecomment-920849169


   Ok, no pb, here then : https://github.com/apache/juddi/blob/5988d077548d54da86c982295e48df3e771ae53e/juddi-tomcat/pom.xml#L88 no provided I think (I'm not java expert so maybe I guess wrong 😄 )
   And the associated commit : https://github.com/apache/juddi/commit/c6e69a48bfe61dde00a8ff8873bb8f946e06e4ea


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] jbpaux commented on a change in pull request #643: GUACAMOLE-1418: Add support of SQL Server JDBC plugin in Docker Image

Posted by GitBox <gi...@apache.org>.
jbpaux commented on a change in pull request #643:
URL: https://github.com/apache/guacamole-client/pull/643#discussion_r775172043



##########
File path: guacamole-docker/bin/start.sh
##########
@@ -23,7 +23,7 @@
 ##
 ## Automatically configures and starts Guacamole under Tomcat. Guacamole's
 ## guacamole.properties file will be automatically generated based on the
-## linked database container (either MySQL or PostgreSQL) and the linked guacd
+## linked database container (either MySQL,PostgreSQL or SQLServer) and the linked guacd

Review comment:
       done :)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] jbpaux commented on a change in pull request #643: GUACAMOLE-1418: Add support of SQL Server JDBC plugin in Docker Image

Posted by GitBox <gi...@apache.org>.
jbpaux commented on a change in pull request #643:
URL: https://github.com/apache/guacamole-client/pull/643#discussion_r775050106



##########
File path: guacamole-docker/README.md
##########
@@ -163,6 +163,63 @@ The process for doing this via the `mysql` utility included with MySQL is
 documented in
 [the Guacamole manual](http://guacamole.apache.org/doc/gug/jdbc-auth.html#jdbc-auth-mysql).
 
+Deploying Guacamole with SQLServer authentication
+--------------------------------------------------
+
+    docker run --name some-guacamole --link some-guacd:guacd \
+        --link some-sqlserver:sqlserver      \
+        -e SQLSERVER_DATABASE=guacamole_db  \
+        -e SQLSERVER_USER=guacamole_user    \
+        -e SQLSERVER_PASSWORD=some_password \
+        -e SQLSERVER_DATABASE_FILE=/run/secrets/<secret_name> \
+        -e SQLSERVER_USER_FILE=/run/secrets/<secret_name> \
+        -e SQLSERVER_PASSWORD_FILE=/run/secrets/<secret_name> \
+        -d -p 8080:8080 guacamole/guacamole
+
+Linking Guacamole to SQLServer requires three environment variables. If any of
+these environment variables are omitted, you will receive an error message, and
+the image will stop:
+
+1. `SQLSERVER_DATABASE` - The name of the database to use for Guacamole
+   authentication.
+2. `SQLSERVER_USER` - The user that Guacamole will use to connect to SQLServer.
+3. `SQLSERVER_PASSWORD` - The password that Guacamole will provide when
+   connecting to SQLServer as `SQLSERVER_USER`.
+4. `SQLSERVER_DATABASE_FILE` - The path of the docker secret containing the name
+   of database to use for Guacamole authentication.
+5. `SQLSERVER_USER_FILE` - The path of the docker secret containing the name of
+   the user that Guacamole will use to connect to SQLServer.
+6. `SQLSERVER_PASSWORD_FILE` - The path of the docker secret containing the
+   password that Guacamole will provide when connecting to SQLServer as
+   `SQLSERVER_USER.

Review comment:
       Yes you're right, I was just following the other examples. I've updated the text with your instructions.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] necouchman commented on a change in pull request #643: GUACAMOLE-1418: Add support of SQL Server JDBC plugin in Docker Image

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #643:
URL: https://github.com/apache/guacamole-client/pull/643#discussion_r775078942



##########
File path: guacamole-docker/bin/start.sh
##########
@@ -23,7 +23,7 @@
 ##
 ## Automatically configures and starts Guacamole under Tomcat. Guacamole's
 ## guacamole.properties file will be automatically generated based on the
-## linked database container (either MySQL or PostgreSQL) and the linked guacd
+## linked database container (either MySQL,PostgreSQL or SQLServer) and the linked guacd

Review comment:
       #NitPick, but missing a space, here, after the comma...




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] necouchman merged pull request #643: GUACAMOLE-1418: Add support of SQL Server JDBC plugin in Docker Image

Posted by GitBox <gi...@apache.org>.
necouchman merged pull request #643:
URL: https://github.com/apache/guacamole-client/pull/643


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] jbpaux commented on pull request #643: GUACAMOLE-1418: Add support of SQL Server JDBC plugin in Docker Image

Posted by GitBox <gi...@apache.org>.
jbpaux commented on pull request #643:
URL: https://github.com/apache/guacamole-client/pull/643#issuecomment-927628082


   Any feedback about this PR @mike-jumper @necouchman ? 😅 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] jbpaux commented on pull request #643: GUACAMOLE-1418: Add support of SQL Server JDBC plugin in Docker Image

Posted by GitBox <gi...@apache.org>.
jbpaux commented on pull request #643:
URL: https://github.com/apache/guacamole-client/pull/643#issuecomment-1000826035


   Hi, do you plan to merge it or it requires some updates  @necouchman  ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org