Skip to content
This repository has been archived by the owner on Feb 8, 2024. It is now read-only.

Simplify the db connection tab #720

Merged
merged 2 commits into from
Apr 6, 2022
Merged

Conversation

ravicious
Copy link
Member

As described in gravitational/webapps.e#177, we want to replace the db
tab with just two sections:

  • "Connect with CLI" which will show the command to use in terminal
  • "Connect with GUI" which links to our documentation

After gravitational/teleport#11720 gets merged, the "Connect with CLI"
section will be massively simplified: it'll be basically just something like:

psql postgres://localhost:12345

Moreover, @smallinsky suggested that tsh should be responsible for creating
those CLI connection commands. We should also do this in the future as
it'll let us support new protocols as soon as they land in tsh, without
us having to touch Teleterm codebase, for the most part.

As described in gravitational/webapps.e#177, we want to replace the db
tab with just two sections:

* "Connect with CLI" which will show the command to use in terminal
* "Connect with GUI" which links to our documentation

After gravitational/teleport#11720 gets merged, the "Connect with CLI"
section will be massively simplified: it'll be basically just something like:

    psql postgres://localhost:12345

Moreover, @smallinsky suggested that tsh should be responsible for creating
those CLI connection commands. We should also do this in the future as
it'll let us support new protocols as soon as they land in tsh, without
us having to touch Teleterm codebase, for the most part.
Comment on lines +74 to 86
let cliConnectionString: string;

