Mark internal configuration options and sections with underscore

This commit is contained in:
Unrud 2020-02-19 09:50:27 +01:00
parent 0bda1f4c16
commit 5371be2b39
12 changed files with 58 additions and 63 deletions

View File

@ -51,7 +51,7 @@ def run():
groups = {} groups = {}
for section, values in config.DEFAULT_CONFIG_SCHEMA.items(): for section, values in config.DEFAULT_CONFIG_SCHEMA.items():
if values.get("_internal", False): if section.startswith("_"):
continue continue
group = parser.add_argument_group(section) group = parser.add_argument_group(section)
groups[group] = [] groups[group] = []
@ -65,7 +65,7 @@ def run():
kwargs["dest"] = "%s_%s" % (section, option) kwargs["dest"] = "%s_%s" % (section, option)
groups[group].append(kwargs["dest"]) groups[group].append(kwargs["dest"])
del kwargs["value"] del kwargs["value"]
if "internal" in kwargs: with contextlib.suppress(KeyError):
del kwargs["internal"] del kwargs["internal"]
if kwargs["type"] == bool: if kwargs["type"] == bool:

View File

@ -281,7 +281,7 @@ class Application(
logger.warning("Access to principal path %r denied by " logger.warning("Access to principal path %r denied by "
"rights backend", principal_path) "rights backend", principal_path)
if self.configuration.get("internal", "internal_server"): if self.configuration.get("_internal", "internal_server"):
# Verify content length # Verify content length
content_length = int(environ.get("CONTENT_LENGTH") or 0) content_length = int(environ.get("CONTENT_LENGTH") or 0)
if content_length: if content_length:

View File

@ -87,6 +87,7 @@ def _convert_to_bool(value):
return RawConfigParser.BOOLEAN_STATES[value.lower()] return RawConfigParser.BOOLEAN_STATES[value.lower()]
INTERNAL_OPTIONS = ("_allow_extra",)
# Default configuration # Default configuration
DEFAULT_CONFIG_SCHEMA = OrderedDict([ DEFAULT_CONFIG_SCHEMA = OrderedDict([
("server", OrderedDict([ ("server", OrderedDict([
@ -204,8 +205,7 @@ DEFAULT_CONFIG_SCHEMA = OrderedDict([
"type": bool})])), "type": bool})])),
("headers", OrderedDict([ ("headers", OrderedDict([
("_allow_extra", True)])), ("_allow_extra", True)])),
("internal", OrderedDict([ ("_internal", OrderedDict([
("_internal", True),
("filesystem_fsync", { ("filesystem_fsync", {
"value": "True", "value": "True",
"help": "sync all changes to filesystem during requests", "help": "sync all changes to filesystem during requests",
@ -292,16 +292,13 @@ class Configuration:
self._schema = schema self._schema = schema
self._values = {} self._values = {}
self._configs = [] self._configs = []
values = {} default = {section: {option: self._schema[section][option]["value"]
for section in schema: for option in self._schema[section]
values[section] = {} if option not in INTERNAL_OPTIONS}
for option in schema[section]: for section in self._schema}
if option.startswith("_"): self.update(default, "default config", privileged=True)
continue
values[section][option] = schema[section][option]["value"]
self.update(values, "default config", internal=True)
def update(self, config, source=None, internal=False): def update(self, config, source=None, privileged=False):
"""Update the configuration. """Update the configuration.
``config`` a dict of the format {SECTION: {OPTION: VALUE, ...}, ...}. ``config`` a dict of the format {SECTION: {OPTION: VALUE, ...}, ...}.
@ -312,34 +309,33 @@ class Configuration:
``source`` a description of the configuration source (used in error ``source`` a description of the configuration source (used in error
messages). messages).
``internal`` allows updating "_internal" sections. ``privileged`` allows updating sections and options starting with "_".
""" """
source = source or "unspecified config" source = source or "unspecified config"
new_values = {} new_values = {}
for section in config: for section in config:
if (section not in self._schema or not internal and if (section not in self._schema or
self._schema[section].get("_internal", False)): section.startswith("_") and not privileged):
raise RuntimeError( raise ValueError(
"Invalid section %r in %s" % (section, source)) "Invalid section %r in %s" % (section, source))
new_values[section] = {} new_values[section] = {}
if "_allow_extra" in self._schema[section]: extra_type = None
allow_extra_options = self._schema[section]["_allow_extra"] if self._schema[section].get("_allow_extra"):
elif "type" in self._schema[section]: extra_type = str
if "type" in self._schema[section]:
if "type" in config[section]: if "type" in config[section]:
plugin_type = config[section]["type"] plugin = config[section]["type"]
else: else:
plugin_type = self.get(section, "type") plugin = self.get(section, "type")
allow_extra_options = plugin_type not in self._schema[section][ if plugin not in self._schema[section]["type"]["internal"]:
"type"].get("internal", []) extra_type = str
else:
allow_extra_options = False
for option in config[section]: for option in config[section]:
type_ = extra_type
if option in self._schema[section]: if option in self._schema[section]:
type_ = self._schema[section][option]["type"] type_ = self._schema[section][option]["type"]
elif allow_extra_options: if (not type_ or option in INTERNAL_OPTIONS or
type_ = str option.startswith("_") and not privileged):
else:
raise RuntimeError("Invalid option %r in section %r in " raise RuntimeError("Invalid option %r in section %r in "
"%s" % (option, section, source)) "%s" % (option, section, source))
raw_value = config[section][option] raw_value = config[section][option]
@ -352,12 +348,10 @@ class Configuration:
"Invalid %s value for option %r in section %r in %s: " "Invalid %s value for option %r in section %r in %s: "
"%r" % (type_.__name__, option, section, source, "%r" % (type_.__name__, option, section, source,
raw_value)) from e raw_value)) from e
self._configs.append((config, source, bool(internal))) self._configs.append((config, source, bool(privileged)))
for section in new_values: for section in new_values:
if section not in self._values: self._values[section] = self._values.get(section, {})
self._values[section] = {} self._values[section].update(new_values[section])
for option in new_values[section]:
self._values[section][option] = new_values[section][option]
def get(self, section, option): def get(self, section, option):
"""Get the value of ``option`` in ``section``.""" """Get the value of ``option`` in ``section``."""
@ -417,6 +411,6 @@ class Configuration:
section, option)) section, option))
schema[section][option] = value schema[section][option] = value
copy = type(self)(schema) copy = type(self)(schema)
for config, source, internal in self._configs: for config, source, privileged in self._configs:
copy.update(config, source, internal) copy.update(config, source, privileged)
return copy return copy

View File

@ -200,8 +200,8 @@ def serve(configuration, shutdown_socket):
logger.info("Starting Radicale") logger.info("Starting Radicale")
# Copy configuration before modifying # Copy configuration before modifying
configuration = configuration.copy() configuration = configuration.copy()
configuration.update({"internal": {"internal_server": "True"}}, "server", configuration.update({"_internal": {"internal_server": "True"}}, "server",
internal=True) privileged=True)
use_ssl = configuration.get("server", "ssl") use_ssl = configuration.get("server", "ssl")
server_class = ParallelHTTPSServer if use_ssl else ParallelHTTPServer server_class = ParallelHTTPSServer if use_ssl else ParallelHTTPServer

View File

@ -125,7 +125,7 @@ class Storage(
return os.path.join(filesystem_folder, "collection-root") return os.path.join(filesystem_folder, "collection-root")
def _fsync(self, fd): def _fsync(self, fd):
if self.configuration.get("internal", "filesystem_fsync"): if self.configuration.get("_internal", "filesystem_fsync"):
pathutils.fsync(fd) pathutils.fsync(fd)
def _sync_directory(self, path): def _sync_directory(self, path):
@ -134,7 +134,7 @@ class Storage(
This only works on POSIX and does nothing on other systems. This only works on POSIX and does nothing on other systems.
""" """
if not self.configuration.get("internal", "filesystem_fsync"): if not self.configuration.get("_internal", "filesystem_fsync"):
return return
if os.name == "posix": if os.name == "posix":
try: try:

View File

@ -42,11 +42,9 @@ def get_file_content(file_name):
def configuration_to_dict(configuration): def configuration_to_dict(configuration):
d = {} """Convert configuration to a dict with raw values."""
for section in configuration.sections(): return {section: {option: configuration.get_raw(section, option)
if configuration._schema[section].get("_internal", False): for option in configuration.options(section)
continue if not option.startswith("_")}
d[section] = {} for section in configuration.sections()
for option in configuration.options(section): if not section.startswith("_")}
d[section][option] = configuration.get_raw(section, option)
return d

View File

@ -44,9 +44,9 @@ class TestBaseAuthRequests(BaseTest):
self.configuration.update({ self.configuration.update({
"storage": {"filesystem_folder": self.colpath}, "storage": {"filesystem_folder": self.colpath},
# Disable syncing to disk for better performance # Disable syncing to disk for better performance
"internal": {"filesystem_fsync": "False"}, "_internal": {"filesystem_fsync": "False"},
# Set incorrect authentication delay to a very low value # Set incorrect authentication delay to a very low value
"auth": {"delay": "0.002"}}, "test", internal=True) "auth": {"delay": "0.002"}}, "test", privileged=True)
def teardown(self): def teardown(self):
shutil.rmtree(self.colpath) shutil.rmtree(self.colpath)

View File

@ -1350,9 +1350,9 @@ permissions: RrWw""")
"storage": {"type": self.storage_type, "storage": {"type": self.storage_type,
"filesystem_folder": self.colpath}, "filesystem_folder": self.colpath},
# Disable syncing to disk for better performance # Disable syncing to disk for better performance
"internal": {"filesystem_fsync": "False"}, "_internal": {"filesystem_fsync": "False"},
"rights": {"file": rights_file_path, "rights": {"file": rights_file_path,
"type": "from_file"}}, "test", internal=True) "type": "from_file"}}, "test", privileged=True)
self.application = Application(self.configuration) self.application = Application(self.configuration)
def teardown(self): def teardown(self):
@ -1373,8 +1373,8 @@ class TestMultiFileSystem(BaseFileSystemTest, BaseRequestsMixIn):
def test_fsync(self): def test_fsync(self):
"""Create a directory and file with syncing enabled.""" """Create a directory and file with syncing enabled."""
self.configuration.update({ self.configuration.update({"_internal": {"filesystem_fsync": "True"}},
"internal": {"filesystem_fsync": "True"}}, "test", internal=True) "test", privileged=True)
self.application = Application(self.configuration) self.application = Application(self.configuration)
self.mkcalendar("/calendar.ics/") self.mkcalendar("/calendar.ics/")

View File

@ -133,13 +133,13 @@ class TestConfig:
def test_internal(self): def test_internal(self):
configuration = config.load() configuration = config.load()
configuration.update({"internal": {"internal_server": "True"}}, "test", configuration.update({"_internal": {"internal_server": "True"}},
internal=True) "test", privileged=True)
with pytest.raises(Exception) as exc_info: with pytest.raises(Exception) as exc_info:
configuration.update( configuration.update(
{"internal": {"internal_server": "True"}}, "test") {"_internal": {"internal_server": "True"}}, "test")
e = exc_info.value e = exc_info.value
assert "Invalid section 'internal'" in str(e) assert "Invalid section '_internal'" in str(e)
def test_plugin_schema(self): def test_plugin_schema(self):
plugin_schema = {"auth": {"new_option": {"value": "False", plugin_schema = {"auth": {"new_option": {"value": "False",

View File

@ -37,7 +37,8 @@ class TestBaseRightsRequests(BaseTest):
self.configuration.update({ self.configuration.update({
"storage": {"filesystem_folder": self.colpath}, "storage": {"filesystem_folder": self.colpath},
# Disable syncing to disk for better performance # Disable syncing to disk for better performance
"internal": {"filesystem_fsync": "False"}}, "test", internal=True) "_internal": {"filesystem_fsync": "False"}},
"test", privileged=True)
def teardown(self): def teardown(self):
shutil.rmtree(self.colpath) shutil.rmtree(self.colpath)

View File

@ -68,7 +68,8 @@ class TestBaseServerRequests(BaseTest):
# Enable debugging for new processes # Enable debugging for new processes
"logging": {"level": "debug"}, "logging": {"level": "debug"},
# Disable syncing to disk for better performance # Disable syncing to disk for better performance
"internal": {"filesystem_fsync": "False"}}, "test", internal=True) "_internal": {"filesystem_fsync": "False"}},
"test", privileged=True)
self.thread = threading.Thread(target=server.serve, args=( self.thread = threading.Thread(target=server.serve, args=(
self.configuration, shutdown_socket_out)) self.configuration, shutdown_socket_out))
ssl_context = ssl.create_default_context() ssl_context = ssl.create_default_context()
@ -147,7 +148,7 @@ class TestBaseServerRequests(BaseTest):
def test_command_line_interface(self): def test_command_line_interface(self):
config_args = [] config_args = []
for section, values in config.DEFAULT_CONFIG_SCHEMA.items(): for section, values in config.DEFAULT_CONFIG_SCHEMA.items():
if values.get("_internal", False): if section.startswith("_"):
continue continue
for option, data in values.items(): for option, data in values.items():
if option.startswith("_"): if option.startswith("_"):

View File

@ -35,7 +35,8 @@ class TestBaseWebRequests(BaseTest):
self.configuration.update({ self.configuration.update({
"storage": {"filesystem_folder": self.colpath}, "storage": {"filesystem_folder": self.colpath},
# Disable syncing to disk for better performance # Disable syncing to disk for better performance
"internal": {"filesystem_fsync": "False"}}, "test", internal=True) "_internal": {"filesystem_fsync": "False"}},
"test", privileged=True)
self.application = Application(self.configuration) self.application = Application(self.configuration)
def teardown(self): def teardown(self):