JCLOUDS-242. Fixing AWSEC2SecurityGroupExtension based on JCLOUDS-242 fix.

apis/ec2/src/main/java/org/jclouds/ec2/compute/config/EC2ComputeServiceDependenciesModule.java

@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 package org.jclouds.ec2.compute.config;
+
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static org.jclouds.ec2.reference.EC2Constants.PROPERTY_EC2_TIMEOUT_SECURITYGROUP_PRESENT;
 import static org.jclouds.util.Predicates2.retry;
@@ -44,6 +45,7 @@ import org.jclouds.ec2.compute.functions.AddElasticIpsToNodemetadata;
 import org.jclouds.ec2.compute.functions.CreateUniqueKeyPair;
 import org.jclouds.ec2.compute.functions.CredentialsForInstance;
 import org.jclouds.ec2.compute.functions.EC2ImageParser;
+import org.jclouds.ec2.compute.functions.EC2SecurityGroupIdFromName;
 import org.jclouds.ec2.compute.functions.EC2SecurityGroupToSecurityGroup;
 import org.jclouds.ec2.compute.functions.PasswordCredentialsFromWindowsInstance;
 import org.jclouds.ec2.compute.functions.RunningInstanceToNodeMetadata;
@@ -126,7 +128,9 @@ public class EC2ComputeServiceDependenciesModule extends AbstractModule {
       bind(new TypeLiteral<CacheLoader<RegionAndName, String>>() {
       }).annotatedWith(Names.named("SECURITY")).to(CreateSecurityGroupIfNeeded.class);
       bind(new TypeLiteral<CacheLoader<RegionAndName, String>>() {
-      }).annotatedWith(Names.named("ELASTICIP")).to(LoadPublicIpForInstanceOrNull.class);   
+      }).annotatedWith(Names.named("ELASTICIP")).to(LoadPublicIpForInstanceOrNull.class);
+      bind(new TypeLiteral<Function<String, String>>() {
+      }).annotatedWith(Names.named("SECGROUP_NAME_TO_ID")).to(EC2SecurityGroupIdFromName.class);
       bind(new TypeLiteral<Function<PasswordDataAndPrivateKey, LoginCredentials>>() {
       }).to(WindowsLoginCredentialsFromEncryptedData.class);
       bind(new TypeLiteral<Function<RunningInstance, LoginCredentials>>() {
@@ -147,7 +151,7 @@ public class EC2ComputeServiceDependenciesModule extends AbstractModule {
    @Provides
    @Singleton
    public Function<RunningInstance, NodeMetadata> bindNodeConverter(RunningInstanceToNodeMetadata baseConverter,
-            AddElasticIpsToNodemetadata addElasticIpsToNodemetadata,
+              AddElasticIpsToNodemetadata addElasticIpsToNodemetadata,
             @Named(EC2Constants.PROPERTY_EC2_AUTO_ALLOCATE_ELASTIC_IPS) boolean autoAllocateElasticIps) {
       if (!autoAllocateElasticIps)
          return baseConverter;

apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/EC2SecurityGroupIdFromName.java

@@ -0,0 +1,51 @@
+/*
+ * 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.jclouds.ec2.compute.functions;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import javax.inject.Inject;
+import javax.inject.Singleton;
+
+import org.jclouds.aws.util.AWSUtils;
+import org.jclouds.ec2.EC2Api;
+
+import com.google.common.base.Function;
+import com.google.common.collect.Iterables;
+
+/**
+ * @author Andrew Bayer
+ */
+@Singleton
+public class EC2SecurityGroupIdFromName implements Function<String, String> {
+   protected EC2Api api;
+
+   @Inject
+   public EC2SecurityGroupIdFromName(EC2Api api) {
+      this.api = checkNotNull(api, "api");
+   }
+
+   @Override
+   public String apply(String input) {
+      checkNotNull(input, "input");
+      String[] parts = AWSUtils.parseHandle(input);
+      String region = parts[0];
+      String name = parts[1];
+
+      return  Iterables.getOnlyElement(api.getSecurityGroupApi().get().describeSecurityGroupsInRegion(region, name), null).getId();
+   }
+}

providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/config/AWSEC2ComputeServiceDependenciesModule.java

@@ -53,10 +53,10 @@ import org.jclouds.ec2.compute.config.EC2ComputeServiceDependenciesModule;
 import org.jclouds.ec2.compute.domain.PasswordDataAndPrivateKey;
 import org.jclouds.ec2.compute.domain.RegionAndName;
 import org.jclouds.ec2.compute.extensions.EC2ImageExtension;
-import org.jclouds.ec2.compute.extensions.EC2SecurityGroupExtension;
 import org.jclouds.ec2.compute.functions.CreateUniqueKeyPair;
 import org.jclouds.ec2.compute.functions.CredentialsForInstance;
 import org.jclouds.ec2.compute.functions.EC2ImageParser;
+import org.jclouds.ec2.compute.functions.EC2SecurityGroupIdFromName;
 import org.jclouds.ec2.compute.functions.EC2SecurityGroupToSecurityGroup;
 import org.jclouds.ec2.compute.functions.PasswordCredentialsFromWindowsInstance;
 import org.jclouds.ec2.compute.functions.WindowsLoginCredentialsFromEncryptedData;
@@ -96,7 +96,9 @@ public class AWSEC2ComputeServiceDependenciesModule extends EC2ComputeServiceDep
       bind(new TypeLiteral<CacheLoader<RegionAndName, String>>() {
       }).annotatedWith(Names.named("SECURITY")).to(AWSEC2CreateSecurityGroupIfNeeded.class);
       bind(new TypeLiteral<CacheLoader<RegionAndName, String>>() {
-      }).annotatedWith(Names.named("ELASTICIP")).to(LoadPublicIpForInstanceOrNull.class);   
+      }).annotatedWith(Names.named("ELASTICIP")).to(LoadPublicIpForInstanceOrNull.class);
+      bind(new TypeLiteral<Function<String, String>>() {
+      }).annotatedWith(Names.named("SECGROUP_NAME_TO_ID")).to(EC2SecurityGroupIdFromName.class);
       bind(new TypeLiteral<Function<PasswordDataAndPrivateKey, LoginCredentials>>() {
       }).to(WindowsLoginCredentialsFromEncryptedData.class);
       bind(new TypeLiteral<Function<RunningInstance, LoginCredentials>>() {

providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/extensions/AWSEC2SecurityGroupExtension.java

@@ -17,53 +17,31 @@
 package org.jclouds.aws.ec2.compute.extensions;
 
 import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.common.base.Predicates.equalTo;
-import static com.google.common.base.Predicates.not;
-import static com.google.common.base.Predicates.notNull;
-import static com.google.common.collect.Iterables.concat;
-import static com.google.common.collect.Iterables.filter;
-import static com.google.common.collect.Iterables.getOnlyElement;
-import static com.google.common.collect.Iterables.toArray;
-import static com.google.common.collect.Iterables.transform;
 
-import java.util.NoSuchElementException;
 import java.util.Set;
-import java.util.concurrent.ConcurrentMap;
 
 import javax.inject.Inject;
 import javax.inject.Named;
-import javax.inject.Provider;
 
 import org.jclouds.Constants;
 import org.jclouds.aws.ec2.AWSEC2Api;
 import org.jclouds.aws.util.AWSUtils;
 import org.jclouds.collect.Memoized;
 import org.jclouds.compute.domain.SecurityGroup;
-import org.jclouds.compute.domain.SecurityGroupBuilder;
-import org.jclouds.compute.extensions.SecurityGroupExtension;
 import org.jclouds.compute.functions.GroupNamingConvention;
-import org.jclouds.compute.functions.GroupNamingConvention.Factory;
 import org.jclouds.domain.Location;
 import org.jclouds.ec2.compute.domain.RegionAndName;
-import org.jclouds.ec2.compute.domain.RegionNameAndIngressRules;
 import org.jclouds.ec2.compute.extensions.EC2SecurityGroupExtension;
-import org.jclouds.ec2.domain.RunningInstance;
-import org.jclouds.ec2.domain.UserIdGroupPair;
 import org.jclouds.location.Region;
 import org.jclouds.net.domain.IpPermission;
 import org.jclouds.net.domain.IpProtocol;
 
 import com.google.common.base.Function;
-import com.google.common.base.Predicate;
 import com.google.common.base.Supplier;
 import com.google.common.cache.LoadingCache;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Multimap;
-import com.google.common.collect.Sets;
-import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.ListeningExecutorService;
-import com.google.common.util.concurrent.UncheckedTimeoutException;
 
 /**
  * An extension to compute service to allow for the manipulation of {@link SecurityGroup}s. Implementation
@@ -73,6 +51,7 @@ import com.google.common.util.concurrent.UncheckedTimeoutException;
  */
 public class AWSEC2SecurityGroupExtension extends EC2SecurityGroupExtension {
    protected final AWSEC2Api client;
+   protected final Function<String, String> groupNameToId;
 
    @Inject
    public AWSEC2SecurityGroupExtension(AWSEC2Api client,
@@ -81,19 +60,21 @@ public class AWSEC2SecurityGroupExtension extends EC2SecurityGroupExtension {
                                        Function<org.jclouds.ec2.domain.SecurityGroup, SecurityGroup> groupConverter,
                                        @Memoized Supplier<Set<? extends Location>> locations,
                                        @Named("SECURITY") LoadingCache<RegionAndName, String> groupCreator,
-                                       GroupNamingConvention.Factory namingConvention) {
+                                       GroupNamingConvention.Factory namingConvention,
+                                       @Named("SECGROUP_NAME_TO_ID") Function<String, String> groupNameToId) {
       super(client, userExecutor, regions, groupConverter, locations, groupCreator, namingConvention);
       this.client = checkNotNull(client, "client");
+      this.groupNameToId = checkNotNull(groupNameToId, "groupNameToId");
    }
 
 
    @Override
    public SecurityGroup addIpPermission(IpPermission ipPermission, SecurityGroup group) {
-
       String region = AWSUtils.getRegionFromLocationOrNull(group.getLocation());
       String name = group.getName();
+      String id = groupNameToId.apply(new RegionAndName(region, name).slashEncode());
 
-      client.getSecurityGroupApi().get().authorizeSecurityGroupIngressInRegion(region, name, ipPermission);
+      client.getSecurityGroupApi().get().authorizeSecurityGroupIngressInRegion(region, id, ipPermission);
 
       return getSecurityGroupById(new RegionAndName(region, group.getName()).slashEncode());
    }
@@ -105,6 +86,7 @@ public class AWSEC2SecurityGroupExtension extends EC2SecurityGroupExtension {
                                         Iterable<String> groupIds, SecurityGroup group) {
       String region = AWSUtils.getRegionFromLocationOrNull(group.getLocation());
       String name = group.getName();
+      String id = groupNameToId.apply(new RegionAndName(region, name).slashEncode());
 
       IpPermission.Builder builder = IpPermission.builder();
 
@@ -126,7 +108,7 @@ public class AWSEC2SecurityGroupExtension extends EC2SecurityGroupExtension {
          }
       }
 
-      client.getSecurityGroupApi().get().authorizeSecurityGroupIngressInRegion(region, name, builder.build());
+      client.getSecurityGroupApi().get().authorizeSecurityGroupIngressInRegion(region, id, builder.build());
 
       return getSecurityGroupById(new RegionAndName(region, group.getName()).slashEncode());
    }
@@ -135,8 +117,9 @@ public class AWSEC2SecurityGroupExtension extends EC2SecurityGroupExtension {
    public SecurityGroup removeIpPermission(IpPermission ipPermission, SecurityGroup group) {
       String region = AWSUtils.getRegionFromLocationOrNull(group.getLocation());
       String name = group.getName();
+      String id = groupNameToId.apply(new RegionAndName(region, name).slashEncode());
 
-      client.getSecurityGroupApi().get().revokeSecurityGroupIngressInRegion(region, name, ipPermission);
+      client.getSecurityGroupApi().get().revokeSecurityGroupIngressInRegion(region, id, ipPermission);
 
       return getSecurityGroupById(new RegionAndName(region, group.getName()).slashEncode());
    }
@@ -148,7 +131,7 @@ public class AWSEC2SecurityGroupExtension extends EC2SecurityGroupExtension {
                                            Iterable<String> groupIds, SecurityGroup group) {
       String region = AWSUtils.getRegionFromLocationOrNull(group.getLocation());
       String name = group.getName();
-
+      String id = groupNameToId.apply(new RegionAndName(region, name).slashEncode());
 
       IpPermission.Builder builder = IpPermission.builder();
 
@@ -170,7 +153,7 @@ public class AWSEC2SecurityGroupExtension extends EC2SecurityGroupExtension {
          }
       }
 
-      client.getSecurityGroupApi().get().revokeSecurityGroupIngressInRegion(region, name, builder.build());
+      client.getSecurityGroupApi().get().revokeSecurityGroupIngressInRegion(region, id, builder.build());
 
       return getSecurityGroupById(new RegionAndName(region, group.getName()).slashEncode());
    }

providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/loaders/AWSEC2CreateSecurityGroupIfNeeded.java

@@ -32,11 +32,11 @@ import org.jclouds.aws.ec2.features.AWSSecurityGroupApi;
 import org.jclouds.compute.reference.ComputeServiceConstants;
 import org.jclouds.ec2.compute.domain.RegionAndName;
 import org.jclouds.ec2.compute.domain.RegionNameAndIngressRules;
-import org.jclouds.ec2.domain.UserIdGroupPair;
 import org.jclouds.logging.Logger;
 import org.jclouds.net.domain.IpPermission;
 import org.jclouds.net.domain.IpProtocol;
 
+import com.google.common.base.Function;
 import com.google.common.base.Predicate;
 import com.google.common.cache.CacheLoader;
 import com.google.common.collect.ImmutableSet;
@@ -54,16 +54,19 @@ public class AWSEC2CreateSecurityGroupIfNeeded extends CacheLoader<RegionAndName
    protected Logger logger = Logger.NULL;
    protected final AWSSecurityGroupApi securityApi;
    protected final Predicate<RegionAndName> securityGroupEventualConsistencyDelay;
-
+   protected final Function<String, String> groupNameToId;
    @Inject
    public AWSEC2CreateSecurityGroupIfNeeded(AWSEC2Api ec2Api,
-         @Named("SECURITY") Predicate<RegionAndName> securityGroupEventualConsistencyDelay) {
+                                            @Named("SECGROUP_NAME_TO_ID") Function<String, String> groupNameToId,
-      this(checkNotNull(ec2Api, "ec2Api").getSecurityGroupApi().get(), securityGroupEventualConsistencyDelay);
+                                            @Named("SECURITY") Predicate<RegionAndName> securityGroupEventualConsistencyDelay) {
+      this(checkNotNull(ec2Api, "ec2Api").getSecurityGroupApi().get(), groupNameToId, securityGroupEventualConsistencyDelay);
    }
 
    public AWSEC2CreateSecurityGroupIfNeeded(AWSSecurityGroupApi securityApi,
-         @Named("SECURITY") Predicate<RegionAndName> securityGroupEventualConsistencyDelay) {
+                                            @Named("SECGROUP_NAME_TO_ID") Function<String, String> groupNameToId,
+                                            @Named("SECURITY") Predicate<RegionAndName> securityGroupEventualConsistencyDelay) {
       this.securityApi = checkNotNull(securityApi, "securityApi");
+      this.groupNameToId = checkNotNull(groupNameToId, "groupNameToId");
       this.securityGroupEventualConsistencyDelay = checkNotNull(securityGroupEventualConsistencyDelay,
             "securityGroupEventualConsistencyDelay");
    }
@@ -117,7 +120,7 @@ public class AWSEC2CreateSecurityGroupIfNeeded extends CacheLoader<RegionAndName
          Set<IpPermission> perms = permissions.build();
 
          if (perms.size() > 0) {
-            String id = Iterables.get(securityApi.describeSecurityGroupsInRegion(region, name), 0).getId();
+            String id = groupNameToId.apply(new RegionAndName(region, name).slashEncode());
             logger.debug(">> authorizing securityGroup region(%s) name(%s) IpPermissions(%s)", region, name, perms);
             securityApi.authorizeSecurityGroupIngressInRegion(region, id, perms);
             logger.debug("<< authorized securityGroup(%s)", name);

providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/extensions/AWSEC2SecurityGroupExtensionExpectTest.java

@@ -28,10 +28,8 @@ import org.jclouds.aws.ec2.compute.internal.BaseAWSEC2ComputeServiceExpectTest;
 import org.jclouds.compute.domain.SecurityGroup;
 import org.jclouds.compute.domain.SecurityGroupBuilder;
 import org.jclouds.compute.extensions.SecurityGroupExtension;
-import org.jclouds.domain.Location;
 import org.jclouds.domain.LocationBuilder;
 import org.jclouds.domain.LocationScope;
-import org.jclouds.ec2.compute.domain.RegionAndName;
 import org.jclouds.http.HttpRequest;
 import org.jclouds.http.HttpResponse;
 import org.jclouds.net.domain.IpPermission;
@@ -46,7 +44,6 @@ import com.google.common.collect.Iterables;
 import com.google.common.collect.LinkedHashMultimap;
 import com.google.common.collect.Multimap;
 import com.google.common.collect.Sets;
-import com.google.common.util.concurrent.Futures;
 
 /**
  * 
@@ -76,7 +73,7 @@ public class AWSEC2SecurityGroupExtensionExpectTest extends BaseAWSEC2ComputeSer
                                  .endpoint("https://ec2." + region + ".amazonaws.com/")
                                  .addHeader("Host", "ec2." + region + ".amazonaws.com")
                                  .addFormParam("Action", "AuthorizeSecurityGroupIngress")
-                                 .addFormParam("GroupId", "jclouds#some-group")
+                                 .addFormParam("GroupId", "sg-3c6ef654")
                                  .addFormParam("IpPermissions.0.FromPort", "22")
                                  .addFormParam("IpPermissions.0.IpProtocol", "tcp")
                                  .addFormParam("IpPermissions.0.IpRanges.0.CidrIp", "0.0.0.0/0")
@@ -150,7 +147,7 @@ public class AWSEC2SecurityGroupExtensionExpectTest extends BaseAWSEC2ComputeSer
                                  .endpoint("https://ec2." + region + ".amazonaws.com/")
                                  .addHeader("Host", "ec2." + region + ".amazonaws.com")
                                  .addFormParam("Action", "AuthorizeSecurityGroupIngress")
-                                 .addFormParam("GroupId", "jclouds#some-group")
+                                 .addFormParam("GroupId", "sg-3c6ef654")
                                  .addFormParam("IpPermissions.0.FromPort", "22")
                                  .addFormParam("IpPermissions.0.IpProtocol", "tcp")
                                  .addFormParam("IpPermissions.0.IpRanges.0.CidrIp", "0.0.0.0/0")
@@ -221,7 +218,7 @@ public class AWSEC2SecurityGroupExtensionExpectTest extends BaseAWSEC2ComputeSer
                                  .endpoint("https://ec2." + region + ".amazonaws.com/")
                                  .addHeader("Host", "ec2." + region + ".amazonaws.com")
                                  .addFormParam("Action", "AuthorizeSecurityGroupIngress")
-                                 .addFormParam("GroupId", "jclouds#some-group")
+                                 .addFormParam("GroupId", "sg-3c6ef654")
                                  .addFormParam("IpPermissions.0.FromPort", "22")
                                  .addFormParam("IpPermissions.0.Groups.0.GroupName", "jclouds#some-group")
                                  .addFormParam("IpPermissions.0.Groups.0.UserId", "993194456877")
@@ -300,7 +297,7 @@ public class AWSEC2SecurityGroupExtensionExpectTest extends BaseAWSEC2ComputeSer
                                  .endpoint("https://ec2." + region + ".amazonaws.com/")
                                  .addHeader("Host", "ec2." + region + ".amazonaws.com")
                                  .addFormParam("Action", "AuthorizeSecurityGroupIngress")
-                                 .addFormParam("GroupId", "jclouds#some-group")
+                                 .addFormParam("GroupId", "sg-3c6ef654")
                                  .addFormParam("IpPermissions.0.FromPort", "22")
                                  .addFormParam("IpPermissions.0.Groups.0.GroupName", "jclouds#some-group")
                                  .addFormParam("IpPermissions.0.Groups.0.UserId", "993194456877")

providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/loaders/AWSEC2CreateSecurityGroupIfNeededTest.java

@@ -29,8 +29,8 @@ import java.util.concurrent.ExecutionException;
 import org.jclouds.aws.ec2.features.AWSSecurityGroupApi;
 import org.jclouds.ec2.compute.domain.RegionAndName;
 import org.jclouds.ec2.compute.domain.RegionNameAndIngressRules;
+import org.jclouds.ec2.compute.functions.EC2SecurityGroupIdFromName;
 import org.jclouds.ec2.domain.SecurityGroup;
-import org.jclouds.ec2.domain.UserIdGroupPair;
 import org.jclouds.net.domain.IpPermission;
 import org.jclouds.net.domain.IpProtocol;
 import org.testng.annotations.Test;
@@ -56,6 +56,8 @@ public class AWSEC2CreateSecurityGroupIfNeededTest {
       SecurityGroup group = createNiceMock(SecurityGroup.class);
       Set<SecurityGroup> groups = ImmutableSet.<SecurityGroup> of(group);
 
+      EC2SecurityGroupIdFromName groupIdFromName = createMock(EC2SecurityGroupIdFromName.class);
+
       ImmutableSet.Builder<IpPermission> permissions = ImmutableSet.builder();
 
       permissions.add(IpPermission.builder()
@@ -80,21 +82,22 @@ public class AWSEC2CreateSecurityGroupIfNeededTest {
       
       client.createSecurityGroupInRegion("region", "group", "group");
       expect(group.getOwnerId()).andReturn("ownerId");
-      expect(group.getId()).andReturn("sg-123456");
+      expect(groupIdFromName.apply("region/group")).andReturn("sg-123456");
-      expect(client.describeSecurityGroupsInRegion("region", "group")).andReturn(Set.class.cast(groups));
       client.authorizeSecurityGroupIngressInRegion("region", "sg-123456", permissions.build());
       expect(client.describeSecurityGroupsInRegion("region", "group")).andReturn(Set.class.cast(groups));
 
 
       replay(client);
       replay(group);
+      replay(groupIdFromName);
 
-      AWSEC2CreateSecurityGroupIfNeeded function = new AWSEC2CreateSecurityGroupIfNeeded(client, tester);
+      AWSEC2CreateSecurityGroupIfNeeded function = new AWSEC2CreateSecurityGroupIfNeeded(client, groupIdFromName, tester);
 
       assertEquals("group", function.load(new RegionNameAndIngressRules("region", "group", new int[] { 22 }, true)));
 
       verify(client);
       verify(group);
+      verify(groupIdFromName);
 
    }
 }