zuul/zuul
Code Issues Proposed changes

Merge "Manage OIDC signing key rotation"

Browse Source
This commit is contained in:
Zuul
2025-05-15 00:00:38 +00:00
committed by Gerrit Code Review
parent 4704cb088e b3e4e038b7
commit a97e724b37
7 changed files with 92 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
tests/fixtures/config/multi-tenant/main-reconfig.yaml vendored
View File

@ -1,6 +1,7 @@
- tenant:
name: tenant-one
max-job-timeout: 1800
max-oidc-ttl: 300
allowed-reporters:
- gerrit
allowed-labels:

1
tests/fixtures/config/multi-tenant/main.yaml vendored
View File

@ -1,6 +1,7 @@
- tenant:
name: tenant-one
max-job-timeout: 1800
max-oidc-ttl: 300
allowed-reporters:
- gerrit
allowed-labels:

13
tests/unit/test_v3.py
View File

@ -5636,6 +5636,19 @@ class TestOIDCSigningKeys(ZuulTestCase):
encryption.serialize_rsa_private_key(private_key3))
self.assertEqual(version4, 1)
def test_oidc_key_rotation_without_old_key(self):
# Test that when there is no old key, it will create a new one
keystore = self.scheds.first.sched.keystore
algorithm = "RS256"
rotation_interval = 5
max_ttl = 2
keystore.rotateOidcSigningKeys(algorithm, rotation_interval, max_ttl)
with keystore.createZKContext() as context:
test_keys1 = OIDCSigningKeys.loadKeys(
context, algorithm)
self.assertEqual(len(test_keys1.keys), 1)
class TestValidateAllBroken(ZuulTestCase):
# Test we fail while validating all tenants with one broken tenant

3
tests/unit/test_zk.py
View File

@ -1604,6 +1604,9 @@ class TestSystemConfigCache(ZooKeeperBaseTestCase):
"default_ansible_version": "X",
"web_root": "/web/root",
"websocket_url": "/web/socket",
"oidc_signing_key_rotation_interval": 3600,
"oidc_supported_signing_algorithms": ["RS256", "ES256"],
"oidc_default_signing_algorithm": "RS256",
})
self.config_cache.set(uac, attrs)

3
zuul/configloader.py
View File

@ -1877,6 +1877,7 @@ class TenantParser(object):
'max-dependencies': int,
'max-nodes-per-job': int,
'max-job-timeout': int,
'max-oidc-ttl': int,
'source': self.validateTenantSources(),
'exclude-unprotected-branches': bool,
'exclude-locked-branches': bool,
@ -1920,6 +1921,8 @@ class TenantParser(object):
tenant.max_nodes_per_job = conf['max-nodes-per-job']
if conf.get('max-job-timeout') is not None:
tenant.max_job_timeout = int(conf['max-job-timeout'])
if conf.get('max-oidc-ttl') is not None:
tenant.max_oidc_ttl = int(conf['max-oidc-ttl'])
if conf.get('exclude-unprotected-branches') is not None:
tenant.exclude_unprotected_branches = \
conf['exclude-unprotected-branches']

49
zuul/model.py
View File

