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

Pack web files and mysql config files into binaries #5502

Closed
morgo opened this issue Dec 4, 2019 · 5 comments · Fixed by #5654
Closed

Pack web files and mysql config files into binaries #5502

morgo opened this issue Dec 4, 2019 · 5 comments · Fixed by #5654
Labels
good first issue Good issue for new contributors Type: Enhancement Logical improvement (somewhere between a bug and feature)

Comments

@morgo
Copy link
Contributor

morgo commented Dec 4, 2019

Feature Description

The examples currently have to refer to VTROOT and VTTOP, because they need to know how to start vtctld while referencing the correct web files:

https://github.com/vitessio/vitess/blob/master/examples/local/env.sh#L21
..
export VTTOP=${VTTOP-$VTROOT/src/vitess.io/vitess}

And then:

https://github.com/vitessio/vitess/blob/master/examples/local/vtctld-up.sh#L39-L40
...
  -web_dir $VTTOP/web/vtctld \
  -web_dir2 $VTTOP/web/vtctld2/app \

There are several technologies that allow you to embed web files and produce a single binary. We should look at doing this for both the web files, and config/mycnf/* and config/init_db.sql.

This will simplify the installation, similar to how MySQL switched from mysql_install_db bootstrapping the server to mysqld --initialize (embedded in the server).

Use Case(s)

Simplified distribution and configuration. This will soon be the only reason that VTTOP and VTROOT need to be specified outside of compile time.

@morgo morgo added the Type: Enhancement Logical improvement (somewhere between a bug and feature) label Dec 4, 2019
@morgo
Copy link
Contributor Author

morgo commented Dec 7, 2019

There is an additional runtime usage for VTROOT, and that is the use of hooks. We will need to figure out another answer for this if this variable is to be removed.

@morgo morgo added the good first issue Good issue for new contributors label Dec 9, 2019
@enisoc
Copy link
Member

enisoc commented Dec 12, 2019

I'm concerned about embedding init_db.sql for security reasons.

That file contains insecure defaults (password-less users). We don't yet support generating passwords automatically; our only, admittedly poor, mitigation right now is a strong recommendation that users customize init_db.sql before running Vitess in production.

We also don't yet support injecting "extra init_db.sql" in the style of EXTRA_MY_CNF. So if we embed this file, it will become impossible to customize without creating a custom build, and we will move even farther away from a good security position.

@morgo
Copy link
Contributor Author

morgo commented Jan 3, 2020

our only, admittedly poor, mitigation right now is a strong recommendation that users customize init_db.sql before running Vitess in production.

I agree it is poor to expect users customize it. I don't see a material difference between asking a user to edit the file before running the bootstrap and setting passwords in MySQL after bootstrapping.

I would like to see us auto-generate passwords and report them in the output. But I see this as a separate issue.

@enisoc
Copy link
Member

enisoc commented Jan 4, 2020

I don't see a material difference between asking a user to edit the file before running the bootstrap and setting passwords in MySQL after bootstrapping.

How would setting the MySQL-level passwords later work? I don't see a good way we can recommend users to do that across all tablets and have things still work.

@morgo
Copy link
Contributor Author

morgo commented Jan 4, 2020

For MySQL, an identity is the combination of a user + the host they are coming from. So there are two users we need to be aware of which allow remote login:

GRANT REPLICATION SLAVE ON *.* TO 'vt_repl'@'%';
GRANT SUPER, PROCESS, REPLICATION SLAVE, RELOAD
  ON *.* TO 'orc_client_user'@'%';

The orchestrator configuration is really risky since it includes SUPER from a remote host. User's always have permissions to change their own password, so the install of orchestrator should suggest changing the password:

SET PASSWORD='newpassword';

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good issue for new contributors Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants