From 7a4d61556d4630338a50873e669a0b6c7943132b Mon Sep 17 00:00:00 2001 From: fernandorocagonzalez Date: Fri, 24 Jul 2026 20:50:44 +0200 Subject: [PATCH 1/2] Allow assigning instance groups to inventory sources An inventory update runs inside an execution environment on whatever node the scheduler picks, so that node is the one that needs network access to the inventory source. Until now the only routing control was Inventory.instance_groups, which also decides where playbook jobs using that inventory run. Pinning an inventory to a remote execution node so jobs can reach its hosts also dragged the source syncs onto that node, which often has no route back to the system the inventory pulls from (forum thread d/17, upstream awx issue 13995). InventorySource already inherits the instance_groups relation from UnifiedJobTemplate, so no schema change is needed; the API simply never exposed it and InventoryUpdate.preferred_instance_groups ignored it. - InventoryUpdate.preferred_instance_groups now consults the source first, then falls back to the existing inventory -> organization -> global chain, matching the job template precedence semantics - new /api/v2/inventory_sources/N/instance_groups/ endpoint, wired into related links, association order preserved - attaching requires use permission on the instance group plus admin on the inventory, same rule as job templates and inventories - instance group lookup on the inventory source form, association on add/edit, labels on the detail view - docs note explaining that syncs run on execution nodes Sources with no instance groups keep the exact previous behaviour. --- awx/api/serializers.py | 1 + awx/api/urls/inventory_source.py | 2 + awx/api/views/__init__.py | 8 +++ awx/main/access.py | 14 +++++ awx/main/models/inventory.py | 5 +- .../functional/api/test_instance_group.py | 16 +++++- awx/main/tests/functional/test_instances.py | 6 ++- .../functional/test_rbac_instance_groups.py | 16 ++++++ awx/ui/src/api/models/InventorySources.js | 5 +- .../InventorySourceAdd/InventorySourceAdd.js | 10 +++- .../InventorySourceAdd.test.js | 25 +++++++++ .../InventorySourceDetail.js | 22 ++++++++ .../InventorySourceEdit.js | 51 ++++++++++++++++++- .../InventorySourceEdit.test.js | 15 +++++- .../Inventory/shared/InventorySourceForm.js | 15 +++++- docs/inventory_source_instance_groups.md | 39 ++++++++++++++ 16 files changed, 239 insertions(+), 11 deletions(-) create mode 100644 docs/inventory_source_instance_groups.md diff --git a/awx/api/serializers.py b/awx/api/serializers.py index c81599979..cdae364f1 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 713a14c00..c90102e83 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 20ae8ddf1..b3f7b3426 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 ca6fbfc18..255428c2a 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, *args, **kwargs): + if relationship == 'instance_groups': + return self.can_attach(obj, sub_obj, relationship, *args, **kwargs) + return super(InventorySourceAccess, self).can_unattach(obj, sub_obj, relationship, *args, **kwargs) + class InventoryUpdateAccess(BaseAccess): """ diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index d054f6b21..b6b8dbabc 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 aa8204c6d..ecf191c24 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 4ed652179..c0607568e 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 418e5a351..e3e7f3ce4 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,21 @@ 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) + + @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 66ad6dbc9..fde348721 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 9ea1df96d..9f552d45f 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 524cc6c6b..b9c5e697d 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 3cf073b29..36b409a02 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 }] }) + } />