Create basic admin command app #5597

erikjohnston · 2019-07-02T16:20:20Z

Add a basic app that is meant to be run as a "command". This lets server admins run functions as a command (using their existing config) without having to use an admin API. This has the advantage that resource intensive actions can be run separately from production synapse instances.

As a starting point this supports a single command export-data that exposes the functionality from #5589.

Note: This doesn't hook into replication at all, so caches will not be correctly invalidated. I think this is fine given the intention of this tool, though we might want to consider turning off caching entirely to ensure we have a consistent view?

Based on #5589.

richvdh

some thoughts on all this...

synapse/config/_base.py

synapse/app/admin_cmd.py

synapse/config/_base.py

richvdh · 2019-07-03T14:11:48Z

synapse/config/_base.py

@@ -201,6 +201,26 @@ def load_config(cls, description, argv):



why don't we move load_config to a global function called load_config_for_worker_app?

Could do, though I tend to think of the load_config* functions as creators of HomeserverConfig, so to me feels natural they'd be part of the class.

richvdh · 2019-07-03T14:13:46Z

synapse/config/_base.py

+        return obj
+
+    @classmethod
+    def create_argument_parser(cls, description):


probably makes more sense to construct the ArgumentParser in the caller and pass it in, making this add_arguments_to_parser?

what would happen if we moved both create_argument_parser and load_config_with_parser to a derived class of HomeserverConfig?

I'm not sure that would really buy us very much tbh?

richvdh · 2019-07-03T14:19:56Z

synapse/config/_base.py

+            `config_parser.parse_args(..)`
+        """
+
+        obj = cls()


might be better to do this in the caller?

I'm not fond of the idea that callers need to create a blank object and then call a function to properly instantiate it, rather than just call a class method that gives back a fully instantiated object.

I'm not sure, but happy to park this.

synapse/app/admin_cmd.py

codecov · 2019-07-15T12:49:42Z

Codecov Report

Merging #5597 into develop will increase coverage by 0.04%.
The diff coverage is 63.39%.

@@            Coverage Diff             @@
##           develop   #5597      +/-   ##
==========================================
+ Coverage    63.16%   63.2%   +0.04%     
==========================================
  Files          328     328              
  Lines        35930   36011      +81     
  Branches      5917    5934      +17     
==========================================
+ Hits         22694   22762      +68     
- Misses       11609   11624      +15     
+ Partials      1627    1625       -2

codecov · 2019-07-15T12:49:43Z

Codecov Report

Merging #5597 into develop will increase coverage by 0.01%.
The diff coverage is 91.3%.

@@             Coverage Diff             @@
##           develop    #5597      +/-   ##
===========================================
+ Coverage    63.34%   63.35%   +0.01%     
===========================================
  Files          331      331              
  Lines        36129    36140      +11     
  Branches      5952     5954       +2     
===========================================
+ Hits         22885    22898      +13     
+ Misses       11613    11611       -2     
  Partials      1631     1631

…min_api_cmd

synapse/app/admin_cmd.py

Co-Authored-By: Aaron Raimist <aaron@raim.ist>

richvdh

Generally seems sane. What does the --help look like>

synapse/config/_base.py

richvdh · 2019-07-16T10:34:42Z

synapse/config/_base.py

+            `config_parser.parse_args(..)`
+        """
+
+        obj = cls()


I'm not sure, but happy to park this.

erikjohnston · 2019-07-16T10:40:23Z

Help looks like:

$ python -m synapse.app.admin_cmd --help
usage: admin_cmd.py [-h] [-c CONFIG_FILE] [--keys-directory DIRECTORY] [-D]
                    [--print-pidfile] [--manhole PORT]
                    [-d SQLITE_DATABASE_PATH] [-v] [-f LOG_FILE]
                    [--log-config LOG_CONFIG] [-n] [--enable-registration]
                    <admin_command> ...
Synapse Admin Command
optional arguments:
  -h, --help            show this help message and exit
  -c CONFIG_FILE, --config-path CONFIG_FILE
                        Specify config file. Can be given multiple times and
                        may specify directories containing *.yaml files.
  --keys-directory DIRECTORY
                        Where files such as certs and signing keys are stored
                        when their location is not given explicitly in the
                        config. Defaults to the directory containing the last
                        config file
server:
  -D, --daemonize       Daemonize the home server
  --print-pidfile       Print the path to the pidfile just before daemonizing
  --manhole PORT        Turn on the twisted telnet manhole service on the
                        given port.
database:
  -d SQLITE_DATABASE_PATH, --database-path SQLITE_DATABASE_PATH
                        The path to a sqlite database to use.
