mirror of
https://github.com/ansible/awx.git
synced 2026-01-16 12:20:45 -03:30
SCM inventory acceptance and bug updates
* fixed a problem where the SCM last revision was not getting reset * change the 400 editing error to just reset the last revision * handle `source_vars` in the same way as custom scripts * block unallowed `source_vars` in the validator * hide POST in OPTIONS for inventory source sublist if not allowed
This commit is contained in:
AlanCoding
parent
55c2f5a2d6
commit
97ca6bc897
@ -1474,7 +1474,11 @@ class InventorySourceOptionsSerializer(BaseSerializer):
|
||||
return res
|
||||
|
||||
def validate_source_vars(self, value):
|
||||
return vars_validate_or_raise(value)
|
||||
ret = vars_validate_or_raise(value)
|
||||
for env_k in parse_yaml_or_json(value):
|
||||
if env_k in settings.INV_ENV_VARIABLE_BLACKLIST:
|
||||
raise serializers.ValidationError(_("`{}` is a prohibited environment variable".format(env_k)))
|
||||
return ret
|
||||
|
||||
def validate(self, attrs):
|
||||
# TODO: Validate source, validate source_regions
|
||||
|
||||
@ -1120,7 +1120,7 @@ class ProjectSchedulesList(SubListCreateAPIView):
|
||||
new_in_148 = True
|
||||
|
||||
|
||||
class ProjectScmInventorySources(SubListCreateAPIView):
|
||||
class ProjectScmInventorySources(SubListAPIView):
|
||||
|
||||
view_name = _("Project SCM Inventory Sources")
|
||||
model = InventorySource
|
||||
@ -1689,32 +1689,31 @@ class InventoryList(ListCreateAPIView):
|
||||
|
||||
class ControlledByScmMixin(object):
|
||||
'''
|
||||
Special method to lock-down items managed by SCM inventory source via
|
||||
project update, which are not protected by the task manager.
|
||||
Special method to reset SCM inventory commit hash
|
||||
if anything that it manages changes.
|
||||
'''
|
||||
|
||||
def _raise_if_unallowed(self, obj):
|
||||
if (self.request.method not in SAFE_METHODS and obj and
|
||||
obj.inventory_sources.filter(
|
||||
update_on_project_update=True, source='scm').exists()):
|
||||
def _reset_inv_src_rev(self, obj):
|
||||
if self.request.method in SAFE_METHODS or not obj:
|
||||
return
|
||||
project_following_sources = obj.inventory_sources.filter(
|
||||
update_on_project_update=True, source='scm')
|
||||
if project_following_sources:
|
||||
# Allow inventory changes unrelated to variables
|
||||
if self.model == Inventory and (
|
||||
not self.request or not self.request.data or
|
||||
parse_yaml_or_json(self.request.data.get('variables', '')) == parse_yaml_or_json(obj.variables)):
|
||||
return
|
||||
raise PermissionDenied(detail=_(
|
||||
'This object is managed by updates to the project '
|
||||
'of its inventory source. Remove the inventory source '
|
||||
'in order to make this edit.'))
|
||||
project_following_sources.update(scm_last_revision='')
|
||||
|
||||
def get_object(self):
|
||||
obj = super(ControlledByScmMixin, self).get_object()
|
||||
self._raise_if_unallowed(obj)
|
||||
self._reset_inv_src_rev(obj)
|
||||
return obj
|
||||
|
||||
def get_parent_object(self):
|
||||
obj = super(ControlledByScmMixin, self).get_parent_object()
|
||||
self._raise_if_unallowed(obj)
|
||||
self._reset_inv_src_rev(obj)
|
||||
return obj
|
||||
|
||||
|
||||
@ -2288,6 +2287,8 @@ class InventoryInventorySourcesList(SubListCreateAPIView):
|
||||
model = InventorySource
|
||||
serializer_class = InventorySourceSerializer
|
||||
parent_model = Inventory
|
||||
# Sometimes creation blocked by SCM inventory source restrictions
|
||||
always_allow_superuser = False
|
||||
relationship = 'inventory_sources'
|
||||
parent_key = 'inventory'
|
||||
new_in_14 = True
|
||||
@ -2297,6 +2298,7 @@ class InventorySourceList(ListCreateAPIView):
|
||||
|
||||
model = InventorySource
|
||||
serializer_class = InventorySourceSerializer
|
||||
always_allow_superuser = False
|
||||
new_in_14 = True
|
||||
|
||||
|
||||
|
||||
@ -376,8 +376,6 @@ class BaseAccess(object):
|
||||
return access_method(obj)
|
||||
elif method in ['start']:
|
||||
return self.can_start(obj, validate_license=False)
|
||||
elif method in ['add']: # 2 args with data
|
||||
return self.can_add({})
|
||||
elif method in ['attach', 'unattach']: # parent/sub-object call
|
||||
access_method = getattr(self, "can_%s" % method)
|
||||
if type(parent_obj) == Team:
|
||||
@ -739,21 +737,25 @@ class InventorySourceAccess(BaseAccess):
|
||||
else:
|
||||
return False
|
||||
|
||||
@check_superuser
|
||||
def can_add(self, data):
|
||||
if not data or 'inventory' not in data:
|
||||
return False
|
||||
if not self.check_related('source_project', Project, data, role_field='admin_role'):
|
||||
if not self.check_related('source_project', Project, data, role_field='use_role'):
|
||||
return False
|
||||
# Checks for admin or change permission on inventory.
|
||||
return self.check_related('inventory', Inventory, data)
|
||||
return (
|
||||
self.check_related('inventory', Inventory, data) and
|
||||
not InventorySource.objects.filter(
|
||||
inventory=data.get('inventory'),
|
||||
update_on_project_update=True, source='scm').exists())
|
||||
|
||||
def can_change(self, obj, data):
|
||||
# Checks for admin or change permission on group.
|
||||
if obj and obj.inventory:
|
||||
return (
|
||||
self.user.can_access(Inventory, 'change', obj.inventory, None) and
|
||||
self.check_related('credential', Credential, data, obj=obj, role_field='use_role')
|
||||
self.check_related('credential', Credential, data, obj=obj, role_field='use_role') and
|
||||
self.check_related('source_project', Project, data, obj=obj, role_field='use_role')
|
||||
)
|
||||
# Can't change inventory sources attached to only the inventory, since
|
||||
# these are created automatically from the management command.
|
||||
|
||||
@ -1182,9 +1182,9 @@ class InventorySource(UnifiedJobTemplate, InventorySourceOptions):
|
||||
before_is = self.__class__.objects.get(pk=self.pk)
|
||||
if before_is.source_path != self.source_path or before_is.source_project_id != self.source_project_id:
|
||||
# Reset the scm_revision if file changed to force update
|
||||
self.scm_revision = None
|
||||
if 'scm_revision' not in update_fields:
|
||||
update_fields.append('scm_revision')
|
||||
self.scm_last_revision = ''
|
||||
if 'scm_last_revision' not in update_fields:
|
||||
update_fields.append('scm_last_revision')
|
||||
|
||||
# Do the actual save.
|
||||
super(InventorySource, self).save(*args, **kwargs)
|
||||
|
||||
@ -1723,10 +1723,7 @@ class RunInventoryUpdate(BaseTask):
|
||||
env['FOREMAN_INI_PATH'] = cloud_credential
|
||||
elif inventory_update.source == 'cloudforms':
|
||||
env['CLOUDFORMS_INI_PATH'] = cloud_credential
|
||||
elif inventory_update.source == 'scm':
|
||||
# Parse source_vars to dict, update env.
|
||||
env.update(parse_yaml_or_json(inventory_update.source_vars))
|
||||
elif inventory_update.source == 'custom':
|
||||
elif inventory_update.source in ['scm', 'custom']:
|
||||
for env_k in inventory_update.source_vars_dict:
|
||||
if str(env_k) not in env and str(env_k) not in settings.INV_ENV_VARIABLE_BLACKLIST:
|
||||
env[str(env_k)] = unicode(inventory_update.source_vars_dict[env_k])
|
||||
|
||||
@ -1,7 +1,10 @@
|
||||
import pytest
|
||||
import mock
|
||||
|
||||
from awx.api.versioning import reverse
|
||||
|
||||
from awx.main.models import InventorySource
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_inventory_source_notification_on_cloud_only(get, post, inventory_source_factory, user, notification_template):
|
||||
@ -199,11 +202,23 @@ def test_inventory_source_update(post, inventory_source, alice, role_field, expe
|
||||
post(reverse('api:inventory_source_update_view', kwargs={'pk': inventory_source.id}), {}, alice, expect=expected_status_code)
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_inventory_source_vars_prohibition(post, inventory, admin_user):
|
||||
with mock.patch('awx.api.serializers.settings') as mock_settings:
|
||||
mock_settings.INV_ENV_VARIABLE_BLACKLIST = ('FOOBAR',)
|
||||
r = post(reverse('api:inventory_source_list'),
|
||||
{'name': 'new inv src', 'source_vars': '{\"FOOBAR\": \"val\"}', 'inventory': inventory.pk},
|
||||
admin_user, expect=400)
|
||||
assert 'prohibited environment variable' in r.data['source_vars'][0]
|
||||
assert 'FOOBAR' in r.data['source_vars'][0]
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def scm_inventory(inventory, project):
|
||||
inventory.inventory_sources.create(
|
||||
name='foobar', update_on_project_update=True, source='scm',
|
||||
source_project=project)
|
||||
with mock.patch.object(project, 'update'):
|
||||
inventory.inventory_sources.create(
|
||||
name='foobar', update_on_project_update=True, source='scm',
|
||||
source_project=project, scm_last_revision=project.scm_revision)
|
||||
return inventory
|
||||
|
||||
|
||||
@ -216,37 +231,43 @@ class TestControlledBySCM:
|
||||
def test_safe_method_works(self, get, options, scm_inventory, admin_user):
|
||||
get(scm_inventory.get_absolute_url(), admin_user, expect=200)
|
||||
options(scm_inventory.get_absolute_url(), admin_user, expect=200)
|
||||
assert InventorySource.objects.get(inventory=scm_inventory.pk).scm_last_revision != ''
|
||||
|
||||
def test_vars_edit_prohibited(self, patch, scm_inventory, admin_user):
|
||||
def test_vars_edit_reset(self, patch, scm_inventory, admin_user):
|
||||
patch(scm_inventory.get_absolute_url(), {'variables': 'hello: world'},
|
||||
admin_user, expect=403)
|
||||
admin_user, expect=200)
|
||||
assert InventorySource.objects.get(inventory=scm_inventory.pk).scm_last_revision == ''
|
||||
|
||||
def test_name_edit_allowed(self, patch, scm_inventory, admin_user):
|
||||
patch(scm_inventory.get_absolute_url(), {'variables': '---', 'name': 'newname'},
|
||||
admin_user, expect=200)
|
||||
assert InventorySource.objects.get(inventory=scm_inventory.pk).scm_last_revision != ''
|
||||
|
||||
def test_host_associations_prohibited(self, post, scm_inventory, admin_user):
|
||||
def test_host_associations_reset(self, post, scm_inventory, admin_user):
|
||||
inv_src = scm_inventory.inventory_sources.first()
|
||||
h = inv_src.hosts.create(name='barfoo', inventory=scm_inventory)
|
||||
g = inv_src.groups.create(name='fooland', inventory=scm_inventory)
|
||||
post(reverse('api:host_groups_list', kwargs={'pk': h.id}), {'id': g.id},
|
||||
admin_user, expect=403)
|
||||
admin_user, expect=204)
|
||||
post(reverse('api:group_hosts_list', kwargs={'pk': g.id}), {'id': h.id},
|
||||
admin_user, expect=403)
|
||||
admin_user, expect=204)
|
||||
assert InventorySource.objects.get(inventory=scm_inventory.pk).scm_last_revision == ''
|
||||
|
||||
def test_group_group_associations_prohibited(self, post, scm_inventory, admin_user):
|
||||
def test_group_group_associations_reset(self, post, scm_inventory, admin_user):
|
||||
inv_src = scm_inventory.inventory_sources.first()
|
||||
g1 = inv_src.groups.create(name='barland', inventory=scm_inventory)
|
||||
g2 = inv_src.groups.create(name='fooland', inventory=scm_inventory)
|
||||
post(reverse('api:group_children_list', kwargs={'pk': g1.id}), {'id': g2.id},
|
||||
admin_user, expect=403)
|
||||
admin_user, expect=204)
|
||||
assert InventorySource.objects.get(inventory=scm_inventory.pk).scm_last_revision == ''
|
||||
|
||||
def test_host_group_delete_prohibited(self, delete, scm_inventory, admin_user):
|
||||
def test_host_group_delete_reset(self, delete, scm_inventory, admin_user):
|
||||
inv_src = scm_inventory.inventory_sources.first()
|
||||
h = inv_src.hosts.create(name='barfoo', inventory=scm_inventory)
|
||||
g = inv_src.groups.create(name='fooland', inventory=scm_inventory)
|
||||
delete(h.get_absolute_url(), admin_user, expect=403)
|
||||
delete(g.get_absolute_url(), admin_user, expect=403)
|
||||
delete(h.get_absolute_url(), admin_user, expect=204)
|
||||
delete(g.get_absolute_url(), admin_user, expect=204)
|
||||
assert InventorySource.objects.get(inventory=scm_inventory.pk).scm_last_revision == ''
|
||||
|
||||
def test_remove_scm_inv_src(self, delete, scm_inventory, admin_user):
|
||||
inv_src = scm_inventory.inventory_sources.first()
|
||||
@ -255,4 +276,14 @@ class TestControlledBySCM:
|
||||
|
||||
def test_adding_inv_src_prohibited(self, post, scm_inventory, admin_user):
|
||||
post(reverse('api:inventory_inventory_sources_list', kwargs={'pk': scm_inventory.id}),
|
||||
{'name': 'new inv src'}, admin_user, expect=400)
|
||||
{'name': 'new inv src'}, admin_user, expect=403)
|
||||
|
||||
def test_adding_inv_src_without_proj_access_prohibited(self, post, project, inventory, rando):
|
||||
inventory.admin_role.members.add(rando)
|
||||
post(reverse('api:inventory_inventory_sources_list', kwargs={'pk': inventory.id}),
|
||||
{'name': 'new inv src', 'source_project': project.pk}, rando, expect=403)
|
||||
|
||||
def test_no_post_in_options(self, options, scm_inventory, admin_user):
|
||||
r = options(reverse('api:inventory_inventory_sources_list', kwargs={'pk': scm_inventory.id}),
|
||||
admin_user, expect=200)
|
||||
assert 'POST' not in r.data['actions']
|
||||
|
||||
@ -241,13 +241,17 @@ def inventory(organization):
|
||||
|
||||
@pytest.fixture
|
||||
def scm_inventory_source(inventory, project):
|
||||
return InventorySource.objects.create(
|
||||
inv_src = InventorySource(
|
||||
name="test-scm-inv",
|
||||
source_project=project,
|
||||
source='scm',
|
||||
source_path='inventory_file',
|
||||
update_on_project_update=True,
|
||||
inventory=inventory)
|
||||
inventory=inventory,
|
||||
scm_last_revision=project.scm_revision)
|
||||
with mock.patch.object(inv_src.source_project, 'update'):
|
||||
inv_src.save()
|
||||
return inv_src
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
|
||||
@ -23,6 +23,13 @@ class TestSCMUpdateFeatures:
|
||||
inv_src.save()
|
||||
mck_update.assert_called_once_with()
|
||||
|
||||
def test_reset_scm_revision(self, scm_inventory_source):
|
||||
starting_rev = scm_inventory_source.scm_last_revision
|
||||
assert starting_rev != ''
|
||||
scm_inventory_source.source_path = '/newfolder/newfile.ini'
|
||||
scm_inventory_source.save()
|
||||
assert scm_inventory_source.scm_last_revision == ''
|
||||
|
||||
def test_source_location(self, scm_inventory_source):
|
||||
# Combines project directory with the inventory file specified
|
||||
inventory_update = InventoryUpdate(
|
||||
|
||||
@ -36,6 +36,7 @@ class TestDependentInventoryUpdate:
|
||||
|
||||
def test_dependent_inventory_updates(self, scm_inventory_source):
|
||||
task = RunProjectUpdate()
|
||||
scm_inventory_source.scm_last_revision = ''
|
||||
proj_update = ProjectUpdate.objects.create(project=scm_inventory_source.source_project)
|
||||
with mock.patch.object(RunInventoryUpdate, 'run') as iu_run_mock:
|
||||
task._update_dependent_inventories(proj_update, [scm_inventory_source])
|
||||
|
||||
@ -1,7 +1,7 @@
|
||||
# SCM Inventory
|
||||
|
||||
Users can create inventory sources that use content in the source tree of
|
||||
a project as an Ansible inventory file.
|
||||
a project as an Ansible inventory source.
|
||||
|
||||
## Usage Details
|
||||
|
||||
@ -34,7 +34,7 @@ _unless the scm revision of the project changes_.
|
||||
|
||||
### RBAC
|
||||
|
||||
User needs `admin` role to the project in order to use it as a source
|
||||
User needs `use` role to the project in order to use it as a source
|
||||
project for inventory (this entails permission to run arbitrary scripts).
|
||||
To update the project, they need `update` permission to the project,
|
||||
even if the update is done indirectly.
|
||||
@ -58,6 +58,15 @@ inside of an inventory all update with a single button click. When this
|
||||
happens for an inventory containing an SCM inventory source, it should
|
||||
update the project.
|
||||
|
||||
### Inventory Source Restriction
|
||||
|
||||
Since automatic inventory updates (triggered by a project update) do not
|
||||
go through the task system, typical protection against conflicting updates
|
||||
is not available. To avoid problems, only 1 inventory source is allowed for
|
||||
inventories that use this feature. That means that if an inventory source
|
||||
has `source=scm` and `update_on_project_update=true`, it can be the only
|
||||
inventory source for its inventory.
|
||||
|
||||
## Supported File Syntax
|
||||
|
||||
> Any Inventory Ansible supports should be supported by this feature
|
||||
@ -73,6 +82,25 @@ https://github.com/ansible/ansible-inventory-backport
|
||||
Because the internal mechanism is different, we need some coverage
|
||||
testing with Ansible versions pre-2.4 and after.
|
||||
|
||||
### Vars Restrictions
|
||||
|
||||
When creating any `scm` type inventory source, the `overwrite_vars` field
|
||||
must be set to `true`. This should be enforced by API validation and
|
||||
the UI should also force this setting.
|
||||
|
||||
Why? This is because `ansible-inventory` is planned to
|
||||
return group vars at the group-level in its JSON output, but the backported
|
||||
script returns them on the host-level. In Ansible 2.4, inventory modules are
|
||||
being refactored into plugins, and showing vars on the group-level depends on
|
||||
this. Since it is not possible to _also_ backport the inventory module
|
||||
refactor to prior Ansible versions, this discrepancy can not be resolved.
|
||||
While "flattening" the group vars down to the host-level is functionally
|
||||
equivalent, upgrading Ansible would cause an anomaly in variable precedence.
|
||||
|
||||
This future variable precedence problem is avoided by forcing overwriting
|
||||
of variables until Ansible 2.3 is deprecated, after which all updates
|
||||
will consistently utilize group-level variables.
|
||||
|
||||
# Acceptance Criteria Use Cases
|
||||
|
||||
Some test scenarios to look at:
|
||||
@ -126,3 +154,18 @@ until the inventory update is finished.
|
||||
|
||||
Note that a failed inventory update does not mark the project as failed.
|
||||
|
||||
## Restricting Instance Group to Run Script On
|
||||
|
||||
Since SCM inventory sources are running scripts written by people with
|
||||
access to the source-control, a user may want to restrict which instance
|
||||
groups the inventory update runs on.
|
||||
|
||||
If the inventory source is set to update on project update, it will run
|
||||
on the same instance (inside of the Tower cluster) as the project update.
|
||||
This can be restricted by limiting the instance groups of the organization
|
||||
that contains the `source_project` of the inventory source.
|
||||
|
||||
If the inventory source is not configured to update on project update,
|
||||
then it will inherit the allowed instance groups from its inventory,
|
||||
like all other inventory syncs.
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user