gravitational / teleport

The easiest, and most secure way to access and protect all of your infrastructure.
https://goteleport.com
GNU Affero General Public License v3.0
17.04k stars 1.71k forks source link

Update Teleport Connect MongoDB database connection function to use `mongosh` instead of deprecated `mongo` #28115

Closed dumez-k closed 1 year ago

dumez-k commented 1 year ago

What would you like Teleport to do? As of Mongodb 5.0 (released July 2021) the mongo shell command mongo has been deprecated and replaced with mongosh.

In Teleport connect we still use the deprecated mongo shell to connect to mongodb databases. We should update this to the new mongosh.

Screenshot 2023-06-21 at 4 33 08 PM

What problem does this solve?

Replaces deprecated CLI

If a workaround exists, please include it.

Copy and pasting the current command it suggests and changing mongo to mongosh manually once you paste in your terminal.

ravicious commented 1 year ago

I'm looking at the code and it looks like we fall back to mongo only if mongosh is not found in your PATH. Could you confirm that this is the case? Did you have mongosh installed at the time of taking the screenshot?

It might also be the case that you started Connect, then installed mongosh and then opened a db connection. In that case Connect might not see mongosh but I'm not totally sure.

In any case, if both mongosh and mongo are not found on the system, we should use mongosh instead.

dumez-k commented 1 year ago

Good call @ravicious, I started Connect, opened a database connection then installed mongosh.

Upon uninstall and reinstall of Connect mongo still persists instead of mongosh.

Agree with your last point as well that we should default to mongosh

ravicious commented 1 year ago

@gzdunek I think the problem here is that the tshd daemon (who is responsible for resolving paths to database CLI tools) inherits the env vars when spawned by the Electron app. So in the end, it's the same problem that we had with new terminal processes. When you open Teleport Connect from the desktop, PATH is set to something like /usr/bin:/bin:/usr/sbin:/sbin and tshd doesn't see tools that are available from your terminal.

I suppose we would need to resolve the shell env at some point during the initialization of the app. Perhaps put it in RuntimeSettings, but maybe there's a more appropriate place for that from the UX standpoint.

I think I'm going to create a separate issue for that, as it's not straightforward (but not super hard either), while the more immediate fix to the problem Kenny is to simply make dbcmd return mongosh if both mongo and mongosh are missing.

gzdunek commented 1 year ago

Yeah, this may be the reason. Putting resolved shell env in RuntimeSettings sounds good, although it will slow down the app startup a bit (on my machine resolveShellEnv takes 600-700 ms). But if we need these values to start tshd, probably there is no another way. For now let's create an issue, so we can discuss the solution there.

while the more immediate fix to the problem Kenny is to simply make dbcmd return mongosh if both mongo and mongosh are missing.

We could do this, but maybe it can wait for the appropriate fix? 🫢

ravicious commented 1 year ago

Putting resolved shell env in RuntimeSettings sounds good, although it will slow down the app startup a bit (on my machine resolveShellEnv takes 600-700 ms).

Yeah, I was thinking about how to make it not slow the startup of the app while still providing synchronous access to runtime settings. I don't think we want to block app startup on this because it can take a varying amount of time depending on your startup files.

I'll elaborate on that in the issue. I should have time to create it today.

We could do this, but maybe it can wait for the appropriate fix? 🫢

I wouldn't say the dbcmd fix is not appropriate. dbcmd itself should probably prefer mongosh if both binaries are missing.

gzdunek commented 1 year ago

I wouldn't say the dbcmd fix is not appropriate. dbcmd itself should probably prefer mongosh if both binaries are missing.

👍