JCLOUDS-367: GCE nodes n>1 ignoring inboundPort

The inboundPort settings of the first node in the group dictated the firewall configuration. Subsequent nodes added to the group had their inboundPort settings ignored.

GCE firewalls specify their "target" (VM instances) by means of tags - if a targetTag on a firewall matches the tag on an instance, the firewall's rules are allowed for the instance. This commit applies a tag for each requested inboundPort to new instances. Then, a firewall is created for each tag (if one does not already exist) which has 'allow' rules for the port.

providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineService.java

@@ -62,7 +62,10 @@ import org.jclouds.domain.Location;
 import org.jclouds.googlecomputeengine.GoogleComputeEngineApi;
 import org.jclouds.googlecomputeengine.compute.options.GoogleComputeEngineTemplateOptions;
 import org.jclouds.googlecomputeengine.config.UserProject;
+import org.jclouds.googlecomputeengine.domain.Firewall;
+import org.jclouds.googlecomputeengine.domain.Network;
 import org.jclouds.googlecomputeengine.domain.Operation;
+import org.jclouds.googlecomputeengine.features.FirewallApi;
 import org.jclouds.http.HttpResponse;
 import org.jclouds.scriptbuilder.functions.InitAdminAccess;
 
@@ -70,6 +73,7 @@ import com.google.common.base.Function;
 import com.google.common.base.Optional;
 import com.google.common.base.Predicate;
 import com.google.common.base.Supplier;
