Skip to content

Conversation

@bjornjorgensen
Copy link
Contributor

@bjornjorgensen bjornjorgensen commented Mar 11, 2023

What changes were proposed in this pull request?

Upgrade fabric8:kubernetes-client from 6.4.1 to 6.5.0

Release notes

Why are the changes needed?

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass GA

@github-actions github-actions bot added the BUILD label Mar 11, 2023
@bjornjorgensen
Copy link
Contributor Author

@dongjoon-hyun FYI

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Thank you for pinging me.

@bjornjorgensen bjornjorgensen changed the title [SPARK-42761] Upgrade fabric8:kubernetes-client to 6.5.0 [SPARK-42761][BUILD] Upgrade fabric8:kubernetes-client to 6.5.0 Mar 11, 2023
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

For the record, this looks like a false alarm. I don't think Apache Spark is affected by SnakeYAML issue because fabric8io community also got the confirmation from snakeyml community officially that they are not affected.

Could you double check it, @bjornjorgensen ?

@dongjoon-hyun
Copy link
Member

If you agree with the above assessment, please remove the misleading CVE information from the PR description, @bjornjorgensen .

@bjornjorgensen
Copy link
Contributor Author

Well, the comment that you are refereeing to, have a link but I cant get in
image

3 weeks later they merged a PR fabric8io/kubernetes-client@43b04f6 that fix this issue.

And yesterday SNYK open a PR to my repo for this issue. bjornjorgensen#102
I can always change the text for this PR, but I haven't seen anything that makes me believe that kubernets-client is not affected by this CVE.

@bjornjorgensen
Copy link
Contributor Author

The maintainers of the library contend that the application's trust would already have had to be compromised or established and therefore dispute the risk associated with this issue on the basis that there is a high bar for exploitation. Thus, no fix is expected.

https://security.snyk.io/vuln/SNYK-JAVA-ORGYAML-3152153

This is part of snakeyaml release notes

2.0 (2023-02-26)

Fix #570: SafeConstructor ignores LoaderOptions setCodePointLimit() (thanks to Robert Patrick)

Update #565: (Backwards-incompatible) Do not allow global tags by default to fix CVE-2022-1471 (thanks to Jonathan Leitschuh)

1.32 (2022-09-12)

Fix #547: Set the limit for incoming data to prevent a CVE report in NIST. By default it is 3MB

https://bitbucket.org/snakeyaml/snakeyaml/wiki/Changes

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Mar 12, 2023

@bjornjorgensen . Do you mean you can not trust winniegy, the fabric8io community member's comment? He close the issue after that comment.
Screenshot 2023-03-12 at 1 26 33 PM

Hence, the migration happens independently from the CVE. It's just for the future release.

In short, the following claim sounds wrong to me according to the context.

3 weeks later they merged a PR fabric8io/kubernetes-client@43b04f6 that fix this issue.

@dongjoon-hyun
Copy link
Member

BTW, I have two additional questions for the following PR you referred.

  1. Do you mean it's the evidence of the previous assessment?

SafeConstructor ignores LoaderOptions setCodePointLimit() (thanks to Robert Patrick)

  1. Do you think the following major version change is safe to us?
- <snakeyaml.version>1.33</snakeyaml.version>
+ <snakeyaml.version>2.5</snakeyaml.version>

@bjornjorgensen
Copy link
Contributor Author

ok, I didn't know who this user was. I have updated the PR text now.

@bjornjorgensen
Copy link
Contributor Author

Now that it turns out that the information that I have been given is not correct. This change the whole picture with why we should include this PR. I think we can wait until we are done with 3.4 so that we are on the safe side.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Thank you for updating, @bjornjorgensen . +1, LGTM for Apache Spark 3.5.0 for now.
Let's merge and test this new dependency together in master branch for a while.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-42761][BUILD] Upgrade fabric8:kubernetes-client to 6.5.0 [SPARK-42761][BUILD] Upgrade kubernetes-client to 6.5.0 Mar 12, 2023
@dongjoon-hyun dongjoon-hyun changed the title [SPARK-42761][BUILD] Upgrade kubernetes-client to 6.5.0 [SPARK-42761][BUILD][K8S] Upgrade kubernetes-client to 6.5.0 Mar 12, 2023
@fryz
Copy link

fryz commented Apr 20, 2023

Hey @dongjoon-hyun,

Will this fix will be backported to other maintained branches (specifically the one I care about is the 3.3 branch)?

Thanks!

@bjornjorgensen
Copy link
Contributor Author

spark/pom.xml

Line 213 in cd16624

<kubernetes-client.version>5.12.2</kubernetes-client.version>
upgrade from 5.12.2 to 6.5.0 thats alot of work.

@bjornjorgensen
Copy link
Contributor Author

Are there any reasons why I need to get this error messages like this one?
image

@dongjoon-hyun
Copy link
Member

To @fryz . As @bjornjorgensen mentioned, the answer is no.

Will this fix will be backported to other maintained branches (specifically the one I care about is the 3.3 branch)?

@bjornjorgensen bjornjorgensen deleted the kub-client6.5.0 branch June 1, 2023 19:38
snmvaughan pushed a commit to snmvaughan/spark that referenced this pull request Jun 20, 2023
### What changes were proposed in this pull request?
Upgrade fabric8:kubernetes-client from 6.4.1 to 6.5.0

[Release notes](https://github.com/fabric8io/kubernetes-client/releases/tag/v6.5.0)
### Why are the changes needed?

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Pass GA

Closes apache#40381 from bjornjorgensen/kub-client6.5.0.

Authored-by: bjornjorgensen <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants