[Notes] [Git][BuildStream/buildstream][tristan/fix-artifact-config-crash] 2 commits: _artifactcache/artifactcache.py: Error out gracefully when push remote is mal-specified



Title: GitLab

Tristan Van Berkom pushed to branch tristan/fix-artifact-config-crash at BuildStream / buildstream

Commits:

5 changed files:

Changes:

  • buildstream/_artifactcache/artifactcache.py
    ... ... @@ -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
     
    

  • tests/artifactcache/config.py
    ... ... @@ -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)

  • tests/artifactcache/missing-certs/certificates/client.crt

  • tests/artifactcache/missing-certs/certificates/client.key

  • tests/artifactcache/missing-certs/element.bst
    1
    +kind: autotools



  • [Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]