Skip to content

change -mysql_server_bind_address#5386

Merged
morgo merged 2 commits intovitessio:masterfrom
zendesk:lokune/change_mysql_server_bind_address_for_vttest_py
Dec 1, 2019
Merged

change -mysql_server_bind_address#5386
morgo merged 2 commits intovitessio:masterfrom
zendesk:lokune/change_mysql_server_bind_address_for_vttest_py

Conversation

@lokune
Copy link
Copy Markdown

@lokune lokune commented Oct 31, 2019 •
edited
Loading

change -mysql_server_bind_address
3b40fc6 
Signed-off-by: lokune <laban.okune@ma3route.com>
@lokune lokune requested a review from sougou as a code owner October 31, 2019 21:47
@morgo
Copy link
Copy Markdown
Contributor

morgo commented Nov 7, 2019

@enisoc do you know if there are any security consequences here, or is this straight forward to merge?

@morgo morgo requested a review from enisoc November 7, 2019 23:52
@enisoc
Copy link
Copy Markdown
Member

enisoc commented Nov 8, 2019

I would prefer to make this passed in as an option, with the default still being localhost since that's more secure. Would that still work for your use case @lokune?

@lokune
Copy link
Copy Markdown
Author

lokune commented Nov 8, 2019

@enisoc, I was thinking of the same, and yes it will still work for my use case but I assumed it should be fine as is since this is the test module?

@enisoc
Copy link
Copy Markdown
Member

enisoc commented Nov 8, 2019

I was thinking that we wouldn't want people to run something as a test and unintentionally expose a server over the network.

@lokune
Copy link
Copy Markdown
Author

lokune commented Nov 8, 2019

Makes sense... Seems like there has to be changes in vttest/run_local_database.py, local_database.py and vttest/vt_processes.py. Would you like to take over from here? I can also try if you are busy but I would prefer someone more experienced with Vitess codebase.

pass in mysql server bind address
3d36f61 
Signed-off-by: lokune <laban.okune@ma3route.com>
@lokune lokune force-pushed the lokune/change_mysql_server_bind_address_for_vttest_py branch from 8efeec3 to 3d36f61 Compare November 13, 2019 16:49
@lokune
Copy link
Copy Markdown
Author

lokune commented Nov 13, 2019

@enisoc, I decided to give it a shot and made the changes as per your suggestion. Let me know what you think. cc @morgo

@sougou sougou requested a review from morgo December 1, 2019 00:24
morgo
morgo approved these changes Dec 1, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@morgo morgo left a comment

Choose a reason for hiding this comment

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

LGTM

@morgo morgo merged commit fae58cc into vitessio:master Dec 1, 2019
@rafael rafael mentioned this pull request Dec 11, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sougou sougou Awaiting requested review from sougou

@enisoc enisoc Awaiting requested review from enisoc

1 more reviewer

@morgo morgo morgo approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lokune @morgo @enisoc