From 5e13da62a48a3e391bf10d0c73233aaf782748b9 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 6 Feb 2019 10:14:46 -0500 Subject: [PATCH 01/27] Added a max_hosts field to Organization in order to optionally set a limit to the number of hosts an organization that is sharing a license is allowed to manage. related #1542 --- awx/main/models/organization.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/awx/main/models/organization.py b/awx/main/models/organization.py index 8379cfa0cd..ac7b41bda9 100644 --- a/awx/main/models/organization.py +++ b/awx/main/models/organization.py @@ -10,6 +10,7 @@ from django.db.models import Q from django.contrib.auth.models import User from django.contrib.sessions.models import Session from django.utils.timezone import now as tz_now +from django.utils.translation import ugettext_lazy as _ # AWX @@ -42,6 +43,12 @@ class Organization(CommonModel, NotificationFieldsModel, ResourceMixin, CustomVi 'InstanceGroup', blank=True, ) + max_hosts = models.PositiveIntegerField( + blank=True, + default=0, + help_text=_('Maximum number of hosts allowed to be managed by this organization.'), + ) + admin_role = ImplicitRoleField( parent_role='singleton:' + ROLE_SINGLETON_SYSTEM_ADMINISTRATOR, ) From 6d70651611550cb373d4136e16c72f1b1be9b307 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Thu, 7 Feb 2019 16:40:07 -0500 Subject: [PATCH 02/27] Update the inventory_import management command to respect the new Organization.max_hosts limit. --- .../management/commands/inventory_import.py | 35 +++++++++++++++++-- awx/main/models/inventory.py | 4 +++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/awx/main/management/commands/inventory_import.py b/awx/main/management/commands/inventory_import.py index 3224edf5dd..0974dcb7fa 100644 --- a/awx/main/management/commands/inventory_import.py +++ b/awx/main/management/commands/inventory_import.py @@ -904,10 +904,27 @@ class Command(BaseCommand): logger.error(LICENSE_MESSAGE % d) raise CommandError('License count exceeded!') + def check_org_host_limit(self): + license_info = get_licenser().validate() + if license_info.get('license_type', 'UNLICENSED') == 'open': + return + + org = self.inventory.organization + if org is None or org.max_hosts == 0: + return + + active_count = Host.objects.org_active_count(org.id) + if active_count > org.max_hosts: + raise CommandError('Host limit for organization exceeded!') + def mark_license_failure(self, save=True): self.inventory_update.license_error = True self.inventory_update.save(update_fields=['license_error']) + def mark_org_limits_failure(self, save=True): + self.inventory_update.org_host_limit_error = True + self.inventory_update.save(update_fields=['org_host_limit_error']) + def handle(self, *args, **options): self.verbosity = int(options.get('verbosity', 1)) self.set_logging_level() @@ -961,6 +978,13 @@ class Command(BaseCommand): self.mark_license_failure(save=True) raise e + try: + # Check the per-org host limits + self.check_org_host_limit() + except CommandError as e: + self.mark_org_limits_failure(save=True) + raise e + status, tb, exc = 'error', '', None try: if settings.SQL_DEBUG: @@ -1032,9 +1056,17 @@ class Command(BaseCommand): # If the license is not valid, a CommandError will be thrown, # and inventory update will be marked as invalid. # with transaction.atomic() will roll back the changes. + license_fail = True self.check_license() + + # Check the per-org host limits + license_fail = False + self.check_org_host_limit() except CommandError as e: - self.mark_license_failure() + if license_fail: + self.mark_license_failure() + else: + self.mark_org_limits_failure() raise e if settings.SQL_DEBUG: @@ -1062,7 +1094,6 @@ class Command(BaseCommand): else: tb = traceback.format_exc() exc = e - transaction.rollback() if self.invoked_from_dispatcher is False: with ignore_inventory_computed_fields(): diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index b098d2e4a9..df04d5b1ad 100644 --- a/awx/main/models/inventory.py +++ b/awx/main/models/inventory.py @@ -1653,6 +1653,10 @@ class InventoryUpdate(UnifiedJob, InventorySourceOptions, JobNotificationMixin, default=False, editable=False, ) + org_host_limit_error = models.BooleanField( + default=False, + editable=False, + ) source_project_update = models.ForeignKey( 'ProjectUpdate', related_name='scm_inventory_updates', From c3406748de71900354997cc7d658ccb524fd274b Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 13 Feb 2019 10:58:10 -0500 Subject: [PATCH 03/27] Add the new fields to the database --- .../migrations/0063_v350_org_host_limits.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 awx/main/migrations/0063_v350_org_host_limits.py diff --git a/awx/main/migrations/0063_v350_org_host_limits.py b/awx/main/migrations/0063_v350_org_host_limits.py new file mode 100644 index 0000000000..bc410e757a --- /dev/null +++ b/awx/main/migrations/0063_v350_org_host_limits.py @@ -0,0 +1,25 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.16 on 2019-02-15 20:03 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0062_v350_new_playbook_stats'), + ] + + operations = [ + migrations.AddField( + model_name='inventoryupdate', + name='org_host_limit_error', + field=models.BooleanField(default=False, editable=False), + ), + migrations.AddField( + model_name='organization', + name='max_hosts', + field=models.PositiveIntegerField(blank=True, default=0, help_text='Maximum number of hosts allowed to be managed by this organization.'), + ), + ] From e44c73883e59baf5455ba0426585d667b6fd1f2e Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 6 Feb 2019 10:38:40 -0500 Subject: [PATCH 04/27] Expose Organization.max_hosts in the API --- awx/api/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 72c1121c39..f3a541657a 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1229,7 +1229,7 @@ class OrganizationSerializer(BaseSerializer): class Meta: model = Organization - fields = ('*', 'custom_virtualenv',) + fields = ('*', 'max_hosts', 'custom_virtualenv',) def get_related(self, obj): res = super(OrganizationSerializer, self).get_related(obj) From 6399ec59c966a2966c06f20eed952bc707de372b Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 6 Feb 2019 13:26:13 -0500 Subject: [PATCH 05/27] Include in the API the count of hosts used by an organization --- awx/api/views/mixin.py | 8 ++++++-- awx/api/views/organization.py | 2 ++ awx/main/managers.py | 28 ++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/awx/api/views/mixin.py b/awx/api/views/mixin.py index ee174d5091..191327c64f 100644 --- a/awx/api/views/mixin.py +++ b/awx/api/views/mixin.py @@ -29,7 +29,7 @@ from awx.main.models.ha import ( ) from awx.main.models.organization import Team from awx.main.models.projects import Project -from awx.main.models.inventory import Inventory +from awx.main.models.inventory import Inventory, Host from awx.main.models.jobs import JobTemplate from awx.conf.license import ( feature_enabled, @@ -235,6 +235,8 @@ class OrganizationCountsMixin(object): db_results['projects'] = project_qs\ .values('organization').annotate(Count('organization')).order_by('organization') + db_results['hosts'] = Host.objects.active_counts_by_org() + # Other members and admins of organization are always viewable db_results['users'] = org_qs.annotate( users=Count('member_role__members', distinct=True), @@ -246,7 +248,7 @@ class OrganizationCountsMixin(object): org_id = org['id'] count_context[org_id] = { 'inventories': 0, 'teams': 0, 'users': 0, 'job_templates': 0, - 'admins': 0, 'projects': 0} + 'admins': 0, 'projects': 0, 'hosts': 0} for res, count_qs in db_results.items(): if res == 'job_templates_project': @@ -255,6 +257,8 @@ class OrganizationCountsMixin(object): org_reference = JT_inventory_reference elif res == 'users': org_reference = 'id' + elif res == 'hosts': + org_reference = 'inventory__organization' else: org_reference = 'organization' for entry in count_qs: diff --git a/awx/api/views/organization.py b/awx/api/views/organization.py index 3c46ad3cca..0a6e40ba53 100644 --- a/awx/api/views/organization.py +++ b/awx/api/views/organization.py @@ -17,6 +17,7 @@ from awx.conf.license import ( from awx.main.models import ( ActivityStream, Inventory, + Host, Project, JobTemplate, WorkflowJobTemplate, @@ -119,6 +120,7 @@ class OrganizationDetail(RelatedJobsPreventDeleteMixin, RetrieveUpdateDestroyAPI organization__id=org_id).count() org_counts['job_templates'] = JobTemplate.accessible_objects(**access_kwargs).filter( project__organization__id=org_id).count() + org_counts['hosts'] = Host.objects.org_active_count(org_id) full_context['related_field_counts'] = {} full_context['related_field_counts'][org_id] = org_counts diff --git a/awx/main/managers.py b/awx/main/managers.py index 341a7e806a..d554792699 100644 --- a/awx/main/managers.py +++ b/awx/main/managers.py @@ -29,6 +29,34 @@ class HostManager(models.Manager): """ return self.order_by().exclude(inventory_sources__source='tower').values('name').distinct().count() + def org_active_count(self, org_id): + """Return count of active, unique hosts used by an organization. + Construction of query involves: + - remove any ordering specified in model's Meta + - Exclude hosts sourced from another Tower + - Consider only hosts where the canonical inventory is owned by the organization + - Restrict the query to only return the name column + - Only consider results that are unique + - Return the count of this query + """ + return self.order_by().exclude( + inventory_sources__source='tower' + ).filter(inventory__organization=org_id).values('name').distinct().count() + + def active_counts_by_org(self): + """Return the counts of active, unique hosts for each organization. + Construction of query involves: + - remove any ordering specified in model's Meta + - Exclude hosts sourced from another Tower + - Consider only hosts where the canonical inventory is owned by each organization + - Restrict the query to only count distinct names + - Return the counts + """ + return self.order_by().exclude( + inventory_sources__source='tower' + ).values('inventory__organization').annotate( + inventory__organization__count=models.Count('name', distinct=True)) + def get_queryset(self): """When the parent instance of the host query set has a `kind=smart` and a `host_filter` set. Use the `host_filter` to generate the queryset for the hosts. From 0e8e5f65e16da84485859ac4979f023256671c7c Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 6 Feb 2019 16:39:18 -0500 Subject: [PATCH 06/27] Update to fix tests --- awx/api/serializers.py | 2 +- .../functional/api/test_organization_counts.py | 17 +++++++++++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index f3a541657a..bbd2aa8781 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1259,7 +1259,7 @@ class OrganizationSerializer(BaseSerializer): if counts_dict is not None and summary_dict is not None: if obj.id not in counts_dict: summary_dict['related_field_counts'] = { - 'inventories': 0, 'teams': 0, 'users': 0, + 'inventories': 0, 'teams': 0, 'users': 0, 'hosts': 0, 'job_templates': 0, 'admins': 0, 'projects': 0} else: summary_dict['related_field_counts'] = counts_dict[obj.id] diff --git a/awx/main/tests/functional/api/test_organization_counts.py b/awx/main/tests/functional/api/test_organization_counts.py index 9c4f536b09..853c3f9937 100644 --- a/awx/main/tests/functional/api/test_organization_counts.py +++ b/awx/main/tests/functional/api/test_organization_counts.py @@ -5,7 +5,7 @@ from awx.api.versioning import reverse @pytest.fixture def organization_resource_creator(organization, user): - def rf(users, admins, job_templates, projects, inventories, teams): + def rf(users, admins, job_templates, projects, inventories, teams, hosts): # Associate one resource of every type with the organization for i in range(users): @@ -48,7 +48,8 @@ COUNTS_PRIMES = { 'job_templates': 3, 'projects': 3, 'inventories': 7, - 'teams': 5 + 'teams': 5, + 'hosts': 0, } COUNTS_ZEROS = { 'users': 0, @@ -56,7 +57,8 @@ COUNTS_ZEROS = { 'job_templates': 0, 'projects': 0, 'inventories': 0, - 'teams': 0 + 'teams': 0, + 'hosts': 0, } @@ -92,7 +94,8 @@ def test_org_counts_detail_member(resourced_organization, user, get): 'job_templates': 0, 'projects': 0, 'inventories': 0, - 'teams': 0 + 'teams': 0, + 'hosts': 0, } @@ -123,7 +126,8 @@ def test_org_counts_list_member(resourced_organization, user, get): 'job_templates': 0, 'projects': 0, 'inventories': 0, - 'teams': 0 + 'teams': 0, + 'hosts': 0, } @@ -230,5 +234,6 @@ def test_JT_associated_with_project(organizations, project, user, get): 'job_templates': 1, 'projects': 1, 'inventories': 0, - 'teams': 0 + 'teams': 0, + 'hosts': 0, } From 36ed890c14f7d681094af6215a2a998bd43f26b9 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Thu, 7 Feb 2019 11:04:47 -0500 Subject: [PATCH 07/27] Add permissions checks for the organization host limit --- awx/main/access.py | 67 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 55 insertions(+), 12 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 93150adef8..e5edac1b04 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -330,6 +330,25 @@ class BaseAccess(object): elif "features" not in validation_info: raise LicenseForbids(_("Features not found in active license.")) + def check_org_host_limit(self, data, add_host_name=None): + validation_info = get_licenser().validate() + if validation_info.get('license_type', 'UNLICENSED') == 'open': + return + + inventory = get_object_from_data('inventory', Inventory, data) + org = inventory.organization + if org is None or org.max_hosts == 0: + return + + active_count = Host.objects.org_active_count(org.id) + if active_count > org.max_hosts: + raise PermissionDenied(_("Organization host limit of %s has been exceeded.") % org.max_hosts) + + if add_host_name: + host_exists = Host.objects.filter(inventory__organization=org.id, name=add_host_name).exists() + if not host_exists and active_count == org.max_hosts: + raise PermissionDenied(_("Organization host limit of %s has been reached.") % org.max_hosts) + def get_user_capabilities(self, obj, method_list=[], parent_obj=None, capabilities_cache={}): if obj is None: return {} @@ -360,7 +379,7 @@ class BaseAccess(object): user_capabilities[display_method] = self.user.is_superuser continue elif display_method == 'copy' and isinstance(obj, Project) and obj.scm_type == '': - # Connot copy manual project without errors + # Cannot copy manual project without errors user_capabilities[display_method] = False continue elif display_method in ['start', 'schedule'] and isinstance(obj, Group): # TODO: remove in 3.3 @@ -628,7 +647,7 @@ class OAuth2ApplicationAccess(BaseAccess): return self.model.objects.filter(organization__in=org_access_qs) def can_change(self, obj, data): - return self.user.is_superuser or self.check_related('organization', Organization, data, obj=obj, + return self.user.is_superuser or self.check_related('organization', Organization, data, obj=obj, role_field='admin_role', mandatory=True) def can_delete(self, obj): @@ -636,7 +655,7 @@ class OAuth2ApplicationAccess(BaseAccess): def can_add(self, data): if self.user.is_superuser: - return True + return True if not data: return Organization.accessible_objects(self.user, 'admin_role').exists() return self.check_related('organization', Organization, data, role_field='admin_role', mandatory=True) @@ -650,29 +669,29 @@ class OAuth2TokenAccess(BaseAccess): - I am the user of the token. I can create an OAuth2 app token when: - I have the read permission of the related application. - I can read, change or delete a personal token when: + I can read, change or delete a personal token when: - I am the user of the token - I am the superuser I can create an OAuth2 Personal Access Token when: - - I am a user. But I can only create a PAT for myself. + - I am a user. But I can only create a PAT for myself. ''' model = OAuth2AccessToken - + select_related = ('user', 'application') - - def filtered_queryset(self): + + def filtered_queryset(self): org_access_qs = Organization.objects.filter( Q(admin_role__members=self.user) | Q(auditor_role__members=self.user)) return self.model.objects.filter(application__organization__in=org_access_qs) | self.model.objects.filter(user__id=self.user.pk) - + def can_delete(self, obj): if (self.user.is_superuser) | (obj.user == self.user): return True elif not obj.application: return False return self.user in obj.application.organization.admin_role - + def can_change(self, obj, data): return self.can_delete(obj) @@ -840,6 +859,10 @@ class HostAccess(BaseAccess): # Check to see if we have enough licenses self.check_license(add_host_name=data.get('name', None)) + + # Check the per-org limit + self.check_org_host_limit(data, add_host_name=data.get('name', None)) + return True def can_change(self, obj, data): @@ -852,6 +875,9 @@ class HostAccess(BaseAccess): if data and 'name' in data: self.check_license(add_host_name=data['name']) + # Check the per-org limit + self.check_org_host_limit(data, add_host_name=data['name']) + # Checks for admin or change permission on inventory, controls whether # the user can edit variable data. return obj and self.user in obj.inventory.admin_role @@ -1346,7 +1372,7 @@ class JobTemplateAccess(BaseAccess): return self.user in project.use_role else: return False - + @check_superuser def can_copy_related(self, obj): ''' @@ -1362,6 +1388,10 @@ class JobTemplateAccess(BaseAccess): # Check license. if validate_license: self.check_license() + + # Check the per-org limit + self.check_org_host_limit({'inventory': obj}) + if obj.survey_enabled: self.check_license(feature='surveys') if Instance.objects.active_count() > 1: @@ -1520,6 +1550,9 @@ class JobAccess(BaseAccess): if validate_license: self.check_license() + # Check the per-org limit + self.check_org_host_limit({'inventory': obj}) + # A super user can relaunch a job if self.user.is_superuser: return True @@ -1886,6 +1919,10 @@ class WorkflowJobTemplateAccess(BaseAccess): if validate_license: # check basic license, node count self.check_license() + + # Check the per-org limit + self.check_org_host_limit({'inventory': obj}) + # if surveys are added to WFJTs, check license here if obj.survey_enabled: self.check_license(feature='surveys') @@ -1957,6 +1994,9 @@ class WorkflowJobAccess(BaseAccess): if validate_license: self.check_license() + # Check the per-org limit + self.check_org_host_limit({'inventory': obj}) + if self.user.is_superuser: return True @@ -2033,6 +2073,9 @@ class AdHocCommandAccess(BaseAccess): if validate_license: self.check_license() + # Check the per-org limit + self.check_org_host_limit(data) + # If a credential is provided, the user should have use access to it. if not self.check_related('credential', Credential, data, role_field='use_role'): return False @@ -2442,7 +2485,7 @@ class ActivityStreamAccess(BaseAccess): model = ActivityStream prefetch_related = ('organization', 'user', 'inventory', 'host', 'group', 'inventory_update', 'credential', 'credential_type', 'team', - 'ad_hoc_command', 'o_auth2_application', 'o_auth2_access_token', + 'ad_hoc_command', 'o_auth2_application', 'o_auth2_access_token', 'notification_template', 'notification', 'label', 'role', 'actor', 'schedule', 'custom_inventory_script', 'unified_job_template', 'workflow_job_template_node',) From 43a0a15f6fc20f84128065414e95a4a2ad965463 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Thu, 7 Feb 2019 16:53:12 -0500 Subject: [PATCH 08/27] Expose the new InventoryUpdate.org_host_limit_error field in the API --- awx/api/serializers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index bbd2aa8781..8dbb1e273e 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -2202,8 +2202,8 @@ class InventoryUpdateSerializer(UnifiedJobSerializer, InventorySourceOptionsSeri class Meta: model = InventoryUpdate - fields = ('*', 'inventory', 'inventory_source', 'license_error', 'source_project_update', - 'custom_virtualenv', '-controller_node',) + fields = ('*', 'inventory', 'inventory_source', 'license_error', 'org_host_limit_error', + 'source_project_update', 'custom_virtualenv', '-controller_node',) def get_related(self, obj): res = super(InventoryUpdateSerializer, self).get_related(obj) From df8a66e504c21ed96ab4189f3d76250616a82e11 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Mon, 11 Feb 2019 15:51:40 -0500 Subject: [PATCH 09/27] Correct the org limit check for changing hosts to use the host's org instead of an inventory passed in from the user data, which is not allowed. --- awx/main/access.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/awx/main/access.py b/awx/main/access.py index e5edac1b04..89c55b5ee1 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -876,7 +876,8 @@ class HostAccess(BaseAccess): self.check_license(add_host_name=data['name']) # Check the per-org limit - self.check_org_host_limit(data, add_host_name=data['name']) + self.check_org_host_limit({'inventory': obj.inventory}, + add_host_name=data['name']) # Checks for admin or change permission on inventory, controls whether # the user can edit variable data. From f5d7ca6913c29e0970d65d90e46e0c873ff53bd4 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Tue, 12 Feb 2019 11:28:30 -0500 Subject: [PATCH 10/27] Update the Organization API list and detail tests to check the host counts --- .../functional/api/test_organization_counts.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/awx/main/tests/functional/api/test_organization_counts.py b/awx/main/tests/functional/api/test_organization_counts.py index 853c3f9937..77552079e1 100644 --- a/awx/main/tests/functional/api/test_organization_counts.py +++ b/awx/main/tests/functional/api/test_organization_counts.py @@ -1,3 +1,5 @@ +import itertools + import pytest from awx.api.versioning import reverse @@ -17,7 +19,10 @@ def organization_resource_creator(organization, user): for i in range(teams): organization.teams.create(name='org-team %s' % i) for i in range(inventories): - inventory = organization.inventories.create(name="associated-inv %s" % i) + organization.inventories.create(name="associated-inv %s" % i) + for i, inventory in zip(range(hosts), itertools.cycle(organization.inventories.all())): + # evenly distribute the hosts over all of the org's inventories + inventory.hosts.create(name="host %s" % i) for i in range(projects): organization.projects.create(name="test-proj %s" % i, description="test-proj-desc") @@ -49,7 +54,7 @@ COUNTS_PRIMES = { 'projects': 3, 'inventories': 7, 'teams': 5, - 'hosts': 0, + 'hosts': 7, } COUNTS_ZEROS = { 'users': 0, @@ -86,6 +91,7 @@ def test_org_counts_detail_member(resourced_organization, user, get): response = get(reverse('api:organization_detail', kwargs={'pk': resourced_organization.pk}), member_user) assert response.status_code == 200 + assert response.data['max_hosts'] == 0 counts = response.data['summary_fields']['related_field_counts'] assert counts == { @@ -95,7 +101,7 @@ def test_org_counts_detail_member(resourced_organization, user, get): 'projects': 0, 'inventories': 0, 'teams': 0, - 'hosts': 0, + 'hosts': 7, } @@ -117,9 +123,9 @@ def test_org_counts_list_member(resourced_organization, user, get): member_user = resourced_organization.member_role.members.get(username='org-member 1') response = get(reverse('api:organization_list'), member_user) assert response.status_code == 200 + assert response.data['results'][0]['max_hosts'] == 0 counts = response.data['results'][0]['summary_fields']['related_field_counts'] - assert counts == { 'users': COUNTS_PRIMES['users'], # Policy is that members can see other users and admins 'admins': COUNTS_PRIMES['admins'], @@ -127,7 +133,7 @@ def test_org_counts_list_member(resourced_organization, user, get): 'projects': 0, 'inventories': 0, 'teams': 0, - 'hosts': 0, + 'hosts': 7, } From 875a1c0b5f23a57d2439d84cbb8cd13825007463 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Tue, 12 Feb 2019 13:15:34 -0500 Subject: [PATCH 11/27] Remove the mention of the max_hosts value from the limit check messages --- awx/main/access.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 89c55b5ee1..e688387753 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -342,12 +342,12 @@ class BaseAccess(object): active_count = Host.objects.org_active_count(org.id) if active_count > org.max_hosts: - raise PermissionDenied(_("Organization host limit of %s has been exceeded.") % org.max_hosts) + raise PermissionDenied(_("The organization host limit has been exceeded.")) if add_host_name: host_exists = Host.objects.filter(inventory__organization=org.id, name=add_host_name).exists() if not host_exists and active_count == org.max_hosts: - raise PermissionDenied(_("Organization host limit of %s has been reached.") % org.max_hosts) + raise PermissionDenied(_("The organization host limit has been exceeded.")) def get_user_capabilities(self, obj, method_list=[], parent_obj=None, capabilities_cache={}): if obj is None: From 4afd0672a17440d1e4b06e78a6f34d5d31aaf848 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Tue, 12 Feb 2019 15:08:48 -0500 Subject: [PATCH 12/27] Add test that AWX doesn't care about the limit when creating new hosts --- .../tests/functional/api/test_inventory.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/awx/main/tests/functional/api/test_inventory.py b/awx/main/tests/functional/api/test_inventory.py index 23ed3a4f7d..9bac17b4a2 100644 --- a/awx/main/tests/functional/api/test_inventory.py +++ b/awx/main/tests/functional/api/test_inventory.py @@ -326,6 +326,24 @@ def test_create_inventory_host(post, inventory, alice, role_field, expected_stat post(reverse('api:inventory_hosts_list', kwargs={'pk': inventory.id}), data, alice, expect=expected_status_code) +@pytest.mark.parametrize("hosts,expected_status_code", [ + (1, 201), + (2, 201), + (3, 201), +]) +@pytest.mark.django_db +def test_create_inventory_host_with_limits(post, admin_user, inventory, hosts, expected_status_code): + # The per-Organization host limits functionality should be a no-op on AWX. + inventory.organization.max_hosts = 2 + inventory.organization.save() + for i in range(hosts): + inventory.hosts.create(name="Existing host %i" % i) + + data = {'name': 'New name', 'description': 'Hello world'} + post(reverse('api:inventory_hosts_list', kwargs={'pk': inventory.id}), + data, admin_user, expect=expected_status_code) + + @pytest.mark.parametrize("role_field,expected_status_code", [ (None, 403), ('admin_role', 201), From 60008dbd745061c647435f425b711c294e4fb77c Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Tue, 12 Feb 2019 15:58:45 -0500 Subject: [PATCH 13/27] Add a test that AWX doesn't care about max_hosts when editing hosts on an inventory. --- awx/main/tests/functional/api/test_inventory.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/awx/main/tests/functional/api/test_inventory.py b/awx/main/tests/functional/api/test_inventory.py index 9bac17b4a2..d261f3e146 100644 --- a/awx/main/tests/functional/api/test_inventory.py +++ b/awx/main/tests/functional/api/test_inventory.py @@ -374,6 +374,18 @@ def test_edit_inventory_host(put, host, alice, role_field, expected_status_code) put(reverse('api:host_detail', kwargs={'pk': host.id}), data, alice, expect=expected_status_code) +@pytest.mark.django_db +def test_edit_inventory_host_with_limits(put, host, admin_user): + # The per-Organization host limits functionality should be a no-op on AWX. + inventory = host.inventory + inventory.organization.max_hosts = 1 + inventory.organization.save() + inventory.hosts.create(name='Alternate host') + + data = {'name': 'New name', 'description': 'Hello world'} + put(reverse('api:host_detail', kwargs={'pk': host.id}), data, admin_user, expect=200) + + @pytest.mark.parametrize("role_field,expected_status_code", [ (None, 403), ('admin_role', 204), From 5e34f6582b3c62ef381f6d7fc364601e0a3b77bd Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Tue, 12 Feb 2019 17:23:47 -0500 Subject: [PATCH 14/27] Tests for start permissions for JobTemplate and WorkflowJob when max_hosts is set. --- awx/main/tests/functional/test_rbac_job_start.py | 13 +++++++++++++ awx/main/tests/functional/test_rbac_workflow.py | 10 ++++++++++ 2 files changed, 23 insertions(+) diff --git a/awx/main/tests/functional/test_rbac_job_start.py b/awx/main/tests/functional/test_rbac_job_start.py index 6748b3df5d..d205d992d3 100644 --- a/awx/main/tests/functional/test_rbac_job_start.py +++ b/awx/main/tests/functional/test_rbac_job_start.py @@ -21,6 +21,19 @@ def test_admin_executing_permissions(deploy_jobtemplate, inventory, machine_cred assert admin_user.can_access(Credential, 'use', machine_credential) +@pytest.mark.django_db +@pytest.mark.job_permissions +def test_admin_executing_permissions_with_limits(deploy_jobtemplate, inventory, user): + admin_user = user('admin-user', True) + + inventory.organization.max_hosts = 1 + inventory.organization.save() + inventory.hosts.create(name="Existing host 1") + inventory.hosts.create(name="Existing host 2") + + assert admin_user.can_access(JobTemplate, 'start', deploy_jobtemplate) + + @pytest.mark.django_db @pytest.mark.job_permissions def test_job_template_start_access(deploy_jobtemplate, user): diff --git a/awx/main/tests/functional/test_rbac_workflow.py b/awx/main/tests/functional/test_rbac_workflow.py index 7a2fede786..04926385c0 100644 --- a/awx/main/tests/functional/test_rbac_workflow.py +++ b/awx/main/tests/functional/test_rbac_workflow.py @@ -140,6 +140,16 @@ class TestWorkflowJobAccess: JobLaunchConfig.objects.create(job=workflow_job) assert WorkflowJobAccess(rando).can_start(workflow_job) + def test_can_start_with_limits(self, workflow_job, inventory, admin_user): + inventory.organization.max_hosts = 1 + inventory.organization.save() + inventory.hosts.create(name="Existing host 1") + inventory.hosts.create(name="Existing host 2") + workflow_job.inventory = inventory + workflow_job.save() + + assert WorkflowJobAccess(admin_user).can_start(workflow_job) + def test_cannot_relaunch_friends_job(self, wfjt, rando, alice): workflow_job = wfjt.workflow_jobs.create(name='foo', created_by=alice) JobLaunchConfig.objects.create( From cf75ea91a1f6bfda7098d37ebe484bbdf7f1de27 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 13 Feb 2019 10:29:26 -0500 Subject: [PATCH 15/27] Properly use the inventory in the can_start permissions checks --- awx/main/access.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index e688387753..5600abb9a8 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -1391,7 +1391,7 @@ class JobTemplateAccess(BaseAccess): self.check_license() # Check the per-org limit - self.check_org_host_limit({'inventory': obj}) + self.check_org_host_limit({'inventory': obj.inventory}) if obj.survey_enabled: self.check_license(feature='surveys') @@ -1552,7 +1552,7 @@ class JobAccess(BaseAccess): self.check_license() # Check the per-org limit - self.check_org_host_limit({'inventory': obj}) + self.check_org_host_limit({'inventory': obj.inventory}) # A super user can relaunch a job if self.user.is_superuser: @@ -1922,7 +1922,7 @@ class WorkflowJobTemplateAccess(BaseAccess): self.check_license() # Check the per-org limit - self.check_org_host_limit({'inventory': obj}) + self.check_org_host_limit({'inventory': obj.inventory}) # if surveys are added to WFJTs, check license here if obj.survey_enabled: @@ -1996,7 +1996,7 @@ class WorkflowJobAccess(BaseAccess): self.check_license() # Check the per-org limit - self.check_org_host_limit({'inventory': obj}) + self.check_org_host_limit({'inventory': obj.inventory}) if self.user.is_superuser: return True From 4d06ae48d3f9af6e486aa63fbcd23b41765dac44 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Fri, 15 Feb 2019 14:28:58 -0500 Subject: [PATCH 16/27] Deal with the (erroneous) case where a job is missing the inventory by bailing out of check_org_host_limit early. Validation catches this situation later on. --- awx/main/access.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/awx/main/access.py b/awx/main/access.py index 5600abb9a8..1c450f98f4 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -336,6 +336,9 @@ class BaseAccess(object): return inventory = get_object_from_data('inventory', Inventory, data) + if inventory is None: # In this case a missing inventory error is launched + return # further down the line, so just ignore it. + org = inventory.organization if org is None or org.max_hosts == 0: return From 3312ebcb05c015b0663f5648c5e56743e6d8d255 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Mon, 18 Feb 2019 16:35:22 -0500 Subject: [PATCH 17/27] Remove the hosts count from related_field_counts in the org api endpoints It is probably not needed, and adds an additional db query. --- awx/api/serializers.py | 2 +- awx/api/views/mixin.py | 8 ++---- awx/api/views/organization.py | 2 -- .../api/test_organization_counts.py | 27 ++++++------------- 4 files changed, 11 insertions(+), 28 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 8dbb1e273e..299a34e554 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1259,7 +1259,7 @@ class OrganizationSerializer(BaseSerializer): if counts_dict is not None and summary_dict is not None: if obj.id not in counts_dict: summary_dict['related_field_counts'] = { - 'inventories': 0, 'teams': 0, 'users': 0, 'hosts': 0, + 'inventories': 0, 'teams': 0, 'users': 0, 'job_templates': 0, 'admins': 0, 'projects': 0} else: summary_dict['related_field_counts'] = counts_dict[obj.id] diff --git a/awx/api/views/mixin.py b/awx/api/views/mixin.py index 191327c64f..ee174d5091 100644 --- a/awx/api/views/mixin.py +++ b/awx/api/views/mixin.py @@ -29,7 +29,7 @@ from awx.main.models.ha import ( ) from awx.main.models.organization import Team from awx.main.models.projects import Project -from awx.main.models.inventory import Inventory, Host +from awx.main.models.inventory import Inventory from awx.main.models.jobs import JobTemplate from awx.conf.license import ( feature_enabled, @@ -235,8 +235,6 @@ class OrganizationCountsMixin(object): db_results['projects'] = project_qs\ .values('organization').annotate(Count('organization')).order_by('organization') - db_results['hosts'] = Host.objects.active_counts_by_org() - # Other members and admins of organization are always viewable db_results['users'] = org_qs.annotate( users=Count('member_role__members', distinct=True), @@ -248,7 +246,7 @@ class OrganizationCountsMixin(object): org_id = org['id'] count_context[org_id] = { 'inventories': 0, 'teams': 0, 'users': 0, 'job_templates': 0, - 'admins': 0, 'projects': 0, 'hosts': 0} + 'admins': 0, 'projects': 0} for res, count_qs in db_results.items(): if res == 'job_templates_project': @@ -257,8 +255,6 @@ class OrganizationCountsMixin(object): org_reference = JT_inventory_reference elif res == 'users': org_reference = 'id' - elif res == 'hosts': - org_reference = 'inventory__organization' else: org_reference = 'organization' for entry in count_qs: diff --git a/awx/api/views/organization.py b/awx/api/views/organization.py index 0a6e40ba53..3c46ad3cca 100644 --- a/awx/api/views/organization.py +++ b/awx/api/views/organization.py @@ -17,7 +17,6 @@ from awx.conf.license import ( from awx.main.models import ( ActivityStream, Inventory, - Host, Project, JobTemplate, WorkflowJobTemplate, @@ -120,7 +119,6 @@ class OrganizationDetail(RelatedJobsPreventDeleteMixin, RetrieveUpdateDestroyAPI organization__id=org_id).count() org_counts['job_templates'] = JobTemplate.accessible_objects(**access_kwargs).filter( project__organization__id=org_id).count() - org_counts['hosts'] = Host.objects.org_active_count(org_id) full_context['related_field_counts'] = {} full_context['related_field_counts'][org_id] = org_counts diff --git a/awx/main/tests/functional/api/test_organization_counts.py b/awx/main/tests/functional/api/test_organization_counts.py index 77552079e1..9c4f536b09 100644 --- a/awx/main/tests/functional/api/test_organization_counts.py +++ b/awx/main/tests/functional/api/test_organization_counts.py @@ -1,5 +1,3 @@ -import itertools - import pytest from awx.api.versioning import reverse @@ -7,7 +5,7 @@ from awx.api.versioning import reverse @pytest.fixture def organization_resource_creator(organization, user): - def rf(users, admins, job_templates, projects, inventories, teams, hosts): + def rf(users, admins, job_templates, projects, inventories, teams): # Associate one resource of every type with the organization for i in range(users): @@ -19,10 +17,7 @@ def organization_resource_creator(organization, user): for i in range(teams): organization.teams.create(name='org-team %s' % i) for i in range(inventories): - organization.inventories.create(name="associated-inv %s" % i) - for i, inventory in zip(range(hosts), itertools.cycle(organization.inventories.all())): - # evenly distribute the hosts over all of the org's inventories - inventory.hosts.create(name="host %s" % i) + inventory = organization.inventories.create(name="associated-inv %s" % i) for i in range(projects): organization.projects.create(name="test-proj %s" % i, description="test-proj-desc") @@ -53,8 +48,7 @@ COUNTS_PRIMES = { 'job_templates': 3, 'projects': 3, 'inventories': 7, - 'teams': 5, - 'hosts': 7, + 'teams': 5 } COUNTS_ZEROS = { 'users': 0, @@ -62,8 +56,7 @@ COUNTS_ZEROS = { 'job_templates': 0, 'projects': 0, 'inventories': 0, - 'teams': 0, - 'hosts': 0, + 'teams': 0 } @@ -91,7 +84,6 @@ def test_org_counts_detail_member(resourced_organization, user, get): response = get(reverse('api:organization_detail', kwargs={'pk': resourced_organization.pk}), member_user) assert response.status_code == 200 - assert response.data['max_hosts'] == 0 counts = response.data['summary_fields']['related_field_counts'] assert counts == { @@ -100,8 +92,7 @@ def test_org_counts_detail_member(resourced_organization, user, get): 'job_templates': 0, 'projects': 0, 'inventories': 0, - 'teams': 0, - 'hosts': 7, + 'teams': 0 } @@ -123,17 +114,16 @@ def test_org_counts_list_member(resourced_organization, user, get): member_user = resourced_organization.member_role.members.get(username='org-member 1') response = get(reverse('api:organization_list'), member_user) assert response.status_code == 200 - assert response.data['results'][0]['max_hosts'] == 0 counts = response.data['results'][0]['summary_fields']['related_field_counts'] + assert counts == { 'users': COUNTS_PRIMES['users'], # Policy is that members can see other users and admins 'admins': COUNTS_PRIMES['admins'], 'job_templates': 0, 'projects': 0, 'inventories': 0, - 'teams': 0, - 'hosts': 7, + 'teams': 0 } @@ -240,6 +230,5 @@ def test_JT_associated_with_project(organizations, project, user, get): 'job_templates': 1, 'projects': 1, 'inventories': 0, - 'teams': 0, - 'hosts': 0, + 'teams': 0 } From 97cc467ae15a4fdb97011bad2ec10c042fa1f5ab Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 20 Feb 2019 10:50:28 -0500 Subject: [PATCH 18/27] Restrict edit permissions on the Organization.max_hosts field to superusers --- awx/api/serializers.py | 14 +++++++++++ .../functional/api/test_organizations.py | 24 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 299a34e554..ca454a0b1e 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1265,6 +1265,20 @@ class OrganizationSerializer(BaseSerializer): summary_dict['related_field_counts'] = counts_dict[obj.id] return summary_dict + def validate(self, attrs): + obj = self.instance + view = self.context['view'] + + obj_limit = getattr(obj, 'max_hosts', None) + api_limit = attrs.get('max_hosts') + + if not view.request.user.is_superuser: + if api_limit is not None and api_limit != obj_limit: + # Only allow superusers to edit the max_hosts field + raise serializers.ValidationError(_('Cannot change max_hosts.')) + + return super(OrganizationSerializer, self).validate(attrs) + class ProjectOptionsSerializer(BaseSerializer): diff --git a/awx/main/tests/functional/api/test_organizations.py b/awx/main/tests/functional/api/test_organizations.py index c1fa99e810..18aeb269ac 100644 --- a/awx/main/tests/functional/api/test_organizations.py +++ b/awx/main/tests/functional/api/test_organizations.py @@ -199,6 +199,30 @@ def test_update_organization(get, put, organization, alice, bob): put(reverse('api:organization_detail', kwargs={'pk': organization.id}), data, user=bob, expect=403) +@pytest.mark.django_db +def test_update_organization_max_hosts(get, put, organization, admin, alice, bob): + # Admin users can get and update max_hosts + data = get(reverse('api:organization_detail', kwargs={'pk': organization.id}), user=admin, expect=200).data + assert organization.max_hosts == 0 + data['max_hosts'] = 3 + put(reverse('api:organization_detail', kwargs={'pk': organization.id}), data, user=admin, expect=200) + organization.refresh_from_db() + assert organization.max_hosts == 3 + + # Organization admins can get the data and can update other fields, but not max_hosts + organization.admin_role.members.add(alice) + data = get(reverse('api:organization_detail', kwargs={'pk': organization.id}), user=alice, expect=200).data + data['max_hosts'] = 5 + put(reverse('api:organization_detail', kwargs={'pk': organization.id}), data, user=alice, expect=400) + organization.refresh_from_db() + assert organization.max_hosts == 3 + + # Ordinary users shouldn't be able to update either. + put(reverse('api:organization_detail', kwargs={'pk': organization.id}), data, user=bob, expect=403) + organization.refresh_from_db() + assert organization.max_hosts == 3 + + @pytest.mark.django_db @mock.patch('awx.main.access.BaseAccess.check_license', lambda *a, **kw: True) def test_delete_organization(delete, organization, admin): From 6ac51b7b134e5093edd82bbfac9091689118d189 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 20 Feb 2019 13:15:51 -0500 Subject: [PATCH 19/27] Update the permission error to include max_hosts and the current host count --- awx/main/access.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index 1c450f98f4..e895986906 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -345,12 +345,16 @@ class BaseAccess(object): active_count = Host.objects.org_active_count(org.id) if active_count > org.max_hosts: - raise PermissionDenied(_("The organization host limit has been exceeded.")) + raise PermissionDenied( + _("Organization host limit of %s has been exceeded, %s hosts active.") % + (org.max_hosts, active_count)) if add_host_name: host_exists = Host.objects.filter(inventory__organization=org.id, name=add_host_name).exists() if not host_exists and active_count == org.max_hosts: - raise PermissionDenied(_("The organization host limit has been exceeded.")) + raise PermissionDenied( + _("Organization host limit of %s has been exceeded, %s hosts active.") % + (org.max_hosts, active_count)) def get_user_capabilities(self, obj, method_list=[], parent_obj=None, capabilities_cache={}): if obj is None: From 7eba55fbded3b91885c2e50b22e5a196acd1b206 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Thu, 21 Feb 2019 11:41:55 -0500 Subject: [PATCH 20/27] Change the wording of the error when adding a host to "Organization host limit of %s would be exceeded...", since the host will probably not actually be made active. --- awx/main/access.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/access.py b/awx/main/access.py index e895986906..18ea0a1516 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -353,7 +353,7 @@ class BaseAccess(object): host_exists = Host.objects.filter(inventory__organization=org.id, name=add_host_name).exists() if not host_exists and active_count == org.max_hosts: raise PermissionDenied( - _("Organization host limit of %s has been exceeded, %s hosts active.") % + _("Organization host limit of %s would be exceeded, %s hosts active.") % (org.max_hosts, active_count)) def get_user_capabilities(self, obj, method_list=[], parent_obj=None, capabilities_cache={}): From 046385d72ebd0050d0cdffa38099dfca6e53e363 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Thu, 21 Feb 2019 15:57:44 -0500 Subject: [PATCH 21/27] Make the inventory_import command log an error message on exception when it happens in the big try/except block in the middle of handle(). Previously it wasn't doing anything with it, except exiting with a code of 1. --- awx/main/management/commands/inventory_import.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/awx/main/management/commands/inventory_import.py b/awx/main/management/commands/inventory_import.py index 0974dcb7fa..af4c2d88d7 100644 --- a/awx/main/management/commands/inventory_import.py +++ b/awx/main/management/commands/inventory_import.py @@ -1104,7 +1104,8 @@ class Command(BaseCommand): self.inventory_source.status = status self.inventory_source.save(update_fields=['status']) - if exc and isinstance(exc, CommandError): - sys.exit(1) - elif exc: + if exc: + logger.error(str(exc)) + if isinstance(exc, CommandError): + sys.exit(1) raise exc From ce5a85a53bceea806c2642320fb00941cffe5bec Mon Sep 17 00:00:00 2001 From: mabashian Date: Thu, 21 Feb 2019 16:42:46 -0500 Subject: [PATCH 22/27] Add support for max hosts on org --- awx/ui/client/features/output/details.component.js | 12 ++++++++++++ awx/ui/client/features/output/details.partial.html | 8 ++++++++ awx/ui/client/features/output/output.strings.js | 1 + .../related/hosts/add/host-add.controller.js | 4 ++-- .../src/inventories-hosts/shared/hosts.service.js | 6 +++--- .../add/organizations-add.controller.js | 12 +++++------- .../edit/organizations-edit.controller.js | 1 - .../client/src/organizations/organizations.form.js | 14 ++++++++++++++ 8 files changed, 45 insertions(+), 13 deletions(-) diff --git a/awx/ui/client/features/output/details.component.js b/awx/ui/client/features/output/details.component.js index 696cfb8adf..13358e9abb 100644 --- a/awx/ui/client/features/output/details.component.js +++ b/awx/ui/client/features/output/details.component.js @@ -236,6 +236,17 @@ function getLicenseErrorDetails () { return { label, value }; } +function getHostLimitErrorDetails () { + if (!resource.model.has('org_host_limit_error')) { + return null; + } + + const label = strings.get('labels.HOST_LIMIT_ERROR'); + const value = resource.model.get('org_host_limit_error'); + + return { label, value }; +} + function getLaunchedByDetails () { const createdBy = resource.model.get('summary_fields.created_by'); const jobTemplate = resource.model.get('summary_fields.job_template'); @@ -804,6 +815,7 @@ function JobDetailsController ( vm.overwrite = getOverwriteDetails(); vm.overwriteVars = getOverwriteVarsDetails(); vm.licenseError = getLicenseErrorDetails(); + vm.hostLimitError = getHostLimitErrorDetails(); // Relaunch and Delete Components vm.job = angular.copy(_.get(resource.model, 'model.GET', {})); diff --git a/awx/ui/client/features/output/details.partial.html b/awx/ui/client/features/output/details.partial.html index 0e7cc3620a..9d5b621864 100644 --- a/awx/ui/client/features/output/details.partial.html +++ b/awx/ui/client/features/output/details.partial.html @@ -81,6 +81,14 @@ + +
+ +
+ {{ vm.hostLimitError.value }} +
+
+
diff --git a/awx/ui/client/features/output/output.strings.js b/awx/ui/client/features/output/output.strings.js index b47eff0ace..7ab92c7d84 100644 --- a/awx/ui/client/features/output/output.strings.js +++ b/awx/ui/client/features/output/output.strings.js @@ -57,6 +57,7 @@ function OutputStrings (BaseString) { EXTRA_VARS: t.s('Extra Variables'), FINISHED: t.s('Finished'), FORKS: t.s('Forks'), + HOST_LIMIT_ERROR: t.s('Host Limit Error'), INSTANCE_GROUP: t.s('Instance Group'), INVENTORY: t.s('Inventory'), INVENTORY_SCM: t.s('Source Project'), diff --git a/awx/ui/client/src/inventories-hosts/inventories/related/hosts/add/host-add.controller.js b/awx/ui/client/src/inventories-hosts/inventories/related/hosts/add/host-add.controller.js index 207bdf4f8f..3513652a98 100644 --- a/awx/ui/client/src/inventories-hosts/inventories/related/hosts/add/host-add.controller.js +++ b/awx/ui/client/src/inventories-hosts/inventories/related/hosts/add/host-add.controller.js @@ -5,9 +5,9 @@ *************************************************/ export default ['$state', '$stateParams', '$scope', 'RelatedHostsFormDefinition', 'ParseTypeChange', - 'GenerateForm', 'HostsService', 'rbacUiControlService', 'GetBasePath', 'ToJSON', 'canAdd', + 'GenerateForm', 'HostsService', 'GetBasePath', 'ToJSON', 'canAdd', function($state, $stateParams, $scope, RelatedHostsFormDefinition, ParseTypeChange, - GenerateForm, HostsService, rbacUiControlService, GetBasePath, ToJSON, canAdd) { + GenerateForm, HostsService, GetBasePath, ToJSON, canAdd) { init(); diff --git a/awx/ui/client/src/inventories-hosts/shared/hosts.service.js b/awx/ui/client/src/inventories-hosts/shared/hosts.service.js index 8624146892..ceddb8ca07 100644 --- a/awx/ui/client/src/inventories-hosts/shared/hosts.service.js +++ b/awx/ui/client/src/inventories-hosts/shared/hosts.service.js @@ -17,9 +17,9 @@ url: function(){ return ''; }, - error: function(data, status) { - ProcessErrors($rootScope, data.data, status, null, { hdr: 'Error!', - msg: 'Call to ' + this.url + '. GET returned: ' + status }); + error: function(data) { + ProcessErrors($rootScope, data.data, data.status, null, { hdr: 'Error!', + msg: 'Call to ' + this.url + '. GET returned: ' + data.status }); }, success: function(data){ return data; diff --git a/awx/ui/client/src/organizations/add/organizations-add.controller.js b/awx/ui/client/src/organizations/add/organizations-add.controller.js index 6acccc4296..f13a9b09c6 100644 --- a/awx/ui/client/src/organizations/add/organizations-add.controller.js +++ b/awx/ui/client/src/organizations/add/organizations-add.controller.js @@ -24,8 +24,6 @@ export default ['$scope', '$rootScope', '$location', '$stateParams', init(); function init(){ - // @issue What is this doing, why - $scope.$emit("HideOrgListHeader"); const virtualEnvs = ConfigData.custom_virtualenvs || []; $scope.custom_virtualenvs_visible = virtualEnvs.length > 1; $scope.custom_virtualenvs_options = virtualEnvs.filter( @@ -43,15 +41,15 @@ export default ['$scope', '$rootScope', '$location', '$stateParams', // Save $scope.formSave = function() { + var fld, params = {}; Wait('start'); + for (fld in form.fields) { + params[fld] = $scope[fld]; + } var url = GetBasePath(base); url += (base !== 'organizations') ? $stateParams.project_id + '/organizations/' : ''; Rest.setUrl(url); - Rest.post({ - name: $scope.name, - description: $scope.description, - custom_virtualenv: $scope.custom_virtualenv - }) + Rest.post(params) .then(({data}) => { const organization_id = data.id, instance_group_url = data.related.instance_groups; diff --git a/awx/ui/client/src/organizations/edit/organizations-edit.controller.js b/awx/ui/client/src/organizations/edit/organizations-edit.controller.js index e58b19aa6d..c6a54eca5a 100644 --- a/awx/ui/client/src/organizations/edit/organizations-edit.controller.js +++ b/awx/ui/client/src/organizations/edit/organizations-edit.controller.js @@ -38,7 +38,6 @@ export default ['$scope', '$location', '$stateParams', 'OrgAdminLookup', } }); - $scope.$emit("HideOrgListHeader"); $scope.instance_groups = InstanceGroupsData; const virtualEnvs = ConfigData.custom_virtualenvs || []; $scope.custom_virtualenvs_visible = virtualEnvs.length > 1; diff --git a/awx/ui/client/src/organizations/organizations.form.js b/awx/ui/client/src/organizations/organizations.form.js index 56cfbebeae..a4a22deee4 100644 --- a/awx/ui/client/src/organizations/organizations.form.js +++ b/awx/ui/client/src/organizations/organizations.form.js @@ -54,6 +54,20 @@ export default ['NotificationsList', 'i18n', dataPlacement: 'right', ngDisabled: '!(organization_obj.summary_fields.user_capabilities.edit || canAdd)', ngShow: 'custom_virtualenvs_visible' + }, + max_hosts: { + label: i18n._('Max Hosts'), + type: 'number', + integer: true, + min: 0, + default: 0, + spinner: true, + dataTitle: i18n._('Max Hosts'), + dataPlacement: 'right', + dataContainer: 'body', + awPopOver: "

" + i18n._("The maximum number of hosts allowed to be managed by this organization. Value defaults to 0 which means no limit. Refer to the Ansible documentation for more details.") + "

", + ngDisabled: '!current_user.is_superuser', + ngShow: 'BRAND_NAME === "Tower"' } }, From 98c5cb1c4c9f6a56caf52cc422813d45e88be441 Mon Sep 17 00:00:00 2001 From: mabashian Date: Tue, 26 Feb 2019 13:15:29 -0500 Subject: [PATCH 23/27] Adds max value to host limit input --- awx/ui/client/src/organizations/organizations.form.js | 1 + 1 file changed, 1 insertion(+) diff --git a/awx/ui/client/src/organizations/organizations.form.js b/awx/ui/client/src/organizations/organizations.form.js index a4a22deee4..a00770f7ba 100644 --- a/awx/ui/client/src/organizations/organizations.form.js +++ b/awx/ui/client/src/organizations/organizations.form.js @@ -60,6 +60,7 @@ export default ['NotificationsList', 'i18n', type: 'number', integer: true, min: 0, + max: 2147483647, default: 0, spinner: true, dataTitle: i18n._('Max Hosts'), From 1b94b616f0f34dfd173714318c43267e4697e0df Mon Sep 17 00:00:00 2001 From: mabashian Date: Tue, 26 Feb 2019 15:58:28 -0500 Subject: [PATCH 24/27] Default empty max hosts to 0 --- .../src/organizations/add/organizations-add.controller.js | 5 ++++- .../src/organizations/edit/organizations-edit.controller.js | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/awx/ui/client/src/organizations/add/organizations-add.controller.js b/awx/ui/client/src/organizations/add/organizations-add.controller.js index f13a9b09c6..cd6aea6cb9 100644 --- a/awx/ui/client/src/organizations/add/organizations-add.controller.js +++ b/awx/ui/client/src/organizations/add/organizations-add.controller.js @@ -46,6 +46,9 @@ export default ['$scope', '$rootScope', '$location', '$stateParams', for (fld in form.fields) { params[fld] = $scope[fld]; } + if (!params.max_hosts || params.max_hosts === '') { + params.max_hosts = 0; + } var url = GetBasePath(base); url += (base !== 'organizations') ? $stateParams.project_id + '/organizations/' : ''; Rest.setUrl(url); @@ -71,7 +74,7 @@ export default ['$scope', '$rootScope', '$location', '$stateParams', let explanation = _.has(data, "name") ? data.name[0] : ""; ProcessErrors($scope, data, status, OrganizationForm, { hdr: 'Error!', - msg: `Failed to save organization. PUT status: ${status}. ${explanation}` + msg: `Failed to save organization. POST status: ${status}. ${explanation}` }); }); }; diff --git a/awx/ui/client/src/organizations/edit/organizations-edit.controller.js b/awx/ui/client/src/organizations/edit/organizations-edit.controller.js index c6a54eca5a..a847068d49 100644 --- a/awx/ui/client/src/organizations/edit/organizations-edit.controller.js +++ b/awx/ui/client/src/organizations/edit/organizations-edit.controller.js @@ -56,7 +56,7 @@ export default ['$scope', '$location', '$stateParams', 'OrgAdminLookup', $scope.organization_name = data.name; for (fld in form.fields) { - if (data[fld]) { + if (typeof data[fld] !== 'undefined') { $scope[fld] = data[fld]; master[fld] = data[fld]; } @@ -97,6 +97,9 @@ export default ['$scope', '$location', '$stateParams', 'OrgAdminLookup', for (fld in form.fields) { params[fld] = $scope[fld]; } + if (!params.max_hosts || params.max_hosts === '') { + params.max_hosts = 0; + } Rest.setUrl(defaultUrl + id + '/'); Rest.put(params) .then(() => { From 03c07c0843db8732d6905e4f9bb0fd15f1c2be87 Mon Sep 17 00:00:00 2001 From: mabashian Date: Wed, 27 Feb 2019 10:07:47 -0500 Subject: [PATCH 25/27] Adds error handling to launch api calls --- .../launchTemplateButton.component.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/awx/ui/client/lib/components/launchTemplateButton/launchTemplateButton.component.js b/awx/ui/client/lib/components/launchTemplateButton/launchTemplateButton.component.js index fe0dfa0f9a..60a56df3fd 100644 --- a/awx/ui/client/lib/components/launchTemplateButton/launchTemplateButton.component.js +++ b/awx/ui/client/lib/components/launchTemplateButton/launchTemplateButton.component.js @@ -56,7 +56,8 @@ function atLaunchTemplateCtrl ( } else { $state.go('output', { id: data.job, type: 'playbook' }, { reload: true }); } - }); + }) + .catch(createErrorHandler('launch job template', 'POST')); } else { const promptData = { launchConf: launchData.data, @@ -78,12 +79,14 @@ function atLaunchTemplateCtrl ( }); promptData.surveyQuestions = processed.surveyQuestions; vm.promptData = promptData; - }); + }) + .catch(createErrorHandler('get survey questions', 'GET')); } else { vm.promptData = promptData; } } - }); + }) + .catch(createErrorHandler('get launch options', 'GET')); } else if (vm.template.type === 'workflow_job_template') { const selectedWorkflowJobTemplate = workflowTemplate.create(); const preLaunchPromises = [ @@ -99,7 +102,8 @@ function atLaunchTemplateCtrl ( .postLaunch({ id: vm.template.id }) .then(({ data }) => { $state.go('workflowResults', { id: data.workflow_job }, { reload: true }); - }); + }) + .catch(createErrorHandler('launch workflow job template', 'POST')); } else { launchData.data.defaults.extra_vars = wfjtData.data.extra_vars; @@ -128,7 +132,8 @@ function atLaunchTemplateCtrl ( vm.promptData = promptData; } } - }); + }) + .catch(createErrorHandler('get launch options', 'GET')); } else { Alert(templatesStrings.get('error.UNKNOWN'), templatesStrings.get('alert.UNKNOWN_LAUNCH')); } From cc3f2e0819264bcdd6f690d2414c276fa5aa6bca Mon Sep 17 00:00:00 2001 From: mabashian Date: Wed, 27 Feb 2019 12:46:18 -0500 Subject: [PATCH 26/27] Properly pass path to error message --- .../launchTemplateButton.component.js | 166 ++++++++++-------- 1 file changed, 89 insertions(+), 77 deletions(-) diff --git a/awx/ui/client/lib/components/launchTemplateButton/launchTemplateButton.component.js b/awx/ui/client/lib/components/launchTemplateButton/launchTemplateButton.component.js index 60a56df3fd..80cfc85367 100644 --- a/awx/ui/client/lib/components/launchTemplateButton/launchTemplateButton.component.js +++ b/awx/ui/client/lib/components/launchTemplateButton/launchTemplateButton.component.js @@ -39,101 +39,113 @@ function atLaunchTemplateCtrl ( if (vm.template.type === 'job_template') { const selectedJobTemplate = jobTemplate.create(); const preLaunchPromises = [ - selectedJobTemplate.getLaunch(vm.template.id), - selectedJobTemplate.optionsLaunch(vm.template.id), + selectedJobTemplate.getLaunch(vm.template.id) + .catch(createErrorHandler(`/api/v2/job_templates/${vm.template.id}/launch`, 'GET')), + selectedJobTemplate.optionsLaunch(vm.template.id) + .catch(createErrorHandler(`/api/v2/job_templates/${vm.template.id}/launch`, 'OPTIONS')) ]; Promise.all(preLaunchPromises) .then(([launchData, launchOptions]) => { - if (selectedJobTemplate.canLaunchWithoutPrompt()) { - selectedJobTemplate - .postLaunch({ id: vm.template.id }) - .then(({ data }) => { - /* Slice Jobs: Redirect to WF Details page if returned - job type is a WF job */ - if (data.type === 'workflow_job' && data.workflow_job !== null) { - $state.go('workflowResults', { id: data.workflow_job }, { reload: true }); - } else { - $state.go('output', { id: data.job, type: 'playbook' }, { reload: true }); - } - }) - .catch(createErrorHandler('launch job template', 'POST')); - } else { - const promptData = { - launchConf: launchData.data, - launchOptions: launchOptions.data, - template: vm.template.id, - templateType: vm.template.type, - prompts: PromptService.processPromptValues({ - launchConf: launchData.data, - launchOptions: launchOptions.data - }), - triggerModalOpen: true - }; - - if (launchData.data.survey_enabled) { - selectedJobTemplate.getSurveyQuestions(vm.template.id) + // If we don't get both of these things then one of the + // promises was rejected + if (launchData && launchOptions) { + if (selectedJobTemplate.canLaunchWithoutPrompt()) { + selectedJobTemplate + .postLaunch({ id: vm.template.id }) .then(({ data }) => { - const processed = PromptService.processSurveyQuestions({ - surveyQuestions: data.spec - }); - promptData.surveyQuestions = processed.surveyQuestions; - vm.promptData = promptData; + /* Slice Jobs: Redirect to WF Details page if returned + job type is a WF job */ + if (data.type === 'workflow_job' && data.workflow_job !== null) { + $state.go('workflowResults', { id: data.workflow_job }, { reload: true }); + } else { + $state.go('output', { id: data.job, type: 'playbook' }, { reload: true }); + } }) - .catch(createErrorHandler('get survey questions', 'GET')); + .catch(createErrorHandler(`/api/v2/job_templates/${vm.template.id}/launch`, 'POST')); } else { - vm.promptData = promptData; + const promptData = { + launchConf: launchData.data, + launchOptions: launchOptions.data, + template: vm.template.id, + templateType: vm.template.type, + prompts: PromptService.processPromptValues({ + launchConf: launchData.data, + launchOptions: launchOptions.data + }), + triggerModalOpen: true + }; + + if (launchData.data.survey_enabled) { + selectedJobTemplate.getSurveyQuestions(vm.template.id) + .then(({ data }) => { + const processed = PromptService.processSurveyQuestions({ + surveyQuestions: data.spec + }); + promptData.surveyQuestions = processed.surveyQuestions; + vm.promptData = promptData; + }) + .catch(createErrorHandler(`/api/v2/job_templates/${vm.template.id}/survey_spec`, 'GET')); + } else { + vm.promptData = promptData; + } } } - }) - .catch(createErrorHandler('get launch options', 'GET')); + }); } else if (vm.template.type === 'workflow_job_template') { const selectedWorkflowJobTemplate = workflowTemplate.create(); const preLaunchPromises = [ - selectedWorkflowJobTemplate.request('get', vm.template.id), - selectedWorkflowJobTemplate.getLaunch(vm.template.id), - selectedWorkflowJobTemplate.optionsLaunch(vm.template.id), + selectedWorkflowJobTemplate.request('get', vm.template.id) + .catch(createErrorHandler(`/api/v2/workflow_job_templates/${vm.template.id}`, 'GET')), + selectedWorkflowJobTemplate.getLaunch(vm.template.id) + .catch(createErrorHandler(`/api/v2/workflow_job_templates/${vm.template.id}/launch`, 'GET')), + selectedWorkflowJobTemplate.optionsLaunch(vm.template.id) + .catch(createErrorHandler(`/api/v2/workflow_job_templates/${vm.template.id}/launch`, 'OPTIONS')), ]; Promise.all(preLaunchPromises) .then(([wfjtData, launchData, launchOptions]) => { - if (selectedWorkflowJobTemplate.canLaunchWithoutPrompt()) { - selectedWorkflowJobTemplate - .postLaunch({ id: vm.template.id }) - .then(({ data }) => { - $state.go('workflowResults', { id: data.workflow_job }, { reload: true }); - }) - .catch(createErrorHandler('launch workflow job template', 'POST')); - } else { - launchData.data.defaults.extra_vars = wfjtData.data.extra_vars; - - const promptData = { - launchConf: selectedWorkflowJobTemplate.getLaunchConf(), - launchOptions: launchOptions.data, - template: vm.template.id, - templateType: vm.template.type, - prompts: PromptService.processPromptValues({ - launchConf: selectedWorkflowJobTemplate.getLaunchConf(), - launchOptions: launchOptions.data - }), - triggerModalOpen: true, - }; - - if (launchData.data.survey_enabled) { - selectedWorkflowJobTemplate.getSurveyQuestions(vm.template.id) + // If we don't get all of these things then one of the + // promises was rejected + if (wfjtData && launchData && launchOptions) { + if (selectedWorkflowJobTemplate.canLaunchWithoutPrompt()) { + selectedWorkflowJobTemplate + .postLaunch({ id: vm.template.id }) .then(({ data }) => { - const processed = PromptService.processSurveyQuestions({ - surveyQuestions: data.spec - }); - promptData.surveyQuestions = processed.surveyQuestions; - vm.promptData = promptData; - }); + $state.go('workflowResults', { id: data.workflow_job }, { reload: true }); + }) + .catch(createErrorHandler(`/api/v2/workflow_job_templates/${vm.template.id}/launch`, 'POST')); } else { - vm.promptData = promptData; + launchData.data.defaults.extra_vars = wfjtData.data.extra_vars; + + const promptData = { + launchConf: selectedWorkflowJobTemplate.getLaunchConf(), + launchOptions: launchOptions.data, + template: vm.template.id, + templateType: vm.template.type, + prompts: PromptService.processPromptValues({ + launchConf: selectedWorkflowJobTemplate.getLaunchConf(), + launchOptions: launchOptions.data + }), + triggerModalOpen: true, + }; + + if (launchData.data.survey_enabled) { + selectedWorkflowJobTemplate.getSurveyQuestions(vm.template.id) + .then(({ data }) => { + const processed = PromptService.processSurveyQuestions({ + surveyQuestions: data.spec + }); + promptData.surveyQuestions = processed.surveyQuestions; + vm.promptData = promptData; + }) + .catch(createErrorHandler(`/api/v2/workflow_job_templates/${vm.template.id}/survey_spec`, 'GET')); + } else { + vm.promptData = promptData; + } } } - }) - .catch(createErrorHandler('get launch options', 'GET')); + }); } else { Alert(templatesStrings.get('error.UNKNOWN'), templatesStrings.get('alert.UNKNOWN_LAUNCH')); } @@ -168,14 +180,14 @@ function atLaunchTemplateCtrl ( } else { $state.go('output', { id: launchRes.data.job, type: 'playbook' }, { reload: true }); } - }).catch(createErrorHandler('launch job template', 'POST')); + }).catch(createErrorHandler(`/api/v2/job_templates/${vm.template.id}/launch`, 'POST')); } else if (vm.promptData.templateType === 'workflow_job_template') { workflowTemplate.create().postLaunch({ id: vm.promptData.template, launchData: jobLaunchData }).then((launchRes) => { $state.go('workflowResults', { id: launchRes.data.workflow_job }, { reload: true }); - }).catch(createErrorHandler('launch workflow job template', 'POST')); + }).catch(createErrorHandler(`/api/v2/workflow_job_templates/${vm.template.id}/launch`, 'POST')); } }; } From a82304765d2fdd92553ba023a813afc2afe4420a Mon Sep 17 00:00:00 2001 From: mabashian Date: Wed, 27 Feb 2019 14:55:10 -0500 Subject: [PATCH 27/27] Adds help text to host limit error shown in the output details of inventory sync jobs --- awx/ui/client/features/output/details.component.js | 3 ++- awx/ui/client/features/output/details.partial.html | 14 +++++++++++++- awx/ui/client/features/output/output.strings.js | 1 + 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/awx/ui/client/features/output/details.component.js b/awx/ui/client/features/output/details.component.js index 13358e9abb..67f3bee4a0 100644 --- a/awx/ui/client/features/output/details.component.js +++ b/awx/ui/client/features/output/details.component.js @@ -242,9 +242,10 @@ function getHostLimitErrorDetails () { } const label = strings.get('labels.HOST_LIMIT_ERROR'); + const tooltip = strings.get('tooltips.HOST_LIMIT'); const value = resource.model.get('org_host_limit_error'); - return { label, value }; + return { tooltip, label, value }; } function getLaunchedByDetails () { diff --git a/awx/ui/client/features/output/details.partial.html b/awx/ui/client/features/output/details.partial.html index 9d5b621864..b2faddea97 100644 --- a/awx/ui/client/features/output/details.partial.html +++ b/awx/ui/client/features/output/details.partial.html @@ -83,7 +83,19 @@
- +
{{ vm.hostLimitError.value }}
diff --git a/awx/ui/client/features/output/output.strings.js b/awx/ui/client/features/output/output.strings.js index 7ab92c7d84..982070837e 100644 --- a/awx/ui/client/features/output/output.strings.js +++ b/awx/ui/client/features/output/output.strings.js @@ -22,6 +22,7 @@ function OutputStrings (BaseString) { CREDENTIAL: t.s('View the Credential'), EXPAND_OUTPUT: t.s('Expand Output'), EXTRA_VARS: t.s('Read-only view of extra variables added to the job template'), + HOST_LIMIT: t.s('When this field is true, the job\'s inventory belongs to an organization that has exceeded it\'s limit of hosts as defined by the system administrator.'), INVENTORY: t.s('View the Inventory'), INVENTORY_SCM: t.s('View the Project'), INVENTORY_SCM_JOB: t.s('View Project checkout results'),