Offload FrozenJob secrets

The following potential problems were observed with FrozenJob secrets:

1) They may be repetitive: since the FrozenJob contains
lists of playbooks and each playbook record has a copy of all the
secrets which should be used for that playbook, if a job has multiple
playbooks the secrets will be repeated for each job.  Consider a base
job with three playbooks: the base job's secrets will be included
three times.

2) They may be large: secrets in ZK are stored encrypted and suffer the
same size explosion that they do when encrypted into zuul.yaml files.

3) Take #1 and #2 together and we have the possibility of having FrozenJob
objects that are larger than 1MB which is a problem for ZK.

Address all three issues by offloading the secrets to a new ZK node if
they are large (using the existing JobData framework) and de-duplicate
them and refer to them by index.

There is no backwards compatability handling here, so the ZK state needs
to be deleted.

Change-Id: I32133e8dd0e933528381f1187d270142046ff08f
This commit is contained in:
James E. Blair 2022-01-06 13:06:05 -08:00
parent 02efa8fb28
commit 488c99dab3
3 changed files with 40 additions and 2 deletions

View File

@ -0,0 +1,9 @@
---
upgrade:
- |
Due to a change in the ZooKeeper data format, the following
upgrade procedure is required:
* Stop all Zuul components
* Run ``zuul delete-state``
* Start all Zuul components

View File

@ -2075,14 +2075,15 @@ class AnsibleJob(object):
The output dictionary simply has decrypted data as its value.
:param dict secrets: The encrypted secrets dictionary from the
:param dict secrets: The playbook secrets dictionary from the
scheduler
:returns: A decrypted secrets dictionary
"""
ret = {}
for secret_name, frozen_secret in secrets.items():
for secret_name, secret_index in secrets.items():
frozen_secret = self.job.secrets[secret_index]
secret = zuul.model.Secret(secret_name, None)
secret.secret_data = yaml.encrypted_load(
frozen_secret['encrypted_data'])

View File

@ -1969,6 +1969,7 @@ class FrozenJob(zkobject.ZKObject):
'secret_parent_data',
'variables',
'parent_data',
'secrets',
)
def __repr__(self):
@ -2140,6 +2141,10 @@ class FrozenJob(zkobject.ZKObject):
def variables(self):
return self._getJobData('_variables')
@property
def secrets(self):
return self._getJobData('_secrets')
@property
def combined_variables(self):
"""
@ -2444,12 +2449,28 @@ class Job(ConfigObject):
role['project'] = role_project.name
return d
def _deduplicateSecrets(self, secrets, playbook):
# secrets is a list of secrets accumulated so far
# playbook is a frozen playbook from _freezePlaybook
# Cast to list so we can modify in place
for secret_key, secret_value in list(playbook['secrets'].items()):
if secret_value in secrets:
playbook['secrets'][secret_key] = secrets.index(secret_value)
else:
secrets.append(secret_value)
playbook['secrets'][secret_key] = len(secrets) - 1
def freezeJob(self, context, tenant, layout, item,
redact_secrets_and_keys):
buildset = item.current_build_set
kw = {}
attributes = (set(FrozenJob.attributes) |
set(FrozenJob.job_data_attributes))
# De-duplicate the secrets across all playbooks, store them in
# this array, and then refer to them by index.
attributes.discard('secrets')
secrets = []
for k in attributes:
# If this is a config object, it's frozen, so it's
# safe to shallow copy.
@ -2467,7 +2488,14 @@ class Job(ConfigObject):
v = [self._freezePlaybook(layout, item, pb,
redact_secrets_and_keys)
for pb in v if pb.source_context]
if not redact_secrets_and_keys:
# If we're redacting, don't de-duplicate so that
# it's clear that the value ("REDACTED") is
# redacted.
for pb in v:
self._deduplicateSecrets(secrets, pb)
kw[k] = v
kw['secrets'] = secrets
kw['affected_projects'] = self._getAffectedProjects(tenant)
kw['config_hash'] = self.getConfigHash(tenant)
# Don't add buildset to attributes since it's not serialized