From f72eee5be4eae04b93ce7f03c39d8eaa3d0fa996 Mon Sep 17 00:00:00 2001 From: Ryan Rossiter Date: Tue, 24 Jan 2017 13:40:45 -0600 Subject: [PATCH 01/13] Add security group functionality This adds security group API helpers to the network manager. This allows users to perform CRUD operations on security groups, security group rules, and interface attachments to security groups. CLI commands were also added to perform these API operations on the command line. Arguments were also added to the instance creation to allow VSIs to be booted with security groups on the public and/or private interfaces of the newly created VSI. Security group information was added to the VSI detail CLI, so the security groups attached to the interfaces of a VSI are output as part of the detail. --- SoftLayer/CLI/routes.py | 17 ++ SoftLayer/CLI/securitygroup/__init__.py | 1 + SoftLayer/CLI/securitygroup/create.py | 31 ++++ SoftLayer/CLI/securitygroup/delete.py | 15 ++ SoftLayer/CLI/securitygroup/detail.py | 59 +++++++ SoftLayer/CLI/securitygroup/edit.py | 28 ++++ SoftLayer/CLI/securitygroup/interface.py | 132 +++++++++++++++ SoftLayer/CLI/securitygroup/list.py | 36 ++++ SoftLayer/CLI/securitygroup/rule.py | 128 ++++++++++++++ SoftLayer/CLI/virt/create.py | 16 ++ SoftLayer/CLI/virt/detail.py | 13 ++ SoftLayer/managers/network.py | 204 +++++++++++++++++++++++ SoftLayer/managers/vs.py | 27 ++- tox.ini | 6 + 14 files changed, 710 insertions(+), 3 deletions(-) create mode 100644 SoftLayer/CLI/securitygroup/__init__.py create mode 100644 SoftLayer/CLI/securitygroup/create.py create mode 100644 SoftLayer/CLI/securitygroup/delete.py create mode 100644 SoftLayer/CLI/securitygroup/detail.py create mode 100644 SoftLayer/CLI/securitygroup/edit.py create mode 100644 SoftLayer/CLI/securitygroup/interface.py create mode 100644 SoftLayer/CLI/securitygroup/list.py create mode 100644 SoftLayer/CLI/securitygroup/rule.py diff --git a/SoftLayer/CLI/routes.py b/SoftLayer/CLI/routes.py index fe2bb12f8..8f142bae2 100644 --- a/SoftLayer/CLI/routes.py +++ b/SoftLayer/CLI/routes.py @@ -209,6 +209,23 @@ ('hardware:credentials', 'SoftLayer.CLI.hardware.credentials:cli'), ('hardware:update-firmware', 'SoftLayer.CLI.hardware.update_firmware:cli'), + ('securitygroup', 'SoftLayer.CLI.securitygroup'), + ('securitygroup:list', 'SoftLayer.CLI.securitygroup.list:cli'), + ('securitygroup:detail', 'SoftLayer.CLI.securitygroup.detail:cli'), + ('securitygroup:create', 'SoftLayer.CLI.securitygroup.create:cli'), + ('securitygroup:edit', 'SoftLayer.CLI.securitygroup.edit:cli'), + ('securitygroup:delete', 'SoftLayer.CLI.securitygroup.delete:cli'), + ('securitygroup:rule-list', 'SoftLayer.CLI.securitygroup.rule:rule_list'), + ('securitygroup:rule-add', 'SoftLayer.CLI.securitygroup.rule:add'), + ('securitygroup:rule-edit', 'SoftLayer.CLI.securitygroup.rule:edit'), + ('securitygroup:rule-remove', 'SoftLayer.CLI.securitygroup.rule:remove'), + ('securitygroup:interface-list', + 'SoftLayer.CLI.securitygroup.interface:interface_list'), + ('securitygroup:interface-add', + 'SoftLayer.CLI.securitygroup.interface:add'), + ('securitygroup:interface-remove', + 'SoftLayer.CLI.securitygroup.interface:remove'), + ('sshkey', 'SoftLayer.CLI.sshkey'), ('sshkey:add', 'SoftLayer.CLI.sshkey.add:cli'), ('sshkey:remove', 'SoftLayer.CLI.sshkey.remove:cli'), diff --git a/SoftLayer/CLI/securitygroup/__init__.py b/SoftLayer/CLI/securitygroup/__init__.py new file mode 100644 index 000000000..936e276c8 --- /dev/null +++ b/SoftLayer/CLI/securitygroup/__init__.py @@ -0,0 +1 @@ +"""Network security groups.""" diff --git a/SoftLayer/CLI/securitygroup/create.py b/SoftLayer/CLI/securitygroup/create.py new file mode 100644 index 000000000..6d759e2eb --- /dev/null +++ b/SoftLayer/CLI/securitygroup/create.py @@ -0,0 +1,31 @@ +"""Create security groups.""" +# :license: MIT, see LICENSE for more details. + +import click + +import SoftLayer +from SoftLayer.CLI import environment +from SoftLayer.CLI import formatting + + +@click.command() +@click.option('--name', '-n', + help="The name of the security group") +@click.option('--description', '-d', + help="The description of the security group") +@environment.pass_env +def cli(env, name, description): + """Create a security group.""" + mgr = SoftLayer.NetworkManager(env.client) + + result = mgr.create_securitygroup(name, description) + table = formatting.KeyValueTable(['name', 'value']) + table.align['name'] = 'r' + table.align['value'] = 'l' + table.add_row(['id', result['id']]) + table.add_row(['name', result.get('name', formatting.blank())]) + table.add_row(['description', + result.get('description', formatting.blank())]) + table.add_row(['created', result['createDate']]) + + env.fout(table) diff --git a/SoftLayer/CLI/securitygroup/delete.py b/SoftLayer/CLI/securitygroup/delete.py new file mode 100644 index 000000000..dfd3080e2 --- /dev/null +++ b/SoftLayer/CLI/securitygroup/delete.py @@ -0,0 +1,15 @@ +"""Delete a security group.""" +# :license: MIT, see LICENSE for more details. + +import click +import SoftLayer +from SoftLayer.CLI import environment + + +@click.command() +@click.argument('securitygroup_id') +@environment.pass_env +def cli(env, securitygroup_id): + """Deletes the given security group""" + mgr = SoftLayer.NetworkManager(env.client) + mgr.delete_securitygroup(securitygroup_id) diff --git a/SoftLayer/CLI/securitygroup/detail.py b/SoftLayer/CLI/securitygroup/detail.py new file mode 100644 index 000000000..187f4d587 --- /dev/null +++ b/SoftLayer/CLI/securitygroup/detail.py @@ -0,0 +1,59 @@ +"""Get details about a security group.""" +# :license: MIT, see LICENSE for more details. + +import click + +import SoftLayer +from SoftLayer.CLI import environment +from SoftLayer.CLI import formatting + + +@click.command() +@click.argument('identifier') +@environment.pass_env +def cli(env, identifier): + """Get details about a security group.""" + + mgr = SoftLayer.NetworkManager(env.client) + + secgroup = mgr.get_securitygroup(identifier) + + table = formatting.KeyValueTable(['name', 'value']) + table.align['name'] = 'r' + table.align['value'] = 'l' + + table.add_row(['id', secgroup['id']]) + table.add_row(['name', secgroup.get('name', formatting.blank())]) + table.add_row(['description', + secgroup.get('description', formatting.blank())]) + + rule_table = formatting.Table(['id', 'remoteIp', 'remoteGroupId', + 'direction', 'ethertype', 'portRangeMin', + 'portRangeMax', 'protocol']) + for rule in secgroup.get('rules', []): + rg_id = rule.get('remoteGroup', {}).get('id', formatting.blank()) + rule_table.add_row([rule['id'], + rule.get('remoteIp', formatting.blank()), + rule.get('remoteGroupId', rg_id), + rule['direction'], + rule.get('ethertype', formatting.blank()), + rule.get('portRangeMin', formatting.blank()), + rule.get('portRangeMax', formatting.blank()), + rule.get('protocol', formatting.blank())]) + + table.add_row(['rules', rule_table]) + + vsi_table = formatting.Table(['id', 'hostname', 'interface', 'ipAddress']) + + for binding in secgroup.get('networkComponentBindings'): + vsi = binding['networkComponent']['guest'] + interface = ('PRIVATE' if binding['networkComponent']['port'] == 0 + else 'PUBLIC') + ip_address = (vsi['primaryBackendIpAddress'] + if binding['networkComponent']['port'] == 0 + else vsi['primaryIpAddress']) + vsi_table.add_row([vsi['id'], vsi['hostname'], interface, ip_address]) + + table.add_row(['servers', vsi_table]) + + env.fout(table) diff --git a/SoftLayer/CLI/securitygroup/edit.py b/SoftLayer/CLI/securitygroup/edit.py new file mode 100644 index 000000000..a8a49c679 --- /dev/null +++ b/SoftLayer/CLI/securitygroup/edit.py @@ -0,0 +1,28 @@ +"""Edit details of a security group.""" +# :license: MIT, see LICENSE for more details. + +import click + +import SoftLayer +from SoftLayer.CLI import environment +from SoftLayer.CLI import exceptions + + +@click.command() +@click.argument('group_id') +@click.option('--name', '-n', + help="The name of the security group") +@click.option('--description', '-d', + help="The description of the security group") +@environment.pass_env +def cli(env, group_id, name, description): + """Edit details of a security group.""" + mgr = SoftLayer.NetworkManager(env.client) + data = {} + if name: + data['name'] = name + if description: + data['description'] = description + + if not mgr.edit_securitygroup(group_id, **data): + raise exceptions.CLIAbort("Failed to edit security group") diff --git a/SoftLayer/CLI/securitygroup/interface.py b/SoftLayer/CLI/securitygroup/interface.py new file mode 100644 index 000000000..fdf90481a --- /dev/null +++ b/SoftLayer/CLI/securitygroup/interface.py @@ -0,0 +1,132 @@ +"""Security group interface operations.""" +# :license: MIT, see LICENSE for more details. + +import click + +import SoftLayer +from SoftLayer.CLI import environment +from SoftLayer.CLI import exceptions +from SoftLayer.CLI import formatting + +COLUMNS = ['networkComponentId', + 'virtualServerId', + 'hostname', + 'interface', + 'ipAddress', ] + + +@click.command() +@click.argument('securitygroup_id') +@click.option('--sortby', + help='Column to sort by', + type=click.Choice(COLUMNS)) +@environment.pass_env +def interface_list(env, securitygroup_id, sortby): + """List interfaces associated with security groups.""" + mgr = SoftLayer.NetworkManager(env.client) + + table = formatting.Table(COLUMNS) + table.sortby = sortby + + mask = ( + '''networkComponentBindings[ + networkComponent[ + id, + port, + guest[ + id, + hostname, + primaryBackendIpAddress, + primaryIpAddress + ] + ] + ]''' + ) + + secgroup = mgr.get_securitygroup(securitygroup_id, mask=mask) + for binding in secgroup.get('networkComponentBindings'): + interface = binding['networkComponent'] + vsi = interface['guest'] + priv_pub = 'PRIVATE' if interface['port'] == 0 else 'PUBLIC' + ip_address = (vsi['primaryBackendIpAddress'] if interface['port'] == 0 + else vsi['primaryIpAddress']) + table.add_row([ + interface['id'], + vsi['id'], + vsi['hostname'], + priv_pub, + ip_address + ]) + + env.fout(table) + + +@click.command() +@click.argument('securitygroup_id') +@click.option('--network-component', '-n', + help=('The network component to associate ' + 'with the security group')) +@click.option('--server', '-s', + help='The server ID to associate with the security group') +@click.option('--interface', '-i', + help='The interface of the server to associate (public/private)') +@environment.pass_env +def add(env, securitygroup_id, network_component, server, interface): + """Attach an interface to a security group.""" + _validate_args(network_component, server, interface) + + mgr = SoftLayer.NetworkManager(env.client) + component_id = _get_component_id(env, network_component, server, interface) + + mgr.attach_securitygroup_component(securitygroup_id, component_id) + + +@click.command() +@click.argument('securitygroup_id') +@click.option('--network-component', '-n', + help=('The network component to remove from ' + 'with the security group')) +@click.option('--server', '-s', + help='The server ID to remove from the security group') +@click.option('--interface', '-i', + help='The interface of the server to remove (public/private)') +@environment.pass_env +def remove(env, securitygroup_id, network_component, server, interface): + """Detach an interface from a security group.""" + _validate_args(network_component, server, interface) + + mgr = SoftLayer.NetworkManager(env.client) + component_id = _get_component_id(env, network_component, server, interface) + + mgr.detach_securitygroup_component(securitygroup_id, component_id) + + +def _validate_args(network_component, server, interface): + use_server = bool(server and interface and not network_component) + use_component = bool(network_component and not bool(server or interface)) + + if not use_server and not use_component: + raise exceptions.CLIAbort("Must set either --network-component " + "or both --server and --interface") + if use_server and interface.lower() not in ['public', 'private']: + raise exceptions.CLIAbort( + "Interface must be either 'public' or 'private'") + + +def _get_component_id(env, network_component, server, interface): + use_server = bool(server and interface and not network_component) + vs_mgr = SoftLayer.VSManager(env.client) + + if use_server: + vs_mask = 'networkComponents[id, port]' + vsi = vs_mgr.get_instance(server, mask=vs_mask) + port = 0 if interface.lower() == 'private' else 1 + component = [c for c in vsi['networkComponents'] if c['port'] == port] + if len(component) != 1: + raise exceptions.CLIAbort("Instance %s has no %s interface" + % (server, interface)) + component_id = component[0]['id'] + else: + component_id = network_component + + return component_id diff --git a/SoftLayer/CLI/securitygroup/list.py b/SoftLayer/CLI/securitygroup/list.py new file mode 100644 index 000000000..0ba9ba1d4 --- /dev/null +++ b/SoftLayer/CLI/securitygroup/list.py @@ -0,0 +1,36 @@ +"""List securitygroups.""" +# :license: MIT, see LICENSE for more details. + +import click + +import SoftLayer +from SoftLayer.CLI import environment +from SoftLayer.CLI import formatting + +COLUMNS = ['id', + 'name', + 'description', ] + + +@click.command() +@click.option('--sortby', + help='Column to sort by', + type=click.Choice(COLUMNS)) +@environment.pass_env +def cli(env, sortby): + """List security groups.""" + + mgr = SoftLayer.NetworkManager(env.client) + + table = formatting.Table(COLUMNS) + table.sortby = sortby + + sgs = mgr.list_securitygroups() + for secgroup in sgs: + table.add_row([ + secgroup['id'], + secgroup.get('name') or formatting.blank(), + secgroup.get('description') or formatting.blank(), + ]) + + env.fout(table) diff --git a/SoftLayer/CLI/securitygroup/rule.py b/SoftLayer/CLI/securitygroup/rule.py new file mode 100644 index 000000000..4abc0c7b3 --- /dev/null +++ b/SoftLayer/CLI/securitygroup/rule.py @@ -0,0 +1,128 @@ +"""Manage security group rules.""" +# :license: MIT, see LICENSE for more details. + +import click + +import SoftLayer +from SoftLayer.CLI import environment +from SoftLayer.CLI import exceptions +from SoftLayer.CLI import formatting + +COLUMNS = ['id', + 'remoteIp', + 'remoteGroupId', + 'direction', + 'etherype', + 'portRangeMin', + 'portRangeMax', + 'protocol'] + + +@click.command() +@click.argument('securitygroup_id') +@click.option('--sortby', + help='Column to sort by', + type=click.Choice(COLUMNS)) +@environment.pass_env +def rule_list(env, securitygroup_id, sortby): + """List security group rules.""" + + mgr = SoftLayer.NetworkManager(env.client) + + table = formatting.Table(COLUMNS) + table.sortby = sortby + + rules = mgr.list_securitygroup_rules(securitygroup_id) + for rule in rules: + table.add_row([ + rule['id'], + rule.get('remoteIp', formatting.blank()), + rule.get('remoteGroupId', formatting.blank()), + rule['direction'], + rule.get('ethertype', formatting.blank()), + rule.get('portRangeMin', formatting.blank()), + rule.get('portRangeMax', formatting.blank()), + rule.get('protocol', formatting.blank()) + ]) + + env.fout(table) + + +@click.command() +@click.argument('securitygroup_id') +@click.option('--remote-ip', '-r', + help='The remote IP/CIDR to enforce') +@click.option('--remote-group', '-s', type=click.INT, + help='The ID of the remote security group to enforce') +@click.option('--direction', '-d', + help='The direction of traffic to enforce') +@click.option('--ethertype', '-e', + help='The ethertype (IPv4 or IPv6) to enforce') +@click.option('--port-range-max', '-M', type=click.INT, + help='The upper port bound to enforce') +@click.option('--port-range-min', '-m', type=click.INT, + help='The lower port bound to enforce') +@click.option('--protocol', '-p', + help='The protocol (icmp, tcp, udp) to enforce') +@environment.pass_env +def add(env, securitygroup_id, remote_ip, remote_group, + direction, ethertype, port_range_max, port_range_min, protocol): + """Add a security group rule to a security group.""" + mgr = SoftLayer.NetworkManager(env.client) + + mgr.add_securitygroup_rule(securitygroup_id, remote_ip, remote_group, + direction, ethertype, port_range_max, + port_range_min, protocol) + + +@click.command() +@click.argument('securitygroup_id') +@click.argument('rule_id') +@click.option('--remote-ip', '-r', + help='The remote IP/CIDR to enforce') +@click.option('--remote-group', '-s', + help='The ID of the remote security group to enforce') +@click.option('--direction', '-d', + help='The direction of traffic to enforce') +@click.option('--ethertype', '-e', + help='The ethertype (IPv4 or IPv6) to enforce') +@click.option('--port-range-max', '-M', + help='The upper port bound to enforce') +@click.option('--port-range-min', '-m', + help='The lower port bound to enforce') +@click.option('--protocol', '-p', + help='The protocol (icmp, tcp, udp) to enforce') +@environment.pass_env +def edit(env, securitygroup_id, rule_id, remote_ip, remote_group, + direction, ethertype, port_range_max, port_range_min, protocol): + """Edit a security group rule in a security group.""" + mgr = SoftLayer.NetworkManager(env.client) + + data = {} + if remote_ip: + data['remote_ip'] = remote_ip + if remote_group: + data['remote_group'] = remote_group + if direction: + data['direction'] = direction + if ethertype: + data['ethertype'] = ethertype + if port_range_max: + data['port_range_max'] = port_range_max + if port_range_min: + data['port_range_min'] = port_range_min + if protocol: + data['protocol'] = protocol + + if not mgr.edit_securitygroup_rule(securitygroup_id, rule_id, **data): + raise exceptions.CLIAbort("Failed to edit security group rule") + + +@click.command() +@click.argument('securitygroup_id') +@click.argument('rule_id') +@environment.pass_env +def remove(env, securitygroup_id, rule_id): + """Remove a rule from a security group.""" + mgr = SoftLayer.NetworkManager(env.client) + mgr.remove_securitygroup_rule(securitygroup_id, rule_id) diff --git a/SoftLayer/CLI/virt/create.py b/SoftLayer/CLI/virt/create.py index d764cfb09..a3fa28b2c 100644 --- a/SoftLayer/CLI/virt/create.py +++ b/SoftLayer/CLI/virt/create.py @@ -119,6 +119,14 @@ def _parse_create_args(client, args): if args.get('vlan_private'): data['private_vlan'] = args['vlan_private'] + if args.get('public_security_group'): + pub_groups = args.get('public_security_group') + data['public_security_groups'] = [group for group in pub_groups] + + if args.get('private_security_group'): + priv_groups = args.get('private_security_group') + data['private_security_groups'] = [group for group in priv_groups] + if args.get('tag'): data['tags'] = ','.join(args['tag']) @@ -201,6 +209,14 @@ def _parse_create_args(client, args): help="The ID of the private VLAN on which you want the virtual " "server placed", type=click.INT) +@helpers.multi_option('--public-security-group', + '-S', + help=('Security group ID to associate with ' + 'the public interface')) +@helpers.multi_option('--private-security-group', + '-s', + help=('Security group ID to associate with ' + 'the private interface')) @click.option('--wait', type=click.INT, help="Wait until VS is finished provisioning for up to X " diff --git a/SoftLayer/CLI/virt/detail.py b/SoftLayer/CLI/virt/detail.py index 9403b1ab3..3ade65b50 100644 --- a/SoftLayer/CLI/virt/detail.py +++ b/SoftLayer/CLI/virt/detail.py @@ -77,6 +77,19 @@ def cli(env, identifier, passwords=False, price=False): vlan['networkSpace'], vlan['vlanNumber'], vlan['id']]) table.add_row(['vlans', vlan_table]) + if result.get('networkComponents'): + secgroup_table = formatting.Table(['interface', 'id', 'name']) + has_secgroups = False + for comp in result.get('networkComponents'): + interface = 'PRIVATE' if comp['port'] == 0 else 'PUBLIC' + for binding in comp['securityGroupBindings']: + has_secgroups = True + secgroup = binding['securityGroup'] + secgroup_table.add_row([ + interface, secgroup['id'], secgroup['name']]) + if has_secgroups: + table.add_row(['security_groups', secgroup_table]) + if result.get('notes'): table.add_row(['notes', result['notes']]) diff --git a/SoftLayer/managers/network.py b/SoftLayer/managers/network.py index 690da99b5..edce0e9a1 100644 --- a/SoftLayer/managers/network.py +++ b/SoftLayer/managers/network.py @@ -49,6 +49,7 @@ def __init__(self, client): self.vlan = client['Network_Vlan'] self.subnet = client['Network_Subnet'] self.network_storage = self.client['Network_Storage'] + self.security_group = self.client['Network_SecurityGroup'] def add_global_ip(self, version=4, test_order=False): """Adds a global IP address to the account. @@ -62,6 +63,41 @@ def add_global_ip(self, version=4, test_order=False): return self.add_subnet('global', version=version, test_order=test_order) + def add_securitygroup_rule(self, group_id, remote_ip=None, + remote_group=None, direction=None, + ethertype=None, port_max=None, + port_min=None, protocol=None): + """Add a rule to a security group + + :param int group_id: The ID of the security group to add this rule to + :param str remote_ip: The remote IP or CIDR to enforce the rule on + :param int remote_group: The remote security group ID to enforce + the rule on + :param str direction: The direction to enforce (egress or ingress) + :param str ethertype: The ethertype to enforce (IPv4 or IPv6) + :param int port_max: The upper port bound to enforce + :param int port_min: The lower port bound to enforce + :param str protocol: The protocol to enforce (icmp, udp, tcp) + """ + rule = {'direction': direction, + 'ethertype': ethertype, + 'portRangeMax': port_max, + 'portRangeMin': port_min, + 'protocol': protocol} + if remote_ip is not None: + rule['remoteIp'] = remote_ip + if remote_group is not None: + rule['remoteGroupId'] = remote_group + return self.add_securitygroup_rules(group_id, [rule]) + + def add_securitygroup_rules(self, group_id, rules): + """Add rules to a security group + + :param int group_id: The ID of the security group to add the rules to + :param list rules: The list of rule dictionaries to add + """ + return self.security_group.addRules(rules, id=group_id) + def add_subnet(self, subnet_type, quantity=None, vlan_id=None, version=4, test_order=False): """Orders a new subnet @@ -135,6 +171,24 @@ def assign_global_ip(self, global_ip_id, target): return self.client['Network_Subnet_IpAddress_Global'].route( target, id=global_ip_id) + def attach_securitygroup_component(self, group_id, component_id): + """Attaches a network component to a security group. + + :param int group_id: The ID of the security group + :param int component_id: The ID of the network component to attach + """ + return self.attach_securitygroup_components(group_id, + [component_id]) + + def attach_securitygroup_components(self, group_id, component_ids): + """Attaches network components to a security group. + + :param int group_id: The ID of the security group + :param list component_ids: The IDs of the network components to attach + """ + return self.security_group.attachNetworkComponents(component_ids, + id=group_id) + def cancel_global_ip(self, global_ip_id): """Cancels the specified global IP address. @@ -158,6 +212,40 @@ def cancel_subnet(self, subnet_id): billing_id = subnet['billingItem']['id'] return self.client['Billing_Item'].cancelService(id=billing_id) + def create_securitygroup(self, name=None, description=None): + """Creates a security group. + + :param string name: The name of the security group + :param string description: The description of the security group + """ + + create_dict = {'name': name, 'description': description} + return self.security_group.createObjects([create_dict])[0] + + def delete_securitygroup(self, group_id): + """Deletes the specified security group. + + :param int group_id: The ID of the security group + """ + delete_dict = {'id': group_id} + self.security_group.deleteObjects([delete_dict]) + + def detach_securitygroup_component(self, group_id, component_id): + """Detaches a network component from a security group. + + :param int group_id: The ID of the security group + :param int component_id: The ID of the component to detach + """ + self.detach_securitygroup_components(group_id, [component_id]) + + def detach_securitygroup_components(self, group_id, component_ids): + """Detaches network components from a security group. + + :param int group_id: The ID of the security group + :param list component_ids: The IDs of the network components to detach + """ + self.security_group.detachNetworkComponents(component_ids, id=group_id) + def edit_rwhois(self, abuse_email=None, address1=None, address2=None, city=None, company_name=None, country=None, first_name=None, last_name=None, postal_code=None, @@ -186,6 +274,65 @@ def edit_rwhois(self, abuse_email=None, address1=None, address2=None, return True + def edit_securitygroup(self, group_id, name=None, description=None): + """Edit security group details. + + :param int group_id: The ID of the security group + :param string name: The name of the security group + :param string description: The description of the security group + """ + obj = {} + if name: + obj['name'] = name + if description: + obj['description'] = description + + if obj: + obj['id'] = group_id + self.security_group.editObjects([obj]) + + return bool(name or description) + + def edit_securitygroup_rule(self, group_id, rule_id, remote_ip=None, + remote_group=None, direction=None, + ethertype=None, port_range_max=None, + port_range_min=None, protocol=None): + """Edit a security group rule. + + :param int group_id: The ID of the security group the rule belongs to + :param int rule_id: The ID of the rule to edit + :param str remote_ip: The remote IP or CIDR to enforce the rule on + :param int remote_group: The remote security group ID to enforce + the rule on + :param str direction: The direction to enforce (egress or ingress) + :param str ethertype: The ethertype to enforce (IPv4 or IPv6) + :param str port_range_max: The upper port bound to enforce + :param str port_range_min: The lower port bound to enforce + :param str protocol: The protocol to enforce (icmp, udp, tcp) + """ + obj = {} + if remote_ip: + obj['remoteIp'] = remote_ip + if remote_group: + obj['remoteGroupId'] = remote_group + if direction: + obj['direction'] = direction + if ethertype: + obj['ethertype'] = ethertype + if port_range_max: + obj['portRangeMax'] = port_range_max + if port_range_min: + obj['portRangeMin'] = port_range_min + if protocol: + obj['protocol'] = protocol + + if obj: + obj['id'] = rule_id + self.security_group.editRules([obj], id=group_id) + + return bool(remote_ip or remote_group or direction or ethertype + or port_range_max or port_range_min or protocol) + def ip_lookup(self, ip_address): """Looks up an IP address and returns network information about it. @@ -203,6 +350,36 @@ def get_rwhois(self): """ return self.account.getRwhoisData() + def get_securitygroup(self, group_id, **kwargs): + """Returns the information about the given security group. + + :param string id: The ID for the security group + :returns: A diction of information about the security group + """ + if 'mask' not in kwargs: + kwargs['mask'] = ( + 'id,' + 'name,' + 'description,' + '''rules[id, remoteIp, remoteGroup, + direction, ethertype, portRangeMin, + portRangeMax, protocol],''' + '''networkComponentBindings[ + networkComponent[ + id, + port, + guest[ + id, + hostname, + primaryBackendIpAddress, + primaryIpAddress + ] + ] + ]''' + ) + + return self.security_group.getObject(id=group_id, **kwargs) + def get_subnet(self, subnet_id, **kwargs): """Returns information about a single subnet. @@ -330,6 +507,33 @@ def list_vlans(self, datacenter=None, vlan_number=None, name=None, return self.account.getNetworkVlans(**kwargs) + def list_securitygroups(self, **kwargs): + """List security groups.""" + return self.security_group.getAllObjects(**kwargs) + + def list_securitygroup_rules(self, group_id): + """List security group rules associated with a security group. + + :param int group_id: The security group to list rules for + """ + return self.security_group.getRules(id=group_id) + + def remove_securitygroup_rule(self, group_id, rule_id): + """Remove a rule from a security group. + + :param int group_id: The ID of the security group + :param int rule_id: The ID of the rule to remove + """ + self.remove_securitygroup_rules(group_id, [rule_id]) + + def remove_securitygroup_rules(self, group_id, rules): + """Remove rules from a security group. + + :param int group_id: The ID of the security group + :param list rules: The list of IDs to remove + """ + self.security_group.removeRules(rules, id=group_id) + def resolve_global_ip_ids(self, identifier): """Resolve global ip ids.""" return utils.resolve_ids(identifier, diff --git a/SoftLayer/managers/vs.py b/SoftLayer/managers/vs.py index 6bcb4b20d..d0a5357ea 100644 --- a/SoftLayer/managers/vs.py +++ b/SoftLayer/managers/vs.py @@ -296,7 +296,8 @@ def _generate_create_dict( datacenter=None, os_code=None, image_id=None, dedicated=False, public_vlan=None, private_vlan=None, userdata=None, nic_speed=None, disks=None, post_uri=None, - private=False, ssh_keys=None): + private=False, ssh_keys=None, public_security_groups=None, + private_security_groups=None): """Returns a dict appropriate to pass into Virtual_Guest::createObject See :func:`create_instance` for a list of available options. @@ -348,6 +349,20 @@ def _generate_create_dict( "primaryBackendNetworkComponent": { "networkVlan": {"id": int(private_vlan)}}}) + if public_security_groups: + secgroups = [{'securityGroup': {'id': int(sg)}} + for sg in public_security_groups] + pnc = data.get('primaryNetworkComponent', {}) + pnc['securityGroupBindings'] = secgroups + data.update({'primaryNetworkComponent': pnc}) + + if private_security_groups: + secgroups = [{'securityGroup': {'id': int(sg)}} + for sg in private_security_groups] + pbnc = data.get('primaryBackendNetworkComponent', {}) + pbnc['securityGroupBindings'] = secgroups + data.update({'primaryBackendNetworkComponent': pbnc}) + if userdata: data['userData'] = [{'value': userdata}] @@ -504,7 +519,8 @@ def create_instance(self, **kwargs): 'disks': ('100','25'), 'local_disk': True, 'memory': 1024, - 'tags': 'test, pleaseCancel' + 'tags': 'test, pleaseCancel', + 'public_security_groups': [12, 15] } vsi = mgr.create_instance(**new_vsi) @@ -530,6 +546,10 @@ def create_instance(self, **kwargs): incur a fee on your account. :param int public_vlan: The ID of the public VLAN on which you want this VS placed. + :param list public_security_groups: The list of security group IDs + to apply to the public interface + :param list private_security_groups: The list of security group IDs + to apply to the private interface :param int private_vlan: The ID of the private VLAN on which you want this VS placed. :param list disks: A list of disk capacities for this server. @@ -573,7 +593,8 @@ def create_instances(self, config_list): 'disks': ('100','25'), 'local_disk': True, 'memory': 1024, - 'tags': 'test, pleaseCancel' + 'tags': 'test, pleaseCancel', + 'public_security_groups': [12, 15] } # using .copy() so we can make changes to individual nodes diff --git a/tox.ini b/tox.ini index 8262ba830..53cc14dcf 100644 --- a/tox.ini +++ b/tox.ini @@ -40,6 +40,12 @@ commands = --max-statements=60 \ --min-public-methods=0 \ --min-similarity-lines=30 +# --max-args=25 \ +# --max-branches=20 \ +# --max-statements=60 \ +# --min-public-methods=0 \ +# --max-public-methods=50 \ +# --min-similarity-lines=30 # invalid-name - Fixtures don't follow proper naming conventions # missing-docstring - Fixtures don't have docstrings From cf03ad1d59c2ce16eb3f4a6bf57e4dc14718a57f Mon Sep 17 00:00:00 2001 From: Ryan Rossiter Date: Tue, 25 Apr 2017 11:22:55 -0500 Subject: [PATCH 02/13] Add direction options to help text --- SoftLayer/CLI/securitygroup/rule.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/SoftLayer/CLI/securitygroup/rule.py b/SoftLayer/CLI/securitygroup/rule.py index 4abc0c7b3..f8f692ec3 100644 --- a/SoftLayer/CLI/securitygroup/rule.py +++ b/SoftLayer/CLI/securitygroup/rule.py @@ -55,7 +55,8 @@ def rule_list(env, securitygroup_id, sortby): @click.option('--remote-group', '-s', type=click.INT, help='The ID of the remote security group to enforce') @click.option('--direction', '-d', - help='The direction of traffic to enforce') + help=('The direction of traffic to enforce ' + '(ingress, egress)')) @click.option('--ethertype', '-e', help='The ethertype (IPv4 or IPv6) to enforce') @click.option('--port-range-max', '-M', type=click.INT, From bd7141d63f2b7e3b422494d3af6c7b643a493221 Mon Sep 17 00:00:00 2001 From: Ryan Rossiter Date: Wed, 26 Apr 2017 11:29:37 -0500 Subject: [PATCH 03/13] Add security groups to get_instance mask In order to list the security groups attached to a VSI, we need to add security groups to the get_instance mask. This is retrieved via networkComponents[securityGroupBindings[securityGroups]]. --- SoftLayer/managers/vs.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/SoftLayer/managers/vs.py b/SoftLayer/managers/vs.py index d0a5357ea..528043603 100644 --- a/SoftLayer/managers/vs.py +++ b/SoftLayer/managers/vs.py @@ -192,7 +192,9 @@ def get_instance(self, instance_id, **kwargs): 'primaryIpAddress,' '''networkComponents[id, status, speed, maxSpeed, name, macAddress, primaryIpAddress, port, - primarySubnet],''' + primarySubnet, + securityGroupBindings[ + securityGroup[id, name]]],''' 'lastKnownPowerState.name,' 'powerState,' 'status,' From 205717ee4e30baf5968f6ea878092f24b354e21d Mon Sep 17 00:00:00 2001 From: Ryan Rossiter Date: Wed, 26 Apr 2017 15:38:16 -0500 Subject: [PATCH 04/13] Alias securitygroup to sg --- SoftLayer/CLI/routes.py | 1 + 1 file changed, 1 insertion(+) diff --git a/SoftLayer/CLI/routes.py b/SoftLayer/CLI/routes.py index 8f142bae2..d729bf020 100644 --- a/SoftLayer/CLI/routes.py +++ b/SoftLayer/CLI/routes.py @@ -273,6 +273,7 @@ 'lb': 'loadbal', 'meta': 'metadata', 'my': 'metadata', + 'sg': 'securitygroup', 'server': 'hardware', 'vm': 'virtual', 'vs': 'virtual', From 8be235c54ade0a845fa834e5a4e37b0bd008d0f0 Mon Sep 17 00:00:00 2001 From: Ryan Rossiter Date: Tue, 2 May 2017 11:02:00 -0500 Subject: [PATCH 05/13] Add check for VSIs with no permission When VSIs are not available to view because of permissions, we need to block them out of the CLI when doing a detail or interface-list. --- SoftLayer/CLI/securitygroup/detail.py | 22 +++++++++++++------- SoftLayer/CLI/securitygroup/interface.py | 26 ++++++++++++++++-------- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/SoftLayer/CLI/securitygroup/detail.py b/SoftLayer/CLI/securitygroup/detail.py index 187f4d587..ac000536c 100644 --- a/SoftLayer/CLI/securitygroup/detail.py +++ b/SoftLayer/CLI/securitygroup/detail.py @@ -46,13 +46,21 @@ def cli(env, identifier): vsi_table = formatting.Table(['id', 'hostname', 'interface', 'ipAddress']) for binding in secgroup.get('networkComponentBindings'): - vsi = binding['networkComponent']['guest'] - interface = ('PRIVATE' if binding['networkComponent']['port'] == 0 - else 'PUBLIC') - ip_address = (vsi['primaryBackendIpAddress'] - if binding['networkComponent']['port'] == 0 - else vsi['primaryIpAddress']) - vsi_table.add_row([vsi['id'], vsi['hostname'], interface, ip_address]) + try: + vsi = binding['networkComponent']['guest'] + vsi_id = vsi['id'] + hostname = vsi['hostname'] + interface = ('PRIVATE' if binding['networkComponent']['port'] == 0 + else 'PUBLIC') + ip_address = (vsi['primaryBackendIpAddress'] + if binding['networkComponent']['port'] == 0 + else vsi['primaryIpAddress']) + except KeyError: + vsi_id = "N/A" + hostname = "Not enough permission to view" + interface = "N/A" + ip_address = "N/A" + vsi_table.add_row([vsi_id, hostname, interface, ip_address]) table.add_row(['servers', vsi_table]) diff --git a/SoftLayer/CLI/securitygroup/interface.py b/SoftLayer/CLI/securitygroup/interface.py index fdf90481a..f999456f2 100644 --- a/SoftLayer/CLI/securitygroup/interface.py +++ b/SoftLayer/CLI/securitygroup/interface.py @@ -45,15 +45,25 @@ def interface_list(env, securitygroup_id, sortby): secgroup = mgr.get_securitygroup(securitygroup_id, mask=mask) for binding in secgroup.get('networkComponentBindings'): - interface = binding['networkComponent'] - vsi = interface['guest'] - priv_pub = 'PRIVATE' if interface['port'] == 0 else 'PUBLIC' - ip_address = (vsi['primaryBackendIpAddress'] if interface['port'] == 0 - else vsi['primaryIpAddress']) + interface_id = binding['networkComponentId'] + try: + interface = binding['networkComponent'] + vsi = interface['guest'] + vsi_id = vsi['id'] + hostname = vsi['hostname'] + priv_pub = 'PRIVATE' if interface['port'] == 0 else 'PUBLIC' + ip_address = (vsi['primaryBackendIpAddress'] if interface['port'] == 0 + else vsi['primaryIpAddress']) + except KeyError: + vsi_id = "N/A" + hostname = "Not enough permission to view" + priv_pub = "N/A" + ip_address = "N/A" + table.add_row([ - interface['id'], - vsi['id'], - vsi['hostname'], + interface_id, + vsi_id, + hostname, priv_pub, ip_address ]) From 7a0c4ea41716ee443b37f9d2b9d180b5deeb5fde Mon Sep 17 00:00:00 2001 From: Ryan Rossiter Date: Tue, 2 May 2017 11:14:16 -0500 Subject: [PATCH 06/13] Fix remoteGroupId in securitygroup detail The mask in the get_securitygroup() function in the manager was wrong, so fixing that allows the remoteGroupId to print in securitygroup detail. --- SoftLayer/managers/network.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SoftLayer/managers/network.py b/SoftLayer/managers/network.py index edce0e9a1..c026024ab 100644 --- a/SoftLayer/managers/network.py +++ b/SoftLayer/managers/network.py @@ -361,7 +361,7 @@ def get_securitygroup(self, group_id, **kwargs): 'id,' 'name,' 'description,' - '''rules[id, remoteIp, remoteGroup, + '''rules[id, remoteIp, remoteGroupId, direction, ethertype, portRangeMin, portRangeMax, protocol],''' '''networkComponentBindings[ From 4381b96cc7457455b0e5a3d0fe1bd497116eb1ee Mon Sep 17 00:00:00 2001 From: Ryan Rossiter Date: Tue, 2 May 2017 15:33:38 -0500 Subject: [PATCH 07/13] Fix pep8 error in interface --- SoftLayer/CLI/securitygroup/interface.py | 3 ++- tox.ini | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/SoftLayer/CLI/securitygroup/interface.py b/SoftLayer/CLI/securitygroup/interface.py index f999456f2..64b05257b 100644 --- a/SoftLayer/CLI/securitygroup/interface.py +++ b/SoftLayer/CLI/securitygroup/interface.py @@ -52,7 +52,8 @@ def interface_list(env, securitygroup_id, sortby): vsi_id = vsi['id'] hostname = vsi['hostname'] priv_pub = 'PRIVATE' if interface['port'] == 0 else 'PUBLIC' - ip_address = (vsi['primaryBackendIpAddress'] if interface['port'] == 0 + ip_address = (vsi['primaryBackendIpAddress'] + if interface['port'] == 0 else vsi['primaryIpAddress']) except KeyError: vsi_id = "N/A" diff --git a/tox.ini b/tox.ini index 53cc14dcf..bb722a01e 100644 --- a/tox.ini +++ b/tox.ini @@ -37,7 +37,7 @@ commands = -d len-as-condition \ --max-args=20 \ --max-branches=20 \ - --max-statements=60 \ + --max-statements=65 \ --min-public-methods=0 \ --min-similarity-lines=30 # --max-args=25 \ From 35f5799daedba22ba2b4747da77ab93c1edb357b Mon Sep 17 00:00:00 2001 From: Ryan Rossiter Date: Tue, 2 May 2017 15:59:22 -0500 Subject: [PATCH 08/13] Set formatting.blank() properly Because some of the .get() calls return an empty string, they're not actually getting filled with formatting.blank(). This changes the get() calls to add the or after the get() call. This way, if the returned value is empty string (so not set), we'll fill it with a formatting.blank() instead of filling it with nothing. --- SoftLayer/CLI/securitygroup/create.py | 5 +++-- SoftLayer/CLI/securitygroup/detail.py | 16 ++++++++-------- SoftLayer/CLI/securitygroup/rule.py | 12 ++++++------ SoftLayer/CLI/virt/detail.py | 3 ++- 4 files changed, 19 insertions(+), 17 deletions(-) diff --git a/SoftLayer/CLI/securitygroup/create.py b/SoftLayer/CLI/securitygroup/create.py index 6d759e2eb..5fc599471 100644 --- a/SoftLayer/CLI/securitygroup/create.py +++ b/SoftLayer/CLI/securitygroup/create.py @@ -23,9 +23,10 @@ def cli(env, name, description): table.align['name'] = 'r' table.align['value'] = 'l' table.add_row(['id', result['id']]) - table.add_row(['name', result.get('name', formatting.blank())]) + table.add_row(['name', + result.get('name') or formatting.blank()]) table.add_row(['description', - result.get('description', formatting.blank())]) + result.get('description') or formatting.blank()]) table.add_row(['created', result['createDate']]) env.fout(table) diff --git a/SoftLayer/CLI/securitygroup/detail.py b/SoftLayer/CLI/securitygroup/detail.py index ac000536c..ca287c937 100644 --- a/SoftLayer/CLI/securitygroup/detail.py +++ b/SoftLayer/CLI/securitygroup/detail.py @@ -23,23 +23,23 @@ def cli(env, identifier): table.align['value'] = 'l' table.add_row(['id', secgroup['id']]) - table.add_row(['name', secgroup.get('name', formatting.blank())]) + table.add_row(['name', secgroup.get('name') or formatting.blank()]) table.add_row(['description', - secgroup.get('description', formatting.blank())]) + secgroup.get('description') or formatting.blank()]) rule_table = formatting.Table(['id', 'remoteIp', 'remoteGroupId', 'direction', 'ethertype', 'portRangeMin', 'portRangeMax', 'protocol']) for rule in secgroup.get('rules', []): - rg_id = rule.get('remoteGroup', {}).get('id', formatting.blank()) + rg_id = rule.get('remoteGroup', {}).get('id') or formatting.blank() rule_table.add_row([rule['id'], - rule.get('remoteIp', formatting.blank()), + rule.get('remoteIp') or formatting.blank(), rule.get('remoteGroupId', rg_id), rule['direction'], - rule.get('ethertype', formatting.blank()), - rule.get('portRangeMin', formatting.blank()), - rule.get('portRangeMax', formatting.blank()), - rule.get('protocol', formatting.blank())]) + rule.get('ethertype') or formatting.blank(), + rule.get('portRangeMin') or formatting.blank(), + rule.get('portRangeMax') or formatting.blank(), + rule.get('protocol') or formatting.blank()]) table.add_row(['rules', rule_table]) diff --git a/SoftLayer/CLI/securitygroup/rule.py b/SoftLayer/CLI/securitygroup/rule.py index f8f692ec3..983537719 100644 --- a/SoftLayer/CLI/securitygroup/rule.py +++ b/SoftLayer/CLI/securitygroup/rule.py @@ -36,13 +36,13 @@ def rule_list(env, securitygroup_id, sortby): for rule in rules: table.add_row([ rule['id'], - rule.get('remoteIp', formatting.blank()), - rule.get('remoteGroupId', formatting.blank()), + rule.get('remoteIp') or formatting.blank(), + rule.get('remoteGroupId') or formatting.blank(), rule['direction'], - rule.get('ethertype', formatting.blank()), - rule.get('portRangeMin', formatting.blank()), - rule.get('portRangeMax', formatting.blank()), - rule.get('protocol', formatting.blank()) + rule.get('ethertype') or formatting.blank(), + rule.get('portRangeMin') or formatting.blank(), + rule.get('portRangeMax') or formatting.blank(), + rule.get('protocol') or formatting.blank() ]) env.fout(table) diff --git a/SoftLayer/CLI/virt/detail.py b/SoftLayer/CLI/virt/detail.py index 3ade65b50..baa2b8553 100644 --- a/SoftLayer/CLI/virt/detail.py +++ b/SoftLayer/CLI/virt/detail.py @@ -86,7 +86,8 @@ def cli(env, identifier, passwords=False, price=False): has_secgroups = True secgroup = binding['securityGroup'] secgroup_table.add_row([ - interface, secgroup['id'], secgroup['name']]) + interface, secgroup['id'], + secgroup.get('name') or formatting.blank()]) if has_secgroups: table.add_row(['security_groups', secgroup_table]) From 509ddd458a97acd11e86f4a00e52cf2d65c68f09 Mon Sep 17 00:00:00 2001 From: Ryan Rossiter Date: Thu, 4 May 2017 14:01:12 -0500 Subject: [PATCH 09/13] Add tests for securitygroup --- SoftLayer/CLI/securitygroup/delete.py | 4 +- SoftLayer/CLI/securitygroup/detail.py | 2 +- SoftLayer/CLI/securitygroup/interface.py | 13 +- SoftLayer/CLI/securitygroup/rule.py | 14 +- .../SoftLayer_Network_SecurityGroup.py | 46 ++++ SoftLayer/managers/network.py | 36 +-- tests/CLI/modules/securitygroup_tests.py | 233 ++++++++++++++++++ tests/managers/network_tests.py | 142 +++++++++++ 8 files changed, 465 insertions(+), 25 deletions(-) create mode 100644 SoftLayer/fixtures/SoftLayer_Network_SecurityGroup.py create mode 100644 tests/CLI/modules/securitygroup_tests.py diff --git a/SoftLayer/CLI/securitygroup/delete.py b/SoftLayer/CLI/securitygroup/delete.py index dfd3080e2..4cbca3e7d 100644 --- a/SoftLayer/CLI/securitygroup/delete.py +++ b/SoftLayer/CLI/securitygroup/delete.py @@ -4,6 +4,7 @@ import click import SoftLayer from SoftLayer.CLI import environment +from SoftLayer.CLI import exceptions @click.command() @@ -12,4 +13,5 @@ def cli(env, securitygroup_id): """Deletes the given security group""" mgr = SoftLayer.NetworkManager(env.client) - mgr.delete_securitygroup(securitygroup_id) + if not mgr.delete_securitygroup(securitygroup_id): + raise exceptions.CLIAbort("Failed to delete security group") diff --git a/SoftLayer/CLI/securitygroup/detail.py b/SoftLayer/CLI/securitygroup/detail.py index ca287c937..97eda9dbc 100644 --- a/SoftLayer/CLI/securitygroup/detail.py +++ b/SoftLayer/CLI/securitygroup/detail.py @@ -45,7 +45,7 @@ def cli(env, identifier): vsi_table = formatting.Table(['id', 'hostname', 'interface', 'ipAddress']) - for binding in secgroup.get('networkComponentBindings'): + for binding in secgroup.get('networkComponentBindings', []): try: vsi = binding['networkComponent']['guest'] vsi_id = vsi['id'] diff --git a/SoftLayer/CLI/securitygroup/interface.py b/SoftLayer/CLI/securitygroup/interface.py index 64b05257b..a2f33ee87 100644 --- a/SoftLayer/CLI/securitygroup/interface.py +++ b/SoftLayer/CLI/securitygroup/interface.py @@ -30,6 +30,7 @@ def interface_list(env, securitygroup_id, sortby): mask = ( '''networkComponentBindings[ + networkComponentId, networkComponent[ id, port, @@ -44,7 +45,7 @@ def interface_list(env, securitygroup_id, sortby): ) secgroup = mgr.get_securitygroup(securitygroup_id, mask=mask) - for binding in secgroup.get('networkComponentBindings'): + for binding in secgroup.get('networkComponentBindings', []): interface_id = binding['networkComponentId'] try: interface = binding['networkComponent'] @@ -89,7 +90,10 @@ def add(env, securitygroup_id, network_component, server, interface): mgr = SoftLayer.NetworkManager(env.client) component_id = _get_component_id(env, network_component, server, interface) - mgr.attach_securitygroup_component(securitygroup_id, component_id) + success = mgr.attach_securitygroup_component(securitygroup_id, + component_id) + if not success: + raise exceptions.CLIAbort("Could not attach network component") @click.command() @@ -109,7 +113,10 @@ def remove(env, securitygroup_id, network_component, server, interface): mgr = SoftLayer.NetworkManager(env.client) component_id = _get_component_id(env, network_component, server, interface) - mgr.detach_securitygroup_component(securitygroup_id, component_id) + success = mgr.detach_securitygroup_component(securitygroup_id, + component_id) + if not success: + raise exceptions.CLIAbort("Could not detach network component") def _validate_args(network_component, server, interface): diff --git a/SoftLayer/CLI/securitygroup/rule.py b/SoftLayer/CLI/securitygroup/rule.py index 983537719..b97b7193f 100644 --- a/SoftLayer/CLI/securitygroup/rule.py +++ b/SoftLayer/CLI/securitygroup/rule.py @@ -12,7 +12,7 @@ 'remoteIp', 'remoteGroupId', 'direction', - 'etherype', + 'ethertype', 'portRangeMin', 'portRangeMax', 'protocol'] @@ -71,9 +71,12 @@ def add(env, securitygroup_id, remote_ip, remote_group, """Add a security group rule to a security group.""" mgr = SoftLayer.NetworkManager(env.client) - mgr.add_securitygroup_rule(securitygroup_id, remote_ip, remote_group, - direction, ethertype, port_range_max, - port_range_min, protocol) + ret = mgr.add_securitygroup_rule(securitygroup_id, remote_ip, remote_group, + direction, ethertype, port_range_max, + port_range_min, protocol) + + if not ret: + raise exceptions.CLIAbort("Failed to add security group rule") @click.command() @@ -126,4 +129,5 @@ def edit(env, securitygroup_id, rule_id, remote_ip, remote_group, def remove(env, securitygroup_id, rule_id): """Remove a rule from a security group.""" mgr = SoftLayer.NetworkManager(env.client) - mgr.remove_securitygroup_rule(securitygroup_id, rule_id) + if not mgr.remove_securitygroup_rule(securitygroup_id, rule_id): + raise exceptions.CLIAbort("Failed to remove security group rule") diff --git a/SoftLayer/fixtures/SoftLayer_Network_SecurityGroup.py b/SoftLayer/fixtures/SoftLayer_Network_SecurityGroup.py new file mode 100644 index 000000000..7e79560f7 --- /dev/null +++ b/SoftLayer/fixtures/SoftLayer_Network_SecurityGroup.py @@ -0,0 +1,46 @@ +getAllObjects = [ + {'id': 100, + 'name': 'secgroup1', + 'description': 'Securitygroup1'}, + {'id': 104, + 'name': 'secgroup2'}, + {'id': 110} +] + +getRules = [ + {'id': 100, + 'direction': 'egress', + 'ethertype': 'IPv4'} +] + +guest_dict = {'id': 5000, + 'hostname': 'test', + 'primaryBackendIpAddress': '10.3.4.5', + 'primaryIpAddress': '169.23.123.43'} + +getObject = { + 'id': 100, + 'name': 'secgroup1', + 'description': 'Securitygroup1', + 'networkComponentBindings': [{'networkComponentId': 1000, + 'networkComponent': {'id': 1000, + 'port': 0, + 'guest': guest_dict}}, + {'networkComponentId': 1001, + 'networkComponent': {'id': 1001, + 'port': 1, + 'guest': guest_dict}}], + 'rules': getRules +} + +createObjects = [{'id': 100, + 'name': 'secgroup1', + 'description': 'Securitygroup1', + 'createDate': '2017-05-05T12:44:43-06:00'}] +editObjects = True +deleteObjects = True +addRules = True +editRules = True +removeRules = True +attachNetworkComponents = True +detachNetworkComponents = True diff --git a/SoftLayer/managers/network.py b/SoftLayer/managers/network.py index c026024ab..6563fb811 100644 --- a/SoftLayer/managers/network.py +++ b/SoftLayer/managers/network.py @@ -79,11 +79,15 @@ def add_securitygroup_rule(self, group_id, remote_ip=None, :param int port_min: The lower port bound to enforce :param str protocol: The protocol to enforce (icmp, udp, tcp) """ - rule = {'direction': direction, - 'ethertype': ethertype, - 'portRangeMax': port_max, - 'portRangeMin': port_min, - 'protocol': protocol} + rule = {'direction': direction} + if ethertype is not None: + rule['ethertype'] = ethertype + if port_max is not None: + rule['portRangeMax'] = port_max + if port_min is not None: + rule['portRangeMin'] = port_min + if protocol is not None: + rule['protocol'] = protocol if remote_ip is not None: rule['remoteIp'] = remote_ip if remote_group is not None: @@ -228,7 +232,7 @@ def delete_securitygroup(self, group_id): :param int group_id: The ID of the security group """ delete_dict = {'id': group_id} - self.security_group.deleteObjects([delete_dict]) + return self.security_group.deleteObjects([delete_dict]) def detach_securitygroup_component(self, group_id, component_id): """Detaches a network component from a security group. @@ -236,7 +240,7 @@ def detach_securitygroup_component(self, group_id, component_id): :param int group_id: The ID of the security group :param int component_id: The ID of the component to detach """ - self.detach_securitygroup_components(group_id, [component_id]) + return self.detach_securitygroup_components(group_id, [component_id]) def detach_securitygroup_components(self, group_id, component_ids): """Detaches network components from a security group. @@ -244,7 +248,8 @@ def detach_securitygroup_components(self, group_id, component_ids): :param int group_id: The ID of the security group :param list component_ids: The IDs of the network components to detach """ - self.security_group.detachNetworkComponents(component_ids, id=group_id) + return self.security_group.detachNetworkComponents(component_ids, + id=group_id) def edit_rwhois(self, abuse_email=None, address1=None, address2=None, city=None, company_name=None, country=None, @@ -281,6 +286,7 @@ def edit_securitygroup(self, group_id, name=None, description=None): :param string name: The name of the security group :param string description: The description of the security group """ + successful = False obj = {} if name: obj['name'] = name @@ -289,9 +295,9 @@ def edit_securitygroup(self, group_id, name=None, description=None): if obj: obj['id'] = group_id - self.security_group.editObjects([obj]) + successful = self.security_group.editObjects([obj]) - return bool(name or description) + return successful def edit_securitygroup_rule(self, group_id, rule_id, remote_ip=None, remote_group=None, direction=None, @@ -310,6 +316,7 @@ def edit_securitygroup_rule(self, group_id, rule_id, remote_ip=None, :param str port_range_min: The lower port bound to enforce :param str protocol: The protocol to enforce (icmp, udp, tcp) """ + successful = False obj = {} if remote_ip: obj['remoteIp'] = remote_ip @@ -328,10 +335,9 @@ def edit_securitygroup_rule(self, group_id, rule_id, remote_ip=None, if obj: obj['id'] = rule_id - self.security_group.editRules([obj], id=group_id) + successful = self.security_group.editRules([obj], id=group_id) - return bool(remote_ip or remote_group or direction or ethertype - or port_range_max or port_range_min or protocol) + return successful def ip_lookup(self, ip_address): """Looks up an IP address and returns network information about it. @@ -524,7 +530,7 @@ def remove_securitygroup_rule(self, group_id, rule_id): :param int group_id: The ID of the security group :param int rule_id: The ID of the rule to remove """ - self.remove_securitygroup_rules(group_id, [rule_id]) + return self.remove_securitygroup_rules(group_id, [rule_id]) def remove_securitygroup_rules(self, group_id, rules): """Remove rules from a security group. @@ -532,7 +538,7 @@ def remove_securitygroup_rules(self, group_id, rules): :param int group_id: The ID of the security group :param list rules: The list of IDs to remove """ - self.security_group.removeRules(rules, id=group_id) + return self.security_group.removeRules(rules, id=group_id) def resolve_global_ip_ids(self, identifier): """Resolve global ip ids.""" diff --git a/tests/CLI/modules/securitygroup_tests.py b/tests/CLI/modules/securitygroup_tests.py new file mode 100644 index 000000000..b234941d6 --- /dev/null +++ b/tests/CLI/modules/securitygroup_tests.py @@ -0,0 +1,233 @@ +""" + SoftLayer.tests.CLI.modules.securitygroup_tests + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + :license: MIT, see LICENSE for more details. +""" +import json + +from SoftLayer import testing + + +class SecurityGroupTests(testing.TestCase): + def test_list_securitygroup(self): + result = self.run_command(['sg', 'list']) + + self.assert_no_fail(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'getAllObjects') + self.assertEqual([{'id': 100, + 'name': 'secgroup1', + 'description': 'Securitygroup1'}, + {'id': 104, + 'name': 'secgroup2', + 'description': None}, + {'id': 110, + 'name': None, + 'description': None}], + json.loads(result.output)) + + def test_securitygroup_detail(self): + result = self.run_command(['sg', 'detail', '100']) + + self.assert_no_fail(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'getObject', identifier='100') + + priv_server_dict = {'id': 5000, 'hostname': 'test', + 'interface': 'PRIVATE', + 'ipAddress': '10.3.4.5'} + pub_server_dict = {'id': 5000, 'hostname': 'test', + 'interface': 'PUBLIC', + 'ipAddress': '169.23.123.43'} + self.assertEqual({'id': 100, + 'name': 'secgroup1', + 'description': 'Securitygroup1', + 'rules': [{'id': 100, + 'direction': 'egress', + 'ethertype': 'IPv4', + 'remoteIp': None, + 'remoteGroupId': None, + 'protocol': None, + 'portRangeMin': None, + 'portRangeMax': None}], + 'servers': [priv_server_dict, + pub_server_dict]}, + json.loads(result.output)) + + def test_securitygroup_create(self): + result = self.run_command(['sg', 'create', '--name=secgroup1', + '--description=Securitygroup1']) + + self.assert_no_fail(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'createObjects', + args=([{'name': 'secgroup1', + 'description': 'Securitygroup1'}],)) + self.assertEqual({'id': 100, + 'name': 'secgroup1', + 'description': 'Securitygroup1', + 'created': '2017-05-05T12:44:43-06:00'}, + json.loads(result.output)) + + def test_securitygroup_edit(self): + result = self.run_command(['sg', 'edit', '104', '--name=foo']) + + self.assert_no_fail(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'editObjects', + args=([{'id': '104', 'name': 'foo'}],)) + + def test_securitygroup_edit_fail(self): + fixture = self.set_mock('SoftLayer_Network_SecurityGroup', + 'editObjects') + fixture.return_value = False + + result = self.run_command(['sg', 'edit', '100', + '--name=foo']) + + self.assertEqual(result.exit_code, 2) + + def test_securitygroup_delete(self): + result = self.run_command(['sg', 'delete', '104']) + + self.assert_no_fail(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'deleteObjects', + args=([{'id': '104'}],)) + + def test_securitygroup_delete_fail(self): + fixture = self.set_mock('SoftLayer_Network_SecurityGroup', + 'deleteObjects') + fixture.return_value = False + + result = self.run_command(['sg', 'delete', '100']) + + self.assertEqual(result.exit_code, 2) + + def test_securitygroup_rule_list(self): + result = self.run_command(['sg', 'rule-list', '100']) + + self.assert_no_fail(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'getRules', identifier='100') + self.assertEqual([{'id': 100, + 'direction': 'egress', + 'ethertype': 'IPv4', + 'remoteIp': None, + 'remoteGroupId': None, + 'protocol': None, + 'portRangeMin': None, + 'portRangeMax': None}], + json.loads(result.output)) + + def test_securitygroup_rule_add(self): + result = self.run_command(['sg', 'rule-add', '100', + '--direction=ingress']) + + self.assert_no_fail(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', 'addRules', + identifier='100', + args=([{'direction': 'ingress'}],)) + + def test_securitygroup_rule_add_fail(self): + fixture = self.set_mock('SoftLayer_Network_SecurityGroup', 'addRules') + fixture.return_value = False + + result = self.run_command(['sg', 'rule-add', '100', + '--direction=ingress']) + + self.assertEqual(result.exit_code, 2) + + def test_securitygroup_rule_edit(self): + result = self.run_command(['sg', 'rule-edit', '100', + '520', '--direction=ingress']) + + self.assert_no_fail(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'editRules', identifier='100', + args=([{'id': '520', + 'direction': 'ingress'}],)) + + def test_securitygroup_rule_edit_fail(self): + fixture = self.set_mock('SoftLayer_Network_SecurityGroup', 'editRules') + fixture.return_value = False + + result = self.run_command(['sg', 'rule-edit', '100', + '520', '--direction=ingress']) + + self.assertEqual(result.exit_code, 2) + + def test_securitygroup_rule_remove(self): + result = self.run_command(['sg', 'rule-remove', '100', '520']) + + self.assert_no_fail(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'removeRules', identifier='100', + args=(['520'],)) + + def test_securitygroup_rule_remove_fail(self): + fixture = self.set_mock('SoftLayer_Network_SecurityGroup', + 'removeRules') + fixture.return_value = False + + result = self.run_command(['sg', 'rule-remove', '100', '520']) + + self.assertEqual(result.exit_code, 2) + + def test_securitygroup_interface_list(self): + result = self.run_command(['sg', 'interface-list', '100']) + + self.assert_no_fail(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'getObject', identifier='100') + self.assertEqual([{'hostname': 'test', + 'interface': 'PRIVATE', + 'ipAddress': '10.3.4.5', + 'networkComponentId': 1000, + 'virtualServerId': 5000}, + {'hostname': 'test', + 'interface': 'PUBLIC', + 'ipAddress': '169.23.123.43', + 'networkComponentId': 1001, + 'virtualServerId': 5000}], + json.loads(result.output)) + + def test_securitygroup_interface_add(self): + result = self.run_command(['sg', 'interface-add', '100', + '--network-component=1000']) + + self.assert_no_fail(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'attachNetworkComponents', + identifier='100', + args=(['1000'],)) + + def test_securitygroup_interface_add_fail(self): + fixture = self.set_mock('SoftLayer_Network_SecurityGroup', + 'attachNetworkComponents') + fixture.return_value = False + + result = self.run_command(['sg', 'interface-add', '100', + '--network-component=500']) + + self.assertEqual(result.exit_code, 2) + + def test_securitygroup_interface_remove(self): + result = self.run_command(['sg', 'interface-remove', '100', + '--network-component=500']) + + self.assert_no_fail(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'detachNetworkComponents', + identifier='100', + args=(['500'],)) + + def test_securitygroup_interface_remove_fail(self): + fixture = self.set_mock('SoftLayer_Network_SecurityGroup', + 'detachNetworkComponents') + fixture.return_value = False + + result = self.run_command(['sg', 'interface-remove', '100', + '--network-component=500']) + + self.assertEqual(result.exit_code, 2) diff --git a/tests/managers/network_tests.py b/tests/managers/network_tests.py index b62a656a0..5ce06f0da 100644 --- a/tests/managers/network_tests.py +++ b/tests/managers/network_tests.py @@ -33,6 +33,43 @@ def test_add_global_ip(self): self.assertEqual(fixtures.SoftLayer_Product_Order.verifyOrder, result) + def test_add_securitygroup_rule(self): + result = self.network.add_securitygroup_rule(100, + remote_ip='10.0.0.0/24', + direction='ingress', + ethertype='IPv4', + port_min=95, + port_max=100, + protocol='tcp') + + self.assertTrue(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'addRules', identifier=100, + args=([{'remoteIp': '10.0.0.0/24', + 'direction': 'ingress', + 'ethertype': 'IPv4', + 'portRangeMin': 95, + 'portRangeMax': 100, + 'protocol': 'tcp'}],)) + + def test_add_securitygroup_rules(self): + rule1 = {'remoteIp': '10.0.0.0/24', + 'direction': 'ingress', + 'ethertype': 'IPv4', + 'portRangeMin': 95, + 'portRangeMax': 100, + 'protocol': 'tcp'} + rule2 = {'remoteGroupId': 102, + 'direction': 'egress', + 'ethertype': 'IPv4'} + + result = self.network.add_securitygroup_rules(100, [rule1, rule2]) + + self.assertTrue(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'addRules', identifier=100, + args=([rule1, rule2],)) + def test_add_subnet_for_ipv4(self): # Test a four public address IPv4 order result = self.network.add_subnet('public', @@ -82,6 +119,24 @@ def test_assign_global_ip(self): identifier=9876, args=('172.16.24.76',)) + def test_attach_securitygroup_component(self): + result = self.network.attach_securitygroup_component(100, 500) + + self.assertTrue(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'attachNetworkComponents', + identifier=100, + args=([500],)) + + def test_attach_securitygroup_components(self): + result = self.network.attach_securitygroup_components(100, [500, 600]) + + self.assertTrue(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'attachNetworkComponents', + identifier=100, + args=([500, 600],)) + def test_cancel_global_ip(self): result = self.network.cancel_global_ip(1234) @@ -96,6 +151,38 @@ def test_cancel_subnet(self): self.assert_called_with('SoftLayer_Billing_Item', 'cancelService', identifier=1056) + def test_create_securitygroup(self): + result = self.network.create_securitygroup(name='foo', + description='bar') + + sg_fixture = fixtures.SoftLayer_Network_SecurityGroup + self.assertEqual(sg_fixture.createObjects, [result]) + + def test_delete_securitygroup(self): + result = self.network.delete_securitygroup(100) + + self.assertTrue(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'deleteObjects', + args=([{'id': 100}],)) + + def test_detach_securitygroup_component(self): + result = self.network.detach_securitygroup_component(100, 500) + + self.assertTrue(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'detachNetworkComponents', + identifier=100, args=([500],)) + + def test_detach_securitygroup_components(self): + result = self.network.detach_securitygroup_components(100, + [500, 600]) + + self.assertTrue(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'detachNetworkComponents', + identifier=100, args=([500, 600],)) + def test_edit_rwhois(self): result = self.network.edit_rwhois( abuse_email='abuse@test.foo', @@ -129,12 +216,37 @@ def test_edit_rwhois(self): identifier='id', args=(expected,)) + def test_edit_securitygroup(self): + result = self.network.edit_securitygroup(100, name='foobar') + + self.assertTrue(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'editObjects', + args=([{'id': 100, + 'name': 'foobar'}],)) + + def test_edit_securitygroup_rule(self): + result = self.network.edit_securitygroup_rule(100, 500, + direction='ingress') + + self.assertTrue(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'editRules', identifier=100, + args=([{'id': 500, + 'direction': 'ingress'}],)) + def test_get_rwhois(self): result = self.network.get_rwhois() self.assertEqual(result, fixtures.SoftLayer_Account.getRwhoisData) self.assert_called_with('SoftLayer_Account', 'getRwhoisData') + def test_get_securitygroup(self): + result = self.network.get_securitygroup(100) + + sg_fixture = fixtures.SoftLayer_Network_SecurityGroup + self.assertEqual(sg_fixture.getObject, result) + def test_get_subnet(self): result = self.network.get_subnet(9876) @@ -240,6 +352,36 @@ def test_list_vlans_with_filters(self): self.assert_called_with('SoftLayer_Account', 'getNetworkVlans', filter=_filter) + def test_list_securitygroups(self): + result = self.network.list_securitygroups() + + sg_fixture = fixtures.SoftLayer_Network_SecurityGroup + self.assertEqual(sg_fixture.getAllObjects, result) + + def test_list_securitygroup_rules(self): + result = self.network.list_securitygroup_rules(100) + + sg_fixture = fixtures.SoftLayer_Network_SecurityGroup + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'getRules', identifier=100) + self.assertEqual(sg_fixture.getRules, result) + + def test_remove_securitygroup_rule(self): + result = self.network.remove_securitygroup_rule(100, 500) + + self.assertTrue(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'removeRules', identifier=100, + args=([500],)) + + def test_remove_securitygroup_rules(self): + result = self.network.remove_securitygroup_rules(100, [500, 600]) + + self.assertTrue(result) + self.assert_called_with('SoftLayer_Network_SecurityGroup', + 'removeRules', identifier=100, + args=([500, 600],)) + def test_summary_by_datacenter(self): result = self.network.summary_by_datacenter() From 4d9f94bae99c33202a56659de0c66034939cf37d Mon Sep 17 00:00:00 2001 From: Ryan Rossiter Date: Wed, 10 May 2017 09:00:51 -0500 Subject: [PATCH 10/13] Fix displaying zeros --- SoftLayer/CLI/securitygroup/detail.py | 10 ++++++++-- SoftLayer/CLI/securitygroup/rule.py | 15 +++++++++++---- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/SoftLayer/CLI/securitygroup/detail.py b/SoftLayer/CLI/securitygroup/detail.py index 97eda9dbc..4a3381ffa 100644 --- a/SoftLayer/CLI/securitygroup/detail.py +++ b/SoftLayer/CLI/securitygroup/detail.py @@ -32,13 +32,19 @@ def cli(env, identifier): 'portRangeMax', 'protocol']) for rule in secgroup.get('rules', []): rg_id = rule.get('remoteGroup', {}).get('id') or formatting.blank() + port_min = rule.get('portRangeMin') + port_max = rule.get('portRangeMax') + if port_min is None: + port_min = formatting.blank() + if port_max is None: + port_max = formatting.blank() rule_table.add_row([rule['id'], rule.get('remoteIp') or formatting.blank(), rule.get('remoteGroupId', rg_id), rule['direction'], rule.get('ethertype') or formatting.blank(), - rule.get('portRangeMin') or formatting.blank(), - rule.get('portRangeMax') or formatting.blank(), + port_min, + port_max, rule.get('protocol') or formatting.blank()]) table.add_row(['rules', rule_table]) diff --git a/SoftLayer/CLI/securitygroup/rule.py b/SoftLayer/CLI/securitygroup/rule.py index b97b7193f..d837ad052 100644 --- a/SoftLayer/CLI/securitygroup/rule.py +++ b/SoftLayer/CLI/securitygroup/rule.py @@ -34,14 +34,21 @@ def rule_list(env, securitygroup_id, sortby): rules = mgr.list_securitygroup_rules(securitygroup_id) for rule in rules: + port_min = rule.get('portRangeMin') + port_max = rule.get('portRangeMax') + if port_min is None: + port_min = formatting.blank() + if port_max is None: + port_max = formatting.blank() + table.add_row([ rule['id'], rule.get('remoteIp') or formatting.blank(), rule.get('remoteGroupId') or formatting.blank(), rule['direction'], rule.get('ethertype') or formatting.blank(), - rule.get('portRangeMin') or formatting.blank(), - rule.get('portRangeMax') or formatting.blank(), + port_min, + port_max, rule.get('protocol') or formatting.blank() ]) @@ -111,9 +118,9 @@ def edit(env, securitygroup_id, rule_id, remote_ip, remote_group, data['direction'] = direction if ethertype: data['ethertype'] = ethertype - if port_range_max: + if port_range_max is not None: data['port_range_max'] = port_range_max - if port_range_min: + if port_range_min is not None: data['port_range_min'] = port_range_min if protocol: data['protocol'] = protocol From 7e036d0045a43f1c6c03f1bed1d16a7be1f0cfcb Mon Sep 17 00:00:00 2001 From: Ryan Rossiter Date: Wed, 31 May 2017 10:15:19 -0500 Subject: [PATCH 11/13] Change port_range_* to port_* Being consistent is a good thing. --- SoftLayer/CLI/securitygroup/rule.py | 24 ++++++++++++------------ SoftLayer/managers/network.py | 16 ++++++++-------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/SoftLayer/CLI/securitygroup/rule.py b/SoftLayer/CLI/securitygroup/rule.py index d837ad052..dfbc93ed9 100644 --- a/SoftLayer/CLI/securitygroup/rule.py +++ b/SoftLayer/CLI/securitygroup/rule.py @@ -66,21 +66,21 @@ def rule_list(env, securitygroup_id, sortby): '(ingress, egress)')) @click.option('--ethertype', '-e', help='The ethertype (IPv4 or IPv6) to enforce') -@click.option('--port-range-max', '-M', type=click.INT, +@click.option('--port-max', '-M', type=click.INT, help='The upper port bound to enforce') -@click.option('--port-range-min', '-m', type=click.INT, +@click.option('--port-min', '-m', type=click.INT, help='The lower port bound to enforce') @click.option('--protocol', '-p', help='The protocol (icmp, tcp, udp) to enforce') @environment.pass_env def add(env, securitygroup_id, remote_ip, remote_group, - direction, ethertype, port_range_max, port_range_min, protocol): + direction, ethertype, port_max, port_min, protocol): """Add a security group rule to a security group.""" mgr = SoftLayer.NetworkManager(env.client) ret = mgr.add_securitygroup_rule(securitygroup_id, remote_ip, remote_group, - direction, ethertype, port_range_max, - port_range_min, protocol) + direction, ethertype, port_max, + port_min, protocol) if not ret: raise exceptions.CLIAbort("Failed to add security group rule") @@ -97,15 +97,15 @@ def add(env, securitygroup_id, remote_ip, remote_group, help='The direction of traffic to enforce') @click.option('--ethertype', '-e', help='The ethertype (IPv4 or IPv6) to enforce') -@click.option('--port-range-max', '-M', +@click.option('--port-max', '-M', help='The upper port bound to enforce') -@click.option('--port-range-min', '-m', +@click.option('--port-min', '-m', help='The lower port bound to enforce') @click.option('--protocol', '-p', help='The protocol (icmp, tcp, udp) to enforce') @environment.pass_env def edit(env, securitygroup_id, rule_id, remote_ip, remote_group, - direction, ethertype, port_range_max, port_range_min, protocol): + direction, ethertype, port_max, port_min, protocol): """Edit a security group rule in a security group.""" mgr = SoftLayer.NetworkManager(env.client) @@ -118,10 +118,10 @@ def edit(env, securitygroup_id, rule_id, remote_ip, remote_group, data['direction'] = direction if ethertype: data['ethertype'] = ethertype - if port_range_max is not None: - data['port_range_max'] = port_range_max - if port_range_min is not None: - data['port_range_min'] = port_range_min + if port_max is not None: + data['port_max'] = port_max + if port_min is not None: + data['port_min'] = port_min if protocol: data['protocol'] = protocol diff --git a/SoftLayer/managers/network.py b/SoftLayer/managers/network.py index 6563fb811..8e7e232bf 100644 --- a/SoftLayer/managers/network.py +++ b/SoftLayer/managers/network.py @@ -301,8 +301,8 @@ def edit_securitygroup(self, group_id, name=None, description=None): def edit_securitygroup_rule(self, group_id, rule_id, remote_ip=None, remote_group=None, direction=None, - ethertype=None, port_range_max=None, - port_range_min=None, protocol=None): + ethertype=None, port_max=None, + port_min=None, protocol=None): """Edit a security group rule. :param int group_id: The ID of the security group the rule belongs to @@ -312,8 +312,8 @@ def edit_securitygroup_rule(self, group_id, rule_id, remote_ip=None, the rule on :param str direction: The direction to enforce (egress or ingress) :param str ethertype: The ethertype to enforce (IPv4 or IPv6) - :param str port_range_max: The upper port bound to enforce - :param str port_range_min: The lower port bound to enforce + :param str port_max: The upper port bound to enforce + :param str port_min: The lower port bound to enforce :param str protocol: The protocol to enforce (icmp, udp, tcp) """ successful = False @@ -326,10 +326,10 @@ def edit_securitygroup_rule(self, group_id, rule_id, remote_ip=None, obj['direction'] = direction if ethertype: obj['ethertype'] = ethertype - if port_range_max: - obj['portRangeMax'] = port_range_max - if port_range_min: - obj['portRangeMin'] = port_range_min + if port_max: + obj['portRangeMax'] = port_max + if port_min: + obj['portRangeMin'] = port_min if protocol: obj['protocol'] = protocol From a7d8aba66075a911ba57b2bae6a942291bab2c8f Mon Sep 17 00:00:00 2001 From: Ryan Rossiter Date: Fri, 28 Jul 2017 10:30:54 -0500 Subject: [PATCH 12/13] Add merged changes to tox.ini --- tox.ini | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tox.ini b/tox.ini index bb722a01e..62f1fe67d 100644 --- a/tox.ini +++ b/tox.ini @@ -35,17 +35,12 @@ commands = -d locally-disabled \ -d no-else-return \ -d len-as-condition \ - --max-args=20 \ + --max-args=25 \ --max-branches=20 \ --max-statements=65 \ --min-public-methods=0 \ + --max-public-methods=35 \ --min-similarity-lines=30 -# --max-args=25 \ -# --max-branches=20 \ -# --max-statements=60 \ -# --min-public-methods=0 \ -# --max-public-methods=50 \ -# --min-similarity-lines=30 # invalid-name - Fixtures don't follow proper naming conventions # missing-docstring - Fixtures don't have docstrings From d0ba59b8797237fcd63499d1b46625afd96b65ab Mon Sep 17 00:00:00 2001 From: Ryan Rossiter Date: Wed, 9 Aug 2017 13:06:52 -0500 Subject: [PATCH 13/13] Add type check in network manager If a dict is passed in to the add_securitygroup_rules function, it is serialized as an iterable and sent over XMLRPC. This now does a type check to make sure it's a list before sending it to be serialized. --- SoftLayer/managers/network.py | 2 ++ tests/managers/network_tests.py | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/SoftLayer/managers/network.py b/SoftLayer/managers/network.py index 8e7e232bf..0e44d7b38 100644 --- a/SoftLayer/managers/network.py +++ b/SoftLayer/managers/network.py @@ -100,6 +100,8 @@ def add_securitygroup_rules(self, group_id, rules): :param int group_id: The ID of the security group to add the rules to :param list rules: The list of rule dictionaries to add """ + if not isinstance(rules, list): + raise TypeError("The rules provided must be a list of dictionaries") return self.security_group.addRules(rules, id=group_id) def add_subnet(self, subnet_type, quantity=None, vlan_id=None, version=4, diff --git a/tests/managers/network_tests.py b/tests/managers/network_tests.py index 5ce06f0da..47c76948f 100644 --- a/tests/managers/network_tests.py +++ b/tests/managers/network_tests.py @@ -70,6 +70,12 @@ def test_add_securitygroup_rules(self): 'addRules', identifier=100, args=([rule1, rule2],)) + def test_add_securitygroup_rules_with_dict_error(self): + rule = {'remoteIp': '10.0.0.0/24', + 'direction': 'ingress'} + self.assertRaises(TypeError, self.network.add_securitygroup_rules, + rule) + def test_add_subnet_for_ipv4(self): # Test a four public address IPv4 order result = self.network.add_subnet('public',