Switch from buildSrc/version.properties to Gradle version catalog (gradle/libs.versions.toml) to enable dependabot to perform automated upgrades on common libs

[cwperks]

Description

Opening this PR in draft to explore what it will take to enable dependabot to perform automated upgrades on the dependency versions listed in buildSrc/version.properties.

This issue came up for discussion on my very first PR on the project: #3772.

Dependabot works on version catalogs: https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#gradle

To test this I pushed to the main branch of my fork and used Dependabot CLI to run dependabot in a dry-run mode where it displays what PRs would be created without actually creating a PR.

To test, I created a sample dependabot configuration like this:

job:
  package-manager: gradle
  allowed-updates:
    - update-type: all
  source:
    provider: github
    repo: cwperks/opensearch
    directory: /

And ran it with ~/go/bin/dependabot update -f ./.github/dependabot_server.yml

See joda update in the output:

updater | +-------------------------------------------------------------------------------------------+
updater | |                            Changes to Dependabot Pull Requests                            |
updater | +---------+---------------------------------------------------------------------------------+
updater | | created | org.apache.ant:ant ( from 1.10.14 to 1.10.15 )                                  |
updater | | created | com.netflix.nebula:gradle-extra-configurations-plugin ( from 10.0.0 to 10.0.1 ) |
updater | | created | com.netflix.nebula:nebula-publishing-plugin ( from 21.0.0 to 21.1.0 )           |
updater | | created | com.netflix.nebula:gradle-info-plugin ( from 12.1.6 to 13.3.0 )                 |
updater | | created | org.apache.rat:apache-rat ( from 0.15 to 0.16.1 )                               |
updater | | created | net.java.dev.jna:jna ( from 5.14.0 to 5.15.0 )                                  |
updater | | created | com.avast.gradle:gradle-docker-compose-plugin ( from 0.17.6 to 0.17.9 )         |
updater | | created | org.apache.maven:maven-model ( from 3.9.6 to 3.9.9 )                            |
updater | | created | com.networknt:json-schema-validator ( from 1.2.0 to 1.5.2 )                     |
updater | | created | org.ajoberstar.grgit:grgit-core ( from 5.2.1 to 5.3.0 )                         |
updater | | created | org.wiremock:wiremock-standalone ( from 3.6.0 to 3.9.1 )                        |
updater | | created | org.spockframework:spock-core ( from 2.3-groovy-3.0 to 2.3-groovy-4.0 )         |
updater | | created | joda-time:joda-time ( from 2.12.7 to 2.13.0 )                                   |
updater | +---------+---------------------------------------------------------------------------------+

Related Issues

Resolves #3782

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

❌ Gradle check result for 724db17: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[dbwiddis]

Opening this PR in draft to explore what it will take to enable dependabot to perform automated upgrades on the dependency versions listed in buildSrc/version.properties.

I recommend switching from Dependabot to using Mend Renovate.

I've outlined several reasons that it's a preferred choice in opensearch-project/.github#97 and added a section to the Maintainer Responsibilities guide here.

One of the options of this configuration is to enable auto-merging for a subset of dependencies. I have done so on my own project here.

[cwperks]

One of the options of this configuration is to enable auto-merging for a subset of dependencies. I have done so on my own project here.

@peternied Has created something similar for dependabot and opensearch-trigger-bot PRs on the security repo. See the automatic-merges workflow here

[dbwiddis]

The gradle version catalog will be really useful in the future

Yes.

[reta]

@cwperks OK, I think I finally glued all pieces together :D (sorry it took so long). So here are 3 parts (or stages) that we have to go through to have dependency management revamped in favour of using versions catalog + be dependabot friendly:

1. Add libs.versions.toml with only [versions] section (version catalog), the dependabot would do nothing with that at this stage
2. Generate version.properties out of libs.versions.toml (so nothing changes there by and large for consumers)
3. Add all dependencies to libs.versions.toml's [libraries] section: that would centralize the dependency management and actually would let dependabot to pick version updates

Does it make sense? If yes, I would like to ask you remove [libraries] section from the TOML file (only keep [versions]) so we could minimize any possible (unknown) risks of breaking things. Thanks!

[cwperks]

@reta I removed the [libraries] section, but kept the logic in place that parses the [versions] section from the TOML file to generate the version.properties file. That way if we add additional sections in the toml file in the future, the logic to handle it is already in place.

