diff --git a/doc/source/user/config.rst b/doc/source/user/config.rst index b2e4be2a87..0eb7bc7995 100644 --- a/doc/source/user/config.rst +++ b/doc/source/user/config.rst @@ -187,14 +187,14 @@ success, the pipeline reports back to Gerrit with ``Verified`` vote of .. attr:: allow-secrets :default: false - This is a boolean which can be used to prevent jobs which - require secrets from running in this pipeline. Some pipelines - run on proposed changes and therefore execute code which has not - yet been reviewed. In such a case, allowing a job to use a - secret could result in that secret being exposed. The default - is ``false``, meaning that in order to run jobs with secrets, - this must be explicitly enabled on each Pipeline where that is - safe. + This is a boolean which can be used to prevent jobs which use + secrets in the untrusted security context from running in this + pipeline. Some pipelines run on proposed changes and therefore + execute code which has not yet been reviewed. In such a case, + allowing a job to use a secret could result in that secret being + exposed. The default is ``false``, meaning that in order to run + jobs which use secrets in the untrusted security context, this + must be explicitly enabled on each Pipeline where that is safe. For more information, see :ref:`secret`. @@ -672,19 +672,6 @@ Here is an example of two job definitions: Authentication information to be made available to the job. This is a dictionary with two potential keys: - .. attr:: inherit - :default: false - - A boolean indicating that the authentication information - referenced by this job should be able to be inherited by - child jobs. Normally when a job inherits from another job, - the auth section is not included. This permits jobs to - inherit the same basic structure and playbook, but ensures - that secret information is unable to be exposed by a child - job which may alter the job's behavior. If it is safe for - the contents of the authentication section to be used by - child jobs, set this to ``true``. - .. attr:: secrets A list of secrets which may be used by the job. A @@ -907,6 +894,16 @@ Here is an example of two job definitions: it should be able to run this job, then it must be explicitly listed. By default, all projects may use the job. + .. attr:: untrusted-secrets + + A boolean value which indicates that this job should not be used + in a pipeline where allow-secrets is ``false``. This is + automatically set to ``true`` if this job is defined in a + :term:`untrusted-project`. It may be explicitly set to obtain + the same behavior for jobs defined in :term:`config projects + `. Once this is set to ``true`` anywhere in the + inheritance hierarchy for a job, it will remain set for all + child jobs and variants (it can not be set to ``false``). .. _project: @@ -1062,22 +1059,34 @@ unencrypted as well for convenience. A Secret may only be used by jobs defined within the same project. To use a secret, a :ref:`job` must specify the secret within its `auth` -section. To protect against jobs in other repositories declaring a -job with a secret as a parent and then exposing that secret, jobs -which inherit from a job with secrets will not inherit the secrets -themselves. To alter that behavior, see the `inherit` job attribute. -Further, jobs which do not permit children to inherit secrets (the -default) are also automatically marked `final`, meaning that their -execution related attributes may not be changed in a project-pipeline -stanza. This is to protect against a job with secrets defined in one -project being used by another project in a way which might expose the -secrets. If a job with secrets is unsafe to be used by other -projects, the `allowed-projects` job attribute can be used to restrict -the projects which can invoke that job. Finally, pipelines which are -used to execute proposed but unreviewed changes can set the -`allow-secrets` attribute to indicate that they should not supply -secrets at all in order to protect against someone proposing a change -which exposes a secret. +section. Secrets are bound to the playbooks associated with the +specific job definition where they were declared. Additional pre or +post playbooks which appear in child jobs will not have access to the +secrets, nor will playbooks which override the main playbook (if any) +of the job which declared the secret. This protects against jobs in +other repositories declaring a job with a secret as a parent and then +exposing that secret. + +It is possible to use secrets for jobs defined in :term:`config +projects ` as well as :term:`untrusted projects +`, however their use differs slightly. Because +playbooks in a config project which use secrets run in the +:term:`trusted execution context` where proposed changes are not used +in executing jobs, it is safe for those secrets to be used in all +types of pipelines. However, because playbooks defined in an +untrusted project are run in the :term:`untrusted execution context` +where proposed changes are used in job execution, it is dangerous to +allow those secrets to be used in pipelines which are used to execute +proposed but unreviewed changes. By default, pipelines will refuse to +run jobs which have playbooks that use secrets in the untrusted +execution context to protect against someone proposing a change which +exposes a secret. To permit this (for instance, in a pipeline which +only runs after code review), the :attr:`pipeline.allow-secrets` +attribute may be set. + +If a job with secrets is unsafe to be used by other projects, the +`allowed-projects` job attribute can be used to restrict the projects +which can invoke that job. .. attr:: secret diff --git a/doc/source/user/jobs.rst b/doc/source/user/jobs.rst index 7f1c3cb56a..5f36c30b98 100644 --- a/doc/source/user/jobs.rst +++ b/doc/source/user/jobs.rst @@ -121,6 +121,9 @@ Might be used in a template as:: {{ credentials.username }} {{ credentials.password }} +Secrets are only available to playbooks associated with the job +definition which uses the secret; they are not available to playbooks +associated with child jobs or job variants. Zuul Variables ~~~~~~~~~~~~~~ diff --git a/tests/base.py b/tests/base.py index b14491c3bd..480db83826 100755 --- a/tests/base.py +++ b/tests/base.py @@ -1451,12 +1451,12 @@ class RecordingAnsibleJob(zuul.executor.server.AnsibleJob): self.recordResult(result) return result - def runAnsible(self, cmd, timeout, config_file, trusted): + def runAnsible(self, cmd, timeout, playbook): build = self.executor_server.job_builds[self.job.unique] if self.executor_server._run_ansible: result = super(RecordingAnsibleJob, self).runAnsible( - cmd, timeout, config_file, trusted) + cmd, timeout, playbook) else: result = build.run() return result diff --git a/tests/fixtures/layouts/untrusted-secrets.yaml b/tests/fixtures/layouts/untrusted-secrets.yaml new file mode 100644 index 0000000000..cfa03e0def --- /dev/null +++ b/tests/fixtures/layouts/untrusted-secrets.yaml @@ -0,0 +1,26 @@ +- pipeline: + name: check + manager: independent + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + Verified: 1 + failure: + gerrit: + Verified: -1 + +- job: + name: base + parent: null + +- job: + name: project1-test + untrusted-secrets: true + +- project: + name: org/project1 + check: + jobs: + - project1-test diff --git a/tests/unit/test_model.py b/tests/unit/test_model.py index 6a63125e4d..00f2592de3 100644 --- a/tests/unit/test_model.py +++ b/tests/unit/test_model.py @@ -54,6 +54,8 @@ class TestJob(BaseTestCase): encryption.deserialize_rsa_keypair(f.read()) self.context = model.SourceContext(self.project, 'master', 'test', True) + self.untrusted_context = model.SourceContext(self.project, 'master', + 'test', False) m = yaml.Mark('name', 0, 0, 0, '', 0) self.start_mark = configloader.ZuulMark(m, m, '') @@ -97,16 +99,15 @@ class TestJob(BaseTestCase): def test_job_inheritance(self): # This is standard job inheritance. - base_pre = model.PlaybookContext(self.context, 'base-pre', []) - base_run = model.PlaybookContext(self.context, 'base-run', []) - base_post = model.PlaybookContext(self.context, 'base-post', []) + base_pre = model.PlaybookContext(self.context, 'base-pre', [], []) + base_run = model.PlaybookContext(self.context, 'base-run', [], []) + base_post = model.PlaybookContext(self.context, 'base-post', [], []) base = model.Job('base') base.timeout = 30 base.pre_run = [base_pre] base.run = [base_run] base.post_run = [base_post] - base.auth = model.AuthContext() py27 = model.Job('py27') self.assertIsNone(py27.timeout) @@ -118,23 +119,21 @@ class TestJob(BaseTestCase): [x.path for x in py27.run]) self.assertEqual(['base-post'], [x.path for x in py27.post_run]) - self.assertIsNone(py27.auth) def test_job_variants(self): # This simulates freezing a job. - py27_pre = model.PlaybookContext(self.context, 'py27-pre', []) - py27_run = model.PlaybookContext(self.context, 'py27-run', []) - py27_post = model.PlaybookContext(self.context, 'py27-post', []) + secrets = ['foo'] + py27_pre = model.PlaybookContext(self.context, 'py27-pre', [], secrets) + py27_run = model.PlaybookContext(self.context, 'py27-run', [], secrets) + py27_post = model.PlaybookContext(self.context, 'py27-post', [], + secrets) py27 = model.Job('py27') py27.timeout = 30 py27.pre_run = [py27_pre] py27.run = [py27_run] py27.post_run = [py27_post] - auth = model.AuthContext() - auth.secrets.append('foo') - py27.auth = auth job = py27.copy() self.assertEqual(30, job.timeout) @@ -151,7 +150,9 @@ class TestJob(BaseTestCase): [x.path for x in job.run]) self.assertEqual(['py27-post'], [x.path for x in job.post_run]) - self.assertEqual(auth, job.auth) + self.assertEqual(secrets, job.pre_run[0].secrets) + self.assertEqual(secrets, job.run[0].secrets) + self.assertEqual(secrets, job.post_run[0].secrets) # Set the job to final for the following checks job.final = True @@ -341,7 +342,7 @@ class TestJob(BaseTestCase): conf = yaml.safe_load(''' - secret: - name: pypi-credentials + name: trusted-secret data: username: test-username longpassword: !encrypted/pkcs1-oaep @@ -384,8 +385,14 @@ class TestJob(BaseTestCase): conf['_source_context'] = self.context conf['_start_mark'] = self.start_mark - secret = configloader.SecretParser.fromYaml(layout, conf) - layout.addSecret(secret) + trusted_secret = configloader.SecretParser.fromYaml(layout, conf) + layout.addSecret(trusted_secret) + + conf['name'] = 'untrusted-secret' + conf['_source_context'] = self.untrusted_context + + untrusted_secret = configloader.SecretParser.fromYaml(layout, conf) + layout.addSecret(untrusted_secret) base = configloader.JobParser.fromYaml(self.tenant, self.layout, { '_source_context': self.context, @@ -395,86 +402,100 @@ class TestJob(BaseTestCase): 'timeout': 30, }) layout.addJob(base) - pypi_upload_without_inherit = configloader.JobParser.fromYaml( - tenant, layout, { - '_source_context': self.context, - '_start_mark': self.start_mark, - 'name': 'pypi-upload-without-inherit', - 'parent': 'base', - 'timeout': 40, - 'auth': { - 'secrets': [ - 'pypi-credentials', - ] - } - }) - layout.addJob(pypi_upload_without_inherit) - pypi_upload_with_inherit = configloader.JobParser.fromYaml( - tenant, layout, { - '_source_context': self.context, - '_start_mark': self.start_mark, - 'name': 'pypi-upload-with-inherit', - 'parent': 'base', - 'timeout': 40, - 'auth': { - 'inherit': True, - 'secrets': [ - 'pypi-credentials', - ] - } - }) - layout.addJob(pypi_upload_with_inherit) - pypi_upload_with_inherit_false = configloader.JobParser.fromYaml( - tenant, layout, { - '_source_context': self.context, - '_start_mark': self.start_mark, - 'name': 'pypi-upload-with-inherit-false', - 'parent': 'base', - 'timeout': 40, - 'auth': { - 'inherit': False, - 'secrets': [ - 'pypi-credentials', - ] - } - }) - layout.addJob(pypi_upload_with_inherit_false) - in_repo_job_without_inherit = configloader.JobParser.fromYaml( - tenant, layout, { - '_source_context': self.context, - '_start_mark': self.start_mark, - 'name': 'in-repo-job-without-inherit', - 'parent': 'pypi-upload-without-inherit', - }) - layout.addJob(in_repo_job_without_inherit) - in_repo_job_with_inherit = configloader.JobParser.fromYaml( - tenant, layout, { - '_source_context': self.context, - '_start_mark': self.start_mark, - 'name': 'in-repo-job-with-inherit', - 'parent': 'pypi-upload-with-inherit', - }) - layout.addJob(in_repo_job_with_inherit) - in_repo_job_with_inherit_false = configloader.JobParser.fromYaml( - tenant, layout, { - '_source_context': self.context, - '_start_mark': self.start_mark, - 'name': 'in-repo-job-with-inherit-false', - 'parent': 'pypi-upload-with-inherit-false', - }) - layout.addJob(in_repo_job_with_inherit_false) - self.assertIsNone(in_repo_job_without_inherit.auth) - self.assertEqual(1, len(in_repo_job_with_inherit.auth.secrets)) - self.assertEqual(in_repo_job_with_inherit.auth.secrets[0].name, - 'pypi-credentials') - self.assertIsNone(in_repo_job_with_inherit_false.auth) - self.assertEqual(in_repo_job_with_inherit.auth.secrets[0]. + trusted_secrets_job = configloader.JobParser.fromYaml( + tenant, layout, { + '_source_context': self.context, + '_start_mark': self.start_mark, + 'name': 'trusted-secrets', + 'parent': 'base', + 'timeout': 40, + 'auth': { + 'secrets': [ + 'trusted-secret', + ] + } + }) + layout.addJob(trusted_secrets_job) + untrusted_secrets_job = configloader.JobParser.fromYaml( + tenant, layout, { + '_source_context': self.untrusted_context, + '_start_mark': self.start_mark, + 'name': 'untrusted-secrets', + 'parent': 'base', + 'timeout': 40, + 'auth': { + 'secrets': [ + 'untrusted-secret', + ] + } + }) + layout.addJob(untrusted_secrets_job) + trusted_secrets_trusted_child_job = configloader.JobParser.fromYaml( + tenant, layout, { + '_source_context': self.context, + '_start_mark': self.start_mark, + 'name': 'trusted-secrets-trusted-child', + 'parent': 'trusted-secrets', + }) + layout.addJob(trusted_secrets_trusted_child_job) + trusted_secrets_untrusted_child_job = configloader.JobParser.fromYaml( + tenant, layout, { + '_source_context': self.untrusted_context, + '_start_mark': self.start_mark, + 'name': 'trusted-secrets-untrusted-child', + 'parent': 'trusted-secrets', + }) + layout.addJob(trusted_secrets_untrusted_child_job) + untrusted_secrets_trusted_child_job = configloader.JobParser.fromYaml( + tenant, layout, { + '_source_context': self.context, + '_start_mark': self.start_mark, + 'name': 'untrusted-secrets-trusted-child', + 'parent': 'untrusted-secrets', + }) + layout.addJob(untrusted_secrets_trusted_child_job) + untrusted_secrets_untrusted_child_job = \ + configloader.JobParser.fromYaml( + tenant, layout, { + '_source_context': self.untrusted_context, + '_start_mark': self.start_mark, + 'name': 'untrusted-secrets-untrusted-child', + 'parent': 'untrusted-secrets', + }) + layout.addJob(untrusted_secrets_untrusted_child_job) + + self.assertIsNone(trusted_secrets_job.untrusted_secrets) + self.assertTrue(untrusted_secrets_job.untrusted_secrets) + self.assertIsNone( + trusted_secrets_trusted_child_job.untrusted_secrets) + self.assertIsNone( + trusted_secrets_untrusted_child_job.untrusted_secrets) + self.assertTrue( + untrusted_secrets_trusted_child_job.untrusted_secrets) + self.assertTrue( + untrusted_secrets_untrusted_child_job.untrusted_secrets) + + self.assertEqual(trusted_secrets_job.implied_run[0].secrets[0].name, + 'trusted-secret') + self.assertEqual(trusted_secrets_job.implied_run[0].secrets[0]. secret_data['longpassword'], 'test-passwordtest-password') - self.assertEqual(in_repo_job_with_inherit.auth.secrets[0]. + self.assertEqual(trusted_secrets_job.implied_run[0].secrets[0]. secret_data['password'], 'test-password') + self.assertEqual( + len(trusted_secrets_trusted_child_job.implied_run[0].secrets), 0) + self.assertEqual( + len(trusted_secrets_untrusted_child_job.implied_run[0].secrets), 0) + + self.assertEqual(untrusted_secrets_job.implied_run[0].secrets[0].name, + 'untrusted-secret') + self.assertEqual( + len(untrusted_secrets_trusted_child_job.implied_run[0].secrets), 0) + self.assertEqual( + len(untrusted_secrets_untrusted_child_job.implied_run[0].secrets), + 0) def test_job_inheritance_job_tree(self): tenant = model.Tenant('tenant') @@ -688,9 +709,7 @@ class TestJob(BaseTestCase): 'name': 'job', 'parent': None, }) - auth = model.AuthContext() - auth.secrets.append('foo') - job.auth = auth + job.untrusted_secrets = True self.layout.addJob(job) diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index e80a30a7e5..960a922aa9 100755 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -2818,6 +2818,18 @@ class TestScheduler(ZuulTestCase): self.assertEqual(B.data['status'], 'MERGED') self.assertEqual(B.reported, 2) + @simple_layout('layouts/untrusted-secrets.yaml') + def test_untrusted_secrets(self): + "Test untrusted secrets" + A = self.fake_gerrit.addFakeChange('org/project1', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + self.assertHistory([]) + self.assertEqual(A.patchsets[0]['approvals'][0]['value'], "-1") + self.assertIn('does not allow jobs with secrets', + A.messages[0]) + @simple_layout('layouts/tags.yaml') def test_tags(self): "Test job tags" diff --git a/zuul/configloader.py b/zuul/configloader.py index 339562a8b9..d453b74aee 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -372,6 +372,7 @@ class JobParser(object): 'allowed-projects': to_list(str), 'override-branch': str, 'description': str, + 'untrusted-secrets': bool } return vs.Schema(job) @@ -426,20 +427,6 @@ class JobParser(object): job = model.Job(conf['name']) job.source_context = conf.get('_source_context') - if 'auth' in conf: - job.auth = model.AuthContext() - if 'inherit' in conf['auth']: - job.auth.inherit = conf['auth']['inherit'] - - for secret_name in conf['auth'].get('secrets', []): - secret = layout.secrets[secret_name] - if secret.source_context != job.source_context: - raise Exception( - "Unable to use secret %s. Secrets must be " - "defined in the same project in which they " - "are used" % secret_name) - job.auth.secrets.append(secret.decrypt( - job.source_context.project.private_key)) is_variant = layout.hasJob(conf['name']) if 'parent' in conf: @@ -462,6 +449,33 @@ class JobParser(object): if not is_variant: parent = layout.getJob(tenant.default_base_job) job.inheritFrom(parent) + # Secrets are part of the playbook context so we must establish + # them earlier than playbooks. + secrets = [] + if 'auth' in conf: + for secret_name in conf['auth'].get('secrets', []): + secret = layout.secrets[secret_name] + if secret.source_context != job.source_context: + raise Exception( + "Unable to use secret %s. Secrets must be " + "defined in the same project in which they " + "are used" % secret_name) + secrets.append(secret.decrypt( + job.source_context.project.private_key)) + + # A job in an untrusted repo that uses secrets requires + # special care. We must note this, and carry this flag + # through inheritance to ensure that we don't run this job in + # an unsafe check pipeline. + if secrets and not conf['_source_context'].trusted: + job.untrusted_secrets = True + + if 'untrusted-secrets' in conf: + if conf['untrusted-secrets']: + job.untrusted_secrets = True + else: + raise Exception("Once set, the untrusted_secrets " + "attribute may not be unset") # Roles are part of the playbook context so we must establish # them earlier than playbooks. @@ -481,21 +495,23 @@ class JobParser(object): for pre_run_name in as_list(conf.get('pre-run')): pre_run = model.PlaybookContext(job.source_context, - pre_run_name, job.roles) + pre_run_name, job.roles, + secrets) job.pre_run = job.pre_run + (pre_run,) for post_run_name in as_list(conf.get('post-run')): post_run = model.PlaybookContext(job.source_context, - post_run_name, job.roles) + post_run_name, job.roles, + secrets) job.post_run = (post_run,) + job.post_run if 'run' in conf: run = model.PlaybookContext(job.source_context, conf['run'], - job.roles) + job.roles, secrets) job.run = (run,) else: if not project_pipeline: run_name = os.path.join('playbooks', job.name) run = model.PlaybookContext(job.source_context, run_name, - job.roles) + job.roles, secrets) job.implied_run = (run,) + job.implied_run for k in JobParser.simple_attributes: diff --git a/zuul/driver/bubblewrap/__init__.py b/zuul/driver/bubblewrap/__init__.py index 5370484ab8..ace5b009f5 100644 --- a/zuul/driver/bubblewrap/__init__.py +++ b/zuul/driver/bubblewrap/__init__.py @@ -158,7 +158,6 @@ class BubblewrapDriver(Driver, WrapperInterface): '--ro-bind', '/etc/resolv.conf', '/etc/resolv.conf', '--ro-bind', '/etc/hosts', '/etc/hosts', '--ro-bind', '{ssh_auth_sock}', '{ssh_auth_sock}', - '--dir', '{work_dir}', '--bind', '{work_dir}', '{work_dir}', '--dev', '/dev', '--chdir', '{work_dir}', diff --git a/zuul/executor/client.py b/zuul/executor/client.py index fbefa8d136..6d1a54f1ff 100644 --- a/zuul/executor/client.py +++ b/zuul/executor/client.py @@ -228,11 +228,6 @@ 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: - secret_data = copy.deepcopy(secret.secret_data) - params['secrets'][secret.name] = secret_data params['zuul'] = zuul_params projects = set() diff --git a/zuul/executor/server.py b/zuul/executor/server.py index 8d23cb77c3..95e8e0b54b 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -251,6 +251,8 @@ class JobDirPlaybook(object): self.roles_path = [] self.ansible_config = os.path.join(self.root, 'ansible.cfg') self.project_link = os.path.join(self.root, 'project') + self.secrets = os.path.join(self.root, 'secrets.yaml') + self.has_secrets = False def addRole(self): count = len(self.roles) @@ -271,18 +273,19 @@ class JobDir(object): log streaming daemon find job logs. ''' # root - # .ansible - # fact-cache/localhost - # ansible + # ansible (mounted in bwrap read-only) # inventory.yaml - # playbook_0 + # .ansible (mounted in bwrap read-write) + # fact-cache/localhost + # playbook_0 (mounted in bwrap for each playbook read-only) + # secrets.yaml # project -> ../trusted/project_0/... # role_0 -> ../trusted/project_0/... - # trusted + # trusted (mounted in bwrap read-only) # project_0 # # - # work + # work (mounted in bwrap read-write) # .ssh # known_hosts # src @@ -311,8 +314,8 @@ class JobDir(object): ssh_dir = os.path.join(self.work_root, '.ssh') os.mkdir(ssh_dir, 0o700) # Create ansible cache directory - ansible_cache = os.path.join(self.root, '.ansible') - self.fact_cache = os.path.join(ansible_cache, 'fact-cache') + self.ansible_cache_root = os.path.join(self.root, '.ansible') + self.fact_cache = os.path.join(self.ansible_cache_root, 'fact-cache') os.makedirs(self.fact_cache) localhost_facts = os.path.join(self.fact_cache, 'localhost') # NOTE(pabelanger): We do not want to leak zuul-executor facts to other @@ -327,8 +330,6 @@ 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 = [] @@ -1260,7 +1261,7 @@ class AnsibleJob(object): for role in playbook['roles']: self.prepareRole(jobdir_playbook, role, args) - self.writeAnsibleConfig(jobdir_playbook) + self.writeAnsibleConfig(jobdir_playbook, playbook) def checkoutTrustedProject(self, project, branch): root = self.jobdir.getTrustedProject(project.canonical_name, @@ -1381,25 +1382,25 @@ class AnsibleJob(object): for key in node['host_keys']: known_hosts.write('%s\n' % key) - secrets = args['secrets'].copy() + def writeAnsibleConfig(self, jobdir_playbook, playbook): + trusted = jobdir_playbook.trusted + + secrets = playbook['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: + with open(jobdir_playbook.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 + jobdir_playbook.has_secrets = True with open(jobdir_playbook.ansible_config, 'w') as config: config.write('[defaults]\n') config.write('hostfile = %s\n' % self.jobdir.inventory) - config.write('local_tmp = %s/.ansible/local_tmp\n' % - self.jobdir.root) - config.write('remote_tmp = %s/.ansible/remote_tmp\n' % - self.jobdir.root) + config.write('local_tmp = %s/local_tmp\n' % + self.jobdir.ansible_cache_root) + config.write('remote_tmp = %s/remote_tmp\n' % + self.jobdir.ansible_cache_root) config.write('retry_files_enabled = False\n') config.write('gathering = smart\n') config.write('fact_caching = jsonfile\n') @@ -1425,13 +1426,12 @@ class AnsibleJob(object): config.write('roles_path = %s\n' % ':'.join( jobdir_playbook.roles_path)) - # On trusted jobs, we want to prevent the printing of args, - # since trusted jobs might have access to secrets that they may - # need to pass to a task or a role. On the other hand, there - # should be no sensitive data in untrusted jobs, and printing - # the args could be useful for debugging. + # On playbooks with secrets we want to prevent the + # printing of args since they may be passed to a task or a + # role. Otherwise, printing the args could be useful for + # debugging. config.write('display_args_to_stdout = %s\n' % - str(not trusted)) + str(not secrets)) config.write('[ssh_connection]\n') # NB: when setting pipelining = True, keep_remote_files @@ -1464,7 +1464,8 @@ class AnsibleJob(object): except Exception: self.log.exception("Exception while killing ansible process:") - def runAnsible(self, cmd, timeout, config_file, trusted): + def runAnsible(self, cmd, timeout, playbook): + config_file = playbook.ansible_config env_copy = os.environ.copy() env_copy.update(self.ssh_agent.env) env_copy['ZUUL_JOB_OUTPUT_FILE'] = self.jobdir.job_output_file @@ -1477,7 +1478,7 @@ class AnsibleJob(object): pythonpath = [self.executor_server.ansible_dir] + pythonpath env_copy['PYTHONPATH'] = os.path.pathsep.join(pythonpath) - if trusted: + if playbook.trusted: opt_prefix = 'trusted' else: opt_prefix = 'untrusted' @@ -1489,6 +1490,11 @@ class AnsibleJob(object): rw_paths = rw_paths.split(":") if rw_paths else [] ro_paths.append(self.executor_server.ansible_dir) + ro_paths.append(self.jobdir.ansible_root) + ro_paths.append(self.jobdir.trusted_root) + ro_paths.append(playbook.root) + + rw_paths.append(self.jobdir.ansible_cache_root) if self.executor_variables_file: ro_paths.append(self.executor_variables_file) @@ -1497,7 +1503,7 @@ class AnsibleJob(object): rw_paths) popen = self.executor_server.execution_wrapper.getPopen( - work_dir=self.jobdir.root, + work_dir=self.jobdir.work_root, ssh_auth_sock=env_copy.get('SSH_AUTH_SOCK')) env_copy['ANSIBLE_CONFIG'] = config_file @@ -1577,8 +1583,8 @@ class AnsibleJob(object): verbose = '-v' cmd = ['ansible-playbook', verbose, playbook.path] - if self.jobdir.has_secrets: - cmd.extend(['-e', '@' + self.jobdir.secrets]) + if playbook.has_secrets: + cmd.extend(['-e', '@' + playbook.secrets]) if success is not None: cmd.extend(['-e', 'success=%s' % str(bool(success))]) @@ -1600,9 +1606,7 @@ class AnsibleJob(object): cmd.extend(['-e@%s' % self.executor_variables_file]) result, code = self.runAnsible( - cmd=cmd, timeout=timeout, - config_file=playbook.ansible_config, - trusted=playbook.trusted) + cmd=cmd, timeout=timeout, playbook=playbook) self.log.debug("Ansible complete, result %s code %s" % ( self.RESULT_MAP[result], code)) return result, code diff --git a/zuul/model.py b/zuul/model.py index 9a89f75d70..092c0ed777 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -651,10 +651,11 @@ class PlaybookContext(object): """ - def __init__(self, source_context, path, roles): + def __init__(self, source_context, path, roles, secrets): self.source_context = source_context self.path = path self.roles = roles + self.secrets = secrets def __repr__(self): return '' % (self.source_context, @@ -668,16 +669,22 @@ class PlaybookContext(object): return False return (self.source_context == other.source_context and self.path == other.path and - self.roles == other.roles) + self.roles == other.roles and + self.secrets == other.secrets) def toDict(self): # Render to a dict to use in passing json to the executor + secrets = {} + for secret in self.secrets: + secret_data = copy.deepcopy(secret.secret_data) + secrets[secret.name] = secret_data return dict( connection=self.source_context.project.connection_name, project=self.source_context.project.name, branch=self.source_context.branch, trusted=self.source_context.trusted, roles=[r.toDict() for r in self.roles], + secrets=secrets, path=self.path) @@ -740,28 +747,6 @@ class ZuulRole(Role): return d -class AuthContext(object): - """The authentication information for a job. - - Authentication information (both the actual data and metadata such - as whether it should be inherited) for a job is grouped together - in this object. - """ - - def __init__(self, inherit=False): - self.inherit = inherit - self.secrets = [] - - def __ne__(self, other): - return not self.__eq__(other) - - def __eq__(self, other): - if not isinstance(other, AuthContext): - return False - return (self.inherit == other.inherit and - self.secrets == other.secrets) - - class Job(object): """A Job represents the defintion of actions to perform. @@ -804,7 +789,6 @@ class Job(object): timeout=None, variables={}, nodeset=NodeSet(), - auth=None, workspace=None, pre_run=(), post_run=(), @@ -817,6 +801,7 @@ class Job(object): required_projects={}, allowed_projects=None, override_branch=None, + untrusted_secrets=None, ) # These are generally internal attributes which are not @@ -932,13 +917,9 @@ class Job(object): if not isinstance(other, Job): raise Exception("Job unable to inherit from %s" % (other,)) - do_not_inherit = set() - if other.auth and not other.auth.inherit: - do_not_inherit.add('auth') - # copy all attributes for k in self.inheritable_attributes: - if (other._get(k) is not None and k not in do_not_inherit): + if (other._get(k) is not None): setattr(self, k, copy.deepcopy(getattr(other, k))) msg = 'inherit from %s' % (repr(other),) @@ -2322,11 +2303,6 @@ class Layout(object): # (that is to say that it must match more than just # the job that is defined in the tree). continue - # If the job does not allow auth inheritance, do not allow - # the project-pipeline variants to update its execution - # attributes. - if frozen_job.auth and not frozen_job.auth.inherit: - frozen_job.final = True # Whether the change matches any of the project pipeline # variants matched = False @@ -2342,8 +2318,7 @@ class Layout(object): change.project.name not in frozen_job.allowed_projects): raise Exception("Project %s is not allowed to run job %s" % (change.project.name, frozen_job.name)) - if ((not pipeline.allow_secrets) and frozen_job.auth and - frozen_job.auth.secrets): + if ((not pipeline.allow_secrets) and frozen_job.untrusted_secrets): raise Exception("Pipeline %s does not allow jobs with " "secrets (job %s)" % ( pipeline.name, frozen_job.name))