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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

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
Comment on lines +98 to +99
cmd.Env = append(cmd.Env, fmt.Sprintf("HTTP_PROXY=%s", proxy_address))
cmd.Env = append(cmd.Env, fmt.Sprintf("HTTPS_PROXY=%s", proxy_address))
Copy link
Contributor

Choose a reason for hiding this comment

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

You can append two things at once. append is variadic.

Suggested change
cmd.Env = append(cmd.Env, fmt.Sprintf("HTTP_PROXY=%s", proxy_address))
cmd.Env = append(cmd.Env, fmt.Sprintf("HTTPS_PROXY=%s", proxy_address))
cmd.Env = append(cmd.Env, fmt.Sprintf("HTTP_PROXY=%s", proxy_address), fmt.Sprintf("HTTPS_PROXY=%s", proxy_address))

Comment on lines +39 to +42
if proxy_port, proxy_port_set := os.LookupEnv(PROXY_PORT); proxy_port_set {
proxy_address = fmt.Sprintf("http://%s:%s", proxy_host, proxy_port)
slog.Info("Found private registry proxy", slog.String("proxy_address", proxy_address))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it the desired behaviour that if the host is set but not the port then the host isn't used? Is there some kind of default port that could be tried? Or should there be some logging in this case, so that when it doesn't work the user can easily see what has happened?

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 wouldn't expect users to set any of these. These environment variables are set in the default setup workflow and we would only ever expect both to be set at the same time. In particular, the proxy uses a random, available port on the runner.

} 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.

Comment on lines +113 to +115
cmd.Env = append(cmd.Env, fmt.Sprintf("GOPROXY=%s", goproxy_val))
cmd.Env = append(cmd.Env, "GOPRIVATE=")
cmd.Env = append(cmd.Env, "GONOPROXY=")
Copy link
Contributor

Choose a reason for hiding this comment

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

As above, you can append them all at once.

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