Skip to content
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

Third party libraries are not latest version #2759

Closed
nak3 opened this issue Jun 21, 2023 · 11 comments · Fixed by #2768
Closed

Third party libraries are not latest version #2759

nak3 opened this issue Jun 21, 2023 · 11 comments · Fixed by #2768
Labels
area/test-and-release kind/bug Categorizes issue or PR as related to a bug. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/question

Comments

@nak3
Copy link
Contributor

nak3 commented Jun 21, 2023

/area test-and-release
/kind question
/kind bug
/kind cleanup

Expected Behavior

  • All deps in go.mod except for k8s libs should be the latest versions (if no reason to stay the old version).

Actual Behavior

Additional Info

  • I haven't found any security issue but our org has a security survey with the question "Using the latest third party libraries?".
  • It might be an issue of hack/update-deps.sh.
@knative-prow knative-prow bot added area/test-and-release kind/question kind/bug Categorizes issue or PR as related to a bug. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. labels Jun 21, 2023
@nak3
Copy link
Contributor Author

nak3 commented Jun 21, 2023

Good timing to see #2758, it looks like manual operation for bump is necessary. Is it better to add gihub action to bump the deps?

@dprotaso
Copy link
Member

dprotaso commented Jun 21, 2023

We should setup dependabot for this repo and exclude knative and k8s deps from being bumped (since we have other automation or do it manually)

@nak3
Copy link
Contributor Author

nak3 commented Jun 28, 2023

It seems that adding dependabot will work well 🎉

Maybe we should add it to net-kourier or a few repos first then add it to the bot as in common knobot - https://github.com/knative-sandbox/knobots (after 1.11 cut and agreemet with the TOC or leads.)

@pierDipi
Copy link
Member

Regarding dependabot see prior art and thoughts: #1942

@nak3
Copy link
Contributor Author

nak3 commented Jun 29, 2023

Ah, thank you!

@nak3
Copy link
Contributor Author

nak3 commented Jul 5, 2023

The following command line bumps all thrid-party libs except for k8s.io and knative.dev.

source vendor/knative.dev/hack/library.sh

for domain in `ls vendor --ignore={k8s.io,knative.dev}` ; do
  echo "Bump $domain"
  go_update_deps --upgrade --domain ${domain}
done

Although we can add it to common update deps (maybe adding it to https://github.com/knative-sandbox/knobots/blob/main/actions/update-deps/entrypoint.sh ) but some repos wants to exclude some specific libs - e.g. Istio does wants to keep istio.io lib version.
So, I am thinking that we should add each repo rather than common action. Please let me know if you have any idea.

@nak3
Copy link
Contributor Author

nak3 commented Jul 6, 2023

Above command does not work perfectly... (I think buoy's issue, though...)

For example github.com/blang/semver/v4, which does not have go.mod in top directory https://github.com/blang/semver/tree/master/v4 fails with

$ go_update_deps --upgrade --domain github.com
=== Update Deps for Golang module: knative.dev/pkg
--- Upgrading to release v9000.1
Error: unable to fetch go import https://github.com/blang/semver/v4?go-get=1: missing <meta name=go-import> in the node tree
Usage:
  buoy float go.mod [flags]

Flags:
  -d, --domain string           domain filter (i.e. knative.dev) [required] (default "knative.dev")
  -h, --help                    help for float
  -m, --module-release string   if the go modules are a different release set than the release, use --module-release, should be '<major>.<minor>' (i.e.: 0.12 or v0.12)
  -r, --release string          release should be '<major>.<minor>' (i.e.: 1.23 or v1.23) [required]
      --ruleset string          The ruleset to evaluate the dependency refs. Rulesets: [Any, ReleaseOrBranch, Release, Branch] (default "Any")

EDIT: Fixed by knative/toolbox#13

@rhuss
Copy link
Contributor

rhuss commented Jul 6, 2023

Also, I think instead of ls --ignore, which is not available everywhere, you should probably use the more safe ls ... | grep -v ... | grep -v which is POSIX compliant.

@ReToCode
Copy link
Member

ReToCode commented Jul 7, 2023

Just as an addition, we also should look into bumping k8s dependencies if possible. We have some got some feedback that this also causes issues for tekton:

how "well" maintained is knative/pkg ? 😝
Mainly asking because.. it still tracks k8s.io/* 0.25.4... which.. becomes more and more a > problem for downstream dependencies (like tektoncd/…)

vdemeester added a commit to vdemeester/tektoncd-pipeline that referenced this issue Jul 7, 2023
This comes with a challenge, which is `k8s.io` dependencies, and thus `replace` is coming
back in the picture (sadly). This will have to be fixed once `knative/pkg` k8s.io
dependencies are up-to-date or at least *compatible* with the rest of the library we use
(`go-containerregistry` is the example today).

This is tracked in knative/pkg here : knative/pkg#2759.

Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
vdemeester added a commit to vdemeester/tektoncd-pipeline that referenced this issue Jul 7, 2023
This comes with a challenge, which is `k8s.io` dependencies, and thus `replace` is coming
back in the picture (sadly). This will have to be fixed once `knative/pkg` k8s.io
dependencies are up-to-date or at least *compatible* with the rest of the library we use
(`go-containerregistry` is the example today).

This is tracked in knative/pkg here : knative/pkg#2759.

Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
vdemeester added a commit to vdemeester/tektoncd-pipeline that referenced this issue Jul 7, 2023
This comes with a challenge, which is `k8s.io` dependencies, and thus `replace` is coming
back in the picture (sadly). This will have to be fixed once `knative/pkg` k8s.io
dependencies are up-to-date or at least *compatible* with the rest of the library we use
(`go-containerregistry` is the example today).

This is tracked in knative/pkg here : knative/pkg#2759.

Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
tekton-robot pushed a commit to tektoncd/pipeline that referenced this issue Jul 7, 2023
This comes with a challenge, which is `k8s.io` dependencies, and thus `replace` is coming
back in the picture (sadly). This will have to be fixed once `knative/pkg` k8s.io
dependencies are up-to-date or at least *compatible* with the rest of the library we use
(`go-containerregistry` is the example today).

This is tracked in knative/pkg here : knative/pkg#2759.

Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
@dprotaso
Copy link
Member

in addition to dependabot you will need a github action to run ./hack/update-codegen.sh in dependabot's PR

https://github.com/orgs/community/discussions/48498#discussioncomment-5159337

@nak3
Copy link
Contributor Author

nak3 commented Jul 11, 2023

I opened knative-extensions/net-gateway-api#498. Could anyone take a look?

khrm added a commit to khrm/triggers that referenced this issue Aug 9, 2023
khrm added a commit to khrm/triggers that referenced this issue Aug 24, 2023
khrm added a commit to khrm/triggers that referenced this issue Aug 24, 2023
khrm added a commit to khrm/triggers that referenced this issue Aug 24, 2023
khrm added a commit to khrm/triggers that referenced this issue Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/test-and-release kind/bug Categorizes issue or PR as related to a bug. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/question
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants