diff --git a/horizon/dashboards/nova/access_and_security/security_groups/forms.py b/horizon/dashboards/nova/access_and_security/security_groups/forms.py index 0a0127c7fd9..2836aee805f 100644 --- a/horizon/dashboards/nova/access_and_security/security_groups/forms.py +++ b/horizon/dashboards/nova/access_and_security/security_groups/forms.py @@ -40,7 +40,6 @@ class CreateGroup(forms.SelfHandlingForm): name = forms.CharField(validators=[validators.validate_slug]) description = forms.CharField() - tenant_id = forms.CharField(widget=forms.HiddenInput()) def handle(self, request, data): try: @@ -56,13 +55,13 @@ def handle(self, request, data): class AddRule(forms.SelfHandlingForm): - ip_protocol = forms.ChoiceField(label=_('IP protocol'), - choices=[('tcp', 'tcp'), - ('udp', 'udp'), - ('icmp', 'icmp')], + ip_protocol = forms.ChoiceField(label=_('IP Protocol'), + choices=[('tcp', 'TCP'), + ('udp', 'UDP'), + ('icmp', 'ICMP')], widget=forms.Select(attrs={'class': 'switchable'})) - from_port = forms.IntegerField(label=_("From port"), + from_port = forms.IntegerField(label=_("From Port"), help_text=_("TCP/UDP: Enter integer value " "between 1 and 65535. ICMP: " "enter a value for ICMP type " @@ -71,7 +70,7 @@ class AddRule(forms.SelfHandlingForm): attrs={'data': _('From port'), 'data-icmp': _('Type')}), validators=[validate_port_range]) - to_port = forms.IntegerField(label=_("To port"), + to_port = forms.IntegerField(label=_("To Port"), help_text=_("TCP/UDP: Enter integer value " "between 1 and 65535. ICMP: " "enter a value for ICMP code " @@ -82,13 +81,14 @@ class AddRule(forms.SelfHandlingForm): validators=[validate_port_range]) source_group = forms.ChoiceField(label=_('Source Group'), required=False) - cidr = forms.CharField(label=_("CIDR"), required=False, + cidr = forms.CharField(label=_("CIDR"), + required=False, + initial="0.0.0.0/0", help_text=_("Classless Inter-Domain Routing " "(i.e. 192.168.0.0/24"), validators=[validate_ipv4_cidr]) security_group_id = forms.IntegerField(widget=forms.HiddenInput()) - tenant_id = forms.CharField(widget=forms.HiddenInput()) def __init__(self, *args, **kwargs): super(AddRule, self).__init__(*args, **kwargs) @@ -120,11 +120,16 @@ def clean(self): 'the "from" port number.') raise ValidationError(msg) - if cidr and source_group: - msg = _('Only either "CIDR" or "Source Group" may be specified') + if source_group and cidr != self.fields['cidr'].initial: + # Specifying a source group *and* a custom CIDR is invalid. + msg = _('Either CIDR or Source Group may be specified, ' + 'but not both.') raise ValidationError(msg) - if cidr: - # if only cidr is specified, make sure source_group is cleaned + elif source_group: + # If a source group is specified, clear the CIDR from its default + cleaned_data['cidr'] = None + else: + # If only cidr is specified, clear the source_group entirely cleaned_data['source_group'] = None return cleaned_data diff --git a/horizon/dashboards/nova/access_and_security/security_groups/tests.py b/horizon/dashboards/nova/access_and_security/security_groups/tests.py index 94306bcde27..a9da39542af 100644 --- a/horizon/dashboards/nova/access_and_security/security_groups/tests.py +++ b/horizon/dashboards/nova/access_and_security/security_groups/tests.py @@ -59,7 +59,6 @@ def test_create_security_groups_post(self): self.mox.ReplayAll() formData = {'method': 'CreateGroup', - 'tenant_id': self.tenant.id, 'name': sec_group.name, 'description': sec_group.description} res = self.client.post(SG_CREATE_URL, formData) @@ -75,7 +74,6 @@ def test_create_security_groups_post_exception(self): self.mox.ReplayAll() formData = {'method': 'CreateGroup', - 'tenant_id': self.tenant.id, 'name': sec_group.name, 'description': sec_group.description} res = self.client.post(SG_CREATE_URL, formData) @@ -137,7 +135,6 @@ def test_edit_rules_add_rule_cidr(self): self.mox.ReplayAll() formData = {'method': 'AddRule', - 'tenant_id': self.tenant.id, 'security_group_id': sec_group.id, 'from_port': rule.from_port, 'to_port': rule.to_port, @@ -147,6 +144,32 @@ def test_edit_rules_add_rule_cidr(self): res = self.client.post(self.edit_url, formData) self.assertRedirectsNoFollow(res, INDEX_URL) + def test_edit_rules_add_rule_cidr_and_source_group(self): + sec_group = self.security_groups.first() + sec_group_other = self.security_groups.get(id=2) + sec_group_list = self.security_groups.list() + rule = self.security_group_rules.first() + + self.mox.StubOutWithMock(api, 'security_group_get') + self.mox.StubOutWithMock(api, 'security_group_list') + api.security_group_get(IsA(http.HttpRequest), + sec_group.id).AndReturn(sec_group) + api.security_group_list( + IsA(http.HttpRequest)).AndReturn(sec_group_list) + self.mox.ReplayAll() + + formData = {'method': 'AddRule', + 'security_group_id': sec_group.id, + 'from_port': rule.from_port, + 'to_port': rule.to_port, + 'ip_protocol': rule.ip_protocol, + 'cidr': "127.0.0.1/32", + 'source_group': sec_group_other.id} + res = self.client.post(self.edit_url, formData) + self.assertNoMessages() + msg = 'Either CIDR or Source Group may be specified, but not both.' + self.assertFormErrors(res, count=1, message=msg) + def test_edit_rules_invalid_port_range(self): sec_group = self.security_groups.first() sec_group_list = self.security_groups.list() @@ -161,7 +184,6 @@ def test_edit_rules_invalid_port_range(self): self.mox.ReplayAll() formData = {'method': 'AddRule', - 'tenant_id': self.tenant.id, 'security_group_id': sec_group.id, 'from_port': rule.from_port, 'to_port': int(rule.from_port) - 1, @@ -192,7 +214,6 @@ def test_edit_rules_add_rule_exception(self): self.mox.ReplayAll() formData = {'method': 'AddRule', - 'tenant_id': self.tenant.id, 'security_group_id': sec_group.id, 'from_port': rule.from_port, 'to_port': rule.to_port, diff --git a/horizon/dashboards/nova/access_and_security/security_groups/views.py b/horizon/dashboards/nova/access_and_security/security_groups/views.py index acbcc95f0db..9d38f9a4b94 100644 --- a/horizon/dashboards/nova/access_and_security/security_groups/views.py +++ b/horizon/dashboards/nova/access_and_security/security_groups/views.py @@ -56,12 +56,16 @@ def get_data(self): return rules def handle_form(self): - tenant_id = self.request.user.tenant_id - security_groups = [(group.id, group.name) - for group in api.security_group_list(self.request)] + try: + groups = api.security_group_list(self.request) + except: + groups = [] + exceptions.handle(self.request, + _("Unable to retrieve security groups.")) + + security_groups = [(group.id, group.name) for group in groups] - initial = {'tenant_id': tenant_id, - 'security_group_id': self.kwargs['security_group_id'], + initial = {'security_group_id': self.kwargs['security_group_id'], 'security_group_list': security_groups} return AddRule.maybe_handle(self.request, initial=initial) diff --git a/horizon/forms/base.py b/horizon/forms/base.py index 7cb272f6d2a..d48e8f278f4 100644 --- a/horizon/forms/base.py +++ b/horizon/forms/base.py @@ -84,10 +84,8 @@ def maybe_handle(cls, request, *args, **kwargs): if not form.is_valid(): return form, None - data = form.clean() - try: - return form, form.handle(request, data) + return form, form.handle(request, form.cleaned_data) except: exceptions.handle(request) return form, None diff --git a/horizon/test.py b/horizon/test.py index 3140837118c..26261570f0d 100644 --- a/horizon/test.py +++ b/horizon/test.py @@ -199,7 +199,8 @@ def assertNoFormErrors(self, response, context_name="form"): assert len(errors) == 0, \ "Unexpected errors were found on the form: %s" % errors - def assertFormErrors(self, response, count=0, context_name="form"): + def assertFormErrors(self, response, count=0, message=None, + context_name="form"): """ Asserts that the response does contain a form in it's context, and that form has errors, if count were given, @@ -213,6 +214,10 @@ def assertFormErrors(self, response, count=0, context_name="form"): assert len(errors) == count, \ "%d errors were found on the form, %d expected" % \ (len(errors), count) + if message and message not in unicode(errors): + self.fail("Expected message not found, instead found: %s" + % ["%s: %s" % (key, [e for e in field_errors]) for + (key, field_errors) in errors.items()]) else: assert len(errors) > 0, "No errors were found on the form"