Exclude java.security.Provider SPI configuration from shaded jar

[jrhee17]

Motivation:

Recently we noticed that java modules don't work well with Armeria. The reason was that META-INF/services/java.security.Provider is included in our SPI, but org.bouncycastle.jce.provider.BouncyCastleProvider doesn't exist in the JAR causing an exception in the following location.

Since we shade our custom BouncyCastleProvider and use it sparingly, we probably don't need to also export the SPI implementations.

armeria/core/src/main/java/com/linecorp/armeria/internal/common/util/MinifiedBouncyCastleProvider.java

Lines 93 to 98 in 6dec849

	Security.addProvider(new MinifiedBouncyCastleProvider());
	try {
	return task.get();
	} finally {
	Security.removeProvider(PROVIDER_NAME);
	}

Note that this problem can happen if a shaded module provides SPI implementations and is not necessarily unique to BouncyCastle. From a quick check I don't think (guava, jctools, reflections, fastutil, futures) have a similar issue though.

A working version with java 9 modules with the built snapshot version can be found in the following repository: https://github.com/jrhee17/my-java-module. Note that this change also requires line/gradle-scripts#184.

Also, did a local test to ensure there are no unintentional changes

user@AL02437565 shaded-diff % tar -zxf /Users/user/Projects/armeria/core/build/libs/armeria-shaded-1.30.0-SNAPSHOT.jar -C new               
user@AL02437565 shaded-diff % tar -zxf /Users/user/Projects/upstream-armeria/core/build/libs/armeria-shaded-1.30.0-SNAPSHOT.jar -C base     
user@AL02437565 shaded-diff % diff -rq new base                                                                                                      
Files new/META-INF/com.linecorp.armeria.versions.properties and base/META-INF/com.linecorp.armeria.versions.properties differ
Only in base/META-INF/services: java.security.Provider
user@AL02437565 shaded-diff %

Modifications:

• Added a shadowExclusions property which excludes files from the shadowed JAR

Result:

• Armeria has more compatibility with Java 9 modules.

[github-actions]

🔍 Build Scan® (commit: a4455c4)

Job name	Status	Build Scan®
build-windows-latest-jdk-21	✅	https://ge.armeria.dev/s/4uu7shict6ewi
build-self-hosted-unsafe-jdk-8	✅	https://ge.armeria.dev/s/e2l63ofaniut6
build-self-hosted-unsafe-jdk-21-snapshot-blockhound	✅	https://ge.armeria.dev/s/piaav6pksudhq
build-self-hosted-unsafe-jdk-17-min-java-17-coverage	✅	https://ge.armeria.dev/s/cncz6zian7l2i
build-self-hosted-unsafe-jdk-17-min-java-11	✅	https://ge.armeria.dev/s/7dm5uhkbhodu6
build-self-hosted-unsafe-jdk-17-leak	✅	https://ge.armeria.dev/s/3lywjicozss7k
build-self-hosted-unsafe-jdk-11	✅	https://ge.armeria.dev/s/obewhflz6af3w
build-macos-12-jdk-21	✅	https://ge.armeria.dev/s/omoe7krdnmmk2

[ikhoon]

How about adding a new property shadowExclusions like jacocoExclusions and excluding shadowExclusions in shadeJar?

armeria/gradle.properties

Line 29 in 6dec849

jacocoExclusions=com/linecorp/armeria/internal/common/CurrentJavaVersionSpecific,com/linecorp/armeria/*/scalapb/**,META-INF/versions/**

[jrhee17]

In order to do so, I think:

• This functionality should be applied to all modules, not just :core. If we take this approach, it probably makes more sense to move this functionality to gradle-scripts
• In the case of :core, the filtering will need to happen twice: once to add versions, another time to remove shadowExclusions

Am I understanding your intention correctly?

[ikhoon]

I didn't think we needed shadowExclusions per module. I imagined adding shadowExclusions in java-shade.gradle. For example:

task shadedJar(

	def shadowExclusions = []
	if (rootProject.hasProperty('shadowExclusions')) {
	    shadowExclusions = rootProject.findProperty('shadowExclusions').split(",")
	}
	shadowExclusions.each {
	    exclude it
	}
}

[ikhoon]

My proposal intended to use the feature provided by Shadow instead of the workaround.
If you think a different exclusion is required for each module, bomGroups pattern is also good to me.

[jrhee17]

I see, this solution also works 👍 Note that I wasn't able to migrate the other exclusions to ensure backwards compat

[ikhoon]

Sounds good. Let's regard them as the default exclusions.

[ikhoon]

🙇‍♂️🙇‍♂️

[minwoox]

👍 👍 👍