Fix validate-tenants isolation

The validate-tenants scheduler subcommand is supposed to perform
complete tenant validation, and in doing so, it interacts with zk.
It is supposed to isolate itself from the production data, but
it appears to accidentally use the same unparsed config cache
as the production system.  This is mostly okay, but if the loading
paths are different, it could lead to writing cache errors into
the production file cache.

The error is caused because the ConfigLoader creates an internal
reference to the unparsed config cache and therefore ignores the
temporary/isolated unparsed config cache created by the scheduler.

To correct this, we will always pass the unparsed config cache
into the configloader.

Change-Id: I40bdbef4b767e19e99f58cbb3aa690bcb840fcd7
This commit is contained in:
James E. Blair 2024-01-31 14:40:32 -08:00
parent 0e6b023a5f
commit fb7d24b245
6 changed files with 35 additions and 26 deletions

View File

@ -42,7 +42,8 @@ class TestConfigLoader(ZuulTestCase):
ansible_manager = AnsibleManager(
default_version=zuul_globals.default_ansible_version)
loader = ConfigLoader(
sched.connections, self.zk_client, zuul_globals, sched.statsd,
sched.connections, self.zk_client, zuul_globals,
sched.unparsed_config_cache, sched.statsd,
keystorage=sched.keystore)
abide = Abide()
loader.loadTPCs(abide, unparsed_abide)

View File

@ -914,9 +914,9 @@ class Client(zuul.cmd.ZuulApp):
self.unparsed_config_cache = None
zuul_globals = SystemAttributes.fromConfig(self.config)
loader = configloader.ConfigLoader(
self.connections, None, zuul_globals)
sched = SchedulerConfig(self.config, self.connections)
loader = configloader.ConfigLoader(
self.connections, None, zuul_globals, None)
tenant_config, script = sched._checkTenantSourceConf(self.config)
try:
unparsed_abide = loader.readConfig(

View File

@ -42,7 +42,6 @@ from zuul.lib.logutil import get_annotated_logger
from zuul.lib.re2util import filter_allowed_disallowed, ZuulRegex
from zuul.lib.varnames import check_varnames
from zuul.zk.components import COMPONENT_REGISTRY
from zuul.zk.config_cache import UnparsedConfigCache
from zuul.zk.semaphore import SemaphoreHandler
ZUUL_CONF_ROOT = ('zuul.yaml', 'zuul.d', '.zuul.yaml', '.zuul.d')
@ -1819,7 +1818,7 @@ class ParseContext(object):
class TenantParser(object):
def __init__(self, connections, zk_client, scheduler, merger, keystorage,
zuul_globals, statsd):
zuul_globals, statsd, unparsed_config_cache):
self.log = logging.getLogger("zuul.TenantParser")
self.connections = connections
self.zk_client = zk_client
@ -1828,7 +1827,7 @@ class TenantParser(object):
self.keystorage = keystorage
self.globals = zuul_globals
self.statsd = statsd
self.unparsed_config_cache = UnparsedConfigCache(self.zk_client)
self.unparsed_config_cache = unparsed_config_cache
classes = vs.Any('pipeline', 'job', 'semaphore', 'project',
'project-template', 'nodeset', 'secret', 'queue')
@ -2857,8 +2856,9 @@ class TenantParser(object):
class ConfigLoader(object):
log = logging.getLogger("zuul.ConfigLoader")
def __init__(self, connections, zk_client, zuul_globals, statsd=None,
scheduler=None, merger=None, keystorage=None):
def __init__(self, connections, zk_client, zuul_globals,
unparsed_config_cache, statsd=None, scheduler=None,
merger=None, keystorage=None):
self.connections = connections
self.zk_client = zk_client
self.globals = zuul_globals
@ -2867,7 +2867,7 @@ class ConfigLoader(object):
self.keystorage = keystorage
self.tenant_parser = TenantParser(
connections, zk_client, scheduler, merger, keystorage,
zuul_globals, statsd)
zuul_globals, statsd, unparsed_config_cache)
self.authz_rule_parser = AuthorizationRuleParser()
self.global_semaphore_parser = GlobalSemaphoreParser()
self.api_root_parser = ApiRootParser()

View File

@ -1143,7 +1143,7 @@ class PipelineManager(metaclass=ABCMeta):
import zuul.configloader
loader = zuul.configloader.ConfigLoader(
self.sched.connections, self.sched.zk_client, self.sched.globals,
self.sched.statsd, self.sched)
self.sched.unparsed_config_cache, self.sched.statsd, self.sched)
log.debug("Loading dynamic layout")

View File

