diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index 38c8247b07..70b4dcaa9f 100644 --- a/awx/main/models/inventory.py +++ b/awx/main/models/inventory.py @@ -927,11 +927,12 @@ class InventorySourceOptions(BaseModel): source_vars_dict = VarsDictProperty('source_vars') def clean_instance_filters(self): + instance_filters = unicode(self.instance_filters or '') if self.source != 'ec2': return '' invalid_filters = [] instance_filter_re = re.compile(r'^(?:tag:.+)|(?:[a-z][a-z\.-]*[a-z])=.*$') - for instance_filter in self.instance_filters.split(','): + for instance_filter in instance_filters.split(','): instance_filter = instance_filter.strip() if not instance_filter: continue @@ -941,16 +942,17 @@ class InventorySourceOptions(BaseModel): raise ValidationError('Invalid filter expression%s: %s' % ('' if len(invalid_filters) == 1 else 's', ', '.join(invalid_filters))) - return self.instance_filters + return instance_filters def clean_group_by(self): + group_by = unicode(self.group_by or '') if self.source != 'ec2': return '' get_choices = getattr(self, 'get_%s_group_by_choices' % self.source) valid_choices = [x[0] for x in get_choices()] choice_transform = lambda x: x.strip().lower() valid_choices = [choice_transform(x) for x in valid_choices] - choices = [choice_transform(x) for x in self.group_by.split(',') if x.strip()] + choices = [choice_transform(x) for x in group_by.split(',') if x.strip()] invalid_choices = [] for c in choices: if c not in valid_choices and c not in invalid_choices: diff --git a/awx/main/tests/inventory.py b/awx/main/tests/inventory.py index 1d9cb517e1..036246e0bd 100644 --- a/awx/main/tests/inventory.py +++ b/awx/main/tests/inventory.py @@ -1256,10 +1256,36 @@ class InventoryUpdatesTest(BaseTransactionTest): 'source': 'ec2', 'credential': aws_cred_id, 'source_regions': '', + 'instance_filters': '', + 'group_by': '', } with self.current_user(self.super_django_user): response = self.put(inv_src_url1, inv_src_data, expect=200) self.assertEqual(response['source_regions'], '') + # Null for instance filters and group_by should be converted to empty + # string. + inv_src_data['instance_filters'] = None + inv_src_data['group_by'] = None + with self.current_user(self.super_django_user): + response = self.put(inv_src_url1, inv_src_data, expect=200) + self.assertEqual(response['instance_filters'], '') + self.assertEqual(response['group_by'], '') + # Invalid string for instance filters. + inv_src_data['instance_filters'] = 'tag-key_123=Name,' + with self.current_user(self.super_django_user): + response = self.put(inv_src_url1, inv_src_data, expect=400) + # Valid string for instance filters. + inv_src_data['instance_filters'] = 'tag-key=Name' + with self.current_user(self.super_django_user): + response = self.put(inv_src_url1, inv_src_data, expect=200) + # Invalid string for group_by. + inv_src_data['group_by'] = 'ec2_region,' + with self.current_user(self.super_django_user): + response = self.put(inv_src_url1, inv_src_data, expect=400) + # Valid string for group_by. + inv_src_data['group_by'] = 'region,key_pair,instance_type' + with self.current_user(self.super_django_user): + response = self.put(inv_src_url1, inv_src_data, expect=200) # All region. inv_src_data['source_regions'] = 'ALL' with self.current_user(self.super_django_user): @@ -1293,6 +1319,8 @@ class InventoryUpdatesTest(BaseTransactionTest): 'source': 'rax', 'credential': rax_cred_id, 'source_regions': '', + 'instance_filters': None, + 'group_by': None, } with self.current_user(self.super_django_user): response = self.put(inv_src_url2, inv_src_data, expect=200)