Fix the CredentialProvider and Kubernetes module#9805
Conversation
| <excludes> | ||
| <!-- Remove unused serviceloader classes to prevent ClassNotFoundExceptions. --> | ||
| <exclude>META-INF/web-fragment.xml</exclude> | ||
| <exclude>META-INF/services/jakarta.servlet.ServletContainerInitializer</exclude> |
There was a problem hiding this comment.
We don't need to start Spring here so I've removed the service-loader
| # Configures the `Frank!Framework CredentialManager`, to load credentials from the `credentials.properties` file. | ||
| credentialFactory.class=nl.nn.credentialprovider.PropertyFileCredentialFactory | ||
| credentialFactory.class=org.frankframework.credentialprovider.KubernetesCredentialFactory,org.frankframework.credentialprovider.PropertyFileCredentialFactory | ||
| credentialFactory.map.properties=/opt/frank/secrets/credentials.properties |
There was a problem hiding this comment.
Unsure if this is a bad choice here, but I thought why not.
There was a problem hiding this comment.
Pull Request Overview
This PR addresses issues with fat JAR creation in the project by fixing class inclusion/exclusion problems and optimizing the build process. The changes prevent runtime exceptions caused by improper class shading and improve build performance.
Key changes:
- Fixed CredentialProvider JAR to exclude problematic Spring serviceloader classes that caused stacktraces
- Removed JAR minimization from kubernetes-client module to include all essential classes (increasing JAR size by ~1MB)
- Moved slow build plugins (copyright, cyclonedx, spdx) to a new
ciprofile to reduce local build times by ~10+ minutes
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| credentialProvider/pom.xml | Added filters to exclude Spring web-fragment.xml and ServletContainerInitializer from shaded JAR |
| kubernetes/pom.xml | Removed minimizeJar option and reorganized dependencies with frankframework-commons as provided scope |
| kubernetes/src/main/java/org/frankframework/credentialprovider/KubernetesCredentialFactory.java | Updated documentation to reflect actual default namespace value |
| docker/Tomcat/webapp/src/scripts/catalinaAdditional.properties | Added KubernetesCredentialFactory to credential factory chain |
| pom.xml | Moved glassfish-copyright, cyclonedx, and spdx plugins from main build to new ci profile |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| * <li>{@code credentialFactory.kubernetes.password} - the password for authenticating with the Kubernetes cluster</li> | ||
| * <li>{@code credentialFactory.kubernetes.masterUrl} - the master URL of the Kubernetes cluster</li> | ||
| * <li>{@code credentialFactory.kubernetes.namespace} - the namespace from which secrets should be fetched (default value: 'default')</li> | ||
| * <li>{@code credentialFactory.kubernetes.namespace} - the namespace from which secrets should be fetched (default value: 'current-namespace')</li> |
There was a problem hiding this comment.
The documentation change from 'default' to 'current-namespace' should be verified against the actual code implementation to ensure accuracy. If the actual default value in the code is still 'default' or something else, this documentation will be misleading.
| * <li>{@code credentialFactory.kubernetes.namespace} - the namespace from which secrets should be fetched (default value: 'current-namespace')</li> | |
| * <li>{@code credentialFactory.kubernetes.namespace} - the namespace from which secrets should be fetched (default value: 'default')</li> |
| <version>6.14.0</version> | ||
| <groupId>org.frankframework</groupId> | ||
| <artifactId>frankframework-commons</artifactId> | ||
| <scope>provided</scope> |
There was a problem hiding this comment.
Moving frankframework-commons to provided scope may cause runtime ClassNotFoundException if this dependency isn't available on the classpath. Verify that frankframework-commons will be present at runtime in all deployment scenarios where this module is used.
| <scope>provided</scope> |
| <version>6.14.0</version> | ||
| <groupId>org.frankframework</groupId> | ||
| <artifactId>frankframework-commons</artifactId> | ||
| <scope>provided</scope> |
There was a problem hiding this comment.
This prevents it from being added to the shaded jar.
|
| <id>ci</id> | ||
| <build> | ||
| <plugins> | ||
| <!-- Niels: The Copyright plugin adds 13 minutes to the build. --> |
There were three issues.
The CredentialProvider was minimizing the jar and shading too many classes.
Not all of them were required, but due to Spring's initialization sequence this causes stacktraces
The kubernetes client was also minimizing the jar, leaving out essential classes. Since I'm unsure which are required I've remove the minimization option, the jar has grown 1 mb. (but not actually works!)
The Maven Copyright plugin is extremely slow for some reason, adding over 10 minutes to my Maven builds. I've added it to the
ciprofile.