Merge pull request #25 from thomasmckay/12-mirror-api

PROJQUAY-12 - remove mirror/rules API

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:
+  validate_rule(rule_type, rule_value)
-    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')
 
   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)
+          .update(rule_value=rule_value)
-          .where(RepoMirrorRule.id == rule.id))
+          .where(RepoMirrorRule.id == mirrorRule.id))
   return query.execute()

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/<apirepopath: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

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. """

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)

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 csv = data.values.rule_value,
-        let patterns = csv.split(',');
+            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
            }
         }

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,