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

Upgrade argo image to 2.12+ #5232

Closed
Bobgy opened this issue Mar 3, 2021 · 12 comments · Fixed by #5266 or #5277
Closed

Upgrade argo image to 2.12+ #5232

Bobgy opened this issue Mar 3, 2021 · 12 comments · Fixed by #5266 or #5277
Assignees

Comments

@Bobgy
Copy link
Contributor

Bobgy commented Mar 3, 2021

Part of #4553

/cc @NikeNano

@Bobgy
Copy link
Contributor Author

Bobgy commented Mar 3, 2021

/assign

@Bobgy
Copy link
Contributor Author

Bobgy commented Mar 3, 2021

I found that there's now a maintained tool to extract licenses from go libraries: https://github.com/google/go-licenses.

We can use it instead of our home-made tool.

UPDATE: I took a look and it seems this new tool doesn't support go modules, so it could only get licenses using a library's latest version IIUC. See google/go-licenses#33.

@Bobgy
Copy link
Contributor Author

Bobgy commented Mar 3, 2021

@Bobgy
Copy link
Contributor Author

Bobgy commented Mar 4, 2021

Noticed a problem with kubeflow go-license-tools, although

The command go list -m all lists the current module and all its dependencies:

I found that when using this command in argo repo, it lists a dependency: github.com/fasthttp-contrib/websocket. However, this repo doesn't have a license file.

So I kept investigating, and found that argo is not using this module. It's mentioned in go.sum, but not used anywhere in the repo.

Therefore, I can conclude that go list -m all lists too many dependencies, including those not used.

UPDATE: I take it back.

I learned that I can use go mod graph to search for dependency graph. https://stackoverflow.com/a/63779350/8745218

And I figured out that fasthttp-contrib/websocket is introduced by github.com/gavv/httpexpect/v2 v2.0.3

$ go mod graph | grep fasthttp-contrib/websocket
github.com/gavv/httpexpect/v2@v2.0.3 github.com/fasthttp-contrib/websocket@v0.0.0-20160511215533-1f3b11f56072
$ go mod graph | grep httpexpect
github.com/argoproj/argo/v2 github.com/gavv/httpexpect/v2@v2.0.3

httpexpect is only used in tests, but not built in the binary, so it's not a license issue for us.

@Bobgy
Copy link
Contributor Author

Bobgy commented Mar 4, 2021

In summary, two dependencies are problematic:

Both of them were imported by httpexpect v2.0.3 (a dev dependency), so I guess httpexpect isn't very well maintained either.
Well, httpexpect fixed this problem for fasthttp-contrib/websocket in gavv/httpexpect#74, and released 2.1.0 and 2.2.0 after that.

Because they are dev dependencies not built into images, we don't need to worry about them.

@Bobgy
Copy link
Contributor Author

Bobgy commented Mar 5, 2021

There are several images I'll need to generate license info, so I built a better tool to automate this: https://github.com/Bobgy/go-mod-licenses.

@davidspek
Copy link
Contributor

@Bobgy Just came across this, thought it would be relevant. https://blog.argoproj.io/argo-workflows-v3-0-4d0b69f15a6e

@Bobgy
Copy link
Contributor Author

Bobgy commented Mar 8, 2021

@davidspek thanks, rationale for choosing 2.12 was recorded in #4553 (comment)

@Bobgy
Copy link
Contributor Author

Bobgy commented Mar 8, 2021

Hmm, I just discovered https://github.com/github/licensed, it's written in Ruby, but it seems to support a wide range of languages and a robust workflow for working with licenses & cache.

@Bobgy
Copy link
Contributor Author

Bobgy commented Mar 8, 2021

I'm starting to notice some limitations of my approach:

  • dev dependencies are pulled in as well -- this greatly increases number of licenses and their problems to deal with (but in fact they may not be in the binary).
  • only supports repos using one single license (not yet seeing an exception)

Bobgy added a commit that referenced this issue Mar 10, 2021
…5232 (#5266)

* add notices and licenses for argo 2.12

* feat: upgrade argo images to v2.12.9

* update all refs to argo image version

* add NOTICES generation script

* upgrade argo cli to latest

* fix

* fix

* add license_info.csv back

* make release process safer

* add back third_party/license.txt

* refactor(deployment): move argo manifests to third-party, updates for 2.12.9

* update marketplace snapshots

* set up marketplace presubmit test

* add comment
@Bobgy
Copy link
Contributor Author

Bobgy commented Mar 10, 2021

/reopen
I also need to update gcp marketplace manifests

@google-oss-robot
Copy link

@Bobgy: Reopened this issue.

In response to this:

/reopen
I also need to update gcp marketplace manifests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Bobgy added a commit that referenced this issue Mar 11, 2021
…5232 (#5277)

* feat(gcp): upgrade argo to v2.12.9

* add back missing labels

* fixed snapshot test

* add comment

* fix invalid yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants