Fork me on GitHub
#sql
<
2022-01-16
>
lukasz17:01:51

Oh, sorry - I see what you mean

lukasz17:01:27

What would be the use case for this method actually?

kenny17:01:28

Yep 🙂 I have short lived creds (creds with ttl) to connect to a db (i.e., IAM creds to RDS) and want to make sure a connection is always initialized with creds not past their expiry.

kenny17:01:50

There's probably another way to do what I'm after. This method seemed like the most straightforward way. The fact that it's not supported gives me second thought.

lukasz17:01:40

Looks like HikariCP has facilities to do this - it's not obvious how it works, but could be a good jump off point: https://github.com/brettwooldridge/HikariCP/issues/1363

lukasz17:01:39

Since it's a pool - you can just get a connection for credentials, you have to restart the whole pool as other pooled conns have potentially stale creds. I guess that's why the method you've linked to is not implemented

kenny17:01:11

Erm, maybe, though I believe connections that are alive do not need to be recreated.

kenny17:01:31

Thanks for the link btw. Looks relevant here.

kenny17:01:24

On the former, I got my info from the AWS docs on https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/UsingWithRDS.IAMDBAuth.html. > The token is only used for authentication and doesn't affect the session after it is established.

kenny17:01:11

Assuming my interpretation is correct (bad assumption? haha), I only need to set the password for newly created connections.

lukasz17:01:53

Right, that would be my assumption too - once the connection is live, expiring creds shouldn't matter - it would be quite bad for performance to reconnect on every rotation, so it only should matter when the pool acquires new connections as the demand grows. Perhaps you can set min and max conns to the same number to force auth on pool start? :thinking_face:

lukasz18:01:22

Gonna bookmark that as IAM auth for RDS is on my TODO :-)

kenny18:01:38

Ah, nice find! Reading...

kenny18:01:48

Simple enough. If the getConnection method overload was supported, a workaround like that wouldn't be necessary.

lukasz18:01:21

Yep. Also that final note about max-lifetime option seems VERY important

kenny18:01:52

Not sure I agree. The javadoc on that property is: > This property controls the maximum lifetime of a connection in the pool. When a connection reaches this timeout, even if recently used, it will be retired from the pool. An in-use connection will never be retired, only when it is idle will it be removed. I don't think the connection lifetime has anything to do with password expiry. Do you agree?

lukasz18:01:46

I'd have to actually try this - it kinda makes sense, but without hands on testing it's hard to say what happens. Since I'm on vacation I'm not going to touch this for at least a month or two :-)

✔️ 1
kenny18:01:28

A bit annoying to test. There doesn't seem to be a way to configure the rds token ttl, so you'd have to wait the 15m.

kenny18:01:41

The workaround might work, but has some nuance. I would have expected the below to connect successfully.

(def ds
    (let [hconfig (doto (HikariConfig.)
                    (.setJdbcUrl db-url)
                    (.setDataSourceProperties
                      (doto (Properties.)
                        (.putAll {"user" "computesoftware"}))))
          data-source (proxy [HikariDataSource] [hconfig]
                        (getPassword []
                          "..."))]
      data-source))
2022-01-16 10:31:19 nREPL-session-00710376-ce43-4463-8bd2-8039c8fcd26d [com.zaxxer.hikari.HikariDataSource] INFO - HikariPool-3 - Starting...
2022-01-16 10:31:20 nREPL-session-00710376-ce43-4463-8bd2-8039c8fcd26d [com.zaxxer.hikari.pool.HikariPool] ERROR - HikariPool-3 - Exception during pool initialization.
Execution error (PSQLException) at org.postgresql.core.v3.ConnectionFactoryImpl/doAuthentication (ConnectionFactoryImpl.java:643).
The server requested password-based authentication, but no password was provided.

kenny18:01:07

My guess is that, for some reason, the connection created in checkFailFast (called in HikariPool ctor) does not use the getPassword method.

kenny18:01:53

WAIT but config might be this. Ugh java... 🤯

seancorfield18:01:18

