Skip to content

Go: Support private registries via GOPROXY #19248

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

Merged
merged 9 commits into from
Apr 25, 2025
Merged

Conversation

mbg
Copy link
Member

@mbg mbg commented Apr 8, 2025

This PR is part of work to enable private package registries to be used in Default Setup. See prior work for C#: #18029 and #18850

The existing Default Setup workflow will initialise the Dependabot package proxy, if a private package registry configuration is set. The host, port, certificate, and configurations used by the proxy are then passed to CodeQL in the analyze step. For Go, we will likely need to modify this to make these environment variables available to the autobuild step as well.

The changes in this PR modify the Go extractor to recognise when the corresponding environment variables are set. If so, we use the data from those environment variables to:

  1. Construct the proxy address from the host and port and pass it to go via the HTTP_PROXY and HTTPS_PROXY environment variables.
  2. Store the certificate on disk, and pass the path to go via SSL_CERT_FILE.
  3. Identify goproxy_server configurations and use them to set an appropriate value for the GOPROXY environment variable.

This has the effect that go will attempt to make requests to obtain packages to the GOPROXY servers. These will go via the Dependabot proxy configured by HTTP_PROXY and HTTPS_PROXY, which handles the needed authentication for the GOPROXY servers.

@github-actions github-actions bot added the Go label Apr 8, 2025
} else {
// We only care about private registry configurations that are relevant to Go and
// filter others out at this point.
proxy_configs = make([]RegistryConfig, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

You could use the length of val as a third argument to make, which specifies the capacity of the underlying array. Or maybe it isn't worth it if you only ever expect very few.

Copy link
Member Author

Choose a reason for hiding this comment

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

We only expect a few (indeed, the UI only supports configuring one at the moment). That said, I have noticed since that calling make to initialise the array isn't necessary since append will apparently do this if it is nil anyway.

@mbg mbg force-pushed the mbg/go/private-registries branch from 425ea73 to e805d1e Compare April 25, 2025 11:55
@mbg mbg marked this pull request as ready for review April 25, 2025 11:56
@mbg mbg requested a review from a team as a code owner April 25, 2025 11:56
@mbg mbg requested a review from owen-mc April 25, 2025 11:56
Comment on lines +23 to +31
// The address of the proxy including protocol and port (e.g. http://localhost:1234)
var proxy_address string

// The path to the temporary file that stores the proxy certificate, if any.
var proxy_cert_file string

// An array of registry configurations that are relevant to Go.
// This excludes other registry configurations that may be available, but are not relevant to Go.
var proxy_configs []RegistryConfig
Copy link
Contributor

Choose a reason for hiding this comment

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

These no longer need to be global variables. I think it would be clearer if they were just local variables.

Copy link
Member Author

Choose a reason for hiding this comment

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

I somewhat intentionally left these as global variables so that we can access their values elsewhere if needed going forward.

I don't feel strongly about this though, so if you would prefer it to be locals for now while they don't have to be globals, then I can make that change.

@mbg mbg requested a review from owen-mc April 25, 2025 14:45
@mbg mbg merged commit 987af4c into main Apr 25, 2025
14 checks passed
@mbg mbg deleted the mbg/go/private-registries branch April 25, 2025 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants