Skip to content

Commit

Permalink
Merge 36000e4 into 30e4d56
Browse files Browse the repository at this point in the history
  • Loading branch information
JohanLorenzo committed Jan 29, 2018
2 parents 30e4d56 + 36000e4 commit e648862
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 37 deletions.
4 changes: 2 additions & 2 deletions examples/config.example.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
"work_dir": "/path/to/scriptworker/tmp/work",
"schema_file": "/path/to/shipitscript/shipitscript/data/pushapk_task_schema.json",

"ship_it_instance": {
"ship_it_instances": [{
"username": "some@user.name",
"password": "50mep@ssword",
"api_root": "http://ship-it.tld/"
},
}],

"verbose": true
}
5 changes: 5 additions & 0 deletions shipitscript/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,8 @@
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'])
5 changes: 2 additions & 3 deletions shipitscript/script.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from scriptworker.exceptions import ScriptWorkerTaskException

from shipitscript.ship_actions import mark_as_shipped
from shipitscript.task import validate_task_schema, validate_task_scope
from shipitscript.task import validate_task_schema, get_ship_it_instance_config_from_scope
from shipitscript.utils import load_json


Expand All @@ -24,9 +24,8 @@ async def async_main(context):
context.task = scriptworker.client.get_task(context.config)
log.info('Validating task definition...')
validate_task_schema(context)
validate_task_scope(context)
ship_it_instance_config = get_ship_it_instance_config_from_scope(context)

ship_it_instance_config = context.config['ship_it_instance']
release_name = context.task['payload']['release_name']
mark_as_shipped(ship_it_instance_config, release_name)
log.info('Done!')
Expand Down
51 changes: 30 additions & 21 deletions shipitscript/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@

import scriptworker.client

from shipitscript.exceptions import TaskVerificationError
from shipitscript.exceptions import TaskVerificationError, BadInstanceConfigurationError
from shipitscript.utils import get_single_item_from_list

log = logging.getLogger(__name__)

Expand All @@ -28,29 +29,37 @@ def validate_task_schema(context):
scriptworker.client.validate_json_schema(context.task, task_schema)


def validate_task_scope(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]
api_root_of_this_instance = context.config['ship_it_instance']['api_root']
api_root_of_this_instance = api_root_of_this_instance.rstrip('/')
if allowed_api_root != api_root_of_this_instance:
if allowed_api_root in PROTECTED_API_ROOTS or api_root_of_this_instance in PROTECTED_API_ROOTS:
instance_type = scope.split(':')[-1]
raise TaskVerificationError(
'This instance is now allowed to talk to the "{}" instance ({})'.format(instance_type, allowed_api_root)
)
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",

def _get_scope(task):
given_scopes = task['scopes']
scopes = [scope for scope in given_scopes if scope in VALID_SCOPES]
return get_single_item_from_list(
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),
)

number_of_scopes = len(scopes)
if number_of_scopes == 0:
raise TaskVerificationError(
'No valid scope found. Task must have one of these: {}. Given scopes: {}'.format(VALID_SCOPES, given_scopes)
)
elif number_of_scopes > 1:
raise TaskVerificationError('More than one valid scope given: {}'.format(scopes))