@ -8908,6 +8908,12 @@ class SystemAttributes:
all schedulers and will be synchronized via Zookeeper.
"""
_default_oidc_signing_key_rotation_interval = 60 * 60 * 24 * 7 # 1 week
# TODO: When more algorithms are supported, this should be
# fallback to all supported algorithms
_default_oidc_supported_signing_algorithms = ['RS256']
_default_oidc_default_signing_algorithm = "RS256"
def __init__(self):
self.use_relative_priority = False
self.max_hold_expiration = 0
@ -8915,6 +8921,9 @@ class SystemAttributes:
self.default_ansible_version = None
self.web_root = None
self.websocket_url = None
self.oidc_signing_key_rotation_interval = None
self.oidc_supported_signing_algorithms = None
self.oidc_default_signing_algorithm = None
def __eq__(self, other):
if not isinstance(other, self.__class__):
@ -8925,7 +8934,13 @@ class SystemAttributes:
and self.default_hold_expiration == other.default_hold_expiration
and self.default_ansible_version == other.default_ansible_version
and self.web_root == other.web_root
and self.websocket_url == other.websocket_url)
and self.websocket_url == other.websocket_url
and (self.oidc_signing_key_rotation_interval ==
other.oidc_signing_key_rotation_interval)
and (self.oidc_supported_signing_algorithms ==
other.oidc_supported_signing_algorithms)
and (self.oidc_default_signing_algorithm ==
other.oidc_default_signing_algorithm))
@classmethod
def fromConfig(cls, config):
@ -8963,6 +8978,20 @@ class SystemAttributes:
self.web_root = web_root
self.websocket_url = get_default(config, 'web', 'websocket_url', None)
self.oidc_signing_key_rotation_interval = int(get_default(
config, 'oidc', 'signing_key_rotation_interval',
self._default_oidc_signing_key_rotation_interval))
oidc_signing_algorithms_config = get_default(
config, 'oidc', 'supported_signing_algorithms', None)
self.oidc_supported_signing_algorithms = [
alg.strip() for alg in oidc_signing_algorithms_config.split(',')
] if oidc_signing_algorithms_config else \
self._default_oidc_supported_signing_algorithms
self.oidc_default_signing_algorithm = get_default(
config, 'oidc', 'default_signing_algorithm',
self._default_oidc_default_signing_algorithm)
def toDict(self):
attributes = {
@ -8972,6 +9001,12 @@ class SystemAttributes:
"default_ansible_version": self.default_ansible_version,
"web_root": self.web_root,
"websocket_url": self.websocket_url,
"oidc_signing_key_rotation_interval":
self.oidc_signing_key_rotation_interval,
"oidc_supported_signing_algorithms":
self.oidc_supported_signing_algorithms,
"oidc_default_signing_algorithm":
self.oidc_default_signing_algorithm,
}
if COMPONENT_REGISTRY.model_api < 34:
attributes["web_status_url"] = ""
@ -8986,6 +9021,17 @@ class SystemAttributes:
sys_attrs.default_ansible_version = data["default_ansible_version"]
sys_attrs.web_root = data["web_root"]
sys_attrs.websocket_url = data["websocket_url"]
# For the newly added system attributes, we need to use get()
# method to avoid KeyError in scheduler prime() method.
sys_attrs.oidc_signing_key_rotation_interval = data.get(
"oidc_signing_key_rotation_interval",
cls._default_oidc_signing_key_rotation_interval)
sys_attrs.oidc_supported_signing_algorithms = data.get(
"oidc_supported_signing_algorithms",
cls._default_oidc_supported_signing_algorithms)
sys_attrs.oidc_default_signing_algorithm = data.get(
"oidc_default_signing_algorithm",
cls._default_oidc_default_signing_algorithm)
return sys_attrs
@ -10122,6 +10168,7 @@ class Tenant(object):
self.name = name
self.max_nodes_per_job = 5
self.max_job_timeout = 10800
self.max_oidc_ttl = 10800
self.max_changes_per_pipeline = None
self.max_dependencies = None
self.exclude_unprotected_branches = False

23
zuul/scheduler.py
View File

@ -738,6 +738,7 @@ class Scheduler(threading.Thread):
self.log.debug("Starting general cleanup")
if self.general_cleanup_lock.acquire(blocking=False):
try:
self._runOidcSigningKeyRotation()
self._runConfigCacheCleanup()
self._runExecutorApiCleanup()
self._runMergerApiCleanup()
@ -753,6 +754,28 @@ class Scheduler(threading.Thread):
self._runNodeRequestCleanup()
self.log.debug("Finished general cleanup")
def _runOidcSigningKeyRotation(self):
try:
self.log.debug("Running OIDC signing keys rotation")
# Get the max_ttl over all the tenants
max_ttl = max(t.max_oidc_ttl for t in self.abide.tenants.values())
for algorithm in self.globals.oidc_supported_signing_algorithms:
try:
self.keystore.rotateOidcSigningKeys(
algorithm,
self.globals.oidc_signing_key_rotation_interval,
max_ttl
)
except exceptions.AlgorithmNotSupportedException:
self.log.warning(
"OIDC signing algorithm '%s' is not supported!",
algorithm)
self.log.debug("Finished OIDC signing keys rotation")
except Exception:
self.log.exception("Error in OIDC signing keys rotation:")
def _runConfigCacheCleanup(self):
# TODO: The only way the config_object_cache can get smaller
# is if the cleanup is run by a newly (re-)started scheduler.