Add `db open` command #72

tangrufus · 2019-09-25T20:40:11Z

Open remote databases via SSH tunnels in your favourite database client.

Requirement

An database client on your local machine registered as mysql+ssh:// handler, e.g: TablePlus

How does it work?

In a nutshell, it invokes this command on your local machine:

$ open 'mysql+ssh://[email protected]:22/my_db_user:my_db_password@localhost/my_db_name?enviroment=production&usePrivateKey=true'

Which database clients are supported?

I've only tested TablePlus.

Sqeual Pro (nightly) supports mysql:// but seems not support SSH tunnels yet

Does it support vagrant / local databases?

No.

See: #7

tangrufus · 2019-09-25T20:46:31Z

Discussion:

How should we handle app-specific paramters (e.g: ?enviroment=production&usePrivateKey=true)?

ping @swalkinshaw

templates/db_open_yml.go

swalkinshaw · 2019-09-26T04:28:50Z

How should we handle app-specific paramters

By app I'm guessing you mean TablePlus and Sequel Pro for example? I guess we'd have to have some we officially support and hardcode their necessary options? Then we could also let people pass-through whatever extra params they need.

tangrufus · 2019-09-26T09:28:27Z

By app I'm guessing you mean TablePlus and Sequel Pro for example?

Yes.

What do you think about this style?

trellis db open production example.com tableplus
trellis db open production example.com sequel-pro

Putting the app name last because we can make them optional when #70 is done.

The reason is that they can be open in very different ways:

TablePlus: $ open 'mysql+ssh://[email protected]:22/my_db_user:my_db_password@localhost/my_db_name?enviroment=production&usePrivateKey=true'

