Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Java dependencies with as many Red Hat built ones #262

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

pierDipi
Copy link
Member


Example diff for the current pom.xml for EKB 1.15

pierdipi@pierdipi data-plane (release-v1.15) $ git diff --staged pom.xml
diff --git a/data-plane/pom.xml b/data-plane/pom.xml
index 3f8fe445a..94269c695 100644
--- a/data-plane/pom.xml
+++ b/data-plane/pom.xml
@@ -103,6 +103,17 @@
   </profiles>
 
   <repositories>
+    <repository>
+      <id>red-hat-ga</id>
+      <url>https://maven.repository.redhat.com/ga</url>
+      <releases>
+        <enabled>true</enabled>
+      </releases>
+      <snapshots>
+        <enabled>true</enabled>
+      </snapshots>
+    </repository>
+
     <repository>
       <url>https://oss.sonatype.org/content/repositories/snapshots/</url>
       <id>sonatype</id>
@@ -119,9 +130,9 @@
     <dependencies>
       <!-- Vertx -->
       <dependency>
-        <groupId>io.quarkus</groupId>
+        <groupId>com.redhat.quarkus.platform</groupId>
         <artifactId>quarkus-bom</artifactId>
-        <version>${quarkus.version}</version>
+        <version>3.8.5.redhat-00003</version>
         <type>pom</type>
         <scope>import</scope>
       </dependency>
@@ -129,18 +140,18 @@
       <dependency>
         <groupId>org.antlr</groupId>
         <artifactId>antlr4-runtime</artifactId>
-        <version>${antlr.version}</version>
+        <version>4.9.2.redhat-00003</version>
       </dependency>
       <dependency>
         <groupId>org.antlr</groupId>
         <artifactId>antlr4</artifactId>
-        <version>${antlr.version}</version>
+        <version>4.9.2.redhat-00003</version>
       </dependency>
 
       <dependency>
         <groupId>io.vertx</groupId>
         <artifactId>vertx-opentelemetry</artifactId>
-        <version>${vertx.version}</version>
+        <version>4.4.6.redhat-00001</version>
         <exclusions>
           <exclusion>
             <groupId>io.vertx</groupId>
@@ -152,7 +163,7 @@
       <dependency>
         <groupId>org.apache.kafka</groupId>
         <artifactId>kafka-clients</artifactId>
-        <version>${kafka.version}</version>
+        <version>3.7.0.redhat-00007</version>
       </dependency>
 
       <!-- Micrometer -->
@@ -182,7 +193,7 @@
       <dependency>
         <groupId>io.fabric8</groupId>
         <artifactId>kubernetes-client</artifactId>
-        <version>${fabric8.kubernetes.version}</version>
+        <version>6.10.0.redhat-00003</version>
         <exclusions>
           <exclusion>
             <groupId>io.fabric8</groupId>
@@ -193,17 +204,17 @@
       <dependency>
         <groupId>io.fabric8</groupId>
         <artifactId>kubernetes-client-api</artifactId>
-        <version>${fabric8.kubernetes.version}</version>
+        <version>6.10.0.redhat-00003</version>
       </dependency>
       <dependency>
         <groupId>io.fabric8</groupId>
         <artifactId>kubernetes-httpclient-jdk</artifactId>
-        <version>${fabric8.kubernetes.version}</version>
+        <version>6.10.0.redhat-00003</version>
       </dependency>
       <dependency>
         <groupId>io.fabric8</groupId>
         <artifactId>kubernetes-server-mock</artifactId>
-        <version>${fabric8.kubernetes.version}</version>
+        <version>6.10.0.redhat-00003</version>
         <scope>test</scope>
         <exclusions>
           <exclusion>
@@ -225,7 +236,7 @@
       <dependency>
         <groupId>com.fasterxml.jackson</groupId>
         <artifactId>jackson-bom</artifactId>
-        <version>${jackson.version}</version>
+        <version>2.14.2.redhat-00003</version>
         <type>pom</type>
         <scope>import</scope>
       </dependency>
@@ -233,24 +244,24 @@
       <dependency>
         <groupId>org.bitbucket.b_c</groupId>
         <artifactId>jose4j</artifactId>
-        <version>${jose4j.version}</version>
+        <version>0.9.6.redhat-00001</version>
       </dependency>
 
       <!-- Logback -->
       <dependency>
         <groupId>org.slf4j</groupId>
         <artifactId>slf4j-api</artifactId>
-        <version>${slf4j.version}</version>
+        <version>2.0.16.redhat-00001</version>
       </dependency>
       <dependency>
         <groupId>ch.qos.logback</groupId>
         <artifactId>logback-core</artifactId>
-        <version>${ch.qos.logback.version}</version>
+        <version>1.4.14.redhat-00001</version>
       </dependency>
       <dependency>
         <groupId>ch.qos.logback</groupId>
         <artifactId>logback-classic</artifactId>
-        <version>${ch.qos.logback.version}</version>
+        <version>1.4.14.redhat-00001</version>
       </dependency>
       <dependency>
         <groupId>net.logstash.logback</groupId>
@@ -306,12 +317,12 @@
       <dependency>
         <groupId>com.google.protobuf</groupId>
         <artifactId>protobuf-java</artifactId>
-        <version>${protobuf.version}</version>
+        <version>3.25.2.redhat-00001</version>
       </dependency>
       <dependency>
         <groupId>com.google.protobuf</groupId>
         <artifactId>protobuf-java-util</artifactId>
-        <version>${protobuf.version}</version>
+        <version>3.25.2.redhat-00001</version>
       </dependency>
 
       <!-- Rate Limiter -->
@@ -375,7 +386,7 @@
       <dependency>
         <groupId>org.awaitility</groupId>
         <artifactId>awaitility</artifactId>
-        <version>${awaitility.version}</version>
+        <version>4.2.0.redhat-00001</version>
         <scope>test</scope>
       </dependency>
 
@@ -399,7 +410,7 @@
       <dependency>
         <groupId>org.apache.kafka</groupId>
         <artifactId>kafka_2.13</artifactId>
-        <version>${kafka.version}</version>
+        <version>3.7.0.redhat-00007</version>
         <scope>test</scope>
         <exclusions>
           <exclusion>
@@ -415,13 +426,13 @@
       <dependency>
         <groupId>org.scala-lang</groupId>
         <artifactId>scala-library</artifactId>
-        <version>2.13.14</version>
+        <version>2.13.14.redhat-00002</version>
         <scope>test</scope>
       </dependency>
       <dependency>
         <groupId>org.scala-lang</groupId>
         <artifactId>scala-reflect</artifactId>
-        <version>2.13.14</version>
+        <version>2.13.14.redhat-00002</version>
         <scope>test</scope>
       </dependency>
     </dependencies>

and it also builds successfully:

pierdipi@pierdipi data-plane (release-v1.15) $ ./mvnw clean package -DskipTests
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  01:43 min
[INFO] Finished at: 2024-09-11T13:15:52+02:00
[INFO] ------------------------------------------------------------------------

@openshift-ci openshift-ci bot requested review from aliok and rhuss September 11, 2024 11:18
Copy link

openshift-ci bot commented Sep 11, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pierDipi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Pierangelo Di Pilato <[email protected]>
)

func main() {
metadata, err := maven.ScrapRedHatMavenRegistry(maven.RedHatMavenGA)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's only Quarkus version, we might be good with:

➜  ~ curl -s "https://code.quarkus.redhat.com/api/platforms" | jq -r '.platforms[0].streams[0].releases[0].version'
3.8.5.SP1-redhat-00001

Copy link
Contributor

@dsimansk dsimansk Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SP1 is actually newer than 3.8.5.redhat-00003. Where regular 3.8.5 is standard release. Then SP1 stands for security patch 1 of 3.8.5 I believe.

https://maven.repository.redhat.com/ga/com/redhat/quarkus/platform/quarkus-bom/3.8.5.SP1-redhat-00001/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SP == service patch, sorry :)

Copy link
Member Author

@pierDipi pierDipi Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's only Quarkus version, we might be good with:

no, it's not only for Quarkus, see the list in the diff in the PR body

SP1 is actually newer than 3.8.5.redhat-00003. Where regular 3.8.5 is standard release. Then SP1 stands for security patch 1 of 3.8.5 I believe.

hmm, aren't versions here sorted ? https://maven.repository.redhat.com/ga/com/redhat/quarkus/platform/quarkus-bom/maven-metadata.xml

Also latest says 3.8.5.redhat-00003 and not the SP one

<latest>3.8.5.redhat-00003</latest>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are not unfortunately. The service patch actually might be fixing Quarkus platform libs etc, but core quarkus is not changing. Hence the prefix version stays the same, and the rest is incrementing very "naturally".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, ok, I think this is already better than what we currently have in Konflux builds, so special sorting can be added later or we can always add "bump dependencies" using other tools later (like renovate or dependabot)

return fmt.Errorf("failed to update pom file %q: %w", p, err)
}

commitMsg := fmt.Sprintf("[%s] Updated POM file %s with Red Hat dependencies", branchName, p)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
commitMsg := fmt.Sprintf("[%s] Updated POM file %s with Red Hat dependencies", branchName, p)
commitMsg := fmt.Sprintf("[%s] Updated POM file %s with Red Hat dependencies", targetBranch, p)

Shouldn't really make a difference here, but theoretically on "release-next" we'd push to main

@dsimansk
Copy link
Contributor

Putting some thoughts around version in pom. I'm pretty sure that importing BOM in Maven project should mean that version provided by the BOM are considered "managed". Therefore project pom.xml doesn't have to redeclare <version> tags, but rather omit those to use provided values from BOM.

In theory EKB data-plane should work just by using Quarkus' BOM and version will be inherited from it. I'll try to play with it a bit.

@pierDipi
Copy link
Member Author

@dsimansk any news for the last comment?

@openshift-merge-robot openshift-merge-robot added the needs-rebase The PR needs to have conflicts fixed label Oct 9, 2024
@openshift-merge-robot
Copy link

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@pierDipi pierDipi changed the title Replace Java dependencies with Red Hat built ones Replace Java dependencies with as many Red Hat built ones Oct 11, 2024
@pierDipi
Copy link
Member Author

@dsimansk any news? we need this one in some shape or form

@pierDipi
Copy link
Member Author

@creydr I guess you had comments, can you write them here?

@creydr
Copy link
Member

creydr commented Oct 14, 2024

@creydr I guess you had comments, can you write them here?

My main thought about this was, if we could integrate this into the update-to-head jobs, so we have this tested also somehow in CI (no need to do this in this PR)

@pierDipi
Copy link
Member Author

@creydr I guess you had comments, can you write them here?

My main thought about this was, if we could integrate this into the update-to-head jobs, so we have this tested also somehow in CI (no need to do this in this PR)

@creydr wouldn't this be tested in CI when the bot opens the PR ?

@skonto
Copy link
Contributor

skonto commented Oct 22, 2024

How is this solved in other repos? Do people scrape deps?

@pierDipi
Copy link
Member Author

pierDipi commented Oct 22, 2024

In the old system there is a EAP based Java service but requires EAP to run and there is not alternatives for Konflux, the JBS in Konflux is beeing redesigned but it's not there yet

@pierDipi
Copy link
Member Author

We discussed why in the past, I lost the links to the existing tools now :)

@skonto
Copy link
Contributor

skonto commented Oct 22, 2024

In the old system there is a EAP based Java service but requires EAP to run and there is not alternatives for Konflux, the JBS in Konflux is beeing redesigned but it's not there yet

So people will do scraping in each repo that has java deps for now? Interesting.

@pierDipi
Copy link
Member Author

No, scraping is an alternative to doing it manually

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved needs-rebase The PR needs to have conflicts fixed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants