[jira] [Assigned] (JCLOUDS-1234) openstack-nova - Indeterminate/invalid group reference created in ingress rule if duplicate groups in region

2018-01-31 Thread Andrea Turli (JIRA)

 [ 
https://issues.apache.org/jira/browse/JCLOUDS-1234?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Andrea Turli reassigned JCLOUDS-1234:
-

Assignee: Andrea Turli

> openstack-nova - Indeterminate/invalid group reference created in ingress 
> rule if duplicate groups in region
> 
>
> Key: JCLOUDS-1234
> URL: https://issues.apache.org/jira/browse/JCLOUDS-1234
> Project: jclouds
>  Issue Type: Bug
>  Components: jclouds-compute
>Affects Versions: 2.0.0
>Reporter: Geoff Macartney
>Assignee: Andrea Turli
>Priority: Major
>  Labels: openstack-neutron, openstack-nova
> Fix For: 2.1.0
>
>
> When converting a Nova security group to its jclouds representation, the 
> class {{FindSecurityGroupWithNameAndReturnTrue}} is used to find a security 
> group in the list of groups in a location by matching on name with a “query 
> object”:
> https://github.com/apache/jclouds/blob/rel/jclouds-2.0.0/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/predicates/FindSecurityGroupWithNameAndReturnTrue.java#L66-L73
> {code}
> SecurityGroup returnVal = Iterables.find(api.get().list(), new 
> Predicate() {
> @Override
> public boolean apply(SecurityGroup input) {
>return input.getName().equals(securityGroupInRegion.getName());
> }
>  });
> {code}
> However, it is possible for there to be duplicate group names among the 
> security groups in a location. Say we have a location with two groups, G1 and 
> G2, both with name “foobar”. In such a case, if a security group G3 has 
> ingress rules permitting access from “foobar”, then it is not possible with 
> the Nova {{/v2/12345/os-security-groups}} API to know which group is 
> intended, as the only  information it returns about referred groups is the 
> tenant id and name:
> {code}
> "group": {
>"tenant_id": "12345abcde12345abcde12345abcde",
>"name": "foobar"
> },
> {code}
> With this definition of the API the ingress rule is ambiguous. The code for 
> {{FindSecurityGroupWithNameAndReturnTrue}} above implicitly assumes that 
> group names are distinct, and so it will arbitrarily assign the security 
> access to whichever of G1 and G2 it encounters first in the {{find}}, 
> possibly the wrong group, thus mapping the rule incorrectly.
> The fix for this is probably to switch to using the v3 security groups API in 
> Neutron, which returns the actual security group id in the definitions of 
> ingress rules and not just the name.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (JCLOUDS-1234) openstack-nova - Indeterminate/invalid group reference created in ingress rule if duplicate groups in region

2018-01-31 Thread Andrea Turli (JIRA)

 [ 
https://issues.apache.org/jira/browse/JCLOUDS-1234?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Andrea Turli updated JCLOUDS-1234:
--
Fix Version/s: 2.1.0

> openstack-nova - Indeterminate/invalid group reference created in ingress 
> rule if duplicate groups in region
> 
>
> Key: JCLOUDS-1234
> URL: https://issues.apache.org/jira/browse/JCLOUDS-1234
> Project: jclouds
>  Issue Type: Bug
>  Components: jclouds-compute
>Affects Versions: 2.0.0
>Reporter: Geoff Macartney
>Priority: Major
>  Labels: openstack-neutron, openstack-nova
> Fix For: 2.1.0
>
>
> When converting a Nova security group to its jclouds representation, the 
> class {{FindSecurityGroupWithNameAndReturnTrue}} is used to find a security 
> group in the list of groups in a location by matching on name with a “query 
> object”:
> https://github.com/apache/jclouds/blob/rel/jclouds-2.0.0/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/predicates/FindSecurityGroupWithNameAndReturnTrue.java#L66-L73
> {code}
> SecurityGroup returnVal = Iterables.find(api.get().list(), new 
> Predicate() {
> @Override
> public boolean apply(SecurityGroup input) {
>return input.getName().equals(securityGroupInRegion.getName());
> }
>  });
> {code}
> However, it is possible for there to be duplicate group names among the 
> security groups in a location. Say we have a location with two groups, G1 and 
> G2, both with name “foobar”. In such a case, if a security group G3 has 
> ingress rules permitting access from “foobar”, then it is not possible with 
> the Nova {{/v2/12345/os-security-groups}} API to know which group is 
> intended, as the only  information it returns about referred groups is the 
> tenant id and name:
> {code}
> "group": {
>"tenant_id": "12345abcde12345abcde12345abcde",
>"name": "foobar"
> },
> {code}
> With this definition of the API the ingress rule is ambiguous. The code for 
> {{FindSecurityGroupWithNameAndReturnTrue}} above implicitly assumes that 
> group names are distinct, and so it will arbitrarily assign the security 
> access to whichever of G1 and G2 it encounters first in the {{find}}, 
> possibly the wrong group, thus mapping the rule incorrectly.
> The fix for this is probably to switch to using the v3 security groups API in 
> Neutron, which returns the actual security group id in the definitions of 
> ingress rules and not just the name.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

2018-01-31 Thread Ignasi Barrera
nacx commented on this pull request.



> + return ImmutableSet.of();
+  }
+  return 
getSecurityGroupApi(region).listSecurityGroups().concat().transform(neutronSecurityGroupToSecurityGroup.create(location)).toSet();
+   }
+
+   @Override
+   public Set listSecurityGroupsForNode(String id) {
+  RegionAndId regionAndId = RegionAndId.fromSlashEncoded(checkNotNull(id, 
"id"));
+  String region = regionAndId.getRegion();
+
+  SecurityGroupApi securityGroupApi = getSecurityGroupApi(region);
+
+  final 
FluentIterable allGroups 
= securityGroupApi.listSecurityGroups().concat();
+  Location location = locationIndex.get().get(region);
+  return ImmutableSet.copyOf(transform(filter(allGroups, notNull()), 
neutronSecurityGroupToSecurityGroup.create(location)));
+   }