switch (gateway.protocol as GatewayProtocol) {
case 'mongodb':
return (
<Mongo gateway={gateway} title={doc.title} disconnect={disconnect} />
);
cliConnectionString = useMongo(gateway);
break;
case 'postgres':
return (
<Postgres gateway={gateway} title={doc.title} disconnect={disconnect} />
);
cliConnectionString = usePostgres(gateway);
break;
case 'mysql':
return (
<MySql gateway={gateway} title={doc.title} disconnect={disconnect} />
);
default:
return <Box> {`Unknown protocol type ${gateway.protocol}`}</Box>;
cliConnectionString = useMySql(gateway);
break;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

As mentioned in the PR description, this will get removed at some point before the preview release in favor of tsh doing this for Teleterm.

export function useMongo(props: MongoProps) {
const { gateway, disconnect, title } = props;

export function useMongo(gateway: Gateway) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Technically these useMongo type functions no longer need to live in protocol-specific folder, but we're going to remove them anyway later on. So I just kept them where they are.

@ravicious ravicious requested a review from gzdunek April 5, 2022 10:31
@@ -71,20 +71,55 @@ export function DocumentGateway(props: State) {
);
}

let cliConnectionString: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

In such cases I like to create a new function, so I can avoid mutations:

function getCliConnectionString(gateway: Gateway) {
  switch (gateway.protocol as GatewayProtocol) {
    case 'mongodb':
      return useMongo(gateway);
    case 'postgres':
      return usePostgres(gateway);
    case 'mysql':
      return useMySql(gateway);
  }
}

And theoretically, we shouldn't use use... names as they're reserved for hooks and we can't use hooks conditionally (like in switch-case statements), but I'm fine with leaving it if we are going to remove them anyway.

@ravicious
Copy link
Member Author

Request from Roman:

for now, can we just add Redis and SQL Server commands to the frontend? it’s just a couple if simple switch cases
for Redis the command would be redis-cli -h localhost -p <port>
for MSSQL: mssql-cli -S localhost,<port> -U <user> -P <random string> (password is required by MSSQL client IIRC but doesn’t matter since we’re connecting to local proxy)

@ravicious
Copy link
Member Author

Actually, I'm going to make a separate PR with that Redis and MSSQL support. It doesn't make sense to cram in more stuff here.

@ravicious ravicious enabled auto-merge (squash) April 6, 2022 09:59
@ravicious ravicious merged commit f3a6165 into master Apr 6, 2022
@ravicious ravicious deleted the ravicious/simplify-db-conn-tab branch April 6, 2022 10:11
ravicious added a commit that referenced this pull request Apr 26, 2022
As described in gravitational/webapps.e#177, we want to replace the db
tab with just two sections:

* "Connect with CLI" which will show the command to use in terminal
* "Connect with GUI" which links to our documentation

After gravitational/teleport#11720 gets merged, the "Connect with CLI"
section will be massively simplified: it'll be basically just something like:

    psql postgres://localhost:12345

Moreover, @smallinsky suggested that tsh should be responsible for creating
those CLI connection commands. We should also do this in the future as
it'll let us support new protocols as soon as they land in tsh, without
us having to touch Teleterm codebase, for the most part.
ravicious added a commit that referenced this pull request Apr 27, 2022
As described in gravitational/webapps.e#177, we want to replace the db
tab with just two sections:

* "Connect with CLI" which will show the command to use in terminal
* "Connect with GUI" which links to our documentation

After gravitational/teleport#11720 gets merged, the "Connect with CLI"
section will be massively simplified: it'll be basically just something like:

    psql postgres://localhost:12345

Moreover, @smallinsky suggested that tsh should be responsible for creating
those CLI connection commands. We should also do this in the future as
it'll let us support new protocols as soon as they land in tsh, without
us having to touch Teleterm codebase, for the most part.
ravicious added a commit that referenced this pull request Apr 27, 2022
* Limit navigation capabilities to reduce attack surface

At the moment we don't create new windows nor navigate away from the rendered
app, so we can just block everything.

* Update to electron@13.6.9 (#703)

* Use x64 arch when building & packaging Teleterm

Our build system doesn't support arm64 for Mac releases yet (see issue
gravitational/teleport#4226 for more information). Because of that, for
the preview release we're likely going to have only the x64 version of
Teleterm.

This means that the shipped version of tsh should also be the x64 version.

I tried to change electron-builder's config to use x64 for macOS, but the
config options don't seem to work. I tried `mac.defaultArch` as well as
changing `mac.target` in various ways but `electron-builder install-app-deps`
just doesn't pick up those options. Both were set through
`packages/teleterm/package.json`

* Add `Notifications` component and service

* Show errors in `ClusterResources`' tables using standard `Danger` labels

* Use `Notifications` error in `syncRootCluster()` and `removeGateway()`

* Do not block app rendering when initializing function fails

* Fix accessing `serversSyncStatus` Map in `clustersService`

* Revert "Use x64 arch when building & packaging Teleterm"

This reverts commit 276e9a9.

Turns out that for development, we need to use arm64 version of native
deps. The build server is going to use x64 anyway, as per the reverted
commit, but when making manual demo builds, we'll have to remember to
use x64 for Teleterm and tsh.

* Submit modals' forms on `Enter` press

* Remove global `keyDown` handler from `KeyboardArrowsNavigation` as it blocked submitting forms

* Use teleterm/logger in runtimeSettings (#716)

The one from shared/libs/logger calls `window`, which doesn't exist in
the context of Electron main process.

* Improve Teleterm README (#719)

* Mention that `yarn build-term` needs to be run first before attempting
  to run the app in dev mode (already had a couple of people who had
  problems with setting up the app because they didn't run this first).
* Mention the assumption about both repos living in the same folder.
* Move the architecture diagram to the end of the file. Most people
  reading the README are not doing it for the diagram, but build
  instructions.
* Explain when gRPC files need to be recompiled.

* Prevent crash when network or cluster is offline (#712)

* Simplify the db connection tab (#720)

As described in gravitational/webapps.e#177, we want to replace the db
tab with just two sections:

* "Connect with CLI" which will show the command to use in terminal
* "Connect with GUI" which links to our documentation

After gravitational/teleport#11720 gets merged, the "Connect with CLI"
section will be massively simplified: it'll be basically just something like:

    psql postgres://localhost:12345

Moreover, @smallinsky suggested that tsh should be responsible for creating
those CLI connection commands. We should also do this in the future as
it'll let us support new protocols as soon as they land in tsh, without
us having to touch Teleterm codebase, for the most part.

* Resolve shell env (#718)

Co-authored-by: Grzegorz Zdunek <gzdunek@users.noreply.github.com>
Co-authored-by: Grzegorz Zdunek <grzegorz.zdunek@goteleport.com>
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants