You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Andrew Bayer <no...@github.com> on 2013/07/21 21:30:20 UTC
[jclouds] JCLOUDS-195. Add egress firewall rules for CloudStack (#69)
You can merge this Pull Request by running:
git pull https://github.com/abayer/jclouds-1 jclouds-195
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/69
-- Commit Summary --
* JCLOUDS-195. Add egress firewall rules for CloudStack
-- File Changes --
M apis/cloudstack/src/main/java/org/jclouds/cloudstack/features/FirewallApi.java (58)
M apis/cloudstack/src/test/java/org/jclouds/cloudstack/features/FirewallApiExpectTest.java (118)
M apis/cloudstack/src/test/java/org/jclouds/cloudstack/features/FirewallApiLiveTest.java (41)
A apis/cloudstack/src/test/resources/createegressfirewallrulesresponse.json (1)
A apis/cloudstack/src/test/resources/deleteegressfirewallrulesresponse.json (1)
A apis/cloudstack/src/test/resources/getegressfirewallrulesresponse.json (2)
A apis/cloudstack/src/test/resources/listegressfirewallrulesresponse.json (4)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/69.patch
https://github.com/jclouds/jclouds/pull/69.diff
Re: [jclouds] JCLOUDS-195. Add egress firewall rules for CloudStack
(#69)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #69](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/69/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/69#issuecomment-21317461
Re: [jclouds] JCLOUDS-195. Add egress firewall rules for CloudStack
(#69)
Posted by Andrew Bayer <no...@github.com>.
> @@ -178,6 +210,15 @@ protected void checkFirewallRule(FirewallRule rule) {
> assert rule.getProtocol() != null;
> }
>
> + protected void checkEgressFirewallRule(FirewallRule rule) {
> + assertEquals(rule,
> + client.getFirewallApi().getEgressFirewallRule(rule.getId()));
It is possible - ICMP vs TCP etc, I think. Again, I copy-and-pasted, so I didn't write the original test.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/69/files#r5402058
Re: [jclouds] JCLOUDS-195. Add egress firewall rules for CloudStack
(#69)
Posted by Andrew Bayer <no...@github.com>.
Reopened #69.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/69
Re: [jclouds] JCLOUDS-195. Add egress firewall rules for CloudStack
(#69)
Posted by Andrew Bayer <no...@github.com>.
Closed #69.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/69
Re: [jclouds] JCLOUDS-195. Add egress firewall rules for CloudStack
(#69)
Posted by BuildHive <no...@github.com>.
[jclouds ยป jclouds #208](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/208/) UNSTABLE
Looks like there's a problem with this pull request
[(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/69#issuecomment-21317088
Re: [jclouds] JCLOUDS-195. Add egress firewall rules for CloudStack
(#69)
Posted by Andrew Phillips <no...@github.com>.
> @@ -178,6 +210,15 @@ protected void checkFirewallRule(FirewallRule rule) {
> assert rule.getProtocol() != null;
> }
>
> + protected void checkEgressFirewallRule(FirewallRule rule) {
> + assertEquals(rule,
> + client.getFirewallApi().getEgressFirewallRule(rule.getId()));
> + assert rule.getId() != null : rule;
> + assert rule.getStartPort() > 0 : rule;
> + assert rule.getEndPort() >= rule.getStartPort() : rule;
> + assert rule.getProtocol() != null;
Why not `assertTrue` or `assertNotNull` for these?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/69/files#r5401107
Re: [jclouds] JCLOUDS-195. Add egress firewall rules for CloudStack
(#69)
Posted by Andrew Phillips <no...@github.com>.
> @@ -178,6 +210,15 @@ protected void checkFirewallRule(FirewallRule rule) {
> assert rule.getProtocol() != null;
> }
>
> + protected void checkEgressFirewallRule(FirewallRule rule) {
> + assertEquals(rule,
> + client.getFirewallApi().getEgressFirewallRule(rule.getId()));
Is this not enough? The rest seems like a "validity check" of some sort - should that perhaps go in `FirewallRule` itself? Should it even be _possible_ to create a FirewallRule that violates the checks that follow?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/69/files#r5401181
Re: [jclouds] JCLOUDS-195. Add egress firewall rules for CloudStack
(#69)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #526](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/526/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/69#issuecomment-21317521
Re: [jclouds] JCLOUDS-195. Add egress firewall rules for CloudStack
(#69)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #67](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/67/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/69#issuecomment-21316790
Re: [jclouds] JCLOUDS-195. Add egress firewall rules for CloudStack
(#69)
Posted by Andrew Bayer <no...@github.com>.
> @@ -178,6 +210,15 @@ protected void checkFirewallRule(FirewallRule rule) {
> assert rule.getProtocol() != null;
> }
>
> + protected void checkEgressFirewallRule(FirewallRule rule) {
> + assertEquals(rule,
> + client.getFirewallApi().getEgressFirewallRule(rule.getId()));
> + assert rule.getId() != null : rule;
> + assert rule.getStartPort() > 0 : rule;
> + assert rule.getEndPort() >= rule.getStartPort() : rule;
> + assert rule.getProtocol() != null;
No good reason for assert. Just what we were doing for ingress rules, and I copy-and-pasted, since the content's the same.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/69/files#r5402024
Re: [jclouds] JCLOUDS-195. Add egress firewall rules for CloudStack
(#69)
Posted by Andrew Phillips <no...@github.com>.
> + ip.getId(), FirewallRule.Protocol.TCP, CreateFirewallRuleOptions.Builder.startPort(30).endPort(35));
> + assertTrue(jobComplete.apply(job.getJobId()));
> + egressFirewallRule = client.getFirewallApi().getEgressFirewallRule(job.getId());
> +
> + assertEquals(egressFirewallRule.getStartPort(), 30);
> + assertEquals(egressFirewallRule.getEndPort(), 35);
> + assertEquals(egressFirewallRule.getProtocol(), FirewallRule.Protocol.TCP);
> +
> + checkEgressFirewallRule(egressFirewallRule);
> + }
> +
> + @Test(dependsOnMethods = "testCreateEgressFirewallRule")
> + public void testListEgressFirewallRules() {
> + Set<FirewallRule> rules = client.getFirewallApi().listEgressFirewallRules();
> +
> + assert rules != null;
`assertNotNull`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/69/files#r5401146