This is an automated email from the ASF dual-hosted git repository. tvb pushed a commit to branch tristan/change-remote-config in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit 55d15c4d4a94d19881076dc2a885d0f2d0e789d0 Author: Tristan van Berkom <[email protected]> AuthorDate: Fri Jan 22 22:35:26 2021 +0900 _remotespec.py: Modified format to put the certs in an "auth" dictionary. The main motivation behind this format breaking change is that it will significantly improve our ability to clearly document how authentication works in a central location. This also updates the tests/artifactcache/config.py test to adjust it's configuration to the new style. --- src/buildstream/_remotespec.py | 82 ++++++++++++++++++++++++++++-------------- tests/artifactcache/config.py | 24 ++++++------- 2 files changed, 66 insertions(+), 40 deletions(-) diff --git a/src/buildstream/_remotespec.py b/src/buildstream/_remotespec.py index 4279962..ae49c9b 100644 --- a/src/buildstream/_remotespec.py +++ b/src/buildstream/_remotespec.py @@ -17,7 +17,7 @@ # import os -from typing import Optional, cast +from typing import Optional, Tuple, List, cast from urllib.parse import urlparse import grpc from grpc import ChannelCredentials, Channel @@ -201,12 +201,14 @@ class RemoteSpec: def new_from_node( cls, spec_node: MappingNode, basedir: Optional[str] = None, *, remote_execution: bool = False ) -> "RemoteSpec": - valid_keys = ["url", "server-cert", "client-key", "client-cert", "instance-name"] - - if remote_execution: - remote_type = RemoteType.ENDPOINT - push = False - else: + server_cert: Optional[str] = None + client_key: Optional[str] = None + client_cert: Optional[str] = None + push: bool = False + remote_type: str = RemoteType.ENDPOINT + + valid_keys: List[str] = ["url", "instance-name", "auth"] + if not remote_execution: remote_type = cast(str, spec_node.get_enum("type", RemoteType, default=RemoteType.ALL)) push = spec_node.get_bool("push", default=False) valid_keys += ["push", "type"] @@ -224,40 +226,66 @@ class RemoteSpec: instance_name = spec_node.get_str("instance-name", default=None) - def parse_cert(key): - cert = spec_node.get_str(key, default=None) - if cert: - cert = os.path.expanduser(cert) + auth_node = spec_node.get_mapping("auth", None) + if auth_node: + server_cert, client_key, client_cert = cls._parse_auth(auth_node, basedir) - if basedir: - cert = os.path.join(basedir, cert) + return cls( + remote_type, + url, + push=push, + server_cert=server_cert, + client_key=client_key, + client_cert=client_cert, + instance_name=instance_name, + ) - return cert + # _parse_auth(): + # + # Parse the "auth" data + # + # Args: + # auth_node: The auth node + # basedir: The base directory which cert files are relative to, or None + # + # Returns: + # A 3 tuple containing the filenames for the server-cert, + # the client-key and the client-cert + # + @classmethod + def _parse_auth( + cls, auth_node: MappingNode, basedir: Optional[str] = None + ) -> Tuple[Optional[str], Optional[str], Optional[str]]: + + auth_keys = ["server-cert", "client-key", "client-cert"] + auth_values = {} + auth_node.validate_keys(auth_keys) + + for key in auth_keys: + value = auth_node.get_str(key, None) + if value: + value = os.path.expanduser(value) + if basedir: + value = os.path.join(basedir, value) + auth_values[key] = value - cert_keys = ("server-cert", "client-key", "client-cert") - server_cert, client_key, client_cert = tuple(parse_cert(key) for key in cert_keys) + server_cert = auth_values["server-cert"] + client_key = auth_values["client-key"] + client_cert = auth_values["client-cert"] if client_key and not client_cert: - provenance = spec_node.get_node("client-key").get_provenance() + provenance = auth_node.get_node("client-key").get_provenance() raise LoadError( "{}: 'client-key' was specified without 'client-cert'".format(provenance), LoadErrorReason.INVALID_DATA ) if client_cert and not client_key: - provenance = spec_node.get_node("client-cert").get_provenance() + provenance = auth_node.get_node("client-cert").get_provenance() raise LoadError( "{}: 'client-cert' was specified without 'client-key'".format(provenance), LoadErrorReason.INVALID_DATA ) - return cls( - remote_type, - url, - push=push, - server_cert=server_cert, - client_key=client_key, - client_cert=client_cert, - instance_name=instance_name, - ) + return server_cert, client_key, client_cert # _load_credential_files(): # diff --git a/tests/artifactcache/config.py b/tests/artifactcache/config.py index 89d3753..45682ca 100644 --- a/tests/artifactcache/config.py +++ b/tests/artifactcache/config.py @@ -148,7 +148,7 @@ def test_missing_certs(cli, datafiles, config_key, config_value): project_conf = { "name": "test", "min-version": "2.0", - "artifacts": {"url": "https://cache.example.com:12345";, "push": "true", config_key: config_value}, + "artifacts": {"url": "https://cache.example.com:12345";, "push": "true", "auth": {config_key: config_value}}, } project_conf_file = os.path.join(project, "project.conf") _yaml.roundtrip_dump(project_conf, project_conf_file) @@ -201,22 +201,20 @@ def test_only_one(cli, datafiles, override_caches, project_caches, user_caches): ( { "url": "http://localhost.test";, - "server-cert": "~/server.crt", - "client-cert": "~/client.crt", - "client-key": "~/client.key", + "auth": {"server-cert": "~/server.crt", "client-cert": "~/client.crt", "client-key": "~/client.key",}, }, [ { "url": "http://localhost.test";, - "server-cert": "~/server.crt", - "client-cert": "~/client.crt", - "client-key": "~/client.key", + "auth": {"server-cert": "~/server.crt", "client-cert": "~/client.crt", "client-key": "~/client.key",}, }, { "url": "http://localhost2.test";, - "server-cert": "~/server2.crt", - "client-cert": "~/client2.crt", - "client-key": "~/client2.key", + "auth": { + "server-cert": "~/server2.crt", + "client-cert": "~/client2.crt", + "client-key": "~/client2.key", + }, }, ], ), @@ -259,9 +257,9 @@ def test_paths_for_artifact_config_are_expanded(tmpdir, monkeypatch, artifacts_c RemoteType.ALL, config["url"], push=False, - server_cert=os.path.expanduser(config["server-cert"]), - client_cert=os.path.expanduser(config["client-cert"]), - client_key=os.path.expanduser(config["client-key"]), + server_cert=os.path.expanduser(config["auth"]["server-cert"]), + client_cert=os.path.expanduser(config["auth"]["client-cert"]), + client_key=os.path.expanduser(config["auth"]["client-key"]), ) for config in artifacts_config ]