return scopes[0]
def _get_scope(task):
return get_single_item_from_list(
task['scopes'],
condition=lambda scope: scope in VALID_SCOPES,
ErrorClass=TaskVerificationError,
no_item_error_message='No valid scope found. Task must have one of these: {}'.format(VALID_SCOPES),
too_many_item_error_message='More than one valid scope given',
)
4 changes: 2 additions & 2 deletions shipitscript/test/integration/test_integration_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@
"schema_file": "{project_data_dir}/shipit_task_schema.json",
"verbose": true,
"ship_it_instance": {{
"ship_it_instances": [{{
"api_root": "http://some.ship-it.tld/api/root",
"timeout_in_seconds": 1,
"username": "some-username",
"password": "some-password"
}}
}}]
}}'''


Expand Down
26 changes: 18 additions & 8 deletions shipitscript/test/test_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,21 @@
from scriptworker.context import Context
from scriptworker.exceptions import ScriptWorkerTaskException

from shipitscript.exceptions import TaskVerificationError
from shipitscript.exceptions import TaskVerificationError, BadInstanceConfigurationError
from shipitscript.script import get_default_config
from shipitscript.task import validate_task_schema, validate_task_scope, _get_scope
from shipitscript.task import validate_task_schema, get_ship_it_instance_config_from_scope, _get_scope


@pytest.fixture
def context():
context = Context()
context.config = get_default_config()
context.config['ship_it_instance'] = {}
context.config['ship_it_instances'] = [{
'api_root': '',
'timeout_in_seconds': 1,
'username': 'some-username',
'password': 'some-password'
}]
context.task = {
'dependencies': ['someTaskId'],
'payload': {
Expand Down Expand Up @@ -47,15 +52,20 @@ def test_validate_task(context):
('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_validate_scope(context, api_root, scope, raises):
context.config['ship_it_instance']['api_root'] = api_root
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.task['scopes'] = [scope]

if raises:
with pytest.raises(TaskVerificationError):
validate_task_scope(context)
with pytest.raises(BadInstanceConfigurationError):
get_ship_it_instance_config_from_scope(context)
else:
validate_task_scope(context)
assert get_ship_it_instance_config_from_scope(context) == {
'api_root': api_root,
'timeout_in_seconds': 1,
'username': 'some-username',
'password': 'some-password'
}


@pytest.mark.parametrize('scopes, raises', (
Expand Down
36 changes: 35 additions & 1 deletion shipitscript/test/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import os
import pytest
import tempfile
import json

from shipitscript.utils import load_json
from shipitscript.utils import load_json, get_single_item_from_list


def test_load_json_from_file():
Expand All @@ -14,3 +15,36 @@ def test_load_json_from_file():
json.dump(json_object, f)

assert load_json(output_file) == json_object


@pytest.mark.parametrize('list_, condition, expected', (
(['a', 'b', 'c'], lambda item: item == 'b', 'b'),
([{'some_key': 1}, {'some_key': 2}, {'some_key': 3}], lambda item: item['some_key'] == 1, {'some_key': 1}),
))
def test_get_single_item_from_list(list_, condition, expected):
assert get_single_item_from_list(list_, condition) == expected


class SomeCustomError(Exception):
pass


@pytest.mark.parametrize(
'list_, condition, ErrorClass, no_item_error_message, too_many_item_error_message, has_all_params, expected_message',
(
(['a', 'b', 'c'], lambda item: item == 'z', SomeCustomError, 'NO ITEM', 'TOO MANY', True, "NO ITEM. Given: ['a', 'b', 'c']"),
(['a', 'b', 'b'], lambda item: item == 'b', SomeCustomError, 'NO ITEM', 'TOO MANY', True, "TOO MANY. Given: ['a', 'b', 'b']"),
(['a', 'b', 'c'], lambda _: False, ValueError, None, None, False, "No item matched condition. Given: ['a', 'b', 'c']"),
(['a', 'b', 'c'], lambda _: True, ValueError, None, None, False, "Too many items matched condition. Given: ['a', 'b', 'c']"),
)
)
def test_fail_(list_, condition, ErrorClass, no_item_error_message, too_many_item_error_message, has_all_params, expected_message):
with pytest.raises(ErrorClass) as exec_info:
if has_all_params:
get_single_item_from_list(
list_, condition, ErrorClass, no_item_error_message, too_many_item_error_message
)
else:
get_single_item_from_list(list_, condition)

assert str(exec_info.value) == expected_message
14 changes: 14 additions & 0 deletions shipitscript/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,17 @@
def load_json(path):
with open(path, "r") as fh:
return json.load(fh)


def get_single_item_from_list(
list_, condition, ErrorClass=ValueError, no_item_error_message='No item matched condition',
too_many_item_error_message='Too many items matched condition'
):
filtered_list = [item for item in list_ if condition(item)]
number_of_items_in_filtered_list = len(filtered_list)
if number_of_items_in_filtered_list == 0:
raise ErrorClass('{}. Given: {}'.format(no_item_error_message, list_))
elif number_of_items_in_filtered_list > 1:
raise ErrorClass('{}. Given: {}'.format(too_many_item_error_message, list_))

return filtered_list[0]

0 comments on commit e648862

Please sign in to comment.