From d4d21a151189a01d3721785c71ff61dc0f2df003 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Mon, 7 Jun 2021 15:43:57 -0400 Subject: [PATCH 01/12] Remove the managed flag from all existing EEs This flag henceforth is going to be used only for the "control plane" execution environments, which sysadmins will not be allowed to alter. --- .../0145_deregister_managed_ee_objs.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 awx/main/migrations/0145_deregister_managed_ee_objs.py diff --git a/awx/main/migrations/0145_deregister_managed_ee_objs.py b/awx/main/migrations/0145_deregister_managed_ee_objs.py new file mode 100644 index 0000000000..f9906b072b --- /dev/null +++ b/awx/main/migrations/0145_deregister_managed_ee_objs.py @@ -0,0 +1,21 @@ +# Generated by Django 2.2.16 on 2021-06-07 19:36 + +from django.db import migrations + + +def forwards(apps, schema_editor): + ExecutionEnvironment = apps.get_model('main', 'ExecutionEnvironment') + for row in ExecutionEnvironment.objects.filter(managed_by_tower=True): + row.managed_by_tower = False + row.save(update_fields=['managed_by_tower']) + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0144_event_partitions'), + ] + + operations = [ + migrations.RunPython(forwards), + ] From 9aa56b1247f0d36e860dcff521b4feaca10b1cb7 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Mon, 7 Jun 2021 15:55:15 -0400 Subject: [PATCH 02/12] Update the EE resolver logic so that the control plane managed EE is kept separate. --- awx/main/models/projects.py | 6 +++--- awx/main/utils/execution_environments.py | 6 +++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/awx/main/models/projects.py b/awx/main/models/projects.py index d2b62eac7e..1c34871205 100644 --- a/awx/main/models/projects.py +++ b/awx/main/models/projects.py @@ -32,7 +32,7 @@ from awx.main.models.jobs import Job from awx.main.models.mixins import ResourceMixin, TaskManagerProjectUpdateMixin, CustomVirtualEnvMixin, RelatedJobsMixin from awx.main.utils import update_scm_url, polymorphic from awx.main.utils.ansible import skip_directory, could_be_inventory, could_be_playbook -from awx.main.utils.execution_environments import get_default_execution_environment +from awx.main.utils.execution_environments import get_control_plane_execution_environment from awx.main.fields import ImplicitRoleField from awx.main.models.rbac import ( ROLE_SINGLETON_SYSTEM_ADMINISTRATOR, @@ -185,11 +185,11 @@ class ProjectOptions(models.Model): def resolve_execution_environment(self): """ - Project updates, themselves, will use the default execution environment. + Project updates, themselves, will use the control plane execution environment. Jobs using the project can use the default_environment, but the project updates are not flexible enough to allow customizing the image they use. """ - return get_default_execution_environment() + return get_control_plane_execution_environment() def get_project_path(self, check_if_exists=True): local_path = os.path.basename(self.local_path) diff --git a/awx/main/utils/execution_environments.py b/awx/main/utils/execution_environments.py index 67fee9566d..8598b7024f 100644 --- a/awx/main/utils/execution_environments.py +++ b/awx/main/utils/execution_environments.py @@ -6,10 +6,14 @@ from django.conf import settings from awx.main.models.execution_environments import ExecutionEnvironment +def get_control_plane_execution_environment(): + return ExecutionEnvironment.objects.filter(organization=None, managed_by_tower=True).first() + + def get_default_execution_environment(): if settings.DEFAULT_EXECUTION_ENVIRONMENT is not None: return settings.DEFAULT_EXECUTION_ENVIRONMENT - return ExecutionEnvironment.objects.filter(organization=None, managed_by_tower=True).first() + return ExecutionEnvironment.objects.filter(organization=None, managed_by_tower=False).first() def get_default_pod_spec(): From 9f1e8a1ae22b357f88c480f9e592799f0a2abc33 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 9 Jun 2021 10:02:26 -0400 Subject: [PATCH 03/12] Allow sysadmins to be able to change the pull field for managed EEs --- awx/api/views/__init__.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/awx/api/views/__init__.py b/awx/api/views/__init__.py index a96ac4508e..8d34dcb8a9 100644 --- a/awx/api/views/__init__.py +++ b/awx/api/views/__init__.py @@ -695,6 +695,7 @@ class TeamAccessList(ResourceAccessList): class ExecutionEnvironmentList(ListCreateAPIView): + always_allow_superuser = False model = models.ExecutionEnvironment serializer_class = serializers.ExecutionEnvironmentSerializer swagger_topic = "Execution Environments" @@ -702,10 +703,22 @@ class ExecutionEnvironmentList(ListCreateAPIView): class ExecutionEnvironmentDetail(RetrieveUpdateDestroyAPIView): + always_allow_superuser = False model = models.ExecutionEnvironment serializer_class = serializers.ExecutionEnvironmentSerializer swagger_topic = "Execution Environments" + def update(self, request, *args, **kwargs): + instance = self.get_object() + fields_to_check = ['name', 'description', 'organization', 'image', 'credential'] + if instance.managed_by_tower and request.user.can_access(models.ExecutionEnvironment, 'change', instance): + for field in fields_to_check: + left = getattr(instance, field, None) + right = request.data.get(field, None) + if left != right: + raise PermissionDenied(_("Only the 'pull' field can be edited for managed execution environments.")) + return super().update(request, *args, **kwargs) + class ExecutionEnvironmentJobTemplateList(SubListAPIView): From e6e1f97048ab600ea83ff472583e3de61a021ef1 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 9 Jun 2021 10:10:10 -0400 Subject: [PATCH 04/12] Add a signal handler to remove the default EE if it gets deleted --- awx/main/signals.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/awx/main/signals.py b/awx/main/signals.py index 492da999bc..fe3fdbc756 100644 --- a/awx/main/signals.py +++ b/awx/main/signals.py @@ -31,6 +31,7 @@ from crum.signals import current_user_getter # AWX from awx.main.models import ( ActivityStream, + ExecutionEnvironment, Group, Host, InstanceGroup, @@ -623,6 +624,12 @@ def deny_orphaned_approvals(sender, instance, **kwargs): approval.deny() +@receiver(pre_delete, sender=ExecutionEnvironment) +def remove_default_ee(sender, instance, **kwargs): + if instance.id == getattr(settings.DEFAULT_EXECUTION_ENVIRONMENT, 'id', None): + settings.DEFAULT_EXECUTION_ENVIRONMENT = None + + @receiver(post_save, sender=Session) def save_user_session_membership(sender, **kwargs): session = kwargs.get('instance', None) From 19da9955ceb30b1587c4d894cd8081b318375628 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 9 Jun 2021 10:13:36 -0400 Subject: [PATCH 05/12] Make sure that managed EEs can't be deleted --- awx/main/access.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/awx/main/access.py b/awx/main/access.py index f9a6983b5b..34fe973445 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -1370,6 +1370,8 @@ class ExecutionEnvironmentAccess(BaseAccess): return self.check_related('organization', Organization, data, obj=obj, mandatory=True, role_field='execution_environment_admin_role') def can_delete(self, obj): + if obj.managed_by_tower: + raise PermissionDenied return self.can_change(obj, None) From 8ede74a7f694804864f27072ddc42eed8079acc7 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 9 Jun 2021 10:19:26 -0400 Subject: [PATCH 06/12] Deal with the possibility of get_default_pod_spec not finding an EE --- awx/main/utils/execution_environments.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/awx/main/utils/execution_environments.py b/awx/main/utils/execution_environments.py index 8598b7024f..5bc01d879a 100644 --- a/awx/main/utils/execution_environments.py +++ b/awx/main/utils/execution_environments.py @@ -17,6 +17,9 @@ def get_default_execution_environment(): def get_default_pod_spec(): + ee = get_default_execution_environment() + if ee is None: + raise RuntimeError("Unable to find an execution environment.") return { "apiVersion": "v1", @@ -25,7 +28,7 @@ def get_default_pod_spec(): "spec": { "containers": [ { - "image": get_default_execution_environment().image, + "image": ee.image, "name": 'worker', "args": ['ansible-runner', 'worker', '--private-data-dir=/runner'], } From 7a16782ebf894446eeaef95c8a0558ed49ecde58 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 9 Jun 2021 11:50:35 -0400 Subject: [PATCH 07/12] Fix a problem with using PrimaryKeyRelatedField in our settings registry DRF, when using this field, short-circuits the call to .to_representation() when the value is None, since clearly you aren't going to be able to get the .pk attribute off of it in that case. We were previously unconditionally calling .to_representation() which throws an error when we try to clear the value of DEFAULT_EXECUTION_ENVIRONMENT. --- awx/conf/settings.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/awx/conf/settings.py b/awx/conf/settings.py index fc3e228cb4..7e7abeac54 100644 --- a/awx/conf/settings.py +++ b/awx/conf/settings.py @@ -23,6 +23,7 @@ import cachetools # AWX from awx.main.utils import encrypt_field, decrypt_field from awx.conf import settings_registry +from awx.conf.fields import PrimaryKeyRelatedField from awx.conf.models import Setting from awx.conf.migrations._reencrypt import decrypt_field as old_decrypt_field @@ -420,9 +421,9 @@ class SettingsWrapper(UserSettingsHolder): raise ImproperlyConfigured('Setting "{}" is read only.'.format(name)) try: - data = field.to_representation(value) + data = None if value is None and isinstance(field, PrimaryKeyRelatedField) else field.to_representation(value) setting_value = field.run_validation(data) - db_value = field.to_representation(setting_value) + db_value = None if setting_value is None and isinstance(field, PrimaryKeyRelatedField) else field.to_representation(setting_value) except Exception as e: logger.exception('Unable to assign value "%r" to setting "%s".', value, name, exc_info=True) raise e From 673afdf1b5d95686ef89954e43587f8c667f046d Mon Sep 17 00:00:00 2001 From: Shane McDonald Date: Wed, 9 Jun 2021 12:39:33 -0400 Subject: [PATCH 08/12] Only install ansible-core for collection tests Faster! --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index ee3e55b63e..9052b9a518 100644 --- a/Makefile +++ b/Makefile @@ -315,7 +315,7 @@ test_collection: if [ "$(VENV_BASE)" ]; then \ . $(VENV_BASE)/awx/bin/activate; \ fi && \ - pip install ansible && \ + pip install ansible-core && \ py.test $(COLLECTION_TEST_DIRS) -v # The python path needs to be modified so that the tests can find Ansible within the container # First we will use anything expility set as PYTHONPATH From 373cd9c20b9f307de9ad88dd51d1e8f2d2a851fd Mon Sep 17 00:00:00 2001 From: Shane McDonald Date: Wed, 9 Jun 2021 12:41:24 -0400 Subject: [PATCH 09/12] Remove usage of AWXReceptorJob in metadata.py --- awx/api/metadata.py | 4 ++-- awx/main/tasks.py | 7 ++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/awx/api/metadata.py b/awx/api/metadata.py index ced1f3032d..5b8cf2ccb3 100644 --- a/awx/api/metadata.py +++ b/awx/api/metadata.py @@ -24,7 +24,7 @@ from rest_framework.request import clone_request from awx.api.fields import ChoiceNullField from awx.main.fields import JSONField, ImplicitRoleField from awx.main.models import NotificationTemplate -from awx.main.tasks import AWXReceptorJob +from awx.main.utils.execution_environments import get_default_pod_spec # Polymorphic from polymorphic.models import PolymorphicModel @@ -211,7 +211,7 @@ class Metadata(metadata.SimpleMetadata): continue if field == "pod_spec_override": - meta['default'] = AWXReceptorJob().pod_definition + meta['default'] = get_default_pod_spec() # Add type choices if available from the serializer. if field == 'type' and hasattr(serializer, 'get_type_choices'): diff --git a/awx/main/tasks.py b/awx/main/tasks.py index b1e41fbe52..0a06abab49 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -2879,7 +2879,7 @@ class TransmitterThread(threading.Thread): class AWXReceptorJob: - def __init__(self, task=None, runner_params=None): + def __init__(self, task, runner_params=None): self.task = task self.runner_params = runner_params self.unit_id = None @@ -3034,10 +3034,7 @@ class AWXReceptorJob: @property def pod_definition(self): - if self.task and self.task.instance.execution_environment: - ee = self.task.instance.execution_environment - else: - ee = get_default_execution_environment() + ee = self.task.instance.execution_environment default_pod_spec = get_default_pod_spec() From f882ac420d473f6b4950ce7e62118250375dd63e Mon Sep 17 00:00:00 2001 From: Shane McDonald Date: Wed, 9 Jun 2021 12:41:34 -0400 Subject: [PATCH 10/12] Fix tests --- .../tests/functional/api/test_instance_group.py | 4 +++- awx/main/tests/functional/conftest.py | 9 +++++++-- .../task_management/test_container_groups.py | 3 ++- .../functional/test_execution_environments.py | 14 -------------- .../functional/test_inventory_source_injectors.py | 3 ++- awx/main/tests/unit/test_tasks.py | 3 ++- awx_collection/test/awx/conftest.py | 2 +- 7 files changed, 17 insertions(+), 21 deletions(-) delete mode 100644 awx/main/tests/functional/test_execution_environments.py diff --git a/awx/main/tests/functional/api/test_instance_group.py b/awx/main/tests/functional/api/test_instance_group.py index 22309df142..8e4cc03844 100644 --- a/awx/main/tests/functional/api/test_instance_group.py +++ b/awx/main/tests/functional/api/test_instance_group.py @@ -105,7 +105,9 @@ def test_delete_instance_group_jobs_running(delete, instance_group_jobs_running, @pytest.mark.django_db -def test_delete_rename_tower_instance_group_prevented(delete, options, tower_instance_group, instance_group, user, patch, execution_environment): +def test_delete_rename_tower_instance_group_prevented( + delete, options, tower_instance_group, instance_group, user, patch, control_plane_execution_environment, default_job_execution_environment +): url = reverse("api:instance_group_detail", kwargs={'pk': tower_instance_group.pk}) super_user = user('bob', True) diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py index 1150c025aa..a37f34f919 100644 --- a/awx/main/tests/functional/conftest.py +++ b/awx/main/tests/functional/conftest.py @@ -824,5 +824,10 @@ def slice_job_factory(slice_jt_factory): @pytest.fixture -def execution_environment(): - return ExecutionEnvironment.objects.create(name="test-ee", description="test-ee", managed_by_tower=True) +def control_plane_execution_environment(): + return ExecutionEnvironment.objects.create(name="Control Plane EE", managed_by_tower=True) + + +@pytest.fixture +def default_job_execution_environment(): + return ExecutionEnvironment.objects.create(name="Default Job EE", managed_by_tower=False) diff --git a/awx/main/tests/functional/task_management/test_container_groups.py b/awx/main/tests/functional/task_management/test_container_groups.py index 7bbdac218d..33a0f72392 100644 --- a/awx/main/tests/functional/task_management/test_container_groups.py +++ b/awx/main/tests/functional/task_management/test_container_groups.py @@ -35,7 +35,8 @@ def test_containerized_job(containerized_job): @pytest.mark.django_db -def test_kubectl_ssl_verification(containerized_job, execution_environment): +def test_kubectl_ssl_verification(containerized_job, default_job_execution_environment): + containerized_job.execution_environment = default_job_execution_environment cred = containerized_job.instance_group.credential cred.inputs['verify_ssl'] = True key_material = subprocess.run('openssl genrsa 2> /dev/null', shell=True, check=True, stdout=subprocess.PIPE) diff --git a/awx/main/tests/functional/test_execution_environments.py b/awx/main/tests/functional/test_execution_environments.py deleted file mode 100644 index c47f0d9859..0000000000 --- a/awx/main/tests/functional/test_execution_environments.py +++ /dev/null @@ -1,14 +0,0 @@ -import pytest - -from awx.main.models import ExecutionEnvironment - - -@pytest.mark.django_db -def test_execution_environment_creation(execution_environment, organization): - execution_env = ExecutionEnvironment.objects.create( - name='Hello Environment', image='', organization=organization, managed_by_tower=False, credential=None, pull='missing' - ) - assert type(execution_env) is type(execution_environment) - assert execution_env.organization == organization - assert execution_env.name == 'Hello Environment' - assert execution_env.pull == 'missing' diff --git a/awx/main/tests/functional/test_inventory_source_injectors.py b/awx/main/tests/functional/test_inventory_source_injectors.py index aff0356b59..f011e51104 100644 --- a/awx/main/tests/functional/test_inventory_source_injectors.py +++ b/awx/main/tests/functional/test_inventory_source_injectors.py @@ -182,7 +182,8 @@ def create_reference_data(source_dir, env, content): @pytest.mark.django_db @pytest.mark.parametrize('this_kind', CLOUD_PROVIDERS) def test_inventory_update_injected_content(this_kind, inventory, fake_credential_factory): - ExecutionEnvironment.objects.create(name='test EE', managed_by_tower=True) + ExecutionEnvironment.objects.create(name='Control Plane EE', managed_by_tower=True) + ExecutionEnvironment.objects.create(name='Default Job EE', managed_by_tower=False) injector = InventorySource.injectors[this_kind] if injector.plugin_name is None: diff --git a/awx/main/tests/unit/test_tasks.py b/awx/main/tests/unit/test_tasks.py index 4bb1e33c10..ec6c9d1bee 100644 --- a/awx/main/tests/unit/test_tasks.py +++ b/awx/main/tests/unit/test_tasks.py @@ -588,7 +588,8 @@ class TestGenericRun: @pytest.mark.django_db class TestAdhocRun(TestJobExecution): def test_options_jinja_usage(self, adhoc_job, adhoc_update_model_wrapper): - ExecutionEnvironment.objects.create(name='test EE', managed_by_tower=True) + ExecutionEnvironment.objects.create(name='Control Plane EE', managed_by_tower=True) + ExecutionEnvironment.objects.create(name='Default Job EE', managed_by_tower=False) adhoc_job.module_args = '{{ ansible_ssh_pass }}' adhoc_job.websocket_emit_status = mock.Mock() diff --git a/awx_collection/test/awx/conftest.py b/awx_collection/test/awx/conftest.py index dcd75fee11..728df5e15c 100644 --- a/awx_collection/test/awx/conftest.py +++ b/awx_collection/test/awx/conftest.py @@ -265,7 +265,7 @@ def silence_warning(): @pytest.fixture def execution_environment(): - return ExecutionEnvironment.objects.create(name="test-ee", description="test-ee", managed_by_tower=True) + return ExecutionEnvironment.objects.create(name="test-ee", description="test-ee", managed_by_tower=False) @pytest.fixture(scope='session', autouse=True) From 2d1a859719ec2a5a7fd050b8f2ce5b7603fb4b25 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 9 Jun 2021 13:16:49 -0400 Subject: [PATCH 11/12] Remove unused import --- awx/main/tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 0a06abab49..e2eab4c3c3 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -98,7 +98,7 @@ from awx.main.utils import ( parse_yaml_or_json, cleanup_new_process, ) -from awx.main.utils.execution_environments import get_default_execution_environment, get_default_pod_spec, CONTAINER_ROOT, to_container_path +from awx.main.utils.execution_environments import get_default_pod_spec, CONTAINER_ROOT, to_container_path from awx.main.utils.ansible import read_ansible_config from awx.main.utils.external_logging import reconfigure_rsyslog from awx.main.utils.safe_yaml import safe_dump, sanitize_jinja From 486bcd80f826619d74cb0c41314254ac754d1b0d Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 9 Jun 2021 15:45:02 -0400 Subject: [PATCH 12/12] Make sure that the delete capability isn't hardcoded to be the same as edit --- awx/main/access.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/access.py b/awx/main/access.py index 34fe973445..b9a2dfa3da 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -465,7 +465,7 @@ class BaseAccess(object): if display_method == 'schedule': user_capabilities['schedule'] = user_capabilities['start'] continue - elif display_method == 'delete' and not isinstance(obj, (User, UnifiedJob, CredentialInputSource)): + elif display_method == 'delete' and not isinstance(obj, (User, UnifiedJob, CredentialInputSource, ExecutionEnvironment)): user_capabilities['delete'] = user_capabilities['edit'] continue elif display_method == 'copy' and isinstance(obj, (Group, Host)):