logging:
  -v, --verbose         The verbosity level. Specify multiple times to
                        increase verbosity. (Ignored if --log-config is
                        specified.)
  -f LOG_FILE, --log-file LOG_FILE
                        File to log to. (Ignored if --log-config is
                        specified.)
  --log-config LOG_CONFIG
                        Python logging config file
  -n, --no-redirect-stdio
                        Do not redirect stdout/stderr to the log
registration:
  --enable-registration
                        Enable registration for new users.
Admin Commands:
  Choose an admin command to perform.
  <admin_command>       The admin command to perform.
    export-data         Export all data for a user

And

$ python -m synapse.app.admin_cmd export-data  --help
usage: admin_cmd.py export-data [-h] [--output-directory DIRECTORY] user_id
positional arguments:
  user_id               User to extra data from
optional arguments:
  -h, --help            show this help message and exit
  --output-directory DIRECTORY
                        The directory to store the exported data in. Must be
                        emtpy. Defaults to creating a temp directory.

richvdh

lgtm

@skalarproduktraum

v1.2.0rc1 Features -------- - Add support for opentracing. ([\#5544](#5544), [\#5712](#5712)) - Add ability to pull all locally stored events out of synapse that a particular user can see. ([\#5589](#5589)) - Add a basic admin command app to allow server operators to run Synapse admin commands separately from the main production instance. ([\#5597](#5597)) - Add `sender` and `origin_server_ts` fields to `m.replace`. ([\#5613](#5613)) - Add default push rule to ignore reactions. ([\#5623](#5623)) - Include the original event when asking for its relations. ([\#5626](#5626)) - Implement `session_lifetime` configuration option, after which access tokens will expire. ([\#5660](#5660)) - Return "This account has been deactivated" when a deactivated user tries to login. ([\#5674](#5674)) - Enable aggregations support by default ([\#5714](#5714)) Bugfixes -------- - Fix 'utime went backwards' errors on daemonization. ([\#5609](#5609)) - Various minor fixes to the federation request rate limiter. ([\#5621](#5621)) - Forbid viewing relations on an event once it has been redacted. ([\#5629](#5629)) - Fix requests to the `/store_invite` endpoint of identity servers being sent in the wrong format. ([\#5638](#5638)) - Fix newly-registered users not being able to lookup their own profile without joining a room. ([\#5644](#5644)) - Fix bug in #5626 that prevented the original_event field from actually having the contents of the original event in a call to `/relations`. ([\#5654](#5654)) - Fix 3PID bind requests being sent to identity servers as `application/x-form-www-urlencoded` data, which is deprecated. ([\#5658](#5658)) - Fix some problems with authenticating redactions in recent room versions. ([\#5699](#5699), [\#5700](#5700), [\#5707](#5707)) - Ignore redactions of m.room.create events. ([\#5701](#5701)) Updates to the Docker image --------------------------- - Base Docker image on a newer Alpine Linux version (3.8 -> 3.10). ([\#5619](#5619)) - Add missing space in default logging file format generated by the Docker image. ([\#5620](#5620)) Improved Documentation ---------------------- - Add information about nginx normalisation to reverse_proxy.rst. Contributed by @skalarproduktraum - thanks! ([\#5397](#5397)) - --no-pep517 should be --no-use-pep517 in the documentation to setup the development environment. ([\#5651](#5651)) - Improvements to Postgres setup instructions. Contributed by @Lrizika - thanks! ([\#5661](#5661)) - Minor tweaks to postgres documentation. ([\#5675](#5675)) Deprecations and Removals ------------------------- - Remove support for the `invite_3pid_guest` configuration setting. ([\#5625](#5625)) Internal Changes ---------------- - Move logging code out of `synapse.util` and into `synapse.logging`. ([\#5606](#5606), [\#5617](#5617)) - Add a blacklist file to the repo to blacklist certain sytests from failing CI. ([\#5611](#5611)) - Make runtime errors surrounding password reset emails much clearer. ([\#5616](#5616)) - Remove dead code for persiting outgoing federation transactions. ([\#5622](#5622)) - Add `lint.sh` to the scripts-dev folder which will run all linting steps required by CI. ([\#5627](#5627)) - Move RegistrationHandler.get_or_create_user to test code. ([\#5628](#5628)) - Add some more common python virtual-environment paths to the black exclusion list. ([\#5630](#5630)) - Some counter metrics exposed over Prometheus have been renamed, with the old names preserved for backwards compatibility and deprecated. See `docs/metrics-howto.rst` for details. ([\#5636](#5636)) - Unblacklist some user_directory sytests. ([\#5637](#5637)) - Factor out some redundant code in the login implementation. ([\#5639](#5639)) - Update ModuleApi to avoid register(generate_token=True). ([\#5640](#5640)) - Remove access-token support from `RegistrationHandler.register`, and rename it. ([\#5641](#5641)) - Remove access-token support from `RegistrationStore.register`, and rename it. ([\#5642](#5642)) - Improve logging for auto-join when a new user is created. ([\#5643](#5643)) - Remove unused and unnecessary check for FederationDeniedError in _exception_to_failure. ([\#5645](#5645)) - Fix a small typo in a code comment. ([\#5655](#5655)) - Clean up exception handling around client access tokens. ([\#5656](#5656)) - Add a mechanism for per-test homeserver configuration in the unit tests. ([\#5657](#5657)) - Inline issue_access_token. ([\#5659](#5659)) - Update the sytest BuildKite configuration to checkout Synapse in `/src`. ([\#5664](#5664)) - Add a `docker` type to the towncrier configuration. ([\#5673](#5673)) - Convert `synapse.federation.transport.server` to `async`. Might improve some stack traces. ([\#5689](#5689)) - Documentation for opentracing. ([\#5703](#5703))

zner0L · 2019-10-17T13:59:09Z

I am a little confused as of how to use this. Could you give me a usage example, please?

I don't really remember why this was so complicated; I think it dates back to the time when we had to instantiate the Config classes before we could call `add_arguments` - ie before #5597. In any case, I don't think there's a good reason for it any more, and the impact of it being complicated is that `--help` doesn't work correctly.

I don't really remember why this was so complicated; I think it dates back to the time when we had to instantiate the Config classes before we could call `add_arguments` - ie before matrix-org#5597. In any case, I don't think there's a good reason for it any more, and the impact of it being complicated is that `--help` doesn't work correctly.

erikjohnston added 2 commits July 2, 2019 17:12

Add basic admin cmd app

9f3c0a8

Newsfile

10fe904

erikjohnston requested a review from a team July 2, 2019 16:24

richvdh reviewed Jul 3, 2019

View reviewed changes

erikjohnston added 4 commits July 15, 2019 14:09

Change add_arguments to be a static method

823e13d

Fix up comments

37b524f

Move creation of ArgumentParser to caller

fdefb9e

Use set_defaults(func=) style

c8f35d8

erikjohnston force-pushed the erikj/admin_api_cmd branch from dc3fba1 to c8f35d8 Compare July 15, 2019 13:09

erikjohnston requested a review from richvdh July 15, 2019 13:10

erikjohnston added 4 commits July 15, 2019 14:13

Merge branch 'develop' of github.com:matrix-org/synapse into erikj/ad…

e8c53b0

…min_api_cmd

Add FileExfiltrationWriter

1b2067f

s/exfiltrate_user_data/export_user_data/

eca4f5a

Fix invoking add_argument from homeserver.py

03cc8c4

aaronraimist reviewed Jul 15, 2019

View reviewed changes

synapse/app/admin_cmd.py Outdated Show resolved Hide resolved

Fix typo in synapse/app/admin_cmd.py

d0d479c

Co-Authored-By: Aaron Raimist <aaron@raim.ist>

richvdh reviewed Jul 16, 2019

View reviewed changes

Clean up arg name and remove lying comment

f44354e

Remove pointless description

5ed7853

erikjohnston requested a review from richvdh July 16, 2019 12:21

richvdh approved these changes Jul 16, 2019

View reviewed changes

erikjohnston merged commit c831c5b into develop Jul 16, 2019

erikjohnston deleted the erikj/admin_api_cmd branch January 9, 2020 15:50

anoadragon453 added a commit that referenced this pull request Feb 19, 2020

Merge pull request #5597 from matrix-org/erikj/admin_api_cmd

8c226ce

richvdh mentioned this pull request Apr 8, 2020

Fix --help commandline argument #7249

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create basic admin command app #5597

Create basic admin command app #5597

erikjohnston commented Jul 2, 2019

richvdh left a comment

richvdh Jul 3, 2019

erikjohnston Jul 15, 2019

richvdh Jul 3, 2019

richvdh Jul 3, 2019

erikjohnston Jul 15, 2019

richvdh Jul 3, 2019

erikjohnston Jul 15, 2019

richvdh Jul 16, 2019

codecov bot commented Jul 15, 2019

codecov bot commented Jul 15, 2019 •

edited

Loading

richvdh left a comment

richvdh Jul 16, 2019

erikjohnston commented Jul 16, 2019

richvdh left a comment

zner0L commented Oct 17, 2019

		@@ -201,6 +201,26 @@ def load_config(cls, description, argv):

Create basic admin command app #5597

Create basic admin command app #5597

Conversation

erikjohnston commented Jul 2, 2019

richvdh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 15, 2019

Codecov Report

codecov bot commented Jul 15, 2019 • edited Loading

Codecov Report

richvdh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erikjohnston commented Jul 16, 2019

richvdh left a comment

Choose a reason for hiding this comment

zner0L commented Oct 17, 2019

codecov bot commented Jul 15, 2019 •

edited

Loading