Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Require direct references to configuration variables. (#10985)
Browse files Browse the repository at this point in the history
This removes the magic allowing accessing configurable
variables directly from the config object. It is now required
that a specific configuration class is used (e.g. `config.foo`
must be replaced with `config.server.foo`).
  • Loading branch information
clokep committed Oct 6, 2021
1 parent 829f2a8 commit f4b1a9a
Show file tree
Hide file tree
Showing 31 changed files with 124 additions and 160 deletions.
1 change: 1 addition & 0 deletions changelog.d/10985.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Use direct references to config flags.
4 changes: 2 additions & 2 deletions scripts/synapse_port_db
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ class MockHomeserver:
def __init__(self, config):
self.clock = Clock(reactor)
self.config = config
self.hostname = config.server_name
self.hostname = config.server.server_name
self.version_string = "Synapse/" + get_version_string(synapse)

def get_clock(self):
Expand Down Expand Up @@ -583,7 +583,7 @@ class Porter(object):
return

self.postgres_store = self.build_db_store(
self.hs_config.get_single_database()
self.hs_config.database.get_single_database()
)

await self.run_background_updates_on_postgres()
Expand Down
2 changes: 1 addition & 1 deletion scripts/update_synapse_database
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class MockHomeserver(HomeServer):

def __init__(self, config, **kwargs):
super(MockHomeserver, self).__init__(
config.server_name, reactor=reactor, config=config, **kwargs
config.server.server_name, reactor=reactor, config=config, **kwargs
)

self.version_string = "Synapse/" + get_version_string(synapse)
Expand Down
2 changes: 1 addition & 1 deletion synapse/app/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ def refresh_certificate(hs):
if not hs.config.server.has_tls_listener():
return

hs.config.read_certificate_from_disk()
hs.config.tls.read_certificate_from_disk()
hs.tls_server_context_factory = context_factory.ServerContextFactory(hs.config)

if hs._listening_services:
Expand Down
4 changes: 2 additions & 2 deletions synapse/app/admin_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,9 @@ def start(config_options):
# Explicitly disable background processes
config.server.update_user_directory = False
config.worker.run_background_tasks = False
config.start_pushers = False
config.worker.start_pushers = False
config.pusher_shard_config.instances = []
config.send_federation = False
config.worker.send_federation = False
config.federation_shard_config.instances = []

synapse.events.USE_FROZEN_DICTS = config.server.use_frozen_dicts
Expand Down
2 changes: 1 addition & 1 deletion synapse/app/homeserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ def _configure_named_resource(self, name, compress=False):
)

if name in ["media", "federation", "client"]:
if self.config.media.enable_media_repo:
if self.config.server.enable_media_repo:
media_repo = self.get_media_repository_resource()
resources.update(
{MEDIA_PREFIX: media_repo, LEGACY_MEDIA_PREFIX: media_repo}
Expand Down
64 changes: 8 additions & 56 deletions synapse/config/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,21 +118,6 @@ def __init__(self, root_config=None):
"synapse", "res/templates"
)

def __getattr__(self, item: str) -> Any:
"""
Try and fetch a configuration option that does not exist on this class.
This is so that existing configs that rely on `self.value`, where value
is actually from a different config section, continue to work.
"""
if item in ["generate_config_section", "read_config"]:
raise AttributeError(item)

if self.root is None:
raise AttributeError(item)
else:
return self.root._get_unclassed_config(self.section, item)

