From 32a4a8260333a43e842abaa666977128ed0af857 Mon Sep 17 00:00:00 2001 From: Evgeniy Alekseev Date: Thu, 28 Oct 2021 03:15:06 +0300 Subject: [PATCH] improve configuration extension * Allow spaces in lists. This feature has been done in the way as shell interprets arguments by using quotation marks * Clear current content on reload --- docs/configuration.md | 19 +++-- src/ahriman/core/configuration.py | 32 +++++++- tests/ahriman/core/test_configuration.py | 96 +++++++++++++++++------- 3 files changed, 113 insertions(+), 34 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 374ffec0..17da1cc5 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1,6 +1,13 @@ # ahriman configuration -Some groups can be specified for each architecture separately. E.g. if there are `build` and `build:x86_64` groups it will use the option from `build:x86_64` for the `x86_64` architecture and `build` for any other (architecture specific group has higher priority). In case if both groups are presented, architecture specific options will be merged into global ones overriding them. +Some groups can be specified for each architecture separately. E.g. if there are `build` and `build:x86_64` groups it will use the option from `build:x86_64` for the `x86_64` architecture and `build` for any other (architecture specific group has higher priority). In case if both groups are presented, architecture specific options will be merged into global ones overriding them. + +Some values have list of strings type. Those values will be read in the same way as shell does: + +* By default, it splits value by spaces excluding empty elements. +* In case if quotation mark (`"` or `'`) will be found, any spaces inside will be ignored. +* In order to use quotation mark inside value it is required to put it to another quotation mark, e.g. `wor"'"d "with quote"` will be parsed as `["wor'd", "with quote"]` and vice versa. +* Unclosed quotation mark is not allowed and will rise an exception. ## `settings` group @@ -38,11 +45,11 @@ Authorization mapping. Group name must refer to user access level, i.e. it shoul Key is always username (case-insensitive), option value depends on authorization provider: * `OAuth` - by default requires only usernames and ignores values. But in case of direct login method call (via POST request) it will act as `Mapping` authorization method. -* `Mapping` (default) - reads salted password hashes from values, uses SHA512 in order to hash passwords. Password can be set by using `create-user` subcommand. +* `Mapping` (default) - reads salted password hashes from values, uses SHA512 in order to hash passwords. Password can be set by using `user-add` subcommand. ## `build:*` groups -Build related configuration. Group name must refer to architecture, e.g. it should be `build:x86_64` for x86_64 architecture. +Build related configuration. Group name can refer to architecture, e.g. `build:x86_64` can be used for x86_64 architecture specific settings. * `archbuild_flags` - additional flags passed to `archbuild` command, space separated list of strings, optional. * `build_command` - default build command, string, required. @@ -59,7 +66,7 @@ Base repository settings. ## `sign:*` groups -Settings for signing packages or repository. Group name must refer to architecture, e.g. it should be `sign:x86_64` for x86_64 architecture. +Settings for signing packages or repository. Group name can refer to architecture, e.g. `sign:x86_64` can be used for x86_64 architecture specific settings. * `target` - configuration flag to enable signing, space separated list of strings, required. Allowed values are `package` (sign each package separately), `repository` (sign repository database file). * `key` - default PGP key, string, required. This key will also be used for database signing if enabled. @@ -69,7 +76,7 @@ Settings for signing packages or repository. Group name must refer to architectu Report generation settings. -* `target` - list of reports to be generated, space separated list of strings, required. It must point to valid section (or to section with architecture), e.g. `somerandomname` must point to existing section, `email` must point to one of `email` of `email:x86_64` (with architecture it has higher priority). +* `target` - list of reports to be generated, space separated list of strings, required. It must point to valid section (or to section with architecture), e.g. `somerandomname` must point to existing section, `email` must point to one of `email` of `email:x86_64` (the one with architecture has higher priority). Type will be read from several ways: @@ -152,7 +159,7 @@ Requires `boto3` library to be installed. Section name must be either `s3` (plus ## `web:*` groups -Web server settings. If any of `host`/`port` is not set, web integration will be disabled. Group name must refer to architecture, e.g. it should be `web:x86_64` for x86_64 architecture. This feature requires `aiohttp` libraries to be installed. +Web server settings. If any of `host`/`port` is not set, web integration will be disabled. Group name can refer to architecture, e.g. `web:x86_64` can be used for x86_64 architecture specific settings. This feature requires `aiohttp` libraries to be installed. * `address` - optional address in form `proto://host:port` (`port` can be omitted in case of default `proto` ports), will be used instead of `http://{host}:{port}` in case if set, string, optional. This option is required in case if `OAuth` provider is used. * `debug` - enable debug toolbar, boolean, optional, default `no`. diff --git a/src/ahriman/core/configuration.py b/src/ahriman/core/configuration.py index 874a47f9..6c03c17c 100644 --- a/src/ahriman/core/configuration.py +++ b/src/ahriman/core/configuration.py @@ -24,7 +24,7 @@ import logging from logging.config import fileConfig from pathlib import Path -from typing import Any, Dict, List, Optional, Tuple, Type +from typing import Any, Dict, Generator, List, Optional, Tuple, Type from ahriman.core.exceptions import InitializeException @@ -49,7 +49,7 @@ class Configuration(configparser.RawConfigParser): default constructor. In the most cases must not be called directly """ configparser.RawConfigParser.__init__(self, allow_no_value=True, converters={ - "list": lambda value: value.split(), + "list": self.__convert_list, "path": self.__convert_path, }) self.architecture: Optional[str] = None @@ -84,6 +84,32 @@ class Configuration(configparser.RawConfigParser): config.load_logging(quiet) return config + @staticmethod + def __convert_list(value: str) -> List[str]: + """ + convert string value to list of strings + :param value: string configuration value + :return: list of string from the parsed string + """ + def generator() -> Generator[str, None, None]: + quote_mark = None + word = "" + for char in value: + if char in ("'", "\"") and quote_mark is None: # quoted part started, store quote and do nothing + quote_mark = char + elif char == quote_mark: # quoted part ended, reset quotation + quote_mark = None + elif char == " " and quote_mark is None: # found space outside of the quotation, yield the word + yield word + word = "" + else: # append character to the buffer + word += char + if quote_mark: # there is unmatched quote + raise ValueError(f"unmatched quote in {value}") + yield word # sequence done, return whatever we found + + return [word for word in generator() if word] + @staticmethod def section_name(section: str, suffix: str) -> str: """ @@ -204,6 +230,8 @@ class Configuration(configparser.RawConfigParser): """ if self.path is None or self.architecture is None: raise InitializeException("Configuration path and/or architecture are not set") + for section in self.sections(): # clear current content + self.remove_section(section) self.load(self.path) self.merge_sections(self.architecture) diff --git a/tests/ahriman/core/test_configuration.py b/tests/ahriman/core/test_configuration.py index 1da9e74f..1f7ad7a2 100644 --- a/tests/ahriman/core/test_configuration.py +++ b/tests/ahriman/core/test_configuration.py @@ -4,6 +4,7 @@ import pytest from pathlib import Path from pytest_mock import MockerFixture +from unittest import mock from ahriman.core.configuration import Configuration from ahriman.core.exceptions import InitializeException @@ -54,6 +55,64 @@ def test_section_name(configuration: Configuration) -> None: assert configuration.section_name("build", "x86_64") == "build:x86_64" +def test_getlist(configuration: Configuration) -> None: + """ + must return list of string correctly + """ + configuration.set_option("build", "test_list", "a b c") + assert configuration.getlist("build", "test_list") == ["a", "b", "c"] + + +def test_getlist_empty(configuration: Configuration) -> None: + """ + must return list of string correctly for non-existing option + """ + assert configuration.getlist("build", "test_list", fallback=[]) == [] + configuration.set_option("build", "test_list", "") + assert configuration.getlist("build", "test_list") == [] + + +def test_getlist_single(configuration: Configuration) -> None: + """ + must return list of strings for single string + """ + configuration.set_option("build", "test_list", "a") + assert configuration.getlist("build", "test_list") == ["a"] + assert configuration.getlist("build", "test_list") == ["a"] + + +def test_getlist_with_spaces(configuration: Configuration) -> None: + """ + must return list of string if there is string with spaces in quotes + """ + configuration.set_option("build", "test_list", """"ahriman is" cool""") + assert configuration.getlist("build", "test_list") == ["""ahriman is""", """cool"""] + configuration.set_option("build", "test_list", """'ahriman is' cool""") + assert configuration.getlist("build", "test_list") == ["""ahriman is""", """cool"""] + + +def test_getlist_with_quotes(configuration: Configuration) -> None: + """ + must return list of string if there is string with quote inside quote + """ + configuration.set_option("build", "test_list", """"ahriman is" c"'"ool""") + assert configuration.getlist("build", "test_list") == ["""ahriman is""", """c'ool"""] + configuration.set_option("build", "test_list", """'ahriman is' c'"'ool""") + assert configuration.getlist("build", "test_list") == ["""ahriman is""", """c"ool"""] + + +def test_getlist_unmatched_quote(configuration: Configuration) -> None: + """ + must raise exception on unmatched quote in string value + """ + configuration.set_option("build", "test_list", """ahri"man is cool""") + with pytest.raises(ValueError): + configuration.getlist("build", "test_list") + configuration.set_option("build", "test_list", """ahri'man is cool""") + with pytest.raises(ValueError): + configuration.getlist("build", "test_list") + + def test_getpath_absolute_to_absolute(configuration: Configuration) -> None: """ must not change path for absolute path in settings @@ -94,32 +153,6 @@ def test_getpath_without_fallback(configuration: Configuration) -> None: assert configuration.getpath("build", "option") -def test_getlist(configuration: Configuration) -> None: - """ - must return list of string correctly - """ - configuration.set_option("build", "test_list", "a b c") - assert configuration.getlist("build", "test_list") == ["a", "b", "c"] - - -def test_getlist_empty(configuration: Configuration) -> None: - """ - must return list of string correctly for non-existing option - """ - assert configuration.getlist("build", "test_list", fallback=[]) == [] - configuration.set_option("build", "test_list", "") - assert configuration.getlist("build", "test_list") == [] - - -def test_getlist_single(configuration: Configuration) -> None: - """ - must return list of strings for single string - """ - configuration.set_option("build", "test_list", "a") - assert configuration.getlist("build", "test_list") == ["a"] - assert configuration.getlist("build", "test_list") == ["a"] - - def test_gettype(configuration: Configuration) -> None: """ must extract type from variable @@ -222,6 +255,17 @@ def test_reload(configuration: Configuration, mocker: MockerFixture) -> None: merge_mock.assert_called_once() +def test_reload_clear(configuration: Configuration, mocker: MockerFixture) -> None: + """ + must clear current settings before configuration reload + """ + clear_mock = mocker.patch("ahriman.core.configuration.Configuration.remove_section") + sections = configuration.sections() + + configuration.reload() + clear_mock.assert_has_calls([mock.call(section) for section in sections]) + + def test_reload_no_architecture(configuration: Configuration) -> None: """ must raise exception on reload if no architecture set