From: Michael Vorburger Date: Mon, 28 May 2018 16:49:04 +0000 (+0200) Subject: implement dependency checking for security rule's group X-Git-Tag: release/fluorine~20^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=c3156fe40d41ea91cfd4a7b13266dbba7a920ae6;p=neutron.git implement dependency checking for security rule's group JIRA: NEUTRON-158 Change-Id: Ie0454543f7a5707aac6eb1583b3fd3ec55cbdfe1 Signed-off-by: Michael Vorburger --- diff --git a/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/HttpUtils.java b/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/HttpUtils.java index c3b5b6334..5ce5b6358 100644 --- a/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/HttpUtils.java +++ b/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/HttpUtils.java @@ -24,6 +24,9 @@ import org.junit.Assert; public final class HttpUtils { + // copy/pasted from org.opendaylight.neutron.northbound.api.AbstractNeutronNorthbound + public static final int HTTP_MISSING_DEPENDENCY = 442; // see NEUTRON-158 + private HttpUtils() { } static HttpURLConnection httpURLConnectionFactoryGet(URL url) throws IOException { diff --git a/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/NeutronSecurityRuleTests.java b/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/NeutronSecurityRuleTests.java index 7db81c42c..0324bd65a 100644 --- a/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/NeutronSecurityRuleTests.java +++ b/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/NeutronSecurityRuleTests.java @@ -5,11 +5,12 @@ * terms of the Eclipse Public License v1.0 which accompanies this distribution, * and is available at http://www.eclipse.org/legal/epl-v10.html */ - package org.opendaylight.neutron.e2etest; public class NeutronSecurityRuleTests { + private static final String TEST_SECURITY_GROUP_ID = "b60490fe-60a5-40be-af63-1d641381b784"; + private final String base; public NeutronSecurityRuleTests(String base) { @@ -21,14 +22,23 @@ public class NeutronSecurityRuleTests { HttpUtils.test_fetch(url, "Security Rule Collection GET failed"); } - private String singleton_sr_create_test() { + private void singleton_sr_without_groupid_create_test(int responseCode) { + String url = base + "/security-group-rules"; + String content = " {\"security_group_rule\": " + "{\"remote_group_id\": null, \"direction\": \"ingress\", " + + "\"remote_ip_prefix\": null, \"protocol\": \"tcp\", " + "\"ethertype\": \"IPv6\", \"tenant_id\": " + + "\"00f340c7c3b34ab7be1fc690c05a0275\", \"port_range_max\": 77, " + "\"port_range_min\": 77, " + + "\"id\": \"9b4be7fa-e56e-40fb-9516-1f0fa9185669\"}}"; // no security_group_id + HttpUtils.test_create(url, responseCode, content, "Security Rule Singleton Post Failed"); + } + + private String singleton_sr_create_test(int responseCode) { String url = base + "/security-group-rules"; String content = " {\"security_group_rule\": " + "{\"remote_group_id\": null, \"direction\": \"ingress\", " + "\"remote_ip_prefix\": null, \"protocol\": \"tcp\", " + "\"ethertype\": \"IPv6\", \"tenant_id\": " + "\"00f340c7c3b34ab7be1fc690c05a0275\", \"port_range_max\": 77, " + "\"port_range_min\": 77, " + "\"id\": \"9b4be7fa-e56e-40fb-9516-1f0fa9185669\", " + "\"security_group_id\": " - + "\"b60490fe-60a5-40be-af63-1d641381b784\"}}"; - HttpUtils.test_create(url, content, "Security Rule Singleton Post Failed"); + + "\"" + TEST_SECURITY_GROUP_ID + "\"}}"; + HttpUtils.test_create(url, responseCode, content, "Security Rule Singleton Post Failed"); return content; } @@ -41,21 +51,21 @@ public class NeutronSecurityRuleTests { String url = base + "/security-group-rules"; String content = " {\"security_group_rules\": [" + "{" + " \"id\": \"35fb0f34-c8d3-416d-a205-a2c75f7b8e22\"," + " \"direction\": \"egress\"," + " \"ethertype\": \"IPv6\"," + " \"protocol\": \"tcp\"," - + " \"security_group_id\": \"70f1b157-e79b-44dc-85a8-7de0fc9f2aab\"," + + " \"security_group_id\": \"b60490fe-60a5-40be-af63-1d641381b784\"," + " \"tenant_id\": \"2640ee2ac2474bf3906e482047204fcb\"" + "}," + "{" + " \"id\": \"63814eed-bc12-4fe4-8b17-2af178224c71\"," + " \"direction\": \"egress\"," + " \"ethertype\": \"IPv4\"," + " \"protocol\": \"6\"," - + " \"security_group_id\": \"70f1b157-e79b-44dc-85a8-7de0fc9f2aab\"," + + " \"security_group_id\": \"b60490fe-60a5-40be-af63-1d641381b784\"," + " \"tenant_id\": \"2640ee2ac2474bf3906e482047204fcb\"" + "}," + "{" + " \"id\": \"ccb9823e-559b-4d84-b656-2739f8e56d89\"," + " \"direction\": \"ingress\"," + " \"ethertype\": \"IPv6\"," + " \"protocol\": 6," - + " \"remote_group_id\": \"70f1b157-e79b-44dc-85a8-7de0fc9f2aab\"," - + " \"security_group_id\": \"70f1b157-e79b-44dc-85a8-7de0fc9f2aab\"," + + " \"remote_group_id\": \"b60490fe-60a5-40be-af63-1d641381b784\"," + + " \"security_group_id\": \"b60490fe-60a5-40be-af63-1d641381b784\"," + " \"tenant_id\": \"2640ee2ac2474bf3906e482047204fcb\"" + "}," + "{" + " \"id\": \"fbc3f809-7378-40a4-822f-7a70f6ccba98\"," + " \"direction\": \"ingress\"," + " \"ethertype\": \"IPv4\"," + " \"protocol\": \"udp\"," - + " \"remote_group_id\": \"70f1b157-e79b-44dc-85a8-7de0fc9f2aab\"," - + " \"security_group_id\": \"70f1b157-e79b-44dc-85a8-7de0fc9f2aab\"," + + " \"remote_group_id\": \"b60490fe-60a5-40be-af63-1d641381b784\"," + + " \"security_group_id\": \"b60490fe-60a5-40be-af63-1d641381b784\"," + " \"tenant_id\": \"2640ee2ac2474bf3906e482047204fcb\"" + "}" + "]}"; HttpUtils.test_create(url, content, "Security Rule Multiple Post Failed"); } @@ -66,7 +76,7 @@ public class NeutronSecurityRuleTests { + "\"remote_ip_prefix\": null, \"protocol\": \"tcp\", " + "\"ethertype\": \"IPv6\", \"tenant_id\": " + "\"00f340c7c3b34ab7be1fc690c05a0275\", \"port_range_max\": 77, " + "\"port_range_min\": 77, " + "\"id\": \"9b4be7fa-e56e-40fb-9516-1f0fa9185669\", " + "\"security_group_id\": " - + "\"b60490fe-60a5-40be-af63-1d641381b784\"}}"; + + "\"" + TEST_SECURITY_GROUP_ID + "\"}}"; HttpUtils.test_modify(url, content, "Security Rule Singleton Put Failed"); } @@ -106,7 +116,7 @@ public class NeutronSecurityRuleTests { + "\"ethertype\": \"IPv4\", \"tenant_id\": " + "\"00f340c7c3b34ab7be1fc690c05a0275\", \"port_range_max\": 77, " + "\"port_range_min\": 77, " + "\"id\": \"01234567-0123-0123-0123-01234567890a\", " + "\"security_group_id\": " - + "\"b60490fe-60a5-40be-af63-1d641381b784\"}}"; + + "\"" + TEST_SECURITY_GROUP_ID + "\"}}"; HttpUtils.test_create(url, content, "Security Rule bug4043 IPv4 Failed"); url = url + "/01234567-0123-0123-0123-01234567890a"; @@ -140,7 +150,10 @@ public class NeutronSecurityRuleTests { public static void runTests(String base) { NeutronSecurityRuleTests securityRuleTester = new NeutronSecurityRuleTests(base); - String createJsonString = securityRuleTester.singleton_sr_create_test(); + securityRuleTester.singleton_sr_without_groupid_create_test(500); + securityRuleTester.singleton_sr_create_test(HttpUtils.HTTP_MISSING_DEPENDENCY); + new NeutronSecurityGroupTests(base).singleton_sg_create(TEST_SECURITY_GROUP_ID); + String createJsonString = securityRuleTester.singleton_sr_create_test(201); securityRuleTester.singleton_sr_get_with_one_query_item_test(createJsonString); securityRuleTester.multiple_sr_create_test(); securityRuleTester.singleton_sr_modify_test(); diff --git a/neutron-spi/src/main/java/org/opendaylight/neutron/spi/INeutronCRUD.java b/neutron-spi/src/main/java/org/opendaylight/neutron/spi/INeutronCRUD.java index e41642925..db67d9794 100644 --- a/neutron-spi/src/main/java/org/opendaylight/neutron/spi/INeutronCRUD.java +++ b/neutron-spi/src/main/java/org/opendaylight/neutron/spi/INeutronCRUD.java @@ -75,6 +75,6 @@ public interface INeutronCRUD> { boolean update(String uuid, T delta); // TODO The Exception Result should eventually be replaced by propagating exceptions, and removed - enum Result { Success, AlreadyExists, Exception } + enum Result { Success, AlreadyExists, DependencyMissing, Exception } } diff --git a/northbound-api/src/main/java/org/opendaylight/neutron/northbound/api/AbstractNeutronNorthbound.java b/northbound-api/src/main/java/org/opendaylight/neutron/northbound/api/AbstractNeutronNorthbound.java index 8d9c67edb..506bbadd9 100644 --- a/northbound-api/src/main/java/org/opendaylight/neutron/northbound/api/AbstractNeutronNorthbound.java +++ b/northbound-api/src/main/java/org/opendaylight/neutron/northbound/api/AbstractNeutronNorthbound.java @@ -8,6 +8,8 @@ package org.opendaylight.neutron.northbound.api; +import static org.opendaylight.neutron.spi.INeutronCRUD.Result.DependencyMissing; + import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.ParameterizedType; @@ -28,6 +30,7 @@ public abstract class AbstractNeutronNorthbound, R e protected static final int HTTP_OK_BOTTOM = 200; protected static final int HTTP_OK_TOP = 299; + private static final int HTTP_MISSING_DEPENDENCY = 442; // see NEUTRON-158 (also in neutron.e2etest.HttpUtils) private static final String INTERFACE_NAME_BASE = " CRUD Interface"; private static final String UUID_NO_EXIST_BASE = " UUID does not exist."; @@ -98,14 +101,18 @@ public abstract class AbstractNeutronNorthbound, R e T singleton = input.getSingleton(); singleton.initDefaults(); - neutronCRUD.add(singleton); + if (neutronCRUD.add(singleton).equals(DependencyMissing)) { + return Response.status(HTTP_MISSING_DEPENDENCY).entity(input).build(); + } } else { if (input.getBulk() == null) { throw new BadRequestException("Invalid requests"); } for (T test : input.getBulk()) { test.initDefaults(); - neutronCRUD.add(test); + if (neutronCRUD.add(test).equals(DependencyMissing)) { + return Response.status(HTTP_MISSING_DEPENDENCY).entity(input).build(); + } } } return Response.status(HttpURLConnection.HTTP_CREATED).entity(input).build(); diff --git a/transcriber/src/main/java/org/opendaylight/neutron/transcriber/AbstractTranscriberInterface.java b/transcriber/src/main/java/org/opendaylight/neutron/transcriber/AbstractTranscriberInterface.java index 12aa6f6af..4c289dd14 100644 --- a/transcriber/src/main/java/org/opendaylight/neutron/transcriber/AbstractTranscriberInterface.java +++ b/transcriber/src/main/java/org/opendaylight/neutron/transcriber/AbstractTranscriberInterface.java @@ -20,7 +20,10 @@ import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.concurrent.ExecutionException; +import java.util.function.Function; import javax.annotation.PreDestroy; +import org.eclipse.jdt.annotation.NonNull; +import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction; import org.opendaylight.controller.md.sal.binding.api.ReadTransaction; @@ -34,6 +37,7 @@ import org.opendaylight.neutron.spi.INeutronAdminAttributes; import org.opendaylight.neutron.spi.INeutronBaseAttributes; import org.opendaylight.neutron.spi.INeutronCRUD; import org.opendaylight.neutron.spi.INeutronObject; +import org.opendaylight.neutron.spi.NeutronObject; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.Uuid; import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.attrs.rev150712.AdminAttributes; import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.attrs.rev150712.BaseAttributes; @@ -402,7 +406,7 @@ public abstract class AbstractTranscriberInterface< } protected static Uuid toUuid(String uuid) { - Preconditions.checkNotNull(uuid); + Preconditions.checkNotNull(uuid, "uuid"); Uuid result; try { result = new Uuid(uuid); @@ -491,7 +495,11 @@ public abstract class AbstractTranscriberInterface< while (retries-- >= 0) { final ReadWriteTransaction tx = getDataBroker().newReadWriteTransaction(); try { - return add(input, tx); + if (areAllDependenciesAvailable(tx, input)) { + return add(input, tx); + } else { + return Result.DependencyMissing; + } } catch (InterruptedException | ExecutionException e) { // TODO replace all this with org.opendaylight.genius.infra.RetryingManagedNewTransactionRunner if (e.getCause() instanceof OptimisticLockFailedException) { @@ -567,4 +575,45 @@ public abstract class AbstractTranscriberInterface< } return false; } + + /** + * Check if this particular (subclass) transcriber's dependencies are met. + * Default implementation just returns true. Some but not all transcribers will customize this. + * + *

Implementations *MUST* use the passed in transaction. They will typically call the + * {@link #exists(String, ReadTransaction)} method on ANOTHER transcriber with it. + * + *

Implementations should chain {@link #ifNonNull(Object, Function)}, or perform null safe comparisons otherwise, + * for any optional non-mandatory {@link NeutronObject} properties which may well be null. + * + * @param tx the transaction within which to perform reads to check for dependencies + * @param neutronObject the incoming main neutron object in which there may be references to dependencies + * + * @return true if all dependencies are available and + * {@link #add(INeutronObject)} operation can proceed; false if there + * are unmet dependencies, which will cause the add to abort, and a respective + * error code returned to the caller. + */ + protected boolean areAllDependenciesAvailable(ReadTransaction tx, S neutronObject) { + return true; + } + + /** + * Utility to perform well readable code of null-safe chains of e.g. + * {@link #exists(String, ReadTransaction)} method calls. + */ + protected static final boolean ifNonNull( + @Nullable X property, Function<@NonNull X, @NonNull Boolean> function) { + if (property != null) { + Boolean result = function.apply(property); + Preconditions.checkNotNull(result, "result"); + return result; + } else { + // We return true, in line with the default implementation + // in org.opendaylight.neutron.transcriber.AbstractTranscriberInterface. + // areAllDependenciesAvailable(ReadTransaction, S) + return true; + } + } + } diff --git a/transcriber/src/main/java/org/opendaylight/neutron/transcriber/NeutronSecurityRuleInterface.java b/transcriber/src/main/java/org/opendaylight/neutron/transcriber/NeutronSecurityRuleInterface.java index a6d16b958..653e76ffe 100644 --- a/transcriber/src/main/java/org/opendaylight/neutron/transcriber/NeutronSecurityRuleInterface.java +++ b/transcriber/src/main/java/org/opendaylight/neutron/transcriber/NeutronSecurityRuleInterface.java @@ -13,6 +13,7 @@ import java.util.List; import javax.inject.Inject; import javax.inject.Singleton; import org.opendaylight.controller.md.sal.binding.api.DataBroker; +import org.opendaylight.controller.md.sal.binding.api.ReadTransaction; import org.opendaylight.neutron.northbound.api.BadRequestException; import org.opendaylight.neutron.spi.INeutronSecurityRuleCRUD; import org.opendaylight.neutron.spi.NeutronSecurityRule; @@ -39,9 +40,12 @@ public final class NeutronSecurityRuleInterface extends String> ETHERTYPE_MAP = new ImmutableBiMap.Builder, String>() .put(EthertypeV4.class, "IPv4").put(EthertypeV6.class, "IPv6").build(); + private final NeutronSecurityGroupInterface securityGroupInterface; + @Inject - public NeutronSecurityRuleInterface(DataBroker db) { + public NeutronSecurityRuleInterface(DataBroker db, NeutronSecurityGroupInterface securityGroupInterface) { super(SecurityRuleBuilder.class, db); + this.securityGroupInterface = securityGroupInterface; } @Override @@ -129,4 +133,11 @@ public final class NeutronSecurityRuleInterface extends } return securityRuleBuilder.build(); } + + @Override + protected boolean areAllDependenciesAvailable(ReadTransaction tx, NeutronSecurityRule securityRule) { + return securityGroupInterface.exists(securityRule.getSecurityRuleGroupID(), tx) + && ifNonNull(securityRule.getSecurityRemoteGroupID(), + remoteGroupID -> securityGroupInterface.exists(remoteGroupID, tx)); + } }