+import com.google.common.collect.Sets;
 import com.google.common.util.concurrent.ListeningExecutorService;
 
 /**
@@ -148,21 +152,34 @@ public class GoogleComputeEngineService extends BaseComputeService {
    }
 
 
-   protected void cleanUpNetworksAndFirewallsForGroup(String groupName) {
+   protected void cleanUpNetworksAndFirewallsForGroup(final String groupName) {
       String resourceName = namingConvention.create().sharedNameForGroup(groupName);
-      AtomicReference<Operation> operation = new AtomicReference<Operation>(api.getFirewallApiForProject(project.get())
+      final Network network = api.getNetworkApiForProject(project.get()).get(resourceName);
-              .delete(resourceName));
+      FirewallApi firewallApi = api.getFirewallApiForProject(project.get());
+      Predicate<Firewall> firewallBelongsToNetwork = new Predicate<Firewall>() {
+         @Override
+         public boolean apply(Firewall input) {
+            return input != null && input.getNetwork().equals(network.getSelfLink());
+         }
+      };
 
-      retry(operationDonePredicate, operationCompleteCheckTimeout, operationCompleteCheckInterval,
+      Set<AtomicReference<Operation>> operations = Sets.newHashSet();
-              MILLISECONDS).apply(operation);
+      for (Firewall firewall : firewallApi.list().concat().filter(firewallBelongsToNetwork)) {
-
+         operations.add(new AtomicReference<Operation>(firewallApi.delete(firewall.getName())));
-      if (operation.get().getHttpError().isPresent()) {
-         HttpResponse response = operation.get().getHttpError().get();
-         logger.warn("delete orphaned firewall failed. Http Error Code: " + response.getStatusCode() +
-                 " HttpError: " + response.getMessage());
       }
 
-      operation = new AtomicReference<Operation>(api.getNetworkApiForProject(project.get()).delete(resourceName));
+      for (AtomicReference<Operation> operation : operations) {
+         retry(operationDonePredicate, operationCompleteCheckTimeout, operationCompleteCheckInterval,
+                 MILLISECONDS).apply(operation);
+
+         if (operation.get().getHttpError().isPresent()) {
+            HttpResponse response = operation.get().getHttpError().get();
+            logger.warn("delete orphaned firewall %s failed. Http Error Code: %d HttpError: %s",
+                    operation.get().getTargetId(), response.getStatusCode(), response.getMessage());
+         }
+      }
+
+      AtomicReference<Operation> operation = new AtomicReference<Operation>(api.getNetworkApiForProject(project.get()).delete(resourceName));
 
       retry(operationDonePredicate, operationCompleteCheckTimeout, operationCompleteCheckInterval,
               MILLISECONDS).apply(operation);

providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceAdapter.java

@@ -29,12 +29,14 @@ import static org.jclouds.util.Predicates2.retry;
 
 import java.net.URI;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 
 import javax.annotation.Resource;
 import javax.inject.Named;
 
+import com.google.common.primitives.Ints;
 import org.jclouds.collect.Memoized;
 import org.jclouds.compute.ComputeServiceAdapter;
 import org.jclouds.compute.domain.Hardware;
@@ -44,6 +46,7 @@ import org.jclouds.compute.reference.ComputeServiceConstants;
 import org.jclouds.domain.Location;
 import org.jclouds.domain.LoginCredentials;
 import org.jclouds.googlecomputeengine.GoogleComputeEngineApi;
+import org.jclouds.googlecomputeengine.compute.functions.FirewallTagNamingConvention;
 import org.jclouds.googlecomputeengine.compute.options.GoogleComputeEngineTemplateOptions;
 import org.jclouds.googlecomputeengine.config.UserProject;
 import org.jclouds.googlecomputeengine.domain.Image;
@@ -55,6 +58,7 @@ import org.jclouds.googlecomputeengine.domain.MachineTypeInZone;
 import org.jclouds.googlecomputeengine.domain.Operation;
 import org.jclouds.googlecomputeengine.domain.SlashEncodedIds;
 import org.jclouds.googlecomputeengine.domain.Zone;
+import org.jclouds.googlecomputeengine.features.InstanceApi;
 import org.jclouds.http.HttpResponse;
 import org.jclouds.logging.Logger;
 
@@ -85,6 +89,7 @@ public class GoogleComputeEngineServiceAdapter implements ComputeServiceAdapter<
    private final Predicate<AtomicReference<Operation>> retryOperationDonePredicate;
    private final long operationCompleteCheckInterval;
    private final long operationCompleteCheckTimeout;
+   private final FirewallTagNamingConvention.Factory firewallTagNamingConvention;
 
    @Inject
    public GoogleComputeEngineServiceAdapter(GoogleComputeEngineApi api,
@@ -95,7 +100,8 @@ public class GoogleComputeEngineServiceAdapter implements ComputeServiceAdapter<
                                             @Named(OPERATION_COMPLETE_INTERVAL) Long operationCompleteCheckInterval,
                                             @Named(OPERATION_COMPLETE_TIMEOUT) Long operationCompleteCheckTimeout,
                                             @Memoized Supplier<Map<URI, ? extends Location>> zones,
-                                            @Memoized Supplier<Map<URI, ? extends Hardware>> hardwareMap) {
+                                            @Memoized Supplier<Map<URI, ? extends Hardware>> hardwareMap,
+                                            FirewallTagNamingConvention.Factory firewallTagNamingConvention) {
       this.api = checkNotNull(api, "google compute api");
       this.userProject = checkNotNull(userProject, "user project name");
       this.metatadaFromTemplateOptions = checkNotNull(metatadaFromTemplateOptions,
@@ -108,6 +114,7 @@ public class GoogleComputeEngineServiceAdapter implements ComputeServiceAdapter<
               operationCompleteCheckInterval, TimeUnit.MILLISECONDS);
       this.zones = checkNotNull(zones, "zones");
       this.hardwareMap = checkNotNull(hardwareMap, "hardwareMap");
+      this.firewallTagNamingConvention = checkNotNull(firewallTagNamingConvention, "firewallTagNamingConvention");
    }
 
    @Override
@@ -138,8 +145,9 @@ public class GoogleComputeEngineServiceAdapter implements ComputeServiceAdapter<
       instanceTemplate.serviceAccounts(options.getServiceAccounts());
       instanceTemplate.image(checkNotNull(template.getImage().getUri(), "image URI is null"));
 
-      Operation operation = api.getInstanceApiForProject(userProject.get())
+      final InstanceApi instanceApi = api.getInstanceApiForProject(userProject.get());
-              .createInZone(name, template.getLocation().getId(), instanceTemplate);
+      final String zone = template.getLocation().getId();
+      Operation operation = instanceApi.createInZone(name, zone, instanceTemplate);
 
       if (options.shouldBlockUntilRunning()) {
          waitOperationDone(operation);
@@ -151,14 +159,13 @@ public class GoogleComputeEngineServiceAdapter implements ComputeServiceAdapter<
       retry(new Predicate<AtomicReference<Instance>>() {
          @Override
          public boolean apply(AtomicReference<Instance> input) {
-            input.set(api.getInstanceApiForProject(userProject.get()).getInZone(template.getLocation().getId(),
+            input.set(instanceApi.getInZone(zone, name));
-                    name));
             return input.get() != null;
          }
       }, operationCompleteCheckTimeout, operationCompleteCheckInterval, MILLISECONDS).apply(instance);
 
-      if (options.getTags().size() > 0) {
+      if (!options.getTags().isEmpty()) {
-         Operation tagsOperation = api.getInstanceApiForProject(userProject.get()).setTagsInZone(template.getLocation().getId(),
+         Operation tagsOperation = instanceApi.setTagsInZone(zone,
                  name, options.getTags(), instance.get().getTags().getFingerprint());
 
          waitOperationDone(tagsOperation);
@@ -166,14 +173,27 @@ public class GoogleComputeEngineServiceAdapter implements ComputeServiceAdapter<
          retry(new Predicate<AtomicReference<Instance>>() {
             @Override
             public boolean apply(AtomicReference<Instance> input) {
-               input.set(api.getInstanceApiForProject(userProject.get()).getInZone(template.getLocation().getId(),
+               input.set(instanceApi.getInZone(zone, name));
-                       name));
                return input.get() != null;
             }
          }, operationCompleteCheckTimeout, operationCompleteCheckInterval, MILLISECONDS).apply(instance);
       }
 
-      InstanceInZone instanceInZone = new InstanceInZone(instance.get(), template.getLocation().getId());
+      // Add tags for security groups
+      final FirewallTagNamingConvention naming = firewallTagNamingConvention.get(group);
+      Set<String> tags = FluentIterable.from(Ints.asList(options.getInboundPorts()))
+              .transform(new Function<Integer, String>(){
+                       @Override
+                       public String apply(Integer input) {
+                          return input != null
+                                  ? naming.name(input)
+                                  : null;
+                       }
+                    })
+              .toSet();
+      instanceApi.setTagsInZone(zone, instance.get().getName(), tags, instance.get().getTags().getFingerprint());
+
+      InstanceInZone instanceInZone = new InstanceInZone(instance.get(), zone);
 
       return new NodeAndInitialCredentials<InstanceInZone>(instanceInZone, instanceInZone.slashEncode(), credentials);
    }

providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/config/GoogleComputeEngineServiceContextModule.java

@@ -29,6 +29,7 @@ import java.util.concurrent.atomic.AtomicReference;
 import javax.inject.Named;
 import javax.inject.Singleton;
 
+import com.google.inject.Scopes;
 import org.jclouds.collect.Memoized;
 import org.jclouds.compute.ComputeService;
 import org.jclouds.compute.ComputeServiceAdapter;
@@ -46,6 +47,7 @@ import org.jclouds.googlecomputeengine.compute.GoogleComputeEngineService;
 import org.jclouds.googlecomputeengine.compute.GoogleComputeEngineServiceAdapter;
 import org.jclouds.googlecomputeengine.compute.extensions.GoogleComputeEngineSecurityGroupExtension;
 import org.jclouds.googlecomputeengine.compute.functions.BuildInstanceMetadata;
+import org.jclouds.googlecomputeengine.compute.functions.FirewallTagNamingConvention;
 import org.jclouds.googlecomputeengine.compute.functions.FirewallToIpPermission;
 import org.jclouds.googlecomputeengine.compute.functions.GoogleComputeEngineImageToImage;
 import org.jclouds.googlecomputeengine.compute.functions.InstanceInZoneToNodeMetadata;
@@ -154,6 +156,7 @@ public class GoogleComputeEngineServiceContextModule
 
       install(new LocationsFromComputeServiceAdapterModule<InstanceInZone, MachineTypeInZone, Image, Zone>() {});
 
+      bind(FirewallTagNamingConvention.Factory.class).in(Scopes.SINGLETON);
    }
 
    @Provides

providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/FirewallTagNamingConvention.java

@@ -0,0 +1,64 @@
+/*
+ * 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.googlecomputeengine.compute.functions;
+
+import com.google.common.base.Predicate;
+import org.jclouds.compute.functions.GroupNamingConvention;
+
+import javax.inject.Inject;
+
+/**
+ * The convention for naming instance tags that firewall rules recognise.
+ *
+ * @author richardcloudsoft
+ */
+public class FirewallTagNamingConvention {
+
+   public static class Factory {
+
+      private final GroupNamingConvention.Factory namingConvention;
+
+      @Inject
+      public Factory(GroupNamingConvention.Factory namingConvention) {
+         this.namingConvention = namingConvention;
+      }
+
+      public FirewallTagNamingConvention get(String groupName) {
+         return new FirewallTagNamingConvention(namingConvention.create().sharedNameForGroup(groupName));
+      }
+   }
+
+   private final String sharedResourceName;
+
+   public FirewallTagNamingConvention(String sharedResourceName) {
+      this.sharedResourceName = sharedResourceName;
+   }
+
+   public String name(int port) {
+      return String.format("%s-port-%s", sharedResourceName, port);
+   }
+
+   public Predicate<? super String> isFirewallTag() {
+      return new Predicate<String>() {
+         @Override
+         public boolean apply(String input) {
+            return input != null && input.startsWith(sharedResourceName + "-port-");
+         }
+      };
+   }
+
+}

providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/InstanceInZoneToNodeMetadata.java

@@ -36,7 +36,9 @@ import org.jclouds.googlecomputeengine.domain.InstanceInZone;
 import org.jclouds.googlecomputeengine.domain.SlashEncodedIds;
 
 import com.google.common.base.Function;
+import com.google.common.base.Predicates;
 import com.google.common.base.Supplier;
+import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableSet;
 
 /**
@@ -51,18 +53,21 @@ public class InstanceInZoneToNodeMetadata implements Function<InstanceInZone, No
    private final Supplier<Map<URI, ? extends Image>> images;
    private final Supplier<Map<URI, ? extends Hardware>> hardwares;
    private final Supplier<Map<URI, ? extends Location>> locations;
+   private final FirewallTagNamingConvention.Factory firewallTagNamingConvention;
 
    @Inject
    public InstanceInZoneToNodeMetadata(Map<Instance.Status, NodeMetadata.Status> toPortableNodeStatus,
                                  GroupNamingConvention.Factory namingConvention,
                                  @Memoized Supplier<Map<URI, ? extends Image>> images,
                                  @Memoized Supplier<Map<URI, ? extends Hardware>> hardwares,
-                                 @Memoized Supplier<Map<URI, ? extends Location>> locations) {
+                                 @Memoized Supplier<Map<URI, ? extends Location>> locations,
+                                 FirewallTagNamingConvention.Factory firewallTagNamingConvention) {
       this.toPortableNodeStatus = toPortableNodeStatus;
       this.nodeNamingConvention = namingConvention.createWithoutPrefix();
       this.images = images;
       this.hardwares = hardwares;
       this.locations = locations;
+      this.firewallTagNamingConvention = checkNotNull(firewallTagNamingConvention, "firewallTagNamingConvention");
    }
 
    @Override
@@ -72,6 +77,10 @@ public class InstanceInZoneToNodeMetadata implements Function<InstanceInZone, No
       Image image = checkNotNull(imagesMap.get(checkNotNull(input.getImage(), "image")),
               "no image for %s. images: %s", input.getImage(), imagesMap.values());
 
+      String group = nodeNamingConvention.groupInUniqueNameOrNull(input.getName());
+      FluentIterable<String> tags = FluentIterable.from(input.getTags().getItems())
+              .filter(Predicates.not(firewallTagNamingConvention.get(group).isFirewallTag()));
+
       return new NodeMetadataBuilder()
               .id(SlashEncodedIds.fromTwoIds(checkNotNull(locations.get().get(input.getZone()), "location for %s", input.getZone()).getId(),
                       input.getName()).slashEncode())
@@ -84,10 +93,10 @@ public class InstanceInZoneToNodeMetadata implements Function<InstanceInZone, No
                       input.getMachineType().toString()))
               .operatingSystem(image.getOperatingSystem())
               .status(toPortableNodeStatus.get(input.getStatus()))
-              .tags(input.getTags().getItems())
+              .tags(tags)
               .uri(input.getSelfLink())
               .userMetadata(input.getMetadata().getItems())
-              .group(nodeNamingConvention.groupInUniqueNameOrNull(input.getName()))
+              .group(group)
               .privateAddresses(collectPrivateAddresses(input))
               .publicAddresses(collectPublicAddresses(input))
               .build();

providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/strategy/CreateNodesWithGroupEncodedIntoNameThenAddToSet.java

@@ -31,6 +31,7 @@ import java.util.concurrent.atomic.AtomicReference;
 import javax.inject.Inject;
 import javax.inject.Named;
 
+import com.google.common.collect.Sets;
 import org.jclouds.Constants;
 import org.jclouds.compute.config.CustomizationResponse;
 import org.jclouds.compute.domain.NodeMetadata;
@@ -40,11 +41,13 @@ import org.jclouds.compute.strategy.CreateNodeWithGroupEncodedIntoName;
 import org.jclouds.compute.strategy.CustomizeNodeAndAddToGoodMapOrPutExceptionIntoBadMap;
 import org.jclouds.compute.strategy.ListNodesStrategy;
 import org.jclouds.googlecomputeengine.GoogleComputeEngineApi;
+import org.jclouds.googlecomputeengine.compute.functions.FirewallTagNamingConvention;
 import org.jclouds.googlecomputeengine.compute.options.GoogleComputeEngineTemplateOptions;
 import org.jclouds.googlecomputeengine.config.UserProject;
 import org.jclouds.googlecomputeengine.domain.Firewall;
 import org.jclouds.googlecomputeengine.domain.Network;
 import org.jclouds.googlecomputeengine.domain.Operation;
+import org.jclouds.googlecomputeengine.features.FirewallApi;
 import org.jclouds.googlecomputeengine.domain.internal.NetworkAndAddressRange;
 import org.jclouds.googlecomputeengine.options.FirewallOptions;
 import org.jclouds.net.domain.IpProtocol;
@@ -72,6 +75,7 @@ public class CreateNodesWithGroupEncodedIntoNameThenAddToSet extends
    private final Predicate<AtomicReference<Operation>> operationDonePredicate;
    private final long operationCompleteCheckInterval;
    private final long operationCompleteCheckTimeout;
+   private final FirewallTagNamingConvention.Factory firewallTagNamingConvention;
 
    @Inject
    protected CreateNodesWithGroupEncodedIntoNameThenAddToSet(
@@ -87,7 +91,8 @@ public class CreateNodesWithGroupEncodedIntoNameThenAddToSet extends
            @Named("global") Predicate<AtomicReference<Operation>> operationDonePredicate,
            @Named(OPERATION_COMPLETE_INTERVAL) Long operationCompleteCheckInterval,
            @Named(OPERATION_COMPLETE_TIMEOUT) Long operationCompleteCheckTimeout,
-           LoadingCache<NetworkAndAddressRange, Network> networkMap) {
+           LoadingCache<NetworkAndAddressRange, Network> networkMap,
+           FirewallTagNamingConvention.Factory firewallTagNamingConvention) {
       super(addNodeWithGroupStrategy, listNodesStrategy, namingConvention, userExecutor,
               customizeNodeAndAddToGoodMapOrPutExceptionIntoBadMapFactory);
 
@@ -99,6 +104,7 @@ public class CreateNodesWithGroupEncodedIntoNameThenAddToSet extends
               "operation completed check timeout");
       this.operationDonePredicate = checkNotNull(operationDonePredicate, "operationDonePredicate");
       this.networkMap = checkNotNull(networkMap, "networkMap");
+      this.firewallTagNamingConvention = checkNotNull(firewallTagNamingConvention, "firewallTagNamingConvention");
    }
 
    @Override
@@ -116,7 +122,7 @@ public class CreateNodesWithGroupEncodedIntoNameThenAddToSet extends
 
       // get or create the network and create a firewall with the users configuration
       Network network = getOrCreateNetwork(templateOptions, sharedResourceName);
-      getOrCreateFirewall(templateOptions, network, sharedResourceName);
+      getOrCreateFirewalls(templateOptions, network, firewallTagNamingConvention.get(group));
       templateOptions.network(network.getSelfLink());
 
       return super.execute(group, count, mutableTemplate, goodNodes, badNodes, customizationResponses);
@@ -133,51 +139,45 @@ public class CreateNodesWithGroupEncodedIntoNameThenAddToSet extends
    }
 
    /**
-    * Tries to find if a firewall already exists for this group, if not it creates one.
+    * Ensures that a firewall exists for every inbound port that the instance requests.
-    *
+    * <p>
+    * For each port, there must be a firewall with a name following the {@link FirewallTagNamingConvention},
+    * with a target tag also following the {@link FirewallTagNamingConvention}, which opens the requested port
+    * for all sources on both TCP and UDP protocols.
     * @see org.jclouds.googlecomputeengine.features.FirewallApi#patch(String, org.jclouds.googlecomputeengine.options.FirewallOptions)
     */
