zuul
/
zuul
Code Issues Proposed changes

Rename admin-rule to authorization-rule 

This is a preparatory step to add access-control for read-level
access to the API and web UI.  Because we will likely end up with
tenant config that looks like:

- tenant:
    name: example
    admin-rules: ['my-admin-rule']
    access-rules: ['my-read-only-rule']

It does not make sense for 'my-read-only-rule' to be defined as:

- admin-rule:
    name: read-only-rule

In other words, the current nomenclature conflates (new word:
nomenconflature) the idea of an abstract authorization rule and
what it authorizes.  The new name makes it more clear than an
authorization-rule can be used to authorize more than just admin
access.

Change-Id: I44da8060a804bc789720bd207c34d802a52b6975
This commit is contained in:
James E. Blair 2022-09-22 14:14:57 -07:00
parent 24d9076b76
commit 3a0eaa1ffe
16 changed files with 112 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
doc/source/authentication.rst
View File

@ -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

7
doc/source/developer/model-changelog.rst
View File

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

4
doc/source/examples/keycloak/etc_zuul/main.yaml
View File

@ -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

48
doc/source/tenants.rst
View File

@ -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}"

24
releasenotes/notes/authz-rule-55a6db54340f2e08.yaml Normal file
View File

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

2
tests/fixtures/config/authorization/rules-templating/main.yaml vendored
View File

@ -1,4 +1,4 @@
- admin-rule:
- authorization-rule:
name: tenant-admin
conditions:
- groups: "{tenant.name}"

3
tests/fixtures/config/authorization/single-tenant/main.yaml vendored
View File

@ -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:

4
tests/fixtures/config/tenant-parser/authorizations-templating.yaml vendored
View File

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

4
tests/fixtures/config/tenant-parser/authorizations.yaml vendored
View File

@ -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

12
tests/unit/test_configloader.py
View File

@ -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,

9
tests/unit/test_zk.py
View File

@ -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):

16
zuul/configloader.py
View File

@ -1460,11 +1460,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):
@ -1654,7 +1652,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:
@ -2530,7 +2528,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):
@ -2582,11 +2580,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()

25
zuul/model.py
View File

@ -7103,13 +7103,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
@ -7127,20 +7127,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):
@ -7148,7 +7155,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
@ -7918,7 +7927,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()
@ -8128,7 +8137,7 @@ class UnparsedBranchCache(object):
class Abide(object):
def __init__(self):
self.admin_rules = {}
self.authz_rules = {}
self.semaphores = {}
self.tenants = {}
# tenant -> project -> list(tpcs)

2
zuul/model_api.py
View File

@ -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

8
zuul/scheduler.py
View File

@ -1313,7 +1313,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:
@ -1372,7 +1372,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)
@ -2044,7 +2044,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)
@ -2065,7 +2065,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)

6
zuul/web/__init__.py
View File

@ -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
@ -2170,7 +2170,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)