From 91ed6652dc5a345af4cc1bedef8a2a5718616438 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Thu, 17 Jan 2019 11:36:53 -0800 Subject: [PATCH] Add pass-to-parent option for secrets This allows jobs to pass secrets to parent jobs. That will allow for jobs which are designed to be used with secrets with the actual secrets supplied by child jobs. Change-Id: I544aec77c86f4cb1c11bf6e14a7d93efea0421d6 --- doc/source/user/config.rst | 53 ++++-- .../pass-to-parent-24d40e7ebcff6d9e.yaml | 8 + .../git/common-config/playbooks/post.yaml | 2 + .../git/common-config/playbooks/pre.yaml | 2 + .../git/common-config/playbooks/run.yaml | 2 + .../git/common-config/zuul.yaml | 78 +++++++++ .../pass-to-parent/git/org_project/README | 1 + .../git/org_project/playbooks/post.yaml | 2 + .../git/org_project/playbooks/pre.yaml | 2 + .../git/org_project/playbooks/run.yaml | 2 + .../pass-to-parent/git/org_project/zuul.yaml | 65 +++++++ .../fixtures/config/pass-to-parent/main.yaml | 8 + tests/unit/test_v3.py | 160 ++++++++++++++++++ zuul/configloader.py | 10 +- zuul/model.py | 56 +++++- 15 files changed, 428 insertions(+), 23 deletions(-) create mode 100644 releasenotes/notes/pass-to-parent-24d40e7ebcff6d9e.yaml create mode 100644 tests/fixtures/config/pass-to-parent/git/common-config/playbooks/post.yaml create mode 100644 tests/fixtures/config/pass-to-parent/git/common-config/playbooks/pre.yaml create mode 100644 tests/fixtures/config/pass-to-parent/git/common-config/playbooks/run.yaml create mode 100644 tests/fixtures/config/pass-to-parent/git/common-config/zuul.yaml create mode 100644 tests/fixtures/config/pass-to-parent/git/org_project/README create mode 100644 tests/fixtures/config/pass-to-parent/git/org_project/playbooks/post.yaml create mode 100644 tests/fixtures/config/pass-to-parent/git/org_project/playbooks/pre.yaml create mode 100644 tests/fixtures/config/pass-to-parent/git/org_project/playbooks/run.yaml create mode 100644 tests/fixtures/config/pass-to-parent/git/org_project/zuul.yaml create mode 100644 tests/fixtures/config/pass-to-parent/main.yaml diff --git a/doc/source/user/config.rst b/doc/source/user/config.rst index 76c8c1d47d..1fb921619b 100644 --- a/doc/source/user/config.rst +++ b/doc/source/user/config.rst @@ -674,16 +674,29 @@ Here is an example of two job definitions: Each item in the list may may be supplied either as a string, in which case it references the name of a :ref:`secret` definition, or as a dict. If an element in this list is given as a dict, it - must have the following fields. + may have the following fields: .. attr:: name + :required: The name to use for the Ansible variable into which the secret content will be placed. .. attr:: secret + :required: - The name to use to find the secret's definition in the configuration. + The name to use to find the secret's definition in the + configuration. + + .. attr:: pass-to-parent + :default: false + + A boolean indicating that this secret should be made + available to playbooks in parent jobs. Use caution when + setting this value -- parent jobs may be in different + projects with different security standards. Setting this to + true makes the secret available to those playbooks and + therefore subject to intentional or accidental exposure. For example: @@ -1311,13 +1324,26 @@ project's branches have different access controls, consider whether all branches of that project are equally trusted before using secrets. To 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. +:attr:`job.secrets`. With one exception, 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. + +The exception to the above is if the +:attr:`job.secrets.pass-to-parent` attribute is set to true. In that +case, the secret is made available not only to the playbooks in the +current job definition, but to all playbooks in all parent jobs as +well. This allows for jobs which are designed to work with secrets +while leaving it up to child jobs to actually supply the secret. Use +this option with care, as it may allow the authors of parent jobs to +accidentially or intentionally expose secrets. If a secret with +`pass-to-parent` set in a child job has the same name as a secret +available to a parent job's playbook, the secret in the child job will +not override the parent, instead it will simply not be available to +that playbook (but will remain available to others). It is possible to use secrets for jobs defined in :term:`config projects ` as well as :term:`untrusted projects @@ -1331,10 +1357,11 @@ 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 are considered `pre-review` and 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.post-review` attribute may be explicitly set to +secrets in the untrusted execution context (including those subject to +:attr:`job.secrets.pass-to-parent` secrets) in order 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.post-review` attribute may be explicitly set to ``true``. In some cases, it may be desirable to prevent a job which is defined diff --git a/releasenotes/notes/pass-to-parent-24d40e7ebcff6d9e.yaml b/releasenotes/notes/pass-to-parent-24d40e7ebcff6d9e.yaml new file mode 100644 index 0000000000..fd57239628 --- /dev/null +++ b/releasenotes/notes/pass-to-parent-24d40e7ebcff6d9e.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + The :attr:`job.secrets.pass-to-parent` attribute has been added to + allow secrets to be made available to playbooks in parent jobs + (for example, to allow for jobs which are designed to use secrets, + but leave it to child jobs to actually supply them). See also the + :ref:`secret` documentation. diff --git a/tests/fixtures/config/pass-to-parent/git/common-config/playbooks/post.yaml b/tests/fixtures/config/pass-to-parent/git/common-config/playbooks/post.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/pass-to-parent/git/common-config/playbooks/post.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/pass-to-parent/git/common-config/playbooks/pre.yaml b/tests/fixtures/config/pass-to-parent/git/common-config/playbooks/pre.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/pass-to-parent/git/common-config/playbooks/pre.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/pass-to-parent/git/common-config/playbooks/run.yaml b/tests/fixtures/config/pass-to-parent/git/common-config/playbooks/run.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/pass-to-parent/git/common-config/playbooks/run.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/pass-to-parent/git/common-config/zuul.yaml b/tests/fixtures/config/pass-to-parent/git/common-config/zuul.yaml new file mode 100644 index 0000000000..ebbbc00354 --- /dev/null +++ b/tests/fixtures/config/pass-to-parent/git/common-config/zuul.yaml @@ -0,0 +1,78 @@ +- pipeline: + name: check + manager: independent + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + Verified: 1 + failure: + gerrit: + Verified: -1 + +- pipeline: + name: gate + manager: dependent + success-message: Build succeeded (gate). + trigger: + gerrit: + - event: comment-added + approval: + - Approved: 1 + success: + gerrit: + Verified: 2 + submit: true + failure: + gerrit: + Verified: -2 + start: + gerrit: + Verified: 0 + precedence: high + post-review: true + +- job: + name: base + parent: null + +- secret: + name: trusted-secret1 + data: + password: !encrypted/pkcs1-oaep + - SyQ97Cc8wRhvJHas4RDkZvnZDeOvMtkbo97xKibHtqGZsKtkIxNKuFmmqsaY4cxhV7mI+ATf6XGzFn5dO57DDbZJUbS1Qs/to540Y3VDlJ4twyBplBVK/RAUGVn4yu9I8s0+y6aZR+wMuyChWfGvUo6Pno7z7gviOonPaM5KE0nA/I2MScaP6nHDh75IyMBXTNH/8ZXKFQTteeTvSfzhtLqTspNLt7tykb2WBKAlp/G4YzdtZpUEeoslHIcmBqXpSau6sG38cRrZZzK801ibfyoBnOWpadMaLciy2V+Yw5RZHqLxp/4WbgKl5M0/RCmIrS+Jb8351Mtf7HZcWS2NwozoD7C7CUU87bTazqACY/MebF8XGXJNISuuBfhqnptDBi0NvZCFe6Wb9hYQwsS5mA0e5gWk2Yqrn78SFlZ7HxeQGct+WuIQoek09UTLlocY7AHTwSUXrD2ab9vdI0mWs+bIb4z3h2YpKw/UVTz/UlaZaCQEy/0aQeuO06xQNomZjL/JTOpNMCgG6drEHyox44k6xv2xL7AmwEuTcwOzqWeCTeF69yfbSPBJMz+ayiQ3JMg/dd7gpATAGiqQrKwvvduGxbLkFKO0vf+wpvqoPiDzfoq988nkpOc927QEodVPuZtIyPeRZLx1uJHXcTRvZkaQzuEpwhmtIdhu6XdbXh8= + +- job: + name: trusted-under-untrusted + parent: parent-job-without-secret + secrets: + name: secret + secret: trusted-secret1 + pass-to-parent: true + files: trusted-under-untrusted.txt + +- job: + name: trusted-parent-job-without-secret + pre-run: playbooks/pre.yaml + run: playbooks/run.yaml + post-run: playbooks/post.yaml + +- job: + name: trusted-under-trusted + parent: trusted-parent-job-without-secret + secrets: + name: secret + secret: trusted-secret1 + pass-to-parent: true + files: trusted-under-trusted.txt + +- project: + check: + jobs: + - trusted-under-untrusted + - trusted-under-trusted + gate: + jobs: + - trusted-under-untrusted + - trusted-under-trusted diff --git a/tests/fixtures/config/pass-to-parent/git/org_project/README b/tests/fixtures/config/pass-to-parent/git/org_project/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/pass-to-parent/git/org_project/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/pass-to-parent/git/org_project/playbooks/post.yaml b/tests/fixtures/config/pass-to-parent/git/org_project/playbooks/post.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/pass-to-parent/git/org_project/playbooks/post.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/pass-to-parent/git/org_project/playbooks/pre.yaml b/tests/fixtures/config/pass-to-parent/git/org_project/playbooks/pre.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/pass-to-parent/git/org_project/playbooks/pre.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/pass-to-parent/git/org_project/playbooks/run.yaml b/tests/fixtures/config/pass-to-parent/git/org_project/playbooks/run.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/pass-to-parent/git/org_project/playbooks/run.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/pass-to-parent/git/org_project/zuul.yaml b/tests/fixtures/config/pass-to-parent/git/org_project/zuul.yaml new file mode 100644 index 0000000000..1b13146f38 --- /dev/null +++ b/tests/fixtures/config/pass-to-parent/git/org_project/zuul.yaml @@ -0,0 +1,65 @@ +- secret: + name: secret1 + data: + password: !encrypted/pkcs1-oaep + - chZJKMXgc2Ns2QMcg0tygOMvsUGO/uOleF0tYJg/QIYGC7DvZpA6kd352ZcR/9vysrQ10gSFc7k83jV5bEOmazAFP4M9loJBHrONVJOFa0pdH9iPnB7hj1blpul4ciUQlkzjTmXPh03og66I9LST3M2lp1utFJGjVF/ugpabJH7vXDKdT/x8MhftDuDUL4e30bqDSyGbQiHMsqKdNcTry0fYXoepopcUnr+K8BJK6Pi3tXRfb2L/0vS82Q11h4HTFkiWg0bAG8ZDiLjmSrXRL8baEgUdCJsj+c5d58vNnu6Hw+yYbU+2ofiv4EUENALGirnutpkRdfNqG9mTP8u0YyaKf41R1v7NwyIGKC5RRn9t/m5mJp0cYBKVfPsQfczV24R5y5v761hm1VvEDBWD4cWZebE5xEh+AqjmU08aqJGmaBzvHWIDt6rNbM7eiyvT5zHtxUjd8fMRLgZTrnMRfPkdDqhkSIcefpGO6TJfZqb//68vOVOAKqWbOFbXG+/HUCVmNiq/8cKjjnZZPKiOaBOpxqziebJt9jpYUmycNeRrIC1CdWH8ISoB0KTJusOtR4UeIAvUgMVKEzq1zmwO4ikDTKJqVWfhpNnxVO6qTShYsIyjoiz2lAsM4jFi719nZsrqlUhkpretY/is/DLKPetTyG55nmA1f0ooFy07rX4= + +- secret: + name: secret2 + data: + password: !encrypted/pkcs1-oaep + - mzrD4LENlK6n6kHQRJuokjxcOS6zNPQnCfuMtcNikmxAB9mFZ/MwAZonSU5ZwdOifC6maoalthhV88uya9S15HK9EXdZ4d1QCb1QmpwFh9edSei45+LH1UHYPoTS49RTl2HXN0hrIi+9mWMczAFnB3vkKrwSbuhblrd6wQna3gJi3Ny+7XUraqksS8xspy8Ft88U4fNF1a8IGv4ThoVnp7iyHwUeA614gwMn3drSHAKVBdgxREPVB11hQNXhcIw8PXKL8CC1mCLzabFz++MF6QLRlDT5X7b7YWyFESvgWBkw1DDeCS8nmZ7v+fbr8R+gH/J3VTqxY2pFzr+maEPW674Bs/tTfGkjqWi8I5jTPdLoNqJZl8JtlWhGFVGbHd9EdoZ+zmtOvqV2cKgTktgIp+5cUfRDBJHMcq8kfXQabpqkSCTYQQYx7Vhpu9WHclnodHxnDnivBGBhP/BVSij7nAy86nqtInykOrkkJekN9NbELpSHhL4F7xmTgrMonqZd79Btr5wshD9xPZ7I0JvEQi2nOPBkyxhrpHONEdQH6pmQMEOB1lWAm7rKw532AB9fuY4c7BLXBlXuqJQgflIziLkoMExiD5iKBNc/xHTC19mzYalw3OrnCfyWeeAIXeIdMxbLUFVSOOKcWl56LAv9hSelDI2M+IeOYPRWGgCy5Cs= + +- secret: + name: secret3 + data: + password: !encrypted/pkcs1-oaep + - d4lPbjytcB7Gw5/Jy5hmIQ4bnCeOznnRMUTbXoEckFVv5OUf8A0TwaAPdwgLqPNCJtX5AsYhvPRcf4zS9jlUs4PJpxM/zxopCzROO35kGoFdU4/gut0AWw+0PkHk6WElFmFiXxhn/BOsVOXvUJ72YiAcRsZeIXyLwG424nRq2LYn2PZXpcN9jF3Ag1Dj5ACDEPAuevfjwqYA26oqhG1tByQe1g3Sa6pvNuskrL5yO3Au1ACgyDFvfqfw71KVIRNt2n1ta3xCY8MuCUn18JR+SGRERR/14nfnW7QULBKr3ObTrGXGohKTWr1JdENXHXyPIrrsTnxaZcb3rnOUz3B+rSjMMGFw/PjsN5j9UHtAtsQStq+LLYgeV2U/zDCX7eOBNLqgWXBnRvRwwgDFToazCqOJT4I7yI4BdL2cbG20g4xrW9SZ22VPlV76LlcLQxZ61j6YNNmG3XVPHNPA0zt6RG3JWF97NdoJrX5Z3Jzm767ffaQzUfpkAWZKfjI06Mi6lEYmKpxh2saY9KozuOnik+ULgc4QWrWBM3lILvkhyhfWw4oaUuT3dDwaIAozWe6KoGY5hpJLwf4J1dtrXqjndolKJSleYBXLHjaTz6qxrA/DO/r3aQL5I3dBL5uyb3iJtORtziN/s4TgnIDhi9ca4IHWxsejKjLPrJ1kmtL2bUk= + +- job: + name: parent-job-without-secret + pre-run: playbooks/pre.yaml + run: playbooks/run.yaml + post-run: playbooks/post.yaml + +- job: + name: parent-job + pre-run: playbooks/pre.yaml + run: playbooks/run.yaml + post-run: playbooks/post.yaml + secrets: + name: parent_secret + secret: secret3 + +- job: + name: no-pass + parent: parent-job + secrets: + name: secret + secret: secret1 + files: no-pass.txt + +- job: + name: pass + parent: parent-job + secrets: + name: secret + secret: secret1 + pass-to-parent: true + files: pass.txt + +- job: + name: override + parent: pass + secrets: + name: secret + secret: secret2 + pass-to-parent: true + files: override.txt + +- project: + gate: + jobs: + - no-pass + - pass + - override diff --git a/tests/fixtures/config/pass-to-parent/main.yaml b/tests/fixtures/config/pass-to-parent/main.yaml new file mode 100644 index 0000000000..208e274b13 --- /dev/null +++ b/tests/fixtures/config/pass-to-parent/main.yaml @@ -0,0 +1,8 @@ +- tenant: + name: tenant-one + source: + gerrit: + config-projects: + - common-config + untrusted-projects: + - org/project diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index d390d7aa4a..35a7fa2f4a 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -3785,6 +3785,166 @@ class TestSecretInheritance(ZuulTestCase): self.assertHistory([]) +class TestSecretPassToParent(ZuulTestCase): + tenant_config_file = 'config/pass-to-parent/main.yaml' + + def _getSecrets(self, job, pbtype): + secrets = [] + build = self.getJobFromHistory(job) + for pb in build.parameters[pbtype]: + secrets.append(pb['secrets']) + return secrets + + def test_secret_no_pass_to_parent(self): + # Test that secrets are not available in the parent if + # pass-to-parent is not set. + file_dict = {'no-pass.txt': ''} + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A', + files=file_dict) + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + self.assertEqual(A.data['status'], 'MERGED') + self.assertHistory([ + dict(name='no-pass', result='SUCCESS', changes='1,1'), + ]) + + self.assertEqual( + self._getSecrets('no-pass', 'playbooks'), + [{'parent_secret': {'password': 'password3'}}]) + self.assertEqual( + self._getSecrets('no-pass', 'pre_playbooks'), + [{'parent_secret': {'password': 'password3'}}]) + self.assertEqual( + self._getSecrets('no-pass', 'post_playbooks'), + [{'parent_secret': {'password': 'password3'}}]) + + def test_secret_pass_to_parent(self): + # Test that secrets are available in the parent if + # pass-to-parent is set. + file_dict = {'pass.txt': ''} + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A', + files=file_dict) + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + self.assertEqual(A.data['status'], 'MERGED') + self.assertHistory([ + dict(name='pass', result='SUCCESS', changes='1,1'), + ]) + + self.assertEqual( + self._getSecrets('pass', 'playbooks'), + [{'parent_secret': {'password': 'password3'}, + 'secret': {'password': 'password1'}}]) + self.assertEqual( + self._getSecrets('pass', 'pre_playbooks'), + [{'parent_secret': {'password': 'password3'}, + 'secret': {'password': 'password1'}}]) + self.assertEqual( + self._getSecrets('pass', 'post_playbooks'), + [{'parent_secret': {'password': 'password3'}, + 'secret': {'password': 'password1'}}]) + + def test_secret_override(self): + # Test that secrets passed to parents don't override existing + # secrets. + file_dict = {'override.txt': ''} + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A', + files=file_dict) + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + self.assertEqual(A.data['status'], 'MERGED') + self.assertHistory([ + dict(name='override', result='SUCCESS', changes='1,1'), + ]) + + self.assertEqual( + self._getSecrets('override', 'playbooks'), + [{'parent_secret': {'password': 'password3'}, + 'secret': {'password': 'password1'}}]) + self.assertEqual( + self._getSecrets('override', 'pre_playbooks'), + [{'parent_secret': {'password': 'password3'}, + 'secret': {'password': 'password1'}}]) + self.assertEqual( + self._getSecrets('override', 'post_playbooks'), + [{'parent_secret': {'password': 'password3'}, + 'secret': {'password': 'password1'}}]) + + def test_secret_ptp_trusted_untrusted(self): + # Test if we pass a secret to a parent and one of the parents + # is untrusted, the job becomes post-review. + file_dict = {'trusted-under-untrusted.txt': ''} + A = self.fake_gerrit.addFakeChange('common-config', 'master', 'A', + files=file_dict) + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + self.assertEqual(A.data['status'], 'MERGED') + self.assertHistory([ + dict(name='trusted-under-untrusted', + result='SUCCESS', changes='1,1'), + ]) + + self.assertEqual( + self._getSecrets('trusted-under-untrusted', 'playbooks'), + [{'secret': {'password': 'trustedpassword1'}}]) + self.assertEqual( + self._getSecrets('trusted-under-untrusted', 'pre_playbooks'), + [{'secret': {'password': 'trustedpassword1'}}]) + self.assertEqual( + self._getSecrets('trusted-under-untrusted', 'post_playbooks'), + [{'secret': {'password': 'trustedpassword1'}}]) + + B = self.fake_gerrit.addFakeChange('common-config', 'master', 'B', + files=file_dict) + self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + self.assertHistory([ + dict(name='trusted-under-untrusted', + result='SUCCESS', changes='1,1'), + ]) + self.assertIn('does not allow post-review', B.messages[0]) + + def test_secret_ptp_trusted_trusted(self): + # Test if we pass a secret to a parent and all of the parents + # are trusted, the job does not become post-review. + file_dict = {'trusted-under-trusted.txt': ''} + A = self.fake_gerrit.addFakeChange('common-config', 'master', 'A', + files=file_dict) + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + self.assertEqual(A.data['status'], 'MERGED') + self.assertHistory([ + dict(name='trusted-under-trusted', + result='SUCCESS', changes='1,1'), + ]) + + self.assertEqual( + self._getSecrets('trusted-under-trusted', 'playbooks'), + [{'secret': {'password': 'trustedpassword1'}}]) + self.assertEqual( + self._getSecrets('trusted-under-trusted', 'pre_playbooks'), + [{'secret': {'password': 'trustedpassword1'}}]) + self.assertEqual( + self._getSecrets('trusted-under-trusted', 'post_playbooks'), + [{'secret': {'password': 'trustedpassword1'}}]) + + B = self.fake_gerrit.addFakeChange('common-config', 'master', 'B', + files=file_dict) + self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + self.assertHistory([ + dict(name='trusted-under-trusted', + result='SUCCESS', changes='1,1'), + dict(name='trusted-under-trusted', + result='SUCCESS', changes='2,1'), + ]) + + class TestSecretLeaks(AnsibleZuulTestCase): tenant_config_file = 'config/secret-leaks/main.yaml' diff --git a/zuul/configloader.py b/zuul/configloader.py index 32c9cfd55f..9bc7aca26f 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -534,7 +534,8 @@ class JobParser(object): 'override-checkout': str} secret = {vs.Required('name'): str, - vs.Required('secret'): str} + vs.Required('secret'): str, + 'pass-to-parent': bool} semaphore = {vs.Required('name'): str, 'resources-first': bool} @@ -648,10 +649,15 @@ class JobParser(object): if isinstance(secret_config, str): secret_name = secret_config secret_alias = secret_config + secret_ptp = False else: secret_name = secret_config['secret'] secret_alias = secret_config['name'] - secrets.append((secret_name, secret_alias)) + secret_ptp = secret_config.get('pass-to-parent', False) + secret_use = model.SecretUse(secret_name, secret_alias) + secret_use.pass_to_parent = secret_ptp + secrets.append(secret_use) + job.secrets = tuple(secrets) # A job in an untrusted repo that uses secrets requires # special care. We must note this, and carry this flag diff --git a/zuul/model.py b/zuul/model.py index 85060b0b8a..cc389a7a26 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -24,6 +24,7 @@ from uuid import uuid4 import urllib.parse import textwrap import types +import itertools from zuul import change_matcher from zuul.lib.config import get_default @@ -822,6 +823,16 @@ class Secret(ConfigObject): return r +class SecretUse(ConfigObject): + """A use of a secret in a Job""" + + def __init__(self, name, alias): + super(SecretUse, self).__init__() + self.name = name + self.alias = alias + self.pass_to_parent = False + + class SourceContext(ConfigObject): """A reference to the branch of a project in configuration. @@ -921,13 +932,13 @@ class PlaybookContext(ConfigObject): def validateReferences(self, layout): # Verify that references to other objects in the layout are # valid. - for (secret_name, secret_alias) in self.secrets: - secret = layout.secrets.get(secret_name) + for secret_use in self.secrets: + secret = layout.secrets.get(secret_use.name) if secret is None: raise Exception( 'The secret "{name}" was not found.'.format( - name=secret_name)) - if secret_alias == 'zuul' or secret_alias == 'nodepool': + name=secret_use.name)) + if secret_use.alias == 'zuul' or secret_use.alias == 'nodepool': raise Exception('Secrets named "zuul" or "nodepool" ' 'are not allowed.') if not secret.source_context.isSameProject(self.source_context): @@ -935,20 +946,26 @@ class PlaybookContext(ConfigObject): "Unable to use secret {name}. Secrets must be " "defined in the same project in which they " "are used".format( - name=secret_name)) + name=secret_use.name)) # Decrypt a copy of the secret to verify it can be done secret.decrypt(self.source_context.project.private_secrets_key) def freezeSecrets(self, layout): secrets = [] - for (secret_name, secret_alias) in self.secrets: - secret = layout.secrets.get(secret_name) + for secret_use in self.secrets: + secret = layout.secrets.get(secret_use.name) decrypted_secret = secret.decrypt( self.source_context.project.private_secrets_key) - decrypted_secret.name = secret_alias + decrypted_secret.name = secret_use.alias secrets.append(decrypted_secret) self.decrypted_secrets = tuple(secrets) + def addSecrets(self, decrypted_secrets): + current_names = set([s.name for s in self.decrypted_secrets]) + new_secrets = [s for s in decrypted_secrets + if s.name not in current_names] + self.decrypted_secrets = self.decrypted_secrets + tuple(new_secrets) + def toDict(self): # Render to a dict to use in passing json to the executor secrets = {} @@ -1101,6 +1118,7 @@ class Job(ConfigObject): _implied_branch=None, _files=(), _irrelevant_files=(), + secrets=(), # secrets aren't inheritable ) self.inheritable_attributes = {} @@ -1456,6 +1474,28 @@ class Job(ConfigObject): # Freeze the nodeset self.nodeset = self.getNodeSet(layout) + # Pass secrets to parents + secrets_for_parents = [s for s in other.secrets if s.pass_to_parent] + if secrets_for_parents: + decrypted_secrets = [] + for secret_use in secrets_for_parents: + secret = layout.secrets.get(secret_use.name) + decrypted_secret = secret.decrypt( + other.source_context.project.private_secrets_key) + decrypted_secret.name = secret_use.alias + decrypted_secrets.append(decrypted_secret) + # Add the secrets to any existing playbooks. If any of + # them are in an untrusted project, then we've just given + # a secret to a playbook which can run in dynamic config, + # therefore it's no longer safe to run this job + # pre-review. The only way pass-to-parent can work with + # pre-review pipeline is if all playbooks are in the + # trusted context. + for pb in itertools.chain(self.pre_run, self.run, self.post_run): + pb.addSecrets(decrypted_secrets) + if not pb.source_context.trusted: + self.post_review = True + if other._get('run') is not None: other_run = self.freezePlaybooks(other.run, layout) self.run = other_run