@ -1010,8 +1010,9 @@ class Scheduler(threading.Thread):
self.primeSystemConfig()
loader = configloader.ConfigLoader(
self.connections, self.zk_client, self.globals, self.statsd, self,
self.merger, self.keystore)
self.connections, self.zk_client, self.globals,
self.unparsed_config_cache, self.statsd,
self, self.merger, self.keystore)
new_tenants = (set(self.unparsed_abide.tenants)
- self.abide.tenants.keys())
@ -1312,7 +1313,8 @@ class Scheduler(threading.Thread):
def updateTenantLayout(self, log, tenant_name):
log.debug("Updating layout of tenant %s", tenant_name)
loader = configloader.ConfigLoader(
self.connections, self.zk_client, self.globals, self.statsd, self,
self.connections, self.zk_client, self.globals,
self.unparsed_config_cache, self.statsd, self,
self.merger, self.keystore)
# Since we are using the ZK 'locked' context manager (in order
# to have a non-blocking lock) with a threading lock, we need
@ -1396,8 +1398,14 @@ class Scheduler(threading.Thread):
self.log.info("Config validation beginning")
start = time.monotonic()
# Use a temporary config cache for the validation
validate_root = f"/zuul/validate/{uuid.uuid4().hex}"
self.unparsed_config_cache = UnparsedConfigCache(self.zk_client,
validate_root)
loader = configloader.ConfigLoader(
self.connections, self.zk_client, self.globals, self.statsd,
self.connections, self.zk_client, self.globals,
self.unparsed_config_cache, self.statsd,
self, self.merger, self.keystore)
tenant_config, script = self._checkTenantSourceConf(self.config)
unparsed_abide = loader.readConfig(
@ -1407,11 +1415,6 @@ class Scheduler(threading.Thread):
available_tenants = list(unparsed_abide.tenants)
tenants_to_load = tenants_to_validate or available_tenants
# Use a temporary config cache for the validation
validate_root = f"/zuul/validate/{uuid.uuid4().hex}"
self.unparsed_config_cache = UnparsedConfigCache(self.zk_client,
validate_root)
try:
abide = Abide()
loader.loadAuthzRules(abide, unparsed_abide)
@ -1457,7 +1460,8 @@ class Scheduler(threading.Thread):
with self.unparsed_abide_lock:
loader = configloader.ConfigLoader(
self.connections, self.zk_client, self.globals, self.statsd,
self.connections, self.zk_client, self.globals,
self.unparsed_config_cache, self.statsd,
self, self.merger, self.keystore)
tenant_config, script = self._checkTenantSourceConf(self.config)
old_unparsed_abide = self.unparsed_abide
@ -1585,7 +1589,8 @@ class Scheduler(threading.Thread):
branch_cache_min_ltimes[connection_name] = ltime
loader = configloader.ConfigLoader(
self.connections, self.zk_client, self.globals, self.statsd,
self.connections, self.zk_client, self.globals,
self.unparsed_config_cache, self.statsd,
self, self.merger, self.keystore)
loader.loadTPCs(self.abide, self.unparsed_abide,
[event.tenant_name])
@ -2201,7 +2206,8 @@ class Scheduler(threading.Thread):
# consistency and future-proofing.
with self.unparsed_abide_lock:
loader = configloader.ConfigLoader(
self.connections, self.zk_client, self.globals, self.statsd,
self.connections, self.zk_client, self.globals,
self.unparsed_config_cache, self.statsd,
self, self.merger, self.keystore)
tenant_config, script = self._checkTenantSourceConf(self.config)
self.unparsed_abide = loader.readConfig(
@ -2223,7 +2229,8 @@ class Scheduler(threading.Thread):
self.ansible_manager = AnsibleManager(
default_version=self.globals.default_ansible_version)
loader = configloader.ConfigLoader(
self.connections, self.zk_client, self.globals, self.statsd,
self.connections, self.zk_client, self.globals,
self.unparsed_config_cache, self.statsd,
self, self.merger, self.keystore)
tenant_names = set(self.abide.tenants)

View File

@ -64,7 +64,7 @@ from zuul.model import (
from zuul.version import get_version_string
from zuul.zk import ZooKeeperClient
from zuul.zk.components import COMPONENT_REGISTRY, WebComponent
from zuul.zk.config_cache import SystemConfigCache
from zuul.zk.config_cache import SystemConfigCache, UnparsedConfigCache
from zuul.zk.event_queues import (
TenantManagementEventQueue,
TenantTriggerEventQueue,
@ -2058,6 +2058,7 @@ class ZuulWeb(object):
self.zk_client,
self.system_config_cache_wake_event.set)
self.unparsed_config_cache = UnparsedConfigCache(self.zk_client)
self.keystore = KeyStorage(
self.zk_client, password=self._get_key_store_password())
self.globals = SystemAttributes.fromConfig(self.config)
@ -2411,7 +2412,7 @@ class ZuulWeb(object):
loader = ConfigLoader(
self.connections, self.zk_client, self.globals,
keystorage=self.keystore)
self.unparsed_config_cache, keystorage=self.keystore)
tenant_names = set(self.abide.tenants)
deleted_tenants = tenant_names.difference(
@ -2429,7 +2430,7 @@ class ZuulWeb(object):
self.log.debug("Updating layout state")
loader = ConfigLoader(
self.connections, self.zk_client, self.globals,
keystorage=self.keystore)
self.unparsed_config_cache, keystorage=self.keystore)
# We need to handle new and deleted tenants, so we need to process all
# tenants currently known and the new ones.