You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by duncangrant <gi...@git.apache.org> on 2016/08/08 13:13:06 UTC
[GitHub] brooklyn-server pull request #292: Shared location customizer
GitHub user duncangrant opened a pull request:
https://github.com/apache/brooklyn-server/pull/292
Shared location customizer
A location customizer that creates a single shared security group for each entity it is applied to as well as an individual entity. The shared entity allows full tcp and udp traffic within members of the group.
This PR includes 85c65ed078060cd01f74732b7cc4bddef8a58006 from @bostko 's #276.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/duncangrant/brooklyn-server extra-customizer
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/292.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #292
----
commit c6dcc3f73f792f4cbad25a530f811fe01c792c5b
Author: Valentin Aitken <bo...@gmail.com>
Date: 2016-07-29T11:04:38Z
Add BrooklynNetworkUtils.portRulesToRanges(Collection<String>)
commit 304a79ab18f57e446d0134de01de2cec7580ef3f
Author: Duncan Grant <du...@cloudsoftcorp.com>
Date: 2016-07-29T12:11:14Z
Add Shared security group customizer
Can be instantiated in yaml
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/292
Thanks @duncangrant - finished reviewing.
For the first commit, @bostko has incorporated those comments. I've created https://github.com/apache/brooklyn-server/pull/298 to get that merged asap (and I have added a few minor tweaks to that PR as well).
For your `SharedLocationSecurityGroupCustomizer`, it looks good. a few very minor comments. The only significant one is whether we add udp support as well. The reason I think it's worth it now is that it will affect our naming: `portRanges` will be ambiguous if we add udp in the future.
If you want, you could just name it "tcpPortRanges" now, and defer adding udp. Up to you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/292
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74581589
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java ---
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.jclouds.networking;
+
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.location.jclouds.BasicJcloudsLocationCustomizer;
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
+import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
+import org.apache.brooklyn.util.net.Cidr;
+import org.apache.brooklyn.util.net.Networking;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.net.domain.IpPermission;
+import org.jclouds.net.domain.IpProtocol;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Function;
+import com.google.common.base.Strings;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
+
+/**
+ * Configures a shared security group on Jclouds locations
+ * <p>
+ * Is based on {@link JcloudsLocationSecurityGroupCustomizer} but can be instantiated
+ * in yaml e.g.
+ * <p>
+ * <pre>
+ * {@code
+ * services:
+ * - type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess
+ * brooklyn.config:
+ * provisioning.properties:
+ * customizers:
+ * - $brooklyn:object:
+ * type: org.apache.brooklyn.location.jclouds.networking.SharedLocationSecurityGroupCustomizer
+ * object.fields: {locationName: "test", tcpPortRanges: ["900-910", "915", "22"], cidr: "82.40.153.101/24"}
+ * }
+ * </pre>
+*/
+public class SharedLocationSecurityGroupCustomizer extends BasicJcloudsLocationCustomizer {
+
+ private String locationName = null;
+
+ private String cidr = null;
+
+ /**
+ * We track inbound ports from the template to open them once we've created the new
+ * security groups
+ */
+ private int[] inboundPorts;
+
+ private RangeSet<Integer> tcpPortRanges;
+ private RangeSet<Integer> udpPortRanges;
+
+ /**
+ * The location name is appended to the name of the shared SG - use if you need distinct shared SGs within the same location
+ *
+ * @param locationName appended to the name of the SG
+ */
+ public void setLocationName(String locationName) {
+ this.locationName = locationName;
+ }
+
+ /**
+ * Extra ports to be opened on the entities in the SG
+ *
+ * @param tcpPortRanges
+ */
+ public void setTcpPortRanges(List<String> tcpPortRanges) {
+ this.tcpPortRanges = Networking.portRulesToRanges(tcpPortRanges);
+ }
+
+ public void setUdpPortRanges(ImmutableList<String> udpPortRanges) {
+ this.udpPortRanges = Networking.portRulesToRanges(udpPortRanges);
+ }
+
+ /**
+ * Set to restict the range that the ports that are to be opened can be accessed from
+ *
+ * @param cidr
+ */
+ public void setCidr(String cidr) {
+ this.cidr = cidr;
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, Template template) {
+ super.customize(location, computeService, template);
+
+ inboundPorts = template.getOptions().getInboundPorts();
+
+ final JcloudsLocationSecurityGroupCustomizer instance = getInstance(getSharedGroupId(location));
+
+ if (cidr != null) instance.setSshCidrSupplier(Suppliers.ofInstance(new Cidr(cidr)));
+
+ instance.customize(location, computeService, template);
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, JcloudsMachineLocation machine) {
+ super.customize(location, computeService, machine);
+
--- End diff --
why
are
there
lots
of blank lines?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #292: Shared location customizer
Posted by duncangrant <gi...@git.apache.org>.
Github user duncangrant commented on the issue:
https://github.com/apache/brooklyn-server/pull/292
@aledsage @neykov I think I've addressed all your comments
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by duncangrant <gi...@git.apache.org>.
Github user duncangrant commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74493342
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java ---
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.jclouds.networking;
+
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.location.jclouds.BasicJcloudsLocationCustomizer;
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
+import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
+import org.apache.brooklyn.util.core.BrooklynNetworkUtils;
+import org.apache.brooklyn.util.net.Cidr;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.compute.options.TemplateOptions;
+import org.jclouds.net.domain.IpPermission;
+import org.jclouds.net.domain.IpProtocol;
+
+import com.google.common.base.Function;
+import com.google.common.base.Strings;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
+
+/**
+ * Configures a shared security group on Jclouds locations
+ * <p>
+ * Is based on {@link JcloudsLocationSecurityGroupCustomizer} but can be instantiated
+ * in yaml e.g.
+ * <p>
+ * services:
+ * - type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess
+ * brooklyn.config:
+ * provisioning.properties:
+ * customizers:
+ * - $brooklyn:object:
+ * type: org.apache.brooklyn.location.jclouds.networking.SharedLocationSecurityGroupCustomizer
+ * object.fields: {locationName: "test", portRanges: ["900-910","915", "22"], cidr: "82.40.153.101/24"}
+ */
+public class SharedLocationSecurityGroupCustomizer extends BasicJcloudsLocationCustomizer {
--- End diff --
I don't think there is anything worth pushing into JcloudsLocationSecurityGroupCustomizer. I would have like to just modify it to have a no args constructor that would let us work with it in yaml but couldn't quite see a way to do it without changing its behaviour.
It tries to be idempotent but then it also seems to catch (and ignore) the error if it does try to add a range a second time so possibly that can happen but I can't see where.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by duncangrant <gi...@git.apache.org>.
Github user duncangrant commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74493525
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java ---
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.jclouds.networking;
+
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.location.jclouds.BasicJcloudsLocationCustomizer;
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
+import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
+import org.apache.brooklyn.util.core.BrooklynNetworkUtils;
+import org.apache.brooklyn.util.net.Cidr;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.compute.options.TemplateOptions;
+import org.jclouds.net.domain.IpPermission;
+import org.jclouds.net.domain.IpProtocol;
+
+import com.google.common.base.Function;
+import com.google.common.base.Strings;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
+
+/**
+ * Configures a shared security group on Jclouds locations
+ * <p>
+ * Is based on {@link JcloudsLocationSecurityGroupCustomizer} but can be instantiated
+ * in yaml e.g.
+ * <p>
+ * services:
+ * - type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess
+ * brooklyn.config:
+ * provisioning.properties:
+ * customizers:
+ * - $brooklyn:object:
+ * type: org.apache.brooklyn.location.jclouds.networking.SharedLocationSecurityGroupCustomizer
+ * object.fields: {locationName: "test", portRanges: ["900-910","915", "22"], cidr: "82.40.153.101/24"}
+ */
+public class SharedLocationSecurityGroupCustomizer extends BasicJcloudsLocationCustomizer {
+
+ private String locationName = null;
+
+ private List<String> portRanges = null;
+
+ private String cidr = null;
+
+ /**
+ * We track inbound ports from the template to open them once we've created the new
+ * security groups
+ */
+ private int[] inboundPorts;
+
+ /**
+ * The location name is appended to the name of the shared SG - use if you need distinct shared SGs within the same location
+ *
+ * @param locationName appended to the name of the SG
+ */
+ public void setLocationName(String locationName) {
+ this.locationName = locationName;
+ }
+
+ /**
+ * Extra ports to be opened on the entities in the SG
+ *
+ * @param portRanges
+ */
+ public void setPortRanges(List<String> portRanges) {
+ this.portRanges = portRanges;
+ }
+
+ /**
+ * Set to restict the range that the ports that are to be opened can be accessed from
+ *
+ * @param cidr
+ */
+ public void setCidr(String cidr) {
+ this.cidr = cidr;
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, TemplateOptions templateOptions) {
+ super.customize(location, computeService, templateOptions);
+ inboundPorts = templateOptions.getInboundPorts();
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, Template template) {
+ super.customize(location, computeService, template);
+
+ final JcloudsLocationSecurityGroupCustomizer instance = getInstance(getSharedGroupId(location));
+
+ if (cidr != null) instance.setSshCidrSupplier(Suppliers.ofInstance(new Cidr(cidr)));
+
+ instance.customize(location, computeService, template);
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, JcloudsMachineLocation machine) {
+ super.customize(location, computeService, machine);
+
+ final JcloudsLocationSecurityGroupCustomizer instance = getInstance(getSharedGroupId(location));
+
+ if (portRanges != null) {
+ RangeSet<Integer> portRanges = BrooklynNetworkUtils.portRulesToRanges(this.portRanges);
+
+ List<IpPermission> ipPermissions =
+ FluentIterable
+ .from(portRanges.asRanges())
+ .transform(new Function<Range<Integer>, IpPermission>() {
+ @Nullable
+ @Override
+ public IpPermission apply(@Nullable Range<Integer> integerRange) {
+ IpPermission extraPermission = IpPermission.builder()
+ .fromPort(integerRange.lowerEndpoint())
+ .toPort(integerRange.upperEndpoint())
+ .ipProtocol(IpProtocol.TCP)
--- End diff --
@grkvlt suggested that too so I'll add it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/292
@duncangrant there are merge conflicts - can you rebase against master please? (Jenkins won't run the tests until that conflict is resolved).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74462321
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java ---
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.jclouds.networking;
+
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.location.jclouds.BasicJcloudsLocationCustomizer;
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
+import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
+import org.apache.brooklyn.util.core.BrooklynNetworkUtils;
+import org.apache.brooklyn.util.net.Cidr;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.compute.options.TemplateOptions;
+import org.jclouds.net.domain.IpPermission;
+import org.jclouds.net.domain.IpProtocol;
+
+import com.google.common.base.Function;
+import com.google.common.base.Strings;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
+
+/**
+ * Configures a shared security group on Jclouds locations
+ * <p>
+ * Is based on {@link JcloudsLocationSecurityGroupCustomizer} but can be instantiated
+ * in yaml e.g.
+ * <p>
+ * services:
+ * - type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess
+ * brooklyn.config:
+ * provisioning.properties:
+ * customizers:
+ * - $brooklyn:object:
+ * type: org.apache.brooklyn.location.jclouds.networking.SharedLocationSecurityGroupCustomizer
+ * object.fields: {locationName: "test", portRanges: ["900-910","915", "22"], cidr: "82.40.153.101/24"}
--- End diff --
nit-pick: consistency of whether there's a space after the comma.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/292
@duncangrant code looks good. A couple of minor parameter renames would make it clearer. Once merge conflicts are dealt with and jenkins confirms tests pass, then good to merge (ideally with the renames also done).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74409198
--- Diff: core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java ---
@@ -19,24 +19,52 @@
package org.apache.brooklyn.util.core;
import java.net.InetAddress;
+import java.util.Collection;
import org.apache.brooklyn.core.location.geo.LocalhostExternalIpLoader;
import org.apache.brooklyn.core.server.BrooklynServiceAttributes;
import org.apache.brooklyn.util.JavaGroovyEquivalents;
import org.apache.brooklyn.util.core.flags.TypeCoercions;
import org.apache.brooklyn.util.net.Networking;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
+import com.google.common.collect.TreeRangeSet;
+
public class BrooklynNetworkUtils {
- /** returns the externally-facing IP address from which this host comes, or 127.0.0.1 if not resolvable */
+ /**
+ * returns the externally-facing IP address from which this host comes, or 127.0.0.1 if not resolvable
+ */
public static String getLocalhostExternalIp() {
return LocalhostExternalIpLoader.getLocalhostIpQuicklyOrDefault();
}
- /** returns a IP address for localhost paying attention to a system property to prevent lookup in some cases */
+ /**
+ * returns a IP address for localhost paying attention to a system property to prevent lookup in some cases
+ */
public static InetAddress getLocalhostInetAddress() {
- return TypeCoercions.coerce(JavaGroovyEquivalents.elvis(BrooklynServiceAttributes.LOCALHOST_IP_ADDRESS.getValue(),
+ return TypeCoercions.coerce(JavaGroovyEquivalents.elvis(BrooklynServiceAttributes.LOCALHOST_IP_ADDRESS.getValue(),
Networking.getLocalHost()), InetAddress.class);
}
+ public static RangeSet<Integer> portRulesToRanges(Collection<String> portRules) {
+ RangeSet<Integer> result = TreeRangeSet.create();
+ for (String portRule : portRules) {
+ if (portRule.contains("-")) {
+ String[] fromTo = portRule.split("-");
+ Preconditions.checkState(fromTo.length == 2);
--- End diff --
I know this came from @bostko 's commit, but if might merge it here...
I'd much prefer us to give a message. This will currently just throw an `IllegalStateException` with no messsage. Someone would need to look at the code to understand what went wrong. I'd change this to something like:
Preconditions.checkState(fromTo.length == 2, "Invalid port range '%s'", portRule);
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74479809
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java ---
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.jclouds.networking;
+
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.location.jclouds.BasicJcloudsLocationCustomizer;
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
+import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
+import org.apache.brooklyn.util.core.BrooklynNetworkUtils;
+import org.apache.brooklyn.util.net.Cidr;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.compute.options.TemplateOptions;
+import org.jclouds.net.domain.IpPermission;
+import org.jclouds.net.domain.IpProtocol;
+
+import com.google.common.base.Function;
+import com.google.common.base.Strings;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
+
+/**
+ * Configures a shared security group on Jclouds locations
+ * <p>
+ * Is based on {@link JcloudsLocationSecurityGroupCustomizer} but can be instantiated
+ * in yaml e.g.
+ * <p>
+ * services:
+ * - type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess
+ * brooklyn.config:
+ * provisioning.properties:
+ * customizers:
+ * - $brooklyn:object:
+ * type: org.apache.brooklyn.location.jclouds.networking.SharedLocationSecurityGroupCustomizer
+ * object.fields: {locationName: "test", portRanges: ["900-910","915", "22"], cidr: "82.40.153.101/24"}
+ */
+public class SharedLocationSecurityGroupCustomizer extends BasicJcloudsLocationCustomizer {
+
+ private String locationName = null;
+
+ private List<String> portRanges = null;
+
+ private String cidr = null;
+
+ /**
+ * We track inbound ports from the template to open them once we've created the new
+ * security groups
+ */
+ private int[] inboundPorts;
+
+ /**
+ * The location name is appended to the name of the shared SG - use if you need distinct shared SGs within the same location
+ *
+ * @param locationName appended to the name of the SG
+ */
+ public void setLocationName(String locationName) {
+ this.locationName = locationName;
+ }
+
+ /**
+ * Extra ports to be opened on the entities in the SG
+ *
+ * @param portRanges
+ */
+ public void setPortRanges(List<String> portRanges) {
+ this.portRanges = portRanges;
+ }
+
+ /**
+ * Set to restict the range that the ports that are to be opened can be accessed from
+ *
+ * @param cidr
+ */
+ public void setCidr(String cidr) {
+ this.cidr = cidr;
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, TemplateOptions templateOptions) {
+ super.customize(location, computeService, templateOptions);
+ inboundPorts = templateOptions.getInboundPorts();
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, Template template) {
+ super.customize(location, computeService, template);
+
+ final JcloudsLocationSecurityGroupCustomizer instance = getInstance(getSharedGroupId(location));
+
+ if (cidr != null) instance.setSshCidrSupplier(Suppliers.ofInstance(new Cidr(cidr)));
+
+ instance.customize(location, computeService, template);
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, JcloudsMachineLocation machine) {
+ super.customize(location, computeService, machine);
+
+ final JcloudsLocationSecurityGroupCustomizer instance = getInstance(getSharedGroupId(location));
+
+ if (portRanges != null) {
+ RangeSet<Integer> portRanges = BrooklynNetworkUtils.portRulesToRanges(this.portRanges);
+
+ List<IpPermission> ipPermissions =
+ FluentIterable
+ .from(portRanges.asRanges())
+ .transform(new Function<Range<Integer>, IpPermission>() {
+ @Nullable
+ @Override
+ public IpPermission apply(@Nullable Range<Integer> integerRange) {
+ IpPermission extraPermission = IpPermission.builder()
+ .fromPort(integerRange.lowerEndpoint())
+ .toPort(integerRange.upperEndpoint())
+ .ipProtocol(IpProtocol.TCP)
--- End diff --
Can we support both tcp and upd ports? I presume it would be fairly easy (e.g. change the `portRanges` field and setter to be `tcpPortRanges`, and add a similar field+setter for `udpPortRanges`)?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74410202
--- Diff: core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java ---
@@ -19,24 +19,52 @@
package org.apache.brooklyn.util.core;
import java.net.InetAddress;
+import java.util.Collection;
import org.apache.brooklyn.core.location.geo.LocalhostExternalIpLoader;
import org.apache.brooklyn.core.server.BrooklynServiceAttributes;
import org.apache.brooklyn.util.JavaGroovyEquivalents;
import org.apache.brooklyn.util.core.flags.TypeCoercions;
import org.apache.brooklyn.util.net.Networking;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
+import com.google.common.collect.TreeRangeSet;
+
public class BrooklynNetworkUtils {
- /** returns the externally-facing IP address from which this host comes, or 127.0.0.1 if not resolvable */
+ /**
+ * returns the externally-facing IP address from which this host comes, or 127.0.0.1 if not resolvable
+ */
public static String getLocalhostExternalIp() {
return LocalhostExternalIpLoader.getLocalhostIpQuicklyOrDefault();
}
- /** returns a IP address for localhost paying attention to a system property to prevent lookup in some cases */
+ /**
+ * returns a IP address for localhost paying attention to a system property to prevent lookup in some cases
+ */
public static InetAddress getLocalhostInetAddress() {
- return TypeCoercions.coerce(JavaGroovyEquivalents.elvis(BrooklynServiceAttributes.LOCALHOST_IP_ADDRESS.getValue(),
+ return TypeCoercions.coerce(JavaGroovyEquivalents.elvis(BrooklynServiceAttributes.LOCALHOST_IP_ADDRESS.getValue(),
Networking.getLocalHost()), InetAddress.class);
}
+ public static RangeSet<Integer> portRulesToRanges(Collection<String> portRules) {
--- End diff --
This should probably go in `Networking` instead of `BrooklynNetworkUtils`.
The difference between the two classes seems to be:
* `Networking` is for generic network utility methods.
* `BrooklynNetworkUtils` is for utility methods that rely on some other part(s) of Brooklyn, or seem too custom in how they are used/configured to be considered a "common utility". For example `getLocalhostExternalIp()` will load the resource `external-ip-address-resolvers.txt` that tells us about external service(s) to use, such as http://jsonip.com/, and will contact that in a time-limited way to get + cache the ip).
Worth adding javadoc to the two classes to that affect, and to include in the latter a reference to `{@link org.apache.brooklyn.util.net.Networking}`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74410438
--- Diff: core/src/main/java/org/apache/brooklyn/util/core/BrooklynNetworkUtils.java ---
@@ -19,24 +19,52 @@
package org.apache.brooklyn.util.core;
import java.net.InetAddress;
+import java.util.Collection;
import org.apache.brooklyn.core.location.geo.LocalhostExternalIpLoader;
import org.apache.brooklyn.core.server.BrooklynServiceAttributes;
import org.apache.brooklyn.util.JavaGroovyEquivalents;
import org.apache.brooklyn.util.core.flags.TypeCoercions;
import org.apache.brooklyn.util.net.Networking;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
+import com.google.common.collect.TreeRangeSet;
+
public class BrooklynNetworkUtils {
- /** returns the externally-facing IP address from which this host comes, or 127.0.0.1 if not resolvable */
+ /**
+ * returns the externally-facing IP address from which this host comes, or 127.0.0.1 if not resolvable
+ */
public static String getLocalhostExternalIp() {
return LocalhostExternalIpLoader.getLocalhostIpQuicklyOrDefault();
}
- /** returns a IP address for localhost paying attention to a system property to prevent lookup in some cases */
+ /**
+ * returns a IP address for localhost paying attention to a system property to prevent lookup in some cases
+ */
public static InetAddress getLocalhostInetAddress() {
- return TypeCoercions.coerce(JavaGroovyEquivalents.elvis(BrooklynServiceAttributes.LOCALHOST_IP_ADDRESS.getValue(),
+ return TypeCoercions.coerce(JavaGroovyEquivalents.elvis(BrooklynServiceAttributes.LOCALHOST_IP_ADDRESS.getValue(),
Networking.getLocalHost()), InetAddress.class);
}
+ public static RangeSet<Integer> portRulesToRanges(Collection<String> portRules) {
+ RangeSet<Integer> result = TreeRangeSet.create();
+ for (String portRule : portRules) {
+ if (portRule.contains("-")) {
+ String[] fromTo = portRule.split("-");
+ Preconditions.checkState(fromTo.length == 2);
+ result.add(closedRange(fromTo[0], fromTo[1]));
+ } else {
+ result.add(closedRange(portRule, portRule));
+ }
+ }
+ return result;
+ }
+
+ private static Range<Integer> closedRange(String from, String to) {
+ return Range.closed(Integer.parseInt(from), Integer.parseInt(to));
--- End diff --
Should we fail-fast with numbers that are out of range (i.e. negative or greater than `Networking.MAX_PORT_NUMBER`)? Any use-case for a kind of port that can exceed 65535 (I don't think there is)?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74462494
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java ---
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.jclouds.networking;
+
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.location.jclouds.BasicJcloudsLocationCustomizer;
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
+import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
+import org.apache.brooklyn.util.core.BrooklynNetworkUtils;
+import org.apache.brooklyn.util.net.Cidr;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.compute.options.TemplateOptions;
+import org.jclouds.net.domain.IpPermission;
+import org.jclouds.net.domain.IpProtocol;
+
+import com.google.common.base.Function;
+import com.google.common.base.Strings;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
+
+/**
+ * Configures a shared security group on Jclouds locations
+ * <p>
+ * Is based on {@link JcloudsLocationSecurityGroupCustomizer} but can be instantiated
+ * in yaml e.g.
+ * <p>
+ * services:
+ * - type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess
+ * brooklyn.config:
+ * provisioning.properties:
+ * customizers:
+ * - $brooklyn:object:
+ * type: org.apache.brooklyn.location.jclouds.networking.SharedLocationSecurityGroupCustomizer
+ * object.fields: {locationName: "test", portRanges: ["900-910","915", "22"], cidr: "82.40.153.101/24"}
+ */
+public class SharedLocationSecurityGroupCustomizer extends BasicJcloudsLocationCustomizer {
+
+ private String locationName = null;
+
+ private List<String> portRanges = null;
+
+ private String cidr = null;
+
+ /**
+ * We track inbound ports from the template to open them once we've created the new
+ * security groups
+ */
+ private int[] inboundPorts;
+
+ /**
+ * The location name is appended to the name of the shared SG - use if you need distinct shared SGs within the same location
+ *
+ * @param locationName appended to the name of the SG
+ */
+ public void setLocationName(String locationName) {
+ this.locationName = locationName;
+ }
+
+ /**
+ * Extra ports to be opened on the entities in the SG
+ *
+ * @param portRanges
+ */
+ public void setPortRanges(List<String> portRanges) {
+ this.portRanges = portRanges;
+ }
+
+ /**
+ * Set to restict the range that the ports that are to be opened can be accessed from
+ *
+ * @param cidr
+ */
+ public void setCidr(String cidr) {
+ this.cidr = cidr;
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, TemplateOptions templateOptions) {
+ super.customize(location, computeService, templateOptions);
+ inboundPorts = templateOptions.getInboundPorts();
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, Template template) {
--- End diff --
+1; you can hopefully do `template.getTemplateOptions().getInboundPorts()` here.
The other method is intended for when you want to modify the templateOptions, before it is used.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74462950
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java ---
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.jclouds.networking;
+
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.location.jclouds.BasicJcloudsLocationCustomizer;
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
+import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
+import org.apache.brooklyn.util.core.BrooklynNetworkUtils;
+import org.apache.brooklyn.util.net.Cidr;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.compute.options.TemplateOptions;
+import org.jclouds.net.domain.IpPermission;
+import org.jclouds.net.domain.IpProtocol;
+
+import com.google.common.base.Function;
+import com.google.common.base.Strings;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
+
+/**
+ * Configures a shared security group on Jclouds locations
+ * <p>
+ * Is based on {@link JcloudsLocationSecurityGroupCustomizer} but can be instantiated
+ * in yaml e.g.
+ * <p>
+ * services:
--- End diff --
Surround with the `<pre>` etc - for an example, see javadoc of https://github.com/apache/brooklyn-server/blob/master/core/src/main/java/org/apache/brooklyn/feed/ssh/SshFeed.java
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74480881
--- Diff: locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizerTest.java ---
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.jclouds.networking;
+
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.*;
+import static org.testng.Assert.assertEquals;
+
+import java.util.List;
+
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
+import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.net.Cidr;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.compute.extensions.SecurityGroupExtension;
+import org.jclouds.domain.Location;
+import org.jclouds.net.domain.IpPermission;
+import org.mockito.Answers;
+import org.mockito.ArgumentCaptor;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Supplier;
+import com.google.common.collect.ImmutableList;
+
+public class SharedLocationSecurityGroupCustomizerTest {
+
+ TestSharedLocationSecurityGroupCustomizer customizer;
+ JcloudsLocationSecurityGroupCustomizer sgCustomizer;
+ ComputeService computeService;
+ Location location;
+ SecurityGroupExtension securityApi;
+ private JcloudsLocation jcloudsLocation = new JcloudsLocation(MutableMap.of("deferConstruction", true));
+
+ @BeforeMethod
+ public void setUp() {
+ sgCustomizer = mock(JcloudsLocationSecurityGroupCustomizer.class);
+ customizer = new TestSharedLocationSecurityGroupCustomizer();
+ location = mock(Location.class);
+ securityApi = mock(SecurityGroupExtension.class);
+ computeService = mock(ComputeService.class, Answers.RETURNS_DEEP_STUBS.get());
+ when(computeService.getSecurityGroupExtension()).thenReturn(Optional.of(securityApi));
+ }
+
+ @Test
+ public void testLocationNameAppended() {
+ customizer.setLocationName("Fred");
+ customizer.customize(jcloudsLocation, computeService, mock(Template.class));
+ assertEquals(customizer.sharedGroupId, "Fred-" + jcloudsLocation.getId());
+ }
+
+ @Test
+ public void testCidrIsSetIfAvailable() {
+ // Set cidr with over specified ip range which will be fixed by Cidr object
+ customizer.setCidr("1.1.1.1/24");
+ customizer.customize(jcloudsLocation, computeService, mock(Template.class));
+ ArgumentCaptor<Supplier<Cidr>> argumentCaptor = ArgumentCaptor.forClass((Class)Supplier.class);
+ verify(sgCustomizer).setSshCidrSupplier(argumentCaptor.capture());
+ Cidr cidr = argumentCaptor.getValue().get();
+ assertEquals(cidr.toString(), "1.1.1.0/24");
+ }
+
+ @Test
+ public void testCidrNotSetIfNotavailable() {
+ customizer.customize(jcloudsLocation, computeService, mock(Template.class));
+ verify(sgCustomizer, never()).setSshCidrSupplier(any(Supplier.class));
+ }
+
+
+ @Test
+ public void testPermissionsSetFromIpRanges() {
+ customizer.setPortRanges(ImmutableList.of("99-100"));
+ when(sgCustomizer.getBrooklynCidrBlock()).thenReturn("10.10.10.10/24");
+ customizer.customize(jcloudsLocation, computeService, mock(JcloudsMachineLocation.class));
+ ArgumentCaptor<List> listArgumentCaptor = ArgumentCaptor.forClass(List.class);
--- End diff --
Nice unit tests! I hadn't seen `ArgumentCaptor` before. Looks really useful!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/292
Thanks @duncangrant - merging now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74463259
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java ---
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.jclouds.networking;
+
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.location.jclouds.BasicJcloudsLocationCustomizer;
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
+import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
+import org.apache.brooklyn.util.core.BrooklynNetworkUtils;
+import org.apache.brooklyn.util.net.Cidr;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.compute.options.TemplateOptions;
+import org.jclouds.net.domain.IpPermission;
+import org.jclouds.net.domain.IpProtocol;
+
+import com.google.common.base.Function;
+import com.google.common.base.Strings;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
+
+/**
+ * Configures a shared security group on Jclouds locations
+ * <p>
+ * Is based on {@link JcloudsLocationSecurityGroupCustomizer} but can be instantiated
+ * in yaml e.g.
+ * <p>
+ * services:
+ * - type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess
+ * brooklyn.config:
+ * provisioning.properties:
+ * customizers:
+ * - $brooklyn:object:
+ * type: org.apache.brooklyn.location.jclouds.networking.SharedLocationSecurityGroupCustomizer
+ * object.fields: {locationName: "test", portRanges: ["900-910","915", "22"], cidr: "82.40.153.101/24"}
+ */
+public class SharedLocationSecurityGroupCustomizer extends BasicJcloudsLocationCustomizer {
+
+ private String locationName = null;
+
+ private List<String> portRanges = null;
+
+ private String cidr = null;
+
+ /**
+ * We track inbound ports from the template to open them once we've created the new
+ * security groups
+ */
+ private int[] inboundPorts;
+
+ /**
+ * The location name is appended to the name of the shared SG - use if you need distinct shared SGs within the same location
+ *
+ * @param locationName appended to the name of the SG
+ */
+ public void setLocationName(String locationName) {
+ this.locationName = locationName;
+ }
+
+ /**
+ * Extra ports to be opened on the entities in the SG
+ *
+ * @param portRanges
+ */
+ public void setPortRanges(List<String> portRanges) {
+ this.portRanges = portRanges;
+ }
+
+ /**
+ * Set to restict the range that the ports that are to be opened can be accessed from
+ *
+ * @param cidr
+ */
+ public void setCidr(String cidr) {
+ this.cidr = cidr;
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, TemplateOptions templateOptions) {
+ super.customize(location, computeService, templateOptions);
+ inboundPorts = templateOptions.getInboundPorts();
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, Template template) {
+ super.customize(location, computeService, template);
+
+ final JcloudsLocationSecurityGroupCustomizer instance = getInstance(getSharedGroupId(location));
+
+ if (cidr != null) instance.setSshCidrSupplier(Suppliers.ofInstance(new Cidr(cidr)));
+
+ instance.customize(location, computeService, template);
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, JcloudsMachineLocation machine) {
+ super.customize(location, computeService, machine);
+
+ final JcloudsLocationSecurityGroupCustomizer instance = getInstance(getSharedGroupId(location));
+
+ if (portRanges != null) {
+ RangeSet<Integer> portRanges = BrooklynNetworkUtils.portRulesToRanges(this.portRanges);
+
+ List<IpPermission> ipPermissions =
+ FluentIterable
+ .from(portRanges.asRanges())
+ .transform(new Function<Range<Integer>, IpPermission>() {
+ @Nullable
+ @Override
+ public IpPermission apply(@Nullable Range<Integer> integerRange) {
+ IpPermission extraPermission = IpPermission.builder()
+ .fromPort(integerRange.lowerEndpoint())
+ .toPort(integerRange.upperEndpoint())
+ .ipProtocol(IpProtocol.TCP)
+ .cidrBlock(instance.getBrooklynCidrBlock())
+ .build();
+ return extraPermission;
+ }
+ })
+ .toList();
+
+ instance.addPermissionsToLocation(machine, ipPermissions);
+ }
+
+ if(inboundPorts != null) {
+ for (int inboundPort : inboundPorts) {
+ IpPermission ipPermission = IpPermission.builder()
+ .fromPort(inboundPort)
+ .toPort(inboundPort)
+ .ipProtocol(IpProtocol.TCP)
+ .cidrBlock(instance.getBrooklynCidrBlock())
+ .build();
+ instance.addPermissionsToLocation(machine, ipPermission);
+ }
+ }
+ }
+
+ private String getSharedGroupId(JcloudsLocation location) {
+ return (Strings.isNullOrEmpty(locationName))
+ ? location.getId()
+ : locationName + "-" + location.getId();
+ }
+
+ //Override-able in Tests
--- End diff --
I like the Guava annotation `@VisibleForTesting`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74505759
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java ---
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.jclouds.networking;
+
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.location.jclouds.BasicJcloudsLocationCustomizer;
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
+import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
+import org.apache.brooklyn.util.core.BrooklynNetworkUtils;
+import org.apache.brooklyn.util.net.Cidr;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.net.domain.IpPermission;
+import org.jclouds.net.domain.IpProtocol;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Function;
+import com.google.common.base.Strings;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
+
+/**
+ * Configures a shared security group on Jclouds locations
+ * <p>
+ * Is based on {@link JcloudsLocationSecurityGroupCustomizer} but can be instantiated
+ * in yaml e.g.
+ * <p>
+ * <pre>
+ * {@code
+ * services:
+ * - type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess
+ * brooklyn.config:
+ * provisioning.properties:
+ * customizers:
+ * - $brooklyn:object:
+ * type: org.apache.brooklyn.location.jclouds.networking.SharedLocationSecurityGroupCustomizer
+ * object.fields: {locationName: "test", tcpPortRanges: ["900-910", "915", "22"], cidr: "82.40.153.101/24"}
+ * }
+ * </pre>
+*/
+public class SharedLocationSecurityGroupCustomizer extends BasicJcloudsLocationCustomizer {
+
+ private String locationName = null;
+
+ private String cidr = null;
+
+ /**
+ * We track inbound ports from the template to open them once we've created the new
+ * security groups
+ */
+ private int[] inboundPorts;
+
+ private RangeSet<Integer> tcpPortRanges;
+ private RangeSet<Integer> udpPortRanges;
+
+ /**
+ * The location name is appended to the name of the shared SG - use if you need distinct shared SGs within the same location
+ *
+ * @param locationName appended to the name of the SG
+ */
+ public void setLocationName(String locationName) {
+ this.locationName = locationName;
+ }
+
+ /**
+ * Extra ports to be opened on the entities in the SG
+ *
+ * @param tcpPortRanges
+ */
+ public void setTcpPortRanges(List<String> tcpPortRanges) {
+ this.tcpPortRanges = BrooklynNetworkUtils.portRulesToRanges(tcpPortRanges);
+ }
+
+ public void setUdpPortRanges(ImmutableList<String> udpPortRanges) {
+ this.udpPortRanges = BrooklynNetworkUtils.portRulesToRanges(udpPortRanges);
+ }
+
+ /**
+ * Set to restict the range that the ports that are to be opened can be accessed from
+ *
+ * @param cidr
+ */
+ public void setCidr(String cidr) {
+ this.cidr = cidr;
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, Template template) {
+ super.customize(location, computeService, template);
+
+ inboundPorts = template.getOptions().getInboundPorts();
+
+ final JcloudsLocationSecurityGroupCustomizer instance = getInstance(getSharedGroupId(location));
+
+ if (cidr != null) instance.setSshCidrSupplier(Suppliers.ofInstance(new Cidr(cidr)));
+
+ instance.customize(location, computeService, template);
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, JcloudsMachineLocation machine) {
+ super.customize(location, computeService, machine);
+
+ final JcloudsLocationSecurityGroupCustomizer instance = getInstance(getSharedGroupId(location));
+
+ ImmutableList.Builder<IpPermission> builder = ImmutableList.<IpPermission>builder();
+
+ builder.addAll(getIpPermissions(instance, tcpPortRanges, IpProtocol.TCP));
+
+ builder.addAll(getIpPermissions(instance, udpPortRanges, IpProtocol.UDP));
+
+ if (inboundPorts != null) {
+
+ for (int inboundPort : inboundPorts) {
+ IpPermission ipPermission = IpPermission.builder()
+ .fromPort(inboundPort)
+ .toPort(inboundPort)
+ .ipProtocol(IpProtocol.TCP)
+ .cidrBlock(instance.getBrooklynCidrBlock())
+ .build();
+ builder.add(ipPermission);
+ }
+ }
+ instance.addPermissionsToLocation(machine, builder.build());
+ }
+
+ private List<IpPermission> getIpPermissions(JcloudsLocationSecurityGroupCustomizer instance, RangeSet<Integer> tcpPortRanges, IpProtocol protocol) {
+ List<IpPermission> ipPermissions = ImmutableList.<IpPermission>of();
+ if (tcpPortRanges != null) {
+ ipPermissions =
+ FluentIterable
+ .from(tcpPortRanges.asRanges())
--- End diff --
Can you rename this variable to `portRanges`, because it is called for both tcp and udp so is a bit confusing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74505875
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java ---
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.jclouds.networking;
+
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.location.jclouds.BasicJcloudsLocationCustomizer;
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
+import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
+import org.apache.brooklyn.util.core.BrooklynNetworkUtils;
+import org.apache.brooklyn.util.net.Cidr;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.net.domain.IpPermission;
+import org.jclouds.net.domain.IpProtocol;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Function;
+import com.google.common.base.Strings;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
+
+/**
+ * Configures a shared security group on Jclouds locations
+ * <p>
+ * Is based on {@link JcloudsLocationSecurityGroupCustomizer} but can be instantiated
+ * in yaml e.g.
+ * <p>
+ * <pre>
+ * {@code
+ * services:
+ * - type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess
+ * brooklyn.config:
+ * provisioning.properties:
+ * customizers:
+ * - $brooklyn:object:
+ * type: org.apache.brooklyn.location.jclouds.networking.SharedLocationSecurityGroupCustomizer
+ * object.fields: {locationName: "test", tcpPortRanges: ["900-910", "915", "22"], cidr: "82.40.153.101/24"}
+ * }
+ * </pre>
+*/
+public class SharedLocationSecurityGroupCustomizer extends BasicJcloudsLocationCustomizer {
+
+ private String locationName = null;
+
+ private String cidr = null;
+
+ /**
+ * We track inbound ports from the template to open them once we've created the new
+ * security groups
+ */
+ private int[] inboundPorts;
+
+ private RangeSet<Integer> tcpPortRanges;
+ private RangeSet<Integer> udpPortRanges;
+
+ /**
+ * The location name is appended to the name of the shared SG - use if you need distinct shared SGs within the same location
+ *
+ * @param locationName appended to the name of the SG
+ */
+ public void setLocationName(String locationName) {
+ this.locationName = locationName;
+ }
+
+ /**
+ * Extra ports to be opened on the entities in the SG
+ *
+ * @param tcpPortRanges
+ */
+ public void setTcpPortRanges(List<String> tcpPortRanges) {
+ this.tcpPortRanges = BrooklynNetworkUtils.portRulesToRanges(tcpPortRanges);
+ }
+
+ public void setUdpPortRanges(ImmutableList<String> udpPortRanges) {
+ this.udpPortRanges = BrooklynNetworkUtils.portRulesToRanges(udpPortRanges);
+ }
+
+ /**
+ * Set to restict the range that the ports that are to be opened can be accessed from
+ *
+ * @param cidr
+ */
+ public void setCidr(String cidr) {
+ this.cidr = cidr;
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, Template template) {
+ super.customize(location, computeService, template);
+
+ inboundPorts = template.getOptions().getInboundPorts();
+
+ final JcloudsLocationSecurityGroupCustomizer instance = getInstance(getSharedGroupId(location));
+
+ if (cidr != null) instance.setSshCidrSupplier(Suppliers.ofInstance(new Cidr(cidr)));
+
+ instance.customize(location, computeService, template);
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, JcloudsMachineLocation machine) {
+ super.customize(location, computeService, machine);
+
+ final JcloudsLocationSecurityGroupCustomizer instance = getInstance(getSharedGroupId(location));
+
+ ImmutableList.Builder<IpPermission> builder = ImmutableList.<IpPermission>builder();
+
+ builder.addAll(getIpPermissions(instance, tcpPortRanges, IpProtocol.TCP));
+
+ builder.addAll(getIpPermissions(instance, udpPortRanges, IpProtocol.UDP));
+
+ if (inboundPorts != null) {
+
+ for (int inboundPort : inboundPorts) {
+ IpPermission ipPermission = IpPermission.builder()
+ .fromPort(inboundPort)
+ .toPort(inboundPort)
+ .ipProtocol(IpProtocol.TCP)
+ .cidrBlock(instance.getBrooklynCidrBlock())
+ .build();
+ builder.add(ipPermission);
+ }
+ }
+ instance.addPermissionsToLocation(machine, builder.build());
+ }
+
+ private List<IpPermission> getIpPermissions(JcloudsLocationSecurityGroupCustomizer instance, RangeSet<Integer> tcpPortRanges, IpProtocol protocol) {
+ List<IpPermission> ipPermissions = ImmutableList.<IpPermission>of();
+ if (tcpPortRanges != null) {
+ ipPermissions =
+ FluentIterable
+ .from(tcpPortRanges.asRanges())
+ .transform(portRangeToPermission(instance, protocol))
+ .toList();
+ }
+ return ipPermissions;
+ }
+
+ private Function<Range<Integer>, IpPermission> portRangeToPermission(final JcloudsLocationSecurityGroupCustomizer instance, final IpProtocol udp) {
--- End diff --
Can you rename the field `udp` to `protocol` as it could have the value TCP or UDP, so is a bit confusing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #292: Shared location customizer
Posted by bostko <gi...@git.apache.org>.
Github user bostko commented on the issue:
https://github.com/apache/brooklyn-server/pull/292
@duncangrant I addressed @aledsage 's comments in my PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74480185
--- Diff: locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizerTest.java ---
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.jclouds.networking;
+
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.*;
+import static org.testng.Assert.assertEquals;
+
+import java.util.List;
+
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
+import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.net.Cidr;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.compute.extensions.SecurityGroupExtension;
+import org.jclouds.domain.Location;
+import org.jclouds.net.domain.IpPermission;
+import org.mockito.Answers;
+import org.mockito.ArgumentCaptor;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Supplier;
+import com.google.common.collect.ImmutableList;
+
+public class SharedLocationSecurityGroupCustomizerTest {
+
+ TestSharedLocationSecurityGroupCustomizer customizer;
+ JcloudsLocationSecurityGroupCustomizer sgCustomizer;
+ ComputeService computeService;
+ Location location;
+ SecurityGroupExtension securityApi;
+ private JcloudsLocation jcloudsLocation = new JcloudsLocation(MutableMap.of("deferConstruction", true));
+
+ @BeforeMethod
--- End diff --
personal preference to always use `@BeforeMethod(alwaysRun=true)`. If someone later adds (or marks) a test as `group="Integration"` then the setUp wouldn't run (but it would work in the Eclipse IDE as that seems to always run the `@BeforeMethod` no matter what the test group).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74462757
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java ---
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.jclouds.networking;
+
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.location.jclouds.BasicJcloudsLocationCustomizer;
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
+import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
+import org.apache.brooklyn.util.core.BrooklynNetworkUtils;
+import org.apache.brooklyn.util.net.Cidr;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.compute.options.TemplateOptions;
+import org.jclouds.net.domain.IpPermission;
+import org.jclouds.net.domain.IpProtocol;
+
+import com.google.common.base.Function;
+import com.google.common.base.Strings;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
+
+/**
+ * Configures a shared security group on Jclouds locations
+ * <p>
+ * Is based on {@link JcloudsLocationSecurityGroupCustomizer} but can be instantiated
+ * in yaml e.g.
+ * <p>
+ * services:
+ * - type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess
+ * brooklyn.config:
+ * provisioning.properties:
+ * customizers:
+ * - $brooklyn:object:
+ * type: org.apache.brooklyn.location.jclouds.networking.SharedLocationSecurityGroupCustomizer
+ * object.fields: {locationName: "test", portRanges: ["900-910","915", "22"], cidr: "82.40.153.101/24"}
+ */
+public class SharedLocationSecurityGroupCustomizer extends BasicJcloudsLocationCustomizer {
+
+ private String locationName = null;
+
+ private List<String> portRanges = null;
+
+ private String cidr = null;
+
+ /**
+ * We track inbound ports from the template to open them once we've created the new
+ * security groups
+ */
+ private int[] inboundPorts;
+
+ /**
+ * The location name is appended to the name of the shared SG - use if you need distinct shared SGs within the same location
+ *
+ * @param locationName appended to the name of the SG
+ */
+ public void setLocationName(String locationName) {
+ this.locationName = locationName;
+ }
+
+ /**
+ * Extra ports to be opened on the entities in the SG
+ *
+ * @param portRanges
+ */
+ public void setPortRanges(List<String> portRanges) {
+ this.portRanges = portRanges;
+ }
+
+ /**
+ * Set to restict the range that the ports that are to be opened can be accessed from
+ *
+ * @param cidr
+ */
+ public void setCidr(String cidr) {
+ this.cidr = cidr;
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, TemplateOptions templateOptions) {
+ super.customize(location, computeService, templateOptions);
+ inboundPorts = templateOptions.getInboundPorts();
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, Template template) {
+ super.customize(location, computeService, template);
+
+ final JcloudsLocationSecurityGroupCustomizer instance = getInstance(getSharedGroupId(location));
+
+ if (cidr != null) instance.setSshCidrSupplier(Suppliers.ofInstance(new Cidr(cidr)));
+
+ instance.customize(location, computeService, template);
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, JcloudsMachineLocation machine) {
+ super.customize(location, computeService, machine);
+
+ final JcloudsLocationSecurityGroupCustomizer instance = getInstance(getSharedGroupId(location));
+
+ if (portRanges != null) {
+ RangeSet<Integer> portRanges = BrooklynNetworkUtils.portRulesToRanges(this.portRanges);
--- End diff --
I'd parse these when they are first set, rather than when they are about to be used - fail fast (rather than after someone has waited for their VM to be provisioned).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74433004
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java ---
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.jclouds.networking;
+
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.location.jclouds.BasicJcloudsLocationCustomizer;
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
+import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
+import org.apache.brooklyn.util.core.BrooklynNetworkUtils;
+import org.apache.brooklyn.util.net.Cidr;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.compute.options.TemplateOptions;
+import org.jclouds.net.domain.IpPermission;
+import org.jclouds.net.domain.IpProtocol;
+
+import com.google.common.base.Function;
+import com.google.common.base.Strings;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
+
+/**
+ * Configures a shared security group on Jclouds locations
+ * <p>
+ * Is based on {@link JcloudsLocationSecurityGroupCustomizer} but can be instantiated
+ * in yaml e.g.
+ * <p>
+ * services:
+ * - type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess
+ * brooklyn.config:
+ * provisioning.properties:
+ * customizers:
+ * - $brooklyn:object:
+ * type: org.apache.brooklyn.location.jclouds.networking.SharedLocationSecurityGroupCustomizer
+ * object.fields: {locationName: "test", portRanges: ["900-910","915", "22"], cidr: "82.40.153.101/24"}
+ */
+public class SharedLocationSecurityGroupCustomizer extends BasicJcloudsLocationCustomizer {
+
+ private String locationName = null;
+
+ private List<String> portRanges = null;
+
+ private String cidr = null;
+
+ /**
+ * We track inbound ports from the template to open them once we've created the new
+ * security groups
+ */
+ private int[] inboundPorts;
+
+ /**
+ * The location name is appended to the name of the shared SG - use if you need distinct shared SGs within the same location
+ *
+ * @param locationName appended to the name of the SG
+ */
+ public void setLocationName(String locationName) {
+ this.locationName = locationName;
+ }
+
+ /**
+ * Extra ports to be opened on the entities in the SG
+ *
+ * @param portRanges
+ */
+ public void setPortRanges(List<String> portRanges) {
+ this.portRanges = portRanges;
+ }
+
+ /**
+ * Set to restict the range that the ports that are to be opened can be accessed from
+ *
+ * @param cidr
+ */
+ public void setCidr(String cidr) {
+ this.cidr = cidr;
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, TemplateOptions templateOptions) {
+ super.customize(location, computeService, templateOptions);
+ inboundPorts = templateOptions.getInboundPorts();
+ }
+
+ @Override
+ public void customize(JcloudsLocation location, ComputeService computeService, Template template) {
--- End diff --
Nit: One would usually override one or the other method (above one and this one) - they are called in succession with the above one being just a convenience, see https://github.com/apache/brooklyn-server/blob/master/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java#L1540-L1541.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74462257
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java ---
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.jclouds.networking;
+
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.location.jclouds.BasicJcloudsLocationCustomizer;
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
+import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
+import org.apache.brooklyn.util.core.BrooklynNetworkUtils;
+import org.apache.brooklyn.util.net.Cidr;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.compute.options.TemplateOptions;
+import org.jclouds.net.domain.IpPermission;
+import org.jclouds.net.domain.IpProtocol;
+
+import com.google.common.base.Function;
+import com.google.common.base.Strings;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
+
+/**
+ * Configures a shared security group on Jclouds locations
+ * <p>
+ * Is based on {@link JcloudsLocationSecurityGroupCustomizer} but can be instantiated
+ * in yaml e.g.
+ * <p>
+ * services:
+ * - type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess
+ * brooklyn.config:
+ * provisioning.properties:
+ * customizers:
+ * - $brooklyn:object:
+ * type: org.apache.brooklyn.location.jclouds.networking.SharedLocationSecurityGroupCustomizer
--- End diff --
indent seems wrong.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #292: Shared location customizer
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/292#discussion_r74463540
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java ---
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.jclouds.networking;
+
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+import org.apache.brooklyn.location.jclouds.BasicJcloudsLocationCustomizer;
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
+import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
+import org.apache.brooklyn.util.core.BrooklynNetworkUtils;
+import org.apache.brooklyn.util.net.Cidr;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.compute.options.TemplateOptions;
+import org.jclouds.net.domain.IpPermission;
+import org.jclouds.net.domain.IpProtocol;
+
+import com.google.common.base.Function;
+import com.google.common.base.Strings;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.Range;
+import com.google.common.collect.RangeSet;
+
+/**
+ * Configures a shared security group on Jclouds locations
+ * <p>
+ * Is based on {@link JcloudsLocationSecurityGroupCustomizer} but can be instantiated
+ * in yaml e.g.
+ * <p>
+ * services:
+ * - type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess
+ * brooklyn.config:
+ * provisioning.properties:
+ * customizers:
+ * - $brooklyn:object:
+ * type: org.apache.brooklyn.location.jclouds.networking.SharedLocationSecurityGroupCustomizer
+ * object.fields: {locationName: "test", portRanges: ["900-910","915", "22"], cidr: "82.40.153.101/24"}
+ */
+public class SharedLocationSecurityGroupCustomizer extends BasicJcloudsLocationCustomizer {
--- End diff --
This is good for this PR, but should we push any of this functionality into `JcloudsLocationSecurityGroupCustomizer` (which this delegates to)?
Is the JcloudsLocationSecurityGroupCustomizer idempotent for if we try adding the same port range multiple times?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #292: Shared location customizer
Posted by duncangrant <gi...@git.apache.org>.
Github user duncangrant commented on the issue:
https://github.com/apache/brooklyn-server/pull/292
@aledsage I've done the renaming and fixed the merge conflicts
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #292: Shared location customizer
Posted by duncangrant <gi...@git.apache.org>.
Github user duncangrant commented on the issue:
https://github.com/apache/brooklyn-server/pull/292
I've removed some whitespace and squashed commits. @grkvlt Do you think we can merge this now?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---