8000 Use go 1.24 tool directive in go.mod by sunjayBhatia · Pull Request #6955 · projectcontour/contour · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Use go 1.24 tool directive in go.mod #6955

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Use go 1.24 tool directive in go.mod #6955

wants to merge 1 commit into from

Conversation

sunjayBhatia
Copy link
Member

Gets rid of tools.go and tools build tag

@sunjayBhatia
Use go 1.24 tool directive in go.mod
87dbdf5 
Gets rid of tools.go and tools build tag

Signed-off-by: Sunjay Bhatia <sunjay.bhatia@broadcom.com>
@sunjayBhatia sunjayBhatia added the release-note/none-required Marks a PR as not requiring a release note. Should only be used for very small changes. label Mar 21, 2025
@sunjayBhatia sunjayBhatia requested a review from a team as a code owner March 21, 2025 17:27
@sunjayBhatia sunjayBhatia requested review from tsaarni, skriss, a team, izturn and clayton-gonsalves and removed request for a team March 21, 2025 17:27
@codecov Codecov
Copy link
codecov bot commented Mar 21, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.72%. Comparing base (1eab1fe) to head (87dbdf5).
Report is 35 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #6955   +/-   ##
=======================================
  Coverage   80.72%   80.72%           
=======================================
  Files         131      131           
  Lines       19868    19868           
=======================================
  Hits        16039    16039           
  Misses       3537     3537           
  Partials      292      292
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tsaarni
Copy link
Member
tsaarni commented Mar 21, 2025

I'm starting to have some doubts if this is a good idea or not. While some development time dependencies have been included in the past, it seems bit alarming to me how the use of go tool increased the apparent number of dependencies so much. Of course, on the positive side, we now explicitly list all golang dependencies but my concern is that scanners like FOSSA might not be able to differentiate between tool and runtime dependencies, and similarly, CVE scanners analyzing source code could mistakenly flag vulnerabilities in development tools as vulnerabilities of Contour itself. If that is the case, then go tool could become very costly for users who might be forced to address vulnerabilities of 100+ new projects because of this change - not a bad thing as such, but can be overwhelming.

@sunjayBhatia
Copy link
Member Author

I'm starting to have some doubts if this is a good idea or not. While some development time dependencies have been included in the past, it seems bit alarming to me how the use of go tool increased the apparent number of dependencies so much. Of course, on the positive side, we now explicitly list all golang dependencies but my concern is that scanners like FOSSA might not be able to differentiate between tool and runtime dependencies, and similarly, CVE scanners analyzing source 8000 code could mistakenly flag vulnerabilities in development tools as vulnerabilities of Contour itself. If that is the case, then go tool could become very costly for users who might be forced to address vulnerabilities of 100+ new projects because of this change - not a bad thing as such, but can be overwhelming.

my hope is that longer term the go tool dependencies will be fully separated out into their own include stanzas and code scanners will be also improved to not alert on tool deps

but yeah agreed this change does introduce a lot of extra imports

@github-actions GitHub Actions
Copy link
github-actions bot commented May 4, 2025

The Contour project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 30d of inactivity, lifecycle/stale is applied
  • After 60d of inactivity since lifecycle/stale was applied, the PR is closed

You can:

  • Ensure your PR is passing all CI checks. PRs that are fully green are more likely to be reviewed. If you are having trouble with CI checks, reach out to the #contour channel in the Kubernetes Slack workspace.
  • Mark this PR as fresh by commenting or pushing a commit
  • Close this PR
  • Offer to help out with triage

Please send feedback to the #contour channel in the Kubernetes Slack

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsaarni tsaarni Awaiting requested review from tsaarni tsaarni is a code owner automatically assigned from projectcontour/maintainers

@skriss skriss Awaiting requested review from skriss skriss is a code owner automatically assigned from projectcontour/maintainers

@izturn izturn Awaiting requested review from izturn izturn was automatically assigned from projectcontour/contour-reviewers

@clayton-gonsalves clayton-gonsalves Awaiting requested review from clayton-gonsalves clayton-gonsalves was automatically assigned from projectcontour/contour-reviewers

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. release-note/none-required Marks a PR as not requiring a release note. Should only be used for very small changes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sunjayBhatia @tsaarni
0