Change Message When ROA Conflicts ROUTE (#1473)

* feat: apply Riccardo suggestion

* feat: clarify messages

whois-api/src/main/java/net/ripe/db/whois/api/rest/search/RpkiRoaMessageGenerator.java

@@ -57,9 +57,9 @@ private RpslMessage validateRoa(final RpslObject rpslObject){
         }
 
         return switch (invalidRpkiRoa.getValue()) {
-            case INVALID_ORIGIN -> new RpslMessage(QueryMessages.roaRouteOriginConflicts(rpslObject.getType().getName(), invalidRpkiRoa.getKey().getMaxLength(), invalidRpkiRoa.getKey().getAsn()));
-            case INVALID_PREFIX_LENGTH -> new RpslMessage(QueryMessages.roaRoutePrefixLengthConflicts(rpslObject.getType().getName(), invalidRpkiRoa.getKey().getMaxLength(), invalidRpkiRoa.getKey().getAsn()));
-            case INVALID_PREFIX_AND_ORIGIN -> new RpslMessage(QueryMessages.roaRouteConflicts(rpslObject.getType().getName(), invalidRpkiRoa.getKey().getMaxLength(), invalidRpkiRoa.getKey().getAsn()));
+            case INVALID_ORIGIN -> new RpslMessage(QueryMessages.roaRouteOriginConflicts(rpslObject.getType().getName().toUpperCase(), invalidRpkiRoa.getKey().getPrefix(), invalidRpkiRoa.getKey().getMaxLength(), invalidRpkiRoa.getKey().getAsn()));
+            case INVALID_PREFIX_LENGTH -> new RpslMessage(QueryMessages.roaRoutePrefixLengthConflicts(rpslObject.getType().getName().toUpperCase(), invalidRpkiRoa.getKey().getPrefix(), invalidRpkiRoa.getKey().getMaxLength(), invalidRpkiRoa.getKey().getAsn()));
+            case INVALID_PREFIX_AND_ORIGIN -> new RpslMessage(QueryMessages.roaRouteConflicts(rpslObject.getType().getName().toUpperCase(), invalidRpkiRoa.getKey().getPrefix(), invalidRpkiRoa.getKey().getMaxLength(), invalidRpkiRoa.getKey().getAsn()));
             default -> null;
         };
     }

whois-api/src/test/java/net/ripe/db/whois/api/rest/WhoisSearchServiceTestIntegration.java

@@ -1860,9 +1860,9 @@ public void search_less_specific_route_existing_roa_validation_enabled_as_json()
         assertThat(whoisResources.getWhoisObjects(), hasSize(1));
 
         assertThat(whoisResources.getWhoisObjects().get(0).getObjectMessages().getMessages().get(0).getText(), is("" +
-                "Warning: this %s object conflicts with an overlapping RPKI ROA with a less specific prefix %s but same origin AS%s.\n" +
-                "As a result an announcement for this prefix may be rejected by many autonomous systems. You should " +
-                "either remove this route: object or update or delete the ROA.\n"));
+                "Warning: this %s object conflicts with an RPKI ROA with a prefix %s and a less specific maximum length /%s but same origin AS%s.\n" +
+                "As a result, many autonomous systems may reject a BGP announcement even if it matches the ROUTE object. " +
+                "You should consider either removing this ROUTE object or updating/deleting the RPKI ROA.\n"));
     }
 
     @Test
@@ -1906,9 +1906,9 @@ public void search_route6_roa_origin_mismatch_validation_enabled_as_json() {
 
         assertThat(whoisResources.getWhoisObjects().get(0).getObjectMessages().getMessages(), hasSize(1));
         assertThat(whoisResources.getWhoisObjects().get(0).getObjectMessages().getMessages().get(0).getText(), is("" +
-                "Warning: this %s object conflicts with an overlapping RPKI ROA with prefix %s but different origin AS%s.\n" +
-                "As a result an announcement for this prefix may be rejected by many autonomous systems. You should " +
-                "either remove this route: object or update or delete the ROA.\n"));
+                "Warning: this %s object conflicts with an RPKI ROA with a prefix %s and a maximum length /%s but a different origin AS%s.\n" +
+                "As a result, many autonomous systems may reject a BGP announcement even if it matches the ROUTE object. " +
+                "You should consider either removing this ROUTE object or updating/deleting the RPKI ROA.\n"));
     }
 
     @Test
@@ -1931,9 +1931,9 @@ public void search_route_roa_origin_mismatch_validation_enabled_as_json() {
 
         assertThat(whoisResources.getWhoisObjects().get(0).getObjectMessages().getMessages(), hasSize(1));
         assertThat(whoisResources.getWhoisObjects().get(0).getObjectMessages().getMessages().get(0).getText(), is("" +
-                "Warning: this %s object conflicts with an overlapping RPKI ROA with prefix %s but different origin AS%s.\n" +
-                "As a result an announcement for this prefix may be rejected by many autonomous systems. You should " +
-                "either remove this route: object or update or delete the ROA.\n"));
+                "Warning: this %s object conflicts with an RPKI ROA with a prefix %s and a maximum length /%s but a different origin AS%s.\n" +
+                "As a result, many autonomous systems may reject a BGP announcement even if it matches the ROUTE object. " +
+                "You should consider either removing this ROUTE object or updating/deleting the RPKI ROA.\n"));
     }
 
     @Test
@@ -1977,9 +1977,9 @@ public void search_route_roa_origin_mismatch_validation_enabled_as_xml() {
 
         assertThat(whoisResources.getWhoisObjects().get(0).getObjectMessages().getMessages(), hasSize(1));
         assertThat(whoisResources.getWhoisObjects().get(0).getObjectMessages().getMessages().get(0).getText(), is("" +
-                "Warning: this %s object conflicts with an overlapping RPKI ROA with prefix %s but different origin AS%s.\n" +
-                "As a result an announcement for this prefix may be rejected by many autonomous systems. You should " +
-                "either remove this route: object or update or delete the ROA.\n"));
+                "Warning: this %s object conflicts with an RPKI ROA with a prefix %s and a maximum length /%s but a different origin AS%s.\n" +
+                "As a result, many autonomous systems may reject a BGP announcement even if it matches the ROUTE object. " +
+                "You should consider either removing this ROUTE object or updating/deleting the RPKI ROA.\n"));
     }
 
     @Test
@@ -2002,9 +2002,9 @@ public void search_route_roa_origin_and_prefix_length_mismatch_validation_enable
 
         assertThat(whoisResources.getWhoisObjects().get(0).getObjectMessages().getMessages(), hasSize(1));
         assertThat(whoisResources.getWhoisObjects().get(0).getObjectMessages().getMessages().get(0).getText(), is("" +
-                "Warning: this %s object conflicts with an overlapping RPKI ROA with a less specific prefix %s and different origin AS%s.\n" +
-                "As a result an announcement for this prefix may be rejected by many autonomous systems. You should " +
-                "either remove this route: object or update or delete the ROA.\n"));
+                "Warning: this %s object conflicts with an RPKI ROA with a prefix %s and a less specific maximum length /%s and a different origin AS%s.\n" +
+                "As a result, many autonomous systems may reject an announcement even if it matches the ROUTE object. " +
+                "You should consider either removing this ROUTE object or updating/deleting the RPKI ROA.\n"));
     }
 
     @Test