+1 to use this. I'd say we need it before merging the PR. Agree?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1175#discussion_r165030128

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

2018-01-31 Thread Andrea Turli
@andreaturli pushed 1 commit.

753638f  fix listSecurityGroupsForNode


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/1175/files/4e0b06b2510001ccd9a83f3166989d6119255f46..753638f986a376e6abd94ccc57ea35bad91a79af


Re: [jclouds/jclouds] [JCLOUDS-1374]This is the first commit of Softlayer LoadBalancer API (#1176)

2018-01-31 Thread swaqos
I think creating a new virtualGuest is not a problem, but the problem is more 
with the network/subnets. I believe with the new CDN accounts, virtualGuest 
creations require subnetId as well, while I believe subnets need to live on 
private networks, and there are limit number of private networks(5), at least . 
Depending on how the private networks are configured, the number of subnets are 
limited, too, so I don't know if it's safe to assume there is room for them. I 
will need to do some more investigations on that.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1176#issuecomment-361973789

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

2018-01-31 Thread Ignasi Barrera
nacx requested changes on this pull request.

I've debugged the failure and it turns to be a test concurrency issue with the 
two tests that verify the behavior of the SG cache. If you apply this patch to 
make sure both tests don't use the same security group, tests should pass:
```diff
diff --git 
a/compute/src/test/java/org/jclouds/compute/extensions/internal/BaseSecurityGroupExtensionLiveTest.java
 
b/compute/src/test/java/org/jclouds/compute/extensions/internal/BaseSecurityGroupExtensionLiveTest.java
index 2aa7e66c87..5c39bcb783 100644
--- 
a/compute/src/test/java/org/jclouds/compute/extensions/internal/BaseSecurityGroupExtensionLiveTest.java
+++ 
b/compute/src/test/java/org/jclouds/compute/extensions/internal/BaseSecurityGroupExtensionLiveTest.java
@@ -406,17 +406,18 @@ public abstract class BaseSecurityGroupExtensionLiveTest 
extends BaseComputeServ
 
@Test(groups = {"integration", "live"}, singleThreaded = true)
public void testSecurityGroupCacheInvalidatedWhenDeletedExternally() throws 
Exception {
+  String testSecurityGroupName = secGroupNameToDelete + "-externally";
   ComputeService computeService = view.getComputeService();
   Optional securityGroupExtension = 
computeService.getSecurityGroupExtension();
   assertTrue(securityGroupExtension.isPresent(), "security extension was 
not present");
   final SecurityGroupExtension security = securityGroupExtension.get();
-  final SecurityGroup seedGroup = 
security.createSecurityGroup(secGroupNameToDelete, 
getNodeTemplate().getLocation());
+  final SecurityGroup seedGroup = 
security.createSecurityGroup(testSecurityGroupName, 
getNodeTemplate().getLocation());
 
   deleteSecurityGroupFromAnotherView(seedGroup);
 
   boolean deleted = security.removeSecurityGroup(seedGroup.getId());
   assertFalse(deleted, "SG deleted externally so should've failed 
deletion");
-  final SecurityGroup recreatedGroup = 
security.createSecurityGroup(secGroupNameToDelete, 
getNodeTemplate().getLocation());
+  final SecurityGroup recreatedGroup = 
security.createSecurityGroup(testSecurityGroupName, 
getNodeTemplate().getLocation());
 
   // Makes sure the security group exists and is re-created and is not 
just returned from cache
   security.addIpPermission(IpPermission.builder()
```

> + return ImmutableSet.of();
+  }
+  return 
getSecurityGroupApi(region).listSecurityGroups().concat().transform(neutronSecurityGroupToSecurityGroup.create(location)).toSet();
+   }
+
+   @Override
+   public Set listSecurityGroupsForNode(String id) {
+  RegionAndId regionAndId = RegionAndId.fromSlashEncoded(checkNotNull(id, 
"id"));
+  String region = regionAndId.getRegion();
+
+  SecurityGroupApi securityGroupApi = getSecurityGroupApi(region);
+
+  final 
FluentIterable allGroups 
= securityGroupApi.listSecurityGroups().concat();
+  Location location = locationIndex.get().get(region);
+  return ImmutableSet.copyOf(transform(filter(allGroups, notNull()), 
neutronSecurityGroupToSecurityGroup.create(location)));
+   }

