From 2ee18082ba77838fe40f4165a884f4ce41a3bbe9 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 7 Jun 2022 17:31:37 +0200 Subject: [PATCH 1/4] Parse maven-metadata.xml instead of html --- .../elastic/apm/attach/AgentDownloader.java | 15 ++-- .../apm/attach/AgentDownloaderTest.java | 4 +- .../MavenCentral_agent_artifact.html | 84 ------------------- .../src/test/resources/maven-metadata.xml | 65 ++++++++++++++ 4 files changed, 74 insertions(+), 94 deletions(-) delete mode 100644 apm-agent-attach-cli/src/test/resources/MavenCentral_agent_artifact.html create mode 100644 apm-agent-attach-cli/src/test/resources/maven-metadata.xml diff --git a/apm-agent-attach-cli/src/main/java/co/elastic/apm/attach/AgentDownloader.java b/apm-agent-attach-cli/src/main/java/co/elastic/apm/attach/AgentDownloader.java index 41cabbcff8..587fa6a3dc 100644 --- a/apm-agent-attach-cli/src/main/java/co/elastic/apm/attach/AgentDownloader.java +++ b/apm-agent-attach-cli/src/main/java/co/elastic/apm/attach/AgentDownloader.java @@ -43,7 +43,7 @@ */ public class AgentDownloader { - private static final String VERSION_EXTRACTION_REGEX = "([0-9]+.[0-9.]+.[0-9.]+)/"; + private static final Pattern VERSION_EXTRACTION_REGEX = Pattern.compile("([0-9]+.[0-9.]+.[0-9.]+)"); private static final String AGENT_GROUP_ID = "co.elastic.apm"; private static final String AGENT_ARTIFACT_ID = "elastic-apm-agent"; private static final String CLI_JAR_VERSION; @@ -220,23 +220,22 @@ void verifyPgpSignature(final Path agentJarFile, final String mavenAgentUrlStrin } static String findLatestVersion() throws Exception { - String agentArtifactMavenBaseUrl = getAgentArtifactMavenBaseUrl(); - HttpURLConnection httpURLConnection = openConnection(agentArtifactMavenBaseUrl); - TreeSet versions = parseMavenArtifactHtml(httpURLConnection.getInputStream()); + String agentArtifactMavenMetadatUrl = getAgentArtifactMavenBaseUrl() + "/maven-metadata.xml"; + HttpURLConnection httpURLConnection = openConnection(agentArtifactMavenMetadatUrl); + TreeSet versions = parseMavenMetadataXml(httpURLConnection.getInputStream()); if (versions.isEmpty()) { - throw new IllegalStateException("Failed to parse agent versions from the contents of " + agentArtifactMavenBaseUrl); + throw new IllegalStateException("Failed to parse agent versions from the contents of " + agentArtifactMavenMetadatUrl); } return versions.last().toString(); } - static TreeSet parseMavenArtifactHtml(InputStream htmlInputStream) throws IOException { + static TreeSet parseMavenMetadataXml(InputStream htmlInputStream) throws IOException { TreeSet versions = new TreeSet<>(); BufferedReader versionsHtmlReader = new BufferedReader(new InputStreamReader(htmlInputStream)); - Pattern pattern = Pattern.compile(VERSION_EXTRACTION_REGEX); String line; while ((line = versionsHtmlReader.readLine()) != null) { try { - Matcher matcher = pattern.matcher(line); + Matcher matcher = VERSION_EXTRACTION_REGEX.matcher(line); if (matcher.find()) { versions.add(Version.of(matcher.group(1))); } diff --git a/apm-agent-attach-cli/src/test/java/co/elastic/apm/attach/AgentDownloaderTest.java b/apm-agent-attach-cli/src/test/java/co/elastic/apm/attach/AgentDownloaderTest.java index c47815d55a..f0376f5161 100644 --- a/apm-agent-attach-cli/src/test/java/co/elastic/apm/attach/AgentDownloaderTest.java +++ b/apm-agent-attach-cli/src/test/java/co/elastic/apm/attach/AgentDownloaderTest.java @@ -104,8 +104,8 @@ void testLatestVersion() throws Exception { @Test void testAgentArtifactMavenPageParsing() throws IOException { - TreeSet versions = AgentDownloader.parseMavenArtifactHtml(AgentDownloaderTest.class.getResourceAsStream( - "/MavenCentral_agent_artifact.html")); + TreeSet versions = AgentDownloader.parseMavenMetadataXml(AgentDownloaderTest.class.getResourceAsStream( + "/maven-metadata.xml")); assertThat(versions).hasSize(50); assertThat(versions.first().toString()).isEqualTo("0.5.1"); assertThat(versions.last().toString()).isEqualTo("1.31.0"); diff --git a/apm-agent-attach-cli/src/test/resources/MavenCentral_agent_artifact.html b/apm-agent-attach-cli/src/test/resources/MavenCentral_agent_artifact.html deleted file mode 100644 index b308616894..0000000000 --- a/apm-agent-attach-cli/src/test/resources/MavenCentral_agent_artifact.html +++ /dev/null @@ -1,84 +0,0 @@ - - - - Central Repository: co/elastic/apm/elastic-apm-agent - - - - - -
-

co/elastic/apm/elastic-apm-agent

-
-
-
- 
../
-0.5.1/                                            2018-05-28 21:16         -      
-0.6.0/                                            2018-07-05 08:26         -      
-0.6.1/                                            2018-08-07 09:37         -      
-0.6.2/                                            2018-08-13 12:30         -      
-0.7.0/                                            2018-09-12 12:57         -      
-0.7.1/                                            2018-10-24 08:55         -      
-1.0.0/                                            2018-11-14 14:46         -      
-1.0.0.RC1/                                        2018-11-06 09:17         -      
-1.0.1/                                            2018-11-15 16:22         -      
-1.1.0/                                            2018-11-27 13:55         -      
-1.10.0/                                           2019-09-30 13:00         -      
-1.10.1/                                           2019-10-31 14:30         -      
-1.11.0/                                           2019-10-31 16:15         -      
-1.12.0/                                           2019-11-21 16:26         -      
-1.13.0/                                           2020-02-10 20:47         -      
-1.14.0/                                           2020-03-04 11:04         -      
-1.15.0/                                           2020-03-28 15:18         -      
-1.16.0/                                           2020-05-13 09:07         -      
-1.17.0/                                           2020-06-17 09:24         -      
-1.18.0/                                           2020-09-08 08:07         -      
-1.18.0.RC1/                                       2020-07-22 15:53         -      
-1.18.1/                                           2020-10-06 11:39         -      
-1.19.0/                                           2020-11-10 20:23         -      
-1.2.0/                                            2018-12-19 08:48         -      
-1.20.0/                                           2021-01-07 19:28         -      
-1.21.0/                                           2021-02-09 12:59         -      
-1.22.0/                                           2021-03-24 11:18         -      
-1.23.0/                                           2021-04-22 10:25         -      
-1.24.0/                                           2021-05-31 09:50         -      
-1.25.0/                                           2021-07-22 07:18         -      
-1.26.0/                                           2021-09-14 10:49         -      
-1.26.1/                                           2021-12-22 16:45         -      
-1.26.2/                                           2021-12-30 16:43         -      
-1.27.0/                                           2021-11-15 15:56         -      
-1.27.1/                                           2021-11-30 15:10         -      
-1.28.0/                                           2021-12-07 13:14         -      
-1.28.1/                                           2021-12-10 15:00         -      
-1.28.2/                                           2021-12-16 14:45         -      
-1.28.3/                                           2021-12-22 13:18         -      
-1.28.4/                                           2021-12-30 15:33         -      
-1.29.0/                                           2022-02-09 09:30         -      
-1.3.0/                                            2019-01-10 14:21         -      
-1.30.0/                                           2022-03-22 10:47         -      
-1.30.1/                                           2022-04-12 12:15         -      
-1.31.0/                                           2022-05-17 06:37         -
-1.32.0-RC1/                               2022-05-17 06:37         -
-1.32.0.RC1/                               2022-05-17 06:37         -
-1.4.0/                                            2019-02-14 09:09         -
-1.5.0/                                            2019-03-26 10:45         -      
-1.6.0/                                            2019-04-16 14:01         -      
-1.6.1/                                            2019-04-26 09:04         -      
-1.7.0/                                            2019-06-13 08:33         -      
-1.8.0/                                            2019-07-30 13:42         -      
-1.9.0/                                            2019-08-22 10:18         -      
-maven-metadata.xml                                2022-05-17 06:48      1962      
-maven-metadata.xml.md5                            2022-05-17 06:48        32      
-maven-metadata.xml.sha1                           2022-05-17 06:48        40      
-maven-metadata.xml.sha256                         2022-05-17 06:48        64      
-maven-metadata.xml.sha512                         2022-05-17 06:48       128      
-
-
-
- - - diff --git a/apm-agent-attach-cli/src/test/resources/maven-metadata.xml b/apm-agent-attach-cli/src/test/resources/maven-metadata.xml new file mode 100644 index 0000000000..3014b4cd58 --- /dev/null +++ b/apm-agent-attach-cli/src/test/resources/maven-metadata.xml @@ -0,0 +1,65 @@ + + co.elastic.apm + elastic-apm-agent + + 1.31.0 + 1.31.0 + + 0.5.1 + 0.6.0 + 0.6.1 + 0.6.2 + 0.7.0 + 0.7.1 + 1.0.0.RC1 + 1.0.0 + 1.0.1 + 1.1.0 + 1.2.0 + 1.3.0 + 1.4.0 + 1.5.0 + 1.6.0 + 1.6.1 + 1.7.0 + 1.8.0 + 1.9.0 + 1.10.0 + 1.10.1 + 1.11.0 + 1.12.0 + 1.13.0 + 1.14.0 + 1.15.0 + 1.16.0 + 1.17.0 + 1.18.0.RC1 + 1.18.0 + 1.18.1 + 1.19.0 + 1.20.0 + 1.21.0 + 1.22.0 + 1.23.0 + 1.24.0 + 1.25.0 + 1.26.0 + 1.26.1 + 1.26.2 + 1.27.0 + 1.27.1 + 1.28.0 + 1.28.1 + 1.28.2 + 1.28.3 + 1.28.4 + 1.29.0 + 1.30.0 + 1.30.1 + 1.31.0 + 1.32.0-RC1 + 1.32.0.RC1 + + 20220517064838 + + From db4839f47e1dc822da07b65c386961a54eec575c Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 7 Jun 2022 19:56:19 +0200 Subject: [PATCH 2/4] Refactor of Version class --- .../elastic/apm/attach/AgentDownloader.java | 7 +- .../apm/attach/AgentDownloaderTest.java | 1 + .../co/elastic/apm/agent/util/Version.java | 80 ++++++++++++------- .../elastic/apm/agent/util/VersionTest.java | 54 ++++++++----- .../agent/report/ApmServerHealthChecker.java | 4 +- .../apm/agent/report/ApmServerClientTest.java | 2 +- 6 files changed, 98 insertions(+), 50 deletions(-) diff --git a/apm-agent-attach-cli/src/main/java/co/elastic/apm/attach/AgentDownloader.java b/apm-agent-attach-cli/src/main/java/co/elastic/apm/attach/AgentDownloader.java index 587fa6a3dc..bf6c435f53 100644 --- a/apm-agent-attach-cli/src/main/java/co/elastic/apm/attach/AgentDownloader.java +++ b/apm-agent-attach-cli/src/main/java/co/elastic/apm/attach/AgentDownloader.java @@ -43,7 +43,7 @@ */ public class AgentDownloader { - private static final Pattern VERSION_EXTRACTION_REGEX = Pattern.compile("([0-9]+.[0-9.]+.[0-9.]+)"); + private static final Pattern VERSION_EXTRACTION_REGEX = Pattern.compile("(.+?)"); private static final String AGENT_GROUP_ID = "co.elastic.apm"; private static final String AGENT_ARTIFACT_ID = "elastic-apm-agent"; private static final String CLI_JAR_VERSION; @@ -237,7 +237,10 @@ static TreeSet parseMavenMetadataXml(InputStream htmlInputStream) throw try { Matcher matcher = VERSION_EXTRACTION_REGEX.matcher(line); if (matcher.find()) { - versions.add(Version.of(matcher.group(1))); + Version version = Version.of(matcher.group(1)); + if (!version.hasSuffix()) { + versions.add(version); + } } } catch (Exception e) { // ignore, probably a regex false positive diff --git a/apm-agent-attach-cli/src/test/java/co/elastic/apm/attach/AgentDownloaderTest.java b/apm-agent-attach-cli/src/test/java/co/elastic/apm/attach/AgentDownloaderTest.java index f0376f5161..ac9a08fa21 100644 --- a/apm-agent-attach-cli/src/test/java/co/elastic/apm/attach/AgentDownloaderTest.java +++ b/apm-agent-attach-cli/src/test/java/co/elastic/apm/attach/AgentDownloaderTest.java @@ -109,5 +109,6 @@ void testAgentArtifactMavenPageParsing() throws IOException { assertThat(versions).hasSize(50); assertThat(versions.first().toString()).isEqualTo("0.5.1"); assertThat(versions.last().toString()).isEqualTo("1.31.0"); + assertThat(versions).doesNotContain(Version.of("1.0.0.RC1")); } } diff --git a/apm-agent-common/src/main/java/co/elastic/apm/agent/util/Version.java b/apm-agent-common/src/main/java/co/elastic/apm/agent/util/Version.java index 66e0f65844..734dfe3485 100644 --- a/apm-agent-common/src/main/java/co/elastic/apm/agent/util/Version.java +++ b/apm-agent-common/src/main/java/co/elastic/apm/agent/util/Version.java @@ -18,46 +18,50 @@ */ package co.elastic.apm.agent.util; +import java.util.Arrays; +import java.util.Objects; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + /** * Based on https://gist.github.com/brianguertin. * This code was released into the public domain by Brian Guertin on July 8, 2016 citing, verbatim the unlicense. */ public class Version implements Comparable { - public static final Version UNKNOWN_VERSION = of("1.0.0"); + private static final Version INVALID = new Version(new int[0], ""); + + private static final Pattern VERSION_REGEX = Pattern.compile("^" + + "(?.*?)" + + "(?(\\d+)(\\.\\d+)*)" + + "(?.*?)" + + "$"); private final int[] numbers; + private final String suffix; public static Version of(String version) { - return new Version(version); - } - - private Version(String version) { - int indexOfDash = version.indexOf('-'); - int indexOfFirstDot = version.indexOf('.'); - if (indexOfDash > 0 && indexOfDash < indexOfFirstDot) { - version = version.substring(indexOfDash + 1); + Matcher matcher = VERSION_REGEX.matcher(version); + if (!matcher.find()) { + return INVALID; } - indexOfDash = version.indexOf('-'); - int indexOfLastDot = version.lastIndexOf('.'); - if (indexOfDash > 0 && indexOfDash > indexOfLastDot) { - version = version.substring(0, indexOfDash); + final String[] parts = matcher.group("version").split("\\."); + int[] numbers = new int[parts.length]; + for (int i = 0; i < parts.length; i++) { + numbers[i] = Integer.parseInt(parts[i]); } - final String[] parts = version.split("\\."); - int[] tmp = new int[parts.length]; - int validPartsIndex = 0; - for (String part : parts) { - try { - tmp[validPartsIndex] = Integer.valueOf(part); - validPartsIndex++; - } catch (NumberFormatException numberFormatException) { - // continue - } - } - numbers = new int[validPartsIndex]; - if (numbers.length > 0) { - System.arraycopy(tmp, 0, numbers, 0, numbers.length); + + String suffixTmp = matcher.group("suffix"); + if (suffixTmp == null) { + suffixTmp = ""; } + String suffix = suffixTmp; + return new Version(numbers, suffix); + } + + private Version(int[] numbers, String suffix) { + this.numbers = numbers; + this.suffix = suffix; } @Override @@ -70,7 +74,7 @@ public int compareTo(Version another) { return left < right ? -1 : 1; } } - return 0; + return another.suffix.compareTo(suffix); } @Override @@ -82,6 +86,26 @@ public String toString() { sb.append('.'); } } + sb.append(suffix); return sb.toString(); } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Version version = (Version) o; + return Arrays.equals(numbers, version.numbers) && suffix.equals(version.suffix); + } + + @Override + public int hashCode() { + int result = Objects.hash(suffix); + result = 31 * result + Arrays.hashCode(numbers); + return result; + } + + public boolean hasSuffix() { + return suffix.length() > 0; + } } diff --git a/apm-agent-common/src/test/java/co/elastic/apm/agent/util/VersionTest.java b/apm-agent-common/src/test/java/co/elastic/apm/agent/util/VersionTest.java index ff18c294c9..50ccf9b08d 100644 --- a/apm-agent-common/src/test/java/co/elastic/apm/agent/util/VersionTest.java +++ b/apm-agent-common/src/test/java/co/elastic/apm/agent/util/VersionTest.java @@ -19,31 +19,49 @@ package co.elastic.apm.agent.util; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import static org.assertj.core.api.Assertions.assertThat; class VersionTest { - @Test - void testVersion() { - assertThat(Version.of("1.2.3")).isEqualByComparingTo(Version.of("1.2.3")); - assertThat(Version.of("1.2.3-SNAPSHOT")).isEqualByComparingTo(Version.of("1.2.3")); - assertThat(Version.of("4.5.13")).isEqualByComparingTo(Version.of("4.5.13")); - assertThat(Version.of("4.5.13.redhat-00001")).isEqualByComparingTo(Version.of("4.5.13")); - assertThat(Version.of("httpclient-4.5.13")).isEqualByComparingTo(Version.of("4.5.13")); - assertThat(Version.of("httpclient-4.5.13.redhat-00001")).isEqualByComparingTo(Version.of("4.5.13")); - assertThat(Version.of("httpclient.4.5.13.redhat-00001")).isEqualByComparingTo(Version.of("4.5.13")); - assertThat(Version.of("httpclient.4.5.13.redhat")).isEqualByComparingTo(Version.of("4.5.13")); - assertThat(Version.of("httpclient.4.5.13-redhat")).isEqualByComparingTo(Version.of("4.5.13")); + @ParameterizedTest + @CsvSource(value = { + "1.0.0, <, 1.0.1", + "1.2.3, =, 1.2.3", + "1.2.3-SNAPSHOT, <, 1.2.3", + "4.5.13, =, 4.5.13", + "4.5.13.redhat-00001, <, 4.5.13", + "httpclient-4.5.13, =, 4.5.13", + "httpclient-4.5.13.redhat-00001, <, 4.5.13", + "httpclient.4.5.13.redhat-00001, =, 4.5.13.redhat-00001", + "httpclient.4.5.13.redhat, <, 4.5.13", + "httpclient.4.5.13-redhat, =, 4.5.13-redhat", + "1, =, 1", + "1, <, 1.2", + "1.2, =, 1.2", + "1.2.3, =, 1.2.3", + "1.2.3.4, =, 1.2.3.4", + "ignore.1.2.3-pre1, =, 1.2.3-pre1" + }) + void testVersion(String version1, String operator, String version2) { + switch (operator) { + case "<": + assertThat(Version.of(version1)).isLessThan(Version.of(version2)); + break; + case "=": + assertThat(Version.of(version1)).isEqualByComparingTo(Version.of(version2)); + assertThat(Version.of(version1)).isEqualTo(Version.of(version2)); + assertThat(Version.of(version1).toString()).isEqualTo(Version.of(version2).toString()); + break; + default: + throw new IllegalArgumentException("Invalid operator: " + operator); + } } @Test - void testToString() { - System.out.println(Version.of("")); - System.out.println(Version.of("1")); - System.out.println(Version.of("1.2")); - System.out.println(Version.of("1.2.3")); - System.out.println(Version.of("1.2.3.4")); - System.out.println(Version.of("1.2.3.ignore")); + void testEmptyVersion() { + assertThat(Version.of("").toString()).isEqualTo(""); } } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerHealthChecker.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerHealthChecker.java index bc86ba96dd..f0f3213800 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerHealthChecker.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/report/ApmServerHealthChecker.java @@ -40,6 +40,8 @@ import static java.nio.charset.StandardCharsets.UTF_8; public class ApmServerHealthChecker implements Callable { + + public static final Version UNKNOWN_VERSION = Version.of("1.0.0"); private static final Logger logger = LoggerFactory.getLogger(ApmServerHealthChecker.class); private static final DslJson dslJson = new DslJson<>(new DslJson.Settings<>()); @@ -102,7 +104,7 @@ public Version withConnection(HttpURLConnection connection) { if (!versions.isEmpty()) { return Collections.min(versions); } - return Version.UNKNOWN_VERSION; + return UNKNOWN_VERSION; } static Version parseVersion(String body) throws java.io.IOException { diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/ApmServerClientTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/ApmServerClientTest.java index c889416940..64d8bb8dba 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/ApmServerClientTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/ApmServerClientTest.java @@ -114,7 +114,7 @@ public void testClientMethodsWithEmptyUrls() throws IOException { // tests setting server_url to an empty string in configuration apmServerClient.start(Lists.emptyList()); awaitUpToOneSecond().untilAsserted( - () -> assertThat(apmServerClient.getApmServerVersion(1, TimeUnit.SECONDS)).isEqualTo(Version.UNKNOWN_VERSION) + () -> assertThat(apmServerClient.getApmServerVersion(1, TimeUnit.SECONDS)).isEqualTo(ApmServerHealthChecker.UNKNOWN_VERSION) ); assertThat(apmServerClient.getCurrentUrl()).isNull(); assertThat(apmServerClient.appendPathToCurrentUrl("/path")).isNull(); From 7251e8074936d379d10a1793aeef2f58cc1cfa55 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Wed, 8 Jun 2022 08:38:39 +0200 Subject: [PATCH 3/4] Fix suffix sorting and tests --- .../java/co/elastic/apm/agent/util/Version.java | 16 +++++++++++----- .../co/elastic/apm/agent/util/VersionTest.java | 4 +++- .../bci/bytebuddy/CustomElementMatchersTest.java | 4 ++-- .../apm/agent/report/ApmServerClientTest.java | 2 +- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/apm-agent-common/src/main/java/co/elastic/apm/agent/util/Version.java b/apm-agent-common/src/main/java/co/elastic/apm/agent/util/Version.java index 734dfe3485..d2f337e566 100644 --- a/apm-agent-common/src/main/java/co/elastic/apm/agent/util/Version.java +++ b/apm-agent-common/src/main/java/co/elastic/apm/agent/util/Version.java @@ -64,6 +64,10 @@ private Version(int[] numbers, String suffix) { this.suffix = suffix; } + public boolean hasSuffix() { + return suffix.length() > 0; + } + @Override public int compareTo(Version another) { final int maxLength = Math.max(numbers.length, another.numbers.length); @@ -74,7 +78,13 @@ public int compareTo(Version another) { return left < right ? -1 : 1; } } - return another.suffix.compareTo(suffix); + if (suffix.isEmpty() || another.suffix.isEmpty()) { + // no suffix is greater than any suffix (1.0.0 > 1.0.0-SNAPSHOT) + return another.suffix.compareTo(suffix); + } else { + // if both have a suffix, sort in natural order (1.0.0-RC2 > 1.0.0-RC1) + return suffix.compareTo(another.suffix); + } } @Override @@ -104,8 +114,4 @@ public int hashCode() { result = 31 * result + Arrays.hashCode(numbers); return result; } - - public boolean hasSuffix() { - return suffix.length() > 0; - } } diff --git a/apm-agent-common/src/test/java/co/elastic/apm/agent/util/VersionTest.java b/apm-agent-common/src/test/java/co/elastic/apm/agent/util/VersionTest.java index 50ccf9b08d..53d0e3ea0d 100644 --- a/apm-agent-common/src/test/java/co/elastic/apm/agent/util/VersionTest.java +++ b/apm-agent-common/src/test/java/co/elastic/apm/agent/util/VersionTest.java @@ -43,12 +43,14 @@ class VersionTest { "1.2, =, 1.2", "1.2.3, =, 1.2.3", "1.2.3.4, =, 1.2.3.4", - "ignore.1.2.3-pre1, =, 1.2.3-pre1" + "ignore.1.2.3-pre1, =, 1.2.3-pre1", + "1.2.3.rc1, <, 1.2.3.rc2" }) void testVersion(String version1, String operator, String version2) { switch (operator) { case "<": assertThat(Version.of(version1)).isLessThan(Version.of(version2)); + assertThat(Version.of(version2)).isGreaterThan(Version.of(version1)); break; case "=": assertThat(Version.of(version1)).isEqualByComparingTo(Version.of(version2)); diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/bytebuddy/CustomElementMatchersTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/bytebuddy/CustomElementMatchersTest.java index 03bbc6e708..7854e6084a 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/bytebuddy/CustomElementMatchersTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/bytebuddy/CustomElementMatchersTest.java @@ -84,9 +84,9 @@ private void testSemVerLteMatcher(ProtectionDomain protectionDomain) { assertThat(implementationVersionLte("3.15.10").matches(protectionDomain)).isFalse(); assertThat(implementationVersionLte("4.2.19").matches(protectionDomain)).isFalse(); assertThat(implementationVersionLte("4.5.5").matches(protectionDomain)).isFalse(); - assertThat(implementationVersionLte("4.5.6").matches(protectionDomain)).isTrue(); assertThat(implementationVersionLte("4.5.5-SNAPSHOT").matches(protectionDomain)).isFalse(); - assertThat(implementationVersionLte("4.5.6-SNAPSHOT").matches(protectionDomain)).isTrue(); + assertThat(implementationVersionLte("4.5.6-SNAPSHOT").matches(protectionDomain)).isFalse(); + assertThat(implementationVersionLte("4.5.6").matches(protectionDomain)).isTrue(); assertThat(implementationVersionLte("4.5.7").matches(protectionDomain)).isTrue(); assertThat(implementationVersionLte("4.7.3").matches(protectionDomain)).isTrue(); assertThat(implementationVersionLte("5.7.3").matches(protectionDomain)).isTrue(); diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/ApmServerClientTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/ApmServerClientTest.java index 64d8bb8dba..88eb9f2cb2 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/report/ApmServerClientTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/report/ApmServerClientTest.java @@ -81,7 +81,7 @@ public void setUp() throws IOException { URL url1 = new URL("http", "localhost", apmServer1.port(), "/"); URL url2 = new URL("http", "localhost", apmServer2.port(), "/proxy"); // APM server 6.x style - apmServer1.stubFor(get(urlEqualTo("/")).willReturn(okForJson(Map.of("ok", Map.of("version", "6.7.0-SNAPSHOT"))))); + apmServer1.stubFor(get(urlEqualTo("/")).willReturn(okForJson(Map.of("ok", Map.of("version", "6.7.1-SNAPSHOT"))))); apmServer1.stubFor(get(urlEqualTo("/test")).willReturn(notFound())); apmServer1.stubFor(get(urlEqualTo("/not-found")).willReturn(notFound())); // APM server 7+ style From 3452bf4f3a29b01bbe2e696f73f29a5c709ea9cc Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Wed, 8 Jun 2022 09:30:01 +0200 Subject: [PATCH 4/4] Ignore suffixes in element matcher --- .../co/elastic/apm/agent/util/Version.java | 4 ++ .../elastic/apm/agent/util/VersionTest.java | 54 +++++++++++-------- .../bci/bytebuddy/CustomElementMatchers.java | 7 ++- .../bytebuddy/CustomElementMatchersTest.java | 4 +- 4 files changed, 43 insertions(+), 26 deletions(-) diff --git a/apm-agent-common/src/main/java/co/elastic/apm/agent/util/Version.java b/apm-agent-common/src/main/java/co/elastic/apm/agent/util/Version.java index d2f337e566..49436f1f9e 100644 --- a/apm-agent-common/src/main/java/co/elastic/apm/agent/util/Version.java +++ b/apm-agent-common/src/main/java/co/elastic/apm/agent/util/Version.java @@ -68,6 +68,10 @@ public boolean hasSuffix() { return suffix.length() > 0; } + public Version withoutSuffix() { + return new Version(numbers, ""); + } + @Override public int compareTo(Version another) { final int maxLength = Math.max(numbers.length, another.numbers.length); diff --git a/apm-agent-common/src/test/java/co/elastic/apm/agent/util/VersionTest.java b/apm-agent-common/src/test/java/co/elastic/apm/agent/util/VersionTest.java index 53d0e3ea0d..5c5e79b620 100644 --- a/apm-agent-common/src/test/java/co/elastic/apm/agent/util/VersionTest.java +++ b/apm-agent-common/src/test/java/co/elastic/apm/agent/util/VersionTest.java @@ -28,34 +28,42 @@ class VersionTest { @ParameterizedTest @CsvSource(value = { - "1.0.0, <, 1.0.1", - "1.2.3, =, 1.2.3", - "1.2.3-SNAPSHOT, <, 1.2.3", - "4.5.13, =, 4.5.13", - "4.5.13.redhat-00001, <, 4.5.13", - "httpclient-4.5.13, =, 4.5.13", - "httpclient-4.5.13.redhat-00001, <, 4.5.13", - "httpclient.4.5.13.redhat-00001, =, 4.5.13.redhat-00001", - "httpclient.4.5.13.redhat, <, 4.5.13", - "httpclient.4.5.13-redhat, =, 4.5.13-redhat", - "1, =, 1", - "1, <, 1.2", - "1.2, =, 1.2", - "1.2.3, =, 1.2.3", - "1.2.3.4, =, 1.2.3.4", - "ignore.1.2.3-pre1, =, 1.2.3-pre1", - "1.2.3.rc1, <, 1.2.3.rc2" + "1.0.0, <, 1.0.1, false", + "1.2.3, =, 1.2.3, false", + "1.2.3-pre1, <, 1.2.3, false", + "1.2.3.rc1, <, 1.2.3.rc2, false", + "1.2.3-SNAPSHOT, <, 1.2.3.RC1, false", + "1.2.3-SNAPSHOT, <, 1.2.3, false", + "4.5.13, =, 4.5.13, false", + "4.5.13.redhat-00001, <, 4.5.13, false", + "httpclient-4.5.13, =, 4.5.13, false", + "httpclient-4.5.13.redhat-00001, =, 4.5.13, true", + "httpclient.4.5.13.redhat-00001, =, 4.5.13.redhat-00001, true", + "httpclient.4.5.13.redhat, =, 4.5.13, true", + "httpclient.4.5.13-redhat, =, 4.5.13-redhat, true", + "1, =, 1, false", + "1, <, 1.2, false", + "1.2, =, 1.2, false", + "1.2.3, =, 1.2.3, false", + "1.2.3.4, =, 1.2.3.4, false", + "ignore.1.2.3-pre1, =, 1.2.3-pre1, false", }) - void testVersion(String version1, String operator, String version2) { + void testVersion(String version1, String operator, String version2, boolean ignoreSuffix) { + Version v1 = Version.of(version1); + Version v2 = Version.of(version2); + if (ignoreSuffix) { + v1 = v1.withoutSuffix(); + v2 = v2.withoutSuffix(); + } switch (operator) { case "<": - assertThat(Version.of(version1)).isLessThan(Version.of(version2)); - assertThat(Version.of(version2)).isGreaterThan(Version.of(version1)); + assertThat(v1).isLessThan(v2); + assertThat(v2).isGreaterThan(v1); break; case "=": - assertThat(Version.of(version1)).isEqualByComparingTo(Version.of(version2)); - assertThat(Version.of(version1)).isEqualTo(Version.of(version2)); - assertThat(Version.of(version1).toString()).isEqualTo(Version.of(version2).toString()); + assertThat(v1).isEqualByComparingTo(v2); + assertThat(v1).isEqualTo(v2); + assertThat(v1.toString()).isEqualTo(v2.toString()); break; default: throw new IllegalArgumentException("Invalid operator: " + operator); diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/CustomElementMatchers.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/CustomElementMatchers.java index 48c87943db..2a2298167f 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/CustomElementMatchers.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/CustomElementMatchers.java @@ -164,8 +164,13 @@ private static ElementMatcher.Junction implementationVersion(f public boolean matches(@Nullable ProtectionDomain protectionDomain) { try { Version pdVersion = readImplementationVersionFromManifest(protectionDomain); - Version limitVersion = Version.of(version); + Version limitVersion = Version.of(version).withoutSuffix(); if (pdVersion != null) { + pdVersion = pdVersion + // ignore suffixes to ensure that 4.5.13.redhat = 4.5.13 + // however, this implies that we'll match 4.5.13-SNAPSHOT = 4.5.13 + // which is not entirely correct as the snapshot may not have all the changes that are in the final version + .withoutSuffix(); return matcher.match(pdVersion, limitVersion); } } catch (Exception e) { diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/bytebuddy/CustomElementMatchersTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/bytebuddy/CustomElementMatchersTest.java index 7854e6084a..03bbc6e708 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/bytebuddy/CustomElementMatchersTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/bytebuddy/CustomElementMatchersTest.java @@ -84,9 +84,9 @@ private void testSemVerLteMatcher(ProtectionDomain protectionDomain) { assertThat(implementationVersionLte("3.15.10").matches(protectionDomain)).isFalse(); assertThat(implementationVersionLte("4.2.19").matches(protectionDomain)).isFalse(); assertThat(implementationVersionLte("4.5.5").matches(protectionDomain)).isFalse(); - assertThat(implementationVersionLte("4.5.5-SNAPSHOT").matches(protectionDomain)).isFalse(); - assertThat(implementationVersionLte("4.5.6-SNAPSHOT").matches(protectionDomain)).isFalse(); assertThat(implementationVersionLte("4.5.6").matches(protectionDomain)).isTrue(); + assertThat(implementationVersionLte("4.5.5-SNAPSHOT").matches(protectionDomain)).isFalse(); + assertThat(implementationVersionLte("4.5.6-SNAPSHOT").matches(protectionDomain)).isTrue(); assertThat(implementationVersionLte("4.5.7").matches(protectionDomain)).isTrue(); assertThat(implementationVersionLte("4.7.3").matches(protectionDomain)).isTrue(); assertThat(implementationVersionLte("5.7.3").matches(protectionDomain)).isTrue();