From 579f41a606621714868bfb6d4195f19aed7d10c0 Mon Sep 17 00:00:00 2001 From: RPKI Team at RIPE NCC Date: Wed, 10 Apr 2024 13:26:27 +0000 Subject: [PATCH] RIPE NCC has merged 4449509c1 * Add control to verify the duration of testing changes [0f27560fe] * SonarQube feedback [d638a3d96] * Incorporate SonarQube feedback [b6935586a] * Finish added updatedAt + rename types with VRPs from ROA* to ApiRoaPrefix* [13ac67c5c] * Add updatedAt to ROA-prefixes in API [27748c78b] * Sonar nits [7ed56db9b] * Add two test cases and document strange behaviour [e63ad9722] * Refactor test style in BgpRisEntryRepositoryBeanTest [2dba60b08] * clearer log message on unusable dir [41ff9925a] * chore(deps): update dependency org.sonarqube:org.sonarqube.gradle.plugin to v4.4.1.3373 [8e46ac777] --- .gitlab-ci.yml | 11 + buildSrc/build.gradle | 2 +- dependencies.gradle | 2 +- scripts/gitlab-deploy-check | 217 ++++++++++++++++++ .../domain/roa/RoaConfigurationPrefix.java | 7 +- .../rest/pojo/{ROA.java => ApiRoaPrefix.java} | 9 +- .../rpki/rest/pojo/ApiRoaPrefixExtended.java | 31 +++ .../net/ripe/rpki/rest/pojo/PublishSet.java | 4 +- .../net/ripe/rpki/rest/pojo/ROAExtended.java | 20 -- .../rest/pojo/ROAWithAnnouncementStatus.java | 14 +- .../rest/service/AnnouncementService.java | 16 +- .../service/CaRoaConfigurationService.java | 121 +++++----- .../ripe/rpki/rest/service/CaStatService.java | 2 +- .../java/net/ripe/rpki/rest/service/Roas.java | 14 +- .../net/ripe/rpki/rest/service/Utils.java | 34 +-- .../monitoring/RoaPrefixesService.java | 3 +- ...RoaAlertIgnoredAnnouncedRoutesCommand.java | 5 +- .../UpdateRoaConfigurationCommand.java | 5 +- .../RepositoryConfigurationBean.java | 2 +- .../server/api/dto/RoaConfigurationData.java | 15 +- .../api/dto/RoaConfigurationPrefixData.java | 45 ++-- .../services/read/BgpRisEntryViewService.java | 2 + .../rpki/services/impl/RoaAlertChecker.java | 23 +- ..._roaconfiguration_prefixes__updated_at.sql | 3 + .../bgpris/BgpRisEntryRepositoryBeanTest.java | 88 ++++--- .../net/ripe/rpki/rest/service/UtilsTest.java | 6 +- 26 files changed, 485 insertions(+), 216 deletions(-) create mode 100755 scripts/gitlab-deploy-check rename src/main/java/net/ripe/rpki/rest/pojo/{ROA.java => ApiRoaPrefix.java} (69%) create mode 100644 src/main/java/net/ripe/rpki/rest/pojo/ApiRoaPrefixExtended.java delete mode 100644 src/main/java/net/ripe/rpki/rest/pojo/ROAExtended.java create mode 100644 src/main/resources/db/migration/V130__roaconfiguration_prefixes__updated_at.sql diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 70eae45..74ba496 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -138,6 +138,17 @@ sonarqube: - if: $CI_COMMIT_BRANCH == "main" - if: $CI_COMMIT_BRANCH == "next" +control/run-on-staging: + image: node:21-alpine + stage: qa + script: + - ./scripts/gitlab-deploy-check + allow_failure: true + rules: + - if: $CI_COMMIT_BRANCH == "main" + when: always + - when: never + ######## Deploy stage ######## .delivr-deploy: &delivr-deploy stage: deploy diff --git a/buildSrc/build.gradle b/buildSrc/build.gradle index 0364ea0..5bcdbc7 100644 --- a/buildSrc/build.gradle +++ b/buildSrc/build.gradle @@ -14,5 +14,5 @@ dependencies { exclude group: 'org.eclipse.jgit', module: 'org.eclipse.jgit' } implementation 'org.eclipse.jgit:org.eclipse.jgit:5.13.3.202401111512-r' - implementation 'org.sonarqube:org.sonarqube.gradle.plugin:4.2.1.3168' + implementation 'org.sonarqube:org.sonarqube.gradle.plugin:4.4.1.3373' } diff --git a/dependencies.gradle b/dependencies.gradle index bc3ee61..4e24011 100644 --- a/dependencies.gradle +++ b/dependencies.gradle @@ -1,4 +1,4 @@ ext { - rpki_commons_version = '1.37' + rpki_commons_version = '1.39.1' spring_boot_version = '2.7.18' } diff --git a/scripts/gitlab-deploy-check b/scripts/gitlab-deploy-check new file mode 100755 index 0000000..6329c95 --- /dev/null +++ b/scripts/gitlab-deploy-check @@ -0,0 +1,217 @@ +#!/usr/bin/env node + +const GitLab = ({apiUrl, apiToken, projectId}) => { + const repeat = (x) => (n) => [...new Array(n)].map(() => x); + + const fetchFromGitLab = async (path, params) => { + const query = Object.entries({"per_page": 100, ...params}) + .filter(([_, value]) => value != null) + .map(([name, value]) => `${name}=${encodeURIComponent(value)}`) + .reduce((acc, x) => `${acc}${acc != "" ? "&" : ""}${x}`, ""); + const response = await fetch( + `${apiUrl}/${path}?${query}`, + { + headers: { + "PRIVATE-TOKEN": apiToken, + "accept": "application/json", + } + } + ); + if (!response.ok) { + switch (response.status) { + case 401: + throw "The configured access token is invalid. A project access token with scope read_api and role reporter is required."; + case 403: + throw "The configured access token has insufficient permissions. The token must have scope read_api and role reporter."; + default: + throw `Failed to GET ${apiUrl}${path}?${query}, server returned: ${response.status}`; + } + } + return response.json(); + }; + + const fetchAll = (f) => async function* (params={}, {concurrency, maxPages}={concurrency: 3, maxPages: 100}) { + const fetcher = (page) => f({...params, page}); + let page = 0; + while (page < maxPages) { + const pages = Math.min(concurrency, maxPages - page); + const results = await Promise.all(repeat(fetcher)(pages).map((f) => f(page++))); + for (const result of results) { + if (result.length === 0) { + break; + } + yield* result; + } + } + }; + + return { + queries: { + commit: (sha) => fetchFromGitLab(`/projects/${projectId}/repository/commits/${sha}`), + deployments: (params={}) => fetchFromGitLab(`/projects/${projectId}/deployments`, params), + merge_requests: (params={}) => fetchFromGitLab(`/projects/${projectId}/merge_requests`, params), + }, + + combinators: { + findFirst: (f) => (params={}, fetchConfig) => async (p) => { + for await (const result of fetchAll(f)(params, fetchConfig)) { + if (p(result)) { + return result; + } + } + }, + findAll: (f) => (params={}, fetchConfig) => async (p=() => true) => { + let acc = []; + for await (const result of fetchAll(f)(params, fetchConfig)) { + if (p(result)) { + acc.push(result); + } + } + return acc; + }, + }, + }; +}; + +const Calendar = { + isAfter: (x, y) => { + const xd = Date.parse(x); + const yd = Date.parse(y); + return xd > yd; + }, + max: (x, y) => Calendar.isAfter(x, y) ? x : y, + compare: (key=(x)=>x) => (x, y) => + Calendar.isAfter(key(x), key(y)) ? 1 : Calendar.isAfter(key(y), key(x)) ? -1 : 0, + showTimestamp: (x) => { + const d = new Date(Date.parse(x)); + const pad = (n) => String(n).padStart(2, "0"); + return `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())} ${d.getUTCHours()}:${pad(d.getUTCMinutes())}:${pad(d.getUTCSeconds())} UTC` + } +} + +const die = (msg) => { + console.error(msg); + process.exit(1); +}; + +const env = (name, def) => { + const val = process.env[name] || def; + if (val == null) { + die(`no value set for $${name}`); + } + return val; +}; + +const context = () => ({ + apiToken: env("DEPLOY_CHECK_ACCESS_TOKEN"), + apiUrl: env("CI_API_V4_URL"), + projectId: env("CI_PROJECT_ID"), + commitSha: env("CI_COMMIT_SHA"), + stagingEnv: env("DEPLOY_CHECK_STAGING_ENV", "prepdev"), + stagingDuration: env("DEPLOY_CHECK_DURATION", 12*60*60), +}); + +const showDuration = (duration) => { + const r = [ + [Math.floor(duration / 86400), "d"], + [Math.floor((duration % 86400) / 3600), "h"], + [Math.floor((duration % 86400 % 3600) / 60), "m"], + [Math.floor(duration % 86400 % 3600 % 60), "s"] + ] + .reduce((acc, [count, suffix]) => count > 0 ? `${acc} ${count}${suffix}` : acc, "") + .trim(); + return r.length > 0 ? r : "zero seconds"; +}; + +const calculateDeploymentDurations = (timeline, mergeRequest) => { + const matchesMR = ({ref, sha}) => { + return sha === mergeRequest.sha + || ref === mergeRequest.source_branch + || ref === `refs/merge-requests/${mergeRequest.iid}/merge`; + }; + + const mapPairs = (f) => (xs) => { + if (xs.length === 0) { + return xs; + } + const {result, last} = xs.reduce(({result, last}, x) => ({ + result: last != null ? [...result, f (last, x)] : result, + last: x, + }), {result: [], last: null}); + return [...result, f(last, null)]; + }; + + const refsWithDuration = mapPairs( + (deployment, replacement) => { + const start = Date.parse(deployment.deployable.finished_at); + const end = Date.parse(replacement?.deployable.started_at || mergeRequest.merged_at); + return [Math.floor((end - start) / 1000), { + iid: deployment.iid, + ref: deployment.ref, + sha: deployment.sha, + started_at: deployment.deployable.started_at, + finished_at: deployment.deployable.finished_at, + }]; + } + ); + + const durations = refsWithDuration(timeline); + return { + merge_request: durations + .filter(([_, refs]) => matchesMR(refs)) + .reduce((acc, [duration]) => acc + duration, 0), + last_commit: durations + .filter(([duration, refs]) => refs.sha === mergeRequest.sha) + .reduce((acc, [duration]) => acc + duration, 0), + }; +}; + +const main = async (ctx) => { + const gitlab = GitLab(ctx); + + const mergeRequest = await gitlab.combinators.findFirst(gitlab.queries.merge_requests)({state: "merged"})( + ({merge_commit_sha}) => merge_commit_sha === ctx.commitSha + ); + if (mergeRequest == null) { + throw `No merge request found with merge commit ${ctx.commitSha}`; + } + + const lastCommit = await gitlab.queries.commit(mergeRequest.sha); + + const deployments = await gitlab.combinators.findAll(gitlab.queries.deployments)({ + environment: ctx.stagingEnv, + finished_after: Calendar.max(lastCommit.created_at, mergeRequest.created_at), + order_by: "finished_at", + status: "success", + })(); + const timeline = [...deployments].sort(Calendar.compare((x) => x.finished_at)) + .reduce((acc, x) => acc.find(({id}) => id === x.id) != null ? acc : [...acc, x], []) + .reduce(({result, pastMerge}, x) => + pastMerge ? { result, pastMerge } : { + result: [...result, x], + pastMerge: Calendar.isAfter(x.finished_at, mergeRequest.merged_at) + }, + {result: [], pastMerge: false} + ).result; + + const durations = calculateDeploymentDurations(timeline, mergeRequest); + const passesThreshold = durations.merge_request > ctx.stagingDuration; + + process.stdout.write(` +[#${mergeRequest.iid}] ${mergeRequest.title} + +Branch ${mergeRequest.source_branch} at ${lastCommit.id} was merged into ${mergeRequest.target_branch} at ${Calendar.showTimestamp(mergeRequest.merged_at)}. + +The changes in #${mergeRequest.iid} ran on ${ctx.stagingEnv} for ${showDuration(durations.merge_request)}. The last commit ran for ${showDuration(durations.last_commit)}. + +${passesThreshold ? "✅" : "❌"} Minimum required staging period of ${showDuration (ctx.stagingDuration)} is ${passesThreshold ? "met" : "not met"}. +`); + + if (!passesThreshold) { + throw "Merge request failed the staging deployment control."; + } +}; + +main(context()).catch( + (err) => die(err) +); diff --git a/src/main/java/net/ripe/rpki/domain/roa/RoaConfigurationPrefix.java b/src/main/java/net/ripe/rpki/domain/roa/RoaConfigurationPrefix.java index cb5a25e..9241347 100644 --- a/src/main/java/net/ripe/rpki/domain/roa/RoaConfigurationPrefix.java +++ b/src/main/java/net/ripe/rpki/domain/roa/RoaConfigurationPrefix.java @@ -12,6 +12,7 @@ import javax.persistence.Column; import javax.persistence.Embeddable; import java.math.BigInteger; +import java.time.Instant; import java.util.List; import java.util.stream.Collectors; @@ -36,6 +37,10 @@ public class RoaConfigurationPrefix { @Column(name = "maximum_length", nullable = true) private Integer maximumLength; + @Getter + @Column(name = "updated_at", insertable = false, updatable = false) + private Instant updatedAt; + protected RoaConfigurationPrefix() { // JPA uses this } @@ -69,7 +74,7 @@ public int getMaximumLength() { } public RoaConfigurationPrefixData toData() { - return new RoaConfigurationPrefixData(getAsn(), getPrefix(), getMaximumLength()); + return new RoaConfigurationPrefixData(getAsn(), getPrefix(), getMaximumLength(), getUpdatedAt()); } @Override diff --git a/src/main/java/net/ripe/rpki/rest/pojo/ROA.java b/src/main/java/net/ripe/rpki/rest/pojo/ApiRoaPrefix.java similarity index 69% rename from src/main/java/net/ripe/rpki/rest/pojo/ROA.java rename to src/main/java/net/ripe/rpki/rest/pojo/ApiRoaPrefix.java index dcdaf21..ce5dbb5 100644 --- a/src/main/java/net/ripe/rpki/rest/pojo/ROA.java +++ b/src/main/java/net/ripe/rpki/rest/pojo/ApiRoaPrefix.java @@ -5,10 +5,16 @@ import lombok.Data; import lombok.NoArgsConstructor; +/** + * An intent for a Validated Roa Payload. + * + * This is one asn-prefix pair with an optional maxlength. This is not a ROA since a ROA is + * 1:n mapping from ASN to prefixes and maxlengths. + */ @NoArgsConstructor @AllArgsConstructor @Data -public class ROA { +public class ApiRoaPrefix { private String asn; private String prefix; // external API (and portal) use maximalLength (sic) @@ -17,6 +23,7 @@ public class ROA { @Override public String toString() { + // The term 'ROA' is kept to have a consistent API. return "ROA{" + "asn='" + asn + '\'' + ", prefix='" + prefix + '\'' + diff --git a/src/main/java/net/ripe/rpki/rest/pojo/ApiRoaPrefixExtended.java b/src/main/java/net/ripe/rpki/rest/pojo/ApiRoaPrefixExtended.java new file mode 100644 index 0000000..adbf520 --- /dev/null +++ b/src/main/java/net/ripe/rpki/rest/pojo/ApiRoaPrefixExtended.java @@ -0,0 +1,31 @@ +package net.ripe.rpki.rest.pojo; + +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.Setter; + +import javax.annotation.Nullable; +import java.time.Instant; + +@EqualsAndHashCode(callSuper = true) +public class ApiRoaPrefixExtended extends ApiRoaPrefix { + @JsonProperty("_numberOfValidsCaused") + @Getter @Setter private int numberOfValidsCaused; + + @JsonProperty("_numberOfInvalidsCaused") + @Getter @Setter private int numberOfInvalidsCaused; + + @JsonProperty("_updatedAt") + @JsonInclude(value = JsonInclude.Include.NON_NULL) + @Getter @Setter private Instant updatedAt; + + @SuppressWarnings("java:S117") + public ApiRoaPrefixExtended(String asn, String prefix, int maximalLength, int numberOfValidsCaused, int numberOfInvalidsCaused, @Nullable Instant updatedAt) { + super(asn, prefix, maximalLength); + this.numberOfValidsCaused = numberOfValidsCaused; + this.numberOfInvalidsCaused = numberOfInvalidsCaused; + this.updatedAt = updatedAt; + } +} diff --git a/src/main/java/net/ripe/rpki/rest/pojo/PublishSet.java b/src/main/java/net/ripe/rpki/rest/pojo/PublishSet.java index 975447f..37c8e09 100644 --- a/src/main/java/net/ripe/rpki/rest/pojo/PublishSet.java +++ b/src/main/java/net/ripe/rpki/rest/pojo/PublishSet.java @@ -9,7 +9,7 @@ public class PublishSet { private String ifMatch; - private List added = Collections.emptyList(); - private List deleted = Collections.emptyList(); + private List added = Collections.emptyList(); + private List deleted = Collections.emptyList(); } diff --git a/src/main/java/net/ripe/rpki/rest/pojo/ROAExtended.java b/src/main/java/net/ripe/rpki/rest/pojo/ROAExtended.java deleted file mode 100644 index 2fab61b..0000000 --- a/src/main/java/net/ripe/rpki/rest/pojo/ROAExtended.java +++ /dev/null @@ -1,20 +0,0 @@ -package net.ripe.rpki.rest.pojo; - -import lombok.EqualsAndHashCode; -import lombok.Getter; -import lombok.Setter; - -@EqualsAndHashCode(callSuper = true) -public class ROAExtended extends ROA { - @SuppressWarnings({"java:S117", "java:S116"}) - @Getter @Setter private int _numberOfValidsCaused; - @SuppressWarnings({"java:S117", "java:S116"}) - @Getter @Setter private int _numberOfInvalidsCaused; - - @SuppressWarnings("java:S117") - public ROAExtended(String asn, String prefix, int maximalLength, int _numberOfValidsCaused, int _numberOfInvalidsCaused) { - super(asn, prefix, maximalLength); - this._numberOfValidsCaused = _numberOfValidsCaused; - this._numberOfInvalidsCaused = _numberOfInvalidsCaused; - } -} diff --git a/src/main/java/net/ripe/rpki/rest/pojo/ROAWithAnnouncementStatus.java b/src/main/java/net/ripe/rpki/rest/pojo/ROAWithAnnouncementStatus.java index 98fef37..6ceccfb 100644 --- a/src/main/java/net/ripe/rpki/rest/pojo/ROAWithAnnouncementStatus.java +++ b/src/main/java/net/ripe/rpki/rest/pojo/ROAWithAnnouncementStatus.java @@ -1,25 +1,29 @@ package net.ripe.rpki.rest.pojo; +import com.fasterxml.jackson.annotation.JsonProperty; import net.ripe.rpki.commons.validation.roa.RouteValidityState; public class ROAWithAnnouncementStatus { - - private ROA roa; + /** + * Use legacy name in JSON serialization + */ + @JsonProperty("roa") + private ApiRoaPrefix roa; private RouteValidityState validity; public ROAWithAnnouncementStatus() { } - public ROAWithAnnouncementStatus(ROA roa, RouteValidityState validity) { + public ROAWithAnnouncementStatus(ApiRoaPrefix roa, RouteValidityState validity) { this.roa = roa; this.validity = validity; } - public ROA getRoa() { + public ApiRoaPrefix getRoa() { return roa; } - public void setRoa(ROA roa) { + public void setRoa(ApiRoaPrefix roa) { this.roa = roa; } diff --git a/src/main/java/net/ripe/rpki/rest/service/AnnouncementService.java b/src/main/java/net/ripe/rpki/rest/service/AnnouncementService.java index bfb9ed1..17cd8aa 100644 --- a/src/main/java/net/ripe/rpki/rest/service/AnnouncementService.java +++ b/src/main/java/net/ripe/rpki/rest/service/AnnouncementService.java @@ -15,10 +15,11 @@ import net.ripe.rpki.commons.validation.roa.RouteValidityState; import net.ripe.rpki.rest.exception.BadRequestException; import net.ripe.rpki.rest.pojo.BgpAnnouncement; -import net.ripe.rpki.rest.pojo.ROA; +import net.ripe.rpki.rest.pojo.ApiRoaPrefix; import net.ripe.rpki.server.api.dto.BgpRisEntry; import net.ripe.rpki.server.api.dto.HostedCertificateAuthorityData; import net.ripe.rpki.server.api.dto.RoaConfigurationData; +import net.ripe.rpki.server.api.dto.RoaConfigurationPrefixData; import net.ripe.rpki.server.api.services.read.BgpRisEntryViewService; import net.ripe.rpki.server.api.services.read.RoaAlertConfigurationViewService; import net.ripe.rpki.server.api.services.read.RoaViewService; @@ -78,17 +79,17 @@ public ResponseEntity> getResourcesForCa(@PathVariable("ca final RoaConfigurationData roaConfiguration = roaViewService.getRoaConfiguration(ca.getId()); final Set ignoredAnnouncements = Utils.getIgnoredAnnouncements(roaAlertConfigurationViewService, ca.getId()); - final List announcedAnnouncements = Utils.makeBgpAnnouncementList(announcements, roaConfiguration.toAllowedRoutes(), ignoredAnnouncements); + final List announcedAnnouncements = Utils.makeBgpAnnouncementList(announcements, roaConfiguration.getPrefixes(), ignoredAnnouncements); // ignoredAnnouncements \ bgp announcements final Set notSeenIgnoredAnnouncedRoutes = Sets.difference(ignoredAnnouncements, bgpRisMapToAnnouncedRoutes(announcements)); - final NestedIntervalMap> currentRouteMap = allowedRoutesToNestedIntervalMap(roaConfiguration.toAllowedRoutes()); + final NestedIntervalMap> currentRouteMap = allowedRoutesToNestedIntervalMap(roaConfiguration.getPrefixes()); // Create synthetic 'not seen' announcements for which a suppression exists. final List notSeenAnnouncements = notSeenIgnoredAnnouncedRoutes.stream().map( ar -> new BgpAnnouncement(ar.getOriginAsn().toString(), ar.getPrefix().toString(), - 0, Utils.ROUTE_VALIDATION_POLICY.validateAnnouncedRoute(currentRouteMap, ar), + 0, RouteOriginValidationPolicy.validateAnnouncedRoute(currentRouteMap, ar), true) ).collect(Collectors.toList()); @@ -105,7 +106,7 @@ private Set bgpRisMapToAnnouncedRoutes(Map> getAffectedAnnouncementsForCaAndRoa(@PathVariable("caName") final CaName caName, @RequestBody final ROA roa) { + public ResponseEntity> getAffectedAnnouncementsForCaAndRoa(@PathVariable("caName") final CaName caName, @RequestBody final ApiRoaPrefix roa) { log.info("Get all announcements affected by the given ROA configuration for CA: {}", caName); final Optional addedRoasErrorMessage = Utils.errorsInUserInputRoas(roa); @@ -125,14 +126,13 @@ public ResponseEntity> getAffectedAnnouncementsForCaAndRoa final Set ignoredAnnouncements = Utils.getIgnoredAnnouncements(roaAlertConfigurationViewService, ca.getId()); final Set routesValidatedByOthers = new HashSet<>(); - final NestedIntervalMap> currentRouteMap = allowedRoutesToNestedIntervalMap(roaConfiguration.toAllowedRoutes()); - final RouteOriginValidationPolicy routeOriginValidationPolicy = new RouteOriginValidationPolicy(); + final NestedIntervalMap> currentRouteMap = allowedRoutesToNestedIntervalMap(roaConfiguration.getPrefixes()); Stream.of(true, false) .filter(announcements::containsKey) .flatMap(verifiedOrNot -> announcements.get(verifiedOrNot).stream()) .map(BgpRisEntry::toAnnouncedRoute) .forEach(announcedRoute -> { - final RouteValidityState currentValidityState = routeOriginValidationPolicy.validateAnnouncedRoute(currentRouteMap, announcedRoute); + final RouteValidityState currentValidityState = RouteOriginValidationPolicy.validateAnnouncedRoute(currentRouteMap, announcedRoute); if (currentValidityState == RouteValidityState.VALID && !(roaAsn.equals(announcedRoute.getOriginAsn()) && roaPrefix.equals(announcedRoute.getPrefix()))) { routesValidatedByOthers.add(announcedRoute); diff --git a/src/main/java/net/ripe/rpki/rest/service/CaRoaConfigurationService.java b/src/main/java/net/ripe/rpki/rest/service/CaRoaConfigurationService.java index e798be7..18546cb 100644 --- a/src/main/java/net/ripe/rpki/rest/service/CaRoaConfigurationService.java +++ b/src/main/java/net/ripe/rpki/rest/service/CaRoaConfigurationService.java @@ -3,19 +3,17 @@ import com.google.common.base.Preconditions; import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.tags.Tag; +import lombok.Value; import lombok.extern.slf4j.Slf4j; import net.ripe.ipresource.*; import net.ripe.ipresource.etree.NestedIntervalMap; -import net.ripe.rpki.commons.validation.roa.AllowedRoute; -import net.ripe.rpki.commons.validation.roa.AnnouncedRoute; -import net.ripe.rpki.commons.validation.roa.RouteOriginValidationPolicy; -import net.ripe.rpki.commons.validation.roa.RouteValidityState; +import net.ripe.rpki.commons.validation.roa.*; import net.ripe.rpki.domain.roa.RoaConfigurationRepository; import net.ripe.rpki.rest.pojo.BgpAnnouncement; import net.ripe.rpki.rest.pojo.BgpAnnouncementChange; import net.ripe.rpki.rest.pojo.PublishSet; -import net.ripe.rpki.rest.pojo.ROA; -import net.ripe.rpki.rest.pojo.ROAExtended; +import net.ripe.rpki.rest.pojo.ApiRoaPrefix; +import net.ripe.rpki.rest.pojo.ApiRoaPrefixExtended; import net.ripe.rpki.rest.pojo.ROAWithAnnouncementStatus; import net.ripe.rpki.server.api.commands.UpdateRoaConfigurationCommand; import net.ripe.rpki.server.api.dto.*; @@ -35,7 +33,7 @@ import java.util.*; import java.util.stream.Collectors; -import static com.google.common.collect.ImmutableMap.of; +import static java.util.Map.of; import static javax.ws.rs.core.MediaType.APPLICATION_JSON; import static net.ripe.rpki.commons.validation.roa.RouteOriginValidationPolicy.allowedRoutesToNestedIntervalMap; import static net.ripe.rpki.rest.service.AbstractCaRestService.API_URL_PREFIX; @@ -67,19 +65,18 @@ public CaRoaConfigurationService(RoaViewService roaViewService, @GetMapping @Operation(summary = "Get all ROAs belonging to a CA") - public ResponseEntity> getROAsForCAWithBrokenAnnouncements(@PathVariable("caName") final CaName caName) { + public ResponseEntity> getROAsForCAWithBrokenAnnouncements(@PathVariable("caName") final CaName caName) { log.info("REST call: Get all ROAs belonging to CA: {}", caName); final HostedCertificateAuthorityData ca = getCa(HostedCertificateAuthorityData.class, caName); final ImmutableResourceSet certifiedResources = ca.getResources(); - final RoaConfigurationData roaConfiguration = roaViewService.getRoaConfiguration(ca.getId()); - final RouteOriginValidationPolicy routeOriginValidationPolicy = new RouteOriginValidationPolicy(); + final var roaConfiguration = roaViewService.getRoaConfiguration(ca.getId()); + final var roaPrefixes = roaConfiguration.getPrefixes(); final Collection announcements = bgpRisEntryViewService.findMostSpecificOverlapping(certifiedResources); - final List allowedRoutes = roaConfiguration.toAllowedRoutes(); - final NestedIntervalMap> allowedRouteMap = allowedRoutesToNestedIntervalMap(allowedRoutes); - final Set ignoredAnnouncements = getIgnoredAnnouncement(ca.getId()); + final NestedIntervalMap> allowedRouteMap = allowedRoutesToNestedIntervalMap(roaPrefixes); + final Set ignoredAnnouncements = getIgnoredAnnouncement(roaAlertConfigurationViewService, ca.getId()); // gather the announcements which are made invalid by some ROAs // and don't have ROAs validating them @@ -87,27 +84,25 @@ public ResponseEntity> getROAsForCAWithBrokenAnnouncements(@Pa for (BgpRisEntry bgp : announcements) { final AnnouncedRoute announcedRoute = bgp.toAnnouncedRoute(); if (!ignoredAnnouncements.contains(announcedRoute)) { - final RouteValidityState validityState = routeOriginValidationPolicy.validateAnnouncedRoute(allowedRouteMap, announcedRoute); + final RouteValidityState validityState = RouteOriginValidationPolicy.validateAnnouncedRoute(allowedRouteMap, announcedRoute); if (validityState == RouteValidityState.INVALID_ASN || validityState == RouteValidityState.INVALID_LENGTH) { invalidAnnouncements.add(bgp); } } } - final List roas = new ArrayList<>(); - allowedRoutes.stream() - .sorted(Comparator - .comparing(ar -> ((AllowedRoute) ar).getAsn().longValue()) - .thenComparing(ar -> ((AllowedRoute) ar).getPrefix())) - .forEach(allowedRoute -> { - final Collection overlappingAnnouncements = bgpRisEntryViewService.findMostSpecificOverlapping(ImmutableResourceSet.of(allowedRoute.getPrefix())); - final NestedIntervalMap> allowed = allowedRoutesToNestedIntervalMap(Collections.singletonList(allowedRoute)); + final List roas = new ArrayList<>(); + roaPrefixes.stream() + .sorted() + .forEach(roaPrefix -> { + final Collection announcementsOverlappingWithCurrentPrefix = bgpRisEntryViewService.findMostSpecificOverlapping(ImmutableResourceSet.of(roaPrefix.getPrefix())); + final NestedIntervalMap> allowed = allowedRoutesToNestedIntervalMap(Collections.singletonList(roaPrefix)); int valids = 0; int invalids = 0; - for (BgpRisEntry bgp : overlappingAnnouncements) { + for (BgpRisEntry bgp : announcementsOverlappingWithCurrentPrefix) { final AnnouncedRoute announcedRoute = new AnnouncedRoute(bgp.getOrigin(), bgp.getPrefix()); if (!ignoredAnnouncements.contains(announcedRoute)) { - final RouteValidityState currentValidityState = routeOriginValidationPolicy.validateAnnouncedRoute(allowed, announcedRoute); + final RouteValidityState currentValidityState = RouteOriginValidationPolicy.validateAnnouncedRoute(allowed, announcedRoute); if (invalidAnnouncements.contains(bgp)) { if (currentValidityState == RouteValidityState.INVALID_ASN || currentValidityState == RouteValidityState.INVALID_LENGTH) { invalids++; @@ -117,7 +112,7 @@ public ResponseEntity> getROAsForCAWithBrokenAnnouncements(@Pa } } } - roas.add(new ROAExtended(allowedRoute.getAsn().toString(), allowedRoute.getPrefix().toString(), allowedRoute.getMaximumLength(), valids, invalids)); + roas.add(new ApiRoaPrefixExtended(roaPrefix.getAsn().toString(), roaPrefix.getPrefix().toString(), roaPrefix.getMaximumLength(), valids, invalids, roaPrefix.getUpdatedAt())); }); return ResponseEntity.ok() .contentType(MediaType.APPLICATION_JSON) @@ -133,7 +128,7 @@ public ResponseEntity> getAffectingROAsForCA(@Pa final HostedCertificateAuthorityData ca = getCa(HostedCertificateAuthorityData.class, caName); final RoaConfigurationData currentRoaConfiguration = roaViewService.getRoaConfiguration(ca.getId()); - final List certifiedRoutes = currentRoaConfiguration.toAllowedRoutes(); + final List certifiedRoutes = currentRoaConfiguration.getPrefixes(); final IpRange announcedPrefix = IpRange.parse(announcement.getPrefix()); final String announcementAsn = announcement.getAsn(); @@ -141,7 +136,7 @@ public ResponseEntity> getAffectingROAsForCA(@Pa .filter(certifiedRoute -> certifiedRoute.getPrefix().contains(announcedPrefix)) .map(certifiedRoute -> { final String routeAsn = certifiedRoute.getAsn().toString(); - final ROA roa = new ROA(routeAsn, certifiedRoute.getPrefix().toString(), certifiedRoute.getMaximumLength()); + final ApiRoaPrefix roa = new ApiRoaPrefix(routeAsn, certifiedRoute.getPrefix().toString(), certifiedRoute.getMaximumLength()); final RouteValidityState validityState = determineValidityState(announcedPrefix, announcementAsn, certifiedRoute); return new ROAWithAnnouncementStatus(roa, validityState); }) @@ -155,7 +150,7 @@ public ResponseEntity> getAffectingROAsForCA(@Pa * @requires certifiedRoute to contain announcedprefix * @return validity state for route */ - static RouteValidityState determineValidityState(IpRange announcedPrefix, String announcementAsn, AllowedRoute certifiedRoute) { + static RouteValidityState determineValidityState(IpRange announcedPrefix, String announcementAsn, RoaPrefixData certifiedRoute) { Preconditions.checkArgument(certifiedRoute.getPrefix().contains(announcedPrefix), "prefix of route %s is not contained by allowed route %s", announcedPrefix, certifiedRoute.getPrefix()); if (!certifiedRoute.getAsn().equals(Asn.parse(announcementAsn))) { @@ -169,7 +164,7 @@ static RouteValidityState determineValidityState(IpRange announcedPrefix, String @PostMapping(path = "stage") @Operation(summary = "Stage ROA changes for the given CA") public ResponseEntity stageRoaChanges(@PathVariable("caName") final CaName caName, - @RequestBody final List futureRoas) { + @RequestBody final List futureRoas) { log.info("REST call: Stage ROAs for CA: {}", caName); final Optional errorMessage = Utils.errorsInUserInputRoas(futureRoas); @@ -183,24 +178,20 @@ public ResponseEntity stageRoaChanges(@PathVariable("caName") final CaName ca final Map> bgpAnnouncements = bgpRisEntryViewService.findMostSpecificContainedAndNotContained(certifiedResources); final RoaConfigurationData currentRoaConfiguration = roaViewService.getRoaConfiguration(ca.getId()); - final Set currentRoutes = new HashSet<>(currentRoaConfiguration.toAllowedRoutes()); - final Set futureRoutes = new HashSet<>(futureRoas.size()); + final Set currentRoutes = new HashSet<>(currentRoaConfiguration.getPrefixes()); - IpResourceSet affectedRanges; - try { - // This will fill up futureRoutes as well - affectedRanges = buildAffectedRanges(futureRoas, futureRoutes, currentRoutes); - } catch (IllegalArgumentException e) { - return ResponseEntity.status(BAD_REQUEST).body(Map.of(ERROR, e.getMessage())); - } + // MAY throw, but IllegalArgumentExceptions are translated to HTTP 500 through @ControllerAdvice + + var effectOfFutureRoas = buildAffectedRanges(futureRoas, currentRoutes.stream().map(RoaPrefixData::toAllowedRoute).collect(Collectors.toSet())); - Optional validationError = Roas.validateRoaUpdate(futureRoutes); + Optional validationError = Roas.validateRoaUpdate(effectOfFutureRoas.futureRoutes); if (validationError.isPresent()) { - return ResponseEntity.status(BAD_REQUEST).body(Map.of(ERROR, validationError.get())); + return ResponseEntity.status(BAD_REQUEST).body(of(ERROR, validationError.get())); } final List bgpAnnouncementChanges = getBgpAnnouncementChanges( - ca, currentRoutes, futureRoutes, bgpAnnouncements, affectedRanges); + roaAlertConfigurationViewService, + ca, currentRoutes, effectOfFutureRoas.futureRoutes, bgpAnnouncements, effectOfFutureRoas.affectedRanges); return ResponseEntity.ok() .contentType(MediaType.APPLICATION_JSON) @@ -208,39 +199,53 @@ public ResponseEntity stageRoaChanges(@PathVariable("caName") final CaName ca .body(bgpAnnouncementChanges); } - private IpResourceSet buildAffectedRanges(List futureRoas, Set futureRoutes, Set currentRoutes) { - final IpResourceSet affectedRanges = new IpResourceSet(); - for (ROA roa : futureRoas) { + /** + * Gather the set of resources span by the futureRoas and add an AllowedRoute to futureRoutes for all + * ROAs in futureRoas that are not in currentRoutes. + */ + private static AffectedRangesForVRPs buildAffectedRanges(List futureRoas, Set currentRoutes) { + var affectedRanges = new IpResourceSet(); + var futureRoutes = new HashSet(); + + for (ApiRoaPrefix roa : futureRoas) { final IpRange roaIpRange = IpRange.parse(roa.getPrefix()); final AllowedRoute route = new AllowedRoute(Asn.parse(roa.getAsn()), roaIpRange, roa.getMaxLength()); futureRoutes.add(route); if (!currentRoutes.contains(route)) affectedRanges.add(roaIpRange); } - for (AllowedRoute route : currentRoutes) { + for (var route : currentRoutes) { if (!futureRoutes.contains(route)) affectedRanges.add(route.getPrefix()); } - return affectedRanges; + + return new AffectedRangesForVRPs(affectedRanges, futureRoutes); + } + + @Value + private static class AffectedRangesForVRPs { + private final IpResourceSet affectedRanges; + private final Set futureRoutes; } - private List getBgpAnnouncementChanges(HostedCertificateAuthorityData ca, - Set currentRoutes, + private static List getBgpAnnouncementChanges( + RoaAlertConfigurationViewService service, + HostedCertificateAuthorityData ca, + Set currentRoutes, Set futureRoutes, Map> bgpAnnouncements, IpResourceSet affectedRanges) { - final NestedIntervalMap> currentRouteMap = allowedRoutesToNestedIntervalMap(currentRoutes); + final NestedIntervalMap> currentRouteMap = allowedRoutesToNestedIntervalMap(currentRoutes); final NestedIntervalMap> futureRouteMap = allowedRoutesToNestedIntervalMap(futureRoutes); - final Set ignoredAnnouncements = getIgnoredAnnouncement(ca.getId()); + final Set ignoredAnnouncements = getIgnoredAnnouncement(service, ca.getId()); - final RouteOriginValidationPolicy routeOriginValidationPolicy = new RouteOriginValidationPolicy(); final List result = new ArrayList<>(); for (Boolean verifiedOrNot : new Boolean[]{true, false}) { if (bgpAnnouncements.containsKey(verifiedOrNot)) { for (BgpRisEntry bgp : bgpAnnouncements.get(verifiedOrNot)) { final AnnouncedRoute announcedRoute = bgp.toAnnouncedRoute(); - final RouteValidityState currentValidityState = routeOriginValidationPolicy.validateAnnouncedRoute(currentRouteMap, announcedRoute); - final RouteValidityState futureValidityState = routeOriginValidationPolicy.validateAnnouncedRoute(futureRouteMap, announcedRoute); + final RouteValidityState currentValidityState = RouteOriginValidationPolicy.validateAnnouncedRoute(currentRouteMap, announcedRoute); + final RouteValidityState futureValidityState = RouteOriginValidationPolicy.validateAnnouncedRoute(futureRouteMap, announcedRoute); final boolean isSuppressed = ignoredAnnouncements.contains(announcedRoute); result.add(new BgpAnnouncementChange(bgp.getOrigin().toString(), bgp.getPrefix().toString(), bgp.getVisibility(), isSuppressed, currentValidityState, futureValidityState, @@ -276,7 +281,7 @@ public ResponseEntity publishROAs(@PathVariable("caName") final CaName caName final HostedCertificateAuthorityData ca = getCa(HostedCertificateAuthorityData.class, caName); final ImmutableResourceSet certifiedResources = ca.getResources(); - for (ROA roa : publishSet.getAdded()) { + for (ApiRoaPrefix roa : publishSet.getAdded()) { PrefixValidationResult validationResult = validatePrefix(roa.getPrefix(), certifiedResources); if (PrefixValidationResult.OWNERSHIP_ERROR == validationResult) { return ResponseEntity.status(BAD_REQUEST).body(of(ERROR, validationResult.getMessage(roa.getPrefix()))); @@ -289,7 +294,7 @@ public ResponseEntity publishROAs(@PathVariable("caName") final CaName caName final String ifMatch = StringUtils.defaultIfEmpty(ifMatchHeader, publishSet.getIfMatch()); try { - var currentRoas = new HashSet<>(roaViewService.getRoaConfiguration(ca.getId()).toAllowedRoutes()); + var currentRoas = roaViewService.getRoaConfiguration(ca.getId()).getPrefixes().stream().map(RoaPrefixData::toAllowedRoute).collect(Collectors.toSet()); var futureRoas = Roas.applyDiff(currentRoas, Roas.toDiff(publishSet)); Roas.validateRoaUpdate(futureRoas) @@ -309,7 +314,7 @@ public ResponseEntity publishROAs(@PathVariable("caName") final CaName caName } } - private Collection getRoaConfigurationPrefixDatas(final Collection roas) { + private Collection getRoaConfigurationPrefixDatas(final Collection roas) { return roas.stream() .map(roa -> new RoaConfigurationPrefixData( Asn.parse(roa.getAsn()), @@ -318,8 +323,8 @@ private Collection getRoaConfigurationPrefixDatas(fi .collect(Collectors.toList()); } - private Set getIgnoredAnnouncement(Long caId) { - final RoaAlertConfigurationData roaAlertSubscription = roaAlertConfigurationViewService.findRoaAlertSubscription(caId); + private static Set getIgnoredAnnouncement(RoaAlertConfigurationViewService service, Long caId) { + final RoaAlertConfigurationData roaAlertSubscription = service.findRoaAlertSubscription(caId); if (roaAlertSubscription != null && roaAlertSubscription.getIgnoredAnnouncements() != null) { return roaAlertSubscription.getIgnoredAnnouncements(); } diff --git a/src/main/java/net/ripe/rpki/rest/service/CaStatService.java b/src/main/java/net/ripe/rpki/rest/service/CaStatService.java index 7eb21e4..19e3b57 100644 --- a/src/main/java/net/ripe/rpki/rest/service/CaStatService.java +++ b/src/main/java/net/ripe/rpki/rest/service/CaStatService.java @@ -131,7 +131,7 @@ private CaStatus getCaStatus(long caId, ImmutableResourceSet certifiedResources) final Map> announcements = bgpRisEntryViewService.findMostSpecificContainedAndNotContained(certifiedResources); final RoaConfigurationData roaConfiguration = roaViewService.getRoaConfiguration(caId); final Set ignoredAnnouncements = Utils.getIgnoredAnnouncements(roaAlertConfigurationViewService, caId); - final List bgpAnnouncements = Utils.makeBgpAnnouncementList(announcements, roaConfiguration.toAllowedRoutes(), ignoredAnnouncements); + final List bgpAnnouncements = Utils.makeBgpAnnouncementList(announcements, roaConfiguration.getPrefixes(), ignoredAnnouncements); int valid = 0, invalid = 0, unknown = 0; for (BgpAnnouncement bgpAnnouncement : bgpAnnouncements) { diff --git a/src/main/java/net/ripe/rpki/rest/service/Roas.java b/src/main/java/net/ripe/rpki/rest/service/Roas.java index c072732..d083e4c 100644 --- a/src/main/java/net/ripe/rpki/rest/service/Roas.java +++ b/src/main/java/net/ripe/rpki/rest/service/Roas.java @@ -1,22 +1,22 @@ package net.ripe.rpki.rest.service; import com.google.common.collect.Streams; -import lombok.AccessLevel; -import lombok.NoArgsConstructor; import lombok.Value; +import lombok.experimental.UtilityClass; import net.ripe.ipresource.Asn; import net.ripe.ipresource.IpRange; import net.ripe.rpki.commons.validation.roa.AllowedRoute; import net.ripe.rpki.commons.validation.roa.AnnouncedRoute; +import net.ripe.rpki.commons.validation.roa.RoaPrefixData; import net.ripe.rpki.rest.pojo.PublishSet; -import net.ripe.rpki.rest.pojo.ROA; +import net.ripe.rpki.rest.pojo.ApiRoaPrefix; import java.util.*; import java.util.stream.Collectors; -@NoArgsConstructor(access = AccessLevel.PRIVATE) +@UtilityClass public class Roas { - + // FIXME: This is sketchy: prefix is only used in error message. Why is the parameter required? public static Optional validateUniqueROAs(String prefix, Map> newOnes) { for (var e : newOnes.entrySet()) { var maxLengths = e.getValue(); @@ -36,7 +36,7 @@ public static RoaDiff toDiff(PublishSet publishSet) { ); } - private static AllowedRoute toAllowedRoute(ROA roa) { + private static AllowedRoute toAllowedRoute(ApiRoaPrefix roa) { var roaIpRange = IpRange.parse(roa.getPrefix()); var maxLength = roa.getMaxLength() != null ? roa.getMaxLength() : roaIpRange.getPrefixLength(); return new AllowedRoute(Asn.parse(roa.getAsn()), roaIpRange, maxLength); @@ -49,7 +49,7 @@ public static Set applyDiff(Set currentRoas, RoaDiff return futureRoas; } - public static Optional validateRoaUpdate(Set futureRoutes) { + public static Optional validateRoaUpdate(Set futureRoutes) { var futureMap = futureRoutes.stream().collect(Collectors.toMap( r -> new AnnouncedRoute(r.getAsn(), r.getPrefix()), r -> Collections.singletonList(r.getMaximumLength()), diff --git a/src/main/java/net/ripe/rpki/rest/service/Utils.java b/src/main/java/net/ripe/rpki/rest/service/Utils.java index 3093b9a..6b72606 100644 --- a/src/main/java/net/ripe/rpki/rest/service/Utils.java +++ b/src/main/java/net/ripe/rpki/rest/service/Utils.java @@ -1,23 +1,17 @@ package net.ripe.rpki.rest.service; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.Streams; -import com.nimbusds.jose.util.Pair; import lombok.AccessLevel; import lombok.NoArgsConstructor; import lombok.NonNull; -import lombok.Value; import net.ripe.ipresource.Asn; import net.ripe.ipresource.IpRange; import net.ripe.ipresource.IpResource; import net.ripe.ipresource.ImmutableResourceSet; import net.ripe.ipresource.etree.NestedIntervalMap; -import net.ripe.rpki.commons.validation.roa.AllowedRoute; -import net.ripe.rpki.commons.validation.roa.AnnouncedRoute; -import net.ripe.rpki.commons.validation.roa.RouteOriginValidationPolicy; -import net.ripe.rpki.commons.validation.roa.RouteValidityState; +import net.ripe.rpki.commons.validation.roa.*; import net.ripe.rpki.rest.pojo.BgpAnnouncement; -import net.ripe.rpki.rest.pojo.ROA; +import net.ripe.rpki.rest.pojo.ApiRoaPrefix; import net.ripe.rpki.server.api.dto.BgpRisEntry; import net.ripe.rpki.server.api.dto.RoaAlertConfigurationData; import net.ripe.rpki.server.api.services.read.RoaAlertConfigurationViewService; @@ -31,14 +25,10 @@ @NoArgsConstructor(access = AccessLevel.PRIVATE) class Utils { - // current implementation in commons is stateless, however, likely not made static to be able to parameterize it - // later. - public static final RouteOriginValidationPolicy ROUTE_VALIDATION_POLICY = new RouteOriginValidationPolicy(); - - static List makeBgpAnnouncementList(Map> announcements, - Iterable currentAllowedRoutes, - Set ignoredAnnouncements) { - final NestedIntervalMap> currentRouteMap = allowedRoutesToNestedIntervalMap(currentAllowedRoutes); + static List makeBgpAnnouncementList(Map> announcements, + Iterable currentAllowedRoutes, + Set ignoredAnnouncements) { + final NestedIntervalMap> currentRouteMap = allowedRoutesToNestedIntervalMap(currentAllowedRoutes); // Verified announcements first, then the rest. return Stream.of(true, false) @@ -47,7 +37,7 @@ static List makeBgpAnnouncementList(Map getIgnoredAnnouncements(RoaAlertConfigurationViewServ return ignoredAnnouncements; } - public static Optional errorsInUserInputRoas(final List roas) { + public static Optional errorsInUserInputRoas(final List roas) { return errorsInUserInputRoas(roas.stream()); } - public static Optional errorsInUserInputRoas(final ROA... roas) { + public static Optional errorsInUserInputRoas(final ApiRoaPrefix... roas) { return errorsInUserInputRoas(Stream.of(roas)); } - private static Optional errorsInUserInputRoas(final Stream roas) { + private static Optional errorsInUserInputRoas(final Stream roas) { final List errors = new ArrayList<>(); roas.forEach(r -> { if (r == null) { @@ -90,7 +80,7 @@ private static Optional errorsInUserInputRoas(final Stream roas) { return errors.isEmpty() ? Optional.empty() : Optional.of(String.join(", ", errors)); } - private static void validateAsn(List errors, @NonNull ROA r) { + private static void validateAsn(List errors, @NonNull ApiRoaPrefix r) { if (r.getAsn() == null) { errors.add("ASN is empty in (" + r + ")"); } else { @@ -102,7 +92,7 @@ private static void validateAsn(List errors, @NonNull ROA r) { } } - private static void validatePrefixAndMaxLength(List errors, @NonNull ROA r) { + private static void validatePrefixAndMaxLength(List errors, @NonNull ApiRoaPrefix r) { if (r.getPrefix() == null) { errors.add("Prefix is empty in (" + r + ")"); } else { diff --git a/src/main/java/net/ripe/rpki/rest/service/monitoring/RoaPrefixesService.java b/src/main/java/net/ripe/rpki/rest/service/monitoring/RoaPrefixesService.java index 84f0339..65d318c 100644 --- a/src/main/java/net/ripe/rpki/rest/service/monitoring/RoaPrefixesService.java +++ b/src/main/java/net/ripe/rpki/rest/service/monitoring/RoaPrefixesService.java @@ -5,6 +5,7 @@ import io.swagger.v3.oas.annotations.tags.Tag; import lombok.AllArgsConstructor; import lombok.Value; +import net.ripe.rpki.commons.validation.roa.RoaPrefixData; import net.ripe.rpki.domain.roa.RoaConfigurationPrefix; import net.ripe.rpki.domain.roa.RoaConfigurationRepository; import net.ripe.rpki.server.api.dto.RoaConfigurationPrefixData; @@ -52,7 +53,7 @@ public ResponseEntity> list .stream() .flatMap(rc -> rc.getPrefixes().stream()) .map(RoaConfigurationPrefix::toData) - .sorted(RoaConfigurationPrefixData.COMPARATOR) + .sorted(RoaPrefixData.ROA_PREFIX_DATA_COMPARATOR) .collect(Collectors.toList()); return ResponseEntity.ok(ValidatedObjectsResponse.of(roas, Collections.singletonMap("origin", "rpki-core"))); } diff --git a/src/main/java/net/ripe/rpki/server/api/commands/UpdateRoaAlertIgnoredAnnouncedRoutesCommand.java b/src/main/java/net/ripe/rpki/server/api/commands/UpdateRoaAlertIgnoredAnnouncedRoutesCommand.java index bc933b7..889b80a 100644 --- a/src/main/java/net/ripe/rpki/server/api/commands/UpdateRoaAlertIgnoredAnnouncedRoutesCommand.java +++ b/src/main/java/net/ripe/rpki/server/api/commands/UpdateRoaAlertIgnoredAnnouncedRoutesCommand.java @@ -2,6 +2,7 @@ import net.ripe.rpki.commons.util.VersionedId; import net.ripe.rpki.commons.validation.roa.AnnouncedRoute; +import net.ripe.rpki.commons.validation.roa.RouteData; import org.apache.commons.lang.StringUtils; import java.util.ArrayList; @@ -19,9 +20,9 @@ public class UpdateRoaAlertIgnoredAnnouncedRoutesCommand extends CertificateAuth public UpdateRoaAlertIgnoredAnnouncedRoutesCommand(VersionedId certificateAuthorityId, Collection added, Collection deleted) { super(certificateAuthorityId, CertificateAuthorityCommandGroup.USER); this.additions = new ArrayList<>(added); - this.additions.sort(AnnouncedRoute.ASN_PREFIX_COMPARATOR); + this.additions.sort(RouteData.ROUTE_DATA_COMPARATOR); this.deletions = new ArrayList<>(deleted); - this.deletions.sort(AnnouncedRoute.ASN_PREFIX_COMPARATOR); + this.deletions.sort(RouteData.ROUTE_DATA_COMPARATOR); } public List getAdditions() { diff --git a/src/main/java/net/ripe/rpki/server/api/commands/UpdateRoaConfigurationCommand.java b/src/main/java/net/ripe/rpki/server/api/commands/UpdateRoaConfigurationCommand.java index f5b550a..b07a6d4 100644 --- a/src/main/java/net/ripe/rpki/server/api/commands/UpdateRoaConfigurationCommand.java +++ b/src/main/java/net/ripe/rpki/server/api/commands/UpdateRoaConfigurationCommand.java @@ -2,6 +2,7 @@ import lombok.Getter; import net.ripe.rpki.commons.util.VersionedId; +import net.ripe.rpki.commons.validation.roa.RoaPrefixData; import net.ripe.rpki.server.api.dto.RoaConfigurationPrefixData; import org.apache.commons.lang.StringUtils; @@ -25,9 +26,9 @@ public UpdateRoaConfigurationCommand(VersionedId certificateAuthorityId, Optiona super(certificateAuthorityId, CertificateAuthorityCommandGroup.USER); this.ifMatch = ifMatch; this.additions = new ArrayList<>(added); - this.additions.sort(RoaConfigurationPrefixData.COMPARATOR); + this.additions.sort(RoaPrefixData.ROA_PREFIX_DATA_COMPARATOR); this.deletions = new ArrayList<>(deleted); - this.deletions.sort(RoaConfigurationPrefixData.COMPARATOR); + this.deletions.sort(RoaPrefixData.ROA_PREFIX_DATA_COMPARATOR); } public List getAdditions() { diff --git a/src/main/java/net/ripe/rpki/server/api/configuration/RepositoryConfigurationBean.java b/src/main/java/net/ripe/rpki/server/api/configuration/RepositoryConfigurationBean.java index c771f62..5379cb8 100644 --- a/src/main/java/net/ripe/rpki/server/api/configuration/RepositoryConfigurationBean.java +++ b/src/main/java/net/ripe/rpki/server/api/configuration/RepositoryConfigurationBean.java @@ -57,7 +57,7 @@ private File validateLocalRepositoryDirectory(String localRepositoryDirectoryStr Files.createDirectories(repository.toPath()); } catch (IOException e) { throw new CertificationConfigurationException("Local repository " + - " (" + repository.toPath() + " surely for testing!) does not exist and cannot be created", e); + " (" + repository.toPath() + ") does not exist and cannot be created", e); } } diff --git a/src/main/java/net/ripe/rpki/server/api/dto/RoaConfigurationData.java b/src/main/java/net/ripe/rpki/server/api/dto/RoaConfigurationData.java index a79f60b..db6db8d 100644 --- a/src/main/java/net/ripe/rpki/server/api/dto/RoaConfigurationData.java +++ b/src/main/java/net/ripe/rpki/server/api/dto/RoaConfigurationData.java @@ -2,6 +2,7 @@ import lombok.NonNull; import net.ripe.rpki.commons.validation.roa.AllowedRoute; +import net.ripe.rpki.commons.validation.roa.RoaPrefixData; import net.ripe.rpki.server.api.support.objects.ValueObjectSupport; import net.ripe.rpki.util.Streams; @@ -16,17 +17,12 @@ public class RoaConfigurationData extends ValueObjectSupport { public RoaConfigurationData(@NonNull List prefixes) { this.prefixes = new ArrayList<>(prefixes); - this.prefixes.sort(RoaConfigurationPrefixData.COMPARATOR); + this.prefixes.sort(RoaPrefixData.ROA_PREFIX_DATA_COMPARATOR); } public List getPrefixes() { return Collections.unmodifiableList(prefixes); } - - public List toAllowedRoutes() { - return toAllowedRoutes(prefixes); - } - public String entityTag() { return Streams.entityTag( prefixes.stream().flatMap(prefix -> Stream.of( @@ -36,13 +32,6 @@ public String entityTag() { )) ); } - - private static List toAllowedRoutes(Collection prefixes) { - return prefixes.stream() - .map(roaPrefix -> new AllowedRoute(roaPrefix.getAsn(), roaPrefix.getPrefix(), roaPrefix.getMaximumLength())) - .collect(Collectors.toList()); - } - private static byte[] stringBytes(Object value) { return String.valueOf(value).getBytes(StandardCharsets.UTF_8); } diff --git a/src/main/java/net/ripe/rpki/server/api/dto/RoaConfigurationPrefixData.java b/src/main/java/net/ripe/rpki/server/api/dto/RoaConfigurationPrefixData.java index f310ac3..924f5db 100644 --- a/src/main/java/net/ripe/rpki/server/api/dto/RoaConfigurationPrefixData.java +++ b/src/main/java/net/ripe/rpki/server/api/dto/RoaConfigurationPrefixData.java @@ -1,6 +1,7 @@ package net.ripe.rpki.server.api.dto; import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.annotation.JsonSerialize; import com.fasterxml.jackson.databind.ser.std.ToStringSerializer; @@ -8,8 +9,10 @@ import net.ripe.ipresource.Asn; import net.ripe.ipresource.IpRange; import net.ripe.rpki.commons.crypto.cms.roa.RoaPrefix; +import net.ripe.rpki.commons.validation.roa.RoaPrefixData; import net.ripe.rpki.server.api.support.objects.ValueObjectSupport; +import java.time.Instant; import java.util.Comparator; import static com.google.common.base.Objects.*; @@ -29,20 +32,7 @@ * ... * */ -public class RoaConfigurationPrefixData extends ValueObjectSupport { - - public static final Comparator COMPARATOR = (a, b) -> { - int rc = a.getAsn().compareTo(b.getAsn()); - if (rc != 0) { - return rc; - } - rc = a.getPrefix().compareTo(b.getPrefix()); - if (rc != 0) { - return rc; - } - return -Integer.compare(a.getMaximumLength(), b.getMaximumLength()); - }; - +public class RoaConfigurationPrefixData extends ValueObjectSupport implements RoaPrefixData { // cycle when serialised using default serialiser @Getter @JsonSerialize(using = ToStringSerializer.class) @@ -55,11 +45,29 @@ public class RoaConfigurationPrefixData extends ValueObjectSupport { private final Integer maximumLength; + /** + * The point in time this prefix was last updated. + * + * No object can have a updatedAt before the point in time this feature was added. + */ + @Getter + @JsonInclude(JsonInclude.Include.NON_NULL) + private final Instant updatedAt; + + public RoaConfigurationPrefixData(Asn asn, RoaPrefix roaPrefix, Instant updatedAt) { + this.asn = checkNotNull(asn, "asn is required"); + checkNotNull(roaPrefix, "roaPrefix is required"); + this.prefix = roaPrefix.getPrefix(); + this.maximumLength = roaPrefix.getMaximumLength(); + this.updatedAt = updatedAt; + } + public RoaConfigurationPrefixData(Asn asn, RoaPrefix roaPrefix) { this.asn = checkNotNull(asn, "asn is required"); checkNotNull(roaPrefix, "roaPrefix is required"); this.prefix = roaPrefix.getPrefix(); this.maximumLength = roaPrefix.getMaximumLength(); + this.updatedAt = null; } public RoaConfigurationPrefixData(Asn asn, IpRange prefix, Integer maximumLength) { @@ -67,6 +75,11 @@ public RoaConfigurationPrefixData(Asn asn, IpRange prefix, Integer maximumLength this(asn, new RoaPrefix(prefix, maximumLength)); } + public RoaConfigurationPrefixData(Asn asn, IpRange prefix, Integer maximumLength, Instant updatedAt) { + // Ensure correct validation of prefix and maximum length by using RoaPrefix constructor. + this(asn, new RoaPrefix(prefix, maximumLength), updatedAt); + } + @JsonIgnore public RoaPrefix getRoaPrefix() { return new RoaPrefix(prefix, maximumLength); @@ -77,10 +90,6 @@ public int getMaximumLength() { return maximumLength == null ? getPrefix().getPrefixLength() : maximumLength; } - public Integer getNullableMaxLength() { - return maximumLength; - } - @Override public int hashCode() { final int prime = 31; diff --git a/src/main/java/net/ripe/rpki/server/api/services/read/BgpRisEntryViewService.java b/src/main/java/net/ripe/rpki/server/api/services/read/BgpRisEntryViewService.java index fcc482b..3f7ff0a 100644 --- a/src/main/java/net/ripe/rpki/server/api/services/read/BgpRisEntryViewService.java +++ b/src/main/java/net/ripe/rpki/server/api/services/read/BgpRisEntryViewService.java @@ -13,6 +13,8 @@ public interface BgpRisEntryViewService { /** * @return all matching BGP RIS entries that do not have a more specific matching entry. + * Take care when using this method: it does not consistently return a covering less specific. + * TODO: Clarify the behaviour of this method. */ Collection findMostSpecificOverlapping(ImmutableResourceSet resources); diff --git a/src/main/java/net/ripe/rpki/services/impl/RoaAlertChecker.java b/src/main/java/net/ripe/rpki/services/impl/RoaAlertChecker.java index a24d381..2a3f986 100644 --- a/src/main/java/net/ripe/rpki/services/impl/RoaAlertChecker.java +++ b/src/main/java/net/ripe/rpki/services/impl/RoaAlertChecker.java @@ -7,14 +7,8 @@ import net.ripe.ipresource.IpResource; import net.ripe.ipresource.ImmutableResourceSet; import net.ripe.ipresource.etree.NestedIntervalMap; -import net.ripe.rpki.commons.validation.roa.AllowedRoute; -import net.ripe.rpki.commons.validation.roa.AnnouncedRoute; -import net.ripe.rpki.commons.validation.roa.RouteOriginValidationPolicy; -import net.ripe.rpki.commons.validation.roa.RouteValidityState; -import net.ripe.rpki.server.api.dto.BgpRisEntry; -import net.ripe.rpki.server.api.dto.CertificateAuthorityData; -import net.ripe.rpki.server.api.dto.RoaAlertConfigurationData; -import net.ripe.rpki.server.api.dto.RoaConfigurationData; +import net.ripe.rpki.commons.validation.roa.*; +import net.ripe.rpki.server.api.dto.*; import net.ripe.rpki.server.api.ports.InternalNamePresenter; import net.ripe.rpki.server.api.services.read.BgpRisEntryViewService; import net.ripe.rpki.server.api.services.read.RoaViewService; @@ -105,15 +99,14 @@ public void checkAndSendRoaAlertEmailToSubscription(RoaAlertConfigurationData co AnnouncedRoutes getAnnouncedRoutesForCA(CertificateAuthorityData ca, Set ignoredAnnouncements) { RoaConfigurationData roaConfiguration = roaService.getRoaConfiguration(ca.getId()); Collection announcements = bgpRisEntryRepository.findMostSpecificOverlapping(ca.getResources()); - RouteOriginValidationPolicy policy = new RouteOriginValidationPolicy(); - NestedIntervalMap> allowedRoutes = allowedRoutesToNestedIntervalMap(roaConfiguration.toAllowedRoutes()); + NestedIntervalMap> allowedRoutes = allowedRoutesToNestedIntervalMap(roaConfiguration.getPrefixes()); AnnouncedRoutes announcedRoutes = new AnnouncedRoutes(); announcements.stream().map(BgpRisEntry::toAnnouncedRoute) .filter(x -> !ignoredAnnouncements.contains(x)) .forEach(announcedRoute -> { - RouteValidityState validityState = policy.validateAnnouncedRoute(allowedRoutes, announcedRoute); + RouteValidityState validityState = RouteOriginValidationPolicy.validateAnnouncedRoute(allowedRoutes, announcedRoute); switch (validityState) { case VALID: announcedRoutes.valids.add(announcedRoute); @@ -139,11 +132,11 @@ private void sendRoaAlertEmailToSubscription(RoaAlertConfigurationData configura List unknowns, Set unsortedIgnoredAlerts) { String humanizedCaName = internalNamePresenter.humanizeCaName(configuration.getCertificateAuthority().getName()); - invalidAsnsToMail.sort(AnnouncedRoute.ASN_PREFIX_COMPARATOR); - invalidLengthsToMail.sort(AnnouncedRoute.ASN_PREFIX_COMPARATOR); - unknowns.sort(AnnouncedRoute.ASN_PREFIX_COMPARATOR); + Collections.sort(invalidAsnsToMail); + Collections.sort(invalidLengthsToMail); + Collections.sort(unknowns); - final SortedSet ignoredAlerts = new TreeSet<>(AnnouncedRoute.ASN_PREFIX_COMPARATOR); + final SortedSet ignoredAlerts = new TreeSet<>(); ignoredAlerts.addAll(unsortedIgnoredAlerts); var parameters = Map.of( diff --git a/src/main/resources/db/migration/V130__roaconfiguration_prefixes__updated_at.sql b/src/main/resources/db/migration/V130__roaconfiguration_prefixes__updated_at.sql new file mode 100644 index 0000000..b30b5f7 --- /dev/null +++ b/src/main/resources/db/migration/V130__roaconfiguration_prefixes__updated_at.sql @@ -0,0 +1,3 @@ +ALTER TABLE roaconfiguration_prefixes ADD COLUMN updated_at TIMESTAMP WITH TIME ZONE DEFAULT now(); +--- Old records do not have a updated_at value +UPDATE roaconfiguration_prefixes SET updated_at = NULL; \ No newline at end of file diff --git a/src/test/java/net/ripe/rpki/bgpris/BgpRisEntryRepositoryBeanTest.java b/src/test/java/net/ripe/rpki/bgpris/BgpRisEntryRepositoryBeanTest.java index 493efed..e6c523b 100644 --- a/src/test/java/net/ripe/rpki/bgpris/BgpRisEntryRepositoryBeanTest.java +++ b/src/test/java/net/ripe/rpki/bgpris/BgpRisEntryRepositoryBeanTest.java @@ -4,85 +4,104 @@ import net.ripe.ipresource.IpRange; import net.ripe.ipresource.ImmutableResourceSet; import net.ripe.rpki.server.api.dto.BgpRisEntry; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; import java.util.Arrays; import java.util.Collection; import java.util.HashSet; import java.util.Map; -import static org.junit.Assert.*; +import static org.assertj.core.api.Assertions.assertThat; -public class BgpRisEntryRepositoryBeanTest { +class BgpRisEntryRepositoryBeanTest { private static final BgpRisEntry BGP_RIS_ENTRY_IPV6_LARGE = new BgpRisEntry(Asn.parse("65000"), IpRange.parse("ff00::/11"), 20); private static final BgpRisEntry BGP_RIS_ENTRY_IPV4_LARGE = new BgpRisEntry(Asn.parse("65000"), IpRange.parse("6.0.0.0/7"), 20); private static final BgpRisEntry BGP_RIS_ENTRY_BELOW_TRESHOLD = new BgpRisEntry(Asn.parse("65000"), IpRange.parse("10.0.0.0/8"), 1); + + private static final BgpRisEntry BGP_RIS_ENTRY_128_1 = new BgpRisEntry(Asn.parse("3333"), IpRange.parse("128.0.0.0/1"), 5); private static final BgpRisEntry BGP_RIS_ENTRY_193_8 = new BgpRisEntry(Asn.parse("3333"), IpRange.parse("193.0.0.0/8"), 5); private static final BgpRisEntry BGP_RIS_ENTRY_FFCE_16 = new BgpRisEntry(Asn.parse("3333"), IpRange.parse("ffce::/16"), 6); private static final BgpRisEntry BGP_RIS_ENTRY_193_16 = new BgpRisEntry(Asn.parse("65535"), IpRange.parse("193.16.0.0/16"), 5); private BgpRisEntryRepositoryBean subject; - @Before + @BeforeEach public void setup() { subject = new BgpRisEntryRepositoryBean(); subject.resetEntries(Arrays.asList(BGP_RIS_ENTRY_BELOW_TRESHOLD, BGP_RIS_ENTRY_193_8, BGP_RIS_ENTRY_FFCE_16, BGP_RIS_ENTRY_IPV6_LARGE, BGP_RIS_ENTRY_IPV4_LARGE)); } @Test - public void should_find_entries_with_overlapping_prefixes() { - assertEquals(entries(BGP_RIS_ENTRY_193_8), subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("0.0.0.0/0"))); - assertEquals(entries(BGP_RIS_ENTRY_193_8), subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("192.0.0.0-193.0.0.1"))); - assertEquals(entries(BGP_RIS_ENTRY_FFCE_16), subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("ffce:abcd::/32"))); - assertEquals(entries(BGP_RIS_ENTRY_193_8, BGP_RIS_ENTRY_FFCE_16), subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("0.0.0.0/0, ffce:abcd::/32"))); + void should_find_entries_with_overlapping_prefixes() { + assertThat(subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("0.0.0.0/0"))).containsOnly(BGP_RIS_ENTRY_193_8); + assertThat(subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("192.0.0.0-193.0.0.1"))).containsOnly(BGP_RIS_ENTRY_193_8); + assertThat(subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("ffce:abcd::/32"))).containsOnly(BGP_RIS_ENTRY_FFCE_16); + assertThat(subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("0.0.0.0/0, ffce:abcd::/32"))).containsOnly(BGP_RIS_ENTRY_193_8, BGP_RIS_ENTRY_FFCE_16); } @Test - public void should_skip_entries_below_threshold() { - assertEquals(entries(), subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("10.0.0.0/8"))); + void should_skip_entries_below_threshold() { + assertThat(subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("10.0.0.0/8"))).isEmpty(); } @Test - public void should_skip_large_prefixes() { - assertEquals(entries(), subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("6.0.0.0/8"))); - assertEquals(entries(), subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("ff00::/12"))); + void should_skip_large_prefixes() { + assertThat(subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("6.0.0.0/8"))).isEmpty(); + assertThat(subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("ff00::/12"))).isEmpty(); } @Test - public void should_skip_entries_without_overlapping_prefixes() { - assertEquals(entries(), subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("9.0.0.0/8"))); + void should_skip_entries_without_overlapping_prefixes() { + assertThat(subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("9.0.0.0/8"))).isEmpty(); } @Test - public void should_not_match_entries_by_origin_asn() { - assertEquals(entries(), subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("AS3333"))); + void should_not_match_entries_by_origin_asn() { + assertThat(subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("AS3333"))).isEmpty(); } @Test - public void should_only_match_most_specific_routing_entries_when_resources_are_full_covered() { - subject.resetEntries(entries(BGP_RIS_ENTRY_193_16, BGP_RIS_ENTRY_193_8)); - assertEquals(entries(BGP_RIS_ENTRY_193_16), subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("193.16.0.0/17"))); + void should_only_match_most_specific_less_specific_when_resources_are_full_covered() { + subject.resetEntries(entries(BGP_RIS_ENTRY_193_16, BGP_RIS_ENTRY_193_8, BGP_RIS_ENTRY_128_1)); + assertThat(subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("193.16.0.0/17"))).containsOnly(BGP_RIS_ENTRY_193_16); } @Test - public void should_match_all_routing_entries_when_most_specific_do_not_fully_cover_resources() { - subject.resetEntries(entries(BGP_RIS_ENTRY_193_16, BGP_RIS_ENTRY_193_8)); - assertEquals(entries(BGP_RIS_ENTRY_193_8, BGP_RIS_ENTRY_193_16), subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("193.16.0.0/15"))); + void should_match_all_routing_entries_when_most_specific_do_not_fully_cover_resources() { + subject.resetEntries(entries(BGP_RIS_ENTRY_193_16, BGP_RIS_ENTRY_193_8, BGP_RIS_ENTRY_128_1)); + assertThat(subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("193.16.0.0/15"))).containsOnly(BGP_RIS_ENTRY_193_8, BGP_RIS_ENTRY_193_16); + } + + @Test + void should_match_all_more_specifics() { + subject.resetEntries(entries(BGP_RIS_ENTRY_193_16, BGP_RIS_ENTRY_193_8, BGP_RIS_ENTRY_128_1)); + assertThat(subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("193.0.0.0/8"))).containsOnly(BGP_RIS_ENTRY_193_8, BGP_RIS_ENTRY_193_16); } + /** + * findMostSpecificOverlapping is not consistent: As an API user I would expect the first level less-specific to be + * returned - but is not. + */ + @Disabled @Test - public void should_split_in_contained_and_not_contained() { + void should_include_first_level_less_specifics() { + subject.resetEntries(entries(BGP_RIS_ENTRY_193_16, BGP_RIS_ENTRY_193_8, BGP_RIS_ENTRY_128_1)); + assertThat(subject.findMostSpecificOverlapping(ImmutableResourceSet.parse("192.0.0.0/7"))).containsOnly(BGP_RIS_ENTRY_193_8, BGP_RIS_ENTRY_193_16, BGP_RIS_ENTRY_128_1); + } + + @Test + void should_split_in_contained_and_not_contained() { subject.resetEntries(entries(BGP_RIS_ENTRY_193_16, BGP_RIS_ENTRY_193_8)); Map> result = subject.findMostSpecificContainedAndNotContained(ImmutableResourceSet.parse("193.16.0.0/15")); - assertEquals(entries(BGP_RIS_ENTRY_193_16), result.get(true)); - assertEquals(entries(BGP_RIS_ENTRY_193_8), result.get(false)); - + assertThat(result.get(true)).containsOnly(BGP_RIS_ENTRY_193_16); + assertThat(result.get(false)).containsOnly(BGP_RIS_ENTRY_193_8); } @Test - public void should_find_less_specific_only_if_resources_remain_after_exact_and_more_specific() { + void should_find_less_specific_only_if_resources_remain_after_exact_and_more_specific() { /* * Bug reported: * @@ -109,11 +128,12 @@ public void should_find_less_specific_only_if_resources_remain_after_exact_and_m BgpRisEntryRepositoryBean bgpRisEntryRepositoryBean = new BgpRisEntryRepositoryBean(); bgpRisEntryRepositoryBean.resetEntries(Arrays.asList(BGP_96_22, BGP_98_24, BGP_100_24, BGP_101_24, BGP_91_8)); - assertEquals(entries(BGP_96_22, BGP_98_24, BGP_100_24, BGP_101_24), bgpRisEntryRepositoryBean.findMostSpecificOverlapping(ImmutableResourceSet.parse("91.194.96.0-91.194.101.255"))); + assertThat(bgpRisEntryRepositoryBean.findMostSpecificOverlapping(ImmutableResourceSet.parse("91.194.96.0-91.194.101.255"))) + .containsOnly(BGP_96_22, BGP_98_24, BGP_100_24, BGP_101_24); } @Test - public void should_suppport_multiple_asns_for_the_same_prefix() { + void should_suppport_multiple_asns_for_the_same_prefix() { /* * Bug reported: * @@ -143,8 +163,8 @@ public void should_suppport_multiple_asns_for_the_same_prefix() { final Map> announcements = bgpRisEntryRepositoryBean .findMostSpecificContainedAndNotContained(ImmutableResourceSet.parse("176.97.158.0/24, 2001:67c:10b8::/48")); - assertEquals(4, announcements.get(true).size()); - assertEquals(0, announcements.get(false).size()); + assertThat(announcements.get(true)).hasSize(4); + assertThat(announcements.get(false)).hasSize(0); } private static Collection entries(BgpRisEntry... entries) { diff --git a/src/test/java/net/ripe/rpki/rest/service/UtilsTest.java b/src/test/java/net/ripe/rpki/rest/service/UtilsTest.java index 7e2c3cb..18303e3 100644 --- a/src/test/java/net/ripe/rpki/rest/service/UtilsTest.java +++ b/src/test/java/net/ripe/rpki/rest/service/UtilsTest.java @@ -1,7 +1,7 @@ package net.ripe.rpki.rest.service; import net.ripe.ipresource.IpRange; -import net.ripe.rpki.rest.pojo.ROA; +import net.ripe.rpki.rest.pojo.ApiRoaPrefix; import org.junit.Test; import static net.ripe.rpki.rest.service.Utils.errorsInUserInputRoas; @@ -13,7 +13,7 @@ public class UtilsTest { @Test public void shouldCheckMaxLength() { - assertThat(errorsInUserInputRoas(new ROA("AS65536", "192.0.2.0/24", 23))) + assertThat(errorsInUserInputRoas(new ApiRoaPrefix("AS65536", "192.0.2.0/24", 23))) .hasValue("Max length '23' must be between 24 and 32 for prefix '192.0.2.0/24'"); } @@ -27,7 +27,7 @@ public void shouldAcceptFittingMaxLength() { @Test public void shouldRejectMissingMaxLength() { - assertThat(errorsInUserInputRoas(new ROA("AS65536", "192.0.2.0/24", null))) + assertThat(errorsInUserInputRoas(new ApiRoaPrefix("AS65536", "192.0.2.0/24", null))) .hasValue("Max length must be specified and must be between 24 and 32 for prefix '192.0.2.0/24'"); }