You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Reijhanniel Jearl Campos <no...@github.com> on 2015/02/25 08:13:03 UTC
[jclouds-labs] JCLOUDS-702: JCloud ProfitBricks provider - NIC &
Firewall API (#139)
Hi! This PR implements ProfitBricks' [NIC](https://www.profitbricks.com/apidoc/1_3/APIDocumentation.html?NICOperations.html) and [Firewall](https://www.profitbricks.com/apidoc/1_3/APIDocumentation.html?FirewallOperations.html) APIs. Looking forward to the code review to correct stuff I might've missed. :)
/cc @nacx @jasminSPC
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs/pull/139
-- Commit Summary --
* JCLOUDS-702: JCloud ProfitBricks provider - NIC & Firewall API
-- File Changes --
M profitbricks/src/main/java/org/jclouds/profitbricks/ProfitBricksApi.java (8)
A profitbricks/src/main/java/org/jclouds/profitbricks/binder/firewall/AddFirewallRuleToNicRequestBinder.java (53)
A profitbricks/src/main/java/org/jclouds/profitbricks/binder/firewall/FirewallBinder.java (95)
A profitbricks/src/main/java/org/jclouds/profitbricks/binder/nic/CreateNicRequestBinder.java (46)
A profitbricks/src/main/java/org/jclouds/profitbricks/binder/nic/SetInternetAccessBinder.java (42)
A profitbricks/src/main/java/org/jclouds/profitbricks/binder/nic/UpdateNicRequestBinder.java (46)
M profitbricks/src/main/java/org/jclouds/profitbricks/compute/internal/ProvisioningStatusAware.java (2)
M profitbricks/src/main/java/org/jclouds/profitbricks/compute/internal/ProvisioningStatusPollingPredicate.java (7)
A profitbricks/src/main/java/org/jclouds/profitbricks/domain/Firewall.java (367)
A profitbricks/src/main/java/org/jclouds/profitbricks/domain/Nic.java (379)
M profitbricks/src/main/java/org/jclouds/profitbricks/domain/Server.java (427)
M profitbricks/src/main/java/org/jclouds/profitbricks/domain/Storage.java (1)
A profitbricks/src/main/java/org/jclouds/profitbricks/domain/internal/FirewallRuleCommonProperties.java (44)
M profitbricks/src/main/java/org/jclouds/profitbricks/features/DataCenterApi.java (12)
A profitbricks/src/main/java/org/jclouds/profitbricks/features/FirewallApi.java (93)
A profitbricks/src/main/java/org/jclouds/profitbricks/features/NicApi.java (84)
M profitbricks/src/main/java/org/jclouds/profitbricks/features/ServerApi.java (20)
M profitbricks/src/main/java/org/jclouds/profitbricks/features/StorageApi.java (11)
M profitbricks/src/main/java/org/jclouds/profitbricks/http/ResponseStatusFromPayloadHttpCommandExecutorService.java (76)
M profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/BaseProfitBricksResponseHandler.java (4)
M profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/datacenter/DataCenterInfoResponseHandler.java (78)
A profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/firewall/BaseFirewallResponseHandler.java (69)
A profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/firewall/FirewallListResponseHandler.java (64)
A profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/firewall/FirewallResponseHandler.java (63)
A profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/firewall/rule/BaseFirewallRuleResponseHandler.java (54)
A profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/firewall/rule/FirewallRuleListResponseHandler.java (54)
A profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/nic/BaseNicResponseHandler.java (82)
A profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/nic/NicListResponseHandler.java (65)
A profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/nic/NicResponseHandler.java (58)
M profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/server/BaseServerResponseHandler.java (74)
M profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/server/ServerInfoResponseHandler.java (34)
M profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/server/ServerListResponseHandler.java (44)
M profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/storage/BaseStorageResponseHandler.java (20)
M profitbricks/src/main/java/org/jclouds/profitbricks/http/parser/storage/StorageListResponseHandler.java (19)
A profitbricks/src/main/java/org/jclouds/profitbricks/util/Strings3.java (43)
A profitbricks/src/test/java/org/jclouds/profitbricks/binder/firewall/AddFirewallRuleToNicRequestBinderTest.java (64)
A profitbricks/src/test/java/org/jclouds/profitbricks/binder/firewall/FirewallBinderTest.java (131)
A profitbricks/src/test/java/org/jclouds/profitbricks/binder/nic/CreateNicRequestBinderTest.java (53)
A profitbricks/src/test/java/org/jclouds/profitbricks/binder/nic/SetInternetAccessBinderTest.java (48)
A profitbricks/src/test/java/org/jclouds/profitbricks/binder/nic/UpdateNicRequestBinderTest.java (54)
A profitbricks/src/test/java/org/jclouds/profitbricks/domain/FirewallRuleBuilderTest.java (82)
A profitbricks/src/test/java/org/jclouds/profitbricks/features/FirewallApiLiveTest.java (146)
A profitbricks/src/test/java/org/jclouds/profitbricks/features/FirewallApiMockTest.java (324)
A profitbricks/src/test/java/org/jclouds/profitbricks/features/NicApiLiveTest.java (136)
A profitbricks/src/test/java/org/jclouds/profitbricks/features/NicApiMockTest.java (222)
M profitbricks/src/test/java/org/jclouds/profitbricks/http/parser/datacenter/DataCenterInfoResponseHandlerTest.java (114)
A profitbricks/src/test/java/org/jclouds/profitbricks/http/parser/firewall/FirewallListResponseHandlerTest.java (87)
A profitbricks/src/test/java/org/jclouds/profitbricks/http/parser/firewall/FirewallResponseHandlerTest.java (71)
A profitbricks/src/test/java/org/jclouds/profitbricks/http/parser/nic/NicListResponseHandlerTest.java (95)
A profitbricks/src/test/java/org/jclouds/profitbricks/http/parser/nic/NicResponseHandlerTest.java (66)
M profitbricks/src/test/java/org/jclouds/profitbricks/http/parser/server/ServerInfoResponseHandlerTest.java (72)
M profitbricks/src/test/java/org/jclouds/profitbricks/http/parser/server/ServerListResponseHandlerTest.java (147)
M profitbricks/src/test/resources/datacenter/datacenter.xml (3)
A profitbricks/src/test/resources/firewall/firewall-activate.xml (12)
A profitbricks/src/test/resources/firewall/firewall-addtonic.xml (23)
A profitbricks/src/test/resources/firewall/firewall-deactivate.xml (12)
A profitbricks/src/test/resources/firewall/firewall-delete.xml (12)
A profitbricks/src/test/resources/firewall/firewall-remove.xml (12)
A profitbricks/src/test/resources/firewall/firewall.xml (23)
A profitbricks/src/test/resources/firewall/firewalls.xml (39)
A profitbricks/src/test/resources/nic/nic-create.xml (13)
A profitbricks/src/test/resources/nic/nic-delete.xml (9)
A profitbricks/src/test/resources/nic/nic-internetaccess.xml (11)
A profitbricks/src/test/resources/nic/nic-update.xml (15)
A profitbricks/src/test/resources/nic/nic.xml (27)
A profitbricks/src/test/resources/nic/nics.xml (49)
M profitbricks/src/test/resources/server/servers.xml (1)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/139.patch
https://github.com/jclouds/jclouds-labs/pull/139.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/139
Re: [jclouds-labs] JCLOUDS-702: JCloud ProfitBricks provider - NIC &
Firewall API (#139)
Posted by Ignasi Barrera <no...@github.com>.
> +public class Strings3 {
> +
> + private static final String MAC_ADDR_FORMAT = "^([0-9a-f]{2}[:]){5}([0-9a-f]{2})$";
> + private static final String IP_ADDR_FORMAT
> + = "^([01]?\\d\\d?|2[0-4]\\d|25[0-5])\\."
> + + "([01]?\\d\\d?|2[0-4]\\d|25[0-5])\\."
> + + "([01]?\\d\\d?|2[0-4]\\d|25[0-5])\\."
> + + "([01]?\\d\\d?|2[0-4]\\d|25[0-5])$";
> + private static final Pattern MAC_ADDR_PATTERN = Pattern.compile(MAC_ADDR_FORMAT);
> + private static final Pattern IP_ADDR_PATTERN = Pattern.compile(IP_ADDR_FORMAT);
> +
> + public static boolean isMacAddress(String in) {
> + return MAC_ADDR_PATTERN.matcher(in).matches();
> + }
> +
> + public static boolean isIpAddress(String in) {
Prefer Guava's `InetAddresses.isInetAddress(String)` to this method?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/139/files#r25333044
Re: [jclouds-labs] JCLOUDS-702: JCloud ProfitBricks provider - NIC &
Firewall API (#139)
Posted by Ignasi Barrera <no...@github.com>.
>
> - }
> + }
Take care of the re-formatting of this class. Changing from spaces to tabs?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/139/files#r25332643
Re: [jclouds-labs] JCLOUDS-702: JCloud ProfitBricks provider - NIC &
Firewall API (#139)
Posted by Ignasi Barrera <no...@github.com>.
Thanks! Code looks pretty good. A couple global comments:
* Take a look at all those unrelated files that are included in this PR that are just reformatted (changing from spaces to tabs). can you remove them from this PR?
* Add a unit test for the Strings3 class, and consider renaming it to `MacAddresses` or similar? :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/139#issuecomment-75945007
Re: [jclouds-labs] JCLOUDS-702: JCloud ProfitBricks provider - NIC &
Firewall API (#139)
Posted by Ignasi Barrera <no...@github.com>.
Pushed to master as [c78a66c7](https://git1-us-west.apache.org/repos/asf?p=jclouds-labs.git;a=commit;h=c78a66c7). Thanks @devcsrj!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/139#issuecomment-76191471
Re: [jclouds-labs] JCLOUDS-702: JCloud ProfitBricks provider - NIC &
Firewall API (#139)
Posted by Reijhanniel Jearl Campos <no...@github.com>.
> + this.sourceMac = sourceMac;
> + return self();
> + }
> +
> + public B targetIp(String targetIp) {
> + this.targetIp = targetIp;
> + return self();
> + }
> +
> + public abstract B self();
> +
> + public abstract D build();
> +
> + protected void checkPortRange() {
> + checkArgument((portRangeEnd == null && portRangeStart == null)
> + || (portRangeEnd != null && portRangeStart != null), "Port range must be both present or null");
Oooh clever. That's something I need to remember
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/139/files#r25338122
Re: [jclouds-labs] JCLOUDS-702: JCloud ProfitBricks provider - NIC &
Firewall API (#139)
Posted by Reijhanniel Jearl Campos <no...@github.com>.
Hi @nacx ! As usual, thanks for the quick review!
So I addressed the initial issues and reformatted the whole project to use the correct formatting - this removed unrelated classes such as `ImageListResponseHandler`, `GetProvisioningStateResponseHandler`). This also fixed the diff on files like `Server` and `Storage` to only diff lines that are actually changed.
With that said, here ya go. :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/139#issuecomment-75961634
Re: [jclouds-labs] JCLOUDS-702: JCloud ProfitBricks provider - NIC &
Firewall API (#139)
Posted by Ignasi Barrera <no...@github.com>.
> + this.sourceMac = sourceMac;
> + return self();
> + }
> +
> + public B targetIp(String targetIp) {
> + this.targetIp = targetIp;
> + return self();
> + }
> +
> + public abstract B self();
> +
> + public abstract D build();
> +
> + protected void checkPortRange() {
> + checkArgument((portRangeEnd == null && portRangeStart == null)
> + || (portRangeEnd != null && portRangeStart != null), "Port range must be both present or null");
[nit] This can be more concise:
```java
checkArgument(!(portRangeEnd == null ^ portRangeStart == null), "Port range must be both present or null");
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/139/files#r25332571
Re: [jclouds-labs] JCLOUDS-702: JCloud ProfitBricks provider - NIC &
Firewall API (#139)
Posted by Ignasi Barrera <no...@github.com>.
> +package org.jclouds.profitbricks.util;
> +
> +import java.util.regex.Pattern;
> +
> +public class MacAddresses {
> +
> + private static final String MAC_ADDR_FORMAT = "^([0-9a-f]{2}[:]){5}([0-9a-f]{2})$";
> + private static final Pattern MAC_ADDR_PATTERN = Pattern.compile(MAC_ADDR_FORMAT);
> +
> + public static boolean isMacAddress(String in) {
> + return MAC_ADDR_PATTERN.matcher(in).matches();
> + }
> +
> + public static void main(String[] args) {
> + System.out.println(isMacAddress("AA:BB:CC:DD:EE:FF".toLowerCase()));
> + }
Remove this method :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/139/files#r25347482
Re: [jclouds-labs] JCLOUDS-702: JCloud ProfitBricks provider - NIC &
Firewall API (#139)
Posted by Ignasi Barrera <no...@github.com>.
Apart from the main method there, code lgtm. Mind squashing when fixing? I'll merge it then.
As usual, nice contribution! Thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/139#issuecomment-75979681
Re: [jclouds-labs] JCLOUDS-702: JCloud ProfitBricks provider - NIC &
Firewall API (#139)
Posted by Ignasi Barrera <no...@github.com>.
The Firewall class and the NicApiMockTest classes are usign Windows line endings. Mind fixing them?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/139#issuecomment-75992887
Re: [jclouds-labs] JCLOUDS-702: JCloud ProfitBricks provider - NIC &
Firewall API (#139)
Posted by Ignasi Barrera <no...@github.com>.
Closed #139.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/139#event-242539696
Re: [jclouds-labs] JCLOUDS-702: JCloud ProfitBricks provider - NIC &
Firewall API (#139)
Posted by Reijhanniel Jearl Campos <no...@github.com>.
> +package org.jclouds.profitbricks.util;
> +
> +import java.util.regex.Pattern;
> +
> +public class MacAddresses {
> +
> + private static final String MAC_ADDR_FORMAT = "^([0-9a-f]{2}[:]){5}([0-9a-f]{2})$";
> + private static final Pattern MAC_ADDR_PATTERN = Pattern.compile(MAC_ADDR_FORMAT);
> +
> + public static boolean isMacAddress(String in) {
> + return MAC_ADDR_PATTERN.matcher(in).matches();
> + }
> +
> + public static void main(String[] args) {
> + System.out.println(isMacAddress("AA:BB:CC:DD:EE:FF".toLowerCase()));
> + }
woops. Haha
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/139/files#r25347581