Tristan Van Berkom pushed to branch tristan/fix-artifact-config-crash-1.2 at BuildStream / buildstream
Commits:
-
8f4aa67e
by Tristan Van Berkom at 2018-09-18T09:38:30Z
-
00f63b3a
by Tristan Van Berkom at 2018-09-18T09:38:35Z
5 changed files:
- buildstream/_artifactcache/artifactcache.py
- tests/artifactcache/config.py
- + tests/artifactcache/missing-certs/certificates/client.crt
- + tests/artifactcache/missing-certs/certificates/client.key
- + tests/artifactcache/missing-certs/element.bst
Changes:
| ... | ... | @@ -51,7 +51,7 @@ class ArtifactCacheSpec(namedtuple('ArtifactCacheSpec', 'url push server_cert cl |
| 51 | 51 |
url = _yaml.node_get(spec_node, str, 'url')
|
| 52 | 52 |
push = _yaml.node_get(spec_node, bool, 'push', default_value=False)
|
| 53 | 53 |
if not url:
|
| 54 |
- provenance = _yaml.node_get_provenance(spec_node)
|
|
| 54 |
+ provenance = _yaml.node_get_provenance(spec_node, 'url')
|
|
| 55 | 55 |
raise LoadError(LoadErrorReason.INVALID_DATA,
|
| 56 | 56 |
"{}: empty artifact cache URL".format(provenance))
|
| 57 | 57 |
|
| ... | ... | @@ -67,6 +67,16 @@ class ArtifactCacheSpec(namedtuple('ArtifactCacheSpec', 'url push server_cert cl |
| 67 | 67 |
if client_cert and basedir:
|
| 68 | 68 |
client_cert = os.path.join(basedir, client_cert)
|
| 69 | 69 |
|
| 70 |
+ if client_key and not client_cert:
|
|
| 71 |
+ provenance = _yaml.node_get_provenance(spec_node, 'client-key')
|
|
| 72 |
+ raise LoadError(LoadErrorReason.INVALID_DATA,
|
|
| 73 |
+ "{}: 'client-key' was specified without 'client-cert'".format(provenance))
|
|
| 74 |
+ |
|
| 75 |
+ if client_cert and not client_key:
|
|
| 76 |
+ provenance = _yaml.node_get_provenance(spec_node, 'client-cert')
|
|
| 77 |
+ raise LoadError(LoadErrorReason.INVALID_DATA,
|
|
| 78 |
+ "{}: 'client-cert' was specified without 'client-key'".format(provenance))
|
|
| 79 |
+ |
|
| 70 | 80 |
return ArtifactCacheSpec(url, push, server_cert, client_key, client_cert)
|
| 71 | 81 |
|
| 72 | 82 |
|
| ... | ... | @@ -9,8 +9,12 @@ from buildstream._context import Context |
| 9 | 9 |
from buildstream._project import Project
|
| 10 | 10 |
from buildstream.utils import _deduplicate
|
| 11 | 11 |
from buildstream import _yaml
|
| 12 |
+from buildstream._exceptions import ErrorDomain, LoadErrorReason
|
|
| 12 | 13 |
|
| 14 |
+from tests.testutils.runcli import cli
|
|
| 13 | 15 |
|
| 16 |
+ |
|
| 17 |
+DATA_DIR = os.path.dirname(os.path.realpath(__file__))
|
|
| 14 | 18 |
cache1 = ArtifactCacheSpec(url='https://example.com/cache1', push=True)
|
| 15 | 19 |
cache2 = ArtifactCacheSpec(url='https://example.com/cache2', push=False)
|
| 16 | 20 |
cache3 = ArtifactCacheSpec(url='https://example.com/cache3', push=False)
|
| ... | ... | @@ -106,3 +110,33 @@ def test_artifact_cache_precedence(tmpdir, override_caches, project_caches, user |
| 106 | 110 |
# Verify that it was correctly read.
|
| 107 | 111 |
expected_cache_specs = list(_deduplicate(itertools.chain(override_caches, project_caches, user_caches)))
|
| 108 | 112 |
assert parsed_cache_specs == expected_cache_specs
|
| 113 |
+ |
|
| 114 |
+ |
|
| 115 |
+# Assert that if either the client key or client cert is specified
|
|
| 116 |
+# without specifying it's counterpart, we get a comprehensive LoadError
|
|
| 117 |
+# instead of an unhandled exception.
|
|
| 118 |
+@pytest.mark.datafiles(DATA_DIR)
|
|
| 119 |
+@pytest.mark.parametrize('config_key, config_value', [
|
|
| 120 |
+ ('client-cert', 'client.crt'),
|
|
| 121 |
+ ('client-key', 'client.key')
|
|
| 122 |
+])
|
|
| 123 |
+def test_missing_certs(cli, datafiles, config_key, config_value):
|
|
| 124 |
+ project = os.path.join(datafiles.dirname, datafiles.basename, 'missing-certs')
|
|
| 125 |
+ |
|
| 126 |
+ project_conf = {
|
|
| 127 |
+ 'name': 'test',
|
|
| 128 |
+ |
|
| 129 |
+ 'artifacts': {
|
|
| 130 |
+ 'url': 'https://cache.example.com:12345',
|
|
| 131 |
+ 'push': 'true',
|
|
| 132 |
+ config_key: config_value
|
|
| 133 |
+ }
|
|
| 134 |
+ }
|
|
| 135 |
+ project_conf_file = os.path.join (project, 'project.conf')
|
|
| 136 |
+ _yaml.dump(project_conf, project_conf_file)
|
|
| 137 |
+ |
|
| 138 |
+ # Use `pull` here to ensure we try to initialize the remotes, triggering the error
|
|
| 139 |
+ #
|
|
| 140 |
+ # This does not happen for a simple `bst show`.
|
|
| 141 |
+ result = cli.run(project=project, args=['pull', 'element.bst'])
|
|
| 142 |
+ result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA)
|
| 1 |
+kind: autotools
|