-
Notifications
You must be signed in to change notification settings - Fork 528
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
Bump deps, add script to make it easier #1260
Conversation
pkg/authn/kubernetes/go.mod
Outdated
k8s.io/api v0.23.2 | ||
k8s.io/apimachinery v0.23.2 | ||
k8s.io/client-go v0.23.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't we explicitly leave these at 0.22?
cc @dprotaso
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ping. I'd like to move forward with this, but if there's something we should do to make Knative's and Knative-using packages' lives easier, I'm all ears.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't we explicitly leave these at 0.22?
@mattmoor -- There was a request to use 0.22.5, but there wasn't a reason provided:
Would suggest using 0.23.3, unless there's a good reason not to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tl;dr It's because that's what Knative and Tekton are pinned to. I'll defer to @dprotaso to explain the cadence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't Knative and Tekton solve this with a replace k8s.io/foo => k8s.io/foo v0.22.5
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They could, based on my understanding.
I think it's important that the upstream take input from downstreams, but set the pace by keeping deps as up-to-date as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll be bumping to 0.23 in one month. K8s 1.21 is still in supported for the next two months: https://kubernetes.io/releases/patch-releases/#1-21
Codecov Report
@@ Coverage Diff @@
## main #1260 +/- ##
==========================================
+ Coverage 73.78% 73.92% +0.13%
==========================================
Files 111 111
Lines 8289 8345 +56
==========================================
+ Hits 6116 6169 +53
- Misses 1571 1573 +2
- Partials 602 603 +1
Continue to review full report at Codecov.
|
This is effectively blocking automatic security updates, like #1287, since dependabot can't manage our deps without help. |
Dependabot gets tripped up by our multi-module repo, so instead I wrote a script that should automate it for us in
hack/bump-deps.sh
.If we want, we can hook it up to GitHub Actions to create PRs for us and drop dependabot, or we can just use dependabot to notify us when bumps are needed and do it manually, like I've been doing lately. A benefit to this is that it's one big PR instead of individual ones.
lmk what you think.
This PR gets us past a containerd dependabot alert that doesn't affect us, but spams us and downstream users with the alert.