#2241 fix bug: when running with a fat jar, need to read entries with openConnection by wkclz · Pull Request #2952 · kubernetes-client/java

wkclz · 2023-12-17T15:42:49Z

When running in IDE or a thin jar, it is ok.
When running with a fat jar in server, new JarFile can not open and read the entries.

… openConnection when running in IDE or a thin jar, it is ok, when running with a fat jar in server, new JarFile can not open and read the entries.

linux-foundation-easycla · 2023-12-17T15:42:56Z

The committers listed above are authorized under a signed CLA.

✅ login: wkclz (47ef71f, ec7e7b9, 88ac227, de71d98, f69bd5e)

k8s-ci-robot · 2023-12-17T15:42:59Z

Welcome @wkclz!

It looks like this is your first PR to kubernetes-client/java 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-client/java has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

brendandburns · 2023-12-21T16:23:32Z

Hrm, do you have a pointer to documentation about why this works this way? I definitely don't want this library to be opening URLs over the network, so is there a way to restrict this to file based entries in the fat jar?

brendandburns · 2023-12-21T16:23:40Z

Also, you need to sign the CLA.

wkclz · 2023-12-23T06:34:51Z

Running Yaml.load("yaml String"), in "io.kubernetes.client.util.Yaml:553"
When I run it in IDE, it can get Class by Kind, But in a fat jar, the class is null, and then, I got "Unknown apiVersionKind " + apiVersion + "/" + kind + " is it registered?"
I found several issues about this problem: issues: #1659, #2655, #2241, #2377 ...

Make a break point to trace the code, ModelMapper.initModelMap() is in the static, it will scan and cache classes to classesByGVK or preBuiltClassesByGVK from "io.kubernetes.client.openapi.models".
The problem is in here: io.kubernetes.client.util.ModelMapper.processJarPackage:
When the project is a fat jar, the packageURL is like this: "nested:/home/apps.jar/!BOOT-INF/lib/client-java-api-19.0.0.jar!/io/kubernetes/client/openapi/models", so it would get an IOException.

If try to get the jar like this: ((JarURLConnection) packageURL.openConnection()).getJarFile(), it will be OK. I have try it by overwrite this class.
This URL is only the jar url in current project, not over the network, so it is safe.
If the URL is not a JarURL, it would an ClassCastException.

In this issue #2241, I output the stack of the problem.

CLA is ok now.

brendandburns · 2023-12-27T17:43:32Z

-    jarFileName = jarFileName.substring(5, jarFileName.indexOf("!"));
-    logger.info("Loading classes from jar {}", jarFileName);
-    try (JarFile jf = new JarFile(jarFileName)) {
+    logger.info("Loading classes from jar {}", packageURL.getFile());


Can you add an explicit check for packageURL.startsWith("jar:") here?

brendandburns · 2023-12-27T17:44:23Z

Ok, this approach works for me, I asked for a single explicit check for the jar: prefix otherwise looks good.

Is there a way that we can add a unit test for this?

In a fat jar, packageURL start with "nested:", not a fat jar, it start with "jar:".

wkclz · 2023-12-28T16:17:10Z

Running unit test with Yaml.load("yaml string"); can only test for a packageURL start with "jar:".
You may create a project with springboot, and package it with the plugin spring-boot-maven-plugin, and then, you will get a packageURL start with "nested:", e.g.: nested:/home/apps.jar/!BOOT-INF/lib/client-java-api-19.0.0.jar!/io/kubernetes/client/openapi/models

packageURL is not jar or nested, it would be return

brendandburns · 2023-12-31T03:05:54Z

/lgtm
/approve

I'm fine to merge this w/o a test given the complexity of creating one.

k8s-ci-robot · 2023-12-31T03:06:02Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brendandburns, wkclz

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [brendandburns]

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

re import URLDecoder

brendandburns · 2024-01-02T17:12:48Z

Looks like this isn't working right in our e2e tests, it can't find the class for nodes any more.

…unit test)

wkclz · 2024-01-03T15:22:15Z

fixed, and tested e2e tests

yue9944882 · 2024-01-04T01:14:01Z

Fixes: #1659

yue9944882 · 2024-01-04T01:21:50Z

