Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A way to pass SSL mode to the Vertx pgClient #1108

Closed
guy1699 opened this issue Jan 4, 2022 · 13 comments
Closed

A way to pass SSL mode to the Vertx pgClient #1108

guy1699 opened this issue Jan 4, 2022 · 13 comments

Comments

@guy1699
Copy link

guy1699 commented Jan 4, 2022

Today there is no way to pass SSL mode by property or URL param when using Postgres.
Correct me if I'm wrong, the only client available is Vertx and it is using io.vertx.pgclient.PgConnectOptions.

The constructor of PgConnectOptions does not inherit the SSL mode from the SqlConnectOptions which Hibernate sends.
https://github.com/eclipse-vertx/vertx-sql-client/blob/352e8da205598ffd9730ef36891dbc28ff544e53/vertx-pg-client/src/main/java/io/vertx/pgclient/PgConnectOptions.java#L139

The DefaultSqlConnectOptions also does not seem to have an option for SSL to be configured, is that correct?

@blafond
Copy link
Member

blafond commented Jan 4, 2022

@guy1699 I'm working on #1059 with the intent on capturing all valid properties on a URL, including for PostgreSQL. Vertx sql clients provide db-specific URL parsing ( see PgConnectionUriParser ).

By leveraging vertx (and tweaking their parsers for general properties) we'll should have what you need.

@guy1699
Copy link
Author

guy1699 commented Jan 4, 2022

Thanks, @blafond , when are you expecting to have it ready, is there a PR already?
Also, I will put my current solution below for reference, if it can help someone.
I appreciate your opinion on using
pgConnectOptions.setTrustAll(true); (io.vertx.pgclient.PgConnectOptions)

Is it means that only Postgres verify request from the pgClient but not vice versa, is this a security risk, how do you see it?

A solution for passing PgConnectOptions properties:

  1. Create new class
    MysqlClientPoolConfiguration extends org.hibernate.reactive.pool.impl.DefaultSqlClientPoolConfiguration
    
  2. add method:
    @Override
    public SqlConnectOptions connectOptions(URI uri) {
    
  3. Set your class in Hibernate properties:
    properties.put("hibernate.vertx.pool.configuration_class", AtriumSqlClientPoolConfiguration.class.getName());
    

@blafond
Copy link
Member

blafond commented Jan 4, 2022

Thx for the input. I think my fix will require a tweak to Vertx SQL client parsers. Created issue: #1115

@DavideD
Copy link
Member

DavideD commented Jan 7, 2022

Isn't this a Vert.x issue?

In Hibernate Reactive we allow passing a custom configuration using hibernate.vertx.pool.configuration_class (like @guy1699 already did) or, for other advance use cases, pass a custom pool using hibernate.vertx.pool.class (it's not described in the documentation yet though).

@blafond Is there anything we need to do on the Hibernate Reactive front? Or this is going to be solved by #1059?

@guy1699
Copy link
Author

guy1699 commented Jan 7, 2022

@DavideD, can you describe how to use a custom pool with hibernate.vertx.pool.class ?
Is this different than what I did?

@DavideD
Copy link
Member

DavideD commented Jan 7, 2022

It's different because you would have to implement ReactiveConnectionPool instead. What you have done seems fine.

@DavideD
Copy link
Member

DavideD commented Jan 7, 2022

But I'm not sure why we have the property considering that ReactiveConnectionPool is a service

@guy1699
Copy link
Author

guy1699 commented Jan 7, 2022

What property, how one implementation of ReactiveConnectionPool is chosen over another @DavideD ?

@DavideD
Copy link
Member

DavideD commented Jan 7, 2022

  1. hibernate.vertx.pool.configuration_class: It allows you to create a custom SqlClientPoolConfiguration
  2. hibernate.vertx.pool.class: It allows you to use a custom ReactiveConnectionPool

The only use case I can think of for 2 is Multitenancy (we have an example in our test cases: ReactiveMultitenantTest). Otherwise I wouldn't recommend to use option 2 if 1 is enough.

These properties are there so that users can have a workaround if Hibernate Reactive doesn't offer a solution for a particular use case. Using one or the other depends by the use case.

@blafond
Copy link
Member

blafond commented Jan 24, 2022

@guy1699 The PR for #1059 includes changes that utilize the vertx sql client and the SqlConnectOptions.fromUri() parsing. Currently the PostgreSQL sslmode parameter is supported and is accessible via PgConnectOptions.getSslMode(). I've tested it locally.

Unfortunately, the number of supported PG connection options is currently limited and an issue is logged: #1115

@DavideD
Copy link
Member

DavideD commented Mar 9, 2022

This will be fixed by #864

@DavideD DavideD closed this as completed Mar 9, 2022
@pendula95
Copy link

For better visibility adding solution for other to find

public class PgClientPoolConfiguration extends DefaultSqlClientPoolConfiguration {

    private static String HIBERNATE_VERTX_PGSQL_SSL = "hibernate.vertx.pgsql.ssl";
    private static String HIBERNATE_VERTX_PGSQL_SSL_MODE = "hibernate.vertx.pgsql.ssl.mode";

    private boolean ssl;
    private String sslMode;

    @Override
    public void configure(Map configuration) {
        super.configure(configuration);
        ssl = ConfigurationHelper.getBoolean(HIBERNATE_VERTX_PGSQL_SSL, configuration);
        sslMode = ConfigurationHelper.getString(HIBERNATE_VERTX_PGSQL_SSL_MODE, configuration);
    }

    @Override
    public SqlConnectOptions connectOptions(URI uri) {
        SqlConnectOptions sqlConnectOptions = super.connectOptions(uri);
        PgConnectOptions pgConnectOptions = new PgConnectOptions(sqlConnectOptions);

        if (ssl) {
            pgConnectOptions.setSsl(true);
            pgConnectOptions.setTrustAll(true);
            pgConnectOptions.setSslMode(SslMode.of(sslMode));
        }
        return pgConnectOptions;
    }
}

@DavideD
Copy link
Member

DavideD commented Feb 22, 2024

@pendula95 , I've create a separate issue to update the documentaion with this: #1866

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants