diff --git a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/features/FirewallAsyncClient.java b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/features/FirewallAsyncClient.java index 6b953c0daa..51360e79b6 100644 --- a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/features/FirewallAsyncClient.java +++ b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/features/FirewallAsyncClient.java @@ -78,14 +78,14 @@ public interface FirewallAsyncClient { ListenableFuture getFirewallRule(@QueryParam("id") long id); /** - * @see FirewallClient#createFirewallRule + * @see FirewallClient#createFirewallRuleForIpAndProtocol */ @GET @QueryParams(keys = "command", values = "createFirewallRule") @Unwrap @Consumes(MediaType.APPLICATION_JSON) - ListenableFuture createFirewallRule(@QueryParam("ipaddressid") long ipAddressId, - @QueryParam("protocol") FirewallRule.Protocol protocol, CreateFirewallRuleOptions... options); + ListenableFuture createFirewallRuleForIpAndProtocol(@QueryParam("ipaddressid") long ipAddressId, + @QueryParam("protocol") FirewallRule.Protocol protocol, CreateFirewallRuleOptions... options); /** * @see FirewallClient#deleteFirewallRule @@ -124,9 +124,9 @@ public interface FirewallAsyncClient { @Unwrap @Consumes(MediaType.APPLICATION_JSON) ListenableFuture createPortForwardingRuleForVirtualMachine( - @QueryParam("virtualmachineid") long virtualMachineId, @QueryParam("ipaddressid") long IPAddressId, - @QueryParam("protocol") String protocol, @QueryParam("privateport") int privatePort, - @QueryParam("publicport") int publicPort); + @QueryParam("ipaddressid") long ipAddressId, @QueryParam("protocol") String protocol, + @QueryParam("publicport") int publicPort, @QueryParam("virtualmachineid") long virtualMachineId, + @QueryParam("privateport") int privatePort); /** * @see FirewallClient#deletePortForwardingRule diff --git a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/features/FirewallClient.java b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/features/FirewallClient.java index 15b5ba68bd..7ea4dddb51 100644 --- a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/features/FirewallClient.java +++ b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/features/FirewallClient.java @@ -71,9 +71,8 @@ public interface FirewallClient { * optional arguments for firewall rule creation * @return */ - AsyncCreateResponse createFirewallRule(long ipAddressId, FirewallRule.Protocol protocol, - CreateFirewallRuleOptions... options); - + AsyncCreateResponse createFirewallRuleForIpAndProtocol(long ipAddressId, + FirewallRule.Protocol protocol, CreateFirewallRuleOptions... options); /** * Deletes a firewall rule @@ -106,21 +105,20 @@ public interface FirewallClient { /** * Creates an port forwarding rule * - * @param virtualMachineId - * the ID of the virtual machine for the port forwarding rule - * @param IPAddressId - * the public IP address id of the forwarding rule, already - * associated via associatePort + * + * @param ipAddressId * @param protocol * the protocol for the rule. Valid values are TCP or UDP. - * @param privatePort - * the private port of the port forwarding rule * @param publicPort * the public port of the port forwarding rule + * @param virtualMachineId + * the ID of the virtual machine for the port forwarding rule + * @param privatePort + * the private port of the port forwarding rule * @return response used to track creation */ - AsyncCreateResponse createPortForwardingRuleForVirtualMachine(long virtualMachineId, long IPAddressId, - String protocol, int privatePort, int publicPort); + AsyncCreateResponse createPortForwardingRuleForVirtualMachine(long ipAddressId, + String protocol, int publicPort, long virtualMachineId, int privatePort); /** * Deletes an port forwarding rule diff --git a/apis/cloudstack/src/test/java/org/jclouds/cloudstack/features/FirewallClientLiveTest.java b/apis/cloudstack/src/test/java/org/jclouds/cloudstack/features/FirewallClientLiveTest.java index e2a504902a..a4d0b4d7ac 100644 --- a/apis/cloudstack/src/test/java/org/jclouds/cloudstack/features/FirewallClientLiveTest.java +++ b/apis/cloudstack/src/test/java/org/jclouds/cloudstack/features/FirewallClientLiveTest.java @@ -33,6 +33,8 @@ import org.jclouds.cloudstack.domain.Network; import org.jclouds.cloudstack.domain.PortForwardingRule; import org.jclouds.cloudstack.domain.PublicIPAddress; import org.jclouds.cloudstack.domain.VirtualMachine; +import org.jclouds.cloudstack.options.CreateFirewallRuleOptions; +import org.jclouds.logging.Logger; import org.jclouds.net.IPSocket; import org.testng.annotations.AfterGroups; import org.testng.annotations.BeforeGroups; @@ -93,12 +95,13 @@ public class FirewallClientLiveTest extends BaseCloudStackClientLiveTest { while (portForwardingRule == null) { ip = reuseOrAssociate.apply(network); try { - AsyncCreateResponse job = client.getFirewallClient().createPortForwardingRuleForVirtualMachine(vm.getId(), - ip.getId(), "tcp", 22, 22); + AsyncCreateResponse job = client.getFirewallClient() + .createPortForwardingRuleForVirtualMachine(ip.getId(), "tcp", 22, vm.getId(), 22); assertTrue(jobComplete.apply(job.getJobId())); portForwardingRule = client.getFirewallClient().getPortForwardingRule(job.getId()); } catch (IllegalStateException e) { + Logger.CONSOLE.error("Failed while trying to allocate ip: " + e); // very likely an ip conflict, so retry; } } @@ -118,27 +121,44 @@ public class FirewallClientLiveTest extends BaseCloudStackClientLiveTest { assert null != response; assertTrue(response.size() >= 0); for (final PortForwardingRule rule : response) { - PortForwardingRule newDetails = client.getFirewallClient().getPortForwardingRule(rule.getId()); - assertEquals(rule.getId(), newDetails.getId()); checkPortForwardingRule(rule); } } @Test(dependsOnMethods = "testCreatePortForwardingRule") public void testCreateFirewallRule() { + if (networksDisabled) + return; + AsyncCreateResponse job = client.getFirewallClient().createFirewallRuleForIpAndProtocol( + ip.getId(), FirewallRule.Protocol.TCP, CreateFirewallRuleOptions.Builder.startPort(30).endPort(35)); + assertTrue(jobComplete.apply(job.getJobId())); + firewallRule = client.getFirewallClient().getFirewallRule(job.getId()); + + assertEquals(firewallRule.getStartPort(), 30); + assertEquals(firewallRule.getEndPort(), 35); + assertEquals(firewallRule.getProtocol(), FirewallRule.Protocol.TCP); + + checkFirewallRule(firewallRule); } @Test(dependsOnMethods = "testCreateFirewallRule") public void testListFirewallRules() { Set rules = client.getFirewallClient().listFirewallRules(); + assert rules != null; - assertTrue(rules.size() >= 0); - // TODO: check each rule + assertTrue(rules.size() > 0); + + for(FirewallRule rule : rules) { + checkFirewallRule(rule); + } } @AfterGroups(groups = "live") protected void tearDown() { + if (firewallRule != null) { + client.getFirewallClient().deleteFirewallRule(firewallRule.getId()); + } if (portForwardingRule != null) { client.getFirewallClient().deletePortForwardingRule(portForwardingRule.getId()); } @@ -151,8 +171,18 @@ public class FirewallClientLiveTest extends BaseCloudStackClientLiveTest { super.tearDown(); } + protected void checkFirewallRule(FirewallRule rule) { + assertEquals(rule, + client.getFirewallClient().getFirewallRule(rule.getId())); + assert rule.getId() > 0 : rule; + assert rule.getStartPort() > 0 : rule; + assert rule.getEndPort() >= rule.getStartPort() : rule; + assert rule.getProtocol() != null; + } + protected void checkPortForwardingRule(PortForwardingRule rule) { - assertEquals(rule.getId(), client.getFirewallClient().getPortForwardingRule(rule.getId()).getId()); + assertEquals(rule, + client.getFirewallClient().getPortForwardingRule(rule.getId())); assert rule.getId() > 0 : rule; assert rule.getIPAddress() != null : rule; assert rule.getIPAddressId() > 0 : rule;