Skip to content

Commit

Permalink
add source group rule interface in security groups edit
Browse files Browse the repository at this point in the history
 * fixes bug 860780
 * continues from abandoned https://review.openstack.org/3268
 * implements UI https://nebula.notableapp.com/posts/391efe1b94711ec4527b6f72a61e789b6a4a54b7
 * modified test to accomodate change
 * fix .form-field width from 96% to 90% to prevent weird overlap

Change-Id: I77af442ea4c0408556bfed8c2d1669b860219f66
  • Loading branch information
andycjw committed Mar 13, 2012
1 parent e206ba6 commit 588c782
Show file tree
Hide file tree
Showing 9 changed files with 106 additions and 24 deletions.
17 changes: 12 additions & 5 deletions horizon/api/nova.py
Expand Up @@ -30,6 +30,7 @@

from horizon.api.base import APIResourceWrapper, APIDictWrapper, url_for

from django.utils.translation import ugettext as _

LOG = logging.getLogger(__name__)

Expand Down Expand Up @@ -166,13 +167,19 @@ def rules(self, value):

class SecurityGroupRule(APIResourceWrapper):
""" Wrapper for individual rules in a SecurityGroup. """
_attrs = ['id', 'ip_protocol', 'from_port', 'to_port', 'ip_range']
_attrs = ['id', 'ip_protocol', 'from_port', 'to_port', 'ip_range', 'group']

def __unicode__(self):
vals = {'from': self.from_port,
'to': self.to_port,
'cidr': self.ip_range['cidr']}
return 'ALLOW %(from)s:%(to)s from %(cidr)s' % vals
if 'name' in self.group:
vals = {'from': self.from_port,
'to': self.to_port,
'group': self.group['name']}
return _('ALLOW %(from)s:%(to)s from %(group)s') % vals
else:
vals = {'from': self.from_port,
'to': self.to_port,
'cidr': self.ip_range['cidr']}
return _('ALLOW %(from)s:%(to)s from %(cidr)s') % vals


def novaclient(request):
Expand Down
Expand Up @@ -80,21 +80,34 @@ class AddRule(forms.SelfHandlingForm):
attrs={'data': _('To port'),
'data-icmp': _('Code')}),
validators=[validate_port_range])
cidr = forms.CharField(label=_("CIDR"),

source_group = forms.ChoiceField(label=_('Source Group'), required=False)
cidr = forms.CharField(label=_("CIDR"), required=False,
help_text=_("Classless Inter-Domain Routing "
"(i.e. 192.168.0.0/24"),
validators=[validate_ipv4_cidr])
# TODO (anthony) source group support
# group_id = forms.CharField()

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)
initials = kwargs.get("initial", {})
current_group_id = initials.get('security_group_id', 0)
security_groups = initials.get('security_group_list', [])
security_groups_choices = [("", "CIDR")] # default choice is CIDR
group_choices = [s for s in security_groups
if str(s[0]) != current_group_id]
if len(group_choices): # add group choice if available
security_groups_choices.append(('Security Group', group_choices))
self.fields['source_group'].choices = security_groups_choices

def clean(self):
cleaned_data = super(AddRule, self).clean()
from_port = cleaned_data.get("from_port", None)
to_port = cleaned_data.get("to_port", None)
cidr = cleaned_data.get("cidr", None)
source_group = cleaned_data.get("source_group", None)

if from_port == None:
msg = _('The "from" port number is invalid.')
Expand All @@ -107,9 +120,12 @@ def clean(self):
'the "from" port number.')
raise ValidationError(msg)

if cidr == None:
msg = _('The "CIDR" is invalid')
if cidr and source_group:
msg = _('Only either "CIDR" or "Source Group" may be specified')
raise ValidationError(msg)
if cidr:
# if only cidr is specified, make sure source_group is cleaned
cleaned_data['source_group'] = None

return cleaned_data

Expand All @@ -120,7 +136,8 @@ def handle(self, request, data):
data['ip_protocol'],
data['from_port'],
data['to_port'],
data['cidr'])
data['cidr'],
data['source_group'])
messages.success(request, _('Successfully added rule: %s') \
% unicode(rule))
except novaclient_exceptions.ClientException, e:
Expand Down
Expand Up @@ -78,8 +78,13 @@ def get_success_url(self, request):
return reverse("horizon:nova:access_and_security:index")


def get_cidr(rule):
return rule.ip_range['cidr']
def get_source(rule):
if 'cidr' in rule.ip_range:
return rule.ip_range['cidr'] + ' (CIDR)'
elif 'name' in rule.group:
return rule.group['name']
else:
return None


class RulesTable(tables.DataTable):
Expand All @@ -88,7 +93,7 @@ class RulesTable(tables.DataTable):
filters=(unicode.upper,))
from_port = tables.Column("from_port", verbose_name=_("From Port"))
to_port = tables.Column("to_port", verbose_name=_("To Port"))
cidr = tables.Column(get_cidr, verbose_name=_("CIDR"))
source = tables.Column(get_source, verbose_name=_("Source"))

def sanitize_id(self, obj_id):
return int(obj_id)
Expand Down
Expand Up @@ -84,9 +84,14 @@ def test_create_security_groups_post_exception(self):

def test_edit_rules_get(self):
sec_group = self.security_groups.first()
sec_group_list = self.security_groups.list()

self.mox.StubOutWithMock(api, 'security_group_get')
api.security_group_get(IsA(http.HttpRequest),
sec_group.id).AndReturn(sec_group)
self.mox.StubOutWithMock(api, 'security_group_list')
api.security_group_list(
IsA(http.HttpRequest)).AndReturn(sec_group_list)
self.mox.ReplayAll()

res = self.client.get(self.edit_url)
Expand All @@ -97,18 +102,25 @@ def test_edit_rules_get(self):

def test_edit_rules_get_exception(self):
sec_group = self.security_groups.first()
sec_group_list = self.security_groups.list()

self.mox.StubOutWithMock(api, 'security_group_get')
exc = novaclient_exceptions.ClientException('ClientException')
api.security_group_get(IsA(http.HttpRequest),
sec_group.id).AndRaise(exc)
self.mox.StubOutWithMock(api, 'security_group_list')
api.security_group_list(
IsA(http.HttpRequest)).AndReturn(sec_group_list)
api.security_group_list(
IsA(http.HttpRequest)).AndReturn(sec_group_list)
self.mox.ReplayAll()

res = self.client.get(self.edit_url)
self.assertRedirects(res, INDEX_URL)

def test_edit_rules_add_rule(self):
def test_edit_rules_add_rule_cidr(self):
sec_group = self.security_groups.first()
sec_group_list = self.security_groups.list()
rule = self.security_group_rules.first()

self.mox.StubOutWithMock(api, 'security_group_rule_create')
Expand All @@ -117,7 +129,11 @@ def test_edit_rules_add_rule(self):
rule.ip_protocol,
int(rule.from_port),
int(rule.to_port),
rule.ip_range['cidr']).AndReturn(rule)
rule.ip_range['cidr'],
None).AndReturn(rule)
self.mox.StubOutWithMock(api, 'security_group_list')
api.security_group_list(
IsA(http.HttpRequest)).AndReturn(sec_group_list)
self.mox.ReplayAll()

formData = {'method': 'AddRule',
Expand All @@ -126,17 +142,22 @@ def test_edit_rules_add_rule(self):
'from_port': rule.from_port,
'to_port': rule.to_port,
'ip_protocol': rule.ip_protocol,
'cidr': rule.ip_range['cidr']}
'cidr': rule.ip_range['cidr'],
'source_group': ''}
res = self.client.post(self.edit_url, formData)
self.assertRedirectsNoFollow(res, INDEX_URL)

def test_edit_rules_invalid_port_range(self):
sec_group = self.security_groups.first()
sec_group_list = self.security_groups.list()
rule = self.security_group_rules.first()

self.mox.StubOutWithMock(api, 'security_group_get')
api.security_group_get(IsA(http.HttpRequest),
sec_group.id).AndReturn(sec_group)
self.mox.StubOutWithMock(api, 'security_group_list')
api.security_group_list(
IsA(http.HttpRequest)).AndReturn(sec_group_list)
self.mox.ReplayAll()

formData = {'method': 'AddRule',
Expand All @@ -145,13 +166,15 @@ def test_edit_rules_invalid_port_range(self):
'from_port': rule.from_port,
'to_port': int(rule.from_port) - 1,
'ip_protocol': rule.ip_protocol,
'cidr': rule.ip_range['cidr']}
'cidr': rule.ip_range['cidr'],
'source_group': ''}
res = self.client.post(self.edit_url, formData)
self.assertNoMessages()
self.assertContains(res, "greater than or equal to")

def test_edit_rules_add_rule_exception(self):
sec_group = self.security_groups.first()
sec_group_list = self.security_groups.list()
rule = self.security_group_rules.first()
exc = novaclient_exceptions.ClientException('ClientException')

Expand All @@ -161,7 +184,11 @@ def test_edit_rules_add_rule_exception(self):
rule.ip_protocol,
int(rule.from_port),
int(rule.to_port),
rule.ip_range['cidr']).AndRaise(exc)
rule.ip_range['cidr'],
None).AndRaise(exc)
self.mox.StubOutWithMock(api, 'security_group_list')
api.security_group_list(
IsA(http.HttpRequest)).AndReturn(sec_group_list)
self.mox.ReplayAll()

formData = {'method': 'AddRule',
Expand All @@ -170,7 +197,8 @@ def test_edit_rules_add_rule_exception(self):
'from_port': rule.from_port,
'to_port': rule.to_port,
'ip_protocol': rule.ip_protocol,
'cidr': rule.ip_range['cidr']}
'cidr': rule.ip_range['cidr'],
'source_group': ''}
res = self.client.post(self.edit_url, formData)
self.assertRedirectsNoFollow(res, INDEX_URL)

Expand Down
Expand Up @@ -57,8 +57,12 @@ def get_data(self):

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)]

initial = {'tenant_id': tenant_id,
'security_group_id': self.kwargs['security_group_id']}
'security_group_id': self.kwargs['security_group_id'],
'security_group_list': security_groups}
return AddRule.maybe_handle(self.request, initial=initial)

def get(self, request, *args, **kwargs):
Expand Down
Expand Up @@ -3,7 +3,7 @@

{% block form_id %}security_group_rule_form{% endblock %}
{% block form_action %}{% url horizon:nova:access_and_security:security_groups:edit_rules security_group.id %}{% endblock %}
{% block form_class %}{{ block.super }} horizontal split_quarter{% endblock %}
{% block form_class %}{{ block.super }} horizontal split_five{% endblock %}

{% block modal_id %}security_group_rule_modal{% endblock %}
{% block modal-header %}{% trans "Edit Security Group Rules" %}{% endblock %}
Expand Down
2 changes: 2 additions & 0 deletions horizon/static/horizon/js/forms.js
Expand Up @@ -11,6 +11,8 @@ horizon.addInitFunction(function () {

horizon.datatables.validate_button();

horizon.forms.handle_source_group();

// Confirmation on deletion of items.
// TODO (tres): These need to be localizable or to just plain go away in favor
// of modals.
Expand Down
16 changes: 16 additions & 0 deletions horizon/static/horizon/js/horizon.js
Expand Up @@ -40,6 +40,22 @@ var Horizon = function() {
initFunctions = [];
};

/* Namespace for core functionality related to Forms. */
horizon.forms = {
handle_source_group: function() {
// Delegate this handler to form, so it only should be init once
$("form").live("change", "#id_source_group", function(evt) {
var $sourceGroup = $(this).find('#id_source_group');
var $cidrContainer = $(this).find('#id_cidr').parent().parent();
if($sourceGroup.val() == "") {
$cidrContainer.removeClass("hide");
} else {
$cidrContainer.addClass("hide");
}
});
}
};

/* Namespace for core functionality related to DataTables. */
horizon.datatables = {
update: function () {
Expand Down
7 changes: 5 additions & 2 deletions openstack_dashboard/static/dashboard/css/style.css
Expand Up @@ -503,6 +503,10 @@ td .icon-updating.ajax-updating {
width: 167px; /* Fits 4 fields to a row */
}

.modal form.horizontal.split_five .form-field {
width: 133px; /* Fits 5 fields to a row */
}

.modal form.horizontal fieldset {
width: 100%;
}
Expand Down Expand Up @@ -551,15 +555,14 @@ td .icon-updating.ajax-updating {
width: 372px;
}

.modal .form-field,
.modal fieldset ul {
width: 90%;
}

.modal fieldset .form-field input,
.modal fieldset .form-field select,
.modal fieldset .form-field textarea {
width: 96%;
width: 90%;
}

.modal .modal-footer input {
Expand Down

0 comments on commit 588c782

Please sign in to comment.