@@ -2083,10 +2083,9 @@ public void search_route6_roa_mismatch_less_specific_as_xml_strings() {
                 "        <attribute name=\"source\" value=\"TEST-NONAUTH\"/>\n" +
                 "    </attributes>\n" +
                 "    <objectmessages>\n" +
-                "        <objectmessage severity=\"Warning\" text=\"Warning: this %s object conflicts with an " +
-                "overlapping RPKI ROA with a less specific prefix %s but same origin AS%s.&#xA;As a result an announcement for this prefix may be" +
-                " rejected by many autonomous systems. You should either remove this route: object or update or delete the ROA.&#xA;\">\n" +
-                "            <args value=\"route6\"/>\n" +
+                "        <objectmessage severity=\"Warning\" text=\"Warning: this %s object conflicts with an RPKI ROA with a prefix %s and a less specific maximum length /%s but same origin AS%s.&#xA;As a result, many autonomous systems may reject a BGP announcement even if it matches the ROUTE object. You should consider either removing this ROUTE object or updating/deleting the RPKI ROA.&#xA;\">\n" +
+                "            <args value=\"ROUTE6\"/>\n" +
+                "            <args value=\"2803:8240::/32\"/>\n" +
                 "            <args value=\"32\"/>\n" +
                 "            <args value=\"52511\"/>\n" +
                 "        </objectmessage>\n" +
@@ -2156,9 +2155,9 @@ public void search_route6_roa_origin_mismatch_less_specific_as_xml_strings() {
                 "        <attribute name=\"source\" value=\"TEST-NONAUTH\"/>\n" +
                 "    </attributes>\n" +
                 "    <objectmessages>\n" +
-                "        <objectmessage severity=\"Warning\" text=\"Warning: this %s object conflicts with an " +
-                "overlapping RPKI ROA with a less specific prefix %s and different origin AS%s.&#xA;As a result an announcement for this prefix may be rejected by many autonomous systems. You should either remove this route: object or update or delete the ROA.&#xA;\">\n" +
-                "            <args value=\"route6\"/>\n" +
+                "        <objectmessage severity=\"Warning\" text=\"Warning: this %s object conflicts with an RPKI ROA with a prefix %s and a less specific maximum length /%s and a different origin AS%s.&#xA;As a result, many autonomous systems may reject an announcement even if it matches the ROUTE object. You should consider either removing this ROUTE object or updating/deleting the RPKI ROA.&#xA;\">\n" +
+                "            <args value=\"ROUTE6\"/>\n" +
+                "            <args value=\"2803:8240::/32\"/>\n" +
                 "            <args value=\"32\"/>\n" +
                 "            <args value=\"52511\"/>\n" +
                 "        </objectmessage>\n" +
@@ -2292,10 +2291,9 @@ public void search_route6_roa_mismatch_origin_as_xml_strings() {
                 "        <attribute name=\"source\" value=\"TEST-NONAUTH\"/>\n" +
                 "    </attributes>\n" +
                 "    <objectmessages>\n" +
-                "        <objectmessage severity=\"Warning\" text=\"Warning: this %s object conflicts with an " +
-                "overlapping RPKI ROA with prefix %s but different origin AS%s.&#xA;As a result an announcement for this prefix " +
-                "may be rejected by many autonomous systems. You should either remove this route: object or update or delete the ROA.&#xA;\">\n" +
-                "            <args value=\"route6\"/>\n" +
+                "        <objectmessage severity=\"Warning\" text=\"Warning: this %s object conflicts with an RPKI ROA with a prefix %s and a maximum length /%s but a different origin AS%s.&#xA;As a result, many autonomous systems may reject a BGP announcement even if it matches the ROUTE object. You should consider either removing this ROUTE object or updating/deleting the RPKI ROA.&#xA;\">\n" +
+                "            <args value=\"ROUTE6\"/>\n" +
+                "            <args value=\"2803:8240::/32\"/>\n" +
                 "            <args value=\"32\"/>\n" +
                 "            <args value=\"52511\"/>\n" +
                 "        </objectmessage>\n" +
@@ -2365,10 +2363,9 @@ public void search_route_roa_mismatch_validation_enabled_as_xml_strings() {
                 "        <attribute name=\"source\" value=\"TEST-NONAUTH\"/>\n" +
                 "    </attributes>\n" +
                 "    <objectmessages>\n" +
-                "        <objectmessage severity=\"Warning\" text=\"Warning: this %s object conflicts with an " +
-                "overlapping RPKI ROA with prefix %s but different origin AS%s.&#xA;As a result an announcement for this prefix " +
-                "may be rejected by many autonomous systems. You should either remove this route: object or update or delete the ROA.&#xA;\">\n" +
-                "            <args value=\"route\"/>\n" +
+                "        <objectmessage severity=\"Warning\" text=\"Warning: this %s object conflicts with an RPKI ROA with a prefix %s and a maximum length /%s but a different origin AS%s.&#xA;As a result, many autonomous systems may reject a BGP announcement even if it matches the ROUTE object. You should consider either removing this ROUTE object or updating/deleting the RPKI ROA.&#xA;\">\n" +
+                "            <args value=\"ROUTE\"/>\n" +
+                "            <args value=\"193.4.0.0/16\"/>\n" +
                 "            <args value=\"16\"/>\n" +
                 "            <args value=\"6505\"/>\n" +
                 "        </objectmessage>\n" +

whois-client/src/main/java/net/ripe/db/whois/query/QueryMessages.java

@@ -60,28 +60,28 @@ public static Message unvalidatedAbuseCShown(final CharSequence key, final CharS
                 "\nAbuse-mailbox validation failed. Please refer to %s for further information.", key, value, orgId);
     }
 
-    public static Message roaRouteOriginConflicts(final String objectType, final int prefix, final long asn){
+    public static Message roaRouteOriginConflicts(final String objectType, final String prefix, final int maxLength, final long asn){
         return new QueryMessage(Type.WARNING, ""
-                + "Warning: this %s object conflicts with an overlapping RPKI ROA with prefix %s but different origin AS%s."
+                + "Warning: this %s object conflicts with an RPKI ROA with a prefix %s and a maximum length /%s but a different origin AS%s."
                 + "\n"
-                + "As a result an announcement for this prefix may be rejected by many autonomous systems. You should" +
-                " either remove this route: object or update or delete the ROA.", objectType, prefix, asn);
+                + "As a result, many autonomous systems may reject a BGP announcement even if it matches the ROUTE object. "
+                + "You should consider either removing this ROUTE object or updating/deleting the RPKI ROA.", objectType, prefix, maxLength, asn);
     }
 
-    public static Message roaRoutePrefixLengthConflicts(final String objectType, final int prefix, final long asn){
+    public static Message roaRoutePrefixLengthConflicts(final String objectType, final String prefix, final int prefixLength, final long asn){
         return new QueryMessage(Type.WARNING, ""
-                + "Warning: this %s object conflicts with an overlapping RPKI ROA with a less specific prefix %s but same origin AS%s."
+                + "Warning: this %s object conflicts with an RPKI ROA with a prefix %s and a less specific maximum length /%s but same origin AS%s."
                 + "\n"
-                + "As a result an announcement for this prefix may be rejected by many autonomous systems. You should" +
-                " either remove this route: object or update or delete the ROA.", objectType, prefix, asn);
+                + "As a result, many autonomous systems may reject a BGP announcement even if it matches the ROUTE object. "
+                + "You should consider either removing this ROUTE object or updating/deleting the RPKI ROA.", objectType, prefix, prefixLength, asn);
     }
 
-    public static Message roaRouteConflicts(final String objectType, final int prefix, final long asn){
+    public static Message roaRouteConflicts(final String objectType, final String prefix, final int prefixLength, final long asn){
         return new QueryMessage(Type.WARNING, ""
-                + "Warning: this %s object conflicts with an overlapping RPKI ROA with a less specific prefix %s and different origin AS%s."
+                + "Warning: this %s object conflicts with an RPKI ROA with a prefix %s and a less specific maximum length /%s and a different origin AS%s."
                 + "\n"
-                + "As a result an announcement for this prefix may be rejected by many autonomous systems. You should" +
-                " either remove this route: object or update or delete the ROA.", objectType, prefix, asn);
+                + "As a result, many autonomous systems may reject an announcement even if it matches the ROUTE object. "
+                + "You should consider either removing this ROUTE object or updating/deleting the RPKI ROA.", objectType, prefix, prefixLength, asn);
     }
 
     public static Message unvalidatedAbuseCShown(final CharSequence key, final CharSequence value) {

whois-commons/src/main/java/net/ripe/db/whois/common/rpki/WhoisRoaChecker.java

@@ -8,6 +8,7 @@
 import org.apache.commons.compress.utils.Lists;
 import org.springframework.stereotype.Component;
 
+import javax.annotation.Nullable;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -22,24 +23,22 @@
 
 @Component
 public class WhoisRoaChecker extends AbstractRpkiRoaChecker {
+
     public WhoisRoaChecker(final RpkiService rpkiService) {
         super(rpkiService);
     }
 
+    @Nullable
     public Map.Entry<Roa, ValidationStatus> validateAndGetInvalidRoa(final RpslObject route){
         /* This method prioritize VALID roas over INVALID roas. So in case of overlap the VALID ROA will se used.
          This is a common behaviour related to roas */
         final Map<Roa, ValidationStatus> roasStatus = validateRoas(route);
-        final Optional<Map.Entry<Roa, ValidationStatus>> roaStatusMap = roasStatus
+        return roasStatus
                 .entrySet()
                 .stream()
                 .filter(getValidOrNotFoundRoas()).findFirst()
-                .or(() -> getInvalidRoas(roasStatus).findFirst());
-
-        if (roaStatusMap.isEmpty()){
-            return null;
-        }
-        return roaStatusMap.get();
+                .or(() -> getInvalidRoas(roasStatus).findFirst())
+                .orElse(null);
     }
 
     @Override
@@ -57,7 +56,7 @@ protected ValidationStatus validate(final RpslObject route, final Roa roa, final
         if (invalidStatus.isEmpty()){
             return VALID;
         }
-        return invalidStatus.size() == 1 ? invalidStatus.get(0) : INVALID_PREFIX_AND_ORIGIN;
+        return invalidStatus.size() == 1 ? invalidStatus.getFirst() : INVALID_PREFIX_AND_ORIGIN;
     }
 
     private Predicate<Map.Entry<Roa, ValidationStatus>> getValidOrNotFoundRoas() {

whois-commons/src/test/java/net/ripe/db/whois/common/rpki/WhoisRoaCheckerTest.java

@@ -57,7 +57,8 @@ public void asn0_invalid() {
 
     @Test
     public void more_specific_roa_prefix_lower_length_invalid() {
-        when(rpkiService.findRoas(eq("10.0.0.0/16"))).thenReturn(Sets.newHashSet(new Roa("AS1", 8, "10.0.0.0/8", "ripe")));
+        when(rpkiService.findRoas(eq("10.0.0.0/16"))).thenReturn(Sets.newHashSet(new Roa("AS1", 8, "10.0.0.0/32",
+                "ripe")));
         final Map.Entry<Roa, ValidationStatus> result = whoisRoaChecker.validateAndGetInvalidRoa(RpslObject.parse(
                 "route: 10.0.0.0/16\n" +
                         "origin: AS1"