fix: make bool checks more strict

cli/shell/shell.go

@@ -6,6 +6,7 @@ package shell
 
 import (
 	"bytes"
+	"fmt"
 	"os"
 	"os/exec"
 	"strconv"
@@ -65,19 +66,20 @@ func GetEnv(envVar string, defaultValue string) string {
 	return defaultValue
 }
 
-// GetEnvBool returns an environment variable as boolean
-func GetEnvBool(key string) bool {
+// GetEnvBool returns an environment variable as boolean, returning also an error if
+// and only if the variable is not present
+func GetEnvBool(key string) (bool, error) {
 	s := os.Getenv(key)
 	if s == "" {
-		return false
+		return false, fmt.Errorf("The %s variable is not set", key)
 	}
 
 	v, err := strconv.ParseBool(s)
 	if err != nil {
-		return false
+		return false, nil
 	}
 
-	return v
+	return v, nil
 }
 
 // GetEnvInteger returns an environment variable as integer, including a default value

cli/shell/shell_test.go

@@ -0,0 +1,46 @@
+// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+// or more contributor license agreements. Licensed under the Elastic License;
+// you may not use this file except in compliance with the Elastic License.
+
+package shell
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetEnvBool(t *testing.T) {
+	type test struct {
+		key   string
+		value string
+	}
+
+	tests := []test{
+		{key: "should_be_true", value: "true"},
+		{key: "should_be_false", value: "false"},
+		{key: "should_be_empty", value: ""},
+		{key: "should_be_not_empty", value: "garbage"},
+	}
+
+	for _, test := range tests {
+		os.Setenv(test.key, test.value)
+
+		val, err := GetEnvBool(test.key)
+
+		if test.key == "should_be_empty" {
+			assert.NotNil(t, err)
+			assert.False(t, val)
+		} else if test.key == "should_be_not_empty" {
+			assert.Nil(t, err)
+			assert.False(t, val)
+		} else if test.key == "should_be_true" {
+			assert.Nil(t, err)
+			assert.True(t, val)
+		} else if test.key == "should_be_false" {
+			assert.Nil(t, err)
+			assert.False(t, val)
+		}
+	}
+}

e2e/utils.go

@@ -67,7 +67,7 @@ func GetElasticArtifactURL(artifact string, version string, OS string, arch stri
 		return downloadURL, nil
 	}
 
-	useCISnapshots := shell.GetEnvBool("ELASTIC_AGENT_USE_CI_SNAPSHOTS")
+	useCISnapshots, _ := shell.GetEnvBool("ELASTIC_AGENT_USE_CI_SNAPSHOTS")
 	if useCISnapshots {
 		// We will use the snapshots produced by Beats CI
 		bucket := "beats-ci-artifacts"
@@ -115,7 +115,7 @@ func GetElasticArtifactURL(artifact string, version string, OS string, arch stri
 		return nil
 	}
 
-	err := backoff.Retry(apiStatus, exp)
+	err = backoff.Retry(apiStatus, exp)
 	if err != nil {
 		return "", err
 	}