[reta]

@reta I removed the [libraries] section, but keeped the logic in place that parses the [versions] section from the TOML file to generate the version.properties file. That way if we add additional sections in the toml file in the future, the logic to handle it is already in place.

@cwperks about that, do you mind if I push a small change to this particular part? I prefer us to not do manual parsing. Thank you.

[cwperks]

@reta I removed the [libraries] section, but keeped the logic in place that parses the [versions] section from the TOML file to generate the version.properties file. That way if we add additional sections in the toml file in the future, the logic to handle it is already in place.

@cwperks about that, do you mind if I push a small change to this particular part? I prefer us to not do manual parsing. Thank you.

Go ahead! You can push to any of my PRs ;)

[github-actions]

❌ Gradle check result for 5003c97: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

✅ Gradle check result for 7edba63: SUCCESS

[reta]

@dblock @dbwiddis @andrross @peternied I think this is first step in right direction (to replace ad-hoc version management with Gradle version catalogs, see please larger picture #16284 (comment)). Curious what do you think folks, any concerns? LGTM to me, thanks a lot @cwperks !

[dbwiddis]

Curious what do you think folks, any concerns?

How are transitive dependencies handled? I've experienced frequent issues in plugins where we need Dependency X which transitively depends on Y. OpenSearch has Y at an earlier version but doesn't have X at all. Will we still be able to force a higher version?

Will plugins be able to access the version number that OpenSearch has easily (version catalog) so that we don't have to maintain (synchronized) version bumps ourselves?

[reta]

Thanks @dbwiddis

How are transitive dependencies handled?

If I am not mistaken, this is out of scope of version catalogs BUT is in scope of the individual modules: it all depends on how the module import the dependency (runtime, compile, ...)

Will we still be able to force a higher version?

I believe the force mechanism will stay explicit (I could be mistaken)

Will plugins be able to access the version number that OpenSearch has easily (version catalog) so that we don't have to maintain (synchronized) version bumps ourselves?

Yes! That's the end goal!

[dbwiddis]

Yes! That's the end goal!

🎉

[cwperks]

Sync'ed with main to resolve conflicts and ensured the latest versions on main for version.properties are copied to libs.versions.toml

[github-actions]

❌ Gradle check result for ef776e2: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[cwperks]

@reta Is this same error appearing on main?

Caused by:
        org.mockito.exceptions.****.MockitoException: Could not modify all classes [class java.lang.Object, interface java.io.Closeable, interface org.apache.hc.client5.http.async.HttpAsyncClient, class org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient, interface java.lang.AutoCloseable, interface org.apache.hc.core5.io.ModalCloseable]
            at app//net.bytebuddy.TypeCache.findOrInsert(TypeCache.java:168)
            at app//net.bytebuddy.TypeCache$WithInlineExpunction.findOrInsert(TypeCache.java:399)
            at app//net.bytebuddy.TypeCache.findOrInsert(TypeCache.java:190)
            at app//net.bytebuddy.TypeCache$WithInlineExpunction.findOrInsert(TypeCache.java:410)
            ... 2 more

            Caused by:
            java.lang.IllegalStateException: 
            Byte Buddy could not instrument all classes within the mock's type hierarchy

            This problem should never occur for javac-compiled classes. This problem has been observed for classes that are:
             - Compiled by older versions of scalac
             - Classes that are part of the Android distribution
                at org.mockito.internal.creation.bytebuddy.InlineBytecodeGenerator.triggerRetransformation(InlineBytecodeGenerator.java:285)
                at org.mockito.internal.creation.bytebuddy.InlineBytecodeGenerator.mockClass(InlineBytecodeGenerator.java:218)
                at org.mockito.internal.creation.bytebuddy.TypeCachingBytecodeGenerator.lambda$mockClass$0(TypeCachingBytecodeGenerator.java:78)
                at net.bytebuddy.TypeCache.findOrInsert(TypeCache.java:168)
                at net.bytebuddy.TypeCache$WithInlineExpunction.findOrInsert(TypeCache.java:399)
                at net.bytebuddy.TypeCache.findOrInsert(TypeCache.java:190)
                at net.bytebuddy.TypeCache$WithInlineExpunction.findOrInsert(TypeCache.java:410)
                at org.mockito.internal.creation.bytebuddy.TypeCachingBytecodeGenerator.mockClass(TypeCachingBytecodeGenerator.java:75)
                at org.mockito.internal.creation.bytebuddy.InlineDelegateByteBuddyMockMaker.createMockType(InlineDelegateByteBuddyMockMaker.java:414)
                at org.mockito.internal.creation.bytebuddy.InlineDelegateByteBuddyMockMaker.doCreateMock(InlineDelegateByteBuddyMockMaker.java:373)
                at org.mockito.internal.creation.bytebuddy.InlineDelegateByteBuddyMockMaker.createMock(InlineDelegateByteBuddyMockMaker.java:352)
                at org.mockito.internal.creation.bytebuddy.InlineByteBuddyMockMaker.createMock(InlineByteBuddyMockMaker.java:56)
                at org.mockito.internal.util.MockUtil.createMock(MockUtil.java:99)
                at org.mockito.internal.MockitoCore.mock(MockitoCore.java:84)
                at org.mockito.Mockito.mock(Mockito.java:2104)
                at org.mockito.Mockito.mock(Mockito.java:2019)
                ... 2 more

                Caused by:
                java.lang.IllegalArgumentException: Java 23 (67) is not supported by the current version of Byte Buddy which officially supports Java 22 (66) - update Byte Buddy or set net.bytebuddy.experimental as a VM property
                    at net.bytebuddy.utility.OpenedClassReader.of(OpenedClassReader.java:100)
                    at net.bytebuddy.dynamic.scaffold.TypeWriter$Default$ForInlining.create(TypeWriter.java:4011)
                    at net.bytebuddy.dynamic.scaffold.TypeWriter$Default.make(TypeWriter.java:2224)
                    at net.bytebuddy.dynamic.DynamicType$Builder$AbstractBase$UsingTypeWriter.make(DynamicType.java:4055)
                    at net.bytebuddy.dynamic.DynamicType$Builder$AbstractBase.make(DynamicType.java:3739)
                    at org.mockito.internal.creation.bytebuddy.InlineBytecodeGenerator.transform(InlineBytecodeGenerator.java:402)
                    at java.instrument/java.lang.instrument.ClassFileTransformer.transform(ClassFileTransformer.java:242)
                    at java.instrument/sun.instrument.TransformerManager.transform(TransformerManager.java:188)
                    at java.instrument/sun.instrument.InstrumentationImpl.transform(InstrumentationImpl.java:610)
                    at java.instrument/sun.instrument.InstrumentationImpl.retransformClasses0(Native Method)
                    at java.instrument/sun.instrument.InstrumentationImpl.retransformClasses(InstrumentationImpl.java:225)
                    at org.mockito.internal.creation.bytebuddy.InlineBytecodeGenerator.triggerRetransformation(InlineBytecodeGenerator.java:281)
                    ... 17 more

> Task :build-tools:compileIntegTestGroovy
> Task :build-tools:processIntegTestResources
> Task :build-tools:integTestClasses

> Task :client:rest:test

Tests with failures:
 - org.opensearch.client.RestClientTests.testIsRunning
 - org.opensearch.client.RestClientTests.testStreamWithUnsupportedMethod
 - org.opensearch.client.RestClientTests.testPerformAsyncWithUnsupportedMethod
 - org.opensearch.client.RestClientTests.testCloseIsIdempotent
 - org.opensearch.client.RestClientTests.testSetNodesDuplicatedHosts
 - org.opensearch.client.RestClientTests.testSetNodesWrongArguments
 - org.opensearch.client.RestClientTests.testSetNodesPreservesOrdering
 - org.opensearch.client.RestClientTests.testPerformAsyncWithWrongEndpoint

97 tests completed, 8 failed, 1 skipped

> Task :client:rest:test FAILED

[reta]

Suggested change

	bytebuddy = "1.14.9"
	bytebuddy = "1.15.4"

[reta]

May be more versions need to be reconciled ..

[reta]

@reta Is this same error appearing on main?

@cwperks no, sadly version.properties and TOML file diverged :( needs to be reconciled

[cwperks]

My bad, I did not update the bytebuddy or mockito versions to the latest on main. Its fully reconciled now.

[github-actions]

✅ Gradle check result for e07253e: SUCCESS