mozilla-services / syncstorage-rs

Sync Storage server in Rust
Mozilla Public License 2.0
863 stars 46 forks source link

tools/tokenserver: package or not? #1569

Closed 0-wiz-0 closed 2 months ago

0-wiz-0 commented 2 months ago

I've created a pkgsrc package for syncstorage-rs, and I'm a bit confused by the remaining Python code. There's a toplevel requirements.txt and there's a tools/tokenserver directory with another requirements.txt file, but AFAICT, the Rust syncserver program provides a tokenserver implementation.

Are the Python toplevel dependencies needed in any way? Should tools/tokenserver be installed when packaging this? If so, what does it do that syncserver doesn't?

Thanks.

┆Issue is synchronized with this Jira Task

jrconlin commented 2 months ago

Hi, There are a few complications involved around: 1) Combining Tokenserver with Syncserver. (Tokenserver has embedded Python in it, mostly because of legacy and "Do Not Implement Custom Cryptography Without Strong Review" lessons. There are a few pending tasks to tackle converting some of that to using modern, standard Rust functions, but they're pending a lot of other tasks and priorities.) 2) Our testing architecture (Our internal integration and load tests are written in Python3. Again, there are pending tasks to modernize that to use pyproject.toml files or poetry, but there are other priorities.)

The root requirements.txt is needed by Syncserver-rs for Tokenserver compatibility. It's needed when the (default) features=tokenserver-auth/py flag is set. The Rust encryption library is not fully tested and should not be used in production.

The tools/integration_tests/requirements.txt file is used by the integration tests The other, various requirements.txt files tend to relate to the various tooling directories.

0-wiz-0 commented 2 months ago

Thanks for the reply! It's still not completely clear to me what should be provided in a binary package for syncserver-rs.

But what about:

And which Python modules should the binary package install?

I hope you can clarify this. Thanks.

jrconlin commented 2 months ago

I suppose it depends on what you consider content for a binary package. The top-level requirements.txt file contains the root python packages used by the integrated python in the tokenserver_auth crate. tokenserver-auth calls pyo3 to pull in tokenserver-auth/src/token/py.rs, which imports tokenlib (defined in ./requirements.txt) and pulls in ./tokenserver-auth/src/oauth/verify.py'. This.pyimportsfxa(also defined in./requirements.txt`).

The ./tools directory mostly contains supplemental tools for operating and managing syncstorage. The ./tools/tokenserver are older scripts that were used by admins managing the tokenserver databases (particularly, when our servers were clusters of MySQL nodes that needed to coordinate.)

If you're looking for a minimal package, then processing the root ./requirements.txt and ./tokenserver-auth/src/oauth/verify.py should be enough. If you're looking to offer folk more utility (particularly if you believe that they're in the very, very rare group that may need to run a multi-node system) you may wish to include the ./tools/tokenserver/* directory. I would probably just offer a README that points folk off to that directory, though, since it's likely that maybe less than a dozen folk may ever be in that situation.

0-wiz-0 commented 2 months ago

Thank you for the explanations. The package has now been added to pkgsrc (without the supplemental tools.).