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

Bump golang.org/x/tools from 0.9.3 to 0.11.0 in /hack/tools #9063

Closed
1 task
killianmuldoon opened this issue Jul 24, 2023 · 8 comments · Fixed by #9288
Closed
1 task

Bump golang.org/x/tools from 0.9.3 to 0.11.0 in /hack/tools #9063

killianmuldoon opened this issue Jul 24, 2023 · 8 comments · Fixed by #9288
Assignees
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@killianmuldoon
Copy link
Contributor

killianmuldoon commented Jul 24, 2023

The dependabot PR to update golang.org/x/tools from 0.9.3 to 0.11.0 resulted in a deprecation warning which caused the linter to fail with

hack/tools/mdbook/releaselink/releaselink.go:32:2: SA1019: "golang.org/x/tools/go/vcs" is deprecated: Use the go list command with -json flag instead, which implements up-to-date import path resolution behavior, module support, and includes the latest security fixes. (staticcheck)
        **"golang.org/x/tools/go/vcs":**

Related bump: #9052

The entire VCS package, which is used in the release link tool to translate the go module into the github URL, has been deprecated and AFAIU there is no replacement package. The advice is to use go list, but I don't know how this can be used in this case.

An alternative would be to take the parts of the code we need from the VCS package and keep them as part of the release link tool.

Fundamentally what we need is a way to map the go module to the URL location of the repo. This tool is used in other projects - at least in CAPA from a quick browse - so we can't simply hardcode the URL.

Using sigs.k8s.io/cluster-api as the base of the URL does not work for downloading release artifacts i.e. https://sigs.k8s.io/cluster-api/releases redirects to https://github.com/kubernetes-sigs/cluster-api/blob/main/releases

Tasks

@killianmuldoon
Copy link
Contributor Author

/help

@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jul 24, 2023
@k8s-ci-robot
Copy link
Contributor

@killianmuldoon:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

  • Why are we solving this issue?
  • To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
  • Does this issue have zero to low barrier of entry?
  • How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/help

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.

@k8s-ci-robot k8s-ci-robot added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Jul 24, 2023
@sbueringer
Copy link
Member

/triage accepted

Thx for opening the issue

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 24, 2023
@sbueringer
Copy link
Member

Fundamentally what we need is a way to map the go module to the URL location of the repo. This tool is used in other projects - at least in CAPA from a quick browse - so we can't simply hardcode the URL.

Maybe we can make it configurable in some way (worst case env var)

@killianmuldoon
Copy link
Contributor Author

Maybe we can make it configurable in some way (worst case env var)

Yeah - we could add it as a variable passed into the command where release link is called, but it would break other users any way we make this a requirement. Depends on how much value we see in maintaining this tool properly - we could deprecate the public version and move a more tightly coupled version into an internal package.

@killianmuldoon
Copy link
Contributor Author

As a bit of extra contest - this is currently used in Cluster API providers for AWS, Cloudstack, IBMCloud and Openstack in public kubernetes sigs repos - https://cs.k8s.io/?q=sigs.k8s.io%2Fcluster-api%2Fhack%2Ftools%2Fmdbook%2Freleaselink&i=nope&files=&excludeFiles=&repos=

@sbueringer
Copy link
Member

sbueringer commented Aug 3, 2023

Note: Once we did this we have to enable dependabot for this dependency again: #9109 (comment)

@killianmuldoon Please add this as a task to the issue description

@killianmuldoon
Copy link
Contributor Author

/assign

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants