From f4aa29430bced1e87282e9d8c4911b0839840199 Mon Sep 17 00:00:00 2001 From: Siddardh Ramesh <30310381+siddardh-ra@users.noreply.github.com> Date: Fri, 5 Aug 2022 21:24:40 +0530 Subject: [PATCH 1/4] Fixed API to pass resource_id rather than dataset's name (#2959) PBENCH-848 Fixed the bug in Inventory API - Passing resource_id instead of dataset name Co-authored-by: siddardh --- lib/pbench/server/api/resources/datasets_inventory.py | 2 +- lib/pbench/test/unit/server/test_datasets_inventory.py | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/pbench/server/api/resources/datasets_inventory.py b/lib/pbench/server/api/resources/datasets_inventory.py index 0a37612d94..c61cd392fa 100644 --- a/lib/pbench/server/api/resources/datasets_inventory.py +++ b/lib/pbench/server/api/resources/datasets_inventory.py @@ -59,7 +59,7 @@ def _get(self, params: ApiParams, _) -> Response: file_tree = FileTree(self.config, self.logger) try: - tarball = file_tree.find_dataset(dataset.name) + tarball = file_tree.find_dataset(dataset.resource_id) except TarballNotFound as e: raise APIAbort(HTTPStatus.NOT_FOUND, str(e)) dataset_location = tarball.unpacked diff --git a/lib/pbench/test/unit/server/test_datasets_inventory.py b/lib/pbench/test/unit/server/test_datasets_inventory.py index 68b51734bc..3df9a4573c 100644 --- a/lib/pbench/test/unit/server/test_datasets_inventory.py +++ b/lib/pbench/test/unit/server/test_datasets_inventory.py @@ -44,6 +44,8 @@ def mock_find_dataset(self, dataset): class Tarball(object): unpacked = Path("/dataset1/") + # Validate the resource_id + Dataset.query(resource_id=dataset) return Tarball def test_get_no_dataset(self, query_get_as): @@ -55,7 +57,7 @@ def test_get_no_dataset(self, query_get_as): def test_dataset_not_present(self, query_get_as): response = query_get_as("fio_2", "metadata.log", HTTPStatus.NOT_FOUND) assert response.json == { - "message": "The dataset tarball named 'fio_2' is not present in the file tree" + "message": "The dataset tarball named 'random_md5_string4' is not present in the file tree" } def test_unauthorized_access(self, query_get_as): @@ -69,6 +71,8 @@ def mock_find_not_unpacked(self, dataset): class Tarball(object): unpacked = None + # Validate the resource_id + Dataset.query(resource_id=dataset) return Tarball monkeypatch.setattr(FileTree, "find_dataset", mock_find_not_unpacked) From 81072773abcb361ff1ab6450af2e546b16012d60 Mon Sep 17 00:00:00 2001 From: Webb Scales <7795764+webbnh@users.noreply.github.com> Date: Fri, 5 Aug 2022 14:34:21 -0400 Subject: [PATCH 2/4] Initial support for building replacements for COPR chroots (#2957) This commit extends the jenkins/Makefile to allow it to build containers in which we can build RPMs for the Server and Agent for all platforms on which we intend to deploy. This will allow us to build RPMs locally as part of the CI pipeline, instead of using COPR. PBENCH-720 --- jenkins/Makefile | 79 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 77 insertions(+), 2 deletions(-) diff --git a/jenkins/Makefile b/jenkins/Makefile index 87834460a1..c0eb12aca5 100644 --- a/jenkins/Makefile +++ b/jenkins/Makefile @@ -1,13 +1,77 @@ +# # A simple Makefile for building and pushing the various pbench-devel-* -# container images. +# and pbench-rpmbuild container images. +# +# This makefile defines the following targets: # -# WARNING: Do not publish images layered on top of UBI images. +# all (the default): builds the CI image and a Fedora-based development image +# image-ci: builds the CI image +# image-fedora: builds the Fedora-based development image +# image-rpmbuild-all: builds all the default containers for building RPMs +# image-rpmbuild-rhel-7: builds a single RPM build container (e.g., for RHEL-7) +# push-ci: pushes the CI image to the registry +# push-fedora: pushes the Fedora-based development image to the registry +# push-rpmbuild-all: pushes all the default RPM build containers to the registry +# + +# Default list of distributions for which to build containers in which to build RPMs +RPMBUILD_BASEIMAGE_DEFAULTS = \ + rhel-9 rhel-8 rhel-7 \ + fedora-36 fedora-35 fedora-34 \ + centos-9 centos-8 + +# Registry where the resulting RPM build container images are to be stored +RPMBUILD_IMAGE_REPO = images.paas.redhat.com/pbench + +# Templates for the various distributions' base images. For the moment, for a +# given distribution, all of the versions can be found by tweaking the image +# tag; so, to generate the appropriate repository for the base image, we just +# have to pick the appropriate template and insert the requested version, which +# is exactly what happens when RPMBUILD_BASEIMAGE_REPO is expanded. # +# Note that registry.access.redhat.com is moving to registry.redhat.io where it +# will no longer permit anonymous access, but it works for the moment. +VER := __version__ +RPMBUILD_BASEIMAGE_rhel = registry.access.redhat.com/ubi${VER}:latest +RPMBUILD_BASEIMAGE_fedora = quay.io/fedora/fedora:${VER} +RPMBUILD_BASEIMAGE_centos = quay.io/centos/centos:stream${VER} +RPMBUILD_BASEIMAGE_REPO = $(subst ${VER},${DIST_VERSION},${RPMBUILD_BASEIMAGE_${DIST_NAME}}) + +# List of packages required to build RPMs, to be installed in the build +# containers. (The jinja2 CLI is not available as an RPM on RHEL, so we +# install it via pip for all platforms, to keep things simple.) +RPMBUILD_PACKAGES = git make python3-jinja2 python3-pip rpmlint rpm-build + +# Default package manager tool (may be overridden for some distros) +PKGMGR = dnf BRANCH := $(shell cat ./branch.name) +# Compose lists of potential distro-version strings with versions 0-99 +# (e.g., `_ALL_centos_VERSIONS` including "centos-7", "centos-8", "centos-9", +# `_ALL_fedora_VERSIONS` including "fedora-32", "fedora-33", and "fedora-34", +# and `_DISTROS` concatenating the contents of all the lists) which will be +# used to drive pattern matching on distro-based target rules. +_DIGITS := 0 1 2 3 4 5 6 7 8 9 +_NUMBERS := $(patsubst 0%,%,$(foreach tens,${_DIGITS},$(foreach ones,${_DIGITS},${tens}${ones}))) +_ALL_DISTRO_NAMES := centos fedora rhel +_ADV_TMPL = _ALL_${d}_VERSIONS := $(foreach v,${_NUMBERS},${d}-${v}) # template for setting version lists +$(foreach d,${_ALL_DISTRO_NAMES},$(eval $(call _ADV_TMPL, ${d}))) # set _ALL_centos_VERSIONS, etc. +_DISTROS := $(foreach d,${_ALL_DISTRO_NAMES},${_ALL_${d}_VERSIONS}) + +# For any given target, extract the distribution name and version from the last +# two fields, e.g., image-rpmbuild-rhel-7 would yield values "rhel" and "7" for +# DIST_NAME and DIST_VERSION, respectively. +%: DIST_VERSION = $(lastword $(subst -, ,$*)) +%: DIST_NAME = $(lastword $(subst ${DIST_VERSION},,$(subst -, ,$*))) + + all: image-ci image-fedora +push-rpmbuild-all: $(RPMBUILD_BASEIMAGE_DEFAULTS:%=push-rpmbuild-%) + +image-rpmbuild-all: $(RPMBUILD_BASEIMAGE_DEFAULTS:%=image-rpmbuild-%) + push-ci: buildah push localhost/pbench-ci-fedora:${BRANCH} quay.io/pbench/pbench-ci-fedora:${BRANCH} @@ -20,6 +84,17 @@ push-fedora: image-fedora: development.fedora.Dockerfile buildah bud -f development.fedora.Dockerfile -t localhost/pbench-devel-fedora:${BRANCH} +${_DISTROS:%=push-rpmbuild-%}: push-rpmbuild-%: + buildah push localhost/pbench-rpmbuild:${*} ${RPMBUILD_IMAGE_REPO}/pbench-rpmbuild:${*} + +image-rpmbuild-rhel-7 : PKGMGR = yum +${_DISTROS:%=image-rpmbuild-%}: image-rpmbuild-%: + container=$$(buildah from ${RPMBUILD_BASEIMAGE_REPO}) && \ + buildah run $$container ${PKGMGR} install -y ${RPMBUILD_PACKAGES} && \ + buildah run $$container ${PKGMGR} clean all && \ + buildah run $$container python3 -m pip install jinja2-cli && \ + buildah commit $$container localhost/pbench-rpmbuild:${*} + image-ubi: development.ubi.Dockerfile buildah bud -f development.ubi.Dockerfile -t localhost/pbench-devel-ubi:${BRANCH} From a7814d39fb35f1d90b86a910a2edeb8945de55cb Mon Sep 17 00:00:00 2001 From: David Butenhof Date: Fri, 5 Aug 2022 16:56:50 -0400 Subject: [PATCH 3/4] Pbench Server client library and functional tests (#2941) * Pbench Server client library and functional tests PBENCH-810 This adds the beginnings of a simple Pbench Server python client library (with a `PbenchServerClient` class) and a few initial functional test modules. The modules are written in Python using the Pytest framework to give us convenient test modularization and fixtures. We use a fixture to establish a Pbench Server connection, for example. The functional test modules currently include a simple validation that we can connect to a server and retrieve the server's endpoints, and a test of Pbench Server user operations including register, login, logout, profile access/update, and delete. The `PBENCH_SERVER` environment variable provides the server host for connection; the `pbench_server_client` fixture will cause failure if the environment variable isn't defined, or if connection fails. --- exec-unittests | 21 +- lib/pbench/client/__init__.py | 311 ++++++++++++++++++ lib/pbench/test/functional/server/conftest.py | 23 ++ .../test/functional/server/test_connect.py | 44 +++ .../test/functional/server/test_user.py | 213 ++++++++++++ lib/pbench/test/unit/client/__init__.py | 1 + lib/pbench/test/unit/client/conftest.py | 24 ++ lib/pbench/test/unit/client/test_connect.py | 53 +++ lib/pbench/test/unit/client/test_login.py | 49 +++ 9 files changed, 731 insertions(+), 8 deletions(-) create mode 100644 lib/pbench/client/__init__.py create mode 100644 lib/pbench/test/functional/server/conftest.py create mode 100644 lib/pbench/test/functional/server/test_connect.py create mode 100644 lib/pbench/test/functional/server/test_user.py create mode 100644 lib/pbench/test/unit/client/__init__.py create mode 100644 lib/pbench/test/unit/client/conftest.py create mode 100644 lib/pbench/test/unit/client/test_connect.py create mode 100644 lib/pbench/test/unit/client/test_login.py diff --git a/exec-unittests b/exec-unittests index 786eb68b71..8ea026bc3d 100755 --- a/exec-unittests +++ b/exec-unittests @@ -12,17 +12,17 @@ fi shift # The first argument will be which major sub-system to run tests for: the -# agent side or the server side of the source tree. +# agent code, the functional test client code, or the server code. major="${1}" shift -major_list="agent server" +major_list="agent client server" if [[ -n "${major}" ]]; then - if [[ "${major}" != "agent" && "${major}" != "server" && "${major}" != "both" ]]; then - printf -- "Expected major sub-system to be either 'agent', 'server', or 'both', got '%s'\n" "${major}" >&2 + if [[ "${major}" != "agent" && "${major}" != "server" && "${major}" != "client" && "${major}" != "all" ]]; then + printf -- "Expected major sub-system to be 'agent', 'client', 'server', or 'all', got '%s'\n" "${major}" >&2 exit 2 fi - if [[ "${major}" != "both" ]]; then + if [[ "${major}" != "all" ]]; then major_list="${major}" fi fi @@ -131,7 +131,10 @@ if [[ -z "${subtst}" || "${subtst}" == "python" ]]; then for _major in ${major_list}; do _pytest_majors="${_pytest_majors} pbench.test.unit.${_major}" if [[ "${_major}" == "agent" ]]; then - _pytest_majors="${_pytest_majors} pbench.test.functional" + # TODO: real functional tests require a deployed instance. Current + # agent "functional" tests are mocked Click tests rather than true + # "functional" tests. + _pytest_majors="${_pytest_majors} pbench.test.functional.agent" fi done @@ -162,7 +165,9 @@ trap "rm -f ${_para_jobs_file}" EXIT INT TERM let count=0 for _major in ${major_list}; do # Verify the Agent or Server Makefile functions correctly. - verify_make_source_tree ${_major} || rc=1 + if [[ "${_major}" == "agent" || "${_major}" == "server" ]]; then + verify_make_source_tree ${_major} || rc=1 + fi if [[ "${_major}" == "agent" ]]; then # The parallel program is really cool. The usage of `parallel` is @@ -186,7 +191,7 @@ for _major in ${major_list}; do (( count++ )) run_legacy ${_major} bin ${posargs} || rc=1 fi - else + elif [[ "${_major}" != "client" ]]; then printf -- "Error - unrecognized major test sub-set, '%s'\n" "${_major}" >&2 rc=1 fi diff --git a/lib/pbench/client/__init__.py b/lib/pbench/client/__init__.py new file mode 100644 index 0000000000..b863300fc4 --- /dev/null +++ b/lib/pbench/client/__init__.py @@ -0,0 +1,311 @@ +from typing import Dict, List, Optional, Union +from urllib import parse + +import requests +from requests.structures import CaseInsensitiveDict + +# A type defined to conform to the semantic definition of a JSON structure +# with Python syntax. +JSONSTRING = str +JSONNUMBER = Union[int, float] +JSONVALUE = Union["JSONOBJECT", "JSONARRAY", JSONSTRING, JSONNUMBER, bool, None] +JSONARRAY = List[JSONVALUE] +JSONOBJECT = Dict[JSONSTRING, JSONVALUE] +JSON = JSONVALUE + + +class PbenchClientError(Exception): + """ + Base class for exceptions reported by the Pbench Server client. + """ + + pass + + +class IncorrectParameterCount(PbenchClientError): + def __init__(self, api: str, cnt: int, uri_params: Optional[JSONOBJECT]): + self.api = api + self.cnt = cnt + self.uri_params = uri_params + + def __str__(self) -> str: + return f"API template {self.api} requires {self.cnt} parameters ({self.uri_params})" + + +class PbenchServerClient: + DEFAULT_SCHEME = "http" + + def __init__(self, host: str): + """ + Create a Pbench Server client object. + + The connect method should be called to establish a connection and set + up the endpoints map before using any other methods. + + Args: + host: Pbench Server hostname + """ + self.host: str = host + url_parts = parse.urlsplit(host) + if url_parts.scheme: + self.scheme = url_parts.scheme + url = self.host + else: + self.scheme = self.DEFAULT_SCHEME + url = f"{self.scheme}://{self.host}" + self.url = url + self.username: Optional[str] = None + self.auth_token: Optional[str] = None + self.session: Optional[requests.Session] = None + self.endpoints: Optional[JSONOBJECT] = None + + def _headers( + self, user_headers: Optional[Dict[str, str]] = None + ) -> CaseInsensitiveDict: + """ + Create an HTTP request headers dictionary. + + The connect method can set default HTTP headers which apply to all + server calls. This method implicitly adds an authentication token + if the client has logged in and also allows the caller to override + that or any other default session header. (For example, to change the + default "accept" datatype, or to force an invalid authentication token + for testing.) + + Args: + user_headers: Addition request headers + + Returns: + Case-insensitive header dictionary + """ + headers = CaseInsensitiveDict() + if self.auth_token: + headers["authorization"] = f"Bearer {self.auth_token}" + if user_headers: + headers.update(user_headers) + return headers + + def _uri(self, api: str, uri_params: Optional[JSONOBJECT] = None) -> str: + """ + Compute the Pbench Server URI for an operation. This uses the endpoints + definition stored by connect. If parameters are given, then it will use + the template "uri" object to find the named URI template and format it + with the provided parameter values. + + Args: + api: The name of the API + uri_params: A dictionary of named parameter values for the template + + Returns: + A fully specified URI + """ + if not uri_params: + return self.endpoints["api"][api] + else: + description = self.endpoints["uri"][api] + template = description["template"] + cnt = len(description["params"]) + if cnt != len(uri_params): + raise IncorrectParameterCount(api, cnt, uri_params) + return template.format(**uri_params) + + def get( + self, + api: str, + uri_params: Optional[JSONOBJECT] = None, + *, + headers: Optional[Dict[str, str]] = None, + **kwargs, + ) -> requests.Response: + """ + Issue a get HTTP operation through the cached session, constructing a + URI from the "api" name and parameters, adding or overwriting HTTP + request headers as specified. + + HTTP errors are raised by exception to ensure they're not overlooked. + + Args: + api: The name of the Pbench Server API + uri_params: A dictionary of named parameters to expand a URI template + headers: A dictionary of header/value pairs + kwargs: Additional `requests` parameters (e.g., params) + + Returns: + An HTTP Response object + """ + url = self._uri(api, uri_params) + response = self.session.get(url, headers=self._headers(headers), **kwargs) + response.raise_for_status() + return response + + def head( + self, + api: str, + uri_params: Optional[JSONOBJECT] = None, + *, + headers: Optional[Dict[str, str]] = None, + **kwargs, + ) -> requests.Response: + """ + Issue a head HTTP operation through the cached session, constructing a + URI from the "api" name and parameters, adding or overwriting HTTP + request headers as specified. `head` is a GET that returns only status + and headers, without a response payload. + + HTTP errors are raised by exception to ensure they're not overlooked. + + Args: + api: The name of the Pbench Server API + uri_params: A dictionary of named parameters to expand a URI template + headers: A dictionary of header/value pairs + kwargs: Additional `requests` parameters (e.g., params) + + Returns: + An HTTP Response object + """ + url = self._uri(api, uri_params) + response = self.session.head(url, headers=self._headers(headers), **kwargs) + response.raise_for_status() + return response + + def put( + self, + api: str, + uri_params: Optional[JSONOBJECT] = None, + *, + json: Optional[Dict[str, str]] = None, + headers: Optional[Dict[str, str]] = None, + **kwargs, + ) -> requests.Response: + """ + Issue a put HTTP operation through the cached session, constructing a + URI from the "api" name and parameters, adding or overwriting HTTP + request headers as specified, and optionally passing a JSON request + payload. + + HTTP errors are raised by exception to ensure they're not overlooked. + + Args: + api: The name of the Pbench Server API + uri_params: A dictionary of named parameters to expand a URI template + json: A JSON request payload as a Python dictionary + headers: A dictionary of header/value pairs + kwargs: Additional `requests` parameters (e.g., params) + + Returns: + An HTTP Response object + """ + url = self._uri(api, uri_params) + response = self.session.put( + url, json=json, headers=self._headers(headers), **kwargs + ) + response.raise_for_status() + return response + + def post( + self, + api: str, + uri_params: Optional[JSONOBJECT] = None, + *, + json: Optional[Dict[str, str]] = None, + headers: Optional[Dict[str, str]] = None, + **kwargs, + ) -> requests.Response: + """ + Issue a post HTTP operation through the cached session, constructing a + URI from the "api" name and parameters, adding or overwriting HTTP + request headers as specified, and optionally passing a JSON request + payload. + + HTTP errors are raised by exception to ensure they're not overlooked. + + Args: + api: The name of the Pbench Server API + uri_params: A dictionary of named parameters to expand a URI template + json: A JSON request payload as a Python dictionary + headers: A dictionary of header/value pairs + kwargs: Additional `requests` parameters (e.g., params) + + Returns: + An HTTP Response object + """ + url = self._uri(api, uri_params) + response = self.session.post( + url, json=json, headers=self._headers(headers), **kwargs + ) + response.raise_for_status() + return response + + def delete( + self, + api: str, + uri_params: Optional[JSONOBJECT] = None, + *, + headers: Optional[Dict[str, str]] = None, + **kwargs, + ) -> requests.Response: + """ + Issue a delete HTTP operation through the cached session, constructing + a URI from the "api" name and parameters, adding or overwriting HTTP + request headers as specified. + + HTTP errors are raised by exception to ensure they're not overlooked. + + Args: + api: The name of the Pbench Server API + uri_params: A dictionary of named parameters to expand a URI template + headers: A dictionary of header/value pairs + kwargs: Additional `requests` parameters (e.g., params) + + Returns: + An HTTP Response object + """ + url = self._uri(api, uri_params) + response = self.session.delete(url, headers=self._headers(headers), **kwargs) + response.raise_for_status() + return response + + def connect(self, headers: Optional[Dict[str, str]] = None) -> None: + """ + Connect to the Pbench Server host using the endpoints API to be sure + that it responds, and cache the endpoints response payload. + + This also allows the client to add default HTTP headers to the session + which will be used for all operations unless overridden for specific + operations. + + Args: + headers: A dict of default HTTP headers + """ + url = parse.urljoin(self.url, "api/v1/endpoints") + self.session = requests.Session() + if headers: + self.session.headers.update(headers) + response = self.session.get(url) + response.raise_for_status() + self.endpoints = response.json() + assert self.endpoints + + def login(self, user: str, password: str) -> None: + """ + Login to a specified username with the password, and store the + resulting authentication token. + + Args: + user: Account username + password: Account password + """ + response = self.post("login", json={"username": user, "password": password}) + response.raise_for_status() + json = response.json() + self.username = json["username"] + self.auth_token = json["auth_token"] + + def logout(self) -> None: + """ + Logout the currently authenticated user and remove the authentication + token. + """ + self.post("logout") + self.username = None + self.auth_token = None diff --git a/lib/pbench/test/functional/server/conftest.py b/lib/pbench/test/functional/server/conftest.py new file mode 100644 index 0000000000..19257c84f4 --- /dev/null +++ b/lib/pbench/test/functional/server/conftest.py @@ -0,0 +1,23 @@ +import os + +import pytest + +from pbench.client import PbenchServerClient + + +@pytest.fixture(scope="module") +def pbench_server_client(): + """ + Used by Pbench Server functional tests to connect to a server. + + If run without a PBENCH_SERVER environment variable pointing to the server + instance, this will fail the test run. + """ + host = os.environ.get("PBENCH_SERVER") + assert ( + host + ), "Pbench Server functional tests require that PBENCH_SERVER be set to the hostname of a server" + pbench_client = PbenchServerClient(host) + pbench_client.connect({"accept": "application/json"}) + assert pbench_client.endpoints + return pbench_client diff --git a/lib/pbench/test/functional/server/test_connect.py b/lib/pbench/test/functional/server/test_connect.py new file mode 100644 index 0000000000..8cb80c8a54 --- /dev/null +++ b/lib/pbench/test/functional/server/test_connect.py @@ -0,0 +1,44 @@ +from pbench.client import PbenchServerClient + + +class TestConnect: + + EXPECTED = ( + "datasets_contents", + "datasets_daterange", + "datasets_delete", + "datasets_detail", + "datasets_inventory", + "datasets_list", + "datasets_mappings", + "datasets_metadata", + "datasets_namespace", + "datasets_publish", + "datasets_search", + "datasets_values", + "elasticsearch", + "endpoints", + "graphql", + "login", + "logout", + "register", + "upload", + "user", + ) + + def test_connect(self, pbench_server_client: PbenchServerClient): + """ + Verify that we can retrieve the Pbench Server endpoints through the + client "connect" API, and that the expected APIs are described. + """ + assert pbench_server_client.session + assert pbench_server_client.session.headers["Accept"] == "application/json" + endpoints = pbench_server_client.endpoints + assert endpoints + assert "api" in endpoints + assert "identification" in endpoints + assert "uri" in endpoints + for a in endpoints["api"].keys(): + assert a in self.EXPECTED + for a in endpoints["uri"].keys(): + assert a in self.EXPECTED diff --git a/lib/pbench/test/functional/server/test_user.py b/lib/pbench/test/functional/server/test_user.py new file mode 100644 index 0000000000..c69d3f8a8a --- /dev/null +++ b/lib/pbench/test/functional/server/test_user.py @@ -0,0 +1,213 @@ +from http import HTTPStatus + +import pytest +from requests import HTTPError + +from pbench.client import PbenchServerClient + + +USERNAME1 = "functional" +USERNAME2 = "nonfunctional" +PASSWORD = "tests" + + +class TestUser: + """ + Pbench Server functional tests for basic "user" operations. + + NOTE: The functional test system is intended to run on a pristine + containerized Pbench Server. We could check for some pre-conditions if we + had a known Administrative user (e.g., admin/admin). + """ + + def test_login_fail(self, pbench_server_client: PbenchServerClient): + """ + Trying to log in to a non-existent user should fail. + + NOTE: This will fail if "nouser" exists. + """ + with pytest.raises( + HTTPError, + match=f"UNAUTHORIZED for url: {pbench_server_client.scheme}://.*?/api/v1/login", + ): + pbench_server_client.login("nouser", "nopassword") + + @pytest.mark.parametrize( + "json", + [ + { + "username": USERNAME1, + "first_name": "Test", + "last_name": "Account", + "password": PASSWORD, + "email": f"{USERNAME1}@example.com", + }, + { + "username": USERNAME2, + "first_name": "Tester", + "last_name": "Accountant", + "password": PASSWORD, + "email": f"{USERNAME2}@example.com", + }, + ], + ) + def test_register(self, pbench_server_client: PbenchServerClient, json): + """ + Test that we can register a new user. + + NOTE: This will fail if the username already exists. + """ + response = pbench_server_client.post("register", json=json) + assert response.status_code == HTTPStatus.CREATED + + def test_register_redux(self, pbench_server_client: PbenchServerClient): + """ + Test that an attempt to register an existing user fails. + """ + json = { + "username": USERNAME1, + "first_name": "Repeat", + "last_name": "Redux", + "password": PASSWORD, + "email": f"{USERNAME1}@example.com", + } + with pytest.raises( + HTTPError, + match=f"FORBIDDEN for url: {pbench_server_client.scheme}://.*?/api/v1/register", + ): + pbench_server_client.post("register", json=json) + + def test_profile_noauth(self, pbench_server_client: PbenchServerClient): + """ + Test that we can't access a user profile without authentication. + """ + with pytest.raises( + HTTPError, + match=f"UNAUTHORIZED for url: {pbench_server_client.scheme}://.*?/api/v1/user/{USERNAME1}", + ): + pbench_server_client.get("user", {"target_username": USERNAME1}) + + def test_login(self, pbench_server_client: PbenchServerClient): + """ + Test that we can log in using our new user. + + NOTE: This assumes test cases will be run in order. There are Pytest + plugins to explicitly control test case order, but Pytest generally + does run tests within a single class in order. Is it worth taking on + another dependency to make this explicit? + """ + pbench_server_client.login(USERNAME1, PASSWORD) + assert pbench_server_client.auth_token + + def test_profile_bad_auth(self, pbench_server_client: PbenchServerClient): + """ + Test that we can't access a user profile with an invalid authentication + token. + """ + with pytest.raises( + HTTPError, + match=f"UNAUTHORIZED for url: {pbench_server_client.scheme}://.*?/api/v1/user/{USERNAME1}", + ): + pbench_server_client.get( + "user", + {"target_username": USERNAME1}, + headers={"Authorization": "Bearer of bad tokens"}, + ) + + def test_profile_not_self(self, pbench_server_client: PbenchServerClient): + """ + Test that we can't access another user's profile. + """ + with pytest.raises( + HTTPError, + match=f"FORBIDDEN for url: {pbench_server_client.scheme}://.*?/api/v1/user/{USERNAME2}", + ): + pbench_server_client.get("user", {"target_username": USERNAME2}) + + def test_profile(self, pbench_server_client: PbenchServerClient): + """ + Test that we can retrieve our own user profile once logged in. + + NOTE: This assumes test cases will be run in order. + """ + response = pbench_server_client.get("user", {"target_username": USERNAME1}) + assert response.json()["username"] == USERNAME1 + + def test_update(self, pbench_server_client: PbenchServerClient): + """ + Test that we can update our own user profile once logged in. + + NOTE: This assumes test cases will be run in order. + """ + response = pbench_server_client.get("user", {"target_username": USERNAME1}) + user = response.json() + assert user["first_name"] != "Mycroft" + user["first_name"] = "Mycroft" + + # Remove fields that PUT will reject, but use a copy so that we can + # compare the new value against what we expect. + # + # This is unfortunate: PUT should IGNORE unmodifiable fields to allow a + # standard REST GET/update/PUT sequence. + payload = user.copy() + del payload["registered_on"] + + response = pbench_server_client.put( + "user", {"target_username": USERNAME1}, json=payload + ) + put_response = response.json() + + response = pbench_server_client.get("user", {"target_username": USERNAME1}) + updated = response.json() + assert user == updated + assert put_response == user + + def test_delete_other(self, pbench_server_client: PbenchServerClient): + """ + Test that we can't delete another user. + + NOTE: This assumes test cases will be run in order. + """ + with pytest.raises( + HTTPError, + match=f"FORBIDDEN for url: {pbench_server_client.scheme}://.*?/api/v1/user/{USERNAME2}", + ): + pbench_server_client.delete("user", {"target_username": USERNAME2}) + + def test_delete_nologin(self, pbench_server_client: PbenchServerClient): + """ + Test that we can't delete a user profile when not logged in. + + NOTE: This assumes test cases will be run in order. + """ + pbench_server_client.logout() + with pytest.raises( + HTTPError, + match=f"UNAUTHORIZED for url: {pbench_server_client.scheme}://.*?/api/v1/user/{USERNAME1}", + ): + pbench_server_client.delete("user", {"target_username": USERNAME1}) + + def test_delete(self, pbench_server_client: PbenchServerClient): + """ + Test that we can delete our own user profile once logged in. + + NOTE: This assumes test cases will be run in order. + """ + pbench_server_client.login(USERNAME1, PASSWORD) + pbench_server_client.delete("user", {"target_username": USERNAME1}) + + def test_logout(self, pbench_server_client: PbenchServerClient): + """ + Test that we can log out from our session. + + NOTE: This assumes test cases will be run in order. + """ + pbench_server_client.logout() + assert pbench_server_client.auth_token is None + + def test_cleanup_user2(self, pbench_server_client: PbenchServerClient): + """ + Remove the second test user to make the test idempotent. + """ + pbench_server_client.login(USERNAME2, PASSWORD) + pbench_server_client.delete("user", {"target_username": USERNAME2}) diff --git a/lib/pbench/test/unit/client/__init__.py b/lib/pbench/test/unit/client/__init__.py new file mode 100644 index 0000000000..de32d41a56 --- /dev/null +++ b/lib/pbench/test/unit/client/__init__.py @@ -0,0 +1 @@ +# Required by pytest to recognize the "client" module diff --git a/lib/pbench/test/unit/client/conftest.py b/lib/pbench/test/unit/client/conftest.py new file mode 100644 index 0000000000..9496cd9302 --- /dev/null +++ b/lib/pbench/test/unit/client/conftest.py @@ -0,0 +1,24 @@ +import pytest +import responses + +from pbench.client import PbenchServerClient + + +@pytest.fixture() +@responses.activate +def connect(): + """ + Fake a connection to the Pbench Server API. + """ + pbench = PbenchServerClient("localhost") + responses.add( + responses.GET, + f"{pbench.url}/api/v1/endpoints", + json={ + "identification": "string", + "api": {"login": f"{pbench.url}/api/v1/login"}, + "uri": {}, + }, + ) + pbench.connect({"accept": "application/json"}) + return pbench diff --git a/lib/pbench/test/unit/client/test_connect.py b/lib/pbench/test/unit/client/test_connect.py new file mode 100644 index 0000000000..36e018deb5 --- /dev/null +++ b/lib/pbench/test/unit/client/test_connect.py @@ -0,0 +1,53 @@ +import responses + +from pbench.client import PbenchServerClient + + +class TestConnect: + def test_construct_host(self): + pbench = PbenchServerClient("10.1.100.2") + assert pbench.host == "10.1.100.2" + assert pbench.scheme == "http" + assert pbench.session is None + assert pbench.endpoints is None + + def test_construct_url(self): + pbench = PbenchServerClient("https://10.1.100.2") + assert pbench.host == "https://10.1.100.2" + assert pbench.scheme == "https" + assert pbench.session is None + assert pbench.endpoints is None + + @responses.activate + def test_connect(self): + pbench = PbenchServerClient("10.1.100.2") + url = f"{pbench.url}/api/v1/endpoints" + + with responses.RequestsMock() as rsp: + rsp.add( + responses.GET, + url, + json={"identification": "string", "api": {}, "uri": {}}, + ) + pbench.connect({"accept": "application/json"}) + assert len(rsp.calls) == 1 + assert rsp.calls[0].request.url == url + assert rsp.calls[0].response.status_code == 200 + + # Check standard requests headers. The requests package uses a + # case-insensitive dictionary for headers, which retains the original + # case but provides case-insensitive lookups. + headers = pbench.session.headers + assert "Accept-Encoding" in headers + assert "Connection" in headers + assert "user-agent" in headers + assert "accept" in headers # Our header was added + assert "AcCePt" in headers # It's case insensitive + assert headers["ACCEPT"] == "application/json" + + # Check that the fake endpoints we returned are captured + endpoints = pbench.endpoints + assert endpoints + assert endpoints["api"] == {} + assert endpoints["identification"] == "string" + assert endpoints["uri"] == {} diff --git a/lib/pbench/test/unit/client/test_login.py b/lib/pbench/test/unit/client/test_login.py new file mode 100644 index 0000000000..b0c0964cc0 --- /dev/null +++ b/lib/pbench/test/unit/client/test_login.py @@ -0,0 +1,49 @@ +from http import HTTPStatus +import pytest +import requests +import responses + + +class TestLogin: + def test_login(self, connect): + """ + Confirm that a successful Pbench Server login captures the 'username' + and 'auth_token' in the client object. + """ + url = f"{connect.url}/api/v1/login" + with responses.RequestsMock() as rsp: + rsp.add( + responses.POST, url, json={"username": "user", "auth_token": "foobar"} + ) + connect.login("user", "password") + assert len(rsp.calls) == 1 + assert rsp.calls[0].request.url == url + assert rsp.calls[0].response.status_code == 200 + + assert connect.username == "user" + assert connect.auth_token == "foobar" + + def test_bad_login(self, connect): + """ + Confirm that a failure from the Pbench Server login API is correctly + handled by the client library, and does not set the client 'username` + and 'auth_token' properties. + """ + url = f"{connect.url}/api/v1/login" + with responses.RequestsMock() as rsp: + rsp.add( + responses.POST, + url, + status=HTTPStatus.UNAUTHORIZED, + json={"username": "user", "auth_token": "foobar"}, + ) + + with pytest.raises(requests.HTTPError): + connect.login("user", "password") + + assert len(rsp.calls) == 1 + assert rsp.calls[0].request.url == url + assert rsp.calls[0].response.status_code == 401 + + assert connect.username is None + assert connect.auth_token is None From c9a9d467a27bc93430874b8d45aadbbf6b0e2122 Mon Sep 17 00:00:00 2001 From: David Butenhof Date: Mon, 8 Aug 2022 07:13:37 -0400 Subject: [PATCH 4/4] Provide a server configuration option mechanism (#2949) * Provide a server configuration option mechanism PBENCH-759 Add a key/value store for server-wide configuration settings. To start, we implement three settings: the maximum dataset retention in days, the current server state, and a "banner" object that can be displayed by clients to inform users of issues, maintenance plans, etc. The retention period default is taken from the `pbench-server.cfg` file. The default server state is "enabled". There is no default banner. This PR also introduces a new pattern towards pure unit testing of the SQLAlchemy database layer by mocking the engine calls through the session to mock `add`, `commit`, and `rollback`, eliminating the use of `sqlite3`. --- docs/API/V1/README.md | 8 + docs/API/V1/contents.md | 6 + docs/API/V1/inventory.md | 6 + docs/API/V1/list.md | 6 + docs/API/V1/server_config.md | 268 ++++++++++ lib/pbench/server/__init__.py | 5 +- lib/pbench/server/api/__init__.py | 9 + lib/pbench/server/api/resources/__init__.py | 116 +++-- .../server/api/resources/datasets_metadata.py | 10 +- .../api/resources/server_configuration.py | 231 +++++++++ lib/pbench/server/api/resources/upload_api.py | 4 + lib/pbench/server/api/resources/users_api.py | 17 + lib/pbench/server/database/database.py | 1 + lib/pbench/server/database/models/datasets.py | 9 +- .../server/database/models/server_config.py | 389 ++++++++++++++ .../server/database/test_server_config_db.py | 473 ++++++++++++++++++ .../test/unit/server/test_authorization.py | 63 ++- .../unit/server/test_endpoint_configure.py | 5 + .../unit/server/test_server_configuration.py | 250 +++++++++ 19 files changed, 1837 insertions(+), 39 deletions(-) create mode 100644 docs/API/V1/server_config.md create mode 100644 lib/pbench/server/api/resources/server_configuration.py create mode 100644 lib/pbench/server/database/models/server_config.py create mode 100644 lib/pbench/test/unit/server/database/test_server_config_db.py create mode 100644 lib/pbench/test/unit/server/test_server_configuration.py diff --git a/docs/API/V1/README.md b/docs/API/V1/README.md index 0157c4add4..2ac60f874d 100644 --- a/docs/API/V1/README.md +++ b/docs/API/V1/README.md @@ -8,6 +8,14 @@ Once you know the hostname of a Pbench Server, you can ask for the API configuration using the [endpoints](endpoints.md) API. This will report the server's version and a list of all API end points supported by the server. +## Pbench Server configuration settings + +Some aspects of Pbench Server operation can be controlled by a user holding the +`ADMIN` [role](../access_model.md) through the +[server configuration](./server_config.md) API, including disabling the Pbench +Server API while the server is undergoing maintenance and setting a banner +message accessible to clients. + ## Pbench Server resources ### Datasets diff --git a/docs/API/V1/contents.md b/docs/API/V1/contents.md index 57a18052be..12c3ced745 100644 --- a/docs/API/V1/contents.md +++ b/docs/API/V1/contents.md @@ -47,6 +47,12 @@ The authenticated client does not have READ access to the specified dataset. Either the `` or the relative `` within the dataset does not exist. +`503` **SERVICE UNAVAILABLE** \ +The server has been disabled using the `server-state` server configuration +setting in the [server configuration](./server_config.md) API. The response +body is an `application/json` document describing the current server state, +a message, and optional JSON data provided by the system administrator. + ## Response body The `application/json` response body consists of a JSON object describing the diff --git a/docs/API/V1/inventory.md b/docs/API/V1/inventory.md index 36c600bb8e..26f1b462f4 100644 --- a/docs/API/V1/inventory.md +++ b/docs/API/V1/inventory.md @@ -49,6 +49,12 @@ The `` refers to a directory. Use `/api/v1/dataset/contents/` to request a JSON response document describing the directory contents. +`503` **SERVICE UNAVAILABLE** \ +The server has been disabled using the `server-state` server configuration +setting in the [server configuration](./server_config.md) API. The response +body is an `application/json` document describing the current server state, +a message, and optional JSON data provided by the system administrator. + ## Response body The `application/octet-stream` response body is the raw byte stream contents of diff --git a/docs/API/V1/list.md b/docs/API/V1/list.md index b303aae385..78c423b826 100644 --- a/docs/API/V1/list.md +++ b/docs/API/V1/list.md @@ -87,6 +87,12 @@ by `owner` or `access=private`. The client asked to filter `access=private` datasets for an `owner` for which the client does not have READ access. +`503` **SERVICE UNAVAILABLE** \ +The server has been disabled using the `server-state` server configuration +setting in the [server configuration](./server_config.md) API. The response +body is an `application/json` document describing the current server state, +a message, and optional JSON data provided by the system administrator. + ## Response body The `application/json` response body contains a list of objects which describe diff --git a/docs/API/V1/server_config.md b/docs/API/V1/server_config.md new file mode 100644 index 0000000000..a6d8dec0c5 --- /dev/null +++ b/docs/API/V1/server_config.md @@ -0,0 +1,268 @@ +# `GET /api/v1/server/configuration[/][{key}]` + +This API returns an `application/json` document describing the Pbench Server +configuration settings. When the `{key}` parameter is specified, the API will +return the specific named server configuration setting. When `{key}` is omitted, +all server configuration settings will be returned. + +## Query parameters + +None. + +## Request headers + +`authorization: bearer` token [_optional_] \ +*Bearer* schema authorization may be specified, but is not required to `GET` +server configuration settings. + +## Response headers + +`content-type: application/json` \ +The return is a serialized JSON object with the requested server configuration +settings. + +## Response status + +`400` **BAD REQUEST** \ +The specified `{key}` value (see [settings](#server-configuration-settings)) +is unknown. + +## Response body + +The `application/json` response body is a JSON document containing the requested +server configuration setting key and value or, if no `{key}` was specified, all +supported server configuration settings. + +### Examples + +``` +GET /api/v1/server/configuration/dataset-lifetime +{ + "dataset-lifetime": "4" +} + +GET /api/v1/server/configuration/ +{ + "dataset-lifetime": "4", + "server-banner": { + "message": "Server will be down for maintenance on Tuesday!", + "contact": "admin@lab.example.com" + } + "server-state": { + "status": "readonly", + "message": "rebuilding index ... back to normal soon" + } +} +``` + +# `PUT /api/v1/server/configuration[/][{key}]` + +This API allows a user holding the `ADMIN` role to modify server configuration +settings. When the `{key}` parameter is specified, the API will modify a single +named configuration setting. When `{key}` is omitted, the `application/json` +request body can be used to modify multiple configuration settings at once. + +## Query parameters + +`value` string \ +When a single configuration setting is specified with `{key}` in the URI, you +can specify a string value for the parameter using this query parameter without +an `application/json` request body. For example, +`PUT /api/v1/server/configuration/key?value=1`. + +You cannot specify complex JSON configuration settings this way. Instead, use +the `value` field in the `application/json` request body. + +## Request body + +When specifying a complex JSON value for a server configuration setting, or when +specifying multiple configuration settings, the data to be set is specified in +an `application/json` request body. + +You can specify a single `{key}` in the URI and then specify the value +using a `value` field in the `application/json` request body instead of using +the `value` query parameter. You can do this even if the value is a simple +string, although it's more useful when you need to specify a JSON object value. +For example, + +``` +PUT /api/v1/server/configuration/server-state +{ + "value": {"status": "enabled"} +} +``` + +If you omit the `{key}` value from the URI, specify all configuration settings +you wish to change in the `application/json` request body. You can specify a +single server configuration setting, or any group of server configuration +settings at once. For example, + +``` +PUT /api/v1/server/configuration/ +{ + "server-state": {"status": "disabled", "message": "down for maintenance"}, + "server-banner": {"message": "Days of 100% uptime: 0"} +} +``` + +## Request headers + +`authorization: bearer` token \ +*Bearer* schema authorization is required to change any server configuration +settings. The authenticated user must have `ADMIN` role. + +## Response headers + +`content-type: application/json` \ +The response body is a serialized JSON object with the selected server +configuration settings. + +## Response status + +`401` **UNAUTHORIZED** \ +The client did not provide an authentication token. + +`403` **FORBIDDEN** \ +The client's authentication token does not correspond to a user with `ADMIN` +role. + +## Response body + +The `application/json` response body for `PUT` is exactly the same as for +[`GET`](#response-body) when the same server configuration settings are +requested, showing only the server configuration settings that were changed +in the `PUT`. + +_This request:_ +``` +PUT /api/v1/server/configuration/dataset-lifetime?value=4 +``` + +_returns this response:_ +``` +{ + "dataset-lifetime": "4" +} +``` + + +_And this request:_ +``` +PUT /api/v1/server/configuration +{ + "dataset-lifetime": "4 days", + "server-state": {"status": "enabled"} +} +``` + +_returns this response:_ +``` +{ + "dataset-lifetime": "4", + "server-state": {"status": "enabled"} +} +``` + +## Server configuration settings + +### dataset-lifetime + +The value for the `dataset-lifetime` server configuration setting is the *maximum* +number of days a dataset can be retained on the server. When each dataset is +uploaded to the server, a "deletion date" represented by the dataset +[metadata](../metadata.md) key `server.deletion` is calculated based on this +value and user preferences (which may specify a shorter lifetime, but not a +longer lifetime). When a dataset has remained on the server past the +`server.deletion` date, it may be removed automatically by the server to +conserve space. + +The number of days is specified as an string representing an integer, optionally +followed by a space and `day` or `days`. For example, "4" or "4 days" or "4 day" +are equivalent. + +``` +{ + "dataset-lifetime": "4" +} +``` + +### server-banner + +This server configuration setting allows a server administrator to set an +informational message that can be retrieved and displayed by any client, for +example as a banner on a UI. The value is a JSON object, containing at least +a `message` field. + +Any additional JSON data may be provided. The server will store the entire +JSON object and return it when a client requests it with +`GET /api/v1/server/configuration/server-banner`. The server will not interpret +any information in this JSON object. + +For example, the following are examples of valid banners: + +``` +{ + "server-banner": { + "message": "Have a Happy Pbench Day" + } +} +``` + +``` +{ + "server-banner": { + "message": "The server will be down for 2 hours on Monday, July 31", + "contact": { + "email": "admin@pbench.example.com", + "phone": "(555) 555-5555", + "hours": ["8:00 EST", "17:00 EST"] + }, + "statistics": { + "datasets": 50000, + "hours-up": 2.3, + "users": 26 + } + } +} +``` + +### server-state + +This server configuration setting allows a server administrator to control +the operating state of the server remotely. As for +[`server-banner`](#server-banner), the value is a JSON object, and any JSON +fields passed in to the server will be returned to a client. The +following fields have special meaning: + +**`status`** \ +The operating status of the server. + +* `enabled`: Normal operation. +* `disabled`: Most server API endpoints will fail with the **503** (service +unavailable) HTTP status. However a few endpoints are always allowed: + * [endpoints](./endpoints.md) for server configuration information; + * [login](./login.md) because only an authenticated user with `ADMIN` role + can modify server configuration settings; + * [logout](./logout.md) for consistency; + * [server_configuration](./server_config.md) to allow re-enabling the server + or modifying the banner. +* `readonly`: The server will respond to `GET` requests for information, but will return **503** (service unavailable) for any attempt to modify server state. (With the same exceptions as listed above for `disabled`.) + +**`message`** \ +A message to explain downtime. This is required when the `status` is `disabled` +or `readonly` and optional otherwise. + +When the server status is `disabled`, or when it's `readonly` and a client +tries to modify some data through the API, the server will fail the request +with a `503` (service unavailable) error. It will return an `application/json` +error response payload containing the full `server-state` JSON object. The +`message` key in an error response is a standard convention, and many clients +will display this as an explanation for the failure. The client will also have +access to any additional information provided in the `server-state` JSON object. + +Note that you can set a `message` when the `status` is `enabled` but it won't +be reported to a client unless a client asks for the `server-state` +configuration setting. The `server-state` message is intended to explain server +downtime when an API call fails. The `server-banner` configuration setting +is generally more appropriate to provide client information under normal +operating conditions. diff --git a/lib/pbench/server/__init__.py b/lib/pbench/server/__init__.py index 3e310f2640..a9bfe069b9 100644 --- a/lib/pbench/server/__init__.py +++ b/lib/pbench/server/__init__.py @@ -166,7 +166,7 @@ def rest_uri(self): return self._get_conf("pbench-server", "rest_uri") @property - def max_retention_period(self) -> timedelta: + def max_retention_period(self) -> int: """ Produce a timedelta representing the number of days the server allows a dataset to be retained. @@ -180,8 +180,7 @@ def max_retention_period(self) -> timedelta: ) except (NoOptionError, NoSectionError): retention_days = self.MAXIMUM_RETENTION_DAYS - - return timedelta(days=retention_days) + return retention_days def _get_conf(self, section, option): """ diff --git a/lib/pbench/server/api/__init__.py b/lib/pbench/server/api/__init__.py index f7c2737f55..6070669c1e 100644 --- a/lib/pbench/server/api/__init__.py +++ b/lib/pbench/server/api/__init__.py @@ -36,6 +36,7 @@ from pbench.server.api.resources.query_apis.datasets_publish import DatasetsPublish from pbench.server.api.resources.query_apis.datasets_search import DatasetsSearch from pbench.server.api.resources.query_apis.elasticsearch_api import Elasticsearch +from pbench.server.api.resources.server_configuration import ServerConfiguration from pbench.server.api.resources.upload_api import Upload from pbench.server.api.resources.users_api import Login, Logout, RegisterUser, UserAPI from pbench.server.database import init_db @@ -165,6 +166,14 @@ def register_endpoints(api, app, config): endpoint="register", resource_class_args=(config, logger), ) + api.add_resource( + ServerConfiguration, + f"{base_uri}/server/configuration", + f"{base_uri}/server/configuration/", + f"{base_uri}/server/configuration/", + endpoint="server_configuration", + resource_class_args=(config, logger), + ) api.add_resource( UserAPI, f"{base_uri}/user/", diff --git a/lib/pbench/server/api/resources/__init__.py b/lib/pbench/server/api/resources/__init__.py index 7b8299f58c..3a6d847617 100644 --- a/lib/pbench/server/api/resources/__init__.py +++ b/lib/pbench/server/api/resources/__init__.py @@ -21,6 +21,7 @@ MetadataBadKey, MetadataNotFound, ) +from pbench.server.database.models.server_config import ServerConfig from pbench.server.database.models.users import User @@ -64,6 +65,30 @@ def __str__(self) -> str: return f"{'User ' + self.user.username if self.user else 'Unauthenticated client'} is not authorized to {self.operation.name} a resource owned by {self.owner} with {self.access} access" +class UnauthorizedAdminAccess(UnauthorizedAccess): + """ + A refinement of the UnauthorizedAccess exception where ADMIN access is + required and we have no associated resource owner and access. + """ + + def __init__( + self, + user: Union[User, None], + operation: "API_OPERATION", + http_status: int = HTTPStatus.FORBIDDEN, + ): + super().__init__( + user=user, + operation=operation, + owner=None, + access=None, + http_status=http_status, + ) + + def __str__(self) -> str: + return f"{'User ' + self.user.username if self.user else 'Unauthenticated client'} is not authorized to {self.operation.name} a server administrative resource" + + class SchemaError(APIAbort): """ Generic base class for errors in processing a JSON schema. @@ -685,11 +710,13 @@ class API_AUTHORIZATION(Enum): USER_ACCESS: The client is authorized against the USER and ACCESS type parameters, which must each appear only once in the various schema for the HTTP method used. + ADMIN: The client's authenticated user has administrator role. """ NONE = auto() DATASET = auto() USER_ACCESS = auto() + ADMIN = auto() class ApiParams(NamedTuple): @@ -711,9 +738,10 @@ class ApiAuthorization(NamedTuple): the desired access role. """ - user: Optional[str] - access: Optional[str] + type: API_AUTHORIZATION role: API_OPERATION + user: Optional[str] = None + access: Optional[str] = None class Schema: @@ -935,6 +963,7 @@ def authorize(self, params: ApiParams) -> Optional[ApiAuthorization]: ds = self.get_param_by_type(ParamType.DATASET, params) if ds: return ApiAuthorization( + type=self.authorization, user=str(ds.value.owner_id), access=ds.value.access, role=self.operation, @@ -944,8 +973,13 @@ def authorize(self, params: ApiParams) -> Optional[ApiAuthorization]: user = self.get_param_by_type(ParamType.USER, params) access = self.get_param_by_type(ParamType.ACCESS, params) return ApiAuthorization( - user=user.value, access=access.value, role=self.operation + type=self.authorization, + user=user.value, + access=access.value, + role=self.operation, ) + elif self.authorization == API_AUTHORIZATION.ADMIN: + return ApiAuthorization(type=self.authorization, role=self.operation) return None @@ -955,8 +989,8 @@ def __init__(self, *schemas: ApiSchema): Construct a dict of schemas accessable by API method Args: - schemas: A list of schemas for each HTTP method supported by an - API class. + schemas: A list of schemas for each HTTP method supported by an + API class. """ self.schemas: Dict[API_METHOD, ApiSchema] = {s.method: s for s in schemas} @@ -1053,7 +1087,13 @@ class ApiBase(Resource): _put, and _delete methods. """ - def __init__(self, config: PbenchServerConfig, logger: Logger, *schemas: ApiSchema): + def __init__( + self, + config: PbenchServerConfig, + logger: Logger, + *schemas: ApiSchema, + always_enabled: bool = False, + ): """ Base class constructor. @@ -1061,13 +1101,17 @@ def __init__(self, config: PbenchServerConfig, logger: Logger, *schemas: ApiSche config: server configuration logger: logger object schemas: ApiSchema objects to provide parameter validation for the - various HTTP methods the API module supports. For example, - for GET, PUT, and DELETE. + various HTTP methods the API module supports. For example, for + GET, PUT, and DELETE. + always_enabled: Most APIs are disabled when the server state is not + enabled. A few, like endpoints and the config APIs, must always + be usable. """ super().__init__() self.config = config self.logger = logger self.schemas = ApiSchemaSet(*schemas) + self.always_enabled = always_enabled def _gather_query_params(self, request: Request, schema: Schema) -> JSONOBJECT: """ @@ -1110,12 +1154,7 @@ def _gather_query_params(self, request: Request, schema: Schema) -> JSONOBJECT: return json - def _check_authorization( - self, - user_id: Optional[str], - access: Optional[str], - role: API_OPERATION, - ): + def _check_authorization(self, mode: ApiAuthorization): """ Check whether an API call is able to access data, based on the API's authorization header, the requested user, the requested access @@ -1142,9 +1181,7 @@ def _check_authorization( Any authenticated ADMIN user can update/delete any data. Args: - user_id: The client's requested user ID (as a string), or None - access: The client's requested access parameter, or None - check_role: Override self.role for access check + ApiAuthorization object with type, role, and user information Raises: UnauthorizedAccess: The user isn't authorized for the requested @@ -1155,6 +1192,8 @@ def _check_authorization( rights to the specified combination of API ROLE, USER, and ACCESS. """ + user_id = mode.user + role = mode.role authorized_user: User = Auth.token_auth.current_user() username = "none" if user_id: @@ -1165,13 +1204,35 @@ def _check_authorization( self.logger.error( "Parser user ID {} not found in User database", user_id ) + + # The ADMIN authorization doesn't involve a target resource owner or + # access, so take care of that first as a special case. If there is + # an authenticated user, and that user holds ADMIN access rights, the + # check passes. Otherwise raise an "admin access" failure. + if mode.type == API_AUTHORIZATION.ADMIN: + self.logger.debug( + "Authorizing {} access for {} to an administrative resource", + role, + authorized_user, + ) + if not authorized_user: + raise UnauthorizedAdminAccess( + authorized_user, role, http_status=HTTPStatus.UNAUTHORIZED + ) + if not authorized_user.is_admin(): + raise UnauthorizedAdminAccess(authorized_user, role) + return + + access = mode.access + self.logger.debug( - "Authorizing {} access for {} to user {} ({}) with access {}", + "Authorizing {} access for {} to user {} ({}) with access {} using {}", role, authorized_user, username, user_id, - access, + mode.access, + mode.type, ) # Accept or reject the described operation according to the following @@ -1435,6 +1496,13 @@ def _dispatch( self.logger.info("In {} {}", method, api_name) + if not self.always_enabled: + disabled = ServerConfig.get_disabled( + readonly=(self.schemas[method].operation == API_OPERATION.READ) + ) + if disabled: + abort(HTTPStatus.SERVICE_UNAVAILABLE, **disabled) + if method is API_METHOD.GET: execute = self._get elif method is API_METHOD.PUT: @@ -1495,16 +1563,8 @@ def _dispatch( # mechanism is required by the API. auth_params = self.schemas.authorize(method, params) if auth_params: - self.logger.info( - "Authorizing with {}:{}, {}", - auth_params.user, - auth_params.access, - auth_params.role, - ) try: - self._check_authorization( - auth_params.user, auth_params.access, auth_params.role - ) + self._check_authorization(auth_params) except UnauthorizedAccess as e: self.logger.warning("{}: {}", api_name, e) abort(e.http_status, message=str(e)) diff --git a/lib/pbench/server/api/resources/datasets_metadata.py b/lib/pbench/server/api/resources/datasets_metadata.py index edbf98e156..f08dce6aff 100644 --- a/lib/pbench/server/api/resources/datasets_metadata.py +++ b/lib/pbench/server/api/resources/datasets_metadata.py @@ -12,6 +12,7 @@ API_METHOD, APIAbort, API_OPERATION, + ApiAuthorization, ApiBase, ApiParams, ApiSchema, @@ -148,7 +149,14 @@ def _put(self, params: ApiParams, _) -> Response: for k in metadata.keys(): if Metadata.get_native_key(k) != Metadata.USER: role = API_OPERATION.UPDATE - self._check_authorization(str(dataset.owner_id), dataset.access, role) + self._check_authorization( + ApiAuthorization( + API_AUTHORIZATION.USER_ACCESS, + role, + str(dataset.owner_id), + dataset.access, + ) + ) # Validate the metadata key values in a separate pass so that we can # fail before committing any changes to the database. diff --git a/lib/pbench/server/api/resources/server_configuration.py b/lib/pbench/server/api/resources/server_configuration.py new file mode 100644 index 0000000000..622cfc8066 --- /dev/null +++ b/lib/pbench/server/api/resources/server_configuration.py @@ -0,0 +1,231 @@ +from http import HTTPStatus +from logging import Logger + +from flask.json import jsonify +from flask.wrappers import Request, Response + +from pbench.server import PbenchServerConfig +from pbench.server.api.resources import ( + API_AUTHORIZATION, + API_METHOD, + API_OPERATION, + APIAbort, + ApiBase, + ApiParams, + ApiSchema, + ParamType, + Parameter, + Schema, +) +from pbench.server.database.models.server_config import ( + ServerConfig, + ServerConfigBadValue, + ServerConfigError, +) + + +class ServerConfiguration(ApiBase): + """ + API class to retrieve and mutate server configuration settings. + """ + + def __init__(self, config: PbenchServerConfig, logger: Logger): + super().__init__( + config, + logger, + ApiSchema( + API_METHOD.PUT, + API_OPERATION.UPDATE, + uri_schema=Schema( + Parameter("key", ParamType.KEYWORD, keywords=ServerConfig.KEYS) + ), + # We aspire to be flexible about how a config option is + # specified on a PUT. The value of a single config option can + # be set using the "value" query parameter or the "value" JSON + # body parameter. You can also specify one config option or + # multiple config options by omitting the key name from the URI + # and specifying the names and values in a JSON request body: + # + # PUT /server/configuration/dataset-lifetime?value=2y + # PUT /server/configuration/dataset-lifetime + # {"value": "2y"} + # PUT /server/configuration + # {"dataset-lifetime": "2y"} + query_schema=Schema(Parameter("value", ParamType.STRING)), + body_schema=Schema( + Parameter("value", ParamType.JSON), + ), + authorization=API_AUTHORIZATION.ADMIN, + ), + ApiSchema( + API_METHOD.GET, + API_OPERATION.READ, + uri_schema=Schema( + Parameter("key", ParamType.KEYWORD, keywords=ServerConfig.KEYS) + ), + authorization=API_AUTHORIZATION.NONE, + ), + always_enabled=True, + ) + + def _get(self, params: ApiParams, request: Request) -> Response: + """ + Get the values of server configuration parameters. + + GET /api/v1/server/configuration/{key} + return the value of a single configuration parameter + + or + + GET /api/v1/server/configuration + return all configuration parameters + + Args: + params: API parameters + request: The original Request object containing query parameters + + Returns: + HTTP Response object + """ + + key = params.uri.get("key") + try: + if key: + s = ServerConfig.get(key) + return jsonify({key: s.value if s else None}) + else: + return jsonify(ServerConfig.get_all()) + except ServerConfigError as e: + raise APIAbort(HTTPStatus.INTERNAL_SERVER_ERROR, str(e)) from e + + def _put_key(self, params: ApiParams) -> Response: + """ + Implement the PUT operation when a system configuration setting key is + specified on the URI as /system/configuration/{key}. + + A single system config setting is set by naming the config key in the + URI and specifying a value using either the "value" query parameter or + a "value" key in a JSON request body. + + We'll complain about JSON request body parameters that are "shadowed" + by the "value" query parameter and might represent client confusion. + We won't complain about unnecessary JSON request body keys if we find + the "value" in the request body as those would normally have been + ignored by schema validation. + + Args: + params: API parameters + request: The original Request object containing query parameters + + Returns: + HTTP Response object + """ + + try: + key = params.uri["key"] + except KeyError: + # This "isn't possible" given the Flask mapping rules, but try + # to report it gracefully instead of letting the KeyError fly. + raise APIAbort( + HTTPStatus.INTERNAL_SERVER_ERROR, + f"Found URI parameters {sorted(params.uri)} not including 'key'", + ) + + # If we have a key in the URL, then we need a "value" for it, which + # we can take either from a query parameter or from the JSON + # request payload. + value = params.query.get("value") + if value: + # If we got the value from the query parameter, complain about + # any JSON request body keys + if params.body: + raise APIAbort( + HTTPStatus.BAD_REQUEST, + "Redundant parameters specified in the JSON request body: " + f"{sorted(params.body.keys())!r}", + ) + else: + value = params.body.get("value") + if not value: + raise APIAbort( + HTTPStatus.BAD_REQUEST, + f"No value found for key system configuration key {key!r}", + ) + + try: + ServerConfig.set(key=key, value=value) + except ServerConfigBadValue as e: + raise APIAbort(HTTPStatus.BAD_REQUEST, str(e)) from e + except ServerConfigError as e: + raise APIAbort(HTTPStatus.INTERNAL_SERVER_ERROR, str(e)) from e + return jsonify({key: value}) + + def _put_body(self, params: ApiParams) -> Response: + """ + Allow setting the value of multiple system configuration settings with + a single PUT by specifying a JSON request body with key/value pairs. + + Args: + params: API parameters + request: The original Request object containing query parameters + + Returns: + HTTP Response object + """ + badkeys = [] + for k, v in params.body.items(): + if k not in ServerConfig.KEYS: + badkeys.append(k) + + if badkeys: + raise APIAbort( + HTTPStatus.BAD_REQUEST, + f"Unrecognized configuration parameters {sorted(badkeys)!r} specified: valid parameters are {sorted(ServerConfig.KEYS)!r}", + ) + + failures = [] + response = {} + fail_status = HTTPStatus.BAD_REQUEST + for k, v in params.body.items(): + try: + c = ServerConfig.set(key=k, value=v) + response[c.key] = c.value + except ServerConfigBadValue as e: + failures.append(str(e)) + except ServerConfigError as e: + fail_status = HTTPStatus.INTERNAL_SERVER_ERROR + self.logger.warning(str(e)) + failures.append(str(e)) + if failures: + raise APIAbort(fail_status, message=", ".join(failures)) + return jsonify(response) + + def _put(self, params: ApiParams, _) -> Response: + """ + Set or modify the values of server configuration keys. + + PUT /api/v1/server/configuration + { + "dataset-lifetime": 10, + "server-state": "running" + } + + PUT /api/v1/server/configuration/dataset-lifetime?value=10 + + PUT /api/v1/server/configuration/dataset-lifetime + { + "value": "10" + } + + Args: + params: API parameters + request: The original Request object containing query parameters + + Returns: + HTTP Response object + """ + + if params.uri: + return self._put_key(params) + else: + return self._put_body(params) diff --git a/lib/pbench/server/api/resources/upload_api.py b/lib/pbench/server/api/resources/upload_api.py index f5c78e5b9e..4cd3f06a33 100644 --- a/lib/pbench/server/api/resources/upload_api.py +++ b/lib/pbench/server/api/resources/upload_api.py @@ -17,6 +17,7 @@ Metadata, States, ) +from pbench.server.database.models.server_config import ServerConfig from pbench.server.filetree import FileTree from pbench.server.utils import UtcTimeHelper, filesize_bytes @@ -59,6 +60,9 @@ def __init__(self, config, logger): @Auth.token_auth.login_required() def put(self, filename: str): + disabled = ServerConfig.get_disabled() + if disabled: + abort(HTTPStatus.SERVICE_UNAVAILABLE, **disabled) # Used to record what steps have been completed during the upload, and # need to be undone on failure diff --git a/lib/pbench/server/api/resources/users_api.py b/lib/pbench/server/api/resources/users_api.py index 9343f6956d..dfbb071eaa 100644 --- a/lib/pbench/server/api/resources/users_api.py +++ b/lib/pbench/server/api/resources/users_api.py @@ -6,6 +6,7 @@ from email_validator import EmailNotValidError from sqlalchemy.exc import SQLAlchemyError, IntegrityError from typing import NamedTuple +from pbench.server.database.models.server_config import ServerConfig from pbench.server.database.models.users import User from pbench.server.database.models.active_tokens import ActiveTokens from pbench.server.api.auth import Auth @@ -45,6 +46,10 @@ def post(self): } To get the auth token user has to perform the login action """ + disabled = ServerConfig.get_disabled() + if disabled: + abort(HTTPStatus.SERVICE_UNAVAILABLE, **disabled) + # get the post data user_data = request.get_json() if not user_data: @@ -377,6 +382,10 @@ def get(self, target_username): "message": "failure message" } """ + disabled = ServerConfig.get_disabled(readonly=True) + if disabled: + abort(HTTPStatus.SERVICE_UNAVAILABLE, **disabled) + result = self.get_valid_target_user(target_username, "GET") if not result.target_user: abort(result.http_status, message=result.http_message) @@ -416,6 +425,10 @@ def put(self, target_username): "message": "failure message" } """ + disabled = ServerConfig.get_disabled() + if disabled: + abort(HTTPStatus.SERVICE_UNAVAILABLE, **disabled) + user_payload = request.get_json() if not user_payload: self.logger.warning("Invalid json object: {}", request.url) @@ -487,6 +500,10 @@ def delete(self, target_username): "message": "failure message" } """ + disabled = ServerConfig.get_disabled() + if disabled: + abort(HTTPStatus.SERVICE_UNAVAILABLE, **disabled) + result = self.get_valid_target_user(target_username, "DELETE") if not result.target_user: abort(result.http_status, message=result.http_message) diff --git a/lib/pbench/server/database/database.py b/lib/pbench/server/database/database.py index 30ab76334a..ac30e9e82c 100644 --- a/lib/pbench/server/database/database.py +++ b/lib/pbench/server/database/database.py @@ -30,6 +30,7 @@ def init_db(server_config, logger): # database models to find if logger and not hasattr(Database.Base, "logger"): Database.Base.logger = logger + if server_config and not hasattr(Database.Base, "server_config"): Database.Base.config = server_config # WARNING: diff --git a/lib/pbench/server/database/models/datasets.py b/lib/pbench/server/database/models/datasets.py index e32353349b..587c731986 100644 --- a/lib/pbench/server/database/models/datasets.py +++ b/lib/pbench/server/database/models/datasets.py @@ -12,6 +12,10 @@ from sqlalchemy.types import TypeDecorator from pbench.server.database.database import Database +from pbench.server.database.models.server_config import ( + OPTION_DATASET_LIFETIME, + ServerConfig, +) from pbench.server.database.models.users import User @@ -1127,7 +1131,10 @@ def validate(dataset: Dataset, key: str, value: Any) -> Any: except date_parser.ParserError as p: raise MetadataBadValue(dataset, key, value, "date/time") from p - maximum = dataset.uploaded + __class__.config.max_retention_period + max_retention = ServerConfig.get(key=OPTION_DATASET_LIFETIME) + maximum = dataset.uploaded + datetime.timedelta( + days=int(max_retention.value) + ) if target > maximum: raise MetadataBadValue( dataset, key, value, f"date/time before {maximum:%Y-%m-%d}" diff --git a/lib/pbench/server/database/models/server_config.py b/lib/pbench/server/database/models/server_config.py new file mode 100644 index 0000000000..9e32cf3aa0 --- /dev/null +++ b/lib/pbench/server/database/models/server_config.py @@ -0,0 +1,389 @@ +import re +from typing import Optional + +from sqlalchemy import Column, Integer, String +from sqlalchemy.exc import IntegrityError, SQLAlchemyError +from sqlalchemy.sql.sqltypes import JSON + +from pbench.server import JSONOBJECT, JSONVALUE +from pbench.server.database.database import Database + + +class ServerConfigError(Exception): + """ + This is a base class for errors reported by the ServerConfig class. It is + never raised directly, but may be used in "except" clauses. + """ + + pass + + +class ServerConfigSqlError(ServerConfigError): + """ + SQLAlchemy errors reported through ServerConfig operations. + + The exception will identify the operation being performed and the config + key; the cause will specify the original SQLAlchemy exception. + """ + + def __init__(self, operation: str, name: str, cause: str): + self.operation = operation + self.name = name + self.cause = cause + + def __str__(self) -> str: + return f"Error {self.operation} index {self.name!r}: {self.cause}" + + +class ServerConfigDuplicate(ServerConfigError): + """ + Attempt to commit a duplicate ServerConfig. + """ + + def __init__(self, name: str, cause: str): + self.name = name + self.cause = cause + + def __str__(self) -> str: + return f"Duplicate config setting {self.name!r}: {self.cause}" + + +class ServerConfigNullKey(ServerConfigError): + """ + Attempt to commit a ServerConfig with an empty key. + """ + + def __init__(self, name: str, cause: str): + self.name = name + self.cause = cause + + def __str__(self) -> str: + return f"Missing key value in {self.name!r}: {self.cause}" + + +class ServerConfigMissingKey(ServerConfigError): + """ + Attempt to set a ServerConfig with an empty key. + """ + + def __str__(self) -> str: + return "Missing key name" + + +class ServerConfigBadKey(ServerConfigError): + """ + Attempt to commit a ServerConfig with a bad key. + """ + + def __init__(self, key: str): + self.key = key + + def __str__(self) -> str: + return f"Configuration key {self.key!r} is unknown" + + +class ServerConfigBadValue(ServerConfigError): + """ + Attempt to assign a bad value to a server configuration option + """ + + def __init__(self, key: str, value: JSONVALUE): + self.key = key + self.value = value + + def __str__(self) -> str: + return f"Unsupported value for configuration key {self.key!r} ({self.value!r})" + + +# Formal timedelta syntax is "[D day[s], ][H]H:MM:SS[.UUUUUU]"; without an +# external package we have no standard way to parse or format timedelta +# strings. Additionally for "lifetime" we don't care about fractional days, +# so we simpify by accepting "D[[ ]day[s]]". +_TIMEDELTA_FORMAT = re.compile(r"(?P\d{1,9})(?:\s*days?)?") + + +def validate_lifetime(key: str, value: JSONVALUE) -> JSONVALUE: + v = str(value) + check = _TIMEDELTA_FORMAT.fullmatch(v) + if check: + days = check.group("days") + else: + raise ServerConfigBadValue(key, value) + return days + + +# Formal "state" syntax is a JSON object with a "status" key designating the +# current server status ("enabled", "disabled", or "readonly" to allow read +# access but not modification of resources), and a "message" string +# describing why the server isn't enabled. The "message" is required only if +# the "status" isn't "enabled". For example, +# +# {"status": "enabled"} +# +# or +# +# {"status": "disabled", "message": "Down for maintanance"} +# +# or +# +# {"status": "readonly", "message": "Available for queries but no changes allowed"} +# +# Additional fields can be set by an administrator and will be retained and +# reported when the state is queried or when an API call fails because the +# state isn't enabled. +STATE_STATUS_KEY = "status" +STATE_MESSAGE_KEY = "message" +STATE_STATUS_KEYWORD_DISABLED = "disabled" +STATE_STATUS_KEYWORD_ENABLED = "enabled" +STATE_STATUS_KEYWORD_READONLY = "readonly" +STATE_STATUS_KEYWORDS = [ + STATE_STATUS_KEYWORD_DISABLED, + STATE_STATUS_KEYWORD_ENABLED, + STATE_STATUS_KEYWORD_READONLY, +] + + +def validate_server_state(key: str, value: JSONVALUE) -> JSONVALUE: + try: + status = value[STATE_STATUS_KEY].lower() + except (KeyError, SyntaxError, TypeError) as e: + raise ServerConfigBadValue(key, value) from e + if status not in STATE_STATUS_KEYWORDS: + raise ServerConfigBadValue(key, value) + + # canonicalize the status value by lowercasing it + value[STATE_STATUS_KEY] = status + if status != STATE_STATUS_KEYWORD_ENABLED and STATE_MESSAGE_KEY not in value: + raise ServerConfigBadValue(key, value) + return value + + +# A set of string values, including a message for users that can be displayed +# as a banner in a client. This must include a "message" and may also include +# other fields such as administrator contacts, maintenance schedule. +BANNER_MESSAGE_KEY = "message" + + +def validate_server_banner(key: str, value: JSONVALUE) -> JSONVALUE: + if not isinstance(value, dict) or BANNER_MESSAGE_KEY not in value: + raise ServerConfigBadValue(key, value) + return value + + +OPTION_DATASET_LIFETIME = "dataset-lifetime" +OPTION_SERVER_BANNER = "server-banner" +OPTION_SERVER_STATE = "server-state" + +SERVER_CONFIGURATION_OPTIONS = { + OPTION_DATASET_LIFETIME: { + "validator": validate_lifetime, + "default": lambda: str(ServerConfig.config.max_retention_period), + }, + OPTION_SERVER_BANNER: { + "validator": validate_server_banner, + "default": lambda: None, + }, + OPTION_SERVER_STATE: { + "validator": validate_server_state, + "default": lambda: {STATE_STATUS_KEY: STATE_STATUS_KEYWORD_ENABLED}, + }, +} + + +class ServerConfig(Database.Base): + """ + A framework to manage Pbench configuration settings. This is a simple + key-value store that can be used flexibly to manage runtime configuration. + + Columns: + id Generated unique ID of table row + key Configuration key name + value Configuration key value + """ + + KEYS = sorted([s for s in SERVER_CONFIGURATION_OPTIONS.keys()]) + + __tablename__ = "serverconfig" + + id = Column(Integer, primary_key=True, autoincrement=True) + key = Column(String(255), unique=True, index=True, nullable=False) + value = Column(JSON, index=False, nullable=True) + + @staticmethod + def _default(key: str) -> JSONVALUE: + try: + config = SERVER_CONFIGURATION_OPTIONS[key] + except KeyError as e: + if key: + raise ServerConfigBadKey(key) from e + else: + raise ServerConfigMissingKey() from e + return config["default"]() + + @staticmethod + def _validate(key: str, value: JSONVALUE) -> JSONVALUE: + try: + config = SERVER_CONFIGURATION_OPTIONS[key] + except KeyError as e: + if key: + raise ServerConfigBadKey(key) from e + else: + raise ServerConfigMissingKey() from e + return config["validator"](key, value) + + @staticmethod + def create(key: str, value: JSONVALUE) -> "ServerConfig": + """ + A simple factory method to construct a new ServerConfig setting and + add it to the database. + + Args: + key: config setting key + value: config setting value + + Returns: + A new ServerConfig object initialized with the parameters and added + to the database. + """ + + v = __class__._validate(key, value) + config = ServerConfig(key=key, value=v) + config.add() + return config + + @staticmethod + def get(key: str, use_default: bool = True) -> "ServerConfig": + """ + Return a ServerConfig object with the specified configuration key + setting. For example, ServerConfig.get("dataset-lifetime"). + + If the setting has no definition, a default value will optionally + be provided. + + Args: + key: System configuration setting name + use_default: If the DB value is None, return a default + + Raises: + ServerConfigSqlError: problem interacting with Database + + Returns: + ServerConfig object with the specified key name or None + """ + try: + config = Database.db_session.query(ServerConfig).filter_by(key=key).first() + if config is None and use_default: + config = ServerConfig(key=key, value=__class__._default(key)) + except SQLAlchemyError as e: + raise ServerConfigSqlError("finding", key, str(e)) from e + return config + + @staticmethod + def set(key: str, value: JSONVALUE) -> "ServerConfig": + """ + Update a ServerConfig key with the specified value. For + example, ServerConfig.set("dataset-lifetime"). + + Args: + key: Configuration setting name + + Returns: + ServerConfig object with the specified key name + """ + + v = __class__._validate(key, value) + config = __class__.get(key, use_default=False) + if config: + config.value = v + config.update() + else: + config = __class__.create(key=key, value=v) + return config + + @staticmethod + def get_disabled(readonly: bool = False) -> Optional[JSONOBJECT]: + """ + Determine whether the current 'server-state' setting disallows the + requested access. By default we check for WRITE access, which + can be overridden by specifying 'readonly=True' + + Args: + readonly: Caller requires only read access to the server API. + + Returns: + None if the server is enabled. If the server is disabled for the + requested access, the entire JSON value is returned and should be + reported to a caller. + """ + state = __class__.get(key=OPTION_SERVER_STATE) + if state: + value = state.value + status = value[STATE_STATUS_KEY] + if status == "disabled" or status == "readonly" and not readonly: + return value + return None + + @staticmethod + def get_all() -> JSONOBJECT: + """ + Return all server config settings as a JSON object + """ + configs = Database.db_session.query(ServerConfig).all() + db = {c.key: c.value for c in configs} + return {k: db[k] if k in db else __class__._default(k) for k in __class__.KEYS} + + def __str__(self) -> str: + """ + Return a string representation of the key-value pair + + Returns: + string representation + """ + return f"{self.key}: {self.value!r}" + + def _decode(self, exception: IntegrityError) -> Exception: + """ + Decode a SQLAlchemy IntegrityError to look for a recognizable UNIQUE + or NOT NULL constraint violation. Return the original exception if + it doesn't match. + + Args: + exception: An IntegrityError to decode + + Returns: + a more specific exception, or the original if decoding fails + """ + # Postgres engine returns (code, message) but sqlite3 engine only + # returns (message); so always take the last element. + cause = exception.orig.args[-1] + if cause.find("UNIQUE constraint") != -1: + return ServerConfigDuplicate(self.key, cause) + elif cause.find("NOT NULL constraint") != -1: + return ServerConfigNullKey(self.key, cause) + return exception + + def add(self): + """ + Add the ServerConfig object to the database + """ + try: + Database.db_session.add(self) + Database.db_session.commit() + except Exception as e: + Database.db_session.rollback() + if isinstance(e, IntegrityError): + raise self._decode(e) from e + raise ServerConfigSqlError("adding", self.key, str(e)) from e + + def update(self): + """ + Update the database row with the modified version of the + ServerConfig object. + """ + try: + Database.db_session.commit() + except Exception as e: + Database.db_session.rollback() + if isinstance(e, IntegrityError): + raise self._decode(e) from e + raise ServerConfigSqlError("updating", self.key, str(e)) from e diff --git a/lib/pbench/test/unit/server/database/test_server_config_db.py b/lib/pbench/test/unit/server/database/test_server_config_db.py new file mode 100644 index 0000000000..1ee89483f5 --- /dev/null +++ b/lib/pbench/test/unit/server/database/test_server_config_db.py @@ -0,0 +1,473 @@ +from typing import Dict, List, Optional + +import pytest +from sqlalchemy.exc import IntegrityError + +from pbench.server.database.database import Database +from pbench.server.database.models.server_config import ( + ServerConfig, + ServerConfigBadKey, + ServerConfigBadValue, + ServerConfigDuplicate, + ServerConfigMissingKey, + ServerConfigNullKey, +) + + +# NOTE: the SQLAlchemy mock infrastructure here is specific to the ServerConfig +# class, but it could "easily" (with substantial effort) be generalized to +# become a shared mock platform for DB operation testing, including across +# multiple DB tables (e.g., by maintaining a dict of session commit lists by +# database table name) + + +class FakeDBOrig: + """ + A simple mocked replacement for SQLAlchemy's engine "origin" object in + exceptions. + """ + + def __init__(self, arg: str): + """ + Create an 'orig' object + + Args: + arg: A text string passing information about the engine's + SQL query. + """ + self.args = [arg] + + +class FakeRow: + """ + Maintain an internal "database row" copy that we can use to verify the + committed records and compare active proxy values against the committed DB + rows. + """ + + def __init__(self, id, key, value): + self.id = id + self.key = key + self.value = value + + def __eq__(self, entity) -> bool: + return ( + isinstance(entity, __class__) + and self.id == entity.id + and self.key == entity.key + and self.value == entity.value + ) + + def __gt__(self, entity) -> bool: + return self.id > entity.id + + def __lt__(self, entity) -> bool: + return self.id < entity.id + + +class FakeQuery: + """ + Model the SQLAlchemy query operations by reducing the list of known + committed DB values based on filter expressions. + """ + + def __init__(self, session: "FakeSession"): + """ + Set up the query using a copy of the full list of known DB objects. + + Args: + session: The associated fake session object + """ + self.selected = list(session.known.values()) + self.session = session + + def all(self) -> List[ServerConfig]: + """ + Return all selected records + """ + return self.selected + + def filter_by(self, **kwargs) -> "FakeQuery": + """ + Reduce the list of matching DB objects by matching against the two + main columns. + + Args: + kwargs: Standard SQLAlchemy signature + key: if present, select by key + value: if present, select by value + + Returns: + The query so filters can be chained + """ + for s in self.selected: + if "id" in kwargs and s.id != kwargs["id"]: + self.selected.remove(s) + elif "key" in kwargs and s.key != kwargs["key"]: + self.selected.remove(s) + elif "value" in kwargs and s.value != kwargs["value"]: + self.selected.remove(s) + return self + + def first(self) -> Optional[ServerConfig]: + """ + Return the first match, or None if there are none left. + """ + return self.selected[0] if self.selected else None + + +class FakeSession: + """ + Mock a SQLAlchemy Session for testing. + """ + + def __init__(self): + """ + Initialize the context of the session. + + NOTE: 'raise_on_commit' can be set by a caller to cause an exception + during the next commit operation. The exception should generally be + a subclass of the SQLAlchemy IntegrityError. This is a "one shot" and + will be reset when the exception is raised. + """ + self.id = 1 + self.added: List[ServerConfig] = [] + self.known: Dict[int, ServerConfig] = {} + self.committed: Dict[int, FakeRow] = {} + self.rolledback = 0 + self.queries: List[FakeQuery] = [] + self.raise_on_commit: Optional[Exception] = None + + def query(self, *entities, **kwargs) -> FakeQuery: + """ + Perform a mocked query on the session, setting up the query context + and returning it + + Args: + entities: The SQLAlchemy entities on which we're operating + kwargs: Additional SQLAlchemy parameters + + Returns: + A mocked query object + """ + q = FakeQuery(self) + self.queries.append(q) + return q + + def add(self, config: ServerConfig): + """ + Add a DB object to a list for testing + + Args: + config: A server configuration setting object + """ + self.added.append(config) + + def commit(self): + """ + Mock a commit operation on the DB session. If the 'raise_on_commit' + property has been set, "fail" by raising the exception. Otherwise, + mock a commit by updating any cached "committed" values if the "known" + proxy objects have changed, and record any new "added" objects. + """ + if self.raise_on_commit: + exc = self.raise_on_commit + self.raise_on_commit = None + raise exc + for k in self.known: + self.committed[k].value = self.known[k].value + for a in self.added: + a.id = self.id + self.id += 1 + self.known[a.id] = a + self.committed[a.id] = FakeRow(id=a.id, key=a.key, value=a.value) + self.added = [] + + def rollback(self): + """ + Just record that rollback was called, since we always raise an error + before changing anything during the mocked commit. + """ + self.rolledback += 1 + + # Clear the proxy state by removing any new objects that weren't yet + # committed, and "rolling back" any proxy values that were updated + # from the committed values. + self.added = [] + for k in self.committed: + self.known[k].value = self.committed[k].value + + +class TestServerConfig: + session = None + + def check_session(self, queries=0, committed: List[FakeRow] = [], rolledback=0): + """ + Encapsulate the common checks we want to make after running test cases. + + Args: + queries: A count of queries we expect to have been made + committed: A list of FakeRow objects we expect to have been + committed + rolledback: True if we expect rollback to have been called + """ + session = self.session + assert session + + # Check the number of queries we've created + assert len(session.queries) == queries + + # 'added' is an internal "dirty" list between 'add' and 'commit' or + # 'rollback'. We test that 'commit' moves elements to the committed + # state and 'rollback' clears the list. We don't ever expect to see + # anything on this list. + assert not session.added + + # Check that the 'committed' list (which stands in for the actual DB + # table) contains the expected rows. + assert sorted(list(session.committed.values())) == sorted(committed) + + # Check whether we've rolled back a transaction due to failure. + assert session.rolledback == rolledback + + @pytest.fixture(autouse=True, scope="function") + def fake_db(self, monkeypatch, server_config): + """ + Fixture to mock a DB session for testing. + + We patch the SQLAlchemy db_session to our fake session. We also store a + server configuration object directly on the Database.Base (normally + done during DB initialization) because that can't be monkeypatched. + """ + self.session = FakeSession() + with monkeypatch.context() as m: + m.setattr(Database, "db_session", self.session) + Database.Base.config = server_config + yield + + def test_bad_key(self): + """Test server config parameter key validation""" + with pytest.raises(ServerConfigBadKey) as exc: + ServerConfig.create(key="not-a-key", value="no-no") + assert str(exc.value) == "Configuration key 'not-a-key' is unknown" + + def test_construct(self): + """Test server config parameter constructor""" + config = ServerConfig(key="dataset-lifetime", value="2") + assert config.key == "dataset-lifetime" + assert config.value == "2" + assert str(config) == "dataset-lifetime: '2'" + self.check_session() + + def test_create(self): + """Test server config parameter creation""" + config = ServerConfig.create(key="dataset-lifetime", value="2") + assert config.key == "dataset-lifetime" + assert config.value == "2" + assert str(config) == "dataset-lifetime: '2'" + self.check_session(committed=[FakeRow(id=1, key="dataset-lifetime", value="2")]) + + def test_construct_duplicate(self): + """Test server config parameter constructor""" + ServerConfig.create(key="dataset-lifetime", value=1) + self.check_session(committed=[FakeRow(id=1, key="dataset-lifetime", value="1")]) + with pytest.raises(ServerConfigDuplicate) as e: + self.session.raise_on_commit = IntegrityError( + statement="", params="", orig=FakeDBOrig("UNIQUE constraint") + ) + ServerConfig.create(key="dataset-lifetime", value=2) + assert str(e).find("dataset-lifetime") != -1 + self.check_session( + committed=[FakeRow(id=1, key="dataset-lifetime", value="1")], rolledback=1 + ) + + def test_construct_null(self): + """Test server config parameter constructor""" + with pytest.raises(ServerConfigNullKey): + self.session.raise_on_commit = IntegrityError( + statement="", params="", orig=FakeDBOrig("NOT NULL constraint") + ) + ServerConfig.create(key="dataset-lifetime", value=2) + self.check_session( + rolledback=1, + ) + + def test_get(self): + """Test that we can retrieve what we set""" + config = ServerConfig.create(key="dataset-lifetime", value="2") + result = ServerConfig.get(key="dataset-lifetime") + assert config == result + self.check_session( + committed=[FakeRow(id=1, key="dataset-lifetime", value="2")], queries=1 + ) + + def test_update(self): + """Test that we can update an existing configuration setting""" + config = ServerConfig.create(key="dataset-lifetime", value="2") + self.check_session(committed=[FakeRow(id=1, key="dataset-lifetime", value="2")]) + config.value = "5" + config.update() + self.check_session(committed=[FakeRow(id=1, key="dataset-lifetime", value="5")]) + result = ServerConfig.get(key="dataset-lifetime") + assert config == result + self.check_session( + committed=[FakeRow(id=1, key="dataset-lifetime", value="5")], queries=1 + ) + + def test_set(self): + """ + Test that we can use set both to create and update configuration + settings + """ + config = ServerConfig.set(key="dataset-lifetime", value="40 days") + result = ServerConfig.get(key="dataset-lifetime") + assert config == result + assert config.value == "40" + ServerConfig.set(key="dataset-lifetime", value=120) + result = ServerConfig.get(key="dataset-lifetime") + assert result.value == "120" + + # NOTE: each `get` does a query, plus each `set` checks whether the + # key already exists: 2 get + 2 set == 4 queries + self.check_session( + committed=[FakeRow(id=1, key="dataset-lifetime", value="120")], queries=4 + ) + + def test_missing(self): + """Check that 'create' complains about a missing key""" + with pytest.raises(ServerConfigMissingKey): + ServerConfig.create(key=None, value=None) + + def test_get_all_default(self): + """ + Test get_all when no values have been set. It should return all the + defined keys, but with None values. + """ + assert ServerConfig.get_all() == { + "dataset-lifetime": "3650", + "server-state": {"status": "enabled"}, + "server-banner": None, + } + + def test_get_all(self): + """ + Set values and make sure they're all reported correctly + """ + ServerConfig.create(key="dataset-lifetime", value="2") + ServerConfig.create(key="server-state", value={"status": "enabled"}) + ServerConfig.create(key="server-banner", value={"message": "Mine"}) + assert ServerConfig.get_all() == { + "dataset-lifetime": "2", + "server-state": {"status": "enabled"}, + "server-banner": {"message": "Mine"}, + } + + @pytest.mark.parametrize( + "value,expected", + [ + (2, "2"), + ("4", "4"), + ("999999999", "999999999"), + ("10 days", "10"), + ("1day", "1"), + ("1 days", "1"), + ("1 day", "1"), + ], + ) + def test_lifetimes(self, value, expected): + """ + Test some lifetimes that should be OK + """ + config = ServerConfig.create(key="dataset-lifetime", value=value) + assert config.value == expected + + @pytest.mark.parametrize( + "value", + [ + "string", + ["list"], + ("tuple"), + {"dict": "ionary"}, + "1d", + "1da", + "9999999999", + "1 daysies", + "1 days, 10:15:20", + ], + ) + def test_bad_lifetimes(self, value): + """ + Test some lifetime values that aren't legal + """ + with pytest.raises(ServerConfigBadValue) as exc: + ServerConfig.create(key="dataset-lifetime", value=value) + assert exc.value.value == value + + @pytest.mark.parametrize( + "key,value", + [ + ("server-state", {"status": "enabled"}), + ("server-state", {"status": "enabled", "message": "All OK"}), + ( + "server-state", + { + "status": "disabled", + "message": "Not so good", + "up_again": "tomorrow", + }, + ), + ( + "server-state", + { + "status": "readonly", + "message": "Look but don't touch", + "reason": "rebuilding", + }, + ), + ( + "server-banner", + {"message": "Perf & Scale Pbench server", "contact": "Call Pete"}, + ), + ( + "server-banner", + {"message": "You should see this", "url": "http://nowhere"}, + ), + ], + ) + def test_create_state_and_banner(self, key, value): + """Test server state and banner setting""" + config = ServerConfig.create(key=key, value=value) + assert config.key == key + assert config.value == value + + @pytest.mark.parametrize( + "value", + [ + None, + 1, + 4.000, + True, + "yes", + ["a", "b"], + {"banner": "Must have 'status' key"}, + {"status": "nonsense", "message": "This uses a bad 'status' value"}, + {"status": "disabled", "banner": "'disabled' requires a message'"}, + ], + ) + def test_bad_state(self, value): + """ + A "server-state" value must include at least "status" and if the value + isn't "enabled" must also contain "message" + """ + with pytest.raises(ServerConfigBadValue) as exc: + ServerConfig.create(key="server-state", value=value) + assert exc.value.value == value + + @pytest.mark.parametrize("value", [1, True, "yes", ["a", "b"], {"banner": "xyzzy"}]) + def test_bad_banner(self, value): + """ + A "server-banner" value must include at least "message" + """ + with pytest.raises(ServerConfigBadValue) as exc: + ServerConfig.create(key="server-banner", value=value) + assert exc.value.value == value diff --git a/lib/pbench/test/unit/server/test_authorization.py b/lib/pbench/test/unit/server/test_authorization.py index 3e5d680e55..d61afbddec 100644 --- a/lib/pbench/test/unit/server/test_authorization.py +++ b/lib/pbench/test/unit/server/test_authorization.py @@ -4,7 +4,9 @@ from pbench.server.api.auth import Auth from pbench.server.api.resources import ( + API_AUTHORIZATION, API_METHOD, + ApiAuthorization, ApiBase, API_OPERATION, ApiSchema, @@ -66,7 +68,11 @@ def test_allowed_admin( self, apibase, server_config, create_drb_user, current_user_admin, ask ): user = self.get_user_id(ask["user"]) - apibase._check_authorization(user, ask["access"], ask["role"]) + apibase._check_authorization( + ApiAuthorization( + API_AUTHORIZATION.USER_ACCESS, ask["role"], user, ask["access"] + ) + ) @pytest.mark.parametrize( "ask", @@ -81,7 +87,11 @@ def test_disallowed_admin(self, apibase, server_config, current_user_admin, ask) user = self.get_user_id(ask["user"]) access = ask["access"] with pytest.raises(UnauthorizedAccess) as exc: - apibase._check_authorization(user, access, ask["role"]) + apibase._check_authorization( + ApiAuthorization( + API_AUTHORIZATION.USER_ACCESS, ask["role"], user, access + ) + ) assert exc.value.owner == (ask["user"] if ask["user"] else "none") assert exc.value.user == current_user_admin @@ -109,7 +119,11 @@ def test_allowed_auth( ask, ): user = self.get_user_id(ask["user"]) - apibase._check_authorization(user, ask["access"], ask["role"]) + apibase._check_authorization( + ApiAuthorization( + API_AUTHORIZATION.USER_ACCESS, ask["role"], user, ask["access"] + ) + ) @pytest.mark.parametrize( "ask", @@ -131,7 +145,11 @@ def test_disallowed_auth( user = self.get_user_id(ask["user"]) access = ask["access"] with pytest.raises(UnauthorizedAccess) as exc: - apibase._check_authorization(user, access, ask["role"]) + apibase._check_authorization( + ApiAuthorization( + API_AUTHORIZATION.USER_ACCESS, ask["role"], user, access + ) + ) assert exc.value.owner == (ask["user"] if ask["user"] else "none") assert exc.value.user == current_user_drb @@ -153,7 +171,11 @@ def test_allowed_noauth( ask, ): user = self.get_user_id(ask["user"]) - apibase._check_authorization(user, ask["access"], ask["role"]) + apibase._check_authorization( + ApiAuthorization( + API_AUTHORIZATION.USER_ACCESS, ask["role"], user, ask["access"] + ) + ) @pytest.mark.parametrize( "ask", @@ -171,6 +193,35 @@ def test_disallowed_noauth( user = self.get_user_id(ask["user"]) access = ask["access"] with pytest.raises(UnauthorizedAccess) as exc: - apibase._check_authorization(user, access, ask["role"]) + apibase._check_authorization( + ApiAuthorization( + API_AUTHORIZATION.USER_ACCESS, ask["role"], user, access + ) + ) assert exc.value.owner == (ask["user"] if ask["user"] else "none") assert exc.value.user is None + + def test_admin_unauth(self, apibase, server_config, current_user_none): + with pytest.raises(UnauthorizedAccess) as exc: + apibase._check_authorization( + ApiAuthorization(API_AUTHORIZATION.ADMIN, API_OPERATION.CREATE) + ) + assert ( + str(exc.value) + == "Unauthenticated client is not authorized to CREATE a server administrative resource" + ) + + def test_admin_user(self, apibase, server_config, current_user_drb): + with pytest.raises(UnauthorizedAccess) as exc: + apibase._check_authorization( + ApiAuthorization(API_AUTHORIZATION.ADMIN, API_OPERATION.CREATE) + ) + assert ( + str(exc.value) + == "User drb is not authorized to CREATE a server administrative resource" + ) + + def test_admin_admin(self, apibase, server_config, current_user_admin): + apibase._check_authorization( + ApiAuthorization(API_AUTHORIZATION.ADMIN, API_OPERATION.CREATE) + ) diff --git a/lib/pbench/test/unit/server/test_endpoint_configure.py b/lib/pbench/test/unit/server/test_endpoint_configure.py index a2eae93af8..c86839b430 100644 --- a/lib/pbench/test/unit/server/test_endpoint_configure.py +++ b/lib/pbench/test/unit/server/test_endpoint_configure.py @@ -59,6 +59,7 @@ def check_config(self, client, server_config, host, my_headers={}): "login": f"{uri}/login", "logout": f"{uri}/logout", "register": f"{uri}/register", + "server_configuration": f"{uri}/server/configuration", "upload": f"{uri}/upload", "user": f"{uri}/user", }, @@ -111,6 +112,10 @@ def check_config(self, client, server_config, host, my_headers={}): "login": {"template": f"{uri}/login", "params": {}}, "logout": {"template": f"{uri}/logout", "params": {}}, "register": {"template": f"{uri}/register", "params": {}}, + "server_configuration": { + "template": f"{uri}/server/configuration/{{key}}", + "params": {"key": {"type": "string"}}, + }, "upload": { "template": f"{uri}/upload/{{filename}}", "params": {"filename": {"type": "string"}}, diff --git a/lib/pbench/test/unit/server/test_server_configuration.py b/lib/pbench/test/unit/server/test_server_configuration.py new file mode 100644 index 0000000000..52f4672106 --- /dev/null +++ b/lib/pbench/test/unit/server/test_server_configuration.py @@ -0,0 +1,250 @@ +from http import HTTPStatus +import logging + +import pytest +import requests +from pbench.server.api.resources import APIAbort, ApiParams +from pbench.server.api.resources.server_configuration import ServerConfiguration + + +class TestServerConfiguration: + @pytest.fixture() + def query_get(self, client, server_config): + """ + Helper fixture to perform the API query and validate an expected + return status. + + Args: + client: Flask test API client fixture + server_config: Pbench config fixture + """ + + def query_api( + key: str, expected_status: HTTPStatus = HTTPStatus.OK + ) -> requests.Response: + k = "" if key is None else f"/{key}" + response = client.get(f"{server_config.rest_uri}/server/configuration{k}") + assert response.status_code == expected_status + return response + + return query_api + + @pytest.fixture() + def query_put(self, client, server_config, pbench_admin_token): + """ + Helper fixture to perform the API query and validate an expected + return status. + + Args: + client: Flask test API client fixture + server_config: Pbench config fixture + pbench_admin_token: Provide an admin authentication token + """ + + def query_api( + key: str, + expected_status: HTTPStatus = HTTPStatus.OK, + token: str = pbench_admin_token, + **kwargs, + ) -> requests.Response: + k = f"/{key}" if key else "" + response = client.put( + f"{server_config.rest_uri}/server/configuration{k}", + headers={"authorization": f"bearer {token}"}, + **kwargs, + ) + assert response.status_code == expected_status + return response + + return query_api + + def test_get_bad_keys(self, query_get): + response = query_get("xyzzy", HTTPStatus.BAD_REQUEST) + assert response.json == { + "message": "Unrecognized keyword ['xyzzy'] given for parameter key; allowed keywords are ['dataset-lifetime', 'server-banner', 'server-state']" + } + + def test_get1(self, query_get): + response = query_get("dataset-lifetime") + assert response.json == {"dataset-lifetime": "3650"} + + @pytest.mark.parametrize("key", (None, "")) + def test_get_all(self, query_get, key): + """ + We use a trailing-slash-insensitive URI mapping so that both + /server/configuration and /server/configuration/ should be mapped to + "get all"; test that both paths work. + """ + response = query_get(key) + assert response.json == { + "dataset-lifetime": "3650", + "server-state": {"status": "enabled"}, + "server-banner": None, + } + + def test_put_bad_uri_key(self, server_config): + """ + A shape of things to come: one true unit test to confirm proper + handling of a condition that ought to be impossible through Flask + routing. + """ + put = ServerConfiguration(server_config, logging.getLogger("test")) + with pytest.raises(APIAbort, match=r"Found URI parameters \['foo', 'plugh'\]"): + put._put_key(ApiParams(uri={"plugh": "xyzzy", "foo": "bar"})) + + def test_put_missing_value(self, query_put): + """ + Test behavior when JSON payload does not contain all required keys. + """ + response = query_put( + key="dataset-lifetime", expected_status=HTTPStatus.BAD_REQUEST + ) + assert ( + response.json.get("message") + == "No value found for key system configuration key 'dataset-lifetime'" + ) + + def test_put_bad_key(self, query_put): + response = query_put(key="fookey", expected_status=HTTPStatus.BAD_REQUEST) + assert response.json == { + "message": "Unrecognized keyword ['fookey'] given for parameter key; allowed keywords are ['dataset-lifetime', 'server-banner', 'server-state']" + } + + def test_put_bad_keys(self, query_put): + response = query_put( + key=None, + json={"fookey": "bar"}, + expected_status=HTTPStatus.BAD_REQUEST, + ) + assert response.json == { + "message": "Unrecognized configuration parameters ['fookey'] specified: valid parameters are ['dataset-lifetime', 'server-banner', 'server-state']" + } + + @pytest.mark.parametrize( + "key,value", + [ + ("dataset-lifetime", "14 years"), + ("server-banner", {"banner": None}), + ("server-state", "running"), + ("server-state", {"status": "disabled"}), + ], + ) + def test_bad_value(self, query_put, key, value): + response = query_put( + key=key, expected_status=HTTPStatus.BAD_REQUEST, json={"value": value} + ) + assert ( + f"Unsupported value for configuration key '{key}'" + in response.json["message"] + ) + + def test_put_redundant_value(self, query_put): + response = query_put( + key="dataset-lifetime", + query_string={"value": "2"}, + json={"value": "5", "dataset-lifetime": "4"}, + expected_status=HTTPStatus.BAD_REQUEST, + ) + assert ( + response.json["message"] + == "Redundant parameters specified in the JSON request body: ['dataset-lifetime', 'value']" + ) + + def test_put_param(self, query_put): + response = query_put(key="dataset-lifetime", query_string={"value": "2"}) + assert response.json == {"dataset-lifetime": "2"} + + def test_put_value(self, query_put): + response = query_put(key="dataset-lifetime", json={"value": "2"}) + assert response.json == {"dataset-lifetime": "2"} + + def test_put_value_unauth(self, query_put): + response = query_put( + key="dataset-lifetime", + token="", + json={"value": "4 days"}, + expected_status=HTTPStatus.UNAUTHORIZED, + ) + assert response.json == { + "message": "Unauthenticated client is not authorized to UPDATE a server administrative resource" + } + + def test_put_value_user(self, query_put, pbench_token): + response = query_put( + key="dataset-lifetime", + token=pbench_token, + json={"value": "4 days"}, + expected_status=HTTPStatus.FORBIDDEN, + ) + assert response.json == { + "message": "User drb is not authorized to UPDATE a server administrative resource" + } + + def test_put_config(self, query_get, query_put): + response = query_put( + key=None, + json={ + "dataset-lifetime": "2", + "server-state": {"status": "enabled"}, + }, + ) + assert response.json == { + "dataset-lifetime": "2", + "server-state": {"status": "enabled"}, + } + response = query_get(None) + assert response.json == { + "dataset-lifetime": "2", + "server-state": {"status": "enabled"}, + "server-banner": None, + } + + def test_disable_api(self, server_config, client, query_put, create_drb_user): + query_put( + key="server-state", + json={ + "value": { + "status": "disabled", + "message": "Disabled for testing", + "contact": "test@example.com", + } + }, + ) + response = client.get(f"{server_config.rest_uri}/datasets/list") + assert response.status_code == HTTPStatus.SERVICE_UNAVAILABLE + assert response.json == { + "contact": "test@example.com", + "status": "disabled", + "message": "Disabled for testing", + } + + def test_disable_api_readonly( + self, server_config, client, query_put, pbench_token, more_datasets + ): + query_put( + key="server-state", + json={ + "value": { + "status": "readonly", + "message": "Limited for testing", + "contact": "test@example.com", + } + }, + ) + response = client.get( + f"{server_config.rest_uri}/datasets/list?owner=drb", + headers={"authorization": f"Bearer {pbench_token}"}, + ) + assert response.status_code == HTTPStatus.OK + assert response.json["total"] == 2 + response = client.put( + f"{server_config.rest_uri}/datasets/metadata/drb", + headers={"authorization": f"Bearer {pbench_token}"}, + json={"metadata": {"dataset.name": "Test"}}, + ) + assert response.status_code == HTTPStatus.SERVICE_UNAVAILABLE + assert response.json == { + "status": "readonly", + "message": "Limited for testing", + "contact": "test@example.com", + }