comet-ml / opik

Open-source end-to-end LLM Development Platform
Apache License 2.0
2.12k stars 131 forks source link

[OPIK-135] Add support to RDS auth for MySQL #306

Closed thiagohora closed 1 month ago

thiagohora commented 1 month ago

Details

Screenshot 2024-09-24 at 12 08 59

Issues

OPIK-135

Testing

Locally, assuming a ROLE with the required access to RDS.

Documentation

In order to enable the Authentication using the AWS IAM mechanism, the user needs the following:

  1. Enable IAM DB authentication. See this link
  2. Create an IAM Policy for IAM database access. See link
  3. Setting AWSAuthenticationPlugin as authentication mode for the target MySQL user. See link.
  4. Setup AWS credentials. See link.
  5. Change database driver to software.amazon.jdbc.Driver instead of com.mysql.cj.jdbc.Driver
    # Use the new env var
    STATE_DB_DRIVER_CLASS=software.amazon.jdbc.Driver
  6. Mofify env var: STATE_DB_PROTOCOL to update the connection string. Use jdbc:aws-wrapper:mysql:// instead of jdbc:mysql://
  7. Set the STATE_DB_PLUGINS to iam or other valid plugins. For more valid options, see link
    STATE_DB_PLUGINS='iam'
  8. Set STATE_DB_PASS to empty.
thiagohora commented 1 month ago

nice! what additional parameters do we add for URL ? i think its cleaner to have an additional config and not url changes. but this looks ok for now

&wrapperPlugins=iam is used to enable the IAM auth, the issue that Dropwizard defines in the DB configuration schema. What we can do is Document how to create a volume to change the config.yml

thiagohora commented 1 month ago

Hi @andrescrz and @Nimrod007,

The E2E tests are failing because the variable changes are not backward compatible.

 Run docker logs opik-backend-1 > /home/runner/work/opik/opik/opik-backend_p3.9.log
java.sql.SQLException: Driver:com.mysql.cj.jdbc.Driver@5aaaa446 returned null for URL:mysql:3306/opik?createDatabaseIfNotExist=true&rewriteBatchedStatements=true
    at org.apache.tomcat.jdbc.pool.PooledConnection.connectUsingDriver(PooledConnection.java:365)
    at org.apache.tomcat.jdbc.pool.PooledConnection.connect(PooledConnection.java:227)
    at org.apache.tomcat.jdbc.pool.ConnectionPool.createConnection(ConnectionPool.java:779)
    at org.apache.tomcat.jdbc.pool.ConnectionPool.borrowConnection(ConnectionPool.java:707)
    at org.apache.tomcat.jdbc.pool.ConnectionPool.init(ConnectionPool.java:506)
    at org.apache.tomcat.jdbc.pool.ConnectionPool.<init>(ConnectionPool.java:155)
    at org.apache.tomcat.jdbc.pool.DataSourceProxy.pCreatePool(DataSourceProxy.java:[11](https://github.com/comet-ml/opik/actions/runs/11010029638/job/30571004042?pr=306#step:8:12)8)
    at org.apache.tomcat.jdbc.pool.DataSourceProxy.createPool(DataSourceProxy.java:107)
andrescrz commented 1 month ago

Hi @andrescrz and @Nimrod007,

The E2E tests are failing because the variable changes are not backward compatible.

Run docker logs opik-backend-1 > /home/runner/work/opik/opik/opik-backend_p3.9.log
java.sql.SQLException: Driver:com.mysql.cj.jdbc.Driver@5aaaa446 returned null for URL:mysql:3306/opik?createDatabaseIfNotExist=true&rewriteBatchedStatements=true
   at org.apache.tomcat.jdbc.pool.PooledConnection.connectUsingDriver(PooledConnection.java:365)
   at org.apache.tomcat.jdbc.pool.PooledConnection.connect(PooledConnection.java:227)
   at org.apache.tomcat.jdbc.pool.ConnectionPool.createConnection(ConnectionPool.java:779)
   at org.apache.tomcat.jdbc.pool.ConnectionPool.borrowConnection(ConnectionPool.java:707)
   at org.apache.tomcat.jdbc.pool.ConnectionPool.init(ConnectionPool.java:506)
   at org.apache.tomcat.jdbc.pool.ConnectionPool.<init>(ConnectionPool.java:155)
   at org.apache.tomcat.jdbc.pool.DataSourceProxy.pCreatePool(DataSourceProxy.java:[11](https://github.com/comet-ml/opik/actions/runs/11010029638/job/30571004042?pr=306#step:8:12)8)
   at org.apache.tomcat.jdbc.pool.DataSourceProxy.createPool(DataSourceProxy.java:107)

Thanks for letting us know. This would've passed if the end 2 end tests Github action was using an image containing the changes in the PR instead of the latest main version.

thiagohora commented 1 month ago

Added disbaled test to help/test setup