diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 5b50bc999c..941209ed99 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -169,7 +169,7 @@ SUMMARIZABLE_FK_FIELDS = { 'inventory_source': ('id', 'name', 'source', 'last_updated', 'status'), 'role': ('id', 'role_field'), 'notification_template': DEFAULT_SUMMARY_FIELDS, - 'instance_group': ('id', 'name', 'controller_id', 'is_container_group'), + 'instance_group': ('id', 'name', 'is_container_group'), 'insights_credential': DEFAULT_SUMMARY_FIELDS, 'source_credential': DEFAULT_SUMMARY_FIELDS + ('kind', 'cloud', 'credential_type_id'), 'target_credential': DEFAULT_SUMMARY_FIELDS + ('kind', 'cloud', 'credential_type_id'), @@ -4816,10 +4816,6 @@ class InstanceGroupSerializer(BaseSerializer): ) jobs_total = serializers.IntegerField(help_text=_('Count of all jobs that target this instance group'), read_only=True) instances = serializers.SerializerMethodField() - is_controller = serializers.BooleanField(help_text=_('Indicates whether instance group controls any other group'), read_only=True) - is_isolated = serializers.BooleanField( - help_text=_('Indicates whether instances in this group are isolated.' 'Isolated groups have a designated controller group.'), read_only=True - ) is_container_group = serializers.BooleanField( required=False, help_text=_('Indicates whether instances in this group are containerized.' 'Containerized groups have a designated Openshift or Kubernetes cluster.'), @@ -4867,9 +4863,6 @@ class InstanceGroupSerializer(BaseSerializer): "jobs_running", "jobs_total", "instances", - "controller", - "is_controller", - "is_isolated", "is_container_group", "credential", "policy_instance_percentage", @@ -4883,8 +4876,6 @@ class InstanceGroupSerializer(BaseSerializer): res = super(InstanceGroupSerializer, self).get_related(obj) res['jobs'] = self.reverse('api:instance_group_unified_jobs_list', kwargs={'pk': obj.pk}) res['instances'] = self.reverse('api:instance_group_instance_list', kwargs={'pk': obj.pk}) - if obj.controller_id: - res['controller'] = self.reverse('api:instance_group_detail', kwargs={'pk': obj.controller_id}) if obj.credential: res['credential'] = self.reverse('api:credential_detail', kwargs={'pk': obj.credential_id}) @@ -4896,10 +4887,6 @@ class InstanceGroupSerializer(BaseSerializer): raise serializers.ValidationError(_('Duplicate entry {}.').format(instance_name)) if not Instance.objects.filter(hostname=instance_name).exists(): raise serializers.ValidationError(_('{} is not a valid hostname of an existing instance.').format(instance_name)) - if Instance.objects.get(hostname=instance_name).is_isolated(): - raise serializers.ValidationError(_('Isolated instances may not be added or removed from instances groups via the API.')) - if self.instance and self.instance.controller_id is not None: - raise serializers.ValidationError(_('Isolated instance group membership may not be managed via the API.')) if value and self.instance and self.instance.is_container_group: raise serializers.ValidationError(_('Containerized instances may not be managed via the API')) return value diff --git a/awx/api/views/__init__.py b/awx/api/views/__init__.py index 789abe11e6..58c6746b6c 100644 --- a/awx/api/views/__init__.py +++ b/awx/api/views/__init__.py @@ -418,14 +418,6 @@ class InstanceGroupDetail(RelatedJobsPreventDeleteMixin, RetrieveUpdateDestroyAP data.pop('policy_instance_list', None) return super(InstanceGroupDetail, self).update_raw_data(data) - def destroy(self, request, *args, **kwargs): - instance = self.get_object() - if instance.controller is not None: - raise PermissionDenied(detail=_("Isolated Groups can not be removed from the API")) - if instance.controlled_groups.count(): - raise PermissionDenied(detail=_("Instance Groups acting as a controller for an Isolated Group can not be removed from the API")) - return super(InstanceGroupDetail, self).destroy(request, *args, **kwargs) - class InstanceGroupUnifiedJobsList(SubListAPIView): diff --git a/awx/api/views/mixin.py b/awx/api/views/mixin.py index ea2e8b38d4..0ab35e71b5 100644 --- a/awx/api/views/mixin.py +++ b/awx/api/views/mixin.py @@ -85,15 +85,6 @@ class InstanceGroupMembershipMixin(object): ig_obj.save(update_fields=['policy_instance_list']) return response - def is_valid_relation(self, parent, sub, created=False): - if sub.is_isolated(): - return {'error': _('Isolated instances may not be added or removed from instances groups via the API.')} - if self.parent_model is InstanceGroup: - ig_obj = self.get_parent_object() - if ig_obj.controller_id is not None: - return {'error': _('Isolated instance group membership may not be managed via the API.')} - return None - def unattach_validate(self, request): (sub_id, res) = super(InstanceGroupMembershipMixin, self).unattach_validate(request) if res: diff --git a/awx/main/tests/functional/api/test_instance_group.py b/awx/main/tests/functional/api/test_instance_group.py index c3cf44fd74..884856eed0 100644 --- a/awx/main/tests/functional/api/test_instance_group.py +++ b/awx/main/tests/functional/api/test_instance_group.py @@ -20,13 +20,7 @@ def tower_instance_group(): @pytest.fixture def instance(): - instance = Instance.objects.create(hostname='iso') - return instance - - -@pytest.fixture -def non_iso_instance(): - return Instance.objects.create(hostname='iamnotanisolatedinstance') + return Instance.objects.create(hostname='instance') @pytest.fixture @@ -36,15 +30,6 @@ def instance_group(job_factory): return ig -@pytest.fixture -def isolated_instance_group(instance_group, instance): - ig = InstanceGroup(name="iso", controller=instance_group) - ig.save() - ig.instances.set([instance]) - ig.save() - return ig - - @pytest.fixture def containerized_instance_group(instance_group, kube_credential): ig = InstanceGroup(name="container") @@ -97,26 +82,6 @@ def source_model(request): return request.getfixturevalue(request.param) -@pytest.mark.django_db -def test_instance_group_is_controller(instance_group, isolated_instance_group, non_iso_instance): - assert not isolated_instance_group.is_controller - assert instance_group.is_controller - - instance_group.instances.set([non_iso_instance]) - - assert instance_group.is_controller - - -@pytest.mark.django_db -def test_instance_group_is_isolated(instance_group, isolated_instance_group): - assert not instance_group.is_isolated - assert isolated_instance_group.is_isolated - - isolated_instance_group.instances.set([]) - - assert isolated_instance_group.is_isolated - - @pytest.mark.django_db def test_delete_instance_group_jobs(delete, instance_group_jobs_successful, instance_group, admin): url = reverse("api:instance_group_detail", kwargs={'pk': instance_group.pk}) @@ -160,61 +125,6 @@ def test_delete_rename_tower_instance_group_prevented(delete, options, tower_ins patch(url, {'name': 'foobar'}, super_user, expect=200) -@pytest.mark.django_db -def test_prevent_delete_iso_and_control_groups(delete, isolated_instance_group, admin): - iso_url = reverse("api:instance_group_detail", kwargs={'pk': isolated_instance_group.pk}) - controller_url = reverse("api:instance_group_detail", kwargs={'pk': isolated_instance_group.controller.pk}) - delete(iso_url, None, admin, expect=403) - delete(controller_url, None, admin, expect=403) - - -@pytest.mark.django_db -def test_prevent_isolated_instance_added_to_non_isolated_instance_group(post, admin, instance, instance_group, isolated_instance_group): - url = reverse("api:instance_group_instance_list", kwargs={'pk': instance_group.pk}) - - assert True is instance.is_isolated() - resp = post(url, {'associate': True, 'id': instance.id}, admin, expect=400) - assert u"Isolated instances may not be added or removed from instances groups via the API." == resp.data['error'] - - -@pytest.mark.django_db -def test_prevent_isolated_instance_added_to_non_isolated_instance_group_via_policy_list(patch, admin, instance, instance_group, isolated_instance_group): - url = reverse("api:instance_group_detail", kwargs={'pk': instance_group.pk}) - - assert True is instance.is_isolated() - resp = patch(url, {'policy_instance_list': [instance.hostname]}, user=admin, expect=400) - assert [u"Isolated instances may not be added or removed from instances groups via the API."] == resp.data['policy_instance_list'] - assert instance_group.policy_instance_list == [] - - -@pytest.mark.django_db -def test_prevent_isolated_instance_removal_from_isolated_instance_group(post, admin, instance, instance_group, isolated_instance_group): - url = reverse("api:instance_group_instance_list", kwargs={'pk': isolated_instance_group.pk}) - - assert True is instance.is_isolated() - resp = post(url, {'disassociate': True, 'id': instance.id}, admin, expect=400) - assert u"Isolated instances may not be added or removed from instances groups via the API." == resp.data['error'] - - -@pytest.mark.django_db -def test_prevent_non_isolated_instance_added_to_isolated_instance_group(post, admin, non_iso_instance, isolated_instance_group): - url = reverse("api:instance_group_instance_list", kwargs={'pk': isolated_instance_group.pk}) - - assert False is non_iso_instance.is_isolated() - resp = post(url, {'associate': True, 'id': non_iso_instance.id}, admin, expect=400) - assert u"Isolated instance group membership may not be managed via the API." == resp.data['error'] - - -@pytest.mark.django_db -def test_prevent_non_isolated_instance_added_to_isolated_instance_group_via_policy_list(patch, admin, non_iso_instance, isolated_instance_group): - url = reverse("api:instance_group_detail", kwargs={'pk': isolated_instance_group.pk}) - - assert False is non_iso_instance.is_isolated() - resp = patch(url, {'policy_instance_list': [non_iso_instance.hostname]}, user=admin, expect=400) - assert [u"Isolated instance group membership may not be managed via the API."] == resp.data['policy_instance_list'] - assert isolated_instance_group.policy_instance_list == [] - - @pytest.mark.django_db @pytest.mark.parametrize('source_model', ['job_template', 'inventory', 'organization'], indirect=True) def test_instance_group_order_persistence(get, post, admin, source_model): @@ -222,7 +132,7 @@ def test_instance_group_order_persistence(get, post, admin, source_model): total = 5 pks = list(range(total)) random.shuffle(pks) - instances = [InstanceGroup.objects.create(name='iso-%d' % i) for i in pks] + instances = [InstanceGroup.objects.create(name='group-%d' % i) for i in pks] view_name = camelcase_to_underscore(source_model.__class__.__name__) url = reverse('api:{}_instance_groups_list'.format(view_name), kwargs={'pk': source_model.pk}) @@ -252,7 +162,6 @@ def test_instance_group_update_fields(patch, instance, instance_group, admin, co # instance group (not containerized) ig_url = reverse("api:instance_group_detail", kwargs={'pk': instance_group.pk}) assert not instance_group.is_container_group - assert not containerized_instance_group.is_isolated resp = patch(ig_url, {'policy_instance_percentage': 15}, admin, expect=200) assert 15 == resp.data['policy_instance_percentage'] resp = patch(ig_url, {'policy_instance_minimum': 15}, admin, expect=200) @@ -263,7 +172,6 @@ def test_instance_group_update_fields(patch, instance, instance_group, admin, co # containerized instance group cg_url = reverse("api:instance_group_detail", kwargs={'pk': containerized_instance_group.pk}) assert containerized_instance_group.is_container_group - assert not containerized_instance_group.is_isolated resp = patch(cg_url, {'policy_instance_percentage': 15}, admin, expect=400) assert ["Containerized instances may not be managed via the API"] == resp.data['policy_instance_percentage'] resp = patch(cg_url, {'policy_instance_minimum': 15}, admin, expect=400)