diff --git a/awx/api/serializers.py b/awx/api/serializers.py index c8159997..cdae364f 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -2516,6 +2516,7 @@ def get_related(self, obj): activity_stream=self.reverse('api:inventory_source_activity_stream_list', kwargs={'pk': obj.pk}), hosts=self.reverse('api:inventory_source_hosts_list', kwargs={'pk': obj.pk}), groups=self.reverse('api:inventory_source_groups_list', kwargs={'pk': obj.pk}), + instance_groups=self.reverse('api:inventory_source_instance_groups_list', kwargs={'pk': obj.pk}), notification_templates_started=self.reverse('api:inventory_source_notification_templates_started_list', kwargs={'pk': obj.pk}), notification_templates_success=self.reverse('api:inventory_source_notification_templates_success_list', kwargs={'pk': obj.pk}), notification_templates_error=self.reverse('api:inventory_source_notification_templates_error_list', kwargs={'pk': obj.pk}), diff --git a/awx/api/urls/inventory_source.py b/awx/api/urls/inventory_source.py index 713a14c0..c90102e8 100644 --- a/awx/api/urls/inventory_source.py +++ b/awx/api/urls/inventory_source.py @@ -12,6 +12,7 @@ InventorySourceSchedulesList, InventorySourceCredentialsList, InventorySourceGroupsList, + InventorySourceInstanceGroupsList, InventorySourceHostsList, InventorySourceNotificationTemplatesErrorList, InventorySourceNotificationTemplatesStartedList, @@ -26,6 +27,7 @@ path('/activity_stream/', InventorySourceActivityStreamList.as_view(), name='inventory_source_activity_stream_list'), path('/schedules/', InventorySourceSchedulesList.as_view(), name='inventory_source_schedules_list'), path('/credentials/', InventorySourceCredentialsList.as_view(), name='inventory_source_credentials_list'), + path('/instance_groups/', InventorySourceInstanceGroupsList.as_view(), name='inventory_source_instance_groups_list'), path('/groups/', InventorySourceGroupsList.as_view(), name='inventory_source_groups_list'), path('/hosts/', InventorySourceHostsList.as_view(), name='inventory_source_hosts_list'), path( diff --git a/awx/api/views/__init__.py b/awx/api/views/__init__.py index 20ae8ddf..b3f7b342 100644 --- a/awx/api/views/__init__.py +++ b/awx/api/views/__init__.py @@ -2294,6 +2294,14 @@ def is_valid_relation(self, parent, sub, created=False): return None +class InventorySourceInstanceGroupsList(SubListAttachDetachAPIView): + model = models.InstanceGroup + serializer_class = serializers.InstanceGroupSerializer + parent_model = models.InventorySource + relationship = 'instance_groups' + filter_read_permission = False + + class InventorySourceUpdateView(RetrieveAPIView): model = models.InventorySource obj_permission_type = 'start' diff --git a/awx/main/access.py b/awx/main/access.py index ca6fbfc1..492bdf5b 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -1117,6 +1117,20 @@ def can_start(self, obj, validate_license=True): return self.user in obj.inventory.update_role return False + @check_superuser + def can_attach(self, obj, sub_obj, relationship, data, skip_sub_obj_read_check=False): + if relationship == 'instance_groups': + if not obj.inventory: + return False + return self.user in sub_obj.use_role and self.user in obj.inventory.admin_role + return super(InventorySourceAccess, self).can_attach(obj, sub_obj, relationship, data, skip_sub_obj_read_check=skip_sub_obj_read_check) + + @check_superuser + def can_unattach(self, obj, sub_obj, relationship, data=None, skip_sub_obj_read_check=False): + if relationship == 'instance_groups': + return self.can_attach(obj, sub_obj, relationship, data, skip_sub_obj_read_check=skip_sub_obj_read_check) + return super(InventorySourceAccess, self).can_unattach(obj, sub_obj, relationship, data) + class InventoryUpdateAccess(BaseAccess): """ diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index d054f6b2..b6b8dbab 100644 --- a/awx/main/models/inventory.py +++ b/awx/main/models/inventory.py @@ -1424,8 +1424,11 @@ def get_notification_friendly_name(self): @property def preferred_instance_groups(self): selected_groups = [] + # Instance groups set directly on the inventory source take precedence over the inventory's + for instance_group in self.inventory_source.instance_groups.all(): + selected_groups.append(instance_group) if self.inventory_source.inventory is not None: - # Add the inventory sources IG to the selected IGs first + # Add the inventory's IGs to the selected IGs next for instance_group in self.inventory_source.inventory.instance_groups.all(): selected_groups.append(instance_group) # If the inventory allows for fallback and we have an organization then also append the orgs IGs to the end of the list diff --git a/awx/main/tests/functional/api/test_instance_group.py b/awx/main/tests/functional/api/test_instance_group.py index aa8204c6..ecf191c2 100644 --- a/awx/main/tests/functional/api/test_instance_group.py +++ b/awx/main/tests/functional/api/test_instance_group.py @@ -137,7 +137,7 @@ def test_delete_rename_tower_instance_group_prevented( @pytest.mark.django_db -@pytest.mark.parametrize('source_model', ['job_template', 'inventory', 'organization'], indirect=True) +@pytest.mark.parametrize('source_model', ['job_template', 'inventory', 'organization', 'inventory_source'], indirect=True) def test_instance_group_order_persistence(get, post, admin, source_model): # create several instance groups in random order total = 5 @@ -167,6 +167,20 @@ def test_instance_group_order_persistence(get, post, admin, source_model): assert [ig['name'] for ig in resp.data['results']] == [ig.name for ig in before] +@pytest.mark.django_db +def test_inventory_source_instance_group_attach_permissions(get, post, rando, inventory_source, instance_group): + url = reverse('api:inventory_source_instance_groups_list', kwargs={'pk': inventory_source.pk}) + inventory_source.inventory.admin_role.members.add(rando) + # inventory admin without use permission on the instance group cannot attach it + post(url, {'associate': True, 'id': instance_group.id}, rando, expect=403) + assert list(inventory_source.instance_groups.all()) == [] + instance_group.use_role.members.add(rando) + post(url, {'associate': True, 'id': instance_group.id}, rando, expect=204) + assert list(inventory_source.instance_groups.all()) == [instance_group] + post(url, {'disassociate': True, 'id': instance_group.id}, rando, expect=204) + assert list(inventory_source.instance_groups.all()) == [] + + @pytest.mark.django_db def test_instance_group_update_fields(patch, instance, instance_group, admin, containerized_instance_group): # policy_instance_ variables can only be updated in instance groups that are NOT containerized diff --git a/awx/main/tests/functional/test_instances.py b/awx/main/tests/functional/test_instances.py index 4ed65217..c0607568 100644 --- a/awx/main/tests/functional/test_instances.py +++ b/awx/main/tests/functional/test_instances.py @@ -423,9 +423,11 @@ def test_inventory_update_instance_groups(self, instance_group_factory, inventor inventory_source.inventory.instance_groups.add(ig_inv) assert iu.preferred_instance_groups == [ig_inv, ig_org] inventory_source.instance_groups.add(ig_tmp) - # API does not allow setting IGs on inventory source, so ignore those - assert iu.preferred_instance_groups == [ig_inv, ig_org] + # Instance groups on the inventory source take precedence over the inventory's + assert iu.preferred_instance_groups == [ig_tmp, ig_inv, ig_org] inventory_source.inventory.prevent_instance_group_fallback = True + assert iu.preferred_instance_groups == [ig_tmp, ig_inv] + inventory_source.instance_groups.remove(ig_tmp) assert iu.preferred_instance_groups == [ig_inv] def test_job_instance_groups(self, instance_group_factory, inventory, project, default_instance_group): diff --git a/awx/main/tests/functional/test_rbac_instance_groups.py b/awx/main/tests/functional/test_rbac_instance_groups.py index 418e5a35..e93ba57d 100644 --- a/awx/main/tests/functional/test_rbac_instance_groups.py +++ b/awx/main/tests/functional/test_rbac_instance_groups.py @@ -4,6 +4,7 @@ InstanceGroupAccess, OrganizationAccess, InventoryAccess, + InventorySourceAccess, JobTemplateAccess, ) @@ -41,6 +42,23 @@ def test_ig_role_based_associability(default_instance_group, rando, organization assert allowed == OrganizationAccess(rando).can_attach(objects.organization, default_instance_group, 'instance_groups', None) +@pytest.mark.django_db +@pytest.mark.parametrize( + "obj_perm,subobj_perm,allowed", + [('admin_role', 'use_role', True), ('admin_role', 'read_role', False), ('use_role', 'use_role', False), ('admin_role', 'admin_role', True)], +) +def test_ig_inventory_source_associability(default_instance_group, rando, inventory_source, obj_perm, subobj_perm, allowed): + if obj_perm: + getattr(inventory_source.inventory, obj_perm).members.add(rando) + if subobj_perm: + getattr(default_instance_group, subobj_perm).members.add(rando) + + assert allowed == InventorySourceAccess(rando).can_attach(inventory_source, default_instance_group, 'instance_groups', None) + assert allowed == InventorySourceAccess(rando).can_unattach(inventory_source, default_instance_group, 'instance_groups', None) + # data is optional for unattach checks, callers are allowed to omit it + assert allowed == InventorySourceAccess(rando).can_unattach(inventory_source, default_instance_group, 'instance_groups') + + @pytest.mark.django_db def test_ig_use_with_org_admin(default_instance_group, rando, org_admin): default_instance_group.use_role.members.add(rando) diff --git a/awx/ui/src/api/models/InventorySources.js b/awx/ui/src/api/models/InventorySources.js index 66ad6dbc..fde34872 100644 --- a/awx/ui/src/api/models/InventorySources.js +++ b/awx/ui/src/api/models/InventorySources.js @@ -2,9 +2,10 @@ import Base from '../Base'; import NotificationsMixin from '../mixins/Notifications.mixin'; import LaunchUpdateMixin from '../mixins/LaunchUpdate.mixin'; import SchedulesMixin from '../mixins/Schedules.mixin'; +import InstanceGroupsMixin from '../mixins/InstanceGroups.mixin'; -class InventorySources extends LaunchUpdateMixin( - NotificationsMixin(SchedulesMixin(Base)) +class InventorySources extends InstanceGroupsMixin( + LaunchUpdateMixin(NotificationsMixin(SchedulesMixin(Base))) ) { constructor(http) { super(http); diff --git a/awx/ui/src/screens/Inventory/InventorySourceAdd/InventorySourceAdd.js b/awx/ui/src/screens/Inventory/InventorySourceAdd/InventorySourceAdd.js index 9ea1df96..9f552d45 100644 --- a/awx/ui/src/screens/Inventory/InventorySourceAdd/InventorySourceAdd.js +++ b/awx/ui/src/screens/Inventory/InventorySourceAdd/InventorySourceAdd.js @@ -11,8 +11,14 @@ function InventorySourceAdd({ inventory }) { const { id, organization } = inventory; const { error, request, result } = useRequest( - useCallback(async (values) => { + useCallback(async ({ instanceGroups, ...values }) => { const { data } = await InventorySourcesAPI.create(values); + /* eslint-disable no-await-in-loop, no-restricted-syntax */ + // Resolve Promises sequentially to maintain order and avoid race condition + for (const group of instanceGroups || []) { + await InventorySourcesAPI.associateInstanceGroup(data.id, group.id); + } + /* eslint-enable no-await-in-loop, no-restricted-syntax */ return data; }, []) ); @@ -34,6 +40,7 @@ function InventorySourceAdd({ inventory }) { source_project, source_script, execution_environment, + instanceGroups, ...remainingForm } = form; @@ -50,6 +57,7 @@ function InventorySourceAdd({ inventory }) { inventory: id, source_script: source_script?.id || null, execution_environment: execution_environment?.id || null, + instanceGroups, ...sourcePath, ...sourceProject, ...remainingForm, diff --git a/awx/ui/src/screens/Inventory/InventorySourceAdd/InventorySourceAdd.test.js b/awx/ui/src/screens/Inventory/InventorySourceAdd/InventorySourceAdd.test.js index 524cc6c6..b9c5e697 100644 --- a/awx/ui/src/screens/Inventory/InventorySourceAdd/InventorySourceAdd.test.js +++ b/awx/ui/src/screens/Inventory/InventorySourceAdd/InventorySourceAdd.test.js @@ -20,6 +20,7 @@ const invSourceData = { update_cache_timeout: 0, update_on_launch: false, verbosity: 1, + instanceGroups: [{ id: 100 }, { id: 200 }], }; const mockInventory = { @@ -96,6 +97,30 @@ describe('', () => { }); }); + test('should associate instance groups after creation', async () => { + InventorySourcesAPI.create.mockResolvedValue({ data: { id: 55 } }); + const { user } = renderWithContexts( + + ); + await user.click(await screen.findByRole('button', { name: 'mock-submit' })); + + await waitFor(() => + expect(InventorySourcesAPI.associateInstanceGroup).toHaveBeenCalledTimes( + 2 + ) + ); + expect(InventorySourcesAPI.associateInstanceGroup).toHaveBeenNthCalledWith( + 1, + 55, + 100 + ); + expect(InventorySourcesAPI.associateInstanceGroup).toHaveBeenNthCalledWith( + 2, + 55, + 200 + ); + }); + test('successful form submission should trigger redirect', async () => { const history = createMemoryHistory({}); InventorySourcesAPI.create.mockResolvedValue({ diff --git a/awx/ui/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.js b/awx/ui/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.js index 3cf073b2..36b409a0 100644 --- a/awx/ui/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.js +++ b/awx/ui/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.js @@ -14,6 +14,7 @@ import CredentialChip from 'components/CredentialChip'; import DeleteButton from 'components/DeleteButton'; import ErrorDetail from 'components/ErrorDetail'; import ExecutionEnvironmentDetail from 'components/ExecutionEnvironmentDetail'; +import InstanceGroupLabels from 'components/InstanceGroupLabels'; import JobCancelButton from 'components/JobCancelButton'; import StatusLabel from 'components/StatusLabel'; import { CardBody, CardActionsRow } from 'components/Card'; @@ -52,6 +53,20 @@ function InventorySourceDetail({ inventorySource }) { }, []) ); + const { result: instanceGroups, request: fetchInstanceGroups } = useRequest( + useCallback(async () => { + const { data } = await InventorySourcesAPI.readInstanceGroups( + inventorySource.id + ); + return data.results; + }, [inventorySource.id]), + [] + ); + + useEffect(() => { + fetchInstanceGroups(); + }, [fetchInstanceGroups]); + const { created, description, @@ -218,6 +233,13 @@ function InventorySourceDetail({ inventorySource }) { + {instanceGroups && instanceGroups.length > 0 && ( + } + /> + )} {source_project && ( { + const { data } = await InventorySourcesAPI.readInstanceGroups(source.id); + return data.results; + }, [source.id]), + null + ); + + useEffect(() => { + fetchInstanceGroups(); + }, [fetchInstanceGroups]); + const { error, request, result } = useRequest( useCallback( - async (values) => { + async ({ instanceGroups, ...values }) => { const { data } = await InventorySourcesAPI.replace(source.id, values); + await InventorySourcesAPI.orderInstanceGroups( + source.id, + instanceGroups, + associatedInstanceGroups + ); return data; }, - [source.id] + [source.id, associatedInstanceGroups] ), null ); @@ -37,6 +61,7 @@ function InventorySourceEdit({ source, inventory }) { source_project, source_script, execution_environment, + instanceGroups, ...remainingForm } = form; @@ -53,6 +78,7 @@ function InventorySourceEdit({ source, inventory }) { inventory: id, source_script: source_script?.id || null, execution_environment: execution_environment?.id || null, + instanceGroups, ...sourcePath, ...sourceProject, ...remainingForm, @@ -63,11 +89,32 @@ function InventorySourceEdit({ source, inventory }) { navigate(detailsUrl); }; + if (instanceGroupsError) { + return ( + + + + + + ); + } + + if (isInstanceGroupsLoading || !associatedInstanceGroups) { + return ( + + + + + + ); + } + return ( onSubmit(mockInvSrc)} + onClick={() => + onSubmit({ ...mockInvSrc, instanceGroups: [{ id: 100 }] }) + } />