You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by Graeme-Miller <gi...@git.apache.org> on 2015/12/10 16:40:06 UTC
[GitHub] incubator-brooklyn pull request: Ported some Clocker changes back ...
GitHub user Graeme-Miller opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/1101
Ported some Clocker changes back to brooklyn.
It is now possible to remove security groups permissions
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Graeme-Miller/incubator-brooklyn security_customizer_remove_group
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/1101.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 #1101
----
commit 5f46a334e2231d37820d753edbff96eb8f2dda10
Author: Graeme-Miller <gr...@cloudsoftcorp.com>
Date: 2015-12-10T15:37:01Z
Ported some Clocker changes back to brooklyn.
It is now possible to remove security groups permissions
----
---
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: Ported some Clocker changes back ...
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1101#issuecomment-164736878
@sjcorbett thanks Sam, have added some tests now.
Also, I amended removePermissionsFromLocation so that is is synchronized on the class, to keep it in line with addPermissionsToLocation
---
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: Ported some Clocker changes back ...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1101#discussion_r47630831
--- Diff: locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizerTest.java ---
@@ -147,6 +149,59 @@ public void testAddPermissionsToNode() {
}
@Test
+ public void testRemovePermissionsFromNode() {
+ IpPermission ssh = newPermission(22);
+ IpPermission jmx = newPermission(31001);
+ String nodeId = "node";
+ SecurityGroup sharedGroup = newGroup(customizer.getNameForSharedSecurityGroup());
+ SecurityGroup group = newGroup("id");
+ when(securityApi.listSecurityGroupsForNode(nodeId)).thenReturn(ImmutableSet.of(sharedGroup, group));
+ when(computeService.getContext().unwrap().getId()).thenReturn("aws-ec2");
+
+ customizer.addPermissionsToLocation(ImmutableList.of(ssh, jmx), nodeId, computeService);
+ customizer.removePermissionsFromLocation(ImmutableList.of(jmx), nodeId, computeService);
+
+ verify(securityApi, never()).removeIpPermission(ssh, group);
+ verify(securityApi, times(1)).removeIpPermission(jmx, group);
+ }
+
+ @Test
+ public void testRemoveMultiplePermissionsFromNode() {
+ IpPermission ssh = newPermission(22);
+ IpPermission jmx = newPermission(31001);
+ String nodeId = "node";
+ SecurityGroup sharedGroup = newGroup(customizer.getNameForSharedSecurityGroup());
+ SecurityGroup group = newGroup("id");
+ when(securityApi.listSecurityGroupsForNode(nodeId)).thenReturn(ImmutableSet.of(sharedGroup, group));
+ when(computeService.getContext().unwrap().getId()).thenReturn("aws-ec2");
+
+ customizer.addPermissionsToLocation(ImmutableList.of(ssh, jmx), nodeId, computeService);
+ customizer.removePermissionsFromLocation(ImmutableList.of(ssh, jmx), nodeId, computeService);
+
+ verify(securityApi, times(1)).removeIpPermission(ssh, group);
+ verify(securityApi, times(1)).removeIpPermission(jmx, group);
+ }
+
+
+ @Test
+ public void testAddPermissionWhenNoExtension() {
+ IpPermission ssh = newPermission(22);
+ IpPermission jmx = newPermission(31001);
+ String nodeId = "node";
+
+ when(securityApi.listSecurityGroupsForNode(nodeId)).thenReturn(Collections.<SecurityGroup>emptySet());
+
+ RuntimeException exception = null;
+ try {
+ customizer.addPermissionsToLocation(ImmutableList.of(ssh, jmx), nodeId, computeService);
--- End diff --
The approach we generally take is to `fail("should have thrown some exception")` after the exceptional statement and then to assert on the exception in the catch block. You can also use `Exceptions.getFirstInteresting(e)` and the recent additions of `shouldHaveFailedPreviously()` and `expectedFailure()` to the `Asserts` class.
---
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: Ported some Clocker changes back ...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1101#issuecomment-164752781
@Graeme-Miller Thanks. One minor comment but am happy for this to be merged as 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] incubator-brooklyn pull request: Ported some Clocker changes back ...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1101#discussion_r48200161
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java ---
@@ -201,6 +229,47 @@ public JcloudsLocationSecurityGroupCustomizer addPermissionsToLocation(final Jcl
return this;
}
}
+ /**
+ * Removes the given security group permissions from the given node with the given compute service.
+ * <p>
+ * Takes no action if the compute service does not have a security group extension.
+ * @param permissions The set of permissions to be removed from the location
+ * @param location Location to remove permissions from
+ */
+ public void removePermissionsFromLocation(final JcloudsMachineLocation location, final Iterable<IpPermission> permissions) {
--- End diff --
@Graeme-Miller should include `@since 0.9.0` when adding public methods like 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: Ported some Clocker changes back ...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1101#issuecomment-163699198
Would be good to add tests for the new additions to `JcloudsLocationSecurityGroupCustomizerTest`.
---
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: Ported some Clocker changes back ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/1101
---
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.
---