From 2727be9bba147d2fc979746fc18ea9274446035e Mon Sep 17 00:00:00 2001 From: Izaak Lauer <8404559+izaaklauer@users.noreply.github.com> Date: Wed, 23 Jun 2021 02:56:13 +0000 Subject: [PATCH 01/14] backport of commit 449904d5389e52b457fe36a04449451fea48550c --- internal/ceb/ceb.go | 9 +++++---- internal/cli/main.go | 3 ++- internal/serverclient/client.go | 5 +++-- internal/util/env.go | 20 ++++++++++++++++++++ website/content/docs/entrypoint/disable.mdx | 4 ++-- website/content/docs/runner/run-manual.mdx | 2 +- 6 files changed, 33 insertions(+), 10 deletions(-) create mode 100644 internal/util/env.go diff --git a/internal/ceb/ceb.go b/internal/ceb/ceb.go index 372f6bd2233..d5fb67b2bfc 100644 --- a/internal/ceb/ceb.go +++ b/internal/ceb/ceb.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/waypoint/internal/plugin" "github.com/hashicorp/waypoint/internal/server" pb "github.com/hashicorp/waypoint/internal/server/gen" + "github.com/hashicorp/waypoint/internal/util" "github.com/hashicorp/waypoint/internal/version" ) @@ -299,11 +300,11 @@ func WithEnvDefaults() Option { cfg.URLServicePort = port cfg.ServerAddr = os.Getenv(envServerAddr) - cfg.ServerRequired = os.Getenv(envCEBServerRequired) != "" - cfg.ServerTls = os.Getenv(envServerTls) != "" - cfg.ServerTlsSkipVerify = os.Getenv(envServerTlsSkipVerify) != "" + cfg.ServerRequired = util.GetEnvBool(envCEBServerRequired, false) + cfg.ServerTls = util.GetEnvBool(envServerTls, false) + cfg.ServerTlsSkipVerify = util.GetEnvBool(envServerTlsSkipVerify, false) cfg.InviteToken = os.Getenv(envCEBToken) - cfg.disable = os.Getenv(envCEBDisable) != "" + cfg.disable = util.GetEnvBool(envCEBDisable, false) ceb.deploymentId = os.Getenv(envDeploymentId) diff --git a/internal/cli/main.go b/internal/cli/main.go index 5dd2f42e0ca..ef9ff2803dd 100644 --- a/internal/cli/main.go +++ b/internal/cli/main.go @@ -19,6 +19,7 @@ import ( "github.com/hashicorp/waypoint-plugin-sdk/terminal" "github.com/hashicorp/waypoint/internal/pkg/signalcontext" + "github.com/hashicorp/waypoint/internal/util" "github.com/hashicorp/waypoint/internal/version" ) @@ -139,7 +140,7 @@ func Commands( } // Set plain mode if set - if os.Getenv(EnvPlain) != "" { + if util.GetEnvBool(EnvPlain, false) { baseCommand.globalOptions = append(baseCommand.globalOptions, WithUI(terminal.NonInteractiveUI(ctx))) } diff --git a/internal/serverclient/client.go b/internal/serverclient/client.go index 4dca38018c4..455414d05c0 100644 --- a/internal/serverclient/client.go +++ b/internal/serverclient/client.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/waypoint/internal/clicontext" "github.com/hashicorp/waypoint/internal/protocolversion" "github.com/hashicorp/waypoint/internal/serverconfig" + "github.com/hashicorp/waypoint/internal/util" ) // ErrNoServerConfig is the error when there is no server configuration @@ -126,8 +127,8 @@ func FromEnv() ConnectOption { return func(c *connectConfig) error { if v := os.Getenv(EnvServerAddr); v != "" { c.Addr = v - c.Tls = os.Getenv(EnvServerTls) != "" - c.TlsSkipVerify = os.Getenv(EnvServerTlsSkipVerify) != "" + c.Tls = util.GetEnvBool(EnvServerTls, false) + c.TlsSkipVerify = util.GetEnvBool(EnvServerTlsSkipVerify, false) c.Auth = os.Getenv(EnvServerToken) != "" } diff --git a/internal/util/env.go b/internal/util/env.go new file mode 100644 index 00000000000..75239ba67ab --- /dev/null +++ b/internal/util/env.go @@ -0,0 +1,20 @@ +package util + +import ( + "os" + "strconv" +) + +// Extracts a boolean from an env var. Falls back to the default if +// The key is unset or not a valid boolean. +func GetEnvBool(key string, defaultValue bool) (value bool) { + envVal := os.Getenv(key) + if envVal == "" { + return defaultValue + } + value, err := strconv.ParseBool(envVal) + if err != nil { + return defaultValue + } + return value +} diff --git a/website/content/docs/entrypoint/disable.mdx b/website/content/docs/entrypoint/disable.mdx index 1b2164a1c47..6ae77e4a5d0 100644 --- a/website/content/docs/entrypoint/disable.mdx +++ b/website/content/docs/entrypoint/disable.mdx @@ -22,10 +22,10 @@ deployments and apps. ## Disable at Runtime You can disable the entrypoint at runtime by setting the `WAYPOINT_CEB_DISABLE` -environment variable to any non-empty value. +environment variable to 1. This environment variable is checked immediately on entrypoint startup. If -it is present and non-empty then the entrypoint will immediately execute the +it is set to 1 then the entrypoint will immediately execute the child process. The entrypoint **will not** attempt to even connect to the server and will not use any network or disk resources. diff --git a/website/content/docs/runner/run-manual.mdx b/website/content/docs/runner/run-manual.mdx index 99449c6a6c7..35ef3bb46d8 100644 --- a/website/content/docs/runner/run-manual.mdx +++ b/website/content/docs/runner/run-manual.mdx @@ -44,7 +44,7 @@ The environment variables specify how to connect to the server: - `WAYPOINT_SERVER_TLS` should be set to "true" if the server is listening on TLS. You may additionally set `WAYPOINT_SERVER_TLS_SKIP_VERIFY` to - a non-empty value if the TLS cert is invalid and can be safely ignored + 1 if the TLS cert is invalid and can be safely ignored such as in a development environment. - `WAYPOINT_SERVER_TOKEN` should be an [auth token](/docs/server/auth) From d2fc3625b3b0915a860d4227dc0962607fc73e7e Mon Sep 17 00:00:00 2001 From: Izaak Lauer <8404559+izaaklauer@users.noreply.github.com> Date: Wed, 23 Jun 2021 03:14:45 +0000 Subject: [PATCH 02/14] backport of commit 3bab6f7e74ebe9f4cc2b265bae604d1107cca2e6 --- website/content/docs/runner/run-manual.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/content/docs/runner/run-manual.mdx b/website/content/docs/runner/run-manual.mdx index 35ef3bb46d8..6790bf0134c 100644 --- a/website/content/docs/runner/run-manual.mdx +++ b/website/content/docs/runner/run-manual.mdx @@ -42,7 +42,7 @@ The environment variables specify how to connect to the server: that the runner can reach. The port should be for the gRPC API, and is typically 9701. -- `WAYPOINT_SERVER_TLS` should be set to "true" if the server is listening +- `WAYPOINT_SERVER_TLS` should be set to 1 if the server is listening on TLS. You may additionally set `WAYPOINT_SERVER_TLS_SKIP_VERIFY` to 1 if the TLS cert is invalid and can be safely ignored such as in a development environment. From 1ca0661118b77dd5ad02ba397a9f7a0180d826b8 Mon Sep 17 00:00:00 2001 From: Izaak Lauer <8404559+izaaklauer@users.noreply.github.com> Date: Wed, 23 Jun 2021 03:18:08 +0000 Subject: [PATCH 03/14] backport of commit 5608e9231c39838ffbb95d00ad9cf2af9496ac20 --- .changelog/1699.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 .changelog/1699.txt diff --git a/.changelog/1699.txt b/.changelog/1699.txt new file mode 100644 index 00000000000..8498ae54ab5 --- /dev/null +++ b/.changelog/1699.txt @@ -0,0 +1,4 @@ +```release-note:improvement +core: Inspect values of enviromment variables meant to be booleans +``` + From f5bfe6a0f60ab7d56f60a4b8a7824cd3a6fab3ff Mon Sep 17 00:00:00 2001 From: Izaak Lauer <8404559+izaaklauer@users.noreply.github.com> Date: Wed, 23 Jun 2021 15:14:07 +0000 Subject: [PATCH 04/14] backport of commit c8a2cbd1c19e93e0f9fb5dba38cbf06c9d6e671d --- .changelog/1699.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/1699.txt b/.changelog/1699.txt index 8498ae54ab5..437f343f25e 100644 --- a/.changelog/1699.txt +++ b/.changelog/1699.txt @@ -1,4 +1,4 @@ ```release-note:improvement -core: Inspect values of enviromment variables meant to be booleans +core: Correct parsing of boolean environment variables ``` From 41d35aa509353f3bbff947a5e30d897268ebdc76 Mon Sep 17 00:00:00 2001 From: Izaak Lauer <8404559+izaaklauer@users.noreply.github.com> Date: Wed, 23 Jun 2021 15:14:33 +0000 Subject: [PATCH 05/14] backport of commit 20ed08faf8daef464855f46b304409eb9e0a77f0 --- internal/ceb/ceb.go | 10 +++++----- internal/cli/main.go | 4 ++-- internal/{util => env}/env.go | 8 ++++---- internal/serverclient/client.go | 6 +++--- 4 files changed, 14 insertions(+), 14 deletions(-) rename internal/{util => env}/env.go (51%) diff --git a/internal/ceb/ceb.go b/internal/ceb/ceb.go index d5fb67b2bfc..b84b75b14ce 100644 --- a/internal/ceb/ceb.go +++ b/internal/ceb/ceb.go @@ -16,11 +16,11 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "github.com/hashicorp/waypoint/internal/env" "github.com/hashicorp/waypoint/internal/pkg/gatedwriter" "github.com/hashicorp/waypoint/internal/plugin" "github.com/hashicorp/waypoint/internal/server" pb "github.com/hashicorp/waypoint/internal/server/gen" - "github.com/hashicorp/waypoint/internal/util" "github.com/hashicorp/waypoint/internal/version" ) @@ -300,11 +300,11 @@ func WithEnvDefaults() Option { cfg.URLServicePort = port cfg.ServerAddr = os.Getenv(envServerAddr) - cfg.ServerRequired = util.GetEnvBool(envCEBServerRequired, false) - cfg.ServerTls = util.GetEnvBool(envServerTls, false) - cfg.ServerTlsSkipVerify = util.GetEnvBool(envServerTlsSkipVerify, false) + cfg.ServerRequired = env.GetEnvBool(envCEBServerRequired, false) + cfg.ServerTls = env.GetEnvBool(envServerTls, false) + cfg.ServerTlsSkipVerify = env.GetEnvBool(envServerTlsSkipVerify, false) cfg.InviteToken = os.Getenv(envCEBToken) - cfg.disable = util.GetEnvBool(envCEBDisable, false) + cfg.disable = env.GetEnvBool(envCEBDisable, false) ceb.deploymentId = os.Getenv(envDeploymentId) diff --git a/internal/cli/main.go b/internal/cli/main.go index ef9ff2803dd..40c811438fb 100644 --- a/internal/cli/main.go +++ b/internal/cli/main.go @@ -18,8 +18,8 @@ import ( "github.com/mitchellh/go-glint" "github.com/hashicorp/waypoint-plugin-sdk/terminal" + "github.com/hashicorp/waypoint/internal/env" "github.com/hashicorp/waypoint/internal/pkg/signalcontext" - "github.com/hashicorp/waypoint/internal/util" "github.com/hashicorp/waypoint/internal/version" ) @@ -140,7 +140,7 @@ func Commands( } // Set plain mode if set - if util.GetEnvBool(EnvPlain, false) { + if env.GetEnvBool(EnvPlain, false) { baseCommand.globalOptions = append(baseCommand.globalOptions, WithUI(terminal.NonInteractiveUI(ctx))) } diff --git a/internal/util/env.go b/internal/env/env.go similarity index 51% rename from internal/util/env.go rename to internal/env/env.go index 75239ba67ab..1bb88644e62 100644 --- a/internal/util/env.go +++ b/internal/env/env.go @@ -1,13 +1,13 @@ -package util +package env import ( "os" "strconv" ) -// Extracts a boolean from an env var. Falls back to the default if -// The key is unset or not a valid boolean. -func GetEnvBool(key string, defaultValue bool) (value bool) { +// GetEnvBool Extracts a boolean from an env var. Falls back to the default +// if the key is unset or not a valid boolean. +func GetEnvBool(key string, defaultValue bool) bool { envVal := os.Getenv(key) if envVal == "" { return defaultValue diff --git a/internal/serverclient/client.go b/internal/serverclient/client.go index 455414d05c0..40a198b0645 100644 --- a/internal/serverclient/client.go +++ b/internal/serverclient/client.go @@ -12,9 +12,9 @@ import ( "google.golang.org/grpc/credentials" "github.com/hashicorp/waypoint/internal/clicontext" + "github.com/hashicorp/waypoint/internal/env" "github.com/hashicorp/waypoint/internal/protocolversion" "github.com/hashicorp/waypoint/internal/serverconfig" - "github.com/hashicorp/waypoint/internal/util" ) // ErrNoServerConfig is the error when there is no server configuration @@ -127,8 +127,8 @@ func FromEnv() ConnectOption { return func(c *connectConfig) error { if v := os.Getenv(EnvServerAddr); v != "" { c.Addr = v - c.Tls = util.GetEnvBool(EnvServerTls, false) - c.TlsSkipVerify = util.GetEnvBool(EnvServerTlsSkipVerify, false) + c.Tls = env.GetEnvBool(EnvServerTls, false) + c.TlsSkipVerify = env.GetEnvBool(EnvServerTlsSkipVerify, false) c.Auth = os.Getenv(EnvServerToken) != "" } From 1830e73dbddd091f0ad84fe6f4d79fc98c7e86f9 Mon Sep 17 00:00:00 2001 From: Izaak Lauer <8404559+izaaklauer@users.noreply.github.com> Date: Wed, 23 Jun 2021 15:14:44 +0000 Subject: [PATCH 06/14] backport of commit c7f0a641415a52aabbf7f895d06f6f751e00307c --- website/content/docs/automating-execution/index.mdx | 6 +++--- website/content/docs/entrypoint/disable.mdx | 4 ++-- website/content/docs/runner/run-manual.mdx | 8 +++++--- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/website/content/docs/automating-execution/index.mdx b/website/content/docs/automating-execution/index.mdx index c79d7518caa..65219f19e38 100644 --- a/website/content/docs/automating-execution/index.mdx +++ b/website/content/docs/automating-execution/index.mdx @@ -43,9 +43,9 @@ configure the Waypoint CLI for communication with the Waypoint server. - `WAYPOINT_SERVER_TOKEN`. Must be set to a Waypoint token, created with [`waypoint token new`](/commands/token-new) - `WAYPOINT_SERVER_ADDR`. The address to the Waypoint server gRPC address. This must be accessible from the network that the client is running on. -- `WAYPOINT_SERVER_TLS`. Should be set to `1` to configure the client to use TLS when +- `WAYPOINT_SERVER_TLS`. Should be set to a truthy value (e.g. "1") to configure the client to use TLS when connecting to the server. -- `WAYPOINT_SERVER_TLS_SKIP_VERIFY`. Current must be set to `1` to disable TLS verification +- `WAYPOINT_SERVER_TLS_SKIP_VERIFY`. Current must be set to truthy value (e.g. "1") to disable TLS verification when communicating with the server. ~> The Waypoint server token is sensitive and should be @@ -71,7 +71,7 @@ If Waypoint detects that the terminal does not support interactivity it will out simpler "non-interactive" output mode. This output is easier to consume in a CI/CD systems logging views, or through log archiving systems. This is automatic and does not need to be explicitly configured, but can be forced by setting -the environment variable `WAYPOINT_PLAIN` to `1`. +the environment variable `WAYPOINT_PLAIN` to a truthy value (e.g. "1"). ## Workspaces diff --git a/website/content/docs/entrypoint/disable.mdx b/website/content/docs/entrypoint/disable.mdx index 6ae77e4a5d0..ef95ff39d6d 100644 --- a/website/content/docs/entrypoint/disable.mdx +++ b/website/content/docs/entrypoint/disable.mdx @@ -22,10 +22,10 @@ deployments and apps. ## Disable at Runtime You can disable the entrypoint at runtime by setting the `WAYPOINT_CEB_DISABLE` -environment variable to 1. +environment variable to a truthy value (e.g. "1"). This environment variable is checked immediately on entrypoint startup. If -it is set to 1 then the entrypoint will immediately execute the +the value is true, then the entrypoint will immediately execute the child process. The entrypoint **will not** attempt to even connect to the server and will not use any network or disk resources. diff --git a/website/content/docs/runner/run-manual.mdx b/website/content/docs/runner/run-manual.mdx index 6790bf0134c..39d5cf19b6d 100644 --- a/website/content/docs/runner/run-manual.mdx +++ b/website/content/docs/runner/run-manual.mdx @@ -42,9 +42,11 @@ The environment variables specify how to connect to the server: that the runner can reach. The port should be for the gRPC API, and is typically 9701. -- `WAYPOINT_SERVER_TLS` should be set to 1 if the server is listening - on TLS. You may additionally set `WAYPOINT_SERVER_TLS_SKIP_VERIFY` to - 1 if the TLS cert is invalid and can be safely ignored +- `WAYPOINT_SERVER_TLS` should be set to a truthy value (e.g. "1") if + the server is listening on TLS. + +- `WAYPOINT_SERVER_TLS_SKIP_VERIFY` should be set to a truthy value + (e.g. "1") if the TLS cert is invalid and can be safely ignored, such as in a development environment. - `WAYPOINT_SERVER_TOKEN` should be an [auth token](/docs/server/auth) From d161a0e4c8355e54f73b48d0cba9b276c1d5dbd9 Mon Sep 17 00:00:00 2001 From: Izaak Lauer <8404559+izaaklauer@users.noreply.github.com> Date: Wed, 23 Jun 2021 15:31:36 +0000 Subject: [PATCH 07/14] backport of commit 7a15570c4d607f9dbeee210bfe25407d997d4a10 --- internal/env/env_test.go | 42 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 internal/env/env_test.go diff --git a/internal/env/env_test.go b/internal/env/env_test.go new file mode 100644 index 00000000000..e23d91b8453 --- /dev/null +++ b/internal/env/env_test.go @@ -0,0 +1,42 @@ +package env + +import ( + "github.com/stretchr/testify/require" + "os" + "testing" +) + +func TestGetEnvBool(t *testing.T) { + envVarTestKey := "WAYPOINT_GET_ENV_BOOL_TEST" + require := require.New(t) + + t.Run("Unset env var returns default", func (t *testing.T) { + require.True(GetEnvBool(envVarTestKey, true)) + require.False(GetEnvBool(envVarTestKey, false)) + }) + + t.Run("Empty env var returns default", func (t *testing.T) { + os.Setenv(envVarTestKey, "") + require.True(GetEnvBool(envVarTestKey, true)) + require.False(GetEnvBool(envVarTestKey, false)) + }) + + t.Run("Non-truthy env var returns true", func (t *testing.T) { + os.Setenv(envVarTestKey, "unparseable") + require.True(GetEnvBool(envVarTestKey, true)) + require.False(GetEnvBool(envVarTestKey, false)) + }) + + t.Run("true/false env vars return non-default", func (t *testing.T) { + os.Setenv(envVarTestKey, "true") + require.True(GetEnvBool(envVarTestKey, false)) + + os.Setenv(envVarTestKey, "false") + require.False(GetEnvBool(envVarTestKey, true)) + }) + + t.Run("1 evaluates as true", func (t *testing.T) { + os.Setenv(envVarTestKey, "1") + require.True(GetEnvBool(envVarTestKey, false)) + }) +} From c5ee1938df17ad0b7e8658b170ab39ad4a5100c1 Mon Sep 17 00:00:00 2001 From: Izaak Lauer <8404559+izaaklauer@users.noreply.github.com> Date: Wed, 23 Jun 2021 16:26:50 +0000 Subject: [PATCH 08/14] backport of commit b73b0fe60a170f323ddb938470889c5f5b1c3cf3 --- internal/env/env_test.go | 3 ++- website/content/docs/automating-execution/index.mdx | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/env/env_test.go b/internal/env/env_test.go index e23d91b8453..58e866675ad 100644 --- a/internal/env/env_test.go +++ b/internal/env/env_test.go @@ -21,7 +21,7 @@ func TestGetEnvBool(t *testing.T) { require.False(GetEnvBool(envVarTestKey, false)) }) - t.Run("Non-truthy env var returns true", func (t *testing.T) { + t.Run("Non-truthy env var returns default", func (t *testing.T) { os.Setenv(envVarTestKey, "unparseable") require.True(GetEnvBool(envVarTestKey, true)) require.False(GetEnvBool(envVarTestKey, false)) @@ -38,5 +38,6 @@ func TestGetEnvBool(t *testing.T) { t.Run("1 evaluates as true", func (t *testing.T) { os.Setenv(envVarTestKey, "1") require.True(GetEnvBool(envVarTestKey, false)) + require.True(GetEnvBool(envVarTestKey, true)) }) } diff --git a/website/content/docs/automating-execution/index.mdx b/website/content/docs/automating-execution/index.mdx index 65219f19e38..3f330322c4d 100644 --- a/website/content/docs/automating-execution/index.mdx +++ b/website/content/docs/automating-execution/index.mdx @@ -45,7 +45,7 @@ configure the Waypoint CLI for communication with the Waypoint server. be accessible from the network that the client is running on. - `WAYPOINT_SERVER_TLS`. Should be set to a truthy value (e.g. "1") to configure the client to use TLS when connecting to the server. -- `WAYPOINT_SERVER_TLS_SKIP_VERIFY`. Current must be set to truthy value (e.g. "1") to disable TLS verification +- `WAYPOINT_SERVER_TLS_SKIP_VERIFY`. Current must be set to a truthy value (e.g. "1") to disable TLS verification when communicating with the server. ~> The Waypoint server token is sensitive and should be From f17d8bff98ba8b24077d3d13444f7a01ef813118 Mon Sep 17 00:00:00 2001 From: Izaak Lauer <8404559+izaaklauer@users.noreply.github.com> Date: Wed, 23 Jun 2021 17:18:29 +0000 Subject: [PATCH 09/14] backport of commit f6206b300d6e70406f8ec29d80c8679dc95f2f4d --- internal/env/env_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/env/env_test.go b/internal/env/env_test.go index 58e866675ad..92f136307bd 100644 --- a/internal/env/env_test.go +++ b/internal/env/env_test.go @@ -10,24 +10,24 @@ func TestGetEnvBool(t *testing.T) { envVarTestKey := "WAYPOINT_GET_ENV_BOOL_TEST" require := require.New(t) - t.Run("Unset env var returns default", func (t *testing.T) { + t.Run("Unset env var returns default", func(t *testing.T) { require.True(GetEnvBool(envVarTestKey, true)) require.False(GetEnvBool(envVarTestKey, false)) }) - t.Run("Empty env var returns default", func (t *testing.T) { + t.Run("Empty env var returns default", func(t *testing.T) { os.Setenv(envVarTestKey, "") require.True(GetEnvBool(envVarTestKey, true)) require.False(GetEnvBool(envVarTestKey, false)) }) - t.Run("Non-truthy env var returns default", func (t *testing.T) { + t.Run("Non-truthy env var returns default", func(t *testing.T) { os.Setenv(envVarTestKey, "unparseable") require.True(GetEnvBool(envVarTestKey, true)) require.False(GetEnvBool(envVarTestKey, false)) }) - t.Run("true/false env vars return non-default", func (t *testing.T) { + t.Run("true/false env vars return non-default", func(t *testing.T) { os.Setenv(envVarTestKey, "true") require.True(GetEnvBool(envVarTestKey, false)) @@ -35,7 +35,7 @@ func TestGetEnvBool(t *testing.T) { require.False(GetEnvBool(envVarTestKey, true)) }) - t.Run("1 evaluates as true", func (t *testing.T) { + t.Run("1 evaluates as true", func(t *testing.T) { os.Setenv(envVarTestKey, "1") require.True(GetEnvBool(envVarTestKey, false)) require.True(GetEnvBool(envVarTestKey, true)) From 80a0d50fa913ea388dad71f1339dbc4a883f6dc2 Mon Sep 17 00:00:00 2001 From: Izaak Lauer <8404559+izaaklauer@users.noreply.github.com> Date: Wed, 23 Jun 2021 17:50:14 +0000 Subject: [PATCH 10/14] backport of commit f1e4ded933e4f69f5f5014c1edc2b72d4d72e391 --- internal/ceb/ceb.go | 29 +++++++++++++++++++--- internal/cli/main.go | 6 ++++- internal/env/env.go | 12 +++++---- internal/env/env_test.go | 44 ++++++++++++++++++++++++--------- internal/serverclient/client.go | 15 +++++++++-- 5 files changed, 83 insertions(+), 23 deletions(-) diff --git a/internal/ceb/ceb.go b/internal/ceb/ceb.go index b84b75b14ce..3486d994834 100644 --- a/internal/ceb/ceb.go +++ b/internal/ceb/ceb.go @@ -300,11 +300,32 @@ func WithEnvDefaults() Option { cfg.URLServicePort = port cfg.ServerAddr = os.Getenv(envServerAddr) - cfg.ServerRequired = env.GetEnvBool(envCEBServerRequired, false) - cfg.ServerTls = env.GetEnvBool(envServerTls, false) - cfg.ServerTlsSkipVerify = env.GetEnvBool(envServerTlsSkipVerify, false) + + serverRequiredBool, err := env.GetEnvBool(envCEBServerRequired, false) + if err != nil { + return err + } + cfg.ServerRequired = serverRequiredBool + + serverTlsBool, err := env.GetEnvBool(envServerTls, false) + if err != nil { + return err + } + cfg.ServerTls = serverTlsBool + + serverTlsSkipVerifyBool, err := env.GetEnvBool(envServerTlsSkipVerify, false) + if err != nil { + return err + } + cfg.ServerTlsSkipVerify = serverTlsSkipVerifyBool + cfg.InviteToken = os.Getenv(envCEBToken) - cfg.disable = env.GetEnvBool(envCEBDisable, false) + + disableCEBBool, err := env.GetEnvBool(envCEBDisable, false) + if err != nil { + return err + } + cfg.disable = disableCEBBool ceb.deploymentId = os.Getenv(envDeploymentId) diff --git a/internal/cli/main.go b/internal/cli/main.go index 40c811438fb..4dc47c62792 100644 --- a/internal/cli/main.go +++ b/internal/cli/main.go @@ -140,7 +140,11 @@ func Commands( } // Set plain mode if set - if env.GetEnvBool(EnvPlain, false) { + outputModeBool, err := env.GetEnvBool(EnvPlain, false) + if err != nil { + log.Warn(err.Error()) + } + if outputModeBool { baseCommand.globalOptions = append(baseCommand.globalOptions, WithUI(terminal.NonInteractiveUI(ctx))) } diff --git a/internal/env/env.go b/internal/env/env.go index 1bb88644e62..860b7ef065d 100644 --- a/internal/env/env.go +++ b/internal/env/env.go @@ -1,20 +1,22 @@ package env import ( + "fmt" "os" "strconv" + "strings" ) // GetEnvBool Extracts a boolean from an env var. Falls back to the default // if the key is unset or not a valid boolean. -func GetEnvBool(key string, defaultValue bool) bool { +func GetEnvBool(key string, defaultValue bool) (bool, error) { envVal := os.Getenv(key) if envVal == "" { - return defaultValue + return defaultValue, nil } - value, err := strconv.ParseBool(envVal) + value, err := strconv.ParseBool(strings.ToLower(envVal))) if err != nil { - return defaultValue + return defaultValue, fmt.Errorf("failed to parse a boolean from environment variable %s=%s",key,envVal) } - return value + return value, nil } diff --git a/internal/env/env_test.go b/internal/env/env_test.go index 92f136307bd..b4ab4bb9781 100644 --- a/internal/env/env_test.go +++ b/internal/env/env_test.go @@ -11,33 +11,55 @@ func TestGetEnvBool(t *testing.T) { require := require.New(t) t.Run("Unset env var returns default", func(t *testing.T) { - require.True(GetEnvBool(envVarTestKey, true)) - require.False(GetEnvBool(envVarTestKey, false)) + b, err := GetEnvBool(envVarTestKey, true) + require.NoError(err) + require.True(b) + + b, err = GetEnvBool(envVarTestKey, false) + require.NoError(err) + require.False(b) }) t.Run("Empty env var returns default", func(t *testing.T) { os.Setenv(envVarTestKey, "") - require.True(GetEnvBool(envVarTestKey, true)) - require.False(GetEnvBool(envVarTestKey, false)) + b, err := GetEnvBool(envVarTestKey, true) + require.NoError(err) + require.True(b) + + b, err = GetEnvBool(envVarTestKey, false) + require.NoError(err) + require.False(b) }) - t.Run("Non-truthy env var returns default", func(t *testing.T) { + t.Run("Non-truthy env var returns an error", func(t *testing.T) { os.Setenv(envVarTestKey, "unparseable") - require.True(GetEnvBool(envVarTestKey, true)) - require.False(GetEnvBool(envVarTestKey, false)) + _, err := GetEnvBool(envVarTestKey, true) + require.Error(err) }) t.Run("true/false env vars return non-default", func(t *testing.T) { os.Setenv(envVarTestKey, "true") - require.True(GetEnvBool(envVarTestKey, false)) + b, err := GetEnvBool(envVarTestKey, false) + require.NoError(err) + require.True(b) os.Setenv(envVarTestKey, "false") - require.False(GetEnvBool(envVarTestKey, true)) + b, err = GetEnvBool(envVarTestKey, true) + require.NoError(err) + require.False(b) + }) + + t.Run("boolean parsing is generous with capitalization", func(t *testing.T) { + os.Setenv(envVarTestKey, "tRuE") + b, err := GetEnvBool(envVarTestKey, false) + require.NoError(err) + require.True(b) }) t.Run("1 evaluates as true", func(t *testing.T) { os.Setenv(envVarTestKey, "1") - require.True(GetEnvBool(envVarTestKey, false)) - require.True(GetEnvBool(envVarTestKey, true)) + b, err := GetEnvBool(envVarTestKey, false) + require.NoError(err) + require.True(b) }) } diff --git a/internal/serverclient/client.go b/internal/serverclient/client.go index 40a198b0645..fbd03541c72 100644 --- a/internal/serverclient/client.go +++ b/internal/serverclient/client.go @@ -127,8 +127,19 @@ func FromEnv() ConnectOption { return func(c *connectConfig) error { if v := os.Getenv(EnvServerAddr); v != "" { c.Addr = v - c.Tls = env.GetEnvBool(EnvServerTls, false) - c.TlsSkipVerify = env.GetEnvBool(EnvServerTlsSkipVerify, false) + + tlsBool, err := env.GetEnvBool(EnvServerTls, false) + if err != nil { + return err + } + c.Tls = tlsBool + + tlsSkipVerifyBool, err := env.GetEnvBool(EnvServerTlsSkipVerify, false) + if err != nil { + return err + } + c.TlsSkipVerify = tlsSkipVerifyBool + c.Auth = os.Getenv(EnvServerToken) != "" } From 34b941e8a2863b8e821a0bd7489e27e113f28d8e Mon Sep 17 00:00:00 2001 From: Izaak Lauer <8404559+izaaklauer@users.noreply.github.com> Date: Wed, 23 Jun 2021 17:56:16 +0000 Subject: [PATCH 11/14] backport of commit 1016465fad15989378e660570222a0de48e94104 --- internal/ceb/ceb.go | 14 +++++--------- internal/env/env.go | 2 +- internal/serverclient/client.go | 8 +++----- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/internal/ceb/ceb.go b/internal/ceb/ceb.go index 3486d994834..1566e69e33f 100644 --- a/internal/ceb/ceb.go +++ b/internal/ceb/ceb.go @@ -283,7 +283,7 @@ type Option func(*CEB, *config) error // environment variables. If this is NOT called, then the environment variable // based confiugration will be ignored. func WithEnvDefaults() Option { - return func(ceb *CEB, cfg *config) error { + return func(ceb *CEB, cfg *config) (err error) { var port int portStr := os.Getenv("PORT") if portStr == "" { @@ -301,31 +301,27 @@ func WithEnvDefaults() Option { cfg.URLServicePort = port cfg.ServerAddr = os.Getenv(envServerAddr) - serverRequiredBool, err := env.GetEnvBool(envCEBServerRequired, false) + cfg.ServerRequired, err = env.GetEnvBool(envCEBServerRequired, false) if err != nil { return err } - cfg.ServerRequired = serverRequiredBool - serverTlsBool, err := env.GetEnvBool(envServerTls, false) + cfg.ServerTls, err = env.GetEnvBool(envServerTls, false) if err != nil { return err } - cfg.ServerTls = serverTlsBool - serverTlsSkipVerifyBool, err := env.GetEnvBool(envServerTlsSkipVerify, false) + cfg.ServerTlsSkipVerify, err = env.GetEnvBool(envServerTlsSkipVerify, false) if err != nil { return err } - cfg.ServerTlsSkipVerify = serverTlsSkipVerifyBool cfg.InviteToken = os.Getenv(envCEBToken) - disableCEBBool, err := env.GetEnvBool(envCEBDisable, false) + cfg.disable, err = env.GetEnvBool(envCEBDisable, false) if err != nil { return err } - cfg.disable = disableCEBBool ceb.deploymentId = os.Getenv(envDeploymentId) diff --git a/internal/env/env.go b/internal/env/env.go index 860b7ef065d..59f95ed8d45 100644 --- a/internal/env/env.go +++ b/internal/env/env.go @@ -14,7 +14,7 @@ func GetEnvBool(key string, defaultValue bool) (bool, error) { if envVal == "" { return defaultValue, nil } - value, err := strconv.ParseBool(strings.ToLower(envVal))) + value, err := strconv.ParseBool(strings.ToLower(envVal)) if err != nil { return defaultValue, fmt.Errorf("failed to parse a boolean from environment variable %s=%s",key,envVal) } diff --git a/internal/serverclient/client.go b/internal/serverclient/client.go index fbd03541c72..d186eb7e805 100644 --- a/internal/serverclient/client.go +++ b/internal/serverclient/client.go @@ -124,21 +124,19 @@ type connectConfig struct { // FromEnv sources the connection information from the environment // using standard environment variables. func FromEnv() ConnectOption { - return func(c *connectConfig) error { + return func(c *connectConfig) (err error) { if v := os.Getenv(EnvServerAddr); v != "" { c.Addr = v - tlsBool, err := env.GetEnvBool(EnvServerTls, false) + c.Tls, err = env.GetEnvBool(EnvServerTls, false) if err != nil { return err } - c.Tls = tlsBool - tlsSkipVerifyBool, err := env.GetEnvBool(EnvServerTlsSkipVerify, false) + c.TlsSkipVerify, err = env.GetEnvBool(EnvServerTlsSkipVerify, false) if err != nil { return err } - c.TlsSkipVerify = tlsSkipVerifyBool c.Auth = os.Getenv(EnvServerToken) != "" } From ae63d410402ff18bbc1d6706d3070635a4dfb155 Mon Sep 17 00:00:00 2001 From: Izaak Lauer <8404559+izaaklauer@users.noreply.github.com> Date: Wed, 23 Jun 2021 17:57:17 +0000 Subject: [PATCH 12/14] backport of commit 8ecd9a9919a601c6b8f354d05e9390ee07bb154c --- internal/env/env.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/env/env.go b/internal/env/env.go index 59f95ed8d45..3db240a8f43 100644 --- a/internal/env/env.go +++ b/internal/env/env.go @@ -16,7 +16,7 @@ func GetEnvBool(key string, defaultValue bool) (bool, error) { } value, err := strconv.ParseBool(strings.ToLower(envVal)) if err != nil { - return defaultValue, fmt.Errorf("failed to parse a boolean from environment variable %s=%s",key,envVal) + return defaultValue, fmt.Errorf("failed to parse a boolean from environment variable %s=%s", key, envVal) } return value, nil } From aed22219cbf614714f79486af9e52fd7a094e5ad Mon Sep 17 00:00:00 2001 From: Izaak Lauer <8404559+izaaklauer@users.noreply.github.com> Date: Wed, 23 Jun 2021 18:21:33 +0000 Subject: [PATCH 13/14] backport of commit 19b409d14764a327f9a8cf1e052232863fd08696 --- internal/ceb/ceb.go | 8 +- internal/cli/main.go | 2 +- internal/env/env.go | 4 +- internal/env/env_test.go | 133 +++++++++++++++++++------------- internal/serverclient/client.go | 4 +- 5 files changed, 87 insertions(+), 64 deletions(-) diff --git a/internal/ceb/ceb.go b/internal/ceb/ceb.go index 1566e69e33f..c9e8dd4deac 100644 --- a/internal/ceb/ceb.go +++ b/internal/ceb/ceb.go @@ -301,24 +301,24 @@ func WithEnvDefaults() Option { cfg.URLServicePort = port cfg.ServerAddr = os.Getenv(envServerAddr) - cfg.ServerRequired, err = env.GetEnvBool(envCEBServerRequired, false) + cfg.ServerRequired, err = env.GetBool(envCEBServerRequired, false) if err != nil { return err } - cfg.ServerTls, err = env.GetEnvBool(envServerTls, false) + cfg.ServerTls, err = env.GetBool(envServerTls, false) if err != nil { return err } - cfg.ServerTlsSkipVerify, err = env.GetEnvBool(envServerTlsSkipVerify, false) + cfg.ServerTlsSkipVerify, err = env.GetBool(envServerTlsSkipVerify, false) if err != nil { return err } cfg.InviteToken = os.Getenv(envCEBToken) - cfg.disable, err = env.GetEnvBool(envCEBDisable, false) + cfg.disable, err = env.GetBool(envCEBDisable, false) if err != nil { return err } diff --git a/internal/cli/main.go b/internal/cli/main.go index 4dc47c62792..c20a119514f 100644 --- a/internal/cli/main.go +++ b/internal/cli/main.go @@ -140,7 +140,7 @@ func Commands( } // Set plain mode if set - outputModeBool, err := env.GetEnvBool(EnvPlain, false) + outputModeBool, err := env.GetBool(EnvPlain, false) if err != nil { log.Warn(err.Error()) } diff --git a/internal/env/env.go b/internal/env/env.go index 3db240a8f43..f53f5a3654f 100644 --- a/internal/env/env.go +++ b/internal/env/env.go @@ -7,9 +7,9 @@ import ( "strings" ) -// GetEnvBool Extracts a boolean from an env var. Falls back to the default +// GetBool Extracts a boolean from an env var. Falls back to the default // if the key is unset or not a valid boolean. -func GetEnvBool(key string, defaultValue bool) (bool, error) { +func GetBool(key string, defaultValue bool) (bool, error) { envVal := os.Getenv(key) if envVal == "" { return defaultValue, nil diff --git a/internal/env/env_test.go b/internal/env/env_test.go index b4ab4bb9781..14ea8858494 100644 --- a/internal/env/env_test.go +++ b/internal/env/env_test.go @@ -1,65 +1,88 @@ package env import ( - "github.com/stretchr/testify/require" "os" "testing" ) -func TestGetEnvBool(t *testing.T) { +func TestGetBool(t *testing.T) { envVarTestKey := "WAYPOINT_GET_ENV_BOOL_TEST" - require := require.New(t) - t.Run("Unset env var returns default", func(t *testing.T) { - b, err := GetEnvBool(envVarTestKey, true) - require.NoError(err) - require.True(b) - - b, err = GetEnvBool(envVarTestKey, false) - require.NoError(err) - require.False(b) - }) - - t.Run("Empty env var returns default", func(t *testing.T) { - os.Setenv(envVarTestKey, "") - b, err := GetEnvBool(envVarTestKey, true) - require.NoError(err) - require.True(b) - - b, err = GetEnvBool(envVarTestKey, false) - require.NoError(err) - require.False(b) - }) - - t.Run("Non-truthy env var returns an error", func(t *testing.T) { - os.Setenv(envVarTestKey, "unparseable") - _, err := GetEnvBool(envVarTestKey, true) - require.Error(err) - }) - - t.Run("true/false env vars return non-default", func(t *testing.T) { - os.Setenv(envVarTestKey, "true") - b, err := GetEnvBool(envVarTestKey, false) - require.NoError(err) - require.True(b) - - os.Setenv(envVarTestKey, "false") - b, err = GetEnvBool(envVarTestKey, true) - require.NoError(err) - require.False(b) - }) - - t.Run("boolean parsing is generous with capitalization", func(t *testing.T) { - os.Setenv(envVarTestKey, "tRuE") - b, err := GetEnvBool(envVarTestKey, false) - require.NoError(err) - require.True(b) - }) - - t.Run("1 evaluates as true", func(t *testing.T) { - os.Setenv(envVarTestKey, "1") - b, err := GetEnvBool(envVarTestKey, false) - require.NoError(err) - require.True(b) - }) + tests := []struct { + name string + defaultVal bool + envVal string + want bool + wantErr bool + }{ + { + name: "Empty env var returns default 1", + defaultVal: true, + envVal: "", + want: true, + wantErr: false, + }, + { + name: "Empty env var returns default 2", + defaultVal: false, + envVal: "", + want: false, + wantErr: false, + }, + { + name: "Non-truthy env var returns err", + defaultVal: false, + envVal: "unparseable", + want: false, + wantErr: true, + }, + { + name: "'true' is true", + defaultVal: false, + envVal: "true", + want: true, + wantErr: false, + }, + { + name: "'false' is true", + defaultVal: true, + envVal: "false", + want: false, + wantErr: false, + }, + { + name: "1 is true", + defaultVal: false, + envVal: "1", + want: true, + wantErr: false, + }, + { + name: "0 is false", + defaultVal: true, + envVal: "0", + want: false, + wantErr: false, + }, + { + name: "Boolean parsing ignores capitalization", + defaultVal: false, + envVal: "tRuE", + want: true, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + os.Setenv(envVarTestKey, tt.envVal) + got, err := GetBool(envVarTestKey, tt.defaultVal) + if (err != nil) != tt.wantErr { + t.Errorf("GetBool() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("GetBool() got = %v, want %v", got, tt.want) + } + }) + } } diff --git a/internal/serverclient/client.go b/internal/serverclient/client.go index d186eb7e805..43943f9b058 100644 --- a/internal/serverclient/client.go +++ b/internal/serverclient/client.go @@ -128,12 +128,12 @@ func FromEnv() ConnectOption { if v := os.Getenv(EnvServerAddr); v != "" { c.Addr = v - c.Tls, err = env.GetEnvBool(EnvServerTls, false) + c.Tls, err = env.GetBool(EnvServerTls, false) if err != nil { return err } - c.TlsSkipVerify, err = env.GetEnvBool(EnvServerTlsSkipVerify, false) + c.TlsSkipVerify, err = env.GetBool(EnvServerTlsSkipVerify, false) if err != nil { return err } From 146782f4fe7118e265c77109204ec57e7541b343 Mon Sep 17 00:00:00 2001 From: Izaak Lauer <8404559+izaaklauer@users.noreply.github.com> Date: Thu, 24 Jun 2021 18:27:07 +0000 Subject: [PATCH 14/14] backport of commit 36fe4ae9ebb437a6762d3ec60720f3786aa9d08b --- internal/ceb/ceb.go | 3 ++- internal/serverclient/client.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/ceb/ceb.go b/internal/ceb/ceb.go index c9e8dd4deac..109809dd0a6 100644 --- a/internal/ceb/ceb.go +++ b/internal/ceb/ceb.go @@ -283,7 +283,7 @@ type Option func(*CEB, *config) error // environment variables. If this is NOT called, then the environment variable // based confiugration will be ignored. func WithEnvDefaults() Option { - return func(ceb *CEB, cfg *config) (err error) { + return func(ceb *CEB, cfg *config) error { var port int portStr := os.Getenv("PORT") if portStr == "" { @@ -301,6 +301,7 @@ func WithEnvDefaults() Option { cfg.URLServicePort = port cfg.ServerAddr = os.Getenv(envServerAddr) + var err error cfg.ServerRequired, err = env.GetBool(envCEBServerRequired, false) if err != nil { return err diff --git a/internal/serverclient/client.go b/internal/serverclient/client.go index 43943f9b058..7729394c7af 100644 --- a/internal/serverclient/client.go +++ b/internal/serverclient/client.go @@ -124,10 +124,11 @@ type connectConfig struct { // FromEnv sources the connection information from the environment // using standard environment variables. func FromEnv() ConnectOption { - return func(c *connectConfig) (err error) { + return func(c *connectConfig) error { if v := os.Getenv(EnvServerAddr); v != "" { c.Addr = v + var err error c.Tls, err = env.GetBool(EnvServerTls, false) if err != nil { return err