+      jf = new JarFile(jarFileName);
+    }
+    if (jf == null) {
+      logger.error("Loading classes from jar with error packageURL: {}", jarFileName);


Suggested change

logger.error("Loading classes from jar with error packageURL: {}", jarFileName);

logger.error("Loading classes from jar with error packageURL: {}", packageURL);

nit: looks like we might want to dump the packageURL here?

the same with packageURL

brendandburns · 2024-01-06T18:29:58Z

@yue9944882 I'll wait for your lgtm on this one.

yue9944882

/lgtm

In a fat jar, packageURL start with "nested:", not a fat jar, it start with "jar:". (cherry picked from commit ec7e7b9)

…eturn packageURL is not jar or nested, it would be return (cherry picked from commit 88ac227)

re import URLDecoder (cherry picked from commit de71d98)

…release-19 Cherry Pick #2952 To Release 19

#2241 fix bug: when running with a fat jar, need to read entries with…

47ef71f

… openConnection when running in IDE or a thin jar, it is ok, when running with a fat jar in server, new JarFile can not open and read the entries.

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Dec 17, 2023

k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 17, 2023

k8s-ci-robot requested review from brendandburns and yue9944882 December 17, 2023 15:43

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Dec 22, 2023

brendandburns reviewed Dec 27, 2023

View reviewed changes

#2952 fix: packageURL must startwith java: or nested:

ec7e7b9

In a fat jar, packageURL start with "nested:", not a fat jar, it start with "jar:".

#2952 packageURL is not jar or nested, it would be return

88ac227

packageURL is not jar or nested, it would be return

k8s-ci-robot assigned brendandburns Dec 31, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 31, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 31, 2023

#2952 re import URLDecoder

de71d98

re import URLDecoder

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 1, 2024

the packageURL is startWith 'file:' when client is a file in target (…

f69bd5e

…unit test)

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 3, 2024

yue9944882 reviewed Jan 4, 2024

View reviewed changes

yue9944882 reviewed Jan 8, 2024

View reviewed changes

k8s-ci-robot assigned yue9944882 Jan 8, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 8, 2024

k8s-ci-robot merged commit 65b3f8f into kubernetes-client:master Jan 8, 2024

This was referenced Dec 5, 2024

Update Kubernetes client deps spring-cloud/spring-cloud-kubernetes#1805

Closed

Cherry Pick #2952 To Release 19 #3814

Merged

ryanjbaxter pushed a commit to ryanjbaxter/java that referenced this pull request Dec 6, 2024

kubernetes-client#2952 packageURL is not jar or nested, it would be r…

b7699df

…eturn packageURL is not jar or nested, it would be return (cherry picked from commit 88ac227)

ryanjbaxter pushed a commit to ryanjbaxter/java that referenced this pull request Dec 6, 2024

kubernetes-client#2952 re import URLDecoder

07541f1

re import URLDecoder (cherry picked from commit de71d98)

k8s-ci-robot added a commit that referenced this pull request Dec 8, 2024

Merge pull request #3814 from ryanjbaxter/cherry-pick-fat-jar-fix-to-…

04ff2f2

…release-19 Cherry Pick #2952 To Release 19

	logger.error("Loading classes from jar with error packageURL: {}", jarFileName);
	logger.error("Loading classes from jar with error packageURL: {}", packageURL);

Conversation

wkclz commented Dec 17, 2023

Uh oh!

linux-foundation-easycla Bot commented Dec 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Dec 17, 2023

Uh oh!

brendandburns commented Dec 21, 2023

Uh oh!

brendandburns commented Dec 21, 2023

Uh oh!

wkclz commented Dec 23, 2023

Uh oh!

brendandburns Dec 27, 2023

Choose a reason for hiding this comment

Uh oh!

brendandburns commented Dec 27, 2023

Uh oh!

wkclz commented Dec 28, 2023

Uh oh!

brendandburns commented Dec 31, 2023

Uh oh!

k8s-ci-robot commented Dec 31, 2023

Uh oh!

brendandburns commented Jan 2, 2024

Uh oh!

wkclz commented Jan 3, 2024

Uh oh!

yue9944882 commented Jan 4, 2024

Uh oh!

yue9944882 Jan 4, 2024

Choose a reason for hiding this comment

Uh oh!

wkclz Jan 4, 2024

Choose a reason for hiding this comment

Uh oh!

brendandburns commented Jan 6, 2024

Uh oh!

yue9944882 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

linux-foundation-easycla Bot commented Dec 17, 2023 •

edited

Loading