You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by googlielmo <gi...@git.apache.org> on 2016/01/14 19:16:06 UTC
[GitHub] incubator-brooklyn pull request: Better control over opening inbou...
GitHub user googlielmo opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/1148
Better control over opening inbound ports
Add config keys to `SoftwareProcess` governing the opening of ports based on sensor names, so it is explicit when you look at the config keys for the relevant entities. ie, `SoftwareProcessImpl.getRequiredOpenPorts` uses a config key whose default value is this:
```
inboundPorts.configRegex=.*\.port
```
Also add a config option whose default value is this:
```
inboundPorts.autoInfer=true
```
If explicitly set to false, it turns off the opening of ports based on naming convention, and also those defined in Java entities that are of type PortRange.
Remove duplication of code between `SameServerDriverLifecycleEffectorTasks` and `SoftwareProcessImpl`.
Make login port (22) also a config key, not a constant, in `SameServerEntity`.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/googlielmo/incubator-brooklyn feature/inbound-port-opening
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/1148.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 #1148
----
commit 6b43665fa021e11447001d6d1f56eadae6a002ae
Author: Guglielmo Nigri <gu...@cloudsoftcorp.com>
Date: 2016-01-14T10:42:51Z
Better control over opening inbound ports
----
---
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] incubator-brooklyn pull request: Better control over opening inbou...
Posted by googlielmo <gi...@git.apache.org>.
Github user googlielmo commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1148#issuecomment-172870882
@ahgittin Thanks for the comments! Will correct the typo, tests are ready and coming in a couple of minutes actually :-)
---
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] incubator-brooklyn pull request: Better control over opening inbou...
Posted by googlielmo <gi...@git.apache.org>.
Github user googlielmo commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1148#discussion_r49983652
--- Diff: brooklyn-server/software/base/src/main/java/org/apache/brooklyn/entity/software/base/InboundPortsUtils.java ---
@@ -0,0 +1,98 @@
+/*
+ * 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.entity.software.base;
--- End diff --
Aargh -- it seems it's not going to work if we move it to the `brooklyn-utils-common` module, because we don't have access to the needed core classes from there (`Entity`, etc.) @grkvlt
---
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] incubator-brooklyn pull request: Better control over opening inbou...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1148#discussion_r50115857
--- Diff: brooklyn-server/software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcess.java ---
@@ -50,13 +59,20 @@
AttributeSensor<String> SUBNET_HOSTNAME = Attributes.SUBNET_HOSTNAME;
AttributeSensor<String> SUBNET_ADDRESS = Attributes.SUBNET_ADDRESS;
- @SuppressWarnings("serial")
ConfigKey<Collection<Integer>> REQUIRED_OPEN_LOGIN_PORTS = ConfigKeys.newConfigKey(
new TypeToken<Collection<Integer>>() {},
"requiredOpenLoginPorts",
"The port(s) to be opened, to allow login",
ImmutableSet.of(22));
+ ConfigKey<String> INBOUND_PORTS_CONFIG_REGEX = ConfigKeys.newStringConfigKey("inboundPorts.configRegex",
+ "Regex governing the opening of ports based on sensor names",
--- End diff --
should be "config names" ?
---
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] incubator-brooklyn pull request: Better control over opening inbou...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1148#issuecomment-172865990
one minor typo (`s/sensor/config/`). are tests coming? and worth adding a short para to the docs also?
happy to merge now with changes to follow if you like.
---
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] incubator-brooklyn pull request: Better control over opening inbou...
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1148#issuecomment-172004804
Looks good, are there any tests we can add for this?
---
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] incubator-brooklyn pull request: Better control over opening inbou...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1148#issuecomment-172369386
@googlielmo looks good, apart from the location of the utility class, which is minor.
---
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] incubator-brooklyn pull request: Better control over opening inbou...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/1148
---
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] incubator-brooklyn pull request: Better control over opening inbou...
Posted by googlielmo <gi...@git.apache.org>.
Github user googlielmo commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1148#discussion_r49982273
--- Diff: brooklyn-server/software/base/src/main/java/org/apache/brooklyn/entity/software/base/InboundPortsUtils.java ---
@@ -0,0 +1,98 @@
+/*
+ * 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.entity.software.base;
--- End diff --
I actually think `util.net` is more appropriate @grkvlt, thanks for the suggestion! I initially put this here for locality with the client code that's using it -- but it makes sense to move 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] incubator-brooklyn pull request: Better control over opening inbou...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1148#discussion_r49950876
--- Diff: brooklyn-server/software/base/src/main/java/org/apache/brooklyn/entity/software/base/InboundPortsUtils.java ---
@@ -0,0 +1,98 @@
+/*
+ * 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.entity.software.base;
--- End diff --
Is this the right package for `InboundPortsUtils`? perhaps `org.apache.brooklyn.util.net` is more appropriate?
---
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] incubator-brooklyn pull request: Better control over opening inbou...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1148#issuecomment-173537492
no docs :( but otherwise looks good.
i'll merge this and put some text into the "Custom Entities" docs page.
---
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] incubator-brooklyn pull request: Better control over opening inbou...
Posted by googlielmo <gi...@git.apache.org>.
Github user googlielmo commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1148#issuecomment-172006388
Good point @Graeme-Miller; actually `org.apache.brooklyn.entity.software.base.SameServerDriverLifecycleEffectorTasksTest#testGetRequiredOpenPorts` covers much of it, but not all, so I'll add a couple of tests for completeness.
---
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.
---