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

Add PostgreSQL REPL implementation #49598

Merged
merged 12 commits into from
Dec 13, 2024
Merged

Add PostgreSQL REPL implementation #49598

merged 12 commits into from
Dec 13, 2024

Conversation

gabrielcorado
Copy link
Contributor

Part of #44956 (RFD 0181)

This PR introduces the PostgreSQL REPL implementation following the RFD definition, which will be used as a client to access PostgreSQL instances through WebUI. It works like a terminal emulator (implemented by golang.org/x/term library) and will be used with the WebUI terminal (like WebUI SSH sessions).

Note

This PR only includes the REPL implementation. The web handlers and WebUI changes will be done in different PRs.

For this first release of the REPL, we're not including informational commands (such as \d or \dt). These commands will require better testing across different PostgreSQL versions and will be introduced in later versions.

@gabrielcorado gabrielcorado added the no-changelog Indicates that a PR does not require a changelog entry label Nov 30, 2024
@flyinghermit flyinghermit removed their request for review December 2, 2024 14:37
Copy link
Contributor

@greedy52 greedy52 left a comment

Choose a reason for hiding this comment

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

First pass. Overall logic looks good to me.

A few things i wonder if we could do (not immediately but maybe after MVP):

  1. common REPL so it can be used for other protocols like MySQL
  2. test postgres repl (or even do this through the webapi websocket) in E2E test (or testcontainers) against real RDS/Postgres

switch {
case strings.HasPrefix(line, commandPrefix) && !readingMultiline:
var exit bool
reply, exit = r.processCommand(line)
Copy link
Contributor

Choose a reason for hiding this comment

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

write non-empty reply before exit?

@gabrielcorado
Copy link
Contributor Author

@greedy52

common REPL so it can be used for other protocols like MySQL

We could do this in the future (when we introduce another database). However, it is a bit difficult to identify which part we can reuse right now.

test postgres repl (or even do this through the webapi websocket) in E2E test (or testcontainers) against real RDS/Postgres

I'll add the integration and E2E tests after we have both parts (web handler and REPL) ready/merged. So we can thoroughly test their usage against the databases.

@gabrielcorado gabrielcorado requested a review from zmb3 December 10, 2024 15:06
@gabrielcorado gabrielcorado added this pull request to the merge queue Dec 13, 2024
Merged via the queue into master with commit 89ea69c Dec 13, 2024
42 checks passed
@gabrielcorado gabrielcorado deleted the gabrielcorado/pg-repl branch December 13, 2024 20:03
gabrielcorado added a commit that referenced this pull request Dec 16, 2024
* feat(repl): add postgres

* refactor(repl): change repl to use a single Run function

* test(repl): reduce usage of require.Eventually blocks

* refactor(repl): code review suggestions

* refactor(repl): code review suggestions

* test(repl): increase timeout values

* fix(repl): commands formatting

* refactor(repl): send close pgconn using a different context

* fix(repl): add proper spacing between multi queries

* test(repl): add fuzz test for processing commands
github-merge-queue bot pushed a commit that referenced this pull request Dec 16, 2024
* Add PostgreSQL REPL implementation (#49598)

* feat(repl): add postgres

* refactor(repl): change repl to use a single Run function

* test(repl): reduce usage of require.Eventually blocks

* refactor(repl): code review suggestions

* refactor(repl): code review suggestions

* test(repl): increase timeout values

* fix(repl): commands formatting

* refactor(repl): send close pgconn using a different context

* fix(repl): add proper spacing between multi queries

* test(repl): add fuzz test for processing commands

* Add WebSocket handler for WebUI database sessions (#49749)

* feat(web): add websocket handler for database webui sessions

* refactor: move common structs into a separate package

* refactor(web): use ALPN local proxy to dial databases

* feat(repl): add default registry

* refactor(web): code review suggestions

* refactor: update repl config parameters

* refactor: move default getter implementation

* feat(web): add supports_interactive field on dbs

* refactor: code review suggestions

* refactor: update database REPL interfaces

* chore(web): remove debug print

* feat: register postgres repl

* refactor(web): update MakeDatabase to receive access checker and interactive

* chore(web): remove unused function

* Database access through WebUI (#49979)

* feat(web): add database terminal access

* chore(web): make explict type cast

* refactor(web): code review suggestions

* chore(web): fix lint errors

* refactor(web): lint errors

* refactor: code review suggestions

* refactor(web): filter wildcard options from connect dialog

* chore(web): lint

* refactor(web): code review suggestions
carloscastrojumo pushed a commit to carloscastrojumo/teleport that referenced this pull request Feb 19, 2025
* feat(repl): add postgres

* refactor(repl): change repl to use a single Run function

* test(repl): reduce usage of require.Eventually blocks

* refactor(repl): code review suggestions

* refactor(repl): code review suggestions

* test(repl): increase timeout values

* fix(repl): commands formatting

* refactor(repl): send close pgconn using a different context

* fix(repl): add proper spacing between multi queries

* test(repl): add fuzz test for processing commands
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Indicates that a PR does not require a changelog entry size/lg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants