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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions go/extractor/cli/go-autobuilder/go-autobuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,13 @@ func restoreRepoLayout(fromDir string, dirEntries []string, scratchDirName strin

// addVersionToMod add a go version directive, e.g. `go 1.14` to a `go.mod` file.
func addVersionToMod(version string) bool {
cmd := exec.Command("go", "mod", "edit", "-go="+version)
cmd := toolchain.GoCommand("mod", "edit", "-go="+version)
return util.RunCmd(cmd)
}

// checkVendor tests to see whether a vendor directory is inconsistent according to the go frontend
func checkVendor() bool {
vendorCheckCmd := exec.Command("go", "list", "-mod=vendor", "./...")
vendorCheckCmd := toolchain.GoCommand("list", "-mod=vendor", "./...")
outp, err := vendorCheckCmd.CombinedOutput()
if err != nil {
badVendorRe := regexp.MustCompile(`(?m)^go: inconsistent vendoring in .*:$`)
Expand Down Expand Up @@ -438,7 +438,7 @@ func installDependencies(workspace project.GoWorkspace) {
util.RunCmd(vendor)
}

install = exec.Command("go", "get", "-v", "./...")
install = toolchain.GoCommand("get", "-v", "./...")
install.Dir = path
log.Printf("Installing dependencies using `go get -v ./...` in `%s`.\n", path)
util.RunCmd(install)
Expand Down
17 changes: 12 additions & 5 deletions go/extractor/toolchain/toolchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,16 @@ func SupportsWorkspaces() bool {
return GetEnvGoSemVer().IsAtLeast(V1_18)
}

// Constructs a `*exec.Cmd` for `go` with the specified arguments.
func GoCommand(arg ...string) *exec.Cmd {
cmd := exec.Command("go", arg...)
util.ApplyProxyEnvVars(cmd)
return cmd
}

// Run `go mod tidy -e` in the directory given by `path`.
func TidyModule(path string) *exec.Cmd {
cmd := exec.Command("go", "mod", "tidy", "-e")
cmd := GoCommand("mod", "tidy", "-e")
cmd.Dir = path
return cmd
}
Expand All @@ -159,21 +166,21 @@ func InitModule(path string) *exec.Cmd {
}
}

modInit := exec.Command("go", "mod", "init", moduleName)
modInit := GoCommand("mod", "init", moduleName)
modInit.Dir = path
return modInit
}

// Constructs a command to run `go mod vendor -e` in the directory given by `path`.
func VendorModule(path string) *exec.Cmd {
modVendor := exec.Command("go", "mod", "vendor", "-e")
modVendor := GoCommand("mod", "vendor", "-e")
modVendor.Dir = path
return modVendor
}

// Constructs a command to run `go version`.
func Version() *exec.Cmd {
version := exec.Command("go", "version")
version := GoCommand("version")
return version
}

Expand Down Expand Up @@ -209,7 +216,7 @@ func RunListWithEnv(format string, patterns []string, additionalEnv []string, fl
func ListWithEnv(format string, patterns []string, additionalEnv []string, flags ...string) *exec.Cmd {
args := append([]string{"list", "-e", "-f", format}, flags...)
args = append(args, patterns...)
cmd := exec.Command("go", args...)
cmd := GoCommand(args...)
cmd.Env = append(os.Environ(), additionalEnv...)
return cmd
}
Expand Down
2 changes: 2 additions & 0 deletions go/extractor/util/BUILD.bazel

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

117 changes: 117 additions & 0 deletions go/extractor/util/registryproxy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
package util

import (
"encoding/json"
"fmt"
"log/slog"
"os"
"os/exec"
"strings"
)

const PROXY_HOST = "CODEQL_PROXY_HOST"
const PROXY_PORT = "CODEQL_PROXY_PORT"
const PROXY_CA_CERTIFICATE = "CODEQL_PROXY_CA_CERTIFICATE"
const PROXY_URLS = "CODEQL_PROXY_URLS"
const GOPROXY_SERVER = "goproxy_server"

type RegistryConfig struct {
Type string `json:"type"`
URL string `json:"url"`
}

var proxy_address string
var proxy_cert_file string
var proxy_configs []RegistryConfig

// Tries to parse the given string value into an array of RegistryConfig values.
func parseRegistryConfigs(str string) ([]RegistryConfig, error) {
var configs []RegistryConfig
if err := json.Unmarshal([]byte(str), &configs); err != nil {
return nil, err
}

return configs, nil
}

func checkEnvVars() {
if proxy_host, proxy_host_set := os.LookupEnv(PROXY_HOST); proxy_host_set {
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))
}
Comment on lines +39 to +42
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.

}

if proxy_cert, proxy_cert_set := os.LookupEnv(PROXY_CA_CERTIFICATE); proxy_cert_set {
// Write the certificate to a temporary file
slog.Info("Found certificate")

f, err := os.CreateTemp("", "codeql-proxy.crt")
if err != nil {
slog.Error("Unable to create temporary file for the proxy certificate", slog.String("error", err.Error()))
}

_, err = f.WriteString(proxy_cert)
if err != nil {
slog.Error("Failed to write to the temporary certificate file", slog.String("error", err.Error()))
}

err = f.Close()
if err != nil {
slog.Error("Failed to close the temporary certificate file", slog.String("error", err.Error()))
} else {
proxy_cert_file = f.Name()
}
}

if proxy_urls, proxy_urls_set := os.LookupEnv(PROXY_URLS); proxy_urls_set {
val, err := parseRegistryConfigs(proxy_urls)
if err != nil {
slog.Error("Unable to parse proxy configurations", slog.String("error", err.Error()))
} 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.

for _, cfg := range val {
if cfg.Type == GOPROXY_SERVER {
proxy_configs = append(proxy_configs, cfg)
slog.Info("Found GOPROXY server", slog.String("url", cfg.URL))
}
}
}
}
}

// Applies private package proxy related environment variables to `cmd`.
func ApplyProxyEnvVars(cmd *exec.Cmd) {
slog.Info(
"Applying private registry proxy environment variables",
slog.String("cmd_args", strings.Join(cmd.Args, " ")),
)

checkEnvVars()

// Preserve environment variables
cmd.Env = os.Environ()

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

}
if proxy_cert_file != "" {
slog.Info("Setting SSL_CERT_FILE", slog.String("proxy_cert_file", proxy_cert_file))
cmd.Env = append(cmd.Env, fmt.Sprintf("SSL_CERT_FILE=%s", proxy_cert_file))
}

if len(proxy_configs) > 0 {
goproxy_val := "https://proxy.golang.org,direct"

for _, cfg := range proxy_configs {
goproxy_val = cfg.URL + "," + goproxy_val
}

cmd.Env = append(cmd.Env, fmt.Sprintf("GOPROXY=%s", goproxy_val))
cmd.Env = append(cmd.Env, "GOPRIVATE=")
cmd.Env = append(cmd.Env, "GONOPROXY=")
Comment on lines +113 to +115
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.

}
}
49 changes: 49 additions & 0 deletions go/extractor/util/registryproxy_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package util

import (
"testing"
)

func parseRegistryConfigsFail(t *testing.T, str string) {
_, err := parseRegistryConfigs(str)

if err == nil {
t.Fatal("Expected `parseRegistryConfigs` to fail, but it succeeded.")
}
}

func parseRegistryConfigsSuccess(t *testing.T, str string) []RegistryConfig {
val, err := parseRegistryConfigs(str)

if err != nil {
t.Fatalf("Expected `parseRegistryConfigs` to succeed, but it failed: %s", err.Error())
}

return val
}

func TestParseRegistryConfigs(t *testing.T) {
// parseRegistryConfigsFail(t, "")

empty := parseRegistryConfigsSuccess(t, "[]")

if len(empty) != 0 {
t.Fatal("Expected `parseRegistryConfigs(\"[]\")` to return no configurations, but got some.")
}

single := parseRegistryConfigsSuccess(t, "[{ \"type\": \"goproxy_server\", \"url\": \"https://proxy.example.com/mod\" }]")

if len(single) != 1 {
t.Fatalf("Expected `parseRegistryConfigs` to return one configuration, but got %d.", len(single))
}

first := single[0]

if first.Type != "goproxy_server" {
t.Fatalf("Expected `Type` to be `goproxy_server`, but got `%s`", first.Type)
}

if first.URL != "https://proxy.example.com/mod" {
t.Fatalf("Expected `URL` to be `https://proxy.example.com/mod`, but got `%s`", first.URL)
}
}