From 9453a0f3c2f6c04a6f67988fa6617a7918e4bbc7 Mon Sep 17 00:00:00 2001 From: Jan Srnicek Date: Fri, 3 Mar 2017 09:21:00 +0100 Subject: HC2VPP-78 - subnet validation fix Validation removed, more descriptive expcetion handling to be added after VPP-649 Change-Id: I6e0a84b2dc3f3c9d3d943874baa508636a1df808 Signed-off-by: Jan Srnicek --- .../ip/subnet/validation/Ipv4SubnetValidator.java | 80 -------------------- .../validation/SubnetValidationException.java | 50 ------------ .../interfaces/ip/v4/Ipv4AddressCustomizer.java | 30 +------- .../subnet/validation/Ipv4SubnetValidatorTest.java | 88 ---------------------- .../ip/v4/Ipv4AddressCustomizerTest.java | 44 +---------- 5 files changed, 4 insertions(+), 288 deletions(-) delete mode 100644 v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/interfaces/ip/subnet/validation/Ipv4SubnetValidator.java delete mode 100644 v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/interfaces/ip/subnet/validation/SubnetValidationException.java delete mode 100644 v3po/v3po2vpp/src/test/java/io/fd/hc2vpp/v3po/interfaces/ip/subnet/validation/Ipv4SubnetValidatorTest.java diff --git a/v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/interfaces/ip/subnet/validation/Ipv4SubnetValidator.java b/v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/interfaces/ip/subnet/validation/Ipv4SubnetValidator.java deleted file mode 100644 index 28cd70440..000000000 --- a/v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/interfaces/ip/subnet/validation/Ipv4SubnetValidator.java +++ /dev/null @@ -1,80 +0,0 @@ -/* - * Copyright (c) 2016 Cisco and/or its affiliates. - * - * Licensed 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 io.fd.hc2vpp.v3po.interfaces.ip.subnet.validation; - -import static com.google.common.base.Preconditions.checkNotNull; - -import com.google.common.base.Function; -import com.google.common.collect.Multimap; -import com.google.common.collect.Multimaps; -import io.fd.hc2vpp.v3po.interfaces.ip.IpWriter; -import java.util.List; -import javax.annotation.Nonnull; -import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.ip.rev140616.interfaces._interface.ipv4.Address; -import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.ip.rev140616.interfaces._interface.ipv4.address.Subnet; -import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.ip.rev140616.interfaces._interface.ipv4.address.subnet.Netmask; -import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.ip.rev140616.interfaces._interface.ipv4.address.subnet.PrefixLength; - -/** - * Validator for detecting if there is an attempt to assign multiple addresses from same subnet - */ -public class Ipv4SubnetValidator implements IpWriter { - - /** - * Checks whether data provided for writing are not in collision with already existing data - */ - public void checkNotAddingToSameSubnet(@Nonnull final List
addresses) - throws SubnetValidationException { - - final Multimap prefixLengthRegister = Multimaps.index(addresses, toPrefixLength()); - final int keySetSize = prefixLengthRegister.keySet().size(); - - if (keySetSize == 0 || keySetSize == addresses.size()) { - //this means that every key is unique(has only one value) or no addresses were prefix-length based ,so there is no conflict - return; - } - - //finds conflicting prefix - final Short conflictingPrefix = prefixLengthRegister.keySet() - .stream() - .filter(a -> prefixLengthRegister.get(a).size() > 1) - .findFirst() - .get(); - - //and reports it with affected addresses - throw SubnetValidationException - .forConflictingData(conflictingPrefix, prefixLengthRegister.get(conflictingPrefix)); - } - - private Function toPrefixLength() { - return (final Address address) -> { - final Subnet subnet = address.getSubnet(); - - if (subnet instanceof PrefixLength) { - return ((PrefixLength) subnet).getPrefixLength(); - } - - if (address.getSubnet() instanceof Netmask) { - return (short) getSubnetMaskLength( - checkNotNull(((Netmask) subnet).getNetmask(), "No netmask defined for %s", subnet) - .getValue()); - } - - throw new IllegalArgumentException("Unsupported subnet : " + subnet); - }; - } -} diff --git a/v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/interfaces/ip/subnet/validation/SubnetValidationException.java b/v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/interfaces/ip/subnet/validation/SubnetValidationException.java deleted file mode 100644 index 001923093..000000000 --- a/v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/interfaces/ip/subnet/validation/SubnetValidationException.java +++ /dev/null @@ -1,50 +0,0 @@ -/* - * Copyright (c) 2016 Cisco and/or its affiliates. - * - * Licensed 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 io.fd.hc2vpp.v3po.interfaces.ip.subnet.validation; - -import static com.google.common.base.Preconditions.checkNotNull; - -import java.util.Collection; -import javax.annotation.Nonnull; -import org.apache.commons.lang3.StringUtils; -import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.ip.rev140616.interfaces._interface.ipv4.Address; - -/** - * Thrown as negative result of subnet validation - */ -public class SubnetValidationException extends Exception { - - private SubnetValidationException(@Nonnull final String message) { - super(message); - } - - public static SubnetValidationException forConflictingData(@Nonnull final Short prefix, @Nonnull Collection
addresses) { - return new SubnetValidationException( - "Attempt to define multiple addresses for same subnet[prefixLen = " + prefixToString(prefix) + "], " - + "addresses : " + addressesToString(addresses)); - } - - private static String prefixToString(final Short prefix) { - return checkNotNull(prefix, "Cannot create " + SubnetValidationException.class.getName() + " for null prefix") - .toString(); - } - - private static String addressesToString(final Collection
addresses) { - return StringUtils.join(checkNotNull(addresses, - "Cannot create " + SubnetValidationException.class.getName() + " for null address list"), " | "); - } -} diff --git a/v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/interfaces/ip/v4/Ipv4AddressCustomizer.java b/v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/interfaces/ip/v4/Ipv4AddressCustomizer.java index 0860436d3..d6cc6a00f 100644 --- a/v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/interfaces/ip/v4/Ipv4AddressCustomizer.java +++ b/v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/interfaces/ip/v4/Ipv4AddressCustomizer.java @@ -23,8 +23,6 @@ import com.google.common.base.Optional; import io.fd.hc2vpp.common.translate.util.FutureJVppCustomizer; import io.fd.hc2vpp.common.translate.util.NamingContext; import io.fd.hc2vpp.v3po.interfaces.ip.IpWriter; -import io.fd.hc2vpp.v3po.interfaces.ip.subnet.validation.Ipv4SubnetValidator; -import io.fd.hc2vpp.v3po.interfaces.ip.subnet.validation.SubnetValidationException; import io.fd.honeycomb.translate.spi.write.ListWriterCustomizer; import io.fd.honeycomb.translate.util.RWUtils; import io.fd.honeycomb.translate.write.WriteContext; @@ -51,19 +49,11 @@ public class Ipv4AddressCustomizer extends FutureJVppCustomizer private static final Logger LOG = LoggerFactory.getLogger(Ipv4AddressCustomizer.class); private final NamingContext interfaceContext; - private final Ipv4SubnetValidator subnetValidator; - - @VisibleForTesting - Ipv4AddressCustomizer(@Nonnull final FutureJVppCore futureJVppCore, @Nonnull final NamingContext interfaceContext, - @Nonnull final Ipv4SubnetValidator subnetValidator) { - super(futureJVppCore); - this.interfaceContext = checkNotNull(interfaceContext, "Interface context cannot be null"); - this.subnetValidator = checkNotNull(subnetValidator, "Subnet validator cannot be null"); - } public Ipv4AddressCustomizer(@Nonnull final FutureJVppCore futureJVppCore, @Nonnull final NamingContext interfaceContext) { - this(futureJVppCore, interfaceContext, new Ipv4SubnetValidator()); + super(futureJVppCore); + this.interfaceContext = checkNotNull(interfaceContext, "Interface context cannot be null"); } @Override @@ -72,21 +62,7 @@ public class Ipv4AddressCustomizer extends FutureJVppCustomizer final String interfaceName = id.firstKeyOf(Interface.class).getName(); final int interfaceIndex = interfaceContext.getIndex(interfaceName, writeContext.getMappingContext()); - - // checks whether address is not from same subnet of some address already defined on this interface - try { - - final InstanceIdentifier parentId = RWUtils.cutId(id, InstanceIdentifier.create(Ipv4.class)); - final Optional ipv4Optional = writeContext.readAfter(parentId); - - //no need to check isPresent() - we are inside of address customizer, therefore there must be Address data - //that is being processed by infrastructure - - subnetValidator.checkNotAddingToSameSubnet(ipv4Optional.get().getAddress()); - } catch (SubnetValidationException e) { - throw new WriteFailedException(id, e); - } - + // TODO - HC2VPP-92 - Add more descriptive exception handling after https://jira.fd.io/browse/VPP-649 setAddress(true, id, interfaceName, interfaceIndex, dataAfter, writeContext); } diff --git a/v3po/v3po2vpp/src/test/java/io/fd/hc2vpp/v3po/interfaces/ip/subnet/validation/Ipv4SubnetValidatorTest.java b/v3po/v3po2vpp/src/test/java/io/fd/hc2vpp/v3po/interfaces/ip/subnet/validation/Ipv4SubnetValidatorTest.java deleted file mode 100644 index faa860f45..000000000 --- a/v3po/v3po2vpp/src/test/java/io/fd/hc2vpp/v3po/interfaces/ip/subnet/validation/Ipv4SubnetValidatorTest.java +++ /dev/null @@ -1,88 +0,0 @@ -/* - * Copyright (c) 2016 Cisco and/or its affiliates. - * - * Licensed 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 io.fd.hc2vpp.v3po.interfaces.ip.subnet.validation; - - -import com.google.common.collect.Lists; -import java.util.List; -import org.junit.Before; -import org.junit.Test; -import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.ip.rev140616.interfaces._interface.ipv4.Address; -import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.ip.rev140616.interfaces._interface.ipv4.AddressBuilder; -import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.ip.rev140616.interfaces._interface.ipv4.address.subnet.NetmaskBuilder; -import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.ip.rev140616.interfaces._interface.ipv4.address.subnet.PrefixLengthBuilder; -import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.DottedQuad; - -public class Ipv4SubnetValidatorTest { - - private Ipv4SubnetValidator subnetValidator; - - @Before - public void init() { - subnetValidator = new Ipv4SubnetValidator(); - } - - @Test(expected = SubnetValidationException.class) - public void testValidateNegativeSameTypes() throws SubnetValidationException { - List
addresses = Lists.newArrayList(); - - addresses.add(new AddressBuilder().setSubnet(new PrefixLengthBuilder().setPrefixLength((short) 24).build()) - .build()); - addresses.add(new AddressBuilder().setSubnet(new PrefixLengthBuilder().setPrefixLength((short) 24).build()) - .build()); - - subnetValidator.checkNotAddingToSameSubnet(addresses); - } - - @Test(expected = SubnetValidationException.class) - public void testValidateNegativeMixedTypes() throws SubnetValidationException { - List
addresses = Lists.newArrayList(); - - addresses.add(new AddressBuilder().setSubnet(new PrefixLengthBuilder().setPrefixLength((short) 24).build()) - .build()); - addresses.add(new AddressBuilder() - .setSubnet(new NetmaskBuilder().setNetmask(new DottedQuad("255.255.255.0")).build()) - .build()); - - subnetValidator.checkNotAddingToSameSubnet(addresses); - } - - @Test - public void testValidatePositiveSameTypes() throws SubnetValidationException { - List
addresses = Lists.newArrayList(); - - addresses.add(new AddressBuilder().setSubnet(new PrefixLengthBuilder().setPrefixLength((short) 24).build()) - .build()); - addresses.add(new AddressBuilder().setSubnet(new PrefixLengthBuilder().setPrefixLength((short) 25).build()) - .build()); - - subnetValidator.checkNotAddingToSameSubnet(addresses); - } - - @Test - public void testValidatePositiveMixedTypes() throws SubnetValidationException { - List
addresses = Lists.newArrayList(); - - addresses.add(new AddressBuilder().setSubnet(new PrefixLengthBuilder().setPrefixLength((short) 24).build()) - .build()); - addresses.add(new AddressBuilder() - .setSubnet(new NetmaskBuilder().setNetmask(new DottedQuad("255.255.0.0")).build()) - .build()); - - subnetValidator.checkNotAddingToSameSubnet(addresses); - } -} diff --git a/v3po/v3po2vpp/src/test/java/io/fd/hc2vpp/v3po/interfaces/ip/v4/Ipv4AddressCustomizerTest.java b/v3po/v3po2vpp/src/test/java/io/fd/hc2vpp/v3po/interfaces/ip/v4/Ipv4AddressCustomizerTest.java index 8f3dafd10..ffae78ea8 100644 --- a/v3po/v3po2vpp/src/test/java/io/fd/hc2vpp/v3po/interfaces/ip/v4/Ipv4AddressCustomizerTest.java +++ b/v3po/v3po2vpp/src/test/java/io/fd/hc2vpp/v3po/interfaces/ip/v4/Ipv4AddressCustomizerTest.java @@ -30,8 +30,6 @@ import static org.mockito.Mockito.when; import com.google.common.base.Optional; import io.fd.hc2vpp.common.test.write.WriterCustomizerTest; import io.fd.hc2vpp.common.translate.util.NamingContext; -import io.fd.hc2vpp.v3po.interfaces.ip.subnet.validation.Ipv4SubnetValidator; -import io.fd.hc2vpp.v3po.interfaces.ip.subnet.validation.SubnetValidationException; import io.fd.honeycomb.translate.write.WriteFailedException; import io.fd.vpp.jvpp.VppBaseCallException; import io.fd.vpp.jvpp.core.dto.IpAddressDetailsReplyDump; @@ -68,9 +66,6 @@ public class Ipv4AddressCustomizerTest extends WriterCustomizerTest { private static final String IFACE_NAME = "eth0"; private static final int IFACE_ID = 123; - @Mock - private Ipv4SubnetValidator subnetValidator; - private NamingContext interfaceContext; private Ipv4AddressCustomizer customizer; @@ -92,11 +87,9 @@ public class Ipv4AddressCustomizerTest extends WriterCustomizerTest { public void setUpTest() throws Exception { interfaceContext = new NamingContext("generatedIfaceName", IFC_CTX_NAME); - customizer = new Ipv4AddressCustomizer(api, interfaceContext, subnetValidator); + customizer = new Ipv4AddressCustomizer(api, interfaceContext); doReturn(future(new IpAddressDetailsReplyDump())).when(api).ipAddressDump(any()); - when(writeContext.readAfter(Mockito.any())) - .thenReturn(Optional.of(new Ipv4Builder().setAddress(Collections.emptyList()).build())); } private void whenSwInterfaceAddDelAddressThenSuccess() { @@ -109,8 +102,6 @@ public class Ipv4AddressCustomizerTest extends WriterCustomizerTest { @Test public void testAddPrefixLengthIpv4Address() throws Exception { - doNothing().when(subnetValidator).checkNotAddingToSameSubnet(Mockito.anyList()); - final InstanceIdentifier
id = getAddressId(IFACE_NAME); when(writeContext.readBefore(id)).thenReturn(Optional.absent()); @@ -151,39 +142,6 @@ public class Ipv4AddressCustomizerTest extends WriterCustomizerTest { fail("WriteFailedException was expected"); } - @Test - public void testAddPrefixLengthIpv4AddressConflicted() throws Exception { - - final InstanceIdentifier
id = getAddressId(IFACE_NAME); - when(writeContext.readBefore(id)).thenReturn(Optional.absent()); - - Ipv4AddressNoZone noZoneIp = new Ipv4AddressNoZone(new Ipv4Address("192.168.2.1")); - PrefixLength length = new PrefixLengthBuilder().setPrefixLength(new Integer(24).shortValue()).build(); - Address data = new AddressBuilder().setIp(noZoneIp).setSubnet(length).build(); - final List
addressList = Arrays.asList(data); - - //throws when validation invoked - Mockito.doThrow(SubnetValidationException.forConflictingData((short) 24, Arrays.asList(data))).when(subnetValidator) - .checkNotAddingToSameSubnet(addressList); - - //fake data return from WriteContext - doReturn(Optional.of(new Ipv4Builder().setAddress(addressList).build())).when(writeContext) - .readAfter(argThat(matchInstanceIdentifier(Ipv4.class))); - - defineMapping(mappingContext, IFACE_NAME, IFACE_ID, IFC_CTX_NAME); - - try { - customizer.writeCurrentAttributes(id, data, writeContext); - } catch (WriteFailedException e) { - //verifies if cause of exception is correct type - assertTrue(e.getCause() instanceof SubnetValidationException); - - //verify that validation call was invoked with data from writeContext - verify(subnetValidator, times(1)).checkNotAddingToSameSubnet(addressList); - } - - } - @Test(expected = WriteFailedException.UpdateFailedException.class) public void testUpdate() throws Exception { final Address data = mock(Address.class); -- cgit 1.2.3-korg