This implementation is still wrong. You have to filter ONLY the security groups 
associated with the given node. You're returning all existing ones.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1175#pullrequestreview-92833226

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

2018-01-31 Thread Andrea Turli
andreaturli commented on this pull request.



> + return ImmutableSet.of();
+  }
+  return 
getSecurityGroupApi(region).listSecurityGroups().concat().transform(neutronSecurityGroupToSecurityGroup.create(location)).toSet();
+   }
+
+   @Override
+   public Set listSecurityGroupsForNode(String id) {
+  RegionAndId regionAndId = RegionAndId.fromSlashEncoded(checkNotNull(id, 
"id"));
+  String region = regionAndId.getRegion();
+
+  SecurityGroupApi securityGroupApi = getSecurityGroupApi(region);
+
+  final 
FluentIterable allGroups 
= securityGroupApi.listSecurityGroups().concat();
+  Location location = locationIndex.get().get(region);
+  return ImmutableSet.copyOf(transform(filter(allGroups, notNull()), 
neutronSecurityGroupToSecurityGroup.create(location)));
+   }

good point! thanks

unfortunately also `NovaSecurityGroupExtension` is not really reliable as it is 
built on
`Optional serverWithSecurityGroupsApi = 
api.getServerWithSecurityGroupsApi(region)` which is most of the time not 
working on recent Openstack installations (they claim the extension is there 
but it is not working!)

We may need to radically change the implementation but not sure how yet


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1175#discussion_r165008461

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

