-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Go: Add diagnostic for private registry usage #21252
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds a Go extractor diagnostic that surfaces detected private registry usage on the Tool Status Page, and refactors the proxy/registry helpers into a dedicated package.
Changes:
- Move the registry proxy logic from
utilinto a newregistriespackage and update the toolchain to use it. - Emit a new diagnostic when Go-relevant private registry configurations are detected.
- Add/adjust tests to cover the new diagnostic behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| go/extractor/util/BUILD.bazel | Removes the registry proxy sources/tests from util after the move. |
| go/extractor/toolchain/toolchain.go | Switches GoCommand to apply proxy env vars via the new registries package. |
| go/extractor/toolchain/BUILD.bazel | Adds Bazel dependency on //go/extractor/registries. |
| go/extractor/registries/registryproxy.go | Hosts proxy env-var parsing and now emits the private-registry diagnostic. |
| go/extractor/registries/registryproxy_test.go | Updates test package name to registries after the move. |
| go/extractor/registries/BUILD.bazel | Introduces Bazel targets for the new registries package and its tests. |
| go/extractor/diagnostics/diagnostics.go | Adds EmitPrivateRegistryUsed diagnostic emitter. |
| go/extractor/diagnostics/diagnostics_test.go | Adds a unit test for EmitPrivateRegistryUsed. |
owen-mc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two very minor nits about efficiency, which only really matter if you expect the slices to be long.
owen-mc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (pending tests passing - it's possible my suggestions don't compile or something)
Co-authored-by: Owen Mansel-Chan <62447351+owen-mc@users.noreply.github.com>
d238988 to
d5c4a19
Compare
This PR adds a new diagnostic for when the Go extractor successfully discovers private registries. The goal of this is to make this easy to see on the Tool Status Page, rather than requiring users to follow the process in https://docs.github.com/en/code-security/how-tos/view-and-interpret-data/viewing-code-scanning-logs#determining-whether-code-scanning-default-setup-used-any-private-registries