Merge "Add zuul.project.src_dir variable" into feature/zuulv3
diff --git a/.zuul.yaml b/.zuul.yaml
index 3922dd3..e2628cb 100644
--- a/.zuul.yaml
+++ b/.zuul.yaml
@@ -8,3 +8,8 @@
- tox-pep8
- tox-py35
- tox-tarball
+ gate:
+ jobs:
+ - tox-docs
+ - tox-pep8
+ - tox-py35
diff --git a/doc/source/user/config.rst b/doc/source/user/config.rst
index b2e4be2..4898e17 100644
--- a/doc/source/user/config.rst
+++ b/doc/source/user/config.rst
@@ -187,14 +187,14 @@
.. 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`.
@@ -438,7 +438,7 @@
jobs on the system should have, progressing through stages of
specialization before arriving at a particular job. A job may inherit
from any other job in any project (however, if the other job is marked
-as ``final``, some attributes may not be overidden).
+as :attr:`job.final`, jobs may not inherit from it).
A job with no parent is called a *base job* and may only be defined in
a :term:`config-project`. Every other job must have a parent, and so
@@ -452,7 +452,8 @@
These may have different selection criteria which indicate to Zuul
that, for instance, the job should behave differently on a different
git branch. Unlike inheritance, all job variants must be defined in
-the same project.
+the same project. Some attributes of jobs marked :attr:`job.final`
+may not be overidden
When Zuul decides to run a job, it performs a process known as
freezing the job. Because any number of job variants may be
@@ -529,6 +530,14 @@
to auto-document Zuul jobs (in which case it is interpreted as
ReStructuredText.
+ .. attr:: final
+ :default: false
+
+ To prevent other jobs from inheriting from this job, and also to
+ prevent changing execution-related attributes when this job is
+ specified in a project's pipeline, set this attribute to
+ ``true``.
+
.. attr:: success-message
:default: SUCCESS
@@ -667,34 +676,13 @@
are in the docs directory. A regular expression or list of
regular expressions.
- .. attr:: auth
+ .. attr:: secrets
- 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
- :ref:`secret` is a named collection of private information
- defined separately in the configuration. The secrets that
- appear here must be defined in the same project as this job
- definition.
-
- In the future, other types of authentication information may
- be added.
+ A list of secrets which may be used by the job. A
+ :ref:`secret` is a named collection of private information
+ defined separately in the configuration. The secrets that
+ appear here must be defined in the same project as this job
+ definition.
.. attr:: nodes
@@ -907,6 +895,16 @@
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
+ <config-project>`. 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:
@@ -1061,23 +1059,35 @@
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.
+use a secret, a :ref:`job` must specify the secret in
+:attr:`job.secrets`. 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 <config-project>` as well as :term:`untrusted projects
+<untrusted-project>`, 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 46b36f5..4e1880a 100644
--- a/doc/source/user/jobs.rst
+++ b/doc/source/user/jobs.rst
@@ -121,6 +121,9 @@
{{ 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 b14491c..480db83 100755
--- a/tests/base.py
+++ b/tests/base.py
@@ -1451,12 +1451,12 @@
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/config/ansible/git/common-config/zuul.yaml b/tests/fixtures/config/ansible/git/common-config/zuul.yaml
index c70191f..ba6227b 100644
--- a/tests/fixtures/config/ansible/git/common-config/zuul.yaml
+++ b/tests/fixtures/config/ansible/git/common-config/zuul.yaml
@@ -87,9 +87,8 @@
flagpath: '{{zuul._test.test_root}}/{{zuul.build}}.flag'
roles:
- zuul: bare-role
- auth:
- secrets:
- - test_secret
+ secrets:
+ - test_secret
- job:
parent: python27
@@ -106,10 +105,9 @@
vartest_job: vartest_job
vartest_secret: vartest_job
vartest_site: vartest_job
- auth:
- secrets:
- - vartest_site
- - vartest_secret
+ secrets:
+ - vartest_site
+ - vartest_secret
- job:
parent: base-urls
diff --git a/tests/fixtures/config/base-jobs/git/common-config/zuul.yaml b/tests/fixtures/config/base-jobs/git/common-config/zuul.yaml
index 0603d23..4cef200 100644
--- a/tests/fixtures/config/base-jobs/git/common-config/zuul.yaml
+++ b/tests/fixtures/config/base-jobs/git/common-config/zuul.yaml
@@ -16,7 +16,7 @@
parent: null
tags:
- mybase
-
+
- job:
name: other-base
parent: null
diff --git a/tests/fixtures/config/final/git/common-config/playbooks/job-final.yaml b/tests/fixtures/config/final/git/common-config/playbooks/job-final.yaml
new file mode 100644
index 0000000..f679dce
--- /dev/null
+++ b/tests/fixtures/config/final/git/common-config/playbooks/job-final.yaml
@@ -0,0 +1,2 @@
+- hosts: all
+ tasks: []
diff --git a/tests/fixtures/config/final/git/common-config/zuul.yaml b/tests/fixtures/config/final/git/common-config/zuul.yaml
new file mode 100644
index 0000000..f08d66e
--- /dev/null
+++ b/tests/fixtures/config/final/git/common-config/zuul.yaml
@@ -0,0 +1,28 @@
+- 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: job-final
+ final: true
+ vars:
+ dont_override_this: dummy
+
+- project:
+ name: org/project
+ check:
+ jobs: []
+
diff --git a/tests/fixtures/config/final/git/org_project/README b/tests/fixtures/config/final/git/org_project/README
new file mode 100644
index 0000000..9daeafb
--- /dev/null
+++ b/tests/fixtures/config/final/git/org_project/README
@@ -0,0 +1 @@
+test
diff --git a/tests/fixtures/config/final/git/org_project/playbooks/placeholder b/tests/fixtures/config/final/git/org_project/playbooks/placeholder
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tests/fixtures/config/final/git/org_project/playbooks/placeholder
diff --git a/tests/fixtures/config/final/main.yaml b/tests/fixtures/config/final/main.yaml
new file mode 100644
index 0000000..208e274
--- /dev/null
+++ b/tests/fixtures/config/final/main.yaml
@@ -0,0 +1,8 @@
+- tenant:
+ name: tenant-one
+ source:
+ gerrit:
+ config-projects:
+ - common-config
+ untrusted-projects:
+ - org/project
diff --git a/tests/fixtures/layouts/untrusted-secrets.yaml b/tests/fixtures/layouts/untrusted-secrets.yaml
new file mode 100644
index 0000000..cfa03e0
--- /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 6a63125..9cc7195 100644
--- a/tests/unit/test_model.py
+++ b/tests/unit/test_model.py
@@ -54,6 +54,8 @@
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 @@
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 @@
[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 @@
[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 @@
conf = yaml.safe_load('''
- secret:
- name: pypi-credentials
+ name: trusted-secret
data:
username: test-username
longpassword: !encrypted/pkcs1-oaep
@@ -384,8 +385,14 @@
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,96 @@
'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,
+ '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,
+ '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 +705,7 @@
'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 e80a30a..960a922 100755
--- a/tests/unit/test_scheduler.py
+++ b/tests/unit/test_scheduler.py
@@ -2818,6 +2818,18 @@
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/tests/unit/test_v3.py b/tests/unit/test_v3.py
index 74d72e7..5e3a6fc 100755
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -67,6 +67,89 @@
"not affect tenant one")
+class TestFinal(ZuulTestCase):
+
+ tenant_config_file = 'config/final/main.yaml'
+
+ def test_final_variant_ok(self):
+ # test clean usage of final parent job
+ in_repo_conf = textwrap.dedent(
+ """
+ - project:
+ name: org/project
+ check:
+ jobs:
+ - job-final
+ """)
+
+ file_dict = {'.zuul.yaml': in_repo_conf}
+ A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
+ files=file_dict)
+ self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+ self.waitUntilSettled()
+
+ self.assertEqual(A.reported, 1)
+ self.assertEqual(A.patchsets[-1]['approvals'][0]['value'], '1')
+
+ def test_final_variant_error(self):
+ # test misuse of final parent job
+ in_repo_conf = textwrap.dedent(
+ """
+ - project:
+ name: org/project
+ check:
+ jobs:
+ - job-final:
+ vars:
+ dont_override_this: bar
+ """)
+ file_dict = {'.zuul.yaml': in_repo_conf}
+ A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
+ files=file_dict)
+ self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+ self.waitUntilSettled()
+
+ # The second patch tried to override some variables.
+ # Thus it should fail.
+ self.assertEqual(A.reported, 1)
+ self.assertEqual(A.patchsets[-1]['approvals'][0]['value'], '-1')
+ self.assertIn('Unable to modify final job', A.messages[0])
+
+ def test_final_inheritance(self):
+ # test misuse of final parent job
+ in_repo_conf = textwrap.dedent(
+ """
+ - job:
+ name: project-test
+ parent: job-final
+
+ - project:
+ name: org/project
+ check:
+ jobs:
+ - project-test
+ """)
+
+ in_repo_playbook = textwrap.dedent(
+ """
+ - hosts: all
+ tasks: []
+ """)
+
+ file_dict = {'.zuul.yaml': in_repo_conf,
+ 'playbooks/project-test.yaml': in_repo_playbook}
+ A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
+ files=file_dict)
+ self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+ self.waitUntilSettled()
+
+ # The second patch tried to override some variables.
+ # Thus it should fail.
+ self.assertEqual(A.reported, 1)
+ self.assertEqual(A.patchsets[-1]['approvals'][0]['value'], '-1')
+ self.assertIn('Unable to inherit from final job', A.messages[0])
+
+
class TestInRepoConfig(ZuulTestCase):
# A temporary class to hold new tests while others are disabled
diff --git a/zuul/ansible/action/normal.py b/zuul/ansible/action/normal.py
index b8a232b..803b0a7 100644
--- a/zuul/ansible/action/normal.py
+++ b/zuul/ansible/action/normal.py
@@ -50,7 +50,7 @@
handler_name = 'handle_{action}'.format(action=self._task.action)
handler = getattr(self, handler_name, None)
if handler:
- handler(self)
+ handler()
return True
return False
diff --git a/zuul/ansible/callback/zuul_stream.py b/zuul/ansible/callback/zuul_stream.py
index 9dd724d..34d4406 100644
--- a/zuul/ansible/callback/zuul_stream.py
+++ b/zuul/ansible/callback/zuul_stream.py
@@ -98,6 +98,7 @@
self._daemon_running = False
self._play = None
self._streamers = []
+ self._streamers_stop = False
self.configure_logger()
self._items_done = False
self._deferred_result = None
@@ -142,6 +143,11 @@
for line in linesplit(s):
if "[Zuul] Task exit code" in line:
return
+ elif self._streamers_stop and "[Zuul] Log not found" in line:
+ return
+ elif "[Zuul] Log not found" in line:
+ # don't output this line
+ pass
else:
ts, ln = line.split(' | ', 1)
ln = ln.strip()
@@ -223,6 +229,7 @@
self._streamers.append(streamer)
def _stop_streamers(self):
+ self._streamers_stop = True
while True:
if not self._streamers:
break
@@ -231,6 +238,7 @@
if streamer.is_alive():
msg = "[Zuul] Log Stream did not terminate"
self._log(msg, job=True, executor=True)
+ self._streamers_stop = False
def _process_result_for_localhost(self, result, is_task=True):
result_dict = dict(result._result)
diff --git a/zuul/ansible/library/zuul_console.py b/zuul/ansible/library/zuul_console.py
index ac85dec..ddada3f 100644
--- a/zuul/ansible/library/zuul_console.py
+++ b/zuul/ansible/library/zuul_console.py
@@ -185,6 +185,7 @@
console = self.chunkConsole(conn, log_uuid)
if console:
break
+ conn.send('[Zuul] Log not found\n')
time.sleep(0.5)
while True:
if self.followConsole(console, conn):
diff --git a/zuul/configloader.py b/zuul/configloader.py
index 0ecbc14..86459b0 100644
--- a/zuul/configloader.py
+++ b/zuul/configloader.py
@@ -324,10 +324,6 @@
@staticmethod
def getSchema():
- auth = {'secrets': to_list(str),
- 'inherit': bool,
- }
-
node = {vs.Required('name'): str,
vs.Required('label'): str,
}
@@ -345,6 +341,7 @@
job = {vs.Required('name'): str,
'parent': vs.Any(str, None),
+ 'final': bool,
'failure-message': str,
'success-message': str,
'failure-url': str,
@@ -355,7 +352,7 @@
'tags': to_list(str),
'branches': to_list(str),
'files': to_list(str),
- 'auth': auth,
+ 'secrets': to_list(str),
'irrelevant-files': to_list(str),
'nodes': vs.Any([node], str),
'timeout': int,
@@ -372,11 +369,13 @@
'allowed-projects': to_list(str),
'override-branch': str,
'description': str,
+ 'untrusted-secrets': bool
}
return vs.Schema(job)
simple_attributes = [
+ 'final',
'timeout',
'workspace',
'voting',
@@ -426,34 +425,54 @@
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:
if conf['parent'] is not None:
+ # Parent job is explicitly specified, so inherit from it.
parent = layout.getJob(conf['parent'])
job.inheritFrom(parent)
else:
+ # Parent is explicitly set as None, so user intends
+ # this to be a base job. That's only okay if we're in
+ # a config project.
if not conf['_source_context'].trusted:
raise Exception(
"Base jobs must be defined in config projects")
else:
+ # Parent is not explicitly set, so inherit from the
+ # default -- but only if this is the primary definition
+ # for the job (ie, not a variant -- variants don't need to
+ # have a parent as long as the main job does).
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 = []
+ for secret_name in conf.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.
@@ -473,21 +492,23 @@
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 5370484..cbaa609 100644
--- a/zuul/driver/bubblewrap/__init__.py
+++ b/zuul/driver/bubblewrap/__init__.py
@@ -22,6 +22,7 @@
import shlex
import subprocess
import sys
+import re
from typing import Dict, List # flake8: noqa
@@ -73,6 +74,7 @@
log = logging.getLogger("zuul.BubblewrapDriver")
mounts_map = {'rw': [], 'ro': []} # type: Dict[str, List]
+ release_file_re = re.compile('^\W+-release$')
def __init__(self):
self.bwrap_command = self._bwrap_command()
@@ -158,7 +160,6 @@
'--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}',
@@ -171,11 +172,16 @@
'--file', '{gid_fd}', '/etc/group',
]
- if os.path.isdir('/lib64'):
- bwrap_command.extend(['--ro-bind', '/lib64', '/lib64'])
- if os.path.isfile('/etc/nsswitch.conf'):
- bwrap_command.extend(['--ro-bind', '/etc/nsswitch.conf',
- '/etc/nsswitch.conf'])
+ for path in ['/lib64',
+ '/etc/nsswitch.conf',
+ '/etc/lsb-release.d',
+ ]:
+ if os.path.exists(path):
+ bwrap_command.extend(['--ro-bind', path, path])
+ for fn in os.listdir('/etc'):
+ if self.release_file_re.match(fn):
+ path = os.path.join('/etc', fn)
+ bwrap_command.extend(['--ro-bind', path, path])
return bwrap_command
diff --git a/zuul/executor/client.py b/zuul/executor/client.py
index f8761c6..e503f41 100644
--- a/zuul/executor/client.py
+++ b/zuul/executor/client.py
@@ -233,11 +233,6 @@
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 8d23cb7..95e8e0b 100644
--- a/zuul/executor/server.py
+++ b/zuul/executor/server.py
@@ -251,6 +251,8 @@
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 @@
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
# <git.example.com>
# <project>
- # work
+ # work (mounted in bwrap read-write)
# .ssh
# known_hosts
# src
@@ -311,8 +314,8 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 9a89f75..cf57851 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -651,10 +651,11 @@
"""
- 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 '<PlaybookContext %s %s>' % (self.source_context,
@@ -668,16 +669,22 @@
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 @@
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 @@
timeout=None,
variables={},
nodeset=NodeSet(),
- auth=None,
workspace=None,
pre_run=(),
post_run=(),
@@ -817,6 +801,7 @@
required_projects={},
allowed_projects=None,
override_branch=None,
+ untrusted_secrets=None,
)
# These are generally internal attributes which are not
@@ -932,13 +917,13 @@
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')
+ if other.final:
+ raise Exception("Unable to inherit from final job %s" %
+ (repr(other),))
# 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 +2307,6 @@
# (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 +2322,7 @@
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))