diff --git a/data/model/repo_mirror.py b/data/model/repo_mirror.py index a9824f3ab..4b9a03a87 100644 --- a/data/model/repo_mirror.py +++ b/data/model/repo_mirror.py @@ -466,17 +466,20 @@ def set_mirroring_robot(repository, robot): # -------------------- Mirroring Rules --------------------------# +def validate_rule(rule_type, rule_value): + if rule_type != RepoMirrorRuleType.TAG_GLOB_CSV: + raise ValidationError('validation failed: rule_type must be TAG_GLOB_CSV') + + if not rule_value or not isinstance(rule_value, list) or len(rule_value) < 1: + raise ValidationError('validation failed: rule_value for TAG_GLOB_CSV must be a list with at least one rule') + def create_rule(repository, rule_value, rule_type=RepoMirrorRuleType.TAG_GLOB_CSV, left_child=None, right_child=None): """ Create a new Rule for mirroring a Repository """ - if rule_type != RepoMirrorRuleType.TAG_GLOB_CSV: - raise ValidationError('validation failed: rule_type must be TAG_GLOB_CSV') - - if not isinstance(rule_value, list) or len(rule_value) < 1: - raise ValidationError('validation failed: rule_value for TAG_GLOB_CSV must be a list with at least one rule') + validate_rule(rule_type, rule_value) rule_kwargs = { 'repository': repository, @@ -509,11 +512,18 @@ def get_root_rule(repository): return None -def change_rule_value(rule, value): +def change_rule(repository, rule_type, rule_value): """ Update the value of an existing rule. """ + + validate_rule(rule_type, rule_value) + + mirrorRule = get_root_rule(repository) + if not mirrorRule: + raise ValidationError('validation failed: rule not found') + query = (RepoMirrorRule - .update(rule_value=value) - .where(RepoMirrorRule.id == rule.id)) + .update(rule_value=rule_value) + .where(RepoMirrorRule.id == mirrorRule.id)) return query.execute() diff --git a/endpoints/api/mirror.py b/endpoints/api/mirror.py index cac7f9caa..9c898c7f5 100644 --- a/endpoints/api/mirror.py +++ b/endpoints/api/mirror.py @@ -11,6 +11,7 @@ import features from auth.auth_context import get_authenticated_user from data import model +from data.database import RepoMirrorRuleType from endpoints.api import (RepositoryParamResource, nickname, path_param, require_repo_admin, resource, validate_json_request, define_json_response, show_if, format_date) @@ -53,13 +54,14 @@ common_properties = { 'type': 'object', 'description': 'Tag mirror rule', 'required': [ - 'rule_type', + 'rule_kind', 'rule_value' ], 'properties': { - 'rule_type': { + 'rule_kind': { 'type': 'string', - 'description': 'Rule type must be "TAG_GLOB_CSV"' + 'description': 'The kind of rule type', + 'enum': ['tag_glob_csv'], }, 'rule_value': { 'type': 'array', @@ -231,7 +233,7 @@ class RepoMirrorResource(RepositoryParamResource): 'sync_retries_remaining': mirror.sync_retries_remaining, 'sync_status': mirror.sync_status.name, 'root_rule': { - 'rule_type': 'TAG_GLOB_CSV', + 'rule_kind': 'tag_glob_csv', 'rule_value': rules }, 'robot_username': robot, @@ -368,6 +370,14 @@ class RepoMirrorResource(RepositoryParamResource): if model.repo_mirror.change_external_registry_config(repo, updates): track_and_log('repo_mirror_config_changed', wrap_repository(repo), changed='no_proxy', to=proxy_values['no_proxy']) + if 'root_rule' in values: + + if values['root_rule']['rule_kind'] != "tag_glob_csv": + raise ValidationError('validation failed: rule_kind must be "tag_glob_csv"') + + if model.repo_mirror.change_rule(repo, RepoMirrorRuleType.TAG_GLOB_CSV, values['root_rule']['rule_value']): + track_and_log('repo_mirror_config_changed', wrap_repository(repo), changed="mirror_rule", to=values['root_rule']['rule_value']) + return '', 201 def _setup_robot_for_mirroring(self, namespace_name, repo_name, robot_username): @@ -423,45 +433,3 @@ class RepoMirrorResource(RepositoryParamResource): if username is None: return None return username.decrypt() - - -@resource('/v1/repository//mirror/rules') -@show_if(features.REPO_MIRROR) -class ManageRepoMirrorRule(RepositoryParamResource): - """ - Operations to manage a single Repository Mirroring Rule. - TODO: At the moment, we are only dealing with a single rule associated with the mirror. - This should change to update the rule and address it using its UUID. - """ - schemas = { - 'MirrorRule': { - 'type': 'object', - 'description': 'A rule used to define how a repository is mirrored.', - 'required': ['root_rule'], - 'properties': { - 'root_rule': common_properties['root_rule'] - } - } - } - - @require_repo_admin - @nickname('changeRepoMirrorRule') - @validate_json_request('MirrorRule') - def put(self, namespace_name, repository_name): - """ - Update an existing RepoMirrorRule - """ - repo = model.repository.get_repository(namespace_name, repository_name) - if not repo: - raise NotFound() - - rule = model.repo_mirror.get_root_rule(repo) - if not rule: - return {'detail': 'The rule appears to be missing.'}, 400 - - data = request.get_json() - if model.repo_mirror.change_rule_value(rule, data['root_rule']['rule_value']): - track_and_log('repo_mirror_config_changed', wrap_repository(repo), changed="mirror_rule", to=data['root_rule']['rule_value']) - return 200 - else: - return {'detail': 'Unable to update rule.'}, 400 diff --git a/endpoints/api/test/test_mirror.py b/endpoints/api/test/test_mirror.py index 292c9b00e..8fcd9ef4a 100644 --- a/endpoints/api/test/test_mirror.py +++ b/endpoints/api/test/test_mirror.py @@ -58,7 +58,7 @@ def test_create_mirror_sets_permissions(existing_robot_permission, expected_perm 'sync_interval': 100, 'sync_start_date': '2019-08-20T17:51:00Z', 'root_rule': { - 'rule_type': 'TAG_GLOB_CSV', + 'rule_kind': 'tag_glob_csv', 'rule_value': ['latest','foo', 'bar'] }, 'robot_username': 'devtable+newmirrorbot', @@ -155,6 +155,11 @@ def test_get_mirror(client): ('verify_tls', None, 400), ('verify_tls', 'abc', 400), + ('root_rule', {'rule_kind': 'tag_glob_csv', 'rule_value': ['3.1', '3.1*']}, 201), + ('root_rule', {'rule_kind': 'tag_glob_csv'}, 400), + ('root_rule', {'rule_kind': 'tag_glob_csv', 'rule_value': []}, 400), + ('root_rule', {'rule_kind': 'incorrect', 'rule_value': ['3.1', '3.1*']}, 400), + ]) def test_change_config(key, value, expected_status, client): """ Verify that changing each attribute works as expected. """ diff --git a/endpoints/api/test/test_security.py b/endpoints/api/test/test_security.py index 9f93413a7..a2b18ef4c 100644 --- a/endpoints/api/test/test_security.py +++ b/endpoints/api/test/test_security.py @@ -1417,11 +1417,6 @@ SECURITY_TESTS = [ (RepositoryStateResource, 'PUT', {'repository': 'devtable/simple'}, None, 'devtable', 400), (RepositoryStateResource, 'PUT', {'repository': 'devtable/simple'}, None, 'freshuser', 403), (RepositoryStateResource, 'PUT', {'repository': 'devtable/simple'}, None, 'reader', 403), - - (ManageRepoMirrorRule, 'PUT', {'repository': 'devtable/simple'}, None, None, 401), - (ManageRepoMirrorRule, 'PUT', {'repository': 'devtable/simple'}, None, 'devtable', 400), - (ManageRepoMirrorRule, 'PUT', {'repository': 'devtable/simple'}, None, 'freshuser', 403), - (ManageRepoMirrorRule, 'PUT', {'repository': 'devtable/simple'}, None, 'reader', 403), ] @pytest.mark.parametrize('resource,method,params,body,identity,expected', SECURITY_TESTS) diff --git a/static/js/directives/repo-view/repo-panel-mirroring.js b/static/js/directives/repo-view/repo-panel-mirroring.js index 4c9459876..5076d2124 100644 --- a/static/js/directives/repo-view/repo-panel-mirroring.js +++ b/static/js/directives/repo-view/repo-panel-mirroring.js @@ -70,7 +70,7 @@ angular.module('quay').directive('repoPanelMirror', function () { }; } - vm.tags = resp.root_rule.rule_value || []; // TODO: Use RepoMirrorRule-specific endpoint + vm.tags = resp.root_rule.rule_value || []; // TODO: These are not consistently provided by the API. Correct that in the API. vm.verifyTLS = resp.external_registry_config.verify_tls; @@ -356,8 +356,16 @@ angular.module('quay').directive('repoPanelMirror', function () { * Update Tag-Rules */ vm.changeTagRules = function(data, callback) { - let csv = data.values.rule_value; - let patterns = csv.split(','); + let csv = data.values.rule_value, + patterns; + + // If already an array then the data has not changed + if (Array.isArray(csv)) { + patterns = csv; + } else { + patterns = csv.split(','); + } + patterns.map(s => s.trim()); // Trim excess whitespace patterns = Array.from(new Set(patterns)); // De-duplicate @@ -370,7 +378,7 @@ angular.module('quay').directive('repoPanelMirror', function () { data = { 'root_rule': { - 'rule_type': 'TAG_GLOB_CSV', + 'rule_kind': "tag_glob_csv", 'rule_value': patterns } } @@ -378,7 +386,7 @@ angular.module('quay').directive('repoPanelMirror', function () { let displayError = ApiService.errorDisplay('Could not change Tag Rules', callback); ApiService - .changeRepoMirrorRule(data, params) + .changeRepoMirrorConfig(data, params) .then(function(resp) { vm.getMirror(); callback(true); @@ -452,7 +460,7 @@ angular.module('quay').directive('repoPanelMirror', function () { } }, 'root_rule': { - 'rule_type': 'TAG_GLOB_CSV', + 'rule_kind': "tag_glob_csv", 'rule_value': patterns } } diff --git a/test/registry/registry_tests.py b/test/registry/registry_tests.py index ec4a6e5d5..56adb24ee 100644 --- a/test/registry/registry_tests.py +++ b/test/registry/registry_tests.py @@ -2246,7 +2246,7 @@ def test_repository_states(state, use_robot, create_mirror, robot_exists, expect 'sync_interval': 1000, 'sync_start_date': '2020-01-01T00:00:00Z', 'root_rule': { - 'rule_type': 'TAG_GLOB_CSV', + 'rule_kind': "tag_glob_csv", 'rule_value': ['latest', '1.3*', 'foo'] }, 'robot_username': robot_full_name,