From 077dca824cf2052e3736bbab5f73b39a27ceea7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Est=C3=A9vez?= Date: Fri, 17 May 2019 02:59:01 -0400 Subject: [PATCH] Checks provisioning state in Rule resource instead of Group (#30) * Checks provisioning state in Rule resource instead of Group * comments by @nacx to proper check and delete rules * Fixes log message deleting rule --- .../compute/config/AzurePredicatesModule.java | 35 +++++++++++++++++++ .../AzureComputeSecurityGroupExtension.java | 23 ++++++------ .../domain/NetworkSecurityRuleProperties.java | 18 ++++++---- .../NetworkSecurityGroupApiMockTest.java | 1 + .../NetworkSecurityRuleApiMockTest.java | 1 + .../resources/networksecurityrulecreate.json | 5 +-- 6 files changed, 64 insertions(+), 19 deletions(-) diff --git a/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/config/AzurePredicatesModule.java b/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/config/AzurePredicatesModule.java index 01650de799..80e1a6e783 100644 --- a/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/config/AzurePredicatesModule.java +++ b/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/config/AzurePredicatesModule.java @@ -45,6 +45,7 @@ import org.jclouds.azurecompute.arm.domain.Image; import org.jclouds.azurecompute.arm.domain.Key.DeletedKeyBundle; import org.jclouds.azurecompute.arm.domain.Key.KeyBundle; import org.jclouds.azurecompute.arm.domain.NetworkSecurityGroup; +import org.jclouds.azurecompute.arm.domain.NetworkSecurityRule; import org.jclouds.azurecompute.arm.domain.Provisionable; import org.jclouds.azurecompute.arm.domain.ResourceDefinition; import org.jclouds.azurecompute.arm.domain.Secret.DeletedSecretBundle; @@ -122,6 +123,12 @@ public class AzurePredicatesModule extends AbstractModule { return new SecurityGroupAvailablePredicateFactory(api, resourceAvailable); } + @Provides + protected SecurityGroupRuleAvailablePredicateFactory provideSecurityGroupRuleAvailablePredicate(final AzureComputeApi api, + Predicate> resourceAvailable) { + return new SecurityGroupRuleAvailablePredicateFactory(api, resourceAvailable); + } + @Provides protected ImageAvailablePredicateFactory provideImageAvailablePredicate(final AzureComputeApi api, final ComputeServiceConstants.Timeouts timeouts, final PollPeriod pollPeriod) { @@ -292,6 +299,34 @@ public class AzurePredicatesModule extends AbstractModule { } } + public static class SecurityGroupRuleAvailablePredicateFactory { + private final AzureComputeApi api; + private final Predicate> resourceAvailable; + + SecurityGroupRuleAvailablePredicateFactory(final AzureComputeApi api, Predicate> resourceAvailable) { + this.api = checkNotNull(api, "api cannot be null"); + this.resourceAvailable = resourceAvailable; + } + + public Predicate create(final String resourceGroup, final String securityGroupName) { + checkNotNull(resourceGroup, "resourceGroup cannot be null"); + checkNotNull(securityGroupName, "securityGroupName cannot be null"); + return new Predicate() { + @Override + public boolean apply(final String name) { + checkNotNull(name, "name cannot be null"); + return resourceAvailable.apply(new Supplier() { + @Override + public Provisionable get() { + NetworkSecurityRule securityRule = api.getNetworkSecurityRuleApi(resourceGroup, securityGroupName).get(name); + return securityRule == null ? null : securityRule.properties(); + } + }); + } + }; + } + } + public static class NetworkAvailablePredicateFactory { private final AzureComputeApi api; private final Predicate> resourceAvailable; diff --git a/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/extensions/AzureComputeSecurityGroupExtension.java b/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/extensions/AzureComputeSecurityGroupExtension.java index b85628c621..bfbd595b9a 100644 --- a/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/extensions/AzureComputeSecurityGroupExtension.java +++ b/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/extensions/AzureComputeSecurityGroupExtension.java @@ -30,13 +30,13 @@ import java.net.URI; import java.util.ArrayList; import java.util.List; import java.util.Set; - import javax.annotation.Resource; import javax.inject.Inject; import javax.inject.Named; import org.jclouds.azurecompute.arm.AzureComputeApi; import org.jclouds.azurecompute.arm.compute.config.AzurePredicatesModule.SecurityGroupAvailablePredicateFactory; +import org.jclouds.azurecompute.arm.compute.config.AzurePredicatesModule.SecurityGroupRuleAvailablePredicateFactory; import org.jclouds.azurecompute.arm.compute.domain.ResourceGroupAndName; import org.jclouds.azurecompute.arm.domain.NetworkInterfaceCard; import org.jclouds.azurecompute.arm.domain.NetworkProfile.NetworkInterface; @@ -78,20 +78,21 @@ public class AzureComputeSecurityGroupExtension implements SecurityGroupExtensio private final AzureComputeApi api; private final Function securityGroupConverter; private final SecurityGroupAvailablePredicateFactory securityGroupAvailable; + private final SecurityGroupRuleAvailablePredicateFactory securityGroupRuleAvailable; private final Predicate resourceDeleted; private final LoadingCache defaultResourceGroup; private final Supplier> regionIds; @Inject - AzureComputeSecurityGroupExtension(AzureComputeApi api, - Function groupConverter, - SecurityGroupAvailablePredicateFactory securityRuleAvailable, + AzureComputeSecurityGroupExtension(AzureComputeApi api, Function groupConverter, + SecurityGroupAvailablePredicateFactory securityGroupAvailable, SecurityGroupRuleAvailablePredicateFactory securityGroupRuleAvailable, @Named(TIMEOUT_RESOURCE_DELETED) Predicate resourceDeleted, LoadingCache defaultResourceGroup, @Region Supplier> regionIds) { this.api = api; this.securityGroupConverter = groupConverter; - this.securityGroupAvailable = securityRuleAvailable; + this.securityGroupAvailable = securityGroupAvailable; + this.securityGroupRuleAvailable = securityGroupRuleAvailable; this.resourceDeleted = resourceDeleted; this.defaultResourceGroup = defaultResourceGroup; this.regionIds = regionIds; @@ -252,8 +253,7 @@ public class AzureComputeSecurityGroupExtension implements SecurityGroupExtensio ruleApi.createOrUpdate(ruleName, properties); - checkState( - securityGroupAvailable.create(resourceGroupAndName.resourceGroup()).apply(networkSecurityGroup.name()), + checkState(securityGroupRuleAvailable.create(resourceGroupAndName.resourceGroup(), networkSecurityGroup.name()).apply(ruleName), "Security group was not updated in the configured timeout"); } @@ -294,10 +294,11 @@ public class AzureComputeSecurityGroupExtension implements SecurityGroupExtensio for (NetworkSecurityRule matchingRule : rules) { logger.debug(">> deleting network security rule %s from %s...", matchingRule.name(), group.getName()); - ruleApi.delete(matchingRule.name()); - checkState( - securityGroupAvailable.create(resourceGroupAndName.resourceGroup()).apply(networkSecurityGroup.name()), - "Security group was not updated in the configured timeout"); + URI uri = ruleApi.delete(matchingRule.name()); + if (uri != null) { + checkState(resourceDeleted.apply(uri), "Rule %s could not be deleted in the configured timeout", matchingRule.id()); + } + } return getSecurityGroupById(group.getId()); diff --git a/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/NetworkSecurityRuleProperties.java b/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/NetworkSecurityRuleProperties.java index e93107e92e..d52c7fc715 100644 --- a/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/NetworkSecurityRuleProperties.java +++ b/providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/NetworkSecurityRuleProperties.java @@ -16,14 +16,14 @@ */ package org.jclouds.azurecompute.arm.domain; -import com.google.auto.value.AutoValue; - import org.jclouds.azurecompute.arm.util.GetEnumValue; import org.jclouds.javax.annotation.Nullable; import org.jclouds.json.SerializedNames; +import com.google.auto.value.AutoValue; + @AutoValue -public abstract class NetworkSecurityRuleProperties { +public abstract class NetworkSecurityRuleProperties implements Provisionable { public enum Protocol { // * is an allowed value, will handle in Tcp("Tcp"), @@ -91,7 +91,10 @@ public abstract class NetworkSecurityRuleProperties { public abstract Direction direction(); - @SerializedNames({"description", "protocol", "sourcePortRange", "destinationPortRange", "sourceAddressPrefix", "destinationAddressPrefix", "access", "priority", "direction"}) + @Nullable + public abstract String provisioningState(); + + @SerializedNames({ "description", "protocol", "sourcePortRange", "destinationPortRange", "sourceAddressPrefix", "destinationAddressPrefix", "access", "priority", "direction", "provisioningState" }) public static NetworkSecurityRuleProperties create(final String description, final Protocol protocol, final String sourcePortRange, @@ -100,7 +103,8 @@ public abstract class NetworkSecurityRuleProperties { final String destinationAddressPrefix, final Access access, final Integer priority, - final Direction direction) { + final Direction direction, + final String provisioningState) { return builder() .description(description) .protocol(protocol) @@ -110,7 +114,7 @@ public abstract class NetworkSecurityRuleProperties { .destinationAddressPrefix(destinationAddressPrefix) .access(access) .priority(priority) - .direction(direction) + .direction(direction).provisioningState(provisioningState) .build(); } @@ -140,6 +144,8 @@ public abstract class NetworkSecurityRuleProperties { public abstract Builder direction(Direction direction); + public abstract Builder provisioningState(String provisioningState); + public abstract NetworkSecurityRuleProperties build(); } } diff --git a/providers/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/NetworkSecurityGroupApiMockTest.java b/providers/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/NetworkSecurityGroupApiMockTest.java index 3dc0e4aa20..4f51954415 100644 --- a/providers/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/NetworkSecurityGroupApiMockTest.java +++ b/providers/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/NetworkSecurityGroupApiMockTest.java @@ -54,6 +54,7 @@ public class NetworkSecurityGroupApiMockTest extends BaseAzureComputeApiMockTest .access(NetworkSecurityRuleProperties.Access.Deny) .priority(4095) .direction(NetworkSecurityRuleProperties.Direction.Outbound) + .provisioningState("Succeeded") .build()); ArrayList ruleList = new ArrayList(); ruleList.add(rule); diff --git a/providers/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/NetworkSecurityRuleApiMockTest.java b/providers/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/NetworkSecurityRuleApiMockTest.java index 1ca4284bb3..bb5f57c60b 100644 --- a/providers/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/NetworkSecurityRuleApiMockTest.java +++ b/providers/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/NetworkSecurityRuleApiMockTest.java @@ -52,6 +52,7 @@ public class NetworkSecurityRuleApiMockTest extends BaseAzureComputeApiMockTest .access(NetworkSecurityRuleProperties.Access.Allow) .priority(4094) .direction(NetworkSecurityRuleProperties.Direction.Inbound) + .provisioningState("Succeeded") .build()); return rule; } diff --git a/providers/azurecompute-arm/src/test/resources/networksecurityrulecreate.json b/providers/azurecompute-arm/src/test/resources/networksecurityrulecreate.json index c09bf088e2..bb714609c1 100644 --- a/providers/azurecompute-arm/src/test/resources/networksecurityrulecreate.json +++ b/providers/azurecompute-arm/src/test/resources/networksecurityrulecreate.json @@ -12,6 +12,7 @@ "destinationAddressPrefix": "*", "access": "Allow", "priority": 4094, - "direction": "Inbound" + "direction": "Inbound", + "provisioningState": "Succeeded" } -} \ No newline at end of file +}