diff --git a/doc/source/user/config.rst b/doc/source/user/config.rst index 76f73c3407..c4f09de5e2 100644 --- a/doc/source/user/config.rst +++ b/doc/source/user/config.rst @@ -738,6 +738,12 @@ unless otherwise specified: be installed (and therefore referenced from Ansible), the `name` attribute may be used to specify an alternate. + A job automatically has the project in which it is defined added to + the roles path if that project appears to contain a role or `roles/` + directory. By default, the project is added to the path under its + own name, however, that may be changed by explicitly listing the + project in the roles list in the usual way. + .. note:: galaxy roles are not yet implemented **galaxy** diff --git a/tests/fixtures/config/implicit-roles/git/common-config/zuul.yaml b/tests/fixtures/config/implicit-roles/git/common-config/zuul.yaml new file mode 100644 index 0000000000..ba91fb5557 --- /dev/null +++ b/tests/fixtures/config/implicit-roles/git/common-config/zuul.yaml @@ -0,0 +1,12 @@ +- pipeline: + name: check + manager: independent + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + verified: 1 + failure: + gerrit: + verified: -1 diff --git a/tests/fixtures/config/implicit-roles/git/org_norole-project/.zuul.yaml b/tests/fixtures/config/implicit-roles/git/org_norole-project/.zuul.yaml new file mode 100644 index 0000000000..74c8e8ea32 --- /dev/null +++ b/tests/fixtures/config/implicit-roles/git/org_norole-project/.zuul.yaml @@ -0,0 +1,15 @@ +- job: + name: implicit-role-fail + +- job: + name: explicit-role-fail + attempts: 1 + roles: + - zuul: org/norole-project + +- project: + name: org/norole-project + check: + jobs: + - implicit-role-fail + - explicit-role-fail diff --git a/tests/fixtures/config/implicit-roles/git/org_norole-project/README b/tests/fixtures/config/implicit-roles/git/org_norole-project/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/implicit-roles/git/org_norole-project/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/implicit-roles/git/org_norole-project/playbooks/explicit-role-fail.yaml b/tests/fixtures/config/implicit-roles/git/org_norole-project/playbooks/explicit-role-fail.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/implicit-roles/git/org_norole-project/playbooks/explicit-role-fail.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/implicit-roles/git/org_norole-project/playbooks/implicit-role-fail.yaml b/tests/fixtures/config/implicit-roles/git/org_norole-project/playbooks/implicit-role-fail.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/implicit-roles/git/org_norole-project/playbooks/implicit-role-fail.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/implicit-roles/git/org_role-project/.zuul.yaml b/tests/fixtures/config/implicit-roles/git/org_role-project/.zuul.yaml new file mode 100644 index 0000000000..42cae95dd7 --- /dev/null +++ b/tests/fixtures/config/implicit-roles/git/org_role-project/.zuul.yaml @@ -0,0 +1,15 @@ +- job: + name: implicit-role-ok + +- job: + name: explicit-role-ok + roles: + - zuul: org/role-project + name: role-name + +- project: + name: org/role-project + check: + jobs: + - implicit-role-ok + - explicit-role-ok diff --git a/tests/fixtures/config/implicit-roles/git/org_role-project/README b/tests/fixtures/config/implicit-roles/git/org_role-project/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/implicit-roles/git/org_role-project/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/implicit-roles/git/org_role-project/playbooks/explicit-role-ok.yaml b/tests/fixtures/config/implicit-roles/git/org_role-project/playbooks/explicit-role-ok.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/implicit-roles/git/org_role-project/playbooks/explicit-role-ok.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/implicit-roles/git/org_role-project/playbooks/implicit-role-ok.yaml b/tests/fixtures/config/implicit-roles/git/org_role-project/playbooks/implicit-role-ok.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/implicit-roles/git/org_role-project/playbooks/implicit-role-ok.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/implicit-roles/git/org_role-project/roles/README b/tests/fixtures/config/implicit-roles/git/org_role-project/roles/README new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/fixtures/config/implicit-roles/main.yaml b/tests/fixtures/config/implicit-roles/main.yaml new file mode 100644 index 0000000000..d5e481a543 --- /dev/null +++ b/tests/fixtures/config/implicit-roles/main.yaml @@ -0,0 +1,9 @@ +- tenant: + name: tenant-one + source: + gerrit: + config-projects: + - common-config + untrusted-projects: + - org/norole-project + - org/role-project diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index 734c45c954..b162469434 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -716,9 +716,7 @@ class TestProjectKeys(ZuulTestCase): self.assertEqual(4096, private_key.key_size) -class TestRoles(ZuulTestCase): - tenant_config_file = 'config/roles/main.yaml' - +class RoleTestCase(ZuulTestCase): def _assertRolePath(self, build, playbook, content): path = os.path.join(self.test_root, build.uuid, 'ansible', playbook, 'ansible.cfg') @@ -738,6 +736,10 @@ class TestRoles(ZuulTestCase): "Should have no roles_path line in %s" % (playbook,)) + +class TestRoles(RoleTestCase): + tenant_config_file = 'config/roles/main.yaml' + def test_role(self): # This exercises a proposed change to a role being checked out # and used. @@ -822,6 +824,57 @@ class TestRoles(ZuulTestCase): A.messages[-1]) +class TestImplicitRoles(RoleTestCase): + tenant_config_file = 'config/implicit-roles/main.yaml' + + def test_missing_roles(self): + # Test implicit and explicit roles for a project which does + # not have roles. The implicit role should be silently + # ignored since the project doesn't supply roles, but if a + # user declares an explicit role, it should error. + self.executor_server.hold_jobs_in_build = True + A = self.fake_gerrit.addFakeChange('org/norole-project', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + self.assertEqual(len(self.builds), 2) + build = self.getBuildByName('implicit-role-fail') + self._assertRolePath(build, 'playbook_0', None) + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + # The retry_limit doesn't get recorded + self.assertHistory([ + dict(name='implicit-role-fail', result='SUCCESS', changes='1,1'), + ]) + + def test_roles(self): + # Test implicit and explicit roles for a project which does + # have roles. In both cases, we should end up with the role + # in the path. In the explicit case, ensure we end up with + # the name we specified. + self.executor_server.hold_jobs_in_build = True + A = self.fake_gerrit.addFakeChange('org/role-project', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + self.assertEqual(len(self.builds), 2) + build = self.getBuildByName('implicit-role-ok') + self._assertRolePath(build, 'playbook_0', 'role_0') + + build = self.getBuildByName('explicit-role-ok') + self._assertRolePath(build, 'playbook_0', 'role_0') + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + self.assertHistory([ + dict(name='implicit-role-ok', result='SUCCESS', changes='1,1'), + dict(name='explicit-role-ok', result='SUCCESS', changes='1,1'), + ], ordered=False) + + class TestShadow(ZuulTestCase): tenant_config_file = 'config/shadow/main.yaml' diff --git a/zuul/configloader.py b/zuul/configloader.py index f8e2d15bf5..48d9134bba 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -425,14 +425,19 @@ class JobParser(object): # Roles are part of the playbook context so we must establish # them earlier than playbooks. + roles = [] if 'roles' in conf: - roles = [] for role in conf.get('roles', []): if 'zuul' in role: r = JobParser._makeZuulRole(tenant, job, role) if r: roles.append(r) - job.addRoles(roles) + # A job's repo should be an implicit role source for that job, + # but not in a project-pipeline variant. + if not project_pipeline: + r = JobParser._makeImplicitRole(job) + roles.insert(0, r) + job.addRoles(roles) for pre_run_name in as_list(conf.get('pre-run')): pre_run = model.PlaybookContext(job.source_context, @@ -554,6 +559,15 @@ class JobParser(object): project.connection_name, project.name) + @staticmethod + def _makeImplicitRole(job): + project = job.source_context.project + name = project.name.split('/')[-1] + return model.ZuulRole(name, + project.connection_name, + project.name, + implicit=True) + class ProjectTemplateParser(object): log = logging.getLogger("zuul.ProjectTemplateParser") diff --git a/zuul/executor/server.py b/zuul/executor/server.py index f291dce9cd..ae362638fe 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -51,6 +51,10 @@ class ExecutorError(Exception): pass +class RoleNotFoundError(ExecutorError): + pass + + class Watchdog(object): def __init__(self, timeout, function, args): self.timeout = timeout @@ -1137,12 +1141,14 @@ class AnsibleJob(object): if os.path.isdir(d): # This repo has a collection of roles if not trusted: + self._blockPluginDirs(d) for entry in os.listdir(d): - if os.path.isdir(os.path.join(d, entry)): - self._blockPluginDirs(os.path.join(d, entry)) + entry_path = os.path.join(d, entry) + if os.path.isdir(entry_path): + self._blockPluginDirs(entry_path) return d # It is neither a bare role, nor a collection of roles - raise ExecutorError("Unable to find role in %s" % (path,)) + raise RoleNotFoundError("Unable to find role in %s" % (path,)) def prepareZuulRole(self, jobdir_playbook, role, args, root): self.log.debug("Prepare zuul role for %s" % (role,)) @@ -1183,10 +1189,17 @@ class AnsibleJob(object): raise ExecutorError("Invalid role name %s", name) os.symlink(path, link) - role_path = self.findRole(link, trusted=jobdir_playbook.trusted) + try: + role_path = self.findRole(link, trusted=jobdir_playbook.trusted) + except RoleNotFoundError: + if role['implicit']: + self.log.info("Implicit role not found in %s", link) + return + raise if role_path is None: # In the case of a bare role, add the containing directory role_path = root + self.log.debug("Adding role path %s", role_path) jobdir_playbook.roles_path.append(role_path) def prepareAnsibleFiles(self, args): diff --git a/zuul/model.py b/zuul/model.py index 1df70dbc06..693b729374 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -702,10 +702,12 @@ class Role(object, metaclass=abc.ABCMeta): class ZuulRole(Role): """A reference to an ansible role in a Zuul project.""" - def __init__(self, target_name, connection_name, project_name): + def __init__(self, target_name, connection_name, project_name, + implicit=False): super(ZuulRole, self).__init__(target_name) self.connection_name = connection_name self.project_name = project_name + self.implicit = implicit def __repr__(self): return '' % (self.project_name, self.target_name) @@ -715,6 +717,8 @@ class ZuulRole(Role): def __eq__(self, other): if not isinstance(other, ZuulRole): return False + # Implicit is not consulted for equality so that we can handle + # implicit to explicit conversions. return (super(ZuulRole, self).__eq__(other) and self.connection_name == other.connection_name and self.project_name == other.project_name) @@ -725,6 +729,7 @@ class ZuulRole(Role): d['type'] = 'zuul' d['connection'] = self.connection_name d['project'] = self.project_name + d['implicit'] = self.implicit return d @@ -867,11 +872,31 @@ class Job(object): self.run = self.implied_run def addRoles(self, roles): - newroles = list(self.roles) + newroles = [] + # Start with a copy of the existing roles, but if any of them + # are implicit roles which are identified as explicit in the + # new roles list, replace them with the explicit version. + changed = False + for existing_role in self.roles: + if existing_role in roles: + new_role = roles[roles.index(existing_role)] + else: + new_role = None + if (new_role and + isinstance(new_role, ZuulRole) and + isinstance(existing_role, ZuulRole) and + existing_role.implicit and not new_role.implicit): + newroles.append(new_role) + changed = True + else: + newroles.append(existing_role) + # Now add the new roles. for role in reversed(roles): if role not in newroles: newroles.insert(0, role) - self.roles = tuple(newroles) + changed = True + if changed: + self.roles = tuple(newroles) def updateVariables(self, other_vars): v = self.variables