Skip to content

Conversation

@Zerpet
Copy link
Member

@Zerpet Zerpet commented May 14, 2025

Summary Of Changes

  • Track internal tooling using go.mod directive
  • Add promtool as tool dependency in root go.mod

Additional Context

New in Go 1.24: https://tip.golang.org/doc/go1.24#tools
https://tip.golang.org/doc/modules/managing-dependencies#tools

Important context: I decided to keep the internal tools module because go get -tool adds all the tool's dependencies into your project. That means, we will end up with a much larger go.mod with many require directives for the tool's dependencies. Whilst this in itself is not a problem (that's what go.mod is for), it has the potential to trigger false positives on security scanning tools.

For example, promtool pulls a version of github.com/golang-jwt/jwt that has a known vulnerability. In this case, I realised it was not a vulnerability on our project, but on the tool dependency. For this reason, I decided to keep the internal tools pattern: to minimise false positives from security scanners

There's still some merit to having the tool directive in go.mod, because it pulls more dependencies, which means is more accurately tracking what our project depends on (even if we don't link to it).

Local Testing

Run go install tool and cd internal/tools; go install tool. All tools build successfully and are installed to GOBIN.

@Zerpet Zerpet requested review from MirahImage and mkuratczyk May 14, 2025 15:57
Zerpet added 3 commits May 14, 2025 16:57
There is some incompatibility between our other tools and promtool. If
promtool is included as a tool in tools module, it will fail to build
with a compilation error, calling with the wrong number of arguments a
Kubernetes API function.

As workaround, promtool must be tracked in the root go.mod file.
Copy link
Member

@MirahImage MirahImage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excited for this change

@Zerpet
Copy link
Member Author

Zerpet commented May 14, 2025

@MirahImage I added extra context to the OP. I have some mixed feelings with the result, because I encountered some dependency conflicts between our tools. As a result, one of the tools moved to the "main" go.mod, resulting in a bit of bloat. This is not necessarily a bad thing, unless/until we start receiving false positives from security scanners 🙃

@Zerpet Zerpet merged commit 30b2b20 into main May 19, 2025
20 of 21 checks passed
@Zerpet Zerpet deleted the move-tools branch May 19, 2025 09:26
@Zerpet Zerpet modified the milestones: 2.13.0, 2.14.0 May 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants