diff options
author | Tristan Van Berkom <tristan.van.berkom@gmail.com> | 2018-09-18 10:13:47 +0000 |
---|---|---|
committer | Tristan Van Berkom <tristan.van.berkom@gmail.com> | 2018-09-18 10:13:47 +0000 |
commit | 9c1847b95aa950abc639b216d4f2686b42813601 (patch) | |
tree | 89bdcb6983b9d95ad8d8689c4bbe836d0b2302e4 | |
parent | c8c51231481d2c881e5cffc2b400a95bb43184fa (diff) | |
parent | c82b5ac6df1c4dbbdc7a1f6fb7bfff9611b18556 (diff) | |
download | buildstream-9c1847b95aa950abc639b216d4f2686b42813601.tar.gz |
Merge branch 'tristan/fix-artifact-config-crash-1.2' into 'bst-1.2'
Fix artifact config crash (1.2)
See merge request BuildStream/buildstream!805
-rw-r--r-- | buildstream/_artifactcache/artifactcache.py | 12 | ||||
-rw-r--r-- | tests/artifactcache/config.py | 34 | ||||
-rw-r--r-- | tests/artifactcache/missing-certs/certificates/client.crt | 0 | ||||
-rw-r--r-- | tests/artifactcache/missing-certs/certificates/client.key | 0 | ||||
-rw-r--r-- | tests/artifactcache/missing-certs/element.bst | 1 |
5 files changed, 46 insertions, 1 deletions
diff --git a/buildstream/_artifactcache/artifactcache.py b/buildstream/_artifactcache/artifactcache.py index aa11d92a9..b0cb4f353 100644 --- a/buildstream/_artifactcache/artifactcache.py +++ b/buildstream/_artifactcache/artifactcache.py @@ -51,7 +51,7 @@ class ArtifactCacheSpec(namedtuple('ArtifactCacheSpec', 'url push server_cert cl url = _yaml.node_get(spec_node, str, 'url') push = _yaml.node_get(spec_node, bool, 'push', default_value=False) if not url: - provenance = _yaml.node_get_provenance(spec_node) + provenance = _yaml.node_get_provenance(spec_node, 'url') raise LoadError(LoadErrorReason.INVALID_DATA, "{}: empty artifact cache URL".format(provenance)) @@ -67,6 +67,16 @@ class ArtifactCacheSpec(namedtuple('ArtifactCacheSpec', 'url push server_cert cl if client_cert and basedir: client_cert = os.path.join(basedir, client_cert) + if client_key and not client_cert: + provenance = _yaml.node_get_provenance(spec_node, 'client-key') + raise LoadError(LoadErrorReason.INVALID_DATA, + "{}: 'client-key' was specified without 'client-cert'".format(provenance)) + + if client_cert and not client_key: + provenance = _yaml.node_get_provenance(spec_node, 'client-cert') + raise LoadError(LoadErrorReason.INVALID_DATA, + "{}: 'client-cert' was specified without 'client-key'".format(provenance)) + return ArtifactCacheSpec(url, push, server_cert, client_key, client_cert) diff --git a/tests/artifactcache/config.py b/tests/artifactcache/config.py index f59474708..8ab0ecee1 100644 --- a/tests/artifactcache/config.py +++ b/tests/artifactcache/config.py @@ -9,8 +9,12 @@ from buildstream._context import Context from buildstream._project import Project from buildstream.utils import _deduplicate from buildstream import _yaml +from buildstream._exceptions import ErrorDomain, LoadErrorReason +from tests.testutils.runcli import cli + +DATA_DIR = os.path.dirname(os.path.realpath(__file__)) cache1 = ArtifactCacheSpec(url='https://example.com/cache1', push=True) cache2 = ArtifactCacheSpec(url='https://example.com/cache2', push=False) cache3 = ArtifactCacheSpec(url='https://example.com/cache3', push=False) @@ -106,3 +110,33 @@ def test_artifact_cache_precedence(tmpdir, override_caches, project_caches, user # Verify that it was correctly read. expected_cache_specs = list(_deduplicate(itertools.chain(override_caches, project_caches, user_caches))) assert parsed_cache_specs == expected_cache_specs + + +# Assert that if either the client key or client cert is specified +# without specifying it's counterpart, we get a comprehensive LoadError +# instead of an unhandled exception. +@pytest.mark.datafiles(DATA_DIR) +@pytest.mark.parametrize('config_key, config_value', [ + ('client-cert', 'client.crt'), + ('client-key', 'client.key') +]) +def test_missing_certs(cli, datafiles, config_key, config_value): + project = os.path.join(datafiles.dirname, datafiles.basename, 'missing-certs') + + project_conf = { + 'name': 'test', + + 'artifacts': { + 'url': 'https://cache.example.com:12345', + 'push': 'true', + config_key: config_value + } + } + project_conf_file = os.path.join(project, 'project.conf') + _yaml.dump(project_conf, project_conf_file) + + # Use `pull` here to ensure we try to initialize the remotes, triggering the error + # + # This does not happen for a simple `bst show`. + result = cli.run(project=project, args=['pull', 'element.bst']) + result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA) diff --git a/tests/artifactcache/missing-certs/certificates/client.crt b/tests/artifactcache/missing-certs/certificates/client.crt new file mode 100644 index 000000000..e69de29bb --- /dev/null +++ b/tests/artifactcache/missing-certs/certificates/client.crt diff --git a/tests/artifactcache/missing-certs/certificates/client.key b/tests/artifactcache/missing-certs/certificates/client.key new file mode 100644 index 000000000..e69de29bb --- /dev/null +++ b/tests/artifactcache/missing-certs/certificates/client.key diff --git a/tests/artifactcache/missing-certs/element.bst b/tests/artifactcache/missing-certs/element.bst new file mode 100644 index 000000000..3c29b4ea1 --- /dev/null +++ b/tests/artifactcache/missing-certs/element.bst @@ -0,0 +1 @@ +kind: autotools |