Remove hardcoded check on scopes

examples/config.example.json

@@ -2,11 +2,13 @@
     "work_dir": "/path/to/scriptworker/tmp/work",
     "schema_file": "/path/to/shipitscript/shipitscript/data/pushapk_task_schema.json",
 
-    "ship_it_instances": [{
-        "username": "some@user.name",
-        "password": "50mep@ssword",
-        "api_root": "http://ship-it.tld/"
-    }],
+    "ship_it_instances": {
+        "project:releng:ship-it:dev": {
+            "username": "some@user.name",
+            "password": "50mep@ssword",
+            "api_root": "http://ship-it.tld/"
+        }
+    },
 
     "verbose": true
 }

shipitscript/exceptions.py

@@ -5,8 +5,3 @@
 class TaskVerificationError(ScriptWorkerTaskException):
     def __init__(self, msg):
         super().__init__(msg, exit_code=STATUSES['malformed-payload'])
-
-
-class BadInstanceConfigurationError(ScriptWorkerTaskException):
-    def __init__(self, msg):
-        super().__init__(msg, exit_code=STATUSES['internal-error'])

shipitscript/task.py

@@ -3,23 +3,14 @@
 
 import scriptworker.client
 
-from shipitscript.exceptions import TaskVerificationError, BadInstanceConfigurationError
+from shipitscript.exceptions import TaskVerificationError
 from shipitscript.utils import get_single_item_from_sequence
 
 log = logging.getLogger(__name__)
 
-ALLOWED_API_ROOT_PER_VALID_SCOPE = {
-    'project:releng:ship-it:production': 'https://ship-it.mozilla.org',
-    'project:releng:ship-it:staging': 'https://ship-it-dev.allizom.org',
-    'project:releng:ship-it:dev': '*',
-}
 
-VALID_SCOPES = tuple(ALLOWED_API_ROOT_PER_VALID_SCOPE.keys())
-PROTECTED_API_ROOTS = tuple([
-    url
-    for url in ALLOWED_API_ROOT_PER_VALID_SCOPE.values()
-    if url != '*'
-])
+# TODO: Make this prefix a param of the instance config, when Thunderbird migrates this task
+_VALID_SCOPES_PREFIX = 'project:releng:ship-it:'
 
 
 def validate_task_schema(context):
@@ -31,36 +22,19 @@ def validate_task_schema(context):
 
 def get_ship_it_instance_config_from_scope(context):
     scope = _get_scope(context.task)
-    allowed_api_root = ALLOWED_API_ROOT_PER_VALID_SCOPE[scope]
     configured_instances = context.config['ship_it_instances']
-    instance_type = scope.split(':')[-1]
 
-    if allowed_api_root in PROTECTED_API_ROOTS:
-        def condition(instance):
-            return instance['api_root'].rstrip('/') == allowed_api_root
-        no_item_error_message = 'This instance is now allowed to talk to the "{}" instance ({})'.format(
-            instance_type, allowed_api_root
-        )
-    else:
-        def condition(instance):
-            return instance['api_root'].rstrip('/') not in PROTECTED_API_ROOTS
-        no_item_error_message = "Couldn't find an API root that is not a protected one",
-
-    return get_single_item_from_sequence(
-        configured_instances,
-        condition=condition,
-        ErrorClass=BadInstanceConfigurationError,
-        no_item_error_message=no_item_error_message,
-        too_many_item_error_message='Too many "{}" instances configured'.format(instance_type, allowed_api_root),
-        append_sequence_to_error_message=False
-    )
+    try:
+        return configured_instances[scope]
+    except KeyError:
+        raise TaskVerificationError('This worker is not configured to handle scope "{}"'.format(scope))
 
 
 def _get_scope(task):
     return get_single_item_from_sequence(
         task['scopes'],
-        condition=lambda scope: scope in VALID_SCOPES,
+        condition=lambda scope: scope.startswith(_VALID_SCOPES_PREFIX),
         ErrorClass=TaskVerificationError,
-        no_item_error_message='No valid scope found. Task must have one of these: {}'.format(VALID_SCOPES),
+        no_item_error_message='No valid scope found. Task must have a scope that starts with "{}"'.format(_VALID_SCOPES_PREFIX),
         too_many_item_error_message='More than one valid scope given',
     )

