Skip to content

feat(pam): conn test for other accs - #328

Open
x032205 wants to merge 4 commits into
mainfrom
PAM-362
Open

feat(pam): conn test for other accs#328
x032205 wants to merge 4 commits into
mainfrom
PAM-362

Conversation

@x032205

@x032205 x032205 commented Jul 22, 2026

Copy link
Copy Markdown
Member

Description 📣

Type ✨

  • Bug fix
  • New feature
  • Improvement
  • Breaking change
  • Documentation

Tests 🛠️

# Here's some code block to paste some code snippets

@linear

linear Bot commented Jul 22, 2026

Copy link
Copy Markdown

PAM-362

@infisical-review-police

Copy link
Copy Markdown

💬 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.

Comment thread packages/gateway-v2/test_connection_handler.go Outdated
@veria-ai

veria-ai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 1 · PR risk: 0/10

@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR expands PAM connection testing to additional account types. The main changes are:

  • Adds SQL tests for PostgreSQL, MySQL, and SQL Server.
  • Adds MongoDB, LDAP, and Kubernetes tests.
  • Centralizes TLS setup and timeout dispatch.
  • Adds the required database and LDAP drivers.

Confidence Score: 4/5

The MongoDB and Kubernetes connection-test result paths need fixes before merging.

  • Valid MongoDB deployments can fail when their authentication database differs from the application database.
  • Kubernetes redirects and several client errors are reported as successful tests.
  • Timed-out LDAP and SSH operations can continue after the handler responds.

packages/gateway-v2/test_connection_handler.go

Important Files Changed

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

Comment thread packages/gateway-v2/test_connection_handler.go
Comment thread packages/gateway-v2/test_connection_handler.go
Comment thread packages/gateway-v2/test_connection_handler.go
@x032205
x032205 requested a review from bernie-g July 22, 2026 21:54
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))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this brick all old gateways that still use postgres/mysql/etc? Since we're switching to just "sql"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there's I think ~3 minor releases that expect postgres. Mysql and the others should not be a concern

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

Successfully merging this pull request may close these issues.

2 participants