You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Hyeonu Jeong <no...@github.com> on 2013/12/12 07:31:38 UTC
[jclouds] Fix CloudStack EgressFirewall request parameter name for
1.6.x (#229)
createEgressFirewall needs networkid instead of ipaddressid:
http://download.cloud.com/releases/3.0.6/api_3.0.6/root_admin/createEgressFirewallRule.html
You can merge this Pull Request by running:
git pull https://github.com/hyeonu/jclouds bugfix-cloudstack-egressfirewall-for-1.6.x
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/229
-- Commit Summary --
* Fix CloudStack EgressFirewall request parameter name for 1.6.x
-- File Changes --
M apis/cloudstack/src/main/java/org/jclouds/cloudstack/features/FirewallAsyncClient.java (11)
M apis/cloudstack/src/main/java/org/jclouds/cloudstack/features/FirewallClient.java (11)
A apis/cloudstack/src/main/java/org/jclouds/cloudstack/options/ListEgressFirewallRulesOptions.java (179)
M apis/cloudstack/src/test/java/org/jclouds/cloudstack/features/FirewallClientExpectTest.java (6)
M apis/cloudstack/src/test/java/org/jclouds/cloudstack/features/FirewallClientLiveTest.java (4)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/229.patch
https://github.com/jclouds/jclouds/pull/229.diff
Re: [jclouds] Fix CloudStack EgressFirewall request parameter name
for 1.6.x (#229)
Posted by Andrew Phillips <no...@github.com>.
Thanks, @hyeonu! Do you have the output of a live test run that demonstrates this in action?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/229#issuecomment-30470290
Re: [jclouds] Fix CloudStack EgressFirewall request parameter name
for 1.6.x (#229)
Posted by Jeremy Daggett <no...@github.com>.
Closed #229.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/229#event-151491646
Re: [jclouds] Fix CloudStack EgressFirewall request parameter name
for 1.6.x (#229)
Posted by Hyeonu Jeong <no...@github.com>.
> + *
> + * @see <a href=
> + * "http://download.cloud.com/releases/3.0.6/api_3.0.6/root_admin/listEgressFirewallRules.html"
> + * />
> + * @author Hyeonu Jeong
> + */
> +public class ListEgressFirewallRulesOptions extends AccountInDomainOptions {
> +
> + public static final ListEgressFirewallRulesOptions NONE = new ListEgressFirewallRulesOptions();
> +
> + /**
> + * @param id
> + * firewall rule ID
> + */
> + public ListEgressFirewallRulesOptions id(String id) {
> + this.queryParameters.replaceValues("id", ImmutableSet.of(id + ""));
Just copied from ListFirewallRulesOptions and added networkId() method.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/229/files#r8321847
Re: [jclouds] Fix CloudStack EgressFirewall request parameter name
for 1.6.x (#229)
Posted by Andrew Phillips <no...@github.com>.
> +
> + /**
> + * {@inheritDoc}
> + */
> + @Override
> + public ListEgressFirewallRulesOptions accountInDomain(String account, String domain) {
> + return ListEgressFirewallRulesOptions.class.cast(super.accountInDomain(account, domain));
> + }
> +
> + /**
> + * {@inheritDoc}
> + */
> + @Override
> + public ListEgressFirewallRulesOptions domainId(String domainId) {
> + return ListEgressFirewallRulesOptions.class.cast(super.domainId(domainId));
> + }
[minor] Move these two above the builder?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/229/files#r8318117
Re: [jclouds] Fix CloudStack EgressFirewall request parameter name
for 1.6.x (#229)
Posted by Andrew Phillips <no...@github.com>.
> Here is live test results(test only testCreateEgressFirewallRule and testListEgressFirewallRules):
Looks good, thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/229#issuecomment-30489332
Re: [jclouds] Fix CloudStack EgressFirewall request parameter name
for 1.6.x (#229)
Posted by Andrew Phillips <no...@github.com>.
> +
> + /**
> + * @param ipAddressId
> + * the id of IP address of the firewall services
> + */
> + public ListEgressFirewallRulesOptions ipAddressId(String ipAddressId) {
> + this.queryParameters.replaceValues("ipaddressid", ImmutableSet.of(ipAddressId + ""));
> + return this;
> + }
> +
> + /**
> + * @param projectId
> + * List firewall rules in this project.
> + */
> + public ListEgressFirewallRulesOptions projectId(String projectId) {
> + this.queryParameters.replaceValues("projectid", ImmutableSet.of(projectId + ""));
See above
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/229/files#r8317992
Re: [jclouds] Fix CloudStack EgressFirewall request parameter name
for 1.6.x (#229)
Posted by Hyeonu Jeong <no...@github.com>.
Here is live test results:
```
-------------------------------------------------------
T E S T S
-------------------------------------------------------
Running org.jclouds.cloudstack.features.FirewallClientLiveTest
Configuring TestNG with: org.apache.maven.surefire.testng.conf.TestNG652Configurator@42704baa
Starting test testCreatePortForwardingRule(org.jclouds.cloudstack.features.FirewallClientLiveTest)
[pool-1-thread-1] Test testCreatePortForwardingRule(org.jclouds.cloudstack.features.FirewallClientLiveTest) succeeded: 1ms
Test suite progress: tests succeeded: 1, failed: 0, skipped: 0.
Starting test testCreateEgressFirewallRule(org.jclouds.cloudstack.features.FirewallClientLiveTest)
[pool-1-thread-2] Test testCreateEgressFirewallRule(org.jclouds.cloudstack.features.FirewallClientLiveTest) succeeded: 0ms
Test suite progress: tests succeeded: 2, failed: 0, skipped: 0.
Starting test testCreateFirewallRule(org.jclouds.cloudstack.features.FirewallClientLiveTest)
[pool-1-thread-2] Test testCreateFirewallRule(org.jclouds.cloudstack.features.FirewallClientLiveTest) succeeded: 0ms
Test suite progress: tests succeeded: 3, failed: 0, skipped: 0.
Starting test testListPortForwardingRules(org.jclouds.cloudstack.features.FirewallClientLiveTest)
[pool-1-thread-2] Test testListPortForwardingRules(org.jclouds.cloudstack.features.FirewallClientLiveTest) succeeded: 18ms
Test suite progress: tests succeeded: 4, failed: 0, skipped: 0.
Starting test testListEgressFirewallRules(org.jclouds.cloudstack.features.FirewallClientLiveTest)
[pool-1-thread-3] Test testListEgressFirewallRules(org.jclouds.cloudstack.features.FirewallClientLiveTest) failed.
Test suite progress: tests succeeded: 4, failed: 1, skipped: 0.
Starting test testListFirewallRules(org.jclouds.cloudstack.features.FirewallClientLiveTest)
[pool-1-thread-3] Test testListFirewallRules(org.jclouds.cloudstack.features.FirewallClientLiveTest) failed.
Test suite progress: tests succeeded: 4, failed: 2, skipped: 0.
Tests run: 6, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 2.233 sec <<< FAILURE!
testListEgressFirewallRules(org.jclouds.cloudstack.features.FirewallClientLiveTest) Time elapsed: 0.019 sec <<< FAILURE!
org.jclouds.http.HttpResponseException: command: GET http://192.168.50.50:8080/client/api?response=json&command=listEgressFirewallRules&listAll=true HTTP/1.1 failed with response: HTTP/1.1 432 null; content: [{ "errorresponse" : {"uuidList":[],"errorcode":432,"errortext":"The given command does not exist"} }]
at org.jclouds.cloudstack.handlers.CloudStackErrorHandler.handleError(CloudStackErrorHandler.java:50)
at org.jclouds.http.handlers.DelegatingErrorHandler.handleError(DelegatingErrorHandler.java:67)
at org.jclouds.http.internal.BaseHttpCommandExecutorService.shouldContinue(BaseHttpCommandExecutorService.java:180)
at org.jclouds.http.internal.BaseHttpCommandExecutorService.invoke(BaseHttpCommandExecutorService.java:150)
at org.jclouds.rest.internal.InvokeSyncToAsyncHttpMethod.invoke(InvokeSyncToAsyncHttpMethod.java:131)
at org.jclouds.rest.internal.InvokeSyncToAsyncHttpMethod.apply(InvokeSyncToAsyncHttpMethod.java:97)
at org.jclouds.rest.internal.InvokeSyncToAsyncHttpMethod.apply(InvokeSyncToAsyncHttpMethod.java:58)
at org.jclouds.reflect.FunctionalReflection$FunctionalInvocationHandler.handleInvocation(FunctionalReflection.java:117)
at com.google.common.reflect.AbstractInvocationHandler.invoke(AbstractInvocationHandler.java:70)
at com.sun.proxy.$Proxy79.listEgressFirewallRules(Unknown Source)
at org.jclouds.cloudstack.features.FirewallClientLiveTest.testListEgressFirewallRules(FirewallClientLiveTest.java:174)
testListFirewallRules(org.jclouds.cloudstack.features.FirewallClientLiveTest) Time elapsed: 0.014 sec <<< FAILURE!
java.lang.AssertionError: expected [true] but found [false]
at org.testng.Assert.fail(Assert.java:94)
at org.testng.Assert.failNotEquals(Assert.java:494)
at org.testng.Assert.assertTrue(Assert.java:42)
at org.testng.Assert.assertTrue(Assert.java:52)
at org.jclouds.cloudstack.features.FirewallClientLiveTest.testListFirewallRules(FirewallClientLiveTest.java:148)
Results :
Failed tests: testListEgressFirewallRules(org.jclouds.cloudstack.features.FirewallClientLiveTest): command: GET http://192.168.50.50:8080/client/api?response=json&command=listEgressFirewallRules&listAll=true HTTP/1.1 failed with response: HTTP/1.1 432 null; content: [{ "errorresponse" : {"uuidList":[],"errorcode":432,"errortext":"The given command does not exist"} }]
testListFirewallRules(org.jclouds.cloudstack.features.FirewallClientLiveTest): expected [true] but found [false]
Tests run: 6, Failures: 2, Errors: 0, Skipped: 0
```
My test cloudstack(earlier version 3.0) doesn't support 'listEgressFirewallRules' command, sorry for that. But service cloudstack(3.0.7) already use it for two or three months. See below logs(keys are removed for security reason):
```
03:02:58.646 [ajp-bio-8009-exec-5] DEBUG [o.j.h.i.JavaUrlHttpCommandExecutorService] Sending request 1229526733: GET https://10.0.7.55/client/api?response=json&command=listEgressFirewallRules&listAll=true&account=ipcaccount_635&domainid=b8cbc3ed-9dbd-414c-98f3-4f54a6083229&networkid=0112ae20-c091-4b7c-8351-f6e7501d053a&<removed api and secret keys> HTTP/1.1
03:02:58.674 [ajp-bio-8009-exec-5] DEBUG [o.j.h.i.JavaUrlHttpCommandExecutorService] Receiving response 1229526733: HTTP/1.1 200 OK
03:02:58.674 [ajp-bio-8009-exec-5] DEBUG [jclouds.wire] << "{ "listegressfirewallrulesresponse" : { } }"
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/229#issuecomment-30481235
Re: [jclouds] Fix CloudStack EgressFirewall request parameter name
for 1.6.x (#229)
Posted by Hyeonu Jeong <no...@github.com>.
> + *
> + * @see <a href=
> + * "http://download.cloud.com/releases/3.0.6/api_3.0.6/root_admin/listEgressFirewallRules.html"
> + * />
> + * @author Hyeonu Jeong
> + */
> +public class ListEgressFirewallRulesOptions extends AccountInDomainOptions {
> +
> + public static final ListEgressFirewallRulesOptions NONE = new ListEgressFirewallRulesOptions();
> +
> + /**
> + * @param id
> + * firewall rule ID
> + */
> + public ListEgressFirewallRulesOptions id(String id) {
> + this.queryParameters.replaceValues("id", ImmutableSet.of(id + ""));
Never thought about that.
I thinks id can be `null`, and `ImmutableSet.of(id)` will generated like `id=null`. cause `null` toString is `null` and empty string toString is ` `.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/229/files#r8322842
Re: [jclouds] Fix CloudStack EgressFirewall request parameter name
for 1.6.x (#229)
Posted by Andrew Phillips <no...@github.com>.
> + *
> + * @see <a href=
> + * "http://download.cloud.com/releases/3.0.6/api_3.0.6/root_admin/listEgressFirewallRules.html"
> + * />
> + * @author Hyeonu Jeong
> + */
> +public class ListEgressFirewallRulesOptions extends AccountInDomainOptions {
> +
> + public static final ListEgressFirewallRulesOptions NONE = new ListEgressFirewallRulesOptions();
> +
> + /**
> + * @param id
> + * firewall rule ID
> + */
> + public ListEgressFirewallRulesOptions id(String id) {
> + this.queryParameters.replaceValues("id", ImmutableSet.of(id + ""));
> I thinks id can be null, and ImmutableSet.of(id) will generate like id=null. cause null toString is null and empty string toString is .
Indeed. Is that what we want?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/229/files#r8325259
Re: [jclouds] Fix CloudStack EgressFirewall request parameter name
for 1.6.x (#229)
Posted by Andrew Phillips <no...@github.com>.
> +
> + /**
> + * @param networkId
> + * the id of network of the firewall services
> + */
> + public ListEgressFirewallRulesOptions networkId(String networkId) {
> + this.queryParameters.replaceValues("networkid", ImmutableSet.of(networkId + ""));
> + return this;
> + }
> +
> + /**
> + * @param ipAddressId
> + * the id of IP address of the firewall services
> + */
> + public ListEgressFirewallRulesOptions ipAddressId(String ipAddressId) {
> + this.queryParameters.replaceValues("ipaddressid", ImmutableSet.of(ipAddressId + ""));
See above
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/229/files#r8317989
Re: [jclouds] Fix CloudStack EgressFirewall request parameter name
for 1.6.x (#229)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #929](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/929/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/229#issuecomment-30394022
Re: [jclouds] Fix CloudStack EgressFirewall request parameter name
for 1.6.x (#229)
Posted by BuildHive <no...@github.com>.
[jclouds ยป jclouds #702](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/702/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/229#issuecomment-30395335
Re: [jclouds] Fix CloudStack EgressFirewall request parameter name
for 1.6.x (#229)
Posted by Andrew Phillips <no...@github.com>.
> + ListEgressFirewallRulesOptions options = new ListEgressFirewallRulesOptions();
> + return options.pageSize(pageSize);
> + }
> +
> + /**
> + * @see ListEgressFirewallRulesOptions#accountInDomain
> + */
> + public static ListEgressFirewallRulesOptions accountInDomain(String account, String domain) {
> + ListEgressFirewallRulesOptions options = new ListEgressFirewallRulesOptions();
> + return options.accountInDomain(account, domain);
> + }
> +
> + /**
> + * @see ListEgressFirewallRulesOptions#domainId
> + */
> + public static ListEgressFirewallRulesOptions domainId(String id) {
Should the parameter be called `domainId`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/229/files#r8318076
Re: [jclouds] Fix CloudStack EgressFirewall request parameter name
for 1.6.x (#229)
Posted by Andrew Phillips <no...@github.com>.
> + *
> + * @see <a href=
> + * "http://download.cloud.com/releases/3.0.6/api_3.0.6/root_admin/listEgressFirewallRules.html"
> + * />
> + * @author Hyeonu Jeong
> + */
> +public class ListEgressFirewallRulesOptions extends AccountInDomainOptions {
> +
> + public static final ListEgressFirewallRulesOptions NONE = new ListEgressFirewallRulesOptions();
> +
> + /**
> + * @param id
> + * firewall rule ID
> + */
> + public ListEgressFirewallRulesOptions id(String id) {
> + this.queryParameters.replaceValues("id", ImmutableSet.of(id + ""));
Ah, so it's probably weird there, too...
Is the ID allowed to be `null`? If not, `ImmutableSet.of(id)` should be sufficient.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/229/files#r8322041
Re: [jclouds] Fix CloudStack EgressFirewall request parameter name
for 1.6.x (#229)
Posted by Everett Toews <no...@github.com>.
During release week we like to do a little house cleaning in the jclouds world. That means sweeping out the pull request queue.
This PR is over 6 months old. Please update us on its status here. If we don't hear anything, we will take that as lazy consensus that the PR is no longer relevant and it will be closed on Friday, Aug 8.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/229#issuecomment-50782346
Re: [jclouds] Fix CloudStack EgressFirewall request parameter name
for 1.6.x (#229)
Posted by Andrew Phillips <no...@github.com>.
> + *
> + * @see <a href=
> + * "http://download.cloud.com/releases/3.0.6/api_3.0.6/root_admin/listEgressFirewallRules.html"
> + * />
> + * @author Hyeonu Jeong
> + */
> +public class ListEgressFirewallRulesOptions extends AccountInDomainOptions {
> +
> + public static final ListEgressFirewallRulesOptions NONE = new ListEgressFirewallRulesOptions();
> +
> + /**
> + * @param id
> + * firewall rule ID
> + */
> + public ListEgressFirewallRulesOptions id(String id) {
> + this.queryParameters.replaceValues("id", ImmutableSet.of(id + ""));
What's the purpose of the `+ ""` here? `id` is a String already..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/229/files#r8317980
Re: [jclouds] Fix CloudStack EgressFirewall request parameter name
for 1.6.x (#229)
Posted by Andrew Phillips <no...@github.com>.
> +
> + /**
> + * @param id
> + * firewall rule ID
> + */
> + public ListEgressFirewallRulesOptions id(String id) {
> + this.queryParameters.replaceValues("id", ImmutableSet.of(id + ""));
> + return this;
> + }
> +
> + /**
> + * @param networkId
> + * the id of network of the firewall services
> + */
> + public ListEgressFirewallRulesOptions networkId(String networkId) {
> + this.queryParameters.replaceValues("networkid", ImmutableSet.of(networkId + ""));
See above
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/229/files#r8317986