shipitscript/test/integration/test_integration_script.py

@@ -35,12 +35,14 @@
     "schema_file": "{project_data_dir}/shipit_task_schema.json",
     "verbose": true,
 
-    "ship_it_instances": [{{
-        "api_root": "http://some.ship-it.tld/api/root",
-        "timeout_in_seconds": 1,
-        "username": "some-username",
-        "password": "some-password"
-    }}]
+    "ship_it_instances": {{
+        "project:releng:ship-it:dev": {{
+            "api_root": "http://some.ship-it.tld/api/root",
+            "timeout_in_seconds": 1,
+            "username": "some-username",
+            "password": "some-password"
+        }}
+    }}
 }}'''
 
 

shipitscript/test/test_task.py

@@ -1,9 +1,10 @@
 import pytest
+import copy
 
 from scriptworker.context import Context
 from scriptworker.exceptions import ScriptWorkerTaskException
 
-from shipitscript.exceptions import TaskVerificationError, BadInstanceConfigurationError
+from shipitscript.exceptions import TaskVerificationError
 from shipitscript.script import get_default_config
 from shipitscript.task import validate_task_schema, get_ship_it_instance_config_from_scope, _get_scope
 
@@ -12,12 +13,14 @@
 def context():
     context = Context()
     context.config = get_default_config()
-    context.config['ship_it_instances'] = [{
-        'api_root': '',
-        'timeout_in_seconds': 1,
-        'username': 'some-username',
-        'password': 'some-password'
-    }]
+    context.config['ship_it_instances'] = {
+        'project:releng:ship-it:dev': {
+            'api_root': 'http://some-ship-it.url',
+            'timeout_in_seconds': 1,
+            'username': 'some-username',
+            'password': 'some-password'
+        }
+    }
     context.task = {
         'dependencies': ['someTaskId'],
         'payload': {
@@ -45,19 +48,14 @@ def test_validate_task(context):
     ('https://ship-it-dev.allizom.org/', 'project:releng:ship-it:staging', False),
     ('https://ship-it.mozilla.org', 'project:releng:ship-it:production', False),
     ('https://ship-it.mozilla.org/', 'project:releng:ship-it:production', False),
-
-    ('https://some-ship-it.url', 'project:releng:ship-it:production', True),
-    ('https://some-ship-it.url', 'project:releng:ship-it:staging', True),
-    # Dev scopes must not reach stage or prod
-    ('https://ship-it-dev.allizom.org', 'project:releng:ship-it:dev', True),
-    ('https://ship-it.mozilla.org', 'project:releng:ship-it:dev', True),
 ))
 def test_get_ship_it_instance_config_from_scope(context, api_root, scope, raises):
-    context.config['ship_it_instances'][0]['api_root'] = api_root
+    context.config['ship_it_instances'][scope] = copy.deepcopy(context.config['ship_it_instances']['project:releng:ship-it:dev'])
+    context.config['ship_it_instances'][scope]['api_root'] = api_root
     context.task['scopes'] = [scope]
 
     if raises:
-        with pytest.raises(BadInstanceConfigurationError):
+        with pytest.raises(TaskVerificationError):
             get_ship_it_instance_config_from_scope(context)
     else:
         assert get_ship_it_instance_config_from_scope(context) == {
@@ -68,12 +66,20 @@ def test_get_ship_it_instance_config_from_scope(context, api_root, scope, raises
         }
 
 
+@pytest.mark.parametrize('scope', (
+    'some:random:scope', 'project:releng:ship-it:staging', 'project:releng:ship-it:production',
+))
+def test_fail_get_ship_it_instance_config_from_scope(context, scope):
+    context.task['scopes'] = [scope]
+    with pytest.raises(TaskVerificationError):
+        get_ship_it_instance_config_from_scope(context)
+
+
 @pytest.mark.parametrize('scopes, raises', (
     (('project:releng:ship-it:dev',), False),
     (('project:releng:ship-it:staging',), False),
     (('project:releng:ship-it:production',), False),
     (('project:releng:ship-it:dev', 'project:releng:ship-it:production',), True),
-    (('project:releng:ship-it:unkown_scope',), True),
     (('some:random:scope',), True),
 ))
 def test_get_scope(scopes, raises):