RIPE NCC has merged da3dda60e

* Setup correct java version in github actions build [af515edf3]
* Add more tests for prefix merge [159dad3d8]
* Apply 1 suggestion(s) to 1 file(s) [e07999561]
* Add tests for updatedAt field for ROA prefix and actually update the fields [da4244588]
* Comments, prettify [ca453a1bb]
* Cleanup [99d725cb6]
* More tests and fixes [ca1641e69]
* Another fix for list-prefixes speed, cleanup [7a91977f7]
* Fix extremely slow list-prefixes [454a8b971]
* Use immutable logic [b42523495]
* Revert to natural primary key, use native SQL, refactor [d2fc9982a]
* Fix ROA prefix deletion [d6299c724]
* Refactor ROA prefixes and fix tests. [ac2d37d35]
* Try to fix detached entity exception [5f0a9408e]
* Fix tests involving ROA prefixes [b63e5813d]
* WIP [273762897]
* Use composite primary keys for RoaConfigurationPrefix [61323b9a6]
* model the primary key as a int [573c53a22]
* Fix and test the comparator [d8f2bd9ee]
* Use documentation ASNs and prefixes in roa prefix tests [b3cdb7103]
* equals/hashCode were not considering updatedAt [16376aae8]
* Make RoaConfiguration retain updatedAt and prevent duplicates [41e5ccf24]
* Also ensure time is accounted for [77a66cf30]
* Add failing test case: Most specific ROA should be preserved on add [2191404f3]
* updatedAt is read only [acfdd88a0]
* Add setter for updatedAt [859672a18]
* Insert into updatedAt [e1e63d94d]
* Add stream utility to filter by unique attributes [6fd264596]
* Refactor for sonarqube [131ba9d20]
* Cleanup imports [05921fe64]
* Use a thread pool of fixed size [07792b642]
* Revert ignored test [98b4d1a71]
* Cleanup [bcf628a11]
* Renaming [f8a7cd934]
* Revert disabled security [6851ad6db]
* Fix date JSON serialisation [d384c74dd]
* Ignore breaking test [ceb2fec2f]
* Compilation fixes [a49d64757]
* Add lastUpdate timestamp [4fddb3a55]
* Comment out security again [e3b488303]
* Revert security [9974c3e12]
* Remove security again [e8e1f0625]
* Cleanup [c2f9a024d]
* Revert disabled security [e88bd86f8]
* Fix DTO for Krill API [1ee65a5b0]
* Fix non-existent publishers [02cf721a7]
* Update dependency org.wiremock:wiremock-jetty12 to v3.9.0 [352ecbfdf]
* Update actions/checkout action to v4 [96f3f3063]

.github/workflows/unit-tests.yml

@@ -6,7 +6,6 @@ jobs:
   build-test:
     name: Build & Test
     runs-on: ubuntu-latest
-    # runs-on: gradle:7.5.1-jdk8
     services:
       postgres:
         image: postgres:15
@@ -19,8 +18,15 @@ jobs:
         options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 5
 
     steps:
-      - uses: actions/checkout@v3
-      - run: ./gradlew test
+      - uses: actions/checkout@v4
+      - name: setup java
+        uses: actions/setup-java@4
+        with:
+          java-version: 17
+          distribution: temurin
+          cache: gradle
+      - name: Run tests
+        run: ./gradlew test
       - name: Test Report
         uses: dorny/test-reporter@v1
         if: success() || failure()    # run this step even if previous step failed

src/main/java/net/ripe/rpki/domain/roa/RoaConfiguration.java

@@ -1,58 +1,60 @@
 package net.ripe.rpki.domain.roa;
 
 import com.google.common.base.Preconditions;
+import jakarta.persistence.*;
 import lombok.Getter;
 import lombok.extern.slf4j.Slf4j;
 import net.ripe.ipresource.Asn;
 import net.ripe.ipresource.ImmutableResourceSet;
+import net.ripe.ipresource.IpRange;
 import net.ripe.rpki.commons.crypto.ValidityPeriod;
-import net.ripe.rpki.commons.validation.roa.AnnouncedRoute;
 import net.ripe.rpki.domain.ManagedCertificateAuthority;
 import net.ripe.rpki.domain.IncomingResourceCertificate;
 import net.ripe.rpki.ncc.core.domain.support.EntitySupport;
 import net.ripe.rpki.server.api.dto.RoaConfigurationData;
-
-import jakarta.persistence.CollectionTable;
-import jakarta.persistence.ElementCollection;
-import jakarta.persistence.Entity;
-import jakarta.persistence.FetchType;
-import jakarta.persistence.GeneratedValue;
-import jakarta.persistence.GenerationType;
-import jakarta.persistence.Id;
-import jakarta.persistence.JoinColumn;
-import jakarta.persistence.OneToOne;
-import jakarta.persistence.SequenceGenerator;
-import jakarta.persistence.Table;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
-import java.util.TreeMap;
+import net.ripe.rpki.util.Streams;
+import org.apache.commons.lang3.tuple.Pair;
+import org.apache.commons.lang3.tuple.Triple;
+import org.hibernate.annotations.DynamicInsert;
+import org.hibernate.annotations.DynamicUpdate;
+
+import java.util.*;
+import java.util.function.Function;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static java.util.Comparator.*;
 
 /**
  * Specification for a ROA. This specification determines how ROAs must be
  * created and managed. The ROA specification can be edited by the user. The
  * system will then take care of managing the required ROAs.
  */
+@DynamicInsert
+@DynamicUpdate
 @Slf4j
 @Entity
 @Table(name = "roaconfiguration")
 @SequenceGenerator(name = "seq_roaconfiguration", sequenceName = "seq_all", allocationSize = 1)
 public class RoaConfiguration extends EntitySupport {
 
+    public static final Comparator<RoaConfigurationPrefix> ROA_CONFIGURATION_PREFIX_COMPARATOR =
+            comparing(RoaConfigurationPrefix::getAsn)
+                    .thenComparing(RoaConfigurationPrefix::getPrefix)
+                    .thenComparing(RoaConfigurationPrefix::getMaximumLength, reverseOrder())
+                    .thenComparing(RoaConfigurationPrefix::getUpdatedAt, nullsLast(naturalOrder()));
+
     @Id
     @GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "seq_roaconfiguration")
     @Getter
     private Long id;
 
     @OneToOne(optional = false)
     @JoinColumn(name = "certificateauthority_id")
+    @Getter
     private ManagedCertificateAuthority certificateAuthority;
 
-    @ElementCollection(fetch = FetchType.EAGER)
-    @CollectionTable(name = "roaconfiguration_prefixes", joinColumns = @JoinColumn(name = "roaconfiguration_id"))
+    @OneToMany(fetch = FetchType.EAGER, mappedBy = "roaConfiguration", cascade = CascadeType.MERGE)
     private Set<RoaConfigurationPrefix> prefixes = new HashSet<>();
 
     public RoaConfiguration() {
@@ -67,31 +69,25 @@ public RoaConfiguration(ManagedCertificateAuthority certificateAuthority, Collec
         this.prefixes.addAll(prefixes);
     }
 
-    public void setPrefixes(Collection<? extends RoaConfigurationPrefix> prefixes) {
-        this.prefixes = convertToSet(convertToMap(prefixes));
+    public void setPrefixes(Collection<RoaConfigurationPrefix> prefixes) {
+        this.prefixes = canonicalRoaPrefixes(prefixes.stream());
     }
 
     public Set<RoaConfigurationPrefix> getPrefixes() {
         return Collections.unmodifiableSet(prefixes);
     }
 
-    public ManagedCertificateAuthority getCertificateAuthority() {
-        return certificateAuthority;
-    }
-
     public RoaConfigurationData convertToData() {
         return new RoaConfigurationData(prefixes.stream()
                 .map(RoaConfigurationPrefix::toData).toList());
     }
 
-    public final void addPrefix(Collection<? extends RoaConfigurationPrefix> roaPrefixes) {
-        Map<AnnouncedRoute, Integer> byPrefix = convertToMap(prefixes);
-        byPrefix.putAll(convertToMap(roaPrefixes));
-        prefixes = convertToSet(byPrefix);
+    public final PrefixDiff addPrefixes(Collection<RoaConfigurationPrefix> roaPrefixes) {
+        return mergePrefixes(roaPrefixes, Collections.emptyList());
     }
 
-    public final void removePrefix(Collection<? extends RoaConfigurationPrefix> roaPrefixes) {
-        prefixes.removeAll(roaPrefixes);
+    public final PrefixDiff removePrefixes(Collection<RoaConfigurationPrefix> roaPrefixes) {
+        return mergePrefixes(Collections.emptyList(), roaPrefixes);
     }
 
     Map<Asn, RoaSpecification> toRoaSpecifications(IncomingResourceCertificate currentIncomingCertificate) {
@@ -114,14 +110,66 @@ Map<Asn, RoaSpecification> toRoaSpecifications(IncomingResourceCertificate curre
         return result;
     }
 
-    private static Set<RoaConfigurationPrefix> convertToSet(Map<AnnouncedRoute, Integer> byPrefix) {
-        return byPrefix.entrySet().stream().map(prefix -> new RoaConfigurationPrefix(prefix.getKey(), prefix.getValue())).collect(Collectors.toSet());
+    private static Pair<Asn, IpRange> prefixKey(RoaConfigurationPrefix r) {
+        return Pair.of(r.getAsn(), r.getPrefix());
+    }
+
+    private static Triple<Asn, IpRange, Integer> prefixMinimalIdentity(RoaConfigurationPrefix r) {
+        return Triple.of(r.getAsn(), r.getPrefix(), r.getMaximumLength());
+    }
+
+    private static boolean differentPrefixes(RoaConfigurationPrefix r1, RoaConfigurationPrefix r2) {
+        return r1 == null ? r2 != null : r2 == null || !prefixMinimalIdentity(r1).equals(prefixMinimalIdentity(r2));
     }
 
-    private static Map<AnnouncedRoute, Integer> convertToMap(Collection<? extends RoaConfigurationPrefix> prefixes) {
-        return prefixes.stream().collect(Collectors.toMap(
-                prefix -> new AnnouncedRoute(prefix.getAsn(), prefix.getPrefix()),
-                RoaConfigurationPrefix::getMaximumLength,
-                (a, b) -> b));
+    /**
+     * Sort the ROA prefixes and keep the first unique one by earliest updatedAt
+     */
+    private Set<RoaConfigurationPrefix> canonicalRoaPrefixes(Stream<RoaConfigurationPrefix> input) {
+        var prefixList = input.sorted(ROA_CONFIGURATION_PREFIX_COMPARATOR)
+                .filter(Streams.distinctByKey(RoaConfiguration::prefixKey))
+                .toList();
+
+        return new HashSet<>(prefixList);
     }
+
+    /**
+     * Apply added and deleted prefixes, and return the ones that were really
+     * added or really deleted based on the current set of prefixes and certain
+     * heuristics such as max_length and update_at fields.
+     */
+    public PrefixDiff mergePrefixes(
+            Collection<RoaConfigurationPrefix> prefixesToAdd,
+            Collection<RoaConfigurationPrefix> prefixesToRemove) {
+
+        var toRemove = prefixesToRemove.stream()
+                .map(RoaConfiguration::prefixMinimalIdentity)
+                .collect(Collectors.toSet());
+
+        var newPrefixes = Stream.concat(prefixes.stream(), prefixesToAdd.stream())
+                .filter(r -> !toRemove.contains(prefixMinimalIdentity(r)))
+                .collect(Collectors.toMap(
+                        RoaConfiguration::prefixKey,
+                        Function.identity(),
+                        // Prefer prefix based on the maxLength + updatedAt heuristics
+                        // defined by the comparator
+                        (r1, r2) -> ROA_CONFIGURATION_PREFIX_COMPARATOR.compare(r1, r2) < 0 ? r1 : r2));
+
+        var existing = prefixes.stream()
+                .collect(Collectors.toMap(RoaConfiguration::prefixKey, Function.identity()));
+
+        var removed = prefixes.stream()
+                .filter(r -> differentPrefixes(r, newPrefixes.get(prefixKey(r))))
+                .toList();
+
+        var added = newPrefixes.values().stream()
+                .filter(r -> differentPrefixes(r, existing.get(prefixKey(r))))
+                .toList();
+
+        added.forEach(r -> r.setRoaConfiguration(this));
+        prefixes = new HashSet<>(newPrefixes.values());
+        return new PrefixDiff(added, removed);
+    }
+
+    public record PrefixDiff(List<RoaConfigurationPrefix> added, List<RoaConfigurationPrefix> removed) {}
 }

src/main/java/net/ripe/rpki/domain/roa/RoaConfigurationMaintenanceServiceBean.java

@@ -65,12 +65,9 @@ private void updateRoaConfigurationsForResources(ManagedCertificateAuthority ca,
                 .collect(Collectors.toSet());
 
         if (!toBeRemoved.isEmpty()) {
-            // Update the config, log the removed prefixes [...]
-            config.removePrefix(toBeRemoved);
+            roaConfigurationRepository.removePrefixes(config, toBeRemoved);
             ca.markConfigurationUpdated();
 
-            roaConfigurationRepository.logRoaPrefixDeletion(config, toBeRemoved);
-
             context.recordEvent(
                     new RoaConfigurationUpdatedDueToChangedResourcesEvent(
                             ca.getVersionedId(),

src/main/java/net/ripe/rpki/domain/roa/RoaConfigurationPrefix.java

@@ -1,49 +1,81 @@
 package net.ripe.rpki.domain.roa;
 
-import lombok.Getter;
+import jakarta.persistence.*;
+import lombok.*;
 import net.ripe.ipresource.Asn;
 import net.ripe.ipresource.IpRange;
 import net.ripe.ipresource.IpResourceType;
 import net.ripe.rpki.commons.validation.roa.AnnouncedRoute;
+import net.ripe.rpki.ripencc.support.persistence.AsnPersistenceConverter;
 import net.ripe.rpki.server.api.dto.RoaConfigurationPrefixData;
 import org.apache.commons.lang.builder.ToStringBuilder;
 import org.apache.commons.lang.builder.ToStringStyle;
 
-import jakarta.persistence.Column;
-import jakarta.persistence.Embeddable;
+import java.io.Serializable;
 import java.math.BigInteger;
 import java.time.Instant;
 import java.util.List;
 
-import static com.google.common.base.Objects.*;
 
-@Embeddable
+@EqualsAndHashCode
+@Entity
+@Table(name = "roaconfiguration_prefixes")
+@IdClass(RoaConfigurationPrefix.RoaConfigurationPrefixIdClass.class)
 public class RoaConfigurationPrefix {
+
+    @NoArgsConstructor
+    @Data
+    public static class RoaConfigurationPrefixIdClass implements Serializable {
+        // via https://stackoverflow.com/a/61258208
+        @Column(name = "asn", nullable = false)
+        @Convert(converter = AsnPersistenceConverter.class)
+        private Asn asn;
+        private BigInteger prefixStart;
+        private BigInteger prefixEnd;
+        @Column(name = "prefix_type_id", nullable = false)
+        private IpResourceType prefixType;
+    }
+
+    @Id
     @Column(name = "asn", nullable = false)
     @Getter
     private Asn asn;
 
+    @Id
     @Column(name = "prefix_start", nullable = false)
     private BigInteger prefixStart;
 
+    @Id
     @Column(name = "prefix_end", nullable = false)
     private BigInteger prefixEnd;
 
+    @Id
     @Column(name = "prefix_type_id", nullable = false)
     private IpResourceType prefixType;
 
-    // Nullable for database compatibility reasons.
-    @Column(name = "maximum_length", nullable = true)
+    @Column(name = "maximum_length")
     private Integer maximumLength;
 
+    @ManyToOne
+    @JoinColumn(name = "roaconfiguration_id", nullable = false)
+    @Setter
+    private RoaConfiguration roaConfiguration;
+
     @Getter
-    @Column(name = "updated_at", insertable = false, updatable = false)
+    @Setter
+    @Column(name = "updated_at")
     private Instant updatedAt;
 
     protected RoaConfigurationPrefix() {
         // JPA uses this
     }
 
+    @PreUpdate
+    @PrePersist
+    void prePersist() {
+        this.updatedAt = Instant.now();
+    }
+
     public RoaConfigurationPrefix(Asn asn, IpRange prefix) {
         this(asn, prefix, null);
     }
@@ -52,12 +84,17 @@ public RoaConfigurationPrefix(Asn asn, IpRange prefix, Integer maximumLength) {
         this(new RoaConfigurationPrefixData(asn, prefix, maximumLength));
     }
 
+    public RoaConfigurationPrefix(Asn asn, IpRange prefix, Integer maximumLength, Instant updatedAt) {
+        this(new RoaConfigurationPrefixData(asn, prefix, maximumLength, updatedAt));
+    }
+
     public RoaConfigurationPrefix(RoaConfigurationPrefixData data) {
         this.asn = data.getAsn();
         this.prefixType = data.getPrefix().getType();
         this.prefixStart = data.getPrefix().getStart().getValue();
         this.prefixEnd = data.getPrefix().getEnd().getValue();
         this.maximumLength = data.getMaximumLength();
+        this.updatedAt = data.getUpdatedAt();
     }
 
     public RoaConfigurationPrefix(AnnouncedRoute route, Integer maximumLength) {
@@ -76,36 +113,13 @@ public RoaConfigurationPrefixData toData() {
         return new RoaConfigurationPrefixData(getAsn(), getPrefix(), getMaximumLength(), getUpdatedAt());
     }
 
-    @Override
-    public int hashCode() {
-        final int prime = 31;
-        int result = 1;
-        result = prime * result + getAsn().hashCode();
-        result = prime * result + getPrefix().hashCode();
-        result = prime * result + getMaximumLength();
-        return result;
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-        if (this == obj) {
-            return true;
-        }
-        if (obj == null || getClass() != obj.getClass()) {
-            return false;
-        }
-        RoaConfigurationPrefix that = (RoaConfigurationPrefix) obj;
-        return equal(getAsn(), that.getAsn())
-                && equal(this.getPrefix(), that.getPrefix())
-                && this.getMaximumLength() == that.getMaximumLength();
-    }
-
     @Override
     public String toString() {
         return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
                 .append("asn", getAsn())
                 .append("prefix", getPrefix())
                 .append("maximumLength", getMaximumLength())
+                .append("updatedAt", getUpdatedAt())
                 .toString();
     }