Merge "Rename admin-rule to authorization-rule"

16 files changed, 112 insertions, 64 deletions

diff --git a/doc/source/authentication.rst b/doc/source/authentication.rst
index e1746d65b..5175e8b9f 100644
--- a/doc/source/authentication.rst
+++ b/doc/source/authentication.rst
@@ -44,7 +44,7 @@ To enable tenant-scoped access to privileged actions, see the Zuul Web Server
 component's section.
 
 To set access rules for a tenant, see :ref:`the documentation about tenant
-definition <admin_rule_definition>`.
+definition <authz_rule_definition>`.
 
 Most of the time, only one authenticator will be needed in Zuul's configuration;
 namely the configuration matching a third party identity provider service like
diff --git a/doc/source/developer/model-changelog.rst b/doc/source/developer/model-changelog.rst
index d27ec8351..c6d8fedbd 100644
--- a/doc/source/developer/model-changelog.rst
+++ b/doc/source/developer/model-changelog.rst
@@ -93,3 +93,10 @@ Version 9
 :Prior Zuul version: 6.3.0
 :Description: Adds nodeset_alternatives and nodeset_index to frozen job.
               Removes nodset from frozen job.  Affects schedulers and executors.
+
+Version 10
+----------
+
+:Prior Zuul version: 6.4.0
+:Description: Renames admin_rules to authz_rules in unparsed abide.
+              Affects schedulers and web.
diff --git a/doc/source/examples/keycloak/etc_zuul/main.yaml b/doc/source/examples/keycloak/etc_zuul/main.yaml
index 9006399b6..0305fcc28 100644
--- a/doc/source/examples/keycloak/etc_zuul/main.yaml
+++ b/doc/source/examples/keycloak/etc_zuul/main.yaml
@@ -1,8 +1,8 @@
-- admin-rule:
+- authorization-rule:
     name: tenant-group
     conditions:
       - groups: "{tenant.name}-admin"
-- admin-rule:
+- authorization-rule:
     name: admin-user
     conditions:
       - preferred_username: admin
diff --git a/doc/source/tenants.rst b/doc/source/tenants.rst
index fbbb458a5..56d0e5666 100644
--- a/doc/source/tenants.rst
+++ b/doc/source/tenants.rst
@@ -16,14 +16,14 @@ A project may appear in more than one tenant; this may be useful if
 you wish to use common job definitions across multiple tenants.
 
 Actions normally available to the Zuul operator only can be performed by specific
-users on Zuul's REST API, if admin rules are listed for the tenant. Admin rules
+users on Zuul's REST API if admin rules are listed for the tenant. Authorization rules
 are also defined in the tenant configuration file.
 
 The tenant configuration file is specified by the
 :attr:`scheduler.tenant_config` setting in ``zuul.conf``.  It is a
 YAML file which, like other Zuul configuration files, is a list of
-configuration objects, though only two types of objects are supported:
-``tenant`` and ``admin-rule``.
+configuration objects, though only a few types of objects (described
+below) are supported.
 
 Alternatively the :attr:`scheduler.tenant_config_script`
 can be the path to an executable that will be executed and its stdout
@@ -398,11 +398,12 @@ configuration. Some examples of tenant definitions are:
 
    .. attr:: admin-rules
 
-      A list of access rules for the tenant. These rules are checked to grant
-      privileged actions to users at the tenant level, through Zuul's REST API.
+      A list of authorization rules to be checked in order to grant
+      administrative access to the tenant through Zuul's REST API and
+      web interface.
 
-      At least one rule in the list must match for the user to be allowed the
-      privileged action.
+      At least one rule in the list must match for the user to be allowed to
+      execute privileged actions.
 
       More information on tenant-scoped actions can be found in
       :ref:`authentication`.
@@ -474,16 +475,18 @@ An example definition looks similar to the normal semaphore object:
       The maximum number of running jobs which can use this semaphore.
 
 
-.. _admin_rule_definition:
+.. _authz_rule_definition:
 
-Access Rule
------------
+Authorization Rule
+------------------
 
-An access rule is a set of conditions the claims of a user's JWT must match
-in order to be allowed to perform protected actions at a tenant's level.
+An authorization rule is a set of conditions the claims of a user's
+JWT must match in order to be allowed to perform actions at a tenant's
+level.
 
-The protected actions available at tenant level are **autohold**, **enqueue**,
-**dequeue** or **promote**.
+When an authorization rule is included in the tenant's `admin-rules`,
+the protected actions available are **autohold**, **enqueue**,
+**dequeue** and **promote**.
 
 .. note::
 
@@ -491,11 +494,11 @@ The protected actions available at tenant level are **autohold**, **enqueue**,
    an authenticator configuration where `allow_authz_override` is set to true.
    See :ref:`authentication` for more details.
 
-Below are some examples of how access rules can be defined:
+Below are some examples of how authorization rules can be defined:
 
 .. code-block:: yaml
 
-   - admin-rule:
+   - authorization-rule:
        name: affiliate_or_admin
        conditions:
          - resources_access:
@@ -503,14 +506,17 @@ Below are some examples of how access rules can be defined:
                roles: "affiliate"
            iss: external_institution
          - resources_access.account.roles: "admin"
-   - admin-rule:
+   - authorization-rule:
        name: alice_or_bob
        conditions:
          - zuul_uid: alice
          - zuul_uid: bob
 
+Zuul previously used ``admin-rule`` for these definitions.  That form
+is still permitted for backwards compatibility, but is deprecated and
+will be removed in a future version of Zuul.
 
-.. attr:: admin-rule
+.. attr:: authorization-rule
 
    The following attributes are supported:
 
@@ -591,8 +597,8 @@ Below are some examples of how access rules can be defined:
          },
         }
 
-Access Rule Templating
-----------------------
+Authorization Rule Templating
+-----------------------------
 
 The special word "{tenant.name}" can be used in conditions' values. It will be automatically
 substituted for the relevant tenant when evaluating authorizations for a given
@@ -600,7 +606,7 @@ set of claims. For example, consider the following rule:
 
 .. code-block:: yaml
 
-   - admin-rule:
+   - authorization-rule:
        name: tenant_in_groups
        conditions:
          - groups: "{tenant.name}"
diff --git a/releasenotes/notes/authz-rule-55a6db54340f2e08.yaml b/releasenotes/notes/authz-rule-55a6db54340f2e08.yaml
new file mode 100644
index 000000000..d3b3fa5f1
--- /dev/null
+++ b/releasenotes/notes/authz-rule-55a6db54340f2e08.yaml
@@ -0,0 +1,24 @@
+---
+upgrade:
+  - |
+    In preparation for expanded access control features in the web
+    interface, and REST API, the ``admin-rule`` tenant configuration
+    object has been renamed to ``authorization-rule``.  When applied
+    to a tenant, the tenant attribute is still ``admin-rules`` since
+    it determines admin access to that tenant.  This change will allow
+    similar rules to be applied to non-admin level access in the
+    future.
+
+    Tenant configs should now follow this example:
+
+    .. code-block:: yaml
+
+       - authorization-rule:
+           name: example-rule
+       - tenant:
+           name: example-tenant
+           admin-rules:
+             - example-rule
+
+    The old form is still permitted for backwards compatability, but
+    will be removed in a later version of Zuul.
diff --git a/tests/fixtures/config/authorization/rules-templating/main.yaml b/tests/fixtures/config/authorization/rules-templating/main.yaml
index c621115a3..f1c85ea25 100644
--- a/tests/fixtures/config/authorization/rules-templating/main.yaml
+++ b/tests/fixtures/config/authorization/rules-templating/main.yaml
@@ -1,4 +1,4 @@
-- admin-rule:
+- authorization-rule:
     name: tenant-admin
     conditions:
       - groups: "{tenant.name}"
diff --git a/tests/fixtures/config/authorization/single-tenant/main.yaml b/tests/fixtures/config/authorization/single-tenant/main.yaml
index 136ecd266..52e1fbdd7 100644
--- a/tests/fixtures/config/authorization/single-tenant/main.yaml
+++ b/tests/fixtures/config/authorization/single-tenant/main.yaml
@@ -1,7 +1,8 @@
-- admin-rule:
+- authorization-rule:
     name: venkman_rule
     conditions:
       - zuul_uid: venkman
+# TODO: rename to authorization-rule; this tests backwards compat for now
 - admin-rule:
     name: columbia_rule
     conditions:
diff --git a/tests/fixtures/config/tenant-parser/authorizations-templating.yaml b/tests/fixtures/config/tenant-parser/authorizations-templating.yaml
index 2b30ea5ab..87b34fa50 100644
--- a/tests/fixtures/config/tenant-parser/authorizations-templating.yaml
+++ b/tests/fixtures/config/tenant-parser/authorizations-templating.yaml
@@ -1,8 +1,8 @@
-- admin-rule:
+- authorization-rule:
     name: tenant-admin
     conditions:
       - group: "{tenant.name}-admin"
-- admin-rule:
+- authorization-rule:
     name: tenant-admin-complex
     conditions:
       - path.to.group: "{tenant.name}-admin"
diff --git a/tests/fixtures/config/tenant-parser/authorizations.yaml b/tests/fixtures/config/tenant-parser/authorizations.yaml
index d84e7d8a6..442355b91 100644
--- a/tests/fixtures/config/tenant-parser/authorizations.yaml
+++ b/tests/fixtures/config/tenant-parser/authorizations.yaml
@@ -1,9 +1,9 @@
-- admin-rule:
+- authorization-rule:
     name: auth-rule-one
     conditions:
       - sub: venkman
       - sub: zeddemore
-- admin-rule:
+- authorization-rule:
     name: auth-rule-two
     conditions:
       - sub: gozer
diff --git a/tests/unit/test_configloader.py b/tests/unit/test_configloader.py
index 571df7550..159329294 100644
--- a/tests/unit/test_configloader.py
+++ b/tests/unit/test_configloader.py
@@ -46,7 +46,7 @@ class TestConfigLoader(ZuulTestCase):
             keystorage=sched.keystore)
         abide = Abide()
         loader.loadTPCs(abide, unparsed_abide)
-        loader.loadAdminRules(abide, unparsed_abide)
+        loader.loadAuthzRules(abide, unparsed_abide)
 
         for tenant_name in unparsed_abide.tenants:
             tlock = tenant_read_lock(self.zk_client, tenant_name)
@@ -886,7 +886,7 @@ class TestAuthorizationRuleParser(ZuulTestCase):
     tenant_config_file = 'config/tenant-parser/authorizations.yaml'
 
     def test_rules_are_loaded(self):
-        rules = self.scheds.first.sched.abide.admin_rules
+        rules = self.scheds.first.sched.abide.authz_rules
         self.assertTrue('auth-rule-one' in rules,
                         self.scheds.first.sched.abide)
         self.assertTrue('auth-rule-two' in rules,
@@ -1022,7 +1022,7 @@ class TestAuthorizationRuleParserWithTemplating(ZuulTestCase):
     tenant_config_file = 'config/tenant-parser/authorizations-templating.yaml'
 
     def test_rules_are_loaded(self):
-        rules = self.scheds.first.sched.abide.admin_rules
+        rules = self.scheds.first.sched.abide.authz_rules
         self.assertTrue('tenant-admin' in rules, self.scheds.first.sched.abide)
         self.assertTrue('tenant-admin-complex' in rules,
                         self.scheds.first.sched.abide)
@@ -1030,7 +1030,7 @@ class TestAuthorizationRuleParserWithTemplating(ZuulTestCase):
     def test_tenant_substitution(self):
         claims_1 = {'group': 'tenant-one-admin'}
         claims_2 = {'group': 'tenant-two-admin'}
-        rules = self.scheds.first.sched.abide.admin_rules
+        rules = self.scheds.first.sched.abide.authz_rules
         tenant_one = self.scheds.first.sched.abide.tenants.get('tenant-one')
         tenant_two = self.scheds.first.sched.abide.tenants.get('tenant-two')
         self.assertTrue(rules['tenant-admin'](claims_1, tenant_one))
@@ -1041,7 +1041,7 @@ class TestAuthorizationRuleParserWithTemplating(ZuulTestCase):
     def test_tenant_substitution_in_list(self):
         claims_1 = {'group': ['tenant-one-admin', 'some-other-tenant']}
         claims_2 = {'group': ['tenant-two-admin', 'some-other-tenant']}
-        rules = self.scheds.first.sched.abide.admin_rules
+        rules = self.scheds.first.sched.abide.authz_rules
         tenant_one = self.scheds.first.sched.abide.tenants.get('tenant-one')
         tenant_two = self.scheds.first.sched.abide.tenants.get('tenant-two')
         self.assertTrue(rules['tenant-admin'](claims_1, tenant_one))
@@ -1057,7 +1057,7 @@ class TestAuthorizationRuleParserWithTemplating(ZuulTestCase):
                 }
             }
         }
-        rules = self.scheds.first.sched.abide.admin_rules
+        rules = self.scheds.first.sched.abide.authz_rules
         tenant_one = self.scheds.first.sched.abide.tenants.get('tenant-one')
         tenant_two = self.scheds.first.sched.abide.tenants.get('tenant-two')
         self.assertTrue(not rules['tenant-admin-complex'](claims_2,
diff --git a/tests/unit/test_zk.py b/tests/unit/test_zk.py
index 237dfc8b5..34ca56d04 100644
--- a/tests/unit/test_zk.py
+++ b/tests/unit/test_zk.py
@@ -47,7 +47,7 @@ from zuul.zk.sharding import (
     NODE_BYTE_SIZE_LIMIT,
 )
 from zuul.zk.components import (
-    BaseComponent, ComponentRegistry, ExecutorComponent
+    BaseComponent, ComponentRegistry, ExecutorComponent, COMPONENT_REGISTRY
 )
 from tests.base import (
     BaseTestCase, HoldableExecutorApi, HoldableMergerApi,
@@ -74,6 +74,9 @@ class ZooKeeperBaseTestCase(BaseTestCase):
         self.addCleanup(self.zk_client.disconnect)
         self.zk_client.connect()
         self.component_registry = ComponentRegistry(self.zk_client)
+        # We don't have any other component to initialize the global
+        # registry in these tests, so we do it ourselves.
+        COMPONENT_REGISTRY.create(self.zk_client)
 
 
 class TestZookeeperClient(ZooKeeperBaseTestCase):
@@ -1347,7 +1350,7 @@ class TestSystemConfigCache(ZooKeeperBaseTestCase):
     def test_set_get(self):
         uac = model.UnparsedAbideConfig()
         uac.tenants = {"foo": "bar"}
-        uac.admin_rules = ["bar", "foo"]
+        uac.authz_rules = ["bar", "foo"]
         attrs = model.SystemAttributes.fromDict({
             "use_relative_priority": True,
             "max_hold_expiration": 7200,
@@ -1362,7 +1365,7 @@ class TestSystemConfigCache(ZooKeeperBaseTestCase):
         uac_cached, cached_attrs = self.config_cache.get()
         self.assertEqual(uac.uuid, uac_cached.uuid)
         self.assertEqual(uac.tenants, uac_cached.tenants)
-        self.assertEqual(uac.admin_rules, uac_cached.admin_rules)
+        self.assertEqual(uac.authz_rules, uac_cached.authz_rules)
         self.assertEqual(attrs, cached_attrs)
 
     def test_cache_empty(self):
diff --git a/zuul/configloader.py b/zuul/configloader.py
index 2b696a832..799402229 100644
--- a/zuul/configloader.py
+++ b/zuul/configloader.py
@@ -1466,11 +1466,9 @@ class AuthorizationRuleParser(object):
         self.schema = self.getSchema()
 
     def getSchema(self):
-
         authRule = {vs.Required('name'): str,
                     vs.Required('conditions'): to_list(dict)
                    }
-
         return vs.Schema(authRule)
 
     def fromYaml(self, conf):
@@ -1660,7 +1658,7 @@ class TenantParser(object):
             tenant.exclude_unprotected_branches = \
                 conf['exclude-unprotected-branches']
         if conf.get('admin-rules') is not None:
-            tenant.authorization_rules = conf['admin-rules']
+            tenant.admin_rules = conf['admin-rules']
         if conf.get('authentication-realm') is not None:
             tenant.default_auth_realm = conf['authentication-realm']
         if conf.get('semaphores') is not None:
@@ -2536,7 +2534,7 @@ class ConfigLoader(object):
         self.tenant_parser = TenantParser(
             connections, zk_client, scheduler, merger, keystorage,
             zuul_globals, statsd)
-        self.admin_rule_parser = AuthorizationRuleParser()
+        self.authz_rule_parser = AuthorizationRuleParser()
         self.global_semaphore_parser = GlobalSemaphoreParser()
 
     def expandConfigPath(self, config_path):
@@ -2588,11 +2586,11 @@ class ConfigLoader(object):
             self.tenant_parser.getSchema()(unparsed_abide.tenants[tenant_name])
         return unparsed_abide
 
-    def loadAdminRules(self, abide, unparsed_abide):
-        abide.admin_rules.clear()
-        for conf_admin_rule in unparsed_abide.admin_rules:
-            admin_rule = self.admin_rule_parser.fromYaml(conf_admin_rule)
-            abide.admin_rules[admin_rule.name] = admin_rule
+    def loadAuthzRules(self, abide, unparsed_abide):
+        abide.authz_rules.clear()
+        for conf_authz_rule in unparsed_abide.authz_rules:
+            authz_rule = self.authz_rule_parser.fromYaml(conf_authz_rule)
+            abide.authz_rules[authz_rule.name] = authz_rule
 
     def loadSemaphores(self, abide, unparsed_abide):
         abide.semaphores.clear()
diff --git a/zuul/model.py b/zuul/model.py
index cfc45d78f..9e0222104 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -7140,13 +7140,13 @@ class UnparsedAbideConfig(object):
         self.uuid = uuid4().hex
         self.ltime = -1
         self.tenants = {}
-        self.admin_rules = []
+        self.authz_rules = []
         self.semaphores = []
 
     def extend(self, conf):
         if isinstance(conf, UnparsedAbideConfig):
             self.tenants.update(conf.tenants)
-            self.admin_rules.extend(conf.admin_rules)
+            self.authz_rules.extend(conf.authz_rules)
             self.semaphores.extend(conf.semaphores)
             return
 
@@ -7164,20 +7164,27 @@ class UnparsedAbideConfig(object):
                     raise Exception("Duplicate configuration for "
                                     f"tenant {value['name']}")
                 self.tenants[value["name"]] = value
+            # TODO: remove deprecated "admin-rule"
             elif key == 'admin-rule':
-                self.admin_rules.append(value)
+                self.authz_rules.append(value)
+            elif key == 'authorization-rule':
+                self.authz_rules.append(value)
             elif key == 'global-semaphore':
                 self.semaphores.append(value)
             else:
                 raise ConfigItemUnknownError(item)
 
     def toDict(self):
-        return {
+        d = {
             "uuid": self.uuid,
             "tenants": self.tenants,
-            "admin_rules": self.admin_rules,
             "semaphores": self.semaphores,
         }
+        if (COMPONENT_REGISTRY.model_api < 10):
+            d["admin_rules"] = self.authz_rules
+        else:
+            d["authz_rules"] = self.authz_rules
+        return d
 
     @classmethod
     def fromDict(cls, data, ltime):
@@ -7185,7 +7192,9 @@ class UnparsedAbideConfig(object):
         unparsed_abide.uuid = data["uuid"]
         unparsed_abide.ltime = ltime
         unparsed_abide.tenants = data["tenants"]
-        unparsed_abide.admin_rules = data["admin_rules"]
+        unparsed_abide.authz_rules = data.get('authz_rules',
+                                              data.get('admin_rules',
+                                                       []))
         unparsed_abide.semaphores = data.get("semaphores", [])
         return unparsed_abide
 
@@ -7955,7 +7964,7 @@ class Tenant(object):
         # The per tenant default ansible version
         self.default_ansible_version = None
 
-        self.authorization_rules = []
+        self.admin_rules = []
         self.default_auth_realm = None
         self.global_semaphores = set()
 
@@ -8165,7 +8174,7 @@ class UnparsedBranchCache(object):
 
 class Abide(object):
     def __init__(self):
-        self.admin_rules = {}
+        self.authz_rules = {}
         self.semaphores = {}
         self.tenants = {}
         # tenant -> project -> list(tpcs)
diff --git a/zuul/model_api.py b/zuul/model_api.py
index 27d4a2b07..9542cf415 100644
--- a/zuul/model_api.py
+++ b/zuul/model_api.py
@@ -14,4 +14,4 @@
 
 # When making ZK schema changes, increment this and add a record to
 # docs/developer/model-changelog.rst
-MODEL_API = 9
+MODEL_API = 10
diff --git a/zuul/scheduler.py b/zuul/scheduler.py
index 84adaaed2..dc136586f 100644
--- a/zuul/scheduler.py
+++ b/zuul/scheduler.py
@@ -1315,7 +1315,7 @@ class Scheduler(threading.Thread):
 
             try:
                 abide = Abide()
-                loader.loadAdminRules(abide, unparsed_abide)
+                loader.loadAuthzRules(abide, unparsed_abide)
                 loader.loadSemaphores(abide, unparsed_abide)
                 loader.loadTPCs(abide, unparsed_abide)
                 for tenant_name in tenants_to_load:
@@ -1374,7 +1374,7 @@ class Scheduler(threading.Thread):
             for tenant_name in deleted_tenants:
                 self.abide.clearTPCs(tenant_name)
 
-            loader.loadAdminRules(self.abide, self.unparsed_abide)
+            loader.loadAuthzRules(self.abide, self.unparsed_abide)
             loader.loadSemaphores(self.abide, self.unparsed_abide)
             loader.loadTPCs(self.abide, self.unparsed_abide)
 
@@ -2046,7 +2046,7 @@ class Scheduler(threading.Thread):
                 tenant_config, from_script=script)
             self.system_config_cache.set(self.unparsed_abide, self.globals)
 
-            loader.loadAdminRules(self.abide, self.unparsed_abide)
+            loader.loadAuthzRules(self.abide, self.unparsed_abide)
             loader.loadSemaphores(self.abide, self.unparsed_abide)
             loader.loadTPCs(self.abide, self.unparsed_abide)
 
@@ -2067,7 +2067,7 @@ class Scheduler(threading.Thread):
             for tenant_name in deleted_tenants:
                 self.abide.clearTPCs(tenant_name)
 
-            loader.loadAdminRules(self.abide, self.unparsed_abide)
+            loader.loadAuthzRules(self.abide, self.unparsed_abide)
             loader.loadSemaphores(self.abide, self.unparsed_abide)
             loader.loadTPCs(self.abide, self.unparsed_abide)
 
diff --git a/zuul/web/__init__.py b/zuul/web/__init__.py
index 9e684b3f6..b686a67d4 100755
--- a/zuul/web/__init__.py
+++ b/zuul/web/__init__.py
@@ -871,8 +871,8 @@ class ZuulWebAPI(object):
             (isinstance(override, list) and tenant.name in override)):
             return True
 
-        for rule_name in tenant.authorization_rules:
-            rule = self.zuulweb.abide.admin_rules.get(rule_name)
+        for rule_name in tenant.admin_rules:
+            rule = self.zuulweb.abide.authz_rules.get(rule_name)
             if not rule:
                 self.log.error('Undefined rule "%s"', rule_name)
                 continue
@@ -2172,7 +2172,7 @@ class ZuulWeb(object):
         for tenant_name in deleted_tenants:
             self.abide.clearTPCs(tenant_name)
 
-        loader.loadAdminRules(self.abide, self.unparsed_abide)
+        loader.loadAuthzRules(self.abide, self.unparsed_abide)
         loader.loadSemaphores(self.abide, self.unparsed_abide)
         loader.loadTPCs(self.abide, self.unparsed_abide)