2018-01-31 Thread Andrea Turli
@nacx here's the result of `NeutronSecurityGroupExtensionLiveTest` against a 
provider with keystone v3 and Neutron support
```
---
 T E S T S
---
Running 
org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest
Configuring TestNG with: TestNG652Configurator
Starting test 
testCreateSecurityGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
Starting test 
testSecurityGroupCacheInvalidatedWhenDeletedExternally(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
Starting test 
testSecurityGroupCacheInvalidated(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-1] Test 
testCreateSecurityGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
 succeeded: 10013ms
Test suite progress: tests succeeded: 1, failed: 0, skipped: 0.
Starting test 
testGetSecurityGroupById(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-4] Test 
testGetSecurityGroupById(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
 succeeded: 560ms
Test suite progress: tests succeeded: 2, failed: 0, skipped: 0.
Starting test 
testAddIpPermission(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-5] Test 
testAddIpPermission(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
 succeeded: 2908ms
Test suite progress: tests succeeded: 3, failed: 0, skipped: 0.
Starting test 
testRemoveIpPermission(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-3] Test 
testSecurityGroupCacheInvalidatedWhenDeletedExternally(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
 succeeded: 16488ms
Test suite progress: tests succeeded: 4, failed: 0, skipped: 0.
[pool-2-thread-2] Test 
testSecurityGroupCacheInvalidated(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
 succeeded: 16835ms
Test suite progress: tests succeeded: 5, failed: 0, skipped: 0.
[pool-2-thread-5] Test 
testRemoveIpPermission(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
 succeeded: 3464ms
Test suite progress: tests succeeded: 6, failed: 0, skipped: 0.
Starting test 
testAddIpPermissionsFromSpec(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-5] Test 
testAddIpPermissionsFromSpec(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
 succeeded: 2700ms
Test suite progress: tests succeeded: 7, failed: 0, skipped: 0.
Starting test 
testAddIpPermissionWithCidrExclusionGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-5] Test 
testAddIpPermissionWithCidrExclusionGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
 skipped.
Test suite progress: tests succeeded: 7, failed: 0, skipped: 1.
[pool-2-thread-5] Test 
testRemoveIpPermissionWithCidrExclusionGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
 skipped.
Test suite progress: tests succeeded: 7, failed: 0, skipped: 2.
Starting test 
testDeleteSecurityGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
[pool-2-thread-5] Test 
testDeleteSecurityGroup(org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest)
 succeeded: 1202ms
Test suite progress: tests succeeded: 8, failed: 0, skipped: 2.
Tests run: 10, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 22.939 sec - 
in 
org.jclouds.openstack.nova.v2_0.compute.extensions.NeutronSecurityGroupExtensionLiveTest

Results :

Tests run: 10, Failures: 0, Errors: 0, Skipped: 2
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1175#issuecomment-361878277

Re: [jclouds/jclouds] [JCLOUDS-1377] add support for injectable Neutron Context into Nova (#1175)

2018-01-31 Thread Andrea Turli
andreaturli commented on this pull request.



> + return ImmutableSet.of();
+  }
+  return 
getSecurityGroupApi(region).listSecurityGroups().concat().transform(neutronSecurityGroupToSecurityGroup.create(location)).toSet();
+   }
+
+   @Override
+   public Set listSecurityGroupsForNode(String id) {
+  RegionAndId regionAndId = RegionAndId.fromSlashEncoded(checkNotNull(id, 
"id"));
+  String region = regionAndId.getRegion();
+
+  SecurityGroupApi securityGroupApi = getSecurityGroupApi(region);
+
+  final 
FluentIterable allGroups 
= securityGroupApi.listSecurityGroups().concat();
+  Location location = locationIndex.get().get(region);
+  return ImmutableSet.copyOf(transform(filter(allGroups, notNull()), 
neutronSecurityGroupToSecurityGroup.create(location)));
+   }

I think we should use 
https://developer.openstack.org/api-ref/compute/#servers-security-groups-servers-os-security-groups

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1175#discussion_r165012937