@staticmethod
def parse_size(value):
if isinstance(value, int):
Expand Down Expand Up @@ -289,7 +274,9 @@ def read_templates(
env.filters.update(
{
"format_ts": _format_ts_filter,
"mxc_to_http": _create_mxc_to_http_filter(self.public_baseurl),
"mxc_to_http": _create_mxc_to_http_filter(
self.root.server.public_baseurl
),
}
)

Expand All @@ -311,8 +298,6 @@ class RootConfig:
config_classes = []

def __init__(self):
self._configs = OrderedDict()

for config_class in self.config_classes:
if config_class.section is None:
raise ValueError("%r requires a section name" % (config_class,))
Expand All @@ -321,42 +306,7 @@ def __init__(self):
conf = config_class(self)
except Exception as e:
raise Exception("Failed making %s: %r" % (config_class.section, e))
self._configs[config_class.section] = conf

def __getattr__(self, item: str) -> Any:
"""
Redirect lookups on this object either to config objects, or values on
config objects, so that `config.tls.blah` works, as well as legacy uses
of things like `config.server.server_name`. It will first look up the config
section name, and then values on those config classes.
"""
if item in self._configs.keys():
return self._configs[item]

return self._get_unclassed_config(None, item)

def _get_unclassed_config(self, asking_section: Optional[str], item: str):
"""
Fetch a config value from one of the instantiated config classes that
has not been fetched directly.
Args:
asking_section: If this check is coming from a Config child, which
one? This section will not be asked if it has the value.
item: The configuration value key.
Raises:
AttributeError if no config classes have the config key. The body
will contain what sections were checked.
"""
for key, val in self._configs.items():
if key == asking_section:
continue

if item in dir(val):
return getattr(val, item)

raise AttributeError(item, "not found in %s" % (list(self._configs.keys()),))
setattr(self, config_class.section, conf)

def invoke_all(self, func_name: str, *args, **kwargs) -> MutableMapping[str, Any]:
"""
Expand All @@ -373,9 +323,11 @@ def invoke_all(self, func_name: str, *args, **kwargs) -> MutableMapping[str, Any
"""
res = OrderedDict()

for name, config in self._configs.items():
for config_class in self.config_classes:
config = getattr(self, config_class.section)

if hasattr(config, func_name):
res[name] = getattr(config, func_name)(*args, **kwargs)
res[config_class.section] = getattr(config, func_name)(*args, **kwargs)

return res

Expand Down
2 changes: 1 addition & 1 deletion synapse/config/account_validity.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def read_config(self, config, **kwargs):
)

if self.account_validity_renew_by_email_enabled:
if not self.public_baseurl:
if not self.root.server.public_baseurl:
raise ConfigError("Can't send renewal emails without 'public_baseurl'")

# Load account validity templates.
Expand Down
2 changes: 1 addition & 1 deletion synapse/config/cas.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def read_config(self, config, **kwargs):

# The public baseurl is required because it is used by the redirect
# template.
public_baseurl = self.public_baseurl
public_baseurl = self.root.server.public_baseurl
if not public_baseurl:
raise ConfigError("cas_config requires a public_baseurl to be set")

Expand Down
9 changes: 4 additions & 5 deletions synapse/config/emailconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import logging
import os
from enum import Enum
from typing import Optional

import attr

Expand Down Expand Up @@ -135,7 +134,7 @@ def read_config(self, config, **kwargs):
# msisdn is currently always remote while Synapse does not support any method of
# sending SMS messages
ThreepidBehaviour.REMOTE
if self.account_threepid_delegate_email
if self.root.registration.account_threepid_delegate_email
else ThreepidBehaviour.LOCAL
)
# Prior to Synapse v1.4.0, there was another option that defined whether Synapse would
Expand All @@ -144,7 +143,7 @@ def read_config(self, config, **kwargs):
# identity server in the process.
self.using_identity_server_from_trusted_list = False
if (
not self.account_threepid_delegate_email
not self.root.registration.account_threepid_delegate_email
and config.get("trust_identity_server_for_password_resets", False) is True
):
# Use the first entry in self.trusted_third_party_id_servers instead
Expand All @@ -156,7 +155,7 @@ def read_config(self, config, **kwargs):

# trusted_third_party_id_servers does not contain a scheme whereas
# account_threepid_delegate_email is expected to. Presume https
self.account_threepid_delegate_email: Optional[str] = (
self.root.registration.account_threepid_delegate_email = (
"https://" + first_trusted_identity_server
)
self.using_identity_server_from_trusted_list = True
Expand Down Expand Up @@ -335,7 +334,7 @@ def read_config(self, config, **kwargs):
"client_base_url", email_config.get("riot_base_url", None)
)