-   private void getOrCreateFirewall(GoogleComputeEngineTemplateOptions templateOptions, Network network,
+   private void getOrCreateFirewalls(GoogleComputeEngineTemplateOptions templateOptions, Network network,
-                                    String sharedResourceName) {
+                                     FirewallTagNamingConvention naming) {
 
-      Firewall firewall = api.getFirewallApiForProject(userProject.get()).get(sharedResourceName);
+      String projectName = userProject.get();
+      FirewallApi firewallApi = api.getFirewallApiForProject(projectName);
+      Set<AtomicReference<Operation>> operations = Sets.newHashSet();
 
-      if (firewall != null) {
-         return;
-      }
-
-      ImmutableSet.Builder<Firewall.Rule> rules = ImmutableSet.builder();
-
-      Firewall.Rule.Builder tcpRule = Firewall.Rule.builder();
-      tcpRule.IpProtocol(IpProtocol.TCP);
-      Firewall.Rule.Builder udpRule = Firewall.Rule.builder();
-      udpRule.IpProtocol(IpProtocol.UDP);
       for (Integer port : templateOptions.getInboundPorts()) {
-         tcpRule.addPort(port);
+         String name = naming.name(port);
-         udpRule.addPort(port);
+         Firewall firewall = firewallApi.get(name);
+         if (firewall == null) {
+            ImmutableSet<Firewall.Rule> rules = ImmutableSet.of(Firewall.Rule.permitTcpRule(port), Firewall.Rule.permitUdpRule(port));
+            FirewallOptions firewallOptions = new FirewallOptions()
+                    .name(name)
+                    .network(network.getSelfLink())
+                    .allowedRules(rules)
+                    .sourceTags(templateOptions.getTags())
+                    .sourceRanges(of(DEFAULT_INTERNAL_NETWORK_RANGE, EXTERIOR_RANGE))
+                    .targetTags(ImmutableSet.of(name));
+            AtomicReference<Operation> operation = new AtomicReference<Operation>(firewallApi.createInNetwork(
+                    firewallOptions.getName(),
+                    network.getSelfLink(),
+                    firewallOptions));
+            operations.add(operation);
+         }
       }
-      rules.add(tcpRule.build());
-      rules.add(udpRule.build());
 
-
+      for (AtomicReference<Operation> operation : operations) {
-      FirewallOptions options = new FirewallOptions()
+         retry(operationDonePredicate, operationCompleteCheckTimeout, operationCompleteCheckInterval,
-              .name(sharedResourceName)
+                 MILLISECONDS).apply(operation);
-              .network(network.getSelfLink())
+         checkState(!operation.get().getHttpError().isPresent(),"Could not create firewall, operation failed" + operation);
-              .sourceTags(templateOptions.getTags())
+      }
-              .allowedRules(rules.build())
-              .sourceRanges(of(DEFAULT_INTERNAL_NETWORK_RANGE, EXTERIOR_RANGE));
-
-      AtomicReference<Operation> operation = new AtomicReference<Operation>(api.getFirewallApiForProject(userProject
-              .get()).createInNetwork(
-              sharedResourceName,
-              network.getSelfLink(),
-              options));
-
-      retry(operationDonePredicate, operationCompleteCheckTimeout, operationCompleteCheckInterval,
-              MILLISECONDS).apply(operation);
-
-      checkState(!operation.get().getHttpError().isPresent(), "Could not create firewall, operation failed" + operation);
    }
 
-
 }

providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/Firewall.java

@@ -250,6 +250,11 @@ public final class Firewall extends Resource {
       private final IpProtocol ipProtocol;
       private final RangeSet<Integer> ports;
 
+      /* Some handy shortcuts */
+      public static Rule permitTcpRule(Integer start, Integer end) { return Rule.builder().IpProtocol(IpProtocol.TCP).addPortRange(start, end).build(); }
+      public static Rule permitTcpRule(Integer port) { return Rule.builder().IpProtocol(IpProtocol.TCP).addPort(port).build(); }
+      public static Rule permitUdpRule(Integer start, Integer end) { return Rule.builder().IpProtocol(IpProtocol.UDP).addPortRange(start, end).build(); }
+      public static Rule permitUdpRule(Integer port) { return Rule.builder().IpProtocol(IpProtocol.UDP).addPort(port).build(); }
       @ConstructorProperties({
               "IpProtocol", "ports"
       })

providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceExpectTest.java

@@ -269,6 +269,26 @@ public class GoogleComputeEngineServiceExpectTest extends BaseGoogleComputeEngin
               .addHeader("Accept", "application/json")
               .addHeader("Authorization", "Bearer " + TOKEN).build();
 
+      HttpRequest getNetworkRequest = HttpRequest.builder()
+              .method("GET")
+              .endpoint("https://www.googleapis" +
+                      ".com/compute/v1beta16/projects/myproject/global/networks/jclouds-test-delete")
+              .addHeader("Accept", "application/json")
+              .addHeader("Authorization", "Bearer " + TOKEN).build();
+
+      HttpResponse getNetworkResponse = HttpResponse.builder().statusCode(200)
+              .payload(staticPayloadFromResource("/GoogleComputeEngineServiceExpectTest/network_get.json")).build();
+
+      HttpRequest listFirewallsRequest = HttpRequest.builder()
+              .method("GET")
+              .endpoint("https://www.googleapis" +
+                      ".com/compute/v1beta16/projects/myproject/global/firewalls")
+              .addHeader("Accept", "application/json")
+              .addHeader("Authorization", "Bearer " + TOKEN).build();
+
+      HttpResponse listFirewallsResponse = HttpResponse.builder().statusCode(200)
+              .payload(staticPayloadFromResource("/GoogleComputeEngineServiceExpectTest/firewall_list.json")).build();
+
       HttpRequest deleteNetworkReqquest = HttpRequest.builder()
               .method("DELETE")
               .endpoint("https://www.googleapis" +
@@ -289,6 +309,8 @@ public class GoogleComputeEngineServiceExpectTest extends BaseGoogleComputeEngin
               .add(GET_ZONE_OPERATION_REQUEST)
               .add(getInstanceRequestForInstance("test-delete-networks"))
               .add(LIST_INSTANCES_REQUEST)
+              .add(getNetworkRequest)
+              .add(listFirewallsRequest)
               .add(deleteFirewallRequest)
               .add(GET_GLOBAL_OPERATION_REQUEST)
               .add(deleteNetworkReqquest)
@@ -313,6 +335,8 @@ public class GoogleComputeEngineServiceExpectTest extends BaseGoogleComputeEngin
               .add(getListInstancesResponseForSingleInstanceAndNetworkAndStatus("test-delete-networks",
                       "test-network", Instance
                       .Status.TERMINATED.name()))
+              .add(getNetworkResponse)
+              .add(listFirewallsResponse)
               .add(SUCESSFULL_OPERATION_RESPONSE)
               .add(GET_GLOBAL_OPERATION_RESPONSE)
               .add(SUCESSFULL_OPERATION_RESPONSE)
@@ -363,6 +387,38 @@ public class GoogleComputeEngineServiceExpectTest extends BaseGoogleComputeEngin
       HttpResponse getInstanceResponse = HttpResponse.builder().statusCode(200)
               .payload(payloadFromStringWithContentType(payload, "application/json")).build();
 
+      HttpRequest getFirewallRequest = HttpRequest
+                 .builder()
+                 .method("GET")
+                 .endpoint("https://www.googleapis" +
+                         ".com/compute/v1beta16/projects/myproject/global/firewalls/jclouds-test-port-22")
+                 .addHeader("Accept", "application/json")
+                 .addHeader("Authorization", "Bearer " + TOKEN).build();
+
+      HttpRequest insertFirewallRequest = HttpRequest
+                 .builder()
+                 .method("POST")
+                 .endpoint("https://www.googleapis.com/compute/v1beta16/projects/myproject/global/firewalls")
+                 .addHeader("Accept", "application/json")
+                 .addHeader("Authorization", "Bearer " + TOKEN)
+                 .payload(payloadFromStringWithContentType("{\"name\":\"jclouds-test-port-22\",\"network\":\"https://www.googleapis" +
+                         ".com/compute/v1beta16/projects/myproject/global/networks/jclouds-test\"," +
+                         "\"sourceRanges\":[\"10.0.0.0/8\",\"0.0.0.0/0\"],\"sourceTags\":[\"aTag\"],\"targetTags\":[\"jclouds-test-port-22\"],\"allowed\":[{\"IPProtocol\":\"tcp\"," +
+                         "\"ports\":[\"22\"]}," +
+                         "{\"IPProtocol\":\"udp\",\"ports\":[\"22\"]}]}",
+                         MediaType.APPLICATION_JSON))
+                 .build();
+
+      HttpRequest setTagsRequest = HttpRequest
+                 .builder()
+                 .method("POST")
+                 .endpoint("https://www.googleapis.com/compute/v1beta16/projects/myproject/zones/us-central1-a/instances/test-1/setTags")
+                 .addHeader("Accept", "application/json")
+                 .addHeader("Authorization", "Bearer " + TOKEN)
+                 .payload(payloadFromStringWithContentType("{\"items\":[\"jclouds-test-port-22\"],\"fingerprint\":\"abcd\"}",
+                         MediaType.APPLICATION_JSON))
+                 .build();
+
       List<HttpRequest> orderedRequests = ImmutableList.<HttpRequest>builder()
               .add(requestForScopes(COMPUTE_READONLY_SCOPE))
               .add(GET_PROJECT_REQUEST)
@@ -377,8 +433,8 @@ public class GoogleComputeEngineServiceExpectTest extends BaseGoogleComputeEngin
               .add(INSERT_NETWORK_REQUEST)
               .add(GET_GLOBAL_OPERATION_REQUEST)
               .add(GET_NETWORK_REQUEST)
-              .add(GET_FIREWALL_REQUEST)
+              .add(getFirewallRequest)
-              .add(INSERT_FIREWALL_REQUEST)
+              .add(insertFirewallRequest)
               .add(GET_GLOBAL_OPERATION_REQUEST)
               .add(LIST_INSTANCES_REQUEST)
               .add(LIST_PROJECT_IMAGES_REQUEST)
@@ -390,6 +446,8 @@ public class GoogleComputeEngineServiceExpectTest extends BaseGoogleComputeEngin
               .add(SET_TAGS_REQUEST)
               .add(GET_ZONE_OPERATION_REQUEST)
               .add(getInstanceRequestForInstance("test-1"))
+              .add(setTagsRequest)
+              .add(setTagsRequest)
               .build();
 
       List<HttpResponse> orderedResponses = ImmutableList.<HttpResponse>builder()
@@ -419,6 +477,8 @@ public class GoogleComputeEngineServiceExpectTest extends BaseGoogleComputeEngin
               .add(SET_TAGS_RESPONSE)
               .add(GET_ZONE_OPERATION_RESPONSE)
               .add(getInstanceResponse)
+              .add(SUCESSFULL_OPERATION_RESPONSE)
+              .add(SUCESSFULL_OPERATION_RESPONSE)
               .build();
 
 

providers/google-compute-engine/src/test/resources/GoogleComputeEngineServiceExpectTest/firewall_list.json

@@ -0,0 +1,37 @@
+{
+   "kind": "compute#firewallList",
+   "id": "projects/google/firewalls",
+   "selfLink": "https://www.googleapis.com/compute/v1beta16/projects/google/global/firewalls",
+   "items": [
+      {
+
+         "kind": "compute#firewall",
+         "id": "12862241031274216284",
+         "creationTimestamp": "2012-04-13T03:05:02.855",
+         "selfLink": "https://www.googleapis.com/compute/v1beta16/projects/myproject/global/firewalls/jclouds-test-delete",
+         "name": "jclouds-test-delete",
+         "description": "Internal traffic from default allowed",
+         "network": "https://www.googleapis.com/compute/v1beta16/projects/myproject/global/networks/jclouds-test-delete",
+         "sourceRanges": [
+            "10.0.0.0/8"
+         ],
+         "allowed": [
+            {
+               "IPProtocol": "tcp",
+               "ports": [
+                  "1-65535"
+               ]
+            },
+            {
+               "IPProtocol": "udp",
+               "ports": [
+                  "1-65535"
+               ]
+            },
+            {
+               "IPProtocol": "icmp"
+            }
+         ]
+      }
+   ]
+}

providers/google-compute-engine/src/test/resources/GoogleComputeEngineServiceExpectTest/network_get.json

@@ -0,0 +1,10 @@
+{
+   "kind": "compute#network",
+   "id": "13024414170909937976",
+   "creationTimestamp": "2012-10-24T20:13:19.967",
+   "selfLink": "https://www.googleapis.com/compute/v1beta16/projects/myproject/global/networks/jclouds-test-delete",
+   "name": "jclouds-test-delete",
+   "description": "Default network for the project",
+   "IPv4Range": "10.0.0.0/8",
+   "gatewayIPv4": "10.0.0.1"
+}