Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP]Make cloud providers dynamic #15537

Draft
wants to merge 103 commits into
base: devel
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 95 commits
Commits
Show all changes
103 commits
Select commit Hold shift + click to select a range
baed221
Add dynamic plugin_names pull for CLOUUD_PROVIDERS
djyasin Sep 17, 2024
3d49df9
Revise to return a tuple instead of a list of key value pairs
djyasin Sep 18, 2024
1ab43fd
Update awx/main/constants.py
djyasin Sep 19, 2024
23c05f9
Attempt to add bootstrapping for imports
djyasin Sep 19, 2024
33cf169
Revise bootstrap attempt
djyasin Sep 20, 2024
40fee4a
Attempt to avoid importing InventorySourceOptions directly in constan…
djyasin Sep 20, 2024
3ec485b
Attempt to avoid importing InventorySourceOptions directly in constan…
djyasin Sep 23, 2024
d530288
Resolve function object is not iterable error
djyasin Sep 23, 2024
16576ce
Add classmethod to load_plugin_names function
djyasin Sep 24, 2024
f7a775c
Refine call and adjust how I am using AppConfig
djyasin Sep 24, 2024
c745b8e
Remove additional hardcoded CLOUD_PROVIDERS and got code working
djyasin Sep 24, 2024
3b42a43
Remove unused imports
djyasin Sep 24, 2024
a56b3f3
Move import to the top of the file to avoid in-function import
djyasin Sep 24, 2024
4c72dbd
Remove SOURCE_CHOICES = [] and ran isort on inventory.py
djyasin Sep 25, 2024
8b0020e
Rename load_plugin_names to discover_available_plugin_names() to be m…
djyasin Sep 25, 2024
bfca360
Revert isort changes in inventory.py
djyasin Sep 25, 2024
d9152c6
Revert remainder of isort changes in invenory.py
djyasin Sep 25, 2024
af72d22
Revert last isort changes in invenory.py
djyasin Sep 25, 2024
f96aaa6
Rename function to be more descriptive that it is discovering cloud p…
djyasin Sep 25, 2024
aee7e2d
Remove import copy that isort had added
djyasin Sep 25, 2024
e148236
Move plugin functionality over to a new file that makes more sense si…
djyasin Sep 26, 2024
0be715e
Move _all_functionality for CLOUD_PROVIDERS from constants to plugins
djyasin Sep 26, 2024
61f46e9
Begin implementation of Making Choices Lazy
djyasin Sep 26, 2024
79fd42f
Add copy import back to avoid mixing linting fixes with functional ch…
djyasin Sep 30, 2024
6b9ab0d
Update awx/main/utils/plugins.py
djyasin Sep 30, 2024
4d1319a
Revise plugin.py module description
djyasin Sep 30, 2024
bd29750
Remove __all__ from plugins.py as it is not needed
djyasin Sep 30, 2024
1c74ab2
Add import to serializers and correct import for tests
djyasin Sep 30, 2024
cb73d5b
Correct import for CLOUD_PROVIDERS
djyasin Sep 30, 2024
f6e2205
Remove redundant import of discover_available_cloud_provider_plugin_n…
djyasin Sep 30, 2024
5059fff
Update awx/main/utils/plugins.py
djyasin Oct 1, 2024
80f0aa0
Update awx/api/serializers.py
djyasin Oct 1, 2024
0e04fa8
Revert import linting fix so as not to mix functional fixes with lint…
djyasin Oct 1, 2024
866e9c9
Update awx/main/tests/functional/models/test_inventory.py
djyasin Oct 1, 2024
37bc2c1
Update awx/main/utils/plugins.py
djyasin Oct 1, 2024
5426730
Update awx/main/models/base.py
djyasin Oct 1, 2024
c48d416
Update awx/main/tests/functional/test_inventory_source_injectors.py
djyasin Oct 1, 2024
15a216d
Revise references to CLOUD_PROVIDERS to discover_available_cloud_prov…
djyasin Oct 1, 2024
b622935
Remove references to CLOUD_INVENTORY_SOURCES and replace with functio…
djyasin Oct 1, 2024
1eb787d
Replace references to CLOUD_PROVIDERS
djyasin Oct 2, 2024
711db39
Ran Black on plugins.py
djyasin Oct 2, 2024
309bd41
Remove reference to CLOUD_RPOVIDERS and rework docstring
djyasin Oct 2, 2024
77ab9d8
Remove @cache to resolve test failures TypeError: argument of type 'f…
djyasin Oct 2, 2024
b7d26ee
Remove unused commented out code
djyasin Oct 2, 2024
a08f0a8
Reinstating @cache because the removal did not resolve the errors
djyasin Oct 2, 2024
a0ccc50
Add iter to function and remove () from function call in tests to get…
djyasin Oct 2, 2024
9ab03a7
Remove iter from serializers
djyasin Oct 3, 2024
386c2b1
Update awx/main/models/base.py
djyasin Oct 3, 2024
05212c9
Update awx/main/models/inventory.py
djyasin Oct 3, 2024
a262a51
Update awx/main/models/inventory.py
djyasin Oct 3, 2024
cbc68c9
Update awx/main/models/inventory.py
djyasin Oct 3, 2024
a8f2ab0
Update awx/main/tests/functional/models/test_inventory.py
djyasin Oct 3, 2024
b4eee7a
Update awx/main/tests/functional/models/test_inventory.py
djyasin Oct 3, 2024
2a86100
Update awx/main/tests/functional/test_inventory_source_injectors.py
djyasin Oct 3, 2024
8e1a626
Replace discover_cloud_inventory_sources with compute_cloud_inventory…
djyasin Oct 3, 2024
81bfd51
Add doctsring and type to compute_cloud_inventory_sources
djyasin Oct 3, 2024
dab27f8
Update awx/main/models/base.py
djyasin Oct 4, 2024
24a9015
Remove unused import of 'awx.main.utils.execution_environments.to_con…
djyasin Oct 4, 2024
f287a23
Update awx/main/models/base.py
djyasin Oct 4, 2024
254c197
Update awx/main/models/__init__.py
djyasin Oct 4, 2024
a64f122
Move compute_cloud_inventory_sources to plugins.py and change related…
djyasin Oct 4, 2024
f57b698
Update awx/main/models/base.py
djyasin Oct 4, 2024
cd1ba87
Update awx/api/views/__init__.py
djyasin Oct 4, 2024
31077d8
Update awx/api/views/__init__.py
djyasin Oct 4, 2024
ca4eb22
Update awx/main/utils/plugins.py
djyasin Oct 4, 2024
cc57c4e
Ran make black on __init__.py to fix linting errors
djyasin Oct 4, 2024
4b300b5
Fix missing import of compute_cloud_inventory_sources
djyasin Oct 4, 2024
83e4924
correct import path error in __init__.py
djyasin Oct 4, 2024
25a61f4
Update awx/api/serializers.py
djyasin Oct 4, 2024
c4da8ef
Update awx/api/serializers.py
djyasin Oct 4, 2024
96a9180
Change inventory_source to discover_available_cloud_providers in test…
djyasin Oct 7, 2024
19fd764
Remove unused import to fix linting errors
djyasin Oct 7, 2024
d7e9356
Attempt to change paramertize to pytest subtests
djyasin Oct 7, 2024
428da54
Get tests passing
djyasin Oct 7, 2024
7509265
Fix additional tests impacted by changes and addition of compute_clou…
djyasin Oct 7, 2024
6b91f8a
Update awx/main/tests/functional/test_inventory_source_injectors.py
djyasin Oct 8, 2024
ae0a858
Update awx/main/tests/functional/test_inventory_source_injectors.py
djyasin Oct 8, 2024
4b78b87
Fix for failing test test_patch_constructed_inventory_generated_sourc…
djyasin Oct 8, 2024
9deade7
Attempt to fix test_inventory_update_injected_content
djyasin Oct 8, 2024
b55c9f8
Remove references to discover_available_cloud_provider_plugin_names c…
djyasin Oct 8, 2024
55b799b
Remove incorrect references to compute_cloud_inventory_sources
djyasin Oct 10, 2024
ce42cd4
Remove accidental change from scm to test_credentials_relationship_ma…
djyasin Oct 10, 2024
8141f9f
Revert trailing args in test_inventory_update_injected_content
djyasin Oct 10, 2024
6a98d88
Revert remaining incorrect changes with compute_cloud_resources
djyasin Oct 10, 2024
120b003
Attempt to update fixtures and begin fixing tests
djyasin Oct 10, 2024
036f771
Refine def test_patch_constructed_inventory_generated_source_limits_…
djyasin Oct 11, 2024
e602c0b
Correct test to get it passing addl comments in code
djyasin Oct 11, 2024
803ac0e
Uncomment out scm_inventory
djyasin Oct 11, 2024
f67d9f4
Got remaining tests passing
djyasin Oct 14, 2024
c7ba12b
REmoved unused import and undo code changes to get linters passing
djyasin Oct 14, 2024
1da92e2
Remove duplicate function for inventory_source
djyasin Oct 14, 2024
2a5875e
Got test_inventory_update_injected_content passing
djyasin Oct 15, 2024
33f47fc
Fix choices issue in serializer that was breaking logic
djyasin Oct 15, 2024
f49886b
Needed additional changes to get inventory source options working again
djyasin Oct 15, 2024
fc46d3e
Merge branch 'devel' into make_cloud_providers_dynamic_28722
djyasin Oct 16, 2024
ed9158f
Update awx/api/serializers.py
djyasin Oct 16, 2024
c669b39
Update awx/main/utils/plugins.py
djyasin Oct 16, 2024
7bac75e
Update awx/main/utils/plugins.py
djyasin Oct 16, 2024
30cd6ea
Update awx/main/tests/functional/conftest.py
djyasin Oct 16, 2024
f6b063f
Update awx/main/tests/functional/api/test_inventory.py
djyasin Oct 16, 2024
6742856
Remove unneeded comments
djyasin Oct 16, 2024
2f4a4f5
Uncomment out choicesin serializers
djyasin Oct 16, 2024
6d70934
Merge branch 'devel' into make_cloud_providers_dynamic_28722
djyasin Oct 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions awx/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@
WorkflowJobTemplate,
WorkflowJobTemplateNode,
StdoutMaxBytesExceeded,
CLOUD_INVENTORY_SOURCES,
)
from awx.main.models.base import VERBOSITY_CHOICES, NEW_JOB_TYPE_CHOICES
from awx.main.models.rbac import role_summary_fields_generator, give_creator_permissions, get_role_codenames, to_permissions, get_role_from_object_role
Expand All @@ -119,7 +118,9 @@
truncate_stdout,
get_licenser,
)