if self.account_validity_renew_by_email_enabled:
if self.root.account_validity.account_validity_renew_by_email_enabled:
expiry_template_html = email_config.get(
"expiry_template_html", "notice_expiry.html"
)
Expand Down
6 changes: 4 additions & 2 deletions synapse/config/key.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,13 @@ def read_config(self, config, config_dir_path, **kwargs):

# list of TrustedKeyServer objects
self.key_servers = list(
_parse_key_servers(key_servers, self.federation_verify_certificates)
_parse_key_servers(
key_servers, self.root.tls.federation_verify_certificates
)
)

self.macaroon_secret_key = config.get(
"macaroon_secret_key", self.registration_shared_secret
"macaroon_secret_key", self.root.registration.registration_shared_secret
)

if not self.macaroon_secret_key:
Expand Down
2 changes: 1 addition & 1 deletion synapse/config/oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def read_config(self, config, **kwargs):
"Multiple OIDC providers have the idp_id %r." % idp_id
)

public_baseurl = self.public_baseurl
public_baseurl = self.root.server.public_baseurl
if public_baseurl is None:
raise ConfigError("oidc_config requires a public_baseurl to be set")
self.oidc_callback_url = public_baseurl + "_synapse/client/oidc/callback"
Expand Down
7 changes: 5 additions & 2 deletions synapse/config/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ def read_config(self, config, **kwargs):
account_threepid_delegates = config.get("account_threepid_delegates") or {}
self.account_threepid_delegate_email = account_threepid_delegates.get("email")
self.account_threepid_delegate_msisdn = account_threepid_delegates.get("msisdn")
if self.account_threepid_delegate_msisdn and not self.public_baseurl:
if (
self.account_threepid_delegate_msisdn
and not self.root.server.public_baseurl
):
raise ConfigError(
"The configuration option `public_baseurl` is required if "
"`account_threepid_delegate.msisdn` is set, such that "
Expand Down Expand Up @@ -85,7 +88,7 @@ def read_config(self, config, **kwargs):
if mxid_localpart:
# Convert the localpart to a full mxid.
self.auto_join_user_id = UserID(
mxid_localpart, self.server_name
mxid_localpart, self.root.server.server_name
).to_string()

if self.autocreate_auto_join_rooms:
Expand Down
2 changes: 1 addition & 1 deletion synapse/config/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def read_config(self, config, **kwargs):
# Only enable the media repo if either the media repo is enabled or the
# current worker app is the media repo.
if (
self.enable_media_repo is False
self.root.server.enable_media_repo is False
and config.get("worker_app") != "synapse.app.media_repository"
):
self.can_load_media_repo = False
Expand Down
2 changes: 1 addition & 1 deletion synapse/config/saml2.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ def _default_saml_config_dict(
"""
import saml2

public_baseurl = self.public_baseurl
public_baseurl = self.root.server.public_baseurl
if public_baseurl is None:
raise ConfigError("saml2_config requires a public_baseurl to be set")

Expand Down
4 changes: 3 additions & 1 deletion synapse/config/server_notices.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ def read_config(self, config, **kwargs):
return

mxid_localpart = c["system_mxid_localpart"]
self.server_notices_mxid = UserID(mxid_localpart, self.server_name).to_string()
self.server_notices_mxid = UserID(
mxid_localpart, self.root.server.server_name
).to_string()
self.server_notices_mxid_display_name = c.get("system_mxid_display_name", None)
self.server_notices_mxid_avatar_url = c.get("system_mxid_avatar_url", None)
# todo: i18n
Expand Down
6 changes: 4 additions & 2 deletions synapse/config/sso.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,10 @@ def read_config(self, config, **kwargs):
# the client's.
# public_baseurl is an optional setting, so we only add the fallback's URL to the
# list if it's provided (because we can't figure out what that URL is otherwise).
if self.public_baseurl:
login_fallback_url = self.public_baseurl + "_matrix/static/client/login"
if self.root.server.public_baseurl:
login_fallback_url = (
self.root.server.public_baseurl + "_matrix/static/client/login"
)
self.sso_client_whitelist.append(login_fallback_url)

def generate_config_section(self, **kwargs):
Expand Down
8 changes: 2 additions & 6 deletions synapse/handlers/account_validity.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,8 @@ def __init__(self, hs: "HomeServer"):
and self._account_validity_renew_by_email_enabled
):
# Don't do email-specific configuration if renewal by email is disabled.
self._template_html = (
hs.config.account_validity.account_validity_template_html
)
self._template_text = (
hs.config.account_validity.account_validity_template_text
)
self._template_html = hs.config.email.account_validity_template_html
self._template_text = hs.config.email.account_validity_template_text
self._renew_email_subject = (
hs.config.account_validity.account_validity_renew_email_subject
)
Expand Down
7 changes: 5 additions & 2 deletions synapse/handlers/room_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -1499,8 +1499,11 @@ async def _remote_join(
if len(remote_room_hosts) == 0:
raise SynapseError(404, "No known servers")

check_complexity = self.hs.config.limit_remote_rooms.enabled
if check_complexity and self.hs.config.limit_remote_rooms.admins_can_join:
check_complexity = self.hs.config.server.limit_remote_rooms.enabled
if (
check_complexity
and self.hs.config.server.limit_remote_rooms.admins_can_join
):
check_complexity = not await self.auth.is_server_admin(user)

if check_complexity:
Expand Down
2 changes: 1 addition & 1 deletion synapse/replication/tcp/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def __init__(self, hs: "HomeServer"):
self._instance_name = hs.get_instance_name()
self._typing_handler = hs.get_typing_handler()

self._notify_pushers = hs.config.start_pushers
self._notify_pushers = hs.config.worker.start_pushers
self._pusher_pool = hs.get_pusherpool()
self._presence_handler = hs.get_presence_handler()

Expand Down
7 changes: 5 additions & 2 deletions synapse/replication/tcp/handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,10 @@ def __init__(self, hs: "HomeServer"):
if hs.config.worker.worker_app is not None:
continue

if stream.NAME == FederationStream.NAME and hs.config.send_federation:
if (
stream.NAME == FederationStream.NAME
and hs.config.worker.send_federation
):
# We only support federation stream if federation sending
# has been disabled on the master.
continue
Expand Down Expand Up @@ -225,7 +228,7 @@ def __init__(self, hs: "HomeServer"):
self._is_master = hs.config.worker.worker_app is None

self._federation_sender = None
if self._is_master and not hs.config.send_federation:
if self._is_master and not hs.config.worker.send_federation:
self._federation_sender = hs.get_federation_sender()

self._server_notices_sender = None
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/client/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def __init__(self, hs: "HomeServer"):
self.auth_handler = hs.get_auth_handler()
self.registration_handler = hs.get_registration_handler()
self.recaptcha_template = hs.config.captcha.recaptcha_template
self.terms_template = hs.config.terms_template
self.terms_template = hs.config.consent.terms_template
self.registration_token_template = (
hs.config.registration.registration_token_template
)
Expand Down
4 changes: 3 additions & 1 deletion synapse/rest/client/push_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ def __init__(self, hs: "HomeServer"):
self.notifier = hs.get_notifier()
self._is_worker = hs.config.worker.worker_app is not None

self._users_new_default_push_rules = hs.config.users_new_default_push_rules
self._users_new_default_push_rules = (
hs.config.server.users_new_default_push_rules
)

async def on_PUT(self, request: SynapseRequest, path: str) -> Tuple[int, JsonDict]:
if self._is_worker:
Expand Down
4 changes: 3 additions & 1 deletion synapse/storage/databases/main/push_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ def __init__(self, database: DatabasePool, db_conn, hs):
prefilled_cache=push_rules_prefill,
)

self._users_new_default_push_rules = hs.config.users_new_default_push_rules
self._users_new_default_push_rules = (
hs.config.server.users_new_default_push_rules
)

@abc.abstractmethod
def get_max_push_rules_stream_id(self):
Expand Down
Loading

0 comments on commit f4b1a9a

Please sign in to comment.