Sequal Pro: $ open xxx.spf
(see: https://github.com/TypistTech/vagrant-trellis-sequel/blob/cc93696de180ba8e72e80039c9837f0de26bd77c/lib/vagrant_plugins/trellis_sequel/template.spf.erb)

swalkinshaw · 2019-09-26T22:47:49Z

We don't really have other options than your suggestion but I think they should be option flags instead?

tangrufus · 2019-11-03T02:04:19Z

Help wanted

10ad118 introduces $ trellis db open --app=tableplus production mysite. However, the implementation is so bad that writting tests is painful.

Whoever can't stand with my code smell, free feel to make pull requests to my branch.

assuming we'd get the output from STDOUT and parse it so there wouldn't be a need for any temp files.
#72 (comment)

I have trouble getting ansible outputs from STDOUT.

There are lots of boilerplate code . Example:

However, refactoring them affects almost every existing commands.
Perphas leave it to another pull request.

Bash autocompletion doesn't work on my machine. Thus, omitted.

Tried to put cmd/tableplus.go into subdirectory - cmd/db/openers/tableplus.go.
However, execCommand & logCmd become unavaible.

Same goes to cmd/db/openers/factory.go (https://github.com/roots/trellis-cli/pull/72/files#diff-a08b51a976e893b57653c908b219ffbcR44-R53)

swalkinshaw · 2019-11-04T20:51:30Z

🤷‍♂ I think it looks mostly good overall. Not sure we can really avoid a lot of boilerplate here (and it's Go after all 😛 ).

re: subdirectory, Go is weird about this and I don't know the best solution. But for now I'd just name the files cmd/db_opener_tableplus.go etc.

tangrufus · 2019-11-09T17:54:07Z

Ignore this PR until #88 is merged

tangrufus · 2019-11-16T17:36:38Z

Added more tests.

Note:

DBOpenCommand is untested (Help wanted)
Not sure how to test DBOpenerFactory returns correct struct (Any golang equivalent of instanceof?)
DBOpenerSequelPro is not removing its spf file in temporary directory because the spf file might be removed before sequel pro reads db credentials from it. (Warning: The SPF contains credentials in plain text)
Intentionally omitting logCmd from DBOpenerTableplus to prevent printing db credentials.

Usage:

$ trells db open --app=tableplus production mysite
$ trells db open --app=sequel-pro production mysite

If you just installed tableplus or sequel-pro, open them normally at least once before running trells db open.

swalkinshaw · 2019-11-17T23:58:44Z

For testing the struct type, I think you want https://golang.org/pkg/reflect/#TypeOf

swalkinshaw

There's already some tests for DBOpenerSequelPro; wouldn't those work the same way for DBOpenCommand as well? Just assert on the mock exec command?

cmd/db_open.go

swalkinshaw · 2019-11-18T00:02:58Z

cmd/db_open.go

+
+	// Template playbook files from package to Trellis
+	playbookPath := "dump_db_credentials.yml"
+	writeFile(playbookPath, templates.TrimSpace(dumpDbCredentialsYmlTemplate))


Every usage of writeFile deletes the file after. Maybe we should just make this a more specific writeTempFile function which also handles the defer delete?

How do we defer the defer?

If we put defer inside writeTempFile, it deletes the files at the end of writeTempFile instead of the end of func (c *DBOpenCommand) Run(args []string) int

Oh yes good call. We do this:

trellis-cli/trellis/testing.go

Lines 32 to 38 in 54aeb5d

return func() {

if err := os.Chdir(old); err != nil {

t.Fatalf("err: %s", err)

}

os.RemoveAll(tempDir)

}

Then it would be called as:

defer writeFile(path, foo)()

cmd/db_open.go

swalkinshaw · 2019-11-18T00:06:55Z

cmd/db_opener_sequel_pro.go

+	if sequelProSpfErr != nil {
+		return fmt.Errorf("Error creating temporary SequelPro SPF file: %s", sequelProSpfErr)
+	}
+	// TODO: [Help Wanted] There is a chance that the SPF file got deleted before SequelPro establish db connection.


Can you explain this more? Was it getting deleted by the defer first?

Was it getting deleted by the defer first?

That is my theory.

If we uncomment the defer line, sequel pro will open but doesn't pre-fill any db info.
If we comment the defer line, sequel pro will open with db info pre-filled and try to connect to the db.

I suspect sequel pro needs to read that spf file after launch, but golang deleted the spf too soon.

Makes some sense. You could verify by logging like this:

func (o *DBOpenerSequelPro) open(c DBCredentials) (err error) { # etc open := execCommand("open", sequelProSpf.Name()) err := cmd.Start() if err != nil { log.Fatal(err) } log.Printf("running sequel pro...") err = cmd.Wait() log.Printf("Command finished with error: %v", err) }

And then add a log printf to deleteFile to see when it's actually run compared to the other log lines

Confirmed my theroy - the SPF file is read twice:

once by OS to open sequel pro

once by sequel pro to get db credentials

Newer version of sequel pro shows this error message if SPF is deleted too early:

Error while reading connection data file Connection data file couldn't be read. (The file “804911772.spf” couldn’t be opened because there is no such file.)

swalkinshaw · 2019-11-18T00:11:20Z

I can work on some tests as well if you want

roots#72 (comment) roots#72 (comment)

roots#72 (comment)

tangrufus · 2019-11-18T01:15:58Z

For testing the struct type, I think you want golang.org/pkg/reflect/#TypeOf

It works!

There's already some tests for DBOpenerSequelPro; wouldn't those work the same way for DBOpenCommand as well? Just assert on the mock exec command?

Not sure how to mock the json file - https://github.com/roots/trellis-cli/pull/72/files#diff-a08b51a976e893b57653c908b219ffbcR135

I can work on some tests as well if you want

Please do! Thanks!

swalkinshaw · 2019-11-19T02:29:40Z

I looked into testing and it's going to be very hard... it is possible to have more control over the mock exec command but it's not nice. We'd have to hardcode a conditional in TestHelperProcess to do what we want for the first ansible command. But that doesn't even solve the issue with writing to a tempfile that we wouldn't know anything about.

I think there's two solutions:

write an interface which models writing the temp file, running ansible cmd and reading the temp file then we could make a test implementation of the struct
make this an integration test that runs on Docker

tangrufus · 2019-11-26T13:33:16Z

write an interface which models writing the temp file, running ansible cmd and reading the temp file then we could make a test implementation of the struct

Something like tangrufus/trellis-cli@b83f85e...TangRufus:playbook-struct ?

swalkinshaw · 2019-11-26T15:33:04Z

Yep that looks like it should do it 👍 . The test struct would mostly be concerned with implementing Run and would just dump some fixture JSON to the tempfile.

tangrufus · 2019-11-26T15:46:19Z

I presume that we want to inject Playbook in main.go.

 	trellis := trellis.NewTrellis(project)
+   playbook := &Playbook{}
	 c.Commands = map[string]cli.CommandFactory{
		 "alias": func() (cli.Command, error) {
-			 return cmd.NewAliasCommand(ui, trellis), nil
+			 return cmd.NewAliasCommand(ui, trellis, playbook), nil
		 },

Am I correct?

swalkinshaw · 2019-11-26T15:47:34Z

Correct yeah. That's the only place available and you can see examples in ssh and vault edit for the sys command executor.

tangrufus · 2019-11-26T16:05:09Z

Question: What are the differences between exec.Command and unix.Exec?

swalkinshaw · 2019-11-26T16:10:11Z

The way I understand it:

os/exec: the go process is still "managing" the new command/process
syscall (or unix.Exec): runs a new process which completely takes over from the current one

I found it was needed for ssh for that process to take over and with vault edit it launches an editor. Maybe there is a way to do that with os/exec but I couldn't get it working.

tangrufus · 2019-11-26T16:26:18Z

The merge was a mistake. Sorry everyone. Reverted in #90

swalkinshaw reviewed Sep 26, 2019

View reviewed changes

templates/db_open_yml.go Outdated Show resolved Hide resolved

tangrufus force-pushed the db-open branch from 308adf0 to 2f6186f Compare October 17, 2019 17:29

tangrufus force-pushed the db-open branch 2 times, most recently from eef5321 to 10ad118 Compare November 3, 2019 01:46

tangrufus force-pushed the db-open branch 3 times, most recently from 3511fcd to 4f445ad Compare November 3, 2019 03:18

tangrufus force-pushed the db-open branch from 63ec4cc to 5c34e1a Compare November 10, 2019 00:20

Add db open command

ba46cc2

tangrufus force-pushed the db-open branch from d6adbc0 to ba46cc2 Compare November 16, 2019 17:30

tangrufus changed the title ~~[WIP] Add db open command~~ Add db open command Nov 16, 2019

tangrufus requested a review from swalkinshaw November 16, 2019 17:36

swalkinshaw reviewed Nov 18, 2019

View reviewed changes

tangrufus added 3 commits November 18, 2019 01:06

Test return type

3de770b

Move constants to the top

75fcab4

roots#72 (comment) roots#72 (comment)

Fix typo

94cf8f4

roots#72 (comment)

Use const instead of var

99eb736

tangrufus merged commit b1afd5e into roots:master Nov 26, 2019

tangrufus deleted the db-open branch November 26, 2019 16:24

tangrufus mentioned this pull request Nov 26, 2019

Revert "Add db open command" #90

Merged

tangrufus restored the db-open branch November 26, 2019 16:29

tangrufus mentioned this pull request Jan 4, 2020

Extract Playbook struct #91

Merged

tangrufus mentioned this pull request Feb 1, 2020

Add db open command with Playbook struct #100

Merged

swalkinshaw mentioned this pull request Dec 15, 2021

Database access command #7

Closed

tangrufus deleted the db-open branch September 28, 2024 00:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `db open` command #72

Add `db open` command #72

tangrufus commented Sep 25, 2019 •

edited

Loading

tangrufus commented Sep 25, 2019 •

edited

Loading

swalkinshaw commented Sep 26, 2019

tangrufus commented Sep 26, 2019 •

edited

Loading

swalkinshaw commented Sep 26, 2019

tangrufus commented Nov 3, 2019

swalkinshaw commented Nov 4, 2019

tangrufus commented Nov 9, 2019

tangrufus commented Nov 16, 2019 •

edited

Loading

swalkinshaw commented Nov 17, 2019

swalkinshaw left a comment

swalkinshaw Nov 18, 2019

tangrufus Nov 18, 2019 •

edited

Loading

swalkinshaw Nov 18, 2019

swalkinshaw Nov 18, 2019

tangrufus Nov 18, 2019

swalkinshaw Nov 18, 2019

tangrufus Feb 1, 2020

swalkinshaw commented Nov 18, 2019

tangrufus commented Nov 18, 2019

swalkinshaw commented Nov 19, 2019

tangrufus commented Nov 26, 2019

swalkinshaw commented Nov 26, 2019

tangrufus commented Nov 26, 2019 •

edited

Loading

swalkinshaw commented Nov 26, 2019

tangrufus commented Nov 26, 2019

swalkinshaw commented Nov 26, 2019

tangrufus commented Nov 26, 2019

	return func() {
	if err := os.Chdir(old); err != nil {
	t.Fatalf("err: %s", err)
	}

	os.RemoveAll(tempDir)
	}

Add db open command #72

Add db open command #72

Conversation

tangrufus commented Sep 25, 2019 • edited Loading

Requirement

How does it work?

Which database clients are supported?

Does it support vagrant / local databases?

tangrufus commented Sep 25, 2019 • edited Loading

swalkinshaw commented Sep 26, 2019

tangrufus commented Sep 26, 2019 • edited Loading

swalkinshaw commented Sep 26, 2019

tangrufus commented Nov 3, 2019

Help wanted

swalkinshaw commented Nov 4, 2019

tangrufus commented Nov 9, 2019

tangrufus commented Nov 16, 2019 • edited Loading

swalkinshaw commented Nov 17, 2019

swalkinshaw left a comment

Choose a reason for hiding this comment

swalkinshaw Nov 18, 2019

Choose a reason for hiding this comment

tangrufus Nov 18, 2019 • edited Loading

Choose a reason for hiding this comment

swalkinshaw Nov 18, 2019

Choose a reason for hiding this comment

swalkinshaw Nov 18, 2019

Choose a reason for hiding this comment

tangrufus Nov 18, 2019

Choose a reason for hiding this comment

swalkinshaw Nov 18, 2019

Choose a reason for hiding this comment

tangrufus Feb 1, 2020

Choose a reason for hiding this comment

swalkinshaw commented Nov 18, 2019

tangrufus commented Nov 18, 2019

swalkinshaw commented Nov 19, 2019

tangrufus commented Nov 26, 2019

swalkinshaw commented Nov 26, 2019

tangrufus commented Nov 26, 2019 • edited Loading

swalkinshaw commented Nov 26, 2019

tangrufus commented Nov 26, 2019

swalkinshaw commented Nov 26, 2019

tangrufus commented Nov 26, 2019

Add `db open` command #72

Add `db open` command #72

tangrufus commented Sep 25, 2019 •

edited

Loading

tangrufus commented Sep 25, 2019 •

edited

Loading

tangrufus commented Sep 26, 2019 •

edited

Loading

tangrufus commented Nov 16, 2019 •

edited

Loading

tangrufus Nov 18, 2019 •

edited

Loading

tangrufus commented Nov 26, 2019 •

edited

Loading