Conversation
|
💬 Discussion in Slack: #pr-review-cli-328-feat-pam-conn-test-for-other-accs Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel. |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
|
| Filename | Overview |
|---|---|
| packages/gateway-v2/test_connection_handler.go | Adds the new protocol tests, shared TLS configuration, and timeout wrapper; MongoDB authentication mapping, Kubernetes status handling, and timeout cleanup need fixes. |
| go.mod | Adds the LDAP, MySQL, and SQL Server dependencies and updates related modules. |
| go.sum | Updates checksums for the added drivers and dependency versions. |
Reviews (1): Last reviewed commit: "feat(pam): conn test for other accs" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| testErr = doTCPReachabilityTest(ctx, target.host, target.port) | ||
| op = func() error { return doTCPReachabilityTest(ctx, target.host, target.port) } | ||
| default: | ||
| writeRPCError(w, http.StatusBadRequest, fmt.Sprintf("unsupported test-connection mode: %q", env.Mode)) |
There was a problem hiding this comment.
Wouldn't this brick all old gateways that still use postgres/mysql/etc? Since we're switching to just "sql"
There was a problem hiding this comment.
Yes, there's I think ~3 minor releases that expect postgres. Mysql and the others should not be a concern
| } | ||
|
|
||
| client := &http.Client{Transport: &http.Transport{TLSClientConfig: tlsConfig}} | ||
| url := fmt.Sprintf("https://%s/api", net.JoinHostPort(host, strconv.Itoa(port))) |
There was a problem hiding this comment.
what about http ones?
There was a problem hiding this comment.
I think we can just force https like this because 99% of cases http would be a misconfiguration. if a customer complains then we can later accommodate
3 participants
Description 📣
Type ✨
Tests 🛠️
# Here's some code block to paste some code snippets