HC2VPP-78 - subnet validation fix 95/5395/5
authorJan Srnicek <[email protected]>
Fri, 3 Mar 2017 08:21:00 +0000 (09:21 +0100)
committerJan Srnicek <[email protected]>
Fri, 3 Mar 2017 08:21:00 +0000 (09:21 +0100)
Validation removed, more descriptive expcetion handling to be added
after VPP-649

Change-Id: I6e0a84b2dc3f3c9d3d943874baa508636a1df808
Signed-off-by: Jan Srnicek <[email protected]>
v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/interfaces/ip/subnet/validation/Ipv4SubnetValidator.java [deleted file]
v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/interfaces/ip/subnet/validation/SubnetValidationException.java [deleted file]
v3po/v3po2vpp/src/main/java/io/fd/hc2vpp/v3po/interfaces/ip/v4/Ipv4AddressCustomizer.java
v3po/v3po2vpp/src/test/java/io/fd/hc2vpp/v3po/interfaces/ip/subnet/validation/Ipv4SubnetValidatorTest.java [deleted file]
v3po/v3po2vpp/src/test/java/io/fd/hc2vpp/v3po/interfaces/ip/v4/Ipv4AddressCustomizerTest.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 (file)
index 28cd704..0000000
+++ /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<Address> addresses)
-            throws SubnetValidationException {
-
-        final Multimap<Short, Address> 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<Address, Short> 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 (file)
index 0019230..0000000
+++ /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<Address> 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<Address> addresses) {
-        return StringUtils.join(checkNotNull(addresses,
-                "Cannot create " + SubnetValidationException.class.getName() + " for null address list"), " | ");
-    }
-}
index 0860436..d6cc6a0 100644 (file)
@@ -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<Ipv4> parentId = RWUtils.cutId(id, InstanceIdentifier.create(Ipv4.class));
-            final Optional<Ipv4> 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 (file)
index faa860f..0000000
+++ /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<Address> 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<Address> 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<Address> 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<Address> 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);
-    }
-}
index 8f3dafd..ffae78e 100644 (file)
@@ -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<Address> 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<Address> 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<Address> 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);