I will mention my pet peeve here: it's :user for JDBC and for c3p0 but it's :username for HikariCP and that catches a lot of people out and causes no a lot of support Qs against next.jdbc (it's specifically called out in the docs).

✔️ 1
seancorfield18:01:49

Another thing to be aware of (also called out in the docs): neither HikariCP nor c3p0 validates the database configuration until you actually get your first connection from the pool -- but people expect such validation to occur at get-datasource, not have it deferred to get-connection.

kenny18:01:16

The name change is very confusing. I've literally been tracing the java calls to make sure I'm not crazy.

kenny18:01:33

Ok I'm pretty sure this is failing because we have overridden the getPassword method and that method is called in the constructor. As per https://stackoverflow.com/questions/3404301/whats-wrong-with-overridable-method-calls-in-constructors, this will not do what you think.

kenny18:01:38

So getPassword yields nil in the ctor because the our proxied class has not finished initializing. So Tom's fix has a caveat that's not mentioned.

seancorfield18:01:51

Heh, yeah, any time I need to go debug a HikariCP issue, my head starts to hurt pretty quickly. c3p0 is worse tho'...

😩 1
lukasz19:01:15

@U083D6HK9 so you'll need a proper subclass, right?

kenny19:01:23

I wonder why the code is so awful. I mean it is java, but this java seems particularly bad. Perhaps a long history?

lukasz19:01:50

Or performance - Hikari has some interesting claims in terms of how it implements object and connection pooling

kenny19:01:58

@U0JEFEZH6 I just found the right combo. It's so easy to shoot yourself in the foot with Hikari haha.

kenny19:01:20

(doto (proxy [HikariDataSource] []
        (getPassword []
          "...;"))
  (.setJdbcUrl db-url)
  (.setDataSourceProperties
    (doto (Properties.)
      (.putAll {"user" "..."}))))

seancorfield19:01:50

@U083D6HK9 Is it really "user" there or "username"?

lukasz19:01:08

☝️ it should be username, everything else uses 'user' I think

kenny19:01:09

This will initialize the data source without going through all the wacky state copying in ctor stuff. It's really just a container for info at this point. To actually start it, you need to call getConnection

kenny19:01:39

No, it's user here. I'm passing data source properties directly since it removes the weird Hikari layer and I can directly reference my db docs on what to pass.

seancorfield19:01:20

Yup, that's not confusing at all 🙂

😆 1
kenny19:01:20

This has got to be one of the most confusing configurations I've ever seen.

kenny19:01:47

I think they really messed up the design here. Multiple named things all doing the exact same thing. I can only imagine how many dev hours have been lost both on the user end and implementor end working on issues in that realm.

kenny19:01:45

Alright, so my current recommendation @U0JEFEZH6 is this:

(def ds (doto (proxy [HikariDataSource] []
                (getPassword []
                  "..."))
          (.setJdbcUrl db-url)
          (.setDataSourceProperties
            (doto (Properties.)
              ;; Pass native connection params here
              (.putAll {"user" "..."})))))

;; Start the pool
(.getConnection ds)
TBD if there's some weird caching layer on getPassword causing problems after token ttl. Will test that next. 🙏:skin-tone-2:

lukasz19:01:38

This warrants a blog post or a GH repo, wink wink, nudge nudge 😉

🙂 1
kenny19:01:47

I also recommend ignoring all the wacky pojo stuff and directly pass Properties since everything bottoms out at properties in the end.

lukasz19:01:53

Oh, good to know - we're using a Clojure wrapper to instantiate the pool but I can see how that will make it redundant

kenny19:01:26

Caveat: I have not used Hikari in prod so I might not know what I'm talking about 😅

kenny19:01:28

I can back up my claims with source code though. In Postgres's case, we know we bottom out at properties by checking the Driver code. https://github.com/pgjdbc/pgjdbc/blob/c12a76f6c42ba3c41d6f4472f3a47d645e9dba33/pgjdbc/src/main/java/org/postgresql/Driver.java#L467

kenny19:01:58

I do believe Hikari introduces some new props, however. Unclear which are Hikari specific and if the Properties will configure them. More digging 🙂

kenny19:01:53

Yep, no idea how to differentiate Hikari knobs from native connection knobs. They seem completely intertwined.

kenny20:01:34

Seems best to cross reference https://jdbc.postgresql.org/documentation/head/connect.html & manually create a spec for Hikari config vs native.

kenny20:01:57

Hmm yeah. Thank you.

kenny14:03:46

As per your suggestion @U0JEFEZH6, I finally got around to writing a blog post about this journey, and the final working solution. Perhaps you'll find it useful 🙂 https://kwill.dev/posts/hikari-rds-iam-auth/

lukasz14:03:23

amazing 🙏 security/compliance team will be very happy soon

😎 1
kenny18:01:57

What's the deal with Hikari having, what appears to be, two ways to configure a data source: 1) via pojo getters and setters on HikariConfig and 2) addDataSourceProperty just putting a kv onto a Properties?

kenny19:01:02

What are folks preferences between 1️⃣ c3p0 2️⃣ DBCP 3️⃣ Hikari ?

3️⃣ 3
lukasz19:01:45

I've used Hikari for over 5 years with RDS and CloudSQL, it's fine - never had issues with it

seancorfield19:01:58

My answer is complicated: I prefer HikariCP despite its quirks but at work we've used c3p0 for maybe a decade at this point. We tried to switch to HikariCP at one point and ran into some performance issues and, rather than spend time figuring out tuning, we just reverted back to c3p0. And haven't tried to change things since then.

kenny19:01:45

Interesting. Yet you still prefer Hikari?

seancorfield20:01:51

It's better maintained and when properly setup it is said to be better performing. If we started over, we'd start with HikariCP and spend the time to tune it properly. There's just not much incentive to switch right now.

seancorfield21:01:52

Yup. I can confirm that with c3p0. If we lose network connectivity to the DB and we're in a high-traffic period, we see threads ramp up in the application which leads to poor throughput(!) and if the DB access isn't restored in a reasonably timely manner, the application can start to "crash" with OoM errors due to running out of threads (unable to create native thread). However, if the connectivity glitch is brief -- which is normally the case -- c3p0 recovers well and application performance returns to normal with no ill effects and, often, almost no dropped requests and only a handful of dropped queries. So, in the "real world", c3p0's behavior is acceptable.

seancorfield21:01:58

And, to be honest, with most applications, if your DB goes away, you can't do much of anything anyway so the difference in behavior is somewhat moot...