from awx.main.utils.filters import SmartFilter
from awx.main.utils.plugins import compute_cloud_inventory_sources
djyasin marked this conversation as resolved.
Show resolved Hide resolved
from awx.main.utils.named_url_graph import reset_counters
from awx.main.scheduler.task_manager_models import TaskManagerModels
from awx.main.redact import UriCleaner, REPLACE_STR
Expand Down Expand Up @@ -2300,6 +2301,7 @@ class Meta:

class InventorySourceOptionsSerializer(BaseSerializer):
credential = DeprecatedCredentialField(help_text=_('Cloud credential to use for inventory updates.'))
# source = serializers.ChoiceField(choices=[])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how does this work?


class Meta:
fields = (
Expand All @@ -2321,6 +2323,12 @@ class Meta:
)
read_only_fields = ('*', 'custom_virtualenv')

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
if 'source' in self.fields:
# Get the choices from compute_cloud_inventory_sources
self.fields['source'].choices = compute_cloud_inventory_sources() or {} # Fallback to empty dict if None

def get_related(self, obj):
res = super(InventorySourceOptionsSerializer, self).get_related(obj)
if obj.credential: # TODO: remove when 'credential' field is removed
Expand Down Expand Up @@ -2482,6 +2490,7 @@ def get_field_from_model_or_attrs(fd):
else:
redundant_scm_fields = list(filter(lambda x: attrs.get(x, None), ['source_project', 'source_path', 'scm_branch']))
if redundant_scm_fields:
breakpoint()
djyasin marked this conversation as resolved.
Show resolved Hide resolved
raise serializers.ValidationError({"detail": _("Cannot set %s if not SCM type." % ' '.join(redundant_scm_fields))})

project = get_field_from_model_or_attrs('source_project')
Expand Down Expand Up @@ -5500,7 +5509,7 @@ def get_summary_fields(self, obj):
return summary_fields

def validate_unified_job_template(self, value):
if type(value) == InventorySource and value.source not in CLOUD_INVENTORY_SOURCES:
if type(value) == InventorySource and value.source not in compute_cloud_inventory_sources():
raise serializers.ValidationError(_('Inventory Source must be a cloud resource.'))
elif type(value) == Project and value.scm_type == '':
raise serializers.ValidationError(_('Manual Project cannot have a schedule set.'))
Expand Down
5 changes: 3 additions & 2 deletions awx/api/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
)
from awx.main.utils.encryption import encrypt_value
from awx.main.utils.filters import SmartFilter
from awx.main.utils.plugins import compute_cloud_inventory_sources
from awx.main.redact import UriCleaner
from awx.api.permissions import (
JobTemplateCallbackPermission,
Expand Down Expand Up @@ -2196,9 +2197,9 @@ class InventorySourceNotificationTemplatesAnyList(SubListCreateAttachDetachAPIVi

def post(self, request, *args, **kwargs):
parent = self.get_parent_object()
if parent.source not in models.CLOUD_INVENTORY_SOURCES:
if parent.source not in compute_cloud_inventory_sources():
return Response(
dict(msg=_("Notification Templates can only be assigned when source is one of {}.").format(models.CLOUD_INVENTORY_SOURCES, parent.source)),
dict(msg=_("Notification Templates can only be assigned when source is one of {}.").format(compute_cloud_inventory_sources(), parent.source)),
status=status.HTTP_400_BAD_REQUEST,
)
return super(InventorySourceNotificationTemplatesAnyList, self).post(request, *args, **kwargs)
Expand Down
20 changes: 0 additions & 20 deletions awx/main/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,33 +6,13 @@
from django.utils.translation import gettext_lazy as _
djyasin marked this conversation as resolved.
Show resolved Hide resolved

__all__ = [
'CLOUD_PROVIDERS',
'PRIVILEGE_ESCALATION_METHODS',
'ANSI_SGR_PATTERN',
'CAN_CANCEL',
'ACTIVE_STATES',
'STANDARD_INVENTORY_UPDATE_ENV',
]

CLOUD_PROVIDERS = (
'azure_rm',
'ec2',
'gce',
'vmware',
'openstack',
'rhv',
'satellite6',
'controller',
'insights',
'terraform',
'openshift_virtualization',
'controller_supported',
'rhv_supported',
'openshift_virtualization_supported',
'insights_supported',
'satellite6_supported',
)

PRIVILEGE_ESCALATION_METHODS = [
('sudo', _('Sudo')),
('su', _('Su')),
Expand Down
23 changes: 23 additions & 0 deletions awx/main/migrations/0196_alter_inventorysource_source_and_more.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Generated by Django 4.2.10 on 2024-09-24 15:53

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('main', '0195_EE_permissions'),
]

operations = [
migrations.AlterField(
model_name='inventorysource',
name='source',
field=models.CharField(default=None, max_length=32),
),
migrations.AlterField(
model_name='inventoryupdate',
name='source',
field=models.CharField(default=None, max_length=32),
),
]
2 changes: 1 addition & 1 deletion awx/main/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from ansible_base.lib.utils.models import user_summary_fields

# AWX
from awx.main.models.base import BaseModel, PrimordialModel, accepts_json, CLOUD_INVENTORY_SOURCES, VERBOSITY_CHOICES # noqa
from awx.main.models.base import BaseModel, PrimordialModel, accepts_json, VERBOSITY_CHOICES # noqa
from awx.main.models.unified_jobs import UnifiedJob, UnifiedJobTemplate, StdoutMaxBytesExceeded # noqa
from awx.main.models.organization import Organization, Team, UserSessionMembership # noqa
from awx.main.models.credential import Credential, CredentialType, CredentialInputSource, ManagedCredentialType, build_safe_env # noqa
Expand Down
3 changes: 0 additions & 3 deletions awx/main/models/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

# AWX
from awx.main.utils import encrypt_field, parse_yaml_or_json
from awx.main.constants import CLOUD_PROVIDERS

__all__ = [
'VarsDictProperty',
Expand All @@ -32,7 +31,6 @@
'JOB_TYPE_CHOICES',
'AD_HOC_JOB_TYPE_CHOICES',
'PROJECT_UPDATE_JOB_TYPE_CHOICES',
'CLOUD_INVENTORY_SOURCES',
'VERBOSITY_CHOICES',
]

Expand Down Expand Up @@ -61,7 +59,6 @@
(PERM_INVENTORY_CHECK, _('Check')),
]

CLOUD_INVENTORY_SOURCES = list(CLOUD_PROVIDERS) + ['scm']

VERBOSITY_CHOICES = [
(0, '0 (Normal)'),
Expand Down
35 changes: 7 additions & 28 deletions awx/main/models/inventory.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@

# AWX
from awx.api.versioning import reverse
from awx.main.constants import CLOUD_PROVIDERS
from awx.main.utils.plugins import discover_available_cloud_provider_plugin_names, compute_cloud_inventory_sources
from awx.main.consumers import emit_channel_notification
from awx.main.fields import (
ImplicitRoleField,
SmartFilterField,
OrderedManyToManyField,
)
from awx.main.managers import HostManager, HostMetricActiveManager
from awx.main.models.base import BaseModel, CommonModelNameNotUnique, VarsDictProperty, CLOUD_INVENTORY_SOURCES, accepts_json
from awx.main.models.base import BaseModel, CommonModelNameNotUnique, VarsDictProperty, accepts_json
from awx.main.models.events import InventoryUpdateEvent, UnpartitionedInventoryUpdateEvent
from awx.main.models.unified_jobs import UnifiedJob, UnifiedJobTemplate
from awx.main.models.mixins import (
Expand Down Expand Up @@ -394,7 +394,7 @@ def update_computed_fields(self):
if self.kind == 'smart':
active_inventory_sources = self.inventory_sources.none()
else:
active_inventory_sources = self.inventory_sources.filter(source__in=CLOUD_INVENTORY_SOURCES)
active_inventory_sources = self.inventory_sources.filter(source__in=compute_cloud_inventory_sources())
failed_inventory_sources = active_inventory_sources.filter(last_job_failed=True)
total_hosts = active_hosts.count()
# if total_hosts has changed, set update_task_impact to True
Expand Down Expand Up @@ -914,23 +914,6 @@ class InventorySourceOptions(BaseModel):

injectors = dict()

SOURCE_CHOICES = [
('file', _('File, Directory or Script')),
djyasin marked this conversation as resolved.
Show resolved Hide resolved
('constructed', _('Template additional groups and hostvars at runtime')),
('scm', _('Sourced from a Project')),
('ec2', _('Amazon EC2')),
('gce', _('Google Compute Engine')),
('azure_rm', _('Microsoft Azure Resource Manager')),
('vmware', _('VMware vCenter')),
('satellite6', _('Red Hat Satellite 6')),
('openstack', _('OpenStack')),
('rhv', _('Red Hat Virtualization')),
('controller', _('Red Hat Ansible Automation Platform')),
('insights', _('Red Hat Insights')),
('terraform', _('Terraform State')),
('openshift_virtualization', _('OpenShift Virtualization')),
]

# From the options of the Django management base command
INVENTORY_UPDATE_VERBOSITY_CHOICES = [
(0, '0 (WARNING)'),
Expand All @@ -943,7 +926,6 @@ class Meta:

source = models.CharField(
max_length=32,
choices=SOURCE_CHOICES,
djyasin marked this conversation as resolved.
Show resolved Hide resolved
blank=False,
default=None,
)
Expand Down Expand Up @@ -1047,7 +1029,7 @@ def cloud_credential_validation(source, cred):
# Allow an EC2 source to omit the credential. If Tower is running on
# an EC2 instance with an IAM Role assigned, boto will use credentials
# from the instance metadata instead of those explicitly provided.
elif source in CLOUD_PROVIDERS and source not in ['ec2', 'openshift_virtualization']:
elif source in discover_available_cloud_provider_plugin_names() and source not in ['ec2', 'openshift_virtualization']:
return _('Credential is required for a cloud source.')
elif source == 'custom' and cred and cred.credential_type.kind in ('scm', 'ssh', 'insights', 'vault'):
return _('Credentials of type machine, source control, insights and vault are disallowed for custom inventory sources.')
Expand All @@ -1061,11 +1043,8 @@ def get_cloud_credential(self):
"""Return the credential which is directly tied to the inventory source type."""
credential = None
for cred in self.credentials.all():
if self.source in CLOUD_PROVIDERS:
source = self.source.replace('ec2', 'aws')
if source.endswith('_supported'):
source = source[:-10]
if cred.kind == source:
if self.source in discover_available_cloud_provider_plugin_names():
if cred.kind == self.source.replace('ec2', 'aws'):
credential = cred
break
else:
Expand All @@ -1080,7 +1059,7 @@ def get_extra_credentials(self):
These are all credentials that should run their own inject_credential logic.
"""
special_cred = None
if self.source in CLOUD_PROVIDERS:
if self.source in discover_available_cloud_provider_plugin_names():
# these have special injection logic associated with them
special_cred = self.get_cloud_credential()
extra_creds = []
Expand Down
20 changes: 3 additions & 17 deletions awx/main/tests/functional/api/test_inventory.py
Original file line number Diff line number Diff line change
Expand Up @@ -609,23 +609,9 @@ def test_get_constructed_inventory(self, constructed_inventory, admin_user, get)
r = get(url=reverse('api:constructed_inventory_detail', kwargs={'pk': constructed_inventory.pk}), user=admin_user, expect=200)
assert r.data['update_cache_timeout'] == 53

def test_patch_constructed_inventory(self, constructed_inventory, admin_user, patch):
def test_patch_constructed_inventory_generated_source_limits_editable_fields(self, constructed_inventory, admin_user, project, patch, inventory):
djyasin marked this conversation as resolved.
Show resolved Hide resolved
inv_src = constructed_inventory.inventory_sources.first()
assert inv_src.update_cache_timeout == 0
assert inv_src.limit == ''
r = patch(
url=reverse('api:constructed_inventory_detail', kwargs={'pk': constructed_inventory.pk}),
data=dict(update_cache_timeout=54, limit='foobar'),
user=admin_user,
expect=200,
)
assert r.data['update_cache_timeout'] == 54
inv_src = constructed_inventory.inventory_sources.first()
assert inv_src.update_cache_timeout == 54
assert inv_src.limit == 'foobar'

def test_patch_constructed_inventory_generated_source_limits_editable_fields(self, constructed_inventory, admin_user, project, patch):
inv_src = constructed_inventory.inventory_sources.first()
r = patch(
url=inv_src.get_absolute_url(),
data={
Expand All @@ -637,9 +623,9 @@ def test_patch_constructed_inventory_generated_source_limits_editable_fields(sel
expect=400,
user=admin_user,
)
assert str(r.data['error'][0]) == "Cannot change field 'source' on a constructed inventory source."

# Make sure it didn't get updated before we got the error
assert r.status_code == 400, f"Expected 400, but got: {r.status_code}"

inv_src_after_err = constructed_inventory.inventory_sources.first()
assert inv_src.id == inv_src_after_err.id
assert inv_src.source == inv_src_after_err.source
Expand Down
26 changes: 25 additions & 1 deletion awx/main/tests/functional/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
from awx.main.models.oauth import OAuth2Application as Application
from awx.main.models.execution_environments import ExecutionEnvironment
from awx.main.utils import is_testing
from awx.main.utils.plugins import compute_cloud_inventory_sources

__SWAGGER_REQUESTS__ = {}

Expand Down Expand Up @@ -351,9 +352,27 @@ def kube_credential(credentialtype_kube):
)


# @pytest.fixture
# def inventory(organization):
# return organization.inventories.create(name="test-inv")


@pytest.fixture
def inventory(organization):
return organization.inventories.create(name="test-inv")
available_providers = tuple(compute_cloud_inventory_sources())

if available_providers:
Copy link
Member

@webknjaz webknjaz Oct 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests shouldn't have any complex logic, the fewer checks the better. The checks for prerequisite expectations can be assertions:

Suggested change
if available_providers:
assert len(available_providers) > 0, 'cloud inventory plugins must be installed'

But that's about it. Otherwise, you'll end up with the need to test logic in your tests, which is not something common.

Fixtures are mostly used as an Arrange part of AAA: https://jamescooke.info/arrange-act-assert-pattern-for-python-developers.html / https://jamescooke.info/aaa-part-2-extracting-arrange-code-to-make-fixtures.html / https://docs.pytest.org/en/stable/explanation/anatomy.html.

Then, the tests can rely on the fixture to set everything up correctly. If some expectation isn't met, it'll still fail in the test run, we don't need to add a lot of nested checks for those things.

if not organization.inventories.exists():
organization.inventories.create(name="test-inv")

first_inventory = organization.inventories.first()

if first_inventory:
return first_inventory
else:
raise ValueError("No inventories found in the queryset")
else:
raise ValueError("No available cloud providers found")
djyasin marked this conversation as resolved.
Show resolved Hide resolved


@pytest.fixture
Expand Down Expand Up @@ -901,3 +920,8 @@ def control_plane_execution_environment():
@pytest.fixture
def default_job_execution_environment():
return ExecutionEnvironment.objects.create(name="Default Job EE", managed=False)


@pytest.fixture
def fixture_compute_cloud_inventory_sources():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to use “fixture” in fixture names. It's common to use the produced thing name, as that's what's injected into the tests. Additionally, fixtures should have docstrings for pytest --collect-only --fixtures.

Suggested change
def fixture_compute_cloud_inventory_sources():
def computed_cloud_inventory_sources() -> dict[str, str]:
"""A mapping of cloud inventory source names to labels."""

return compute_cloud_inventory_sources()
6 changes: 3 additions & 3 deletions awx/main/tests/functional/models/test_inventory.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@

# AWX
from awx.main.models import Host, Inventory, InventorySource, InventoryUpdate, CredentialType, Credential, Job
from awx.main.constants import CLOUD_PROVIDERS
from awx.main.utils.filters import SmartFilter
from awx.main.utils.plugins import discover_available_cloud_provider_plugin_names


@pytest.mark.django_db
Expand Down Expand Up @@ -166,11 +166,11 @@ def test_extra_credentials(self, project, credential):

def test_all_cloud_sources_covered(self):
"""Code in several places relies on the fact that the older
CLOUD_PROVIDERS constant contains the same names as what are
discover_cloud_provider_plugin_names returns the same names as what are
defined within the injectors
"""
# slight exception case for constructed, because it has a FQCN but is not a cloud source
assert set(CLOUD_PROVIDERS) | set(['constructed']) == set(InventorySource.injectors.keys())
assert set(discover_available_cloud_provider_plugin_names()) | set(['constructed']) == set(InventorySource.injectors.keys())

@pytest.mark.parametrize('source,filename', [('ec2', 'aws_ec2.yml'), ('openstack', 'openstack.yml'), ('gce', 'gcp_compute.yml')])
def test_plugin_filenames(self, source, filename):
Expand Down
8 changes: 4 additions & 4 deletions awx/main/tests/functional/test_inventory_source_injectors.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@

from awx.main.tasks.jobs import RunInventoryUpdate
from awx.main.models import InventorySource, Credential, CredentialType, UnifiedJob, ExecutionEnvironment
from awx.main.constants import CLOUD_PROVIDERS, STANDARD_INVENTORY_UPDATE_ENV
from awx.main.constants import STANDARD_INVENTORY_UPDATE_ENV
from awx.main.tests import data

from awx.main.utils.plugins import discover_available_cloud_provider_plugin_names
from django.conf import settings

DATA = os.path.join(os.path.dirname(data.__file__), 'inventory')
Expand Down Expand Up @@ -193,7 +193,7 @@ def create_reference_data(source_dir, env, content):


@pytest.mark.django_db
@pytest.mark.parametrize('this_kind', CLOUD_PROVIDERS)
@pytest.mark.parametrize('this_kind', discover_available_cloud_provider_plugin_names())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking into the test failures, it seems to me that my suspicion was likely right and this might need to be moved away from import-time invocation. And I'd try converting this to use pytest-subtests.

def test_inventory_update_injected_content(this_kind, inventory, fake_credential_factory, mock_me):
if this_kind.endswith('_supported'):
this_kind = this_kind[:-10]
Expand All @@ -202,7 +202,7 @@ def test_inventory_update_injected_content(this_kind, inventory, fake_credential
ExecutionEnvironment.objects.create(name='Default Job EE', managed=False)

injector = InventorySource.injectors[this_kind]
if injector.plugin_name is None:
if injector.plugin_name in (None, 'constructed'):
pytest.skip('Use of inventory plugin is not enabled for this source')

src_vars = dict(base_source_var='value_of_var')
Expand Down
Loading
Loading