From 274ead788df634b2bb6a6bd3ad5c29216187ea82 Mon Sep 17 00:00:00 2001 From: peteski22 Date: Wed, 11 May 2022 19:52:10 +0100 Subject: [PATCH 01/10] Added support for VAULT_PROXY_ADDR Updated documentation to describe the behavior when supplying `VAULT_HTTP_PROXY`. Also added support for `VAULT_PROXY_ADDR` as a 'better name' for `VAULT_HTTP_PROXY`. --- api/client.go | 16 ++-- api/client_test.go | 112 ++++++++++++++++++++++++ website/content/docs/commands/index.mdx | 22 ++++- 3 files changed, 142 insertions(+), 8 deletions(-) diff --git a/api/client.go b/api/client.go index efdf033af235..953672dabbba 100644 --- a/api/client.go +++ b/api/client.go @@ -51,6 +51,7 @@ const ( EnvVaultMFA = "VAULT_MFA" EnvRateLimit = "VAULT_RATE_LIMIT" EnvHTTPProxy = "VAULT_HTTP_PROXY" + EnvVaultProxyAddr = "VAULT_PROXY_ADDR" HeaderIndex = "X-Vault-Index" HeaderForward = "X-Vault-Forward" HeaderInconsistent = "X-Vault-Inconsistent" @@ -338,7 +339,7 @@ func (c *Config) ReadEnvironment() error { var envMaxRetries *uint64 var envSRVLookup bool var limit *rate.Limiter - var envHTTPProxy string + var envVaultProxy string // Parse the environment variables if v := os.Getenv(EnvVaultAddress); v != "" { @@ -411,7 +412,12 @@ func (c *Config) ReadEnvironment() error { } if v := os.Getenv(EnvHTTPProxy); v != "" { - envHTTPProxy = v + envVaultProxy = v + } + + // VAULT_PROXY_ADDR supersedes VAULT_HTTP_PROXY + if v := os.Getenv(EnvVaultProxyAddr); v != "" { + envVaultProxy = v } // Configure the HTTP clients TLS configuration. @@ -451,14 +457,14 @@ func (c *Config) ReadEnvironment() error { c.Timeout = envClientTimeout } - if envHTTPProxy != "" { - url, err := url.Parse(envHTTPProxy) + if envVaultProxy != "" { + u, err := url.Parse(envVaultProxy) if err != nil { return err } transport := c.HttpClient.Transport.(*http.Transport) - transport.Proxy = http.ProxyURL(url) + transport.Proxy = http.ProxyURL(u) } return nil diff --git a/api/client_test.go b/api/client_test.go index 383932b9e73e..c80ffb23fb3d 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -1201,3 +1201,115 @@ func TestClientWithNamespace(t *testing.T) { t.Fatalf("Expected original namespace: \"%s\", got \"%s\"", ogNS, client.Namespace()) } } + +func TestVaultHttpProxyUsedWhenNoProxyEnvVarDoesntIncludeRequestHost(t *testing.T) { + const VaultHttpProxy string = "VAULT_HTTP_PROXY" + const NoProxy string = "NO_PROXY" + + oldVaultProxy := os.Getenv(VaultHttpProxy) + os.Setenv(VaultHttpProxy, "https://hashicorp.com") + defer os.Setenv(VaultHttpProxy, oldVaultProxy) + + oldNoProxy := os.Getenv(NoProxy) + os.Setenv(NoProxy, "terraform.com") + defer os.Setenv(NoProxy, oldNoProxy) + + c := DefaultConfig() + err := c.ReadEnvironment() + if err != nil { + t.Fatalf("Expected no error reading config, found error %v", err) + } + + r, _ := http.NewRequest("GET", "https://vaultproject.io", nil) + proxyUrl, err := c.HttpClient.Transport.(*http.Transport).Proxy(r) + + if err != nil { + t.Fatalf("Expected no error resolving proxy, found error %v", err) + } else if proxyUrl == nil || proxyUrl.String() == "" { + t.Fatalf("Expected proxy to be resolved but no proxy returned") + } +} + +func TestVaultHttpProxyStillUsedWhenNoProxyEnvVarIncludesRequestHost(t *testing.T) { + const VaultHttpProxy string = "VAULT_HTTP_PROXY" + const NoProxy string = "NO_PROXY" + + oldVaultProxy := os.Getenv(VaultHttpProxy) + os.Setenv(VaultHttpProxy, "https://hashicorp.com") + defer os.Setenv(VaultHttpProxy, oldVaultProxy) + + oldNoProxy := os.Getenv(NoProxy) + os.Setenv(NoProxy, "terraform.com,vaultproject.io") + defer os.Setenv(NoProxy, oldNoProxy) + + c := DefaultConfig() + err := c.ReadEnvironment() + if err != nil { + t.Fatalf("Expected no error reading config, found error %v", err) + } + + r, _ := http.NewRequest("GET", "https://vaultproject.io", nil) + proxyUrl, err := c.HttpClient.Transport.(*http.Transport).Proxy(r) + + if err != nil { + t.Fatalf("Expected no error resolving proxy, found error %v", err) + } else if proxyUrl == nil || proxyUrl.String() == "" { + t.Fatalf("Expected proxy to be resolved but no proxy returned") + } +} + +func TestVaultAddrProxyUsedWhenNoProxyEnvVarDoesntIncludeRequestHost(t *testing.T) { + const VaultProxyAddr string = "VAULT_PROXY_ADDR" + const NoProxy string = "NO_PROXY" + + oldVaultProxy := os.Getenv(VaultProxyAddr) + os.Setenv(VaultProxyAddr, "https://hashicorp.com") + defer os.Setenv(VaultProxyAddr, oldVaultProxy) + + oldNoProxy := os.Getenv(NoProxy) + os.Setenv(NoProxy, "terraform.com") + defer os.Setenv(NoProxy, oldNoProxy) + + c := DefaultConfig() + err := c.ReadEnvironment() + if err != nil { + t.Fatalf("Expected no error reading config, found error %v", err) + } + + r, _ := http.NewRequest("GET", "https://vaultproject.io", nil) + proxyUrl, err := c.HttpClient.Transport.(*http.Transport).Proxy(r) + + if err != nil { + t.Fatalf("Expected no error resolving proxy, found error %v", err) + } else if proxyUrl == nil || proxyUrl.String() == "" { + t.Fatalf("Expected proxy to be resolved but no proxy returned") + } +} + +func TestVaultProxyAddrStillUsedWhenNoProxyEnvVarIncludesRequestHost(t *testing.T) { + const VaultProxyAddr string = "VAULT_PROXY_ADDR" + const NoProxy string = "NO_PROXY" + + oldVaultProxy := os.Getenv(VaultProxyAddr) + os.Setenv(VaultProxyAddr, "https://hashicorp.com") + defer os.Setenv(VaultProxyAddr, oldVaultProxy) + + oldNoProxy := os.Getenv(NoProxy) + os.Setenv(NoProxy, "terraform.com,vaultproject.io") + defer os.Setenv(NoProxy, oldNoProxy) + + c := DefaultConfig() + err := c.ReadEnvironment() + if err != nil { + t.Fatalf("Expected no error reading config, found error %v", err) + } + + r, _ := http.NewRequest("GET", "https://vaultproject.io", nil) + proxyUrl, err := c.HttpClient.Transport.(*http.Transport).Proxy(r) + + if err != nil { + t.Fatalf("Expected no error resolving proxy, found error %v", err) + } else if proxyUrl == nil || proxyUrl.String() == "" { + t.Fatalf("Expected proxy to be resolved but no proxy returned") + } +} diff --git a/website/content/docs/commands/index.mdx b/website/content/docs/commands/index.mdx index 14df297707b1..5966fa014719 100644 --- a/website/content/docs/commands/index.mdx +++ b/website/content/docs/commands/index.mdx @@ -379,9 +379,25 @@ used. ### `VAULT_HTTP_PROXY` -HTTP proxy location which should be used to access Vault. When present, this -overrides any other proxies found in the environment. Format should be -`http://server:port`. +HTTP or HTTPS proxy location which should be used by all requests to access Vault. + When present, this overrides the default proxy resolution behavior. +Format should be `http://server:port` or `https://server:port`. +(See ``VAULT_PROXY_ADDR` below) + +### `VAULT_PROXY_ADDR` + +HTTP or HTTPS proxy location which should be used by all requests to access Vault. + When present, this overrides the default proxy resolution behavior. +Format should be `http://server:port` or `https://server:port`. + +~> Note: When using `VAULT_HTTP_PROXY` or `VAULT_PROXY_ADDR` any of the standard + proxy variables found in the environment will be ignored. +Specifically `HTTP_PROXY`, `HTTPS_PROXY` and `NO_PROXY`. +All requests will resolve the specified proxy, there is no way to exclude + domains from consulting the proxy server. + +~> Note: If both `VAULT_HTTP_PROXY` and `VAULT_PROXY_ADDR` environment +variables are supplied, `VAULT_PROXY_ADDR` will be prioritized and preferred. ## Flags From 6cab6f7be110860642f90dc739b0f3d1d6446082 Mon Sep 17 00:00:00 2001 From: peteski22 Date: Wed, 11 May 2022 19:57:40 +0100 Subject: [PATCH 02/10] Added changelog (related to gh-12582) Change is in addition to existing change --- changelog/15377.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelog/15377.txt diff --git a/changelog/15377.txt b/changelog/15377.txt new file mode 100644 index 000000000000..35588a8dafee --- /dev/null +++ b/changelog/15377.txt @@ -0,0 +1,4 @@ +```release-note:improvement +api: Support VAULT_PROXY_ADDR environment variable to allow overriding the Vault client's HTTP proxy. +api: Update docs to fully describe behavior for `VAULT_HTTP_PROXY` and `VAULT_PROXY_ADDR` environment variables. +``` \ No newline at end of file From fabf0872616a40a7ce1d58bf514f84febfda3f04 Mon Sep 17 00:00:00 2001 From: peteski22 Date: Wed, 11 May 2022 20:06:55 +0100 Subject: [PATCH 03/10] Added extra test (gh-15021) Related to GH issue raised regarding behavior for Vault proxy env var. --- api/client_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/api/client_test.go b/api/client_test.go index c80ffb23fb3d..8fdb5d8997b5 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -1313,3 +1313,33 @@ func TestVaultProxyAddrStillUsedWhenNoProxyEnvVarIncludesRequestHost(t *testing. t.Fatalf("Expected proxy to be resolved but no proxy returned") } } + +func TestVaultProxyAddrStillUsedWhenVaultHttpProxySuppliedAlso(t *testing.T) { + const VaultProxyAddr string = "VAULT_PROXY_ADDR" + const VaultHttpProxy string = "VAULT_HTTP_PROXY" + + oldVaultHttpProxy := os.Getenv(VaultHttpProxy) + os.Setenv(VaultHttpProxy, "https://hashicorp.com") + defer os.Setenv(VaultHttpProxy, oldVaultHttpProxy) + + oldVaultProxyAddr := os.Getenv(VaultProxyAddr) + os.Setenv(VaultProxyAddr, "https://terraform.io") + defer os.Setenv(VaultProxyAddr, oldVaultProxyAddr) + + c := DefaultConfig() + err := c.ReadEnvironment() + if err != nil { + t.Fatalf("Expected no error reading config, found error %v", err) + } + + r, _ := http.NewRequest("GET", "https://vaultproject.io", nil) + proxyUrl, err := c.HttpClient.Transport.(*http.Transport).Proxy(r) + + if err != nil { + t.Fatalf("Expected no error resolving proxy, found error %v", err) + } else if proxyUrl == nil || proxyUrl.String() == "" { + t.Fatalf("Expected proxy to be resolved but no proxy returned") + } else if proxyUrl.String() != "https://terraform.io" { + t.Fatalf("Expected VAULT_PROXY_ADDR to be used but was %v", proxyUrl.String()) + } +} From 1f99cb26a4a77c6d3a5e99b80be933402e207da6 Mon Sep 17 00:00:00 2001 From: peteski22 Date: Wed, 11 May 2022 21:29:14 +0100 Subject: [PATCH 04/10] Fixed typos in docs --- website/content/docs/commands/index.mdx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/website/content/docs/commands/index.mdx b/website/content/docs/commands/index.mdx index 5966fa014719..69d257f7c52c 100644 --- a/website/content/docs/commands/index.mdx +++ b/website/content/docs/commands/index.mdx @@ -382,7 +382,8 @@ used. HTTP or HTTPS proxy location which should be used by all requests to access Vault. When present, this overrides the default proxy resolution behavior. Format should be `http://server:port` or `https://server:port`. -(See ``VAULT_PROXY_ADDR` below) + +(See `VAULT_PROXY_ADDR` below). ### `VAULT_PROXY_ADDR` From 0af4b61d093f55fbc29a007853bd0a5479d0010c Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Mon, 16 May 2022 09:03:10 +0100 Subject: [PATCH 05/10] Update website/content/docs/commands/index.mdx Co-authored-by: Loann Le <84412881+taoism4504@users.noreply.github.com> --- website/content/docs/commands/index.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/content/docs/commands/index.mdx b/website/content/docs/commands/index.mdx index 69d257f7c52c..55e9d0fcf457 100644 --- a/website/content/docs/commands/index.mdx +++ b/website/content/docs/commands/index.mdx @@ -394,7 +394,7 @@ Format should be `http://server:port` or `https://server:port`. ~> Note: When using `VAULT_HTTP_PROXY` or `VAULT_PROXY_ADDR` any of the standard proxy variables found in the environment will be ignored. Specifically `HTTP_PROXY`, `HTTPS_PROXY` and `NO_PROXY`. -All requests will resolve the specified proxy, there is no way to exclude +All requests will resolve the specified proxy; there is no way to exclude domains from consulting the proxy server. ~> Note: If both `VAULT_HTTP_PROXY` and `VAULT_PROXY_ADDR` environment From fa150b26cf187c9da70d732c320bcef3106d9842 Mon Sep 17 00:00:00 2001 From: peteski22 Date: Mon, 16 May 2022 16:29:24 +0100 Subject: [PATCH 06/10] Removed reference to docs in changelog --- changelog/15377.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/changelog/15377.txt b/changelog/15377.txt index 35588a8dafee..c7547102c22e 100644 --- a/changelog/15377.txt +++ b/changelog/15377.txt @@ -1,4 +1,3 @@ ```release-note:improvement api: Support VAULT_PROXY_ADDR environment variable to allow overriding the Vault client's HTTP proxy. -api: Update docs to fully describe behavior for `VAULT_HTTP_PROXY` and `VAULT_PROXY_ADDR` environment variables. ``` \ No newline at end of file From 01a08d1ba7c74fb457ef2f23f01f47675b65e6c6 Mon Sep 17 00:00:00 2001 From: peteski22 Date: Fri, 20 May 2022 15:34:34 +0100 Subject: [PATCH 07/10] Resolve re-call of ReadEnvironment As raised in a PR comment, we don't need to call ReadEnvironment() as the DefaultConfig() does that for us. We can just check the config.Error for the error. --- api/client_test.go | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/api/client_test.go b/api/client_test.go index 8fdb5d8997b5..93566eae8777 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -1215,9 +1215,8 @@ func TestVaultHttpProxyUsedWhenNoProxyEnvVarDoesntIncludeRequestHost(t *testing. defer os.Setenv(NoProxy, oldNoProxy) c := DefaultConfig() - err := c.ReadEnvironment() - if err != nil { - t.Fatalf("Expected no error reading config, found error %v", err) + if c.Error != nil { + t.Fatalf("Expected no error reading config, found error %v", c.Error) } r, _ := http.NewRequest("GET", "https://vaultproject.io", nil) @@ -1243,9 +1242,8 @@ func TestVaultHttpProxyStillUsedWhenNoProxyEnvVarIncludesRequestHost(t *testing. defer os.Setenv(NoProxy, oldNoProxy) c := DefaultConfig() - err := c.ReadEnvironment() - if err != nil { - t.Fatalf("Expected no error reading config, found error %v", err) + if c.Error != nil { + t.Fatalf("Expected no error reading config, found error %v", c.Error) } r, _ := http.NewRequest("GET", "https://vaultproject.io", nil) @@ -1271,9 +1269,8 @@ func TestVaultAddrProxyUsedWhenNoProxyEnvVarDoesntIncludeRequestHost(t *testing. defer os.Setenv(NoProxy, oldNoProxy) c := DefaultConfig() - err := c.ReadEnvironment() - if err != nil { - t.Fatalf("Expected no error reading config, found error %v", err) + if c.Error != nil { + t.Fatalf("Expected no error reading config, found error %v", c.Error) } r, _ := http.NewRequest("GET", "https://vaultproject.io", nil) @@ -1299,9 +1296,8 @@ func TestVaultProxyAddrStillUsedWhenNoProxyEnvVarIncludesRequestHost(t *testing. defer os.Setenv(NoProxy, oldNoProxy) c := DefaultConfig() - err := c.ReadEnvironment() - if err != nil { - t.Fatalf("Expected no error reading config, found error %v", err) + if c.Error != nil { + t.Fatalf("Expected no error reading config, found error %v", c.Error) } r, _ := http.NewRequest("GET", "https://vaultproject.io", nil) @@ -1327,9 +1323,8 @@ func TestVaultProxyAddrStillUsedWhenVaultHttpProxySuppliedAlso(t *testing.T) { defer os.Setenv(VaultProxyAddr, oldVaultProxyAddr) c := DefaultConfig() - err := c.ReadEnvironment() - if err != nil { - t.Fatalf("Expected no error reading config, found error %v", err) + if c.Error != nil { + t.Fatalf("Expected no error reading config, found error %v", c.Error) } r, _ := http.NewRequest("GET", "https://vaultproject.io", nil) From 945508135a1c9c91552f6254165891e7eb650827 Mon Sep 17 00:00:00 2001 From: peteski22 Date: Fri, 20 May 2022 17:16:49 +0100 Subject: [PATCH 08/10] Make use of test table --- api/client_test.go | 195 +++++++++++++++++---------------------------- 1 file changed, 71 insertions(+), 124 deletions(-) diff --git a/api/client_test.go b/api/client_test.go index 93566eae8777..a727b8c946a7 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -19,7 +19,6 @@ import ( "github.com/go-test/deep" "github.com/hashicorp/go-hclog" - "github.com/hashicorp/vault/sdk/helper/consts" ) @@ -1202,139 +1201,87 @@ func TestClientWithNamespace(t *testing.T) { } } -func TestVaultHttpProxyUsedWhenNoProxyEnvVarDoesntIncludeRequestHost(t *testing.T) { - const VaultHttpProxy string = "VAULT_HTTP_PROXY" - const NoProxy string = "NO_PROXY" - - oldVaultProxy := os.Getenv(VaultHttpProxy) - os.Setenv(VaultHttpProxy, "https://hashicorp.com") - defer os.Setenv(VaultHttpProxy, oldVaultProxy) - - oldNoProxy := os.Getenv(NoProxy) - os.Setenv(NoProxy, "terraform.com") - defer os.Setenv(NoProxy, oldNoProxy) - - c := DefaultConfig() - if c.Error != nil { - t.Fatalf("Expected no error reading config, found error %v", c.Error) - } - - r, _ := http.NewRequest("GET", "https://vaultproject.io", nil) - proxyUrl, err := c.HttpClient.Transport.(*http.Transport).Proxy(r) - - if err != nil { - t.Fatalf("Expected no error resolving proxy, found error %v", err) - } else if proxyUrl == nil || proxyUrl.String() == "" { - t.Fatalf("Expected proxy to be resolved but no proxy returned") - } -} - -func TestVaultHttpProxyStillUsedWhenNoProxyEnvVarIncludesRequestHost(t *testing.T) { +func TestVaultProxy(t *testing.T) { const VaultHttpProxy string = "VAULT_HTTP_PROXY" - const NoProxy string = "NO_PROXY" - - oldVaultProxy := os.Getenv(VaultHttpProxy) - os.Setenv(VaultHttpProxy, "https://hashicorp.com") - defer os.Setenv(VaultHttpProxy, oldVaultProxy) - - oldNoProxy := os.Getenv(NoProxy) - os.Setenv(NoProxy, "terraform.com,vaultproject.io") - defer os.Setenv(NoProxy, oldNoProxy) - - c := DefaultConfig() - if c.Error != nil { - t.Fatalf("Expected no error reading config, found error %v", c.Error) - } - - r, _ := http.NewRequest("GET", "https://vaultproject.io", nil) - proxyUrl, err := c.HttpClient.Transport.(*http.Transport).Proxy(r) - - if err != nil { - t.Fatalf("Expected no error resolving proxy, found error %v", err) - } else if proxyUrl == nil || proxyUrl.String() == "" { - t.Fatalf("Expected proxy to be resolved but no proxy returned") - } -} - -func TestVaultAddrProxyUsedWhenNoProxyEnvVarDoesntIncludeRequestHost(t *testing.T) { const VaultProxyAddr string = "VAULT_PROXY_ADDR" const NoProxy string = "NO_PROXY" - oldVaultProxy := os.Getenv(VaultProxyAddr) - os.Setenv(VaultProxyAddr, "https://hashicorp.com") - defer os.Setenv(VaultProxyAddr, oldVaultProxy) - - oldNoProxy := os.Getenv(NoProxy) - os.Setenv(NoProxy, "terraform.com") - defer os.Setenv(NoProxy, oldNoProxy) - - c := DefaultConfig() - if c.Error != nil { - t.Fatalf("Expected no error reading config, found error %v", c.Error) - } - - r, _ := http.NewRequest("GET", "https://vaultproject.io", nil) - proxyUrl, err := c.HttpClient.Transport.(*http.Transport).Proxy(r) - - if err != nil { - t.Fatalf("Expected no error resolving proxy, found error %v", err) - } else if proxyUrl == nil || proxyUrl.String() == "" { - t.Fatalf("Expected proxy to be resolved but no proxy returned") - } -} - -func TestVaultProxyAddrStillUsedWhenNoProxyEnvVarIncludesRequestHost(t *testing.T) { - const VaultProxyAddr string = "VAULT_PROXY_ADDR" - const NoProxy string = "NO_PROXY" - - oldVaultProxy := os.Getenv(VaultProxyAddr) - os.Setenv(VaultProxyAddr, "https://hashicorp.com") - defer os.Setenv(VaultProxyAddr, oldVaultProxy) - - oldNoProxy := os.Getenv(NoProxy) - os.Setenv(NoProxy, "terraform.com,vaultproject.io") - defer os.Setenv(NoProxy, oldNoProxy) - - c := DefaultConfig() - if c.Error != nil { - t.Fatalf("Expected no error reading config, found error %v", c.Error) - } - - r, _ := http.NewRequest("GET", "https://vaultproject.io", nil) - proxyUrl, err := c.HttpClient.Transport.(*http.Transport).Proxy(r) - - if err != nil { - t.Fatalf("Expected no error resolving proxy, found error %v", err) - } else if proxyUrl == nil || proxyUrl.String() == "" { - t.Fatalf("Expected proxy to be resolved but no proxy returned") + tests := map[string]struct { + name string + vaultHttpProxy string + vaultProxyAddr string + noProxy string + requestUrl string + checkProxyVarPriority bool + }{ + "VAULT_HTTP_PROXY used when NO_PROXY env var doesn't include request host": { + vaultHttpProxy: "https://hashicorp.com", + vaultProxyAddr: "", + noProxy: "terraform.io", + requestUrl: "https://vaultproject.io", + }, + "VAULT_HTTP_PROXY used when NO_PROXY env var includes request host": { + vaultHttpProxy: "https://hashicorp.com", + vaultProxyAddr: "", + noProxy: "terraform.io,vaultproject.io", + requestUrl: "https://vaultproject.io", + }, + "VAULT_PROXY_ADDR used when NO_PROXY env var doesn't include request host": { + vaultHttpProxy: "", + vaultProxyAddr: "https://hashicorp.com", + noProxy: "terraform.io", + requestUrl: "https://vaultproject.io", + }, + "VAULT_PROXY_ADDR used when NO_PROXY env var includes request host": { + vaultHttpProxy: "", + vaultProxyAddr: "https://hashicorp.com", + noProxy: "terraform.io,vaultproject.io", + requestUrl: "https://vaultproject.io", + }, + "VAULT_PROXY_ADDR used when VAULT_HTTP_PROXY env var also supplied": { + vaultHttpProxy: "https://hashicorp.com", + vaultProxyAddr: "https://terraform.io", + noProxy: "", + requestUrl: "https://vaultproject.io", + checkProxyVarPriority: true, + }, } -} -func TestVaultProxyAddrStillUsedWhenVaultHttpProxySuppliedAlso(t *testing.T) { - const VaultProxyAddr string = "VAULT_PROXY_ADDR" - const VaultHttpProxy string = "VAULT_HTTP_PROXY" + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + if tc.vaultHttpProxy != "" { + oldVaultHttpProxy := os.Getenv(VaultHttpProxy) + os.Setenv(VaultHttpProxy, tc.vaultHttpProxy) + defer os.Setenv(VaultHttpProxy, oldVaultHttpProxy) + } - oldVaultHttpProxy := os.Getenv(VaultHttpProxy) - os.Setenv(VaultHttpProxy, "https://hashicorp.com") - defer os.Setenv(VaultHttpProxy, oldVaultHttpProxy) + if tc.vaultProxyAddr != "" { + oldVaultProxyAddr := os.Getenv(VaultProxyAddr) + os.Setenv(VaultProxyAddr, tc.vaultProxyAddr) + defer os.Setenv(VaultProxyAddr, oldVaultProxyAddr) + } - oldVaultProxyAddr := os.Getenv(VaultProxyAddr) - os.Setenv(VaultProxyAddr, "https://terraform.io") - defer os.Setenv(VaultProxyAddr, oldVaultProxyAddr) + if tc.noProxy != "" { + oldNoProxy := os.Getenv(NoProxy) + os.Setenv(NoProxy, tc.noProxy) + defer os.Setenv(NoProxy, oldNoProxy) + } - c := DefaultConfig() - if c.Error != nil { - t.Fatalf("Expected no error reading config, found error %v", c.Error) - } + c := DefaultConfig() + if c.Error != nil { + t.Fatalf("Expected no error reading config, found error %v", c.Error) + } - r, _ := http.NewRequest("GET", "https://vaultproject.io", nil) - proxyUrl, err := c.HttpClient.Transport.(*http.Transport).Proxy(r) + r, _ := http.NewRequest("GET", tc.requestUrl, nil) + proxyUrl, err := c.HttpClient.Transport.(*http.Transport).Proxy(r) - if err != nil { - t.Fatalf("Expected no error resolving proxy, found error %v", err) - } else if proxyUrl == nil || proxyUrl.String() == "" { - t.Fatalf("Expected proxy to be resolved but no proxy returned") - } else if proxyUrl.String() != "https://terraform.io" { - t.Fatalf("Expected VAULT_PROXY_ADDR to be used but was %v", proxyUrl.String()) + if err != nil { + t.Fatalf("Expected no error resolving proxy, found error %v", err) + } else if proxyUrl == nil || proxyUrl.String() == "" { + t.Fatalf("Expected proxy to be resolved but no proxy returned") + } else if tc.checkProxyVarPriority && proxyUrl.String() != tc.vaultProxyAddr { + t.Fatalf("Expected %v value (%v) to be used but was %v", VaultProxyAddr, tc.vaultProxyAddr, proxyUrl.String()) + } + }) } } From 01fb930335bedd3b17141fec15b74b2d6f2552f6 Mon Sep 17 00:00:00 2001 From: peteski22 Date: Fri, 20 May 2022 18:35:23 +0100 Subject: [PATCH 09/10] Use consts where possible, restructure if conditions --- api/client_test.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/api/client_test.go b/api/client_test.go index a727b8c946a7..00c8d753822d 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -1202,8 +1202,6 @@ func TestClientWithNamespace(t *testing.T) { } func TestVaultProxy(t *testing.T) { - const VaultHttpProxy string = "VAULT_HTTP_PROXY" - const VaultProxyAddr string = "VAULT_PROXY_ADDR" const NoProxy string = "NO_PROXY" tests := map[string]struct { @@ -1250,15 +1248,15 @@ func TestVaultProxy(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { if tc.vaultHttpProxy != "" { - oldVaultHttpProxy := os.Getenv(VaultHttpProxy) - os.Setenv(VaultHttpProxy, tc.vaultHttpProxy) - defer os.Setenv(VaultHttpProxy, oldVaultHttpProxy) + oldVaultHttpProxy := os.Getenv(EnvHTTPProxy) + os.Setenv(EnvHTTPProxy, tc.vaultHttpProxy) + defer os.Setenv(EnvHTTPProxy, oldVaultHttpProxy) } if tc.vaultProxyAddr != "" { - oldVaultProxyAddr := os.Getenv(VaultProxyAddr) - os.Setenv(VaultProxyAddr, tc.vaultProxyAddr) - defer os.Setenv(VaultProxyAddr, oldVaultProxyAddr) + oldVaultProxyAddr := os.Getenv(EnvVaultProxyAddr) + os.Setenv(EnvVaultProxyAddr, tc.vaultProxyAddr) + defer os.Setenv(EnvVaultProxyAddr, oldVaultProxyAddr) } if tc.noProxy != "" { @@ -1274,13 +1272,14 @@ func TestVaultProxy(t *testing.T) { r, _ := http.NewRequest("GET", tc.requestUrl, nil) proxyUrl, err := c.HttpClient.Transport.(*http.Transport).Proxy(r) - if err != nil { t.Fatalf("Expected no error resolving proxy, found error %v", err) - } else if proxyUrl == nil || proxyUrl.String() == "" { + } + if proxyUrl == nil || proxyUrl.String() == "" { t.Fatalf("Expected proxy to be resolved but no proxy returned") - } else if tc.checkProxyVarPriority && proxyUrl.String() != tc.vaultProxyAddr { - t.Fatalf("Expected %v value (%v) to be used but was %v", VaultProxyAddr, tc.vaultProxyAddr, proxyUrl.String()) + } + if tc.checkProxyVarPriority && proxyUrl.String() != tc.vaultProxyAddr { + t.Fatalf("Expected %v value (%v) to be used but was %v", EnvVaultProxyAddr, tc.vaultProxyAddr, proxyUrl.String()) } }) } From 4569d37504f06f324d0d1e24ea8a47c4290bf6fe Mon Sep 17 00:00:00 2001 From: peteski22 Date: Fri, 20 May 2022 18:56:54 +0100 Subject: [PATCH 10/10] Adjusted 'check' for specific proxy URL --- api/client_test.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/api/client_test.go b/api/client_test.go index 00c8d753822d..74f1b9354e36 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -1205,12 +1205,12 @@ func TestVaultProxy(t *testing.T) { const NoProxy string = "NO_PROXY" tests := map[string]struct { - name string - vaultHttpProxy string - vaultProxyAddr string - noProxy string - requestUrl string - checkProxyVarPriority bool + name string + vaultHttpProxy string + vaultProxyAddr string + noProxy string + requestUrl string + expectedResolvedProxyUrl string }{ "VAULT_HTTP_PROXY used when NO_PROXY env var doesn't include request host": { vaultHttpProxy: "https://hashicorp.com", @@ -1237,11 +1237,11 @@ func TestVaultProxy(t *testing.T) { requestUrl: "https://vaultproject.io", }, "VAULT_PROXY_ADDR used when VAULT_HTTP_PROXY env var also supplied": { - vaultHttpProxy: "https://hashicorp.com", - vaultProxyAddr: "https://terraform.io", - noProxy: "", - requestUrl: "https://vaultproject.io", - checkProxyVarPriority: true, + vaultHttpProxy: "https://hashicorp.com", + vaultProxyAddr: "https://terraform.io", + noProxy: "", + requestUrl: "https://vaultproject.io", + expectedResolvedProxyUrl: "https://terraform.io", }, } @@ -1278,8 +1278,8 @@ func TestVaultProxy(t *testing.T) { if proxyUrl == nil || proxyUrl.String() == "" { t.Fatalf("Expected proxy to be resolved but no proxy returned") } - if tc.checkProxyVarPriority && proxyUrl.String() != tc.vaultProxyAddr { - t.Fatalf("Expected %v value (%v) to be used but was %v", EnvVaultProxyAddr, tc.vaultProxyAddr, proxyUrl.String()) + if tc.expectedResolvedProxyUrl != "" && proxyUrl.String() != tc.expectedResolvedProxyUrl { + t.Fatalf("Expected resolved proxy URL to be %v but was %v", tc.expectedResolvedProxyUrl, proxyUrl.String()) } }) }