Skip to content

Update dependencies except for Spring #2768

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 24, 2023

Conversation

rjeberhard
Copy link
Contributor

We're seeing OWASP scan reports about okio, which is included by okhttp3. I'm curious about why Dependabot didn't update this dependency? Whatever the answer, here are all of the missing updates except for those from Spring, which seem to require other, compensating changes.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 21, 2023
@brendandburns
Copy link
Contributor

Thanks for the PR. I'm wondering if dependabot doesn't do this b/c there are explicit versions in okhttp or somesuch?

Anyway, assuming tests pass:

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 21, 2023
@brendandburns
Copy link
Contributor

fwiw, dependabot did send one of them:

#2767

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 22, 2023
@rjeberhard
Copy link
Contributor Author

fwiw, dependabot did send one of them:

#2767

Yes, it's good, of course, that Dependabot found this update. That does leave a mystery of why it never offered PR's for the other updates. I looked at the configuration and it's the simple, typical configuration found in many other projects. Because I could see that the okhttp update had not occurred, I ran mvn versions:display-dependency-updates and reviewed the output.

@brendandburns
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 22, 2023
@brendandburns
Copy link
Contributor

Yeah, it's odd it looks like dependabot has never offered an update for okhttp all of the PRs that I can find that reference okhttp are manual:

https://github.com/search?q=repo%3Akubernetes-client%2Fjava+okhttp&type=pullrequests

@brendandburns
Copy link
Contributor

@rjeberhard looks like the logback dependency doesn't support Java8?

@brendandburns
Copy link
Contributor

@rjeberhard b/c this PR needs a little more work, I merged the protobuf change, so you'll need to rebase. Apologies for the extra work.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 23, 2023
@rjeberhard
Copy link
Contributor Author

@rjeberhard looks like the logback dependency doesn't support Java8?

Yeah, I was too hopeful that I'd be able to do all of the updates. I think that the best plan would be to have specific PR's for each dependency similar to what Dependabot should have created.

@brendandburns
Copy link
Contributor

Ok, individual targetted PRs makes sense.

I also wonder if we need to move java8 support out into a different fork or some other approach to continuing support for Java8...

@brendandburns
Copy link
Contributor

I looked into this more I think logback should move to 1.3.11 not 1.4.11 they claim to be identical except 1.3 supports Java 8...

@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 24, 2023
@rjeberhard
Copy link
Contributor Author

Since this PR is down to just 2 changes, this seems close enough to the goal of having individual PR's. In the future, I'll create individual PR's if Dependabot doesn't generate the necessary updates.

@brendandburns
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 24, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot merged commit 4740c85 into kubernetes-client:master Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants