diff --git a/README.md b/README.md index 032e84c3..0799d2ef 100644 --- a/README.md +++ b/README.md @@ -259,6 +259,22 @@ set OCTOPUS_API_KEY="API-XXXXXXXXXXXXXXXXXXXXXXXXXXXXX" # replace with your API octopus.exe space list # should list all the spaces ``` +### Proxies + +The CLI honours the standard `HTTP_PROXY`, `HTTPS_PROXY` and `NO_PROXY` environment variables. + +To point the CLI at a proxy without affecting other tools, set `OCTOPUS_PROXY` (or the `ProxyUrl` config key, +which `OCTOPUS_PROXY` overrides). It applies to both http and https requests, and `NO_PROXY` still applies. +`http`, `https`, `socks5` and `socks5h` proxy urls are supported. + +```shell +export OCTOPUS_PROXY="http://proxy.example.com:3128" +``` + +Credentials can be embedded in the proxy url, or supplied separately with `OCTOPUS_PROXY_USERNAME` and +`OCTOPUS_PROXY_PASSWORD`. Credentials are read from the environment only, so a proxy password is never +written to the CLI config file. + ### go-octopusdeploy library The CLI depends heavily on the [go-octopusdeploy](https://github.com/OctopusDeploy/go-octopusdeploy) library, which manages diff --git a/go.mod b/go.mod index a46e3a7b..a89bf9c0 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,7 @@ require ( github.com/spf13/viper v1.21.0 github.com/stretchr/testify v1.11.1 golang.org/x/exp v0.0.0-20230129154200-a960b3787bd2 + golang.org/x/net v0.57.0 golang.org/x/term v0.45.0 ) @@ -53,7 +54,6 @@ require ( github.com/subosito/gotenv v1.6.0 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect golang.org/x/crypto v0.54.0 // indirect - golang.org/x/net v0.57.0 // indirect golang.org/x/sys v0.47.0 // indirect golang.org/x/text v0.40.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/pkg/apiclient/client_factory.go b/pkg/apiclient/client_factory.go index 93c73abe..1ca3fd6a 100644 --- a/pkg/apiclient/client_factory.go +++ b/pkg/apiclient/client_factory.go @@ -1,7 +1,6 @@ package apiclient import ( - "crypto/tls" "errors" "fmt" "net/url" @@ -121,13 +120,18 @@ func NewClientFactoryFromConfig(ask question.AskProvider) (ClientFactory, error) return nil, errs } - http.DefaultTransport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + transport, err := NewHttpTransport(ProxySettingsFromConfig(), true) + if err != nil { + return nil, err + } // The spinner is only wanted in interactive mode, but that is not settled // yet: this runs before cobra parses --no-prompt. The round-tripper decides // per request instead. + spinnerRoundTripper := NewSpinnerRoundTripper(ask) + spinnerRoundTripper.Next = transport httpClient := &http.Client{ - Transport: NewSpinnerRoundTripper(ask), + Transport: spinnerRoundTripper, } var credentials octopusApiClient.ICredential diff --git a/pkg/apiclient/proxy.go b/pkg/apiclient/proxy.go new file mode 100644 index 00000000..4bd90b59 --- /dev/null +++ b/pkg/apiclient/proxy.go @@ -0,0 +1,122 @@ +package apiclient + +import ( + "crypto/tls" + "fmt" + "net/http" + "net/url" + "os" + + "github.com/OctopusDeploy/cli/pkg/constants" + "github.com/spf13/viper" + "golang.org/x/net/http/httpproxy" +) + +// ProxySettings is the CLI's proxy configuration. +// +// Url takes precedence over the standard HTTP_PROXY/HTTPS_PROXY variables and +// applies to both schemes; when it is empty those variables are used instead. +// NO_PROXY is honoured either way. http, https, socks5 and socks5h proxies are +// supported, all by net/http itself. +type ProxySettings struct { + Url string + Username string + Password string +} + +// ProxySettingsFromConfig reads the proxy settings from the viper config, which +// covers the ProxyUrl config file key and the OCTOPUS_PROXY environment variable. +// The credentials are deliberately read from the environment only, so that a +// proxy password is never written to the config file in plain text. +func ProxySettingsFromConfig() ProxySettings { + return ProxySettings{ + Url: viper.GetString(constants.ConfigProxyUrl), + Username: os.Getenv(constants.EnvOctopusProxyUsername), + Password: os.Getenv(constants.EnvOctopusProxyPassword), + } +} + +// ProxyFunc returns a function suitable for http.Transport.Proxy. +func (s ProxySettings) ProxyFunc() (func(*http.Request) (*url.URL, error), error) { + config := httpproxy.FromEnvironment() + if s.Url != "" { + // httpproxy silently ignores a proxy address it cannot parse, so validate it here + // to report a typo rather than quietly connecting directly. + if _, err := parseProxyUrl(s.Url); err != nil { + return nil, err + } + config.HTTPProxy = s.Url + config.HTTPSProxy = s.Url + config.CGI = false + } + + proxyForUrl := config.ProxyFunc() + return func(request *http.Request) (*url.URL, error) { + proxyUrl, err := proxyForUrl(request.URL) + if err != nil || proxyUrl == nil { + return nil, err + } + return s.applyCredentials(proxyUrl), nil + }, nil +} + +// applyCredentials adds the configured proxy credentials, unless the proxy url +// already carries its own. +func (s ProxySettings) applyCredentials(proxyUrl *url.URL) *url.URL { + if s.Username == "" || proxyUrl.User != nil { + return proxyUrl + } + withCredentials := *proxyUrl + withCredentials.User = url.UserPassword(s.Username, s.Password) + return &withCredentials +} + +// NewHttpTransport returns the transport the CLI uses to talk to Octopus. It is +// a clone of http.DefaultTransport so the standard defaults are kept, with the +// proxy resolution replaced by ours. +func NewHttpTransport(settings ProxySettings, insecureSkipVerify bool) (*http.Transport, error) { + proxyFunc, err := settings.ProxyFunc() + if err != nil { + return nil, err + } + + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.Proxy = proxyFunc + if insecureSkipVerify { + transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + } + return transport, nil +} + +// RedactProxyUrl removes the password from a proxy url so it can be displayed. +func RedactProxyUrl(rawUrl string) string { + if rawUrl == "" { + return "" + } + parsed, err := parseProxyUrl(rawUrl) + if err != nil { + return "***" // can't parse it, so we can't tell whether it holds a password + } + if parsed.User == nil { + return rawUrl + } + return parsed.Redacted() +} + +// parseProxyUrl mirrors how net/http parses a proxy address: a bare "host:port" +// is treated as http. +func parseProxyUrl(rawUrl string) (*url.URL, error) { + parsed, err := url.Parse(rawUrl) + if err != nil || parsed.Scheme == "" || parsed.Host == "" { + if withScheme, schemeErr := url.Parse("http://" + rawUrl); schemeErr == nil && withScheme.Host != "" { + return withScheme, nil + } + } + if err != nil { + return nil, fmt.Errorf("invalid proxy url '%s': %w", rawUrl, err) + } + if parsed.Host == "" { + return nil, fmt.Errorf("invalid proxy url '%s': no host specified", rawUrl) + } + return parsed, nil +} diff --git a/pkg/apiclient/proxy_test.go b/pkg/apiclient/proxy_test.go new file mode 100644 index 00000000..68603da2 --- /dev/null +++ b/pkg/apiclient/proxy_test.go @@ -0,0 +1,239 @@ +package apiclient_test + +import ( + "encoding/base64" + "net/http" + "net/http/httptest" + "testing" + + "github.com/OctopusDeploy/cli/pkg/apiclient" + "github.com/OctopusDeploy/cli/pkg/constants" + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" +) + +const octopusUrl = "https://octopus.example.com/api/" + +// clearProxyEnvironment stops whatever the machine running the tests has configured +// from leaking into the expectations. +func clearProxyEnvironment(t *testing.T) { + t.Setenv("HTTP_PROXY", "") + t.Setenv("http_proxy", "") + t.Setenv("HTTPS_PROXY", "") + t.Setenv("https_proxy", "") + t.Setenv("NO_PROXY", "") + t.Setenv("no_proxy", "") +} + +func TestProxySettings_ProxyFunc(t *testing.T) { + tests := []struct { + name string + settings apiclient.ProxySettings + env map[string]string + requestUrl string + wantProxy string + }{ + { + name: "no proxy configured at all", + requestUrl: octopusUrl, + }, + { + name: "HTTPS_PROXY is honoured with no explicit configuration", + env: map[string]string{"HTTPS_PROXY": "http://envproxy:3128"}, + requestUrl: octopusUrl, + wantProxy: "http://envproxy:3128", + }, + { + name: "HTTP_PROXY is honoured for plain http requests", + env: map[string]string{"HTTP_PROXY": "http://envproxy:3128"}, + requestUrl: "http://octopus.example.com/api/", + wantProxy: "http://envproxy:3128", + }, + { + name: "HTTPS_PROXY does not apply to plain http requests", + env: map[string]string{"HTTPS_PROXY": "http://envproxy:3128"}, + requestUrl: "http://octopus.example.com/api/", + }, + { + name: "the configured proxy url wins over HTTPS_PROXY", + settings: apiclient.ProxySettings{Url: "http://configured:3128"}, + env: map[string]string{"HTTPS_PROXY": "http://envproxy:3128"}, + requestUrl: octopusUrl, + wantProxy: "http://configured:3128", + }, + { + name: "the configured proxy url applies to plain http requests too", + settings: apiclient.ProxySettings{Url: "http://configured:3128"}, + requestUrl: "http://octopus.example.com/api/", + wantProxy: "http://configured:3128", + }, + { + name: "a proxy url without a scheme is assumed to be http", + settings: apiclient.ProxySettings{Url: "configured:3128"}, + requestUrl: octopusUrl, + wantProxy: "http://configured:3128", + }, + { + name: "socks5 proxies are passed through to net/http", + settings: apiclient.ProxySettings{Url: "socks5://configured:1080"}, + requestUrl: octopusUrl, + wantProxy: "socks5://configured:1080", + }, + { + name: "NO_PROXY excludes the host from the configured proxy", + settings: apiclient.ProxySettings{Url: "http://configured:3128"}, + env: map[string]string{"NO_PROXY": "octopus.example.com"}, + requestUrl: octopusUrl, + }, + { + name: "NO_PROXY excludes the host from HTTPS_PROXY", + env: map[string]string{"HTTPS_PROXY": "http://envproxy:3128", "NO_PROXY": "octopus.example.com"}, + requestUrl: octopusUrl, + }, + { + name: "NO_PROXY leaves other hosts proxied", + settings: apiclient.ProxySettings{Url: "http://configured:3128"}, + env: map[string]string{"NO_PROXY": "internal.example.com"}, + requestUrl: octopusUrl, + wantProxy: "http://configured:3128", + }, + { + name: "loopback servers are never proxied", + settings: apiclient.ProxySettings{Url: "http://configured:3128"}, + requestUrl: "http://localhost:8065/api/", + }, + { + name: "credentials are added to the configured proxy url", + settings: apiclient.ProxySettings{Url: "http://configured:3128", Username: "octo", Password: "s3cret"}, + requestUrl: octopusUrl, + wantProxy: "http://octo:s3cret@configured:3128", + }, + { + name: "credentials are added to a proxy url taken from the environment", + settings: apiclient.ProxySettings{Username: "octo", Password: "s3cret"}, + env: map[string]string{"HTTPS_PROXY": "http://envproxy:3128"}, + requestUrl: octopusUrl, + wantProxy: "http://octo:s3cret@envproxy:3128", + }, + { + name: "credentials in the proxy url win over the environment", + settings: apiclient.ProxySettings{Url: "http://inurl:inurlpassword@configured:3128", Username: "octo", Password: "s3cret"}, + requestUrl: octopusUrl, + wantProxy: "http://inurl:inurlpassword@configured:3128", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clearProxyEnvironment(t) + for key, value := range test.env { + t.Setenv(key, value) + } + + proxyFunc, err := test.settings.ProxyFunc() + if !assert.NoError(t, err) { + return + } + + request, err := http.NewRequest(http.MethodGet, test.requestUrl, nil) + if !assert.NoError(t, err) { + return + } + + proxyUrl, err := proxyFunc(request) + assert.NoError(t, err) + + if test.wantProxy == "" { + assert.Nil(t, proxyUrl) + return + } + if assert.NotNil(t, proxyUrl) { + assert.Equal(t, test.wantProxy, proxyUrl.String()) + } + }) + } +} + +func TestProxySettings_ProxyFuncRejectsAnInvalidProxyUrl(t *testing.T) { + clearProxyEnvironment(t) + + _, err := apiclient.ProxySettings{Url: "http://%zz:3128"}.ProxyFunc() + + assert.ErrorContains(t, err, "invalid proxy url") +} + +func TestProxySettingsFromConfig(t *testing.T) { + clearProxyEnvironment(t) + t.Setenv(constants.EnvOctopusProxyUsername, "octo") + t.Setenv(constants.EnvOctopusProxyPassword, "s3cret") + + viper.Set(constants.ConfigProxyUrl, "http://configured:3128") + t.Cleanup(func() { viper.Set(constants.ConfigProxyUrl, "") }) + + settings := apiclient.ProxySettingsFromConfig() + + assert.Equal(t, apiclient.ProxySettings{Url: "http://configured:3128", Username: "octo", Password: "s3cret"}, settings) +} + +func TestNewHttpTransport_SendsRequestsThroughTheProxy(t *testing.T) { + clearProxyEnvironment(t) + + var proxiedUrl, proxyAuthorization string + proxy := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + proxiedUrl = r.URL.String() + proxyAuthorization = r.Header.Get("Proxy-Authorization") + })) + defer proxy.Close() + + transport, err := apiclient.NewHttpTransport(apiclient.ProxySettings{Url: proxy.URL, Username: "octo", Password: "s3cret"}, false) + if !assert.NoError(t, err) { + return + } + + response, err := (&http.Client{Transport: transport}).Get("http://octopus.example.com/api/") + if !assert.NoError(t, err) { + return + } + defer response.Body.Close() + + assert.Equal(t, "http://octopus.example.com/api/", proxiedUrl) + assert.Equal(t, "Basic "+base64.StdEncoding.EncodeToString([]byte("octo:s3cret")), proxyAuthorization) +} + +// The CLI used to configure TLS by mutating the shared default transport, which +// affects every other user of it in the process. +func TestNewHttpTransport_LeavesTheDefaultTransportAlone(t *testing.T) { + clearProxyEnvironment(t) + + transport, err := apiclient.NewHttpTransport(apiclient.ProxySettings{}, true) + if !assert.NoError(t, err) { + return + } + + assert.True(t, transport.TLSClientConfig.InsecureSkipVerify) + if defaultTlsConfig := http.DefaultTransport.(*http.Transport).TLSClientConfig; defaultTlsConfig != nil { + assert.False(t, defaultTlsConfig.InsecureSkipVerify, "the shared default transport must keep verifying certificates") + } +} + +func TestRedactProxyUrl(t *testing.T) { + tests := []struct { + name string + rawUrl string + want string + }{ + {name: "empty", rawUrl: "", want: ""}, + {name: "no credentials", rawUrl: "http://proxy.example.com:3128", want: "http://proxy.example.com:3128"}, + {name: "no scheme", rawUrl: "proxy.example.com:3128", want: "proxy.example.com:3128"}, + {name: "username only", rawUrl: "http://octo@proxy.example.com:3128", want: "http://octo@proxy.example.com:3128"}, + {name: "username and password", rawUrl: "http://octo:s3cret@proxy.example.com:3128", want: "http://octo:xxxxx@proxy.example.com:3128"}, + {name: "unparseable", rawUrl: "http://octo:s3cret@%zz", want: "***"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assert.Equal(t, test.want, apiclient.RedactProxyUrl(test.rawUrl)) + assert.NotContains(t, apiclient.RedactProxyUrl(test.rawUrl), "s3cret") + }) + } +} diff --git a/pkg/cmd/config/get/get.go b/pkg/cmd/config/get/get.go index e76b11b0..b23b3b3f 100644 --- a/pkg/cmd/config/get/get.go +++ b/pkg/cmd/config/get/get.go @@ -65,7 +65,7 @@ func promptMissing(ask question.Asker) (string, error) { constants.ConfigOutputFormat, constants.ConfigShowOctopus, constants.ConfigEditor, - // constants.ConfigProxyUrl, + constants.ConfigProxyUrl, } var selectKey string diff --git a/pkg/cmd/config/list/list.go b/pkg/cmd/config/list/list.go index 51a1d630..8e64b168 100644 --- a/pkg/cmd/config/list/list.go +++ b/pkg/cmd/config/list/list.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/MakeNowJust/heredoc/v2" + "github.com/OctopusDeploy/cli/pkg/apiclient" "github.com/OctopusDeploy/cli/pkg/constants" "github.com/OctopusDeploy/cli/pkg/factory" "github.com/OctopusDeploy/cli/pkg/output" @@ -43,12 +44,17 @@ func listRun(cmd *cobra.Command) error { configFile.Set(constants.ConfigAccessToken, "***") } + if configFile.IsSet(constants.ConfigProxyUrl) { + configFile.Set(constants.ConfigProxyUrl, apiclient.RedactProxyUrl(configFile.GetString(constants.ConfigProxyUrl))) + } + type ConfigData struct { ApiKey string `json:"apikey"` Editor string `json:"editor"` Host string `json:"host"` NoPrompt string `json:"noprompt"` OutputFormat string `json:"outputformat"` + ProxyUrl string `json:"proxyurl"` Space string `json:"space"` } @@ -70,6 +76,8 @@ func listRun(cmd *cobra.Command) error { configData.Host = configFile.GetString(key) case strings.ToLower(constants.ConfigNoPrompt): configData.NoPrompt = configFile.GetString(key) + case strings.ToLower(constants.ConfigProxyUrl): + configData.ProxyUrl = configFile.GetString(key) case strings.ToLower(constants.ConfigSpace): configData.Space = configFile.GetString(key) case strings.ToLower(constants.ConfigOutputFormat): diff --git a/pkg/cmd/config/set/set.go b/pkg/cmd/config/set/set.go index e27381af..a9d018ab 100644 --- a/pkg/cmd/config/set/set.go +++ b/pkg/cmd/config/set/set.go @@ -91,7 +91,7 @@ func promptMissing(ask question.Asker, key string) (string, string, error) { constants.ConfigOutputFormat, constants.ConfigShowOctopus, constants.ConfigEditor, - // constants.ConfigProxyUrl, + constants.ConfigProxyUrl, } if key == "" { diff --git a/pkg/cmd/login/login.go b/pkg/cmd/login/login.go index d6ed863d..79ade32d 100644 --- a/pkg/cmd/login/login.go +++ b/pkg/cmd/login/login.go @@ -2,7 +2,6 @@ package login import ( "bytes" - "crypto/tls" "encoding/json" "errors" "fmt" @@ -121,17 +120,9 @@ func loginRun(cmd *cobra.Command, f factory.Factory, isPromptEnabled bool, ask q return err } - // The http client could be nil, in which case we just use the default one from http - if httpClient == nil { - httpClient = &http.Client{} - } - - if inputs.ignoreSslErrors { - if httpClient.Transport == nil { - httpClient.Transport = &http.Transport{} - } - - httpClient.Transport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + httpClient, err = ConfigureHttpClient(httpClient, inputs.ignoreSslErrors) + if err != nil { + return err } if inputs.apiKey != "" { @@ -153,6 +144,32 @@ func loginRun(cmd *cobra.Command, f factory.Factory, isPromptEnabled bool, ask q return nil } +// ConfigureHttpClient makes sure login talks to Octopus through the configured proxy. +func ConfigureHttpClient(httpClient *http.Client, ignoreSslErrors bool) (*http.Client, error) { + // the client is nil whenever the CLI has no usable configuration yet, which is the + // common case for login, so build a proxy-aware one rather than letting net/http + // fall back to its default + if httpClient == nil { + transport, err := apiclient.NewHttpTransport(apiclient.ProxySettingsFromConfig(), ignoreSslErrors) + if err != nil { + return nil, err + } + return &http.Client{Transport: transport}, nil + } + + // a configured client already carries a proxy-aware transport, so only the ssl + // override needs applying. Any other transport belongs to a caller (tests mock one + // in here) and is left alone. + if spinnerRoundTripper, ok := httpClient.Transport.(*apiclient.SpinnerRoundTripper); ok && ignoreSslErrors { + transport, err := apiclient.NewHttpTransport(apiclient.ProxySettingsFromConfig(), true) + if err != nil { + return nil, err + } + spinnerRoundTripper.Next = transport + } + return httpClient, nil +} + func loginWithApiKey(configProvider config.IConfigProvider, httpClient *http.Client, server string, apiKey string, cmd *cobra.Command) error { serverLink := output.Cyan(server) diff --git a/pkg/cmd/login/login_test.go b/pkg/cmd/login/login_test.go index 97910e22..918a9722 100644 --- a/pkg/cmd/login/login_test.go +++ b/pkg/cmd/login/login_test.go @@ -3,9 +3,11 @@ package login_test import ( "bytes" "errors" + "net/http" "testing" "github.com/AlecAivazis/survey/v2" + "github.com/OctopusDeploy/cli/pkg/apiclient" "github.com/OctopusDeploy/cli/pkg/cmd/login" cmdRoot "github.com/OctopusDeploy/cli/pkg/cmd/root" "github.com/OctopusDeploy/cli/pkg/constants" @@ -13,6 +15,7 @@ import ( "github.com/OctopusDeploy/cli/test/testutil" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/users" "github.com/spf13/cobra" + "github.com/spf13/viper" "github.com/stretchr/testify/assert" ) @@ -394,3 +397,40 @@ func TestLogin_OpenIdConnect(t *testing.T) { }) } } + +func TestConfigureHttpClient(t *testing.T) { + viper.Set(constants.ConfigProxyUrl, "http://configured:3128") + t.Cleanup(func() { viper.Set(constants.ConfigProxyUrl, "") }) + + t.Run("builds a proxy aware client when the CLI is not configured yet", func(t *testing.T) { + httpClient, err := login.ConfigureHttpClient(nil, false) + assert.NoError(t, err) + + request, _ := http.NewRequest("GET", "https://octopus.example.com/api/", nil) + proxyUrl, err := httpClient.Transport.(*http.Transport).Proxy(request) + assert.NoError(t, err) + assert.Equal(t, "http://configured:3128", proxyUrl.String()) + }) + + t.Run("applies the ssl override without discarding the spinner", func(t *testing.T) { + spinnerRoundTripper := apiclient.NewSpinnerRoundTripper(nil) + httpClient, err := login.ConfigureHttpClient(&http.Client{Transport: spinnerRoundTripper}, true) + assert.NoError(t, err) + + assert.Same(t, spinnerRoundTripper, httpClient.Transport) + assert.True(t, spinnerRoundTripper.Next.(*http.Transport).TLSClientConfig.InsecureSkipVerify) + }) + + // this used to be a type assertion onto *http.Transport, which panics for any + // client that wraps its transport + t.Run("leaves a transport it does not own alone", func(t *testing.T) { + mockClient := testutil.NewMockHttpClientWithTransport(testutil.RoundTripper(func(*http.Request) (*http.Response, error) { + return nil, nil + })) + + httpClient, err := login.ConfigureHttpClient(mockClient, true) + assert.NoError(t, err) + assert.Same(t, mockClient, httpClient) + assert.IsType(t, testutil.RoundTripper(nil), httpClient.Transport) + }) +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 7b2b7999..45d4ff52 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -27,7 +27,7 @@ func setDefaults(v *viper.Viper) { v.SetDefault(constants.ConfigApiKey, "") v.SetDefault(constants.ConfigSpace, "") v.SetDefault(constants.ConfigNoPrompt, false) - // v.SetDefault(constants.ConfigProxyUrl, "") + v.SetDefault(constants.ConfigProxyUrl, "") v.SetDefault(constants.ConfigShowOctopus, true) v.SetDefault(constants.ConfigOutputFormat, "table") @@ -51,6 +51,9 @@ func bindEnvironment(v *viper.Viper) error { if err := v.BindEnv(constants.ConfigSpace, constants.EnvOctopusSpace); err != nil { return err } + if err := v.BindEnv(constants.ConfigProxyUrl, constants.EnvOctopusProxy); err != nil { + return err + } // Envs will take precedence in the specified order if err := v.BindEnv(constants.ConfigEditor, constants.EnvVisual, constants.EnvEditor); err != nil { return err diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go new file mode 100644 index 00000000..d4ec0027 --- /dev/null +++ b/pkg/config/config_test.go @@ -0,0 +1,26 @@ +package config_test + +import ( + "testing" + + "github.com/OctopusDeploy/cli/pkg/config" + "github.com/OctopusDeploy/cli/pkg/constants" + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" +) + +func TestSetup_BindsTheProxyEnvironmentVariable(t *testing.T) { + t.Setenv(constants.EnvOctopusProxy, "http://envproxy:3128") + + v := viper.New() + assert.NoError(t, config.Setup(v)) + + assert.Equal(t, "http://envproxy:3128", v.GetString(constants.ConfigProxyUrl)) +} + +func TestSetup_DefaultsTheProxyToEmpty(t *testing.T) { + v := viper.New() + assert.NoError(t, config.Setup(v)) + + assert.Contains(t, v.AllKeys(), "proxyurl", "the proxy url must be a settable config key") +} diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 39b2ccf0..a248ecf7 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -29,12 +29,12 @@ const ( // keys for key/value store config file const ( - ConfigUrl = "Url" - ConfigApiKey = "ApiKey" - ConfigAccessToken = "AccessToken" - ConfigSpace = "Space" - ConfigNoPrompt = "NoPrompt" - // ConfigProxyUrl = "ProxyUrl" + ConfigUrl = "Url" + ConfigApiKey = "ApiKey" + ConfigAccessToken = "AccessToken" + ConfigSpace = "Space" + ConfigNoPrompt = "NoPrompt" + ConfigProxyUrl = "ProxyUrl" ConfigEditor = "Editor" ConfigShowOctopus = "ShowOctopus" ConfigOutputFormat = "OutputFormat" @@ -45,9 +45,13 @@ const ( EnvOctopusApiKey = "OCTOPUS_API_KEY" EnvOctopusAccessToken = "OCTOPUS_ACCESS_TOKEN" EnvOctopusSpace = "OCTOPUS_SPACE" - EnvEditor = "EDITOR" - EnvVisual = "VISUAL" - EnvCI = "CI" + EnvOctopusProxy = "OCTOPUS_PROXY" + // Proxy credentials are environment-only; they are never stored in the config file + EnvOctopusProxyUsername = "OCTOPUS_PROXY_USERNAME" + EnvOctopusProxyPassword = "OCTOPUS_PROXY_PASSWORD" + EnvEditor = "EDITOR" + EnvVisual = "VISUAL" + EnvCI = "CI" ) const (