From 84699dc524e57ac40de2b5eca5a92cfcee6cbc01 Mon Sep 17 00:00:00 2001 From: lionel Date: Tue, 15 Sep 2015 23:39:15 +0200 Subject: [PATCH 1/3] [LIBCLOUD-749] CloudStack: fixed method ex_authorize_security_group_ingress --- libcloud/compute/drivers/cloudstack.py | 120 ++++++++++++------ .../cloudstack/queryAsyncJobResult_17200.json | 2 +- libcloud/test/compute/test_cloudstack.py | 20 ++- 3 files changed, 93 insertions(+), 49 deletions(-) diff --git a/libcloud/compute/drivers/cloudstack.py b/libcloud/compute/drivers/cloudstack.py index 1c77b3b301..4e5e835934 100644 --- a/libcloud/compute/drivers/cloudstack.py +++ b/libcloud/compute/drivers/cloudstack.py @@ -3489,70 +3489,106 @@ def ex_delete_security_group(self, name): params={'name': name}, method='GET')['success'] - def ex_authorize_security_group_ingress(self, securitygroupname, - protocol, cidrlist, startport, - endport=None): + def ex_authorize_security_group_ingress(self, securitygroupname, protocol, + cidrlist, startport=None, endport=None, + icmptype=None, icmpcode=None, **kwargs): """ Creates a new Security Group Ingress rule - :param domainid: An optional domainId for the security group. - If the account parameter is used, - domainId must also be used. - :type domainid: ``str`` + :param securitygroupname: The name of the security group. + Mutually exclusive with securitygroupid. + :type securitygroupname: ``str`` - :param startport: Start port for this ingress rule - :type startport: ``int`` + :param protocol: Can be TCP, UDP or ICMP. + Sometime other protocols can be used like AH, ESP + or GRE. + :type protocol: ``str`` - :param securitygroupid: The ID of the security group. - Mutually exclusive with securityGroupName - parameter - :type securitygroupid: ``str`` + :param cidrlist: Source address CIDR for which this rule applies. + :type cidrlist: ``str`` - :param cidrlist: The cidr list associated - :type cidrlist: ``list`` + :param startport: Start port of the range for this ingress rule. + Applies to protocols TCP and UDP. + :type startport: ``int`` - :param usersecuritygrouplist: user to security group mapping - :type usersecuritygrouplist: ``dict`` + :param endport: End port of the range for this ingress rule. + It can be None to set only one port. + Applies to protocols TCP and UDP. + :type endport: ``int`` - :param securitygroupname: The name of the security group. - Mutually exclusive with - securityGroupName parameter - :type securitygroupname: ``str`` + :param icmptype: Type of the ICMP packet (eg: 8 for Echo Request). + -1 or None means "all types". + Applies to protocol ICMP. + :type icmptype: ``int`` - :param account: An optional account for the security group. - Must be used with domainId. - :type account: ``str`` + :param icmpcode: Code of the ICMP packet for the specified type. + If the specified type doesn't require a code set this + value to 0. + -1 or None means "all codes". + Applies to protocol ICMP. + :type icmpcode: ``int`` - :param icmpcode: Error code for this icmp message - :type icmpcode: ``int`` + :keyword account: An optional account for the security group. + Must be used with domainId. + :type account: ``str`` - :param protocol: TCP is default. UDP is the other supported protocol - :type protocol: ``str`` + :keyword domainid: An optional domainId for the security group. + If the account parameter is used, domainId must also + be used. - :param icmptype: type of the icmp message being sent - :type icmptype: ``int`` + :keyword projectid: An optional project of the security group + :type projectid: ``str`` - :param projectid: An optional project of the security group - :type projectid: ``str`` + :param securitygroupid: The ID of the security group. + Mutually exclusive with securitygroupname + :type securitygroupid: ``str`` - :param endport: end port for this ingress rule - :type endport: ``int`` + :param usersecuritygrouplist: User to security group mapping + :type usersecuritygrouplist: ``dict`` - :rtype: ``list`` + :rtype: ``dict`` """ + args = kwargs.copy() protocol = protocol.upper() - if protocol not in ('TCP', 'ICMP'): - raise LibcloudError('Only TCP and ICMP are allowed') - args = { + args.update({ 'securitygroupname': securitygroupname, 'protocol': protocol, - 'startport': int(startport), 'cidrlist': cidrlist - } - if endport is None: - args['endport'] = int(startport) + }) + + if protocol not in ('TCP', 'UDP') and \ + (startport is not None or endport is not None): + raise LibcloudError('"startport" and "endport" are only valid with ' + 'protocol TCP or UDP.') + + if protocol != 'ICMP' and (icmptype is not None or icmpcode is not None): + raise LibcloudError('"icmptype" and "icmpcode" are only valid with ' + 'protocol ICMP.') + + if protocol in ('TCP', 'UDP'): + if startport is None: + raise LibcloudError('Protocols TCP and UDP require at least ' + '"startport" to be set.') + if startport is not None and endport is None: + endport = startport + + args.update({ + 'startport': startport, + 'endport': endport + }) + + if protocol == 'ICMP': + if icmptype is None: + icmptype = -1 + if icmpcode is None: + icmpcode = -1 + + args.update({ + 'icmptype': icmptype, + 'icmpcode': icmpcode + }) return self._async_request(command='authorizeSecurityGroupIngress', params=args, diff --git a/libcloud/test/compute/fixtures/cloudstack/queryAsyncJobResult_17200.json b/libcloud/test/compute/fixtures/cloudstack/queryAsyncJobResult_17200.json index d028ab4949..61645f9c42 100644 --- a/libcloud/test/compute/fixtures/cloudstack/queryAsyncJobResult_17200.json +++ b/libcloud/test/compute/fixtures/cloudstack/queryAsyncJobResult_17200.json @@ -1 +1 @@ -{"queryasyncjobresultresponse":{"jobid":17200,"jobstatus":1,"jobprocstatus":0,"jobresultcode":0,"jobresulttype":"object","jobresult":{"securitygroup":[{"egressrule":[],"account":"runseb@gmail.com","domainid":"ab53d864-6f78-4993-bb28-9b8667b535a1","name":"MySG","domain":"runseb@gmail.com","ingressrule":[{"startport":22,"cidr":"0.0.0.0/0","protocol":"tcp","endport":22,"ruleid":"7df1edc8-6e56-48d7-b816-39377506d787"}],"id":"fa334c44-21c6-4809-ad7d-287bbb23c29b"}]}}} +{ "queryasyncjobresultresponse" : {"accountid":"00000000-0000-0000-0000-000000000000","userid":"111111-1111-1111-1111-111111111111","cmd":"org.apache.cloudstack.api.command.user.securitygroup.AuthorizeSecurityGroupIngressCmd","jobstatus":1,"jobprocstatus":0,"jobresultcode":0,"jobresulttype":"object","jobresult":{"securitygroup":{"id":"222222-2222-2222-2222-222222222222","name":"test_sg","description":"Test security group","account":"smith@example.com","domainid":"333333-3333-3333-3333-333333333333","domain":"smith@example.com","ingressrule":[{"ruleid":"444444-4444-4444-4444-444444444444","protocol":"udp","startport":0,"endport":65535,"cidr":"0.0.0.0/0"}],"egressrule":[],"tags":[]}},"created":"2015-09-15T15:00:00+0200","jobid":"17200"} } diff --git a/libcloud/test/compute/test_cloudstack.py b/libcloud/test/compute/test_cloudstack.py index ed83d7069a..7189821e0e 100644 --- a/libcloud/test/compute/test_cloudstack.py +++ b/libcloud/test/compute/test_cloudstack.py @@ -740,12 +740,20 @@ def test_ex_delete_security_group(self): self.assertTrue(res) def test_ex_authorize_security_group_ingress(self): - res = self.driver.ex_authorize_security_group_ingress('MySG', - 'TCP', - '22', - '22', - '0.0.0.0/0') - self.assertTrue(res) + res = self.driver.ex_authorize_security_group_ingress('test_sg', + 'udp', + '0.0.0.0/0', + '0', + '65535') + self.assertEqual(res.get('name'), 'test_sg') + self.assertTrue('ingressrule' in res) + rules = res['ingressrule'] + self.assertEqual(len(rules), 1) + rule = rules[0] + self.assertEqual(rule['cidr'], '0.0.0.0/0') + self.assertEqual(rule['endport'], 65535) + self.assertEqual(rule['protocol'], 'udp') + self.assertEqual(rule['startport'], 0) def test_ex_create_affinity_group(self): res = self.driver.ex_create_affinity_group('MyAG2', From 11b73c0de92d65e83a88b43d526aa52173217259 Mon Sep 17 00:00:00 2001 From: lionel Date: Wed, 16 Sep 2015 00:08:50 +0200 Subject: [PATCH 2/3] [LIBCLOUD-749] CloudStack: minor fixes to comply with pep8. --- libcloud/compute/drivers/cloudstack.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/libcloud/compute/drivers/cloudstack.py b/libcloud/compute/drivers/cloudstack.py index 4e5e835934..0f60195808 100644 --- a/libcloud/compute/drivers/cloudstack.py +++ b/libcloud/compute/drivers/cloudstack.py @@ -3490,8 +3490,9 @@ def ex_delete_security_group(self, name): method='GET')['success'] def ex_authorize_security_group_ingress(self, securitygroupname, protocol, - cidrlist, startport=None, endport=None, - icmptype=None, icmpcode=None, **kwargs): + cidrlist, startport=None, + endport=None, icmptype=None, + icmpcode=None, **kwargs): """ Creates a new Security Group Ingress rule @@ -3522,8 +3523,8 @@ def ex_authorize_security_group_ingress(self, securitygroupname, protocol, :type icmptype: ``int`` :param icmpcode: Code of the ICMP packet for the specified type. - If the specified type doesn't require a code set this - value to 0. + If the specified type doesn't require a code set + this value to 0. -1 or None means "all codes". Applies to protocol ICMP. :type icmpcode: ``int`` @@ -3560,12 +3561,13 @@ def ex_authorize_security_group_ingress(self, securitygroupname, protocol, if protocol not in ('TCP', 'UDP') and \ (startport is not None or endport is not None): - raise LibcloudError('"startport" and "endport" are only valid with ' - 'protocol TCP or UDP.') + raise LibcloudError('"startport" and "endport" are only valid ' + 'with protocol TCP or UDP.') - if protocol != 'ICMP' and (icmptype is not None or icmpcode is not None): - raise LibcloudError('"icmptype" and "icmpcode" are only valid with ' - 'protocol ICMP.') + if protocol != 'ICMP' and \ + (icmptype is not None or icmpcode is not None): + raise LibcloudError('"icmptype" and "icmpcode" are only valid ' + 'with protocol ICMP.') if protocol in ('TCP', 'UDP'): if startport is None: From 57a8733b27bb3d403b101b5b417ce22103e22d39 Mon Sep 17 00:00:00 2001 From: lionel Date: Mon, 21 Sep 2015 20:30:39 +0200 Subject: [PATCH 3/3] Fixed a minor issue with the docstring of the method ex_authorize_security_group_ingress --- libcloud/compute/drivers/cloudstack.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libcloud/compute/drivers/cloudstack.py b/libcloud/compute/drivers/cloudstack.py index 0f60195808..3c4b2433f1 100644 --- a/libcloud/compute/drivers/cloudstack.py +++ b/libcloud/compute/drivers/cloudstack.py @@ -3540,12 +3540,12 @@ def ex_authorize_security_group_ingress(self, securitygroupname, protocol, :keyword projectid: An optional project of the security group :type projectid: ``str`` - :param securitygroupid: The ID of the security group. - Mutually exclusive with securitygroupname - :type securitygroupid: ``str`` + :keyword securitygroupid: The ID of the security group. + Mutually exclusive with securitygroupname + :type securitygroupid: ``str`` - :param usersecuritygrouplist: User to security group mapping - :type usersecuritygrouplist: ``dict`` + :keyword usersecuritygrouplist: User to security group mapping + :type usersecuritygrouplist: ``dict`` :rtype: ``dict`` """