From d13bc36c522fdac62028eb5cc9d0c851bbdd14e4 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Fri, 30 Jun 2017 13:11:37 -0500 Subject: [PATCH] Write secrets into their own file, not into inventory Publishing the inventory as part of publishing logs would be handy in debugging that everything is as one expects. However, secrets currently go into the inventory, which means publishing it is not safe. Write the secrets to their own file. Also, Return errors if users try to define zuul vars or secrets. To support that, we need to pass zuul vars as a top-level param, not inside of vars already. Change-Id: If58b89882a817ff219ed5f8faf2bde31cc8e1a6a --- tests/base.py | 2 +- tests/unit/test_github_driver.py | 2 +- tests/unit/test_scheduler.py | 2 +- zuul/executor/client.py | 6 ++++-- zuul/executor/server.py | 20 ++++++++++++++++++-- 5 files changed, 25 insertions(+), 7 deletions(-) diff --git a/tests/base.py b/tests/base.py index 1cc9999182..fb946385fe 100755 --- a/tests/base.py +++ b/tests/base.py @@ -1308,7 +1308,7 @@ class RecordingExecutorServer(zuul.executor.server.ExecutorServer): self.running_builds.append(build) self.job_builds[job.unique] = build args = json.loads(job.arguments) - args['vars']['zuul']['_test'] = dict(test_root=self._test_root) + args['zuul']['_test'] = dict(test_root=self._test_root) job.arguments = json.dumps(args) self.job_workers[job.unique] = RecordingAnsibleJob(self, job) self.job_workers[job.unique].run() diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index f360866c5e..0cfe3dad4a 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -46,7 +46,7 @@ class TestGithubDriver(ZuulTestCase): self.getJobFromHistory('project-test2').result) job = self.getJobFromHistory('project-test2') - zuulvars = job.parameters['vars']['zuul'] + zuulvars = job.parameters['zuul'] self.assertEqual(A.number, zuulvars['change']) self.assertEqual(A.head_sha, zuulvars['patchset']) self.assertEqual(1, len(A.comments)) diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index 61bf9f87a4..ac2a779cfc 100755 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -2757,7 +2757,7 @@ class TestScheduler(ZuulTestCase): for build in self.history: self.assertEqual(results.get(build.uuid, ''), - build.parameters['vars']['zuul'].get('tags')) + build.parameters['zuul'].get('tags')) def test_timer(self): "Test that a periodic job is triggered" diff --git a/zuul/executor/client.py b/zuul/executor/client.py index 2a205bf335..235f6061b7 100644 --- a/zuul/executor/client.py +++ b/zuul/executor/client.py @@ -253,10 +253,12 @@ class ExecutorClient(object): params['nodes'] = nodes params['groups'] = [group.toDict() for group in nodeset.getGroups()] params['vars'] = copy.deepcopy(job.variables) + params['secrets'] = {} if job.auth: for secret in job.auth.secrets: - params['vars'][secret.name] = copy.deepcopy(secret.secret_data) - params['vars']['zuul'] = zuul_params + secret_data = copy.deepcopy(secret.secret_data) + params['secrets'][secret.name] = secret_data + params['zuul'] = zuul_params projects = set() def make_project_dict(project, override_branch=None): diff --git a/zuul/executor/server.py b/zuul/executor/server.py index 25f2f86de0..0a993b3c43 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -215,6 +215,8 @@ class JobDir(object): pass self.known_hosts = os.path.join(ssh_dir, 'known_hosts') self.inventory = os.path.join(self.ansible_root, 'inventory.yaml') + self.secrets = os.path.join(self.ansible_root, 'secrets.yaml') + self.has_secrets = False self.playbooks = [] # The list of candidate playbooks self.playbook = None # A pointer to the candidate we have chosen self.pre_playbooks = [] @@ -767,7 +769,7 @@ class AnsibleJob(object): def _execute(self): args = json.loads(self.job.arguments) self.log.debug("Beginning job %s for ref %s" % - (self.job.name, args['vars']['zuul']['ref'])) + (self.job.name, args['zuul']['ref'])) self.log.debug("Args: %s" % (self.job.arguments,)) self.log.debug("Job root: %s" % (self.jobdir.root,)) tasks = [] @@ -1171,9 +1173,12 @@ class AnsibleJob(object): jobdir_playbook.roles_path.append(role_path) def prepareAnsibleFiles(self, args): - all_vars = dict(args['vars']) + all_vars = args['vars'].copy() # TODO(mordred) Hack to work around running things with python3 all_vars['ansible_python_interpreter'] = '/usr/bin/python2' + if 'zuul' in all_vars: + raise Exception("Defining vars named 'zuul' is not allowed") + all_vars['zuul'] = args['zuul'].copy() all_vars['zuul']['executor'] = dict( hostname=self.executor_server.hostname, src_root=self.jobdir.src_root, @@ -1192,6 +1197,15 @@ class AnsibleJob(object): for key in node['host_keys']: known_hosts.write('%s\n' % key) + secrets = args['secrets'].copy() + if secrets: + if 'zuul' in secrets: + raise Exception("Defining secrets named 'zuul' is not allowed") + with open(self.jobdir.secrets, 'w') as secrets_yaml: + secrets_yaml.write( + yaml.safe_dump(secrets, default_flow_style=False)) + self.jobdir.has_secrets = True + def writeAnsibleConfig(self, jobdir_playbook): trusted = jobdir_playbook.trusted @@ -1372,6 +1386,8 @@ class AnsibleJob(object): verbose = '-v' cmd = ['ansible-playbook', verbose, playbook.path] + if self.jobdir.has_secrets: + cmd.extend(['-e', '@' + self.jobdir.secrets]) if success is not None: cmd.extend(['-e', 'success=%s' % str(bool(success))])