Tool 2017 expansion improvement (#187)

* Adding new implementation

* Adding test for unset and skip_if_empty

* remove nil check

* Maintain a list of sensitive envs, without redacting contents

* Expand: Use initial OS envs in env expansion.

commandEnvs2 should work the same as the existing commandEnvs.

* Created env package, renamed variables

* Adding e2e tests for run command

* Do not fill missing defaults in environmanet expansion

* Revise tests

* Use shared test cases for integration and run command tests.

* Wire in new implementation, clean up tests

* Adding nil pointer check

* Fixes

* Removed IsSensitive

* Moved tests of GetDeclarationSideEffects next to expand.go

EnvmanSharedTestCases is an exported variable in the env package, to prevent loops in the import paths (as these tests are also
used as integration tests.)

The return value of GetDeclarationSideEffects().ResultEnvrionment has type of map[string]string.

* Minor fixes

* Adding additonal comments

* Typo fix

_tests/integration/run_test.go

@@ -0,0 +1,145 @@
+package integration
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"regexp"
+	"strings"
+	"testing"
+
+	"github.com/bitrise-io/envman/env"
+	"github.com/bitrise-io/go-utils/pathutil"
+	"github.com/stretchr/testify/require"
+)
+
+func TestRun(t *testing.T) {
+	tmpDir, err := pathutil.NormalizedOSTempDirPath("__envman__")
+	require.NoError(t, err)
+
+	envstore := filepath.Join(tmpDir, ".envstore")
+
+	for _, tt := range env.EnvmanSharedTestCases {
+		t.Run(tt.Name, func(t *testing.T) {
+			// Clear and init
+			err := EnvmanInitAtPath(envstore)
+			require.NoError(t, err, "EnvmanInitAtPath()")
+
+			for _, envVar := range tt.Envs {
+				if err := envVar.FillMissingDefaults(); err != nil {
+					require.NoError(t, err, "FillMissingDefaults()")
+				}
+			}
+
+			err = ExportEnvironmentsList(envstore, tt.Envs)
+			require.NoError(t, err, "ExportEnvironmentsList()")
+
+			output, err := EnvmanRun(envstore, tmpDir, []string{"env"})
+			require.NoError(t, err, "EnvmanRun()")
+
+			gotOut, err := parseEnvRawOut(output)
+			require.NoError(t, err, "parseEnvRawOut()")
+
+			// Want envs
+			envsWant := make(map[string]string)
+			for _, envVar := range os.Environ() {
+				key, value := env.SplitEnv(envVar)
+				envsWant[key] = value
+			}
+
+			for _, envCommand := range tt.Want {
+				switch envCommand.Action {
+				case env.SetAction:
+					envsWant[envCommand.Variable.Key] = envCommand.Variable.Value
+				case env.UnsetAction:
+					delete(envsWant, envCommand.Variable.Key)
+				case env.SkipAction:
+				default:
+					t.Fatalf("compare() failed, invalid action: %d", envCommand.Action)
+				}
+			}
+
+			require.Equal(t, envsWant, gotOut)
+		})
+	}
+
+}
+
+// Used for tests only, to parse env command output
+func parseEnvRawOut(output string) (map[string]string, error) {
+	// matches a single line like MYENVKEY_1=myvalue
+	// Shell uses upperscore letters (plus numbers and underscore); Step inputs are lowerscore.
+	// https://pubs.opengroup.org/onlinepubs/9699919799/:
+	// > Environment variable names used by the utilities in the Shell and Utilities volume of POSIX.1-2017
+	// > consist solely of uppercase letters, digits, and the <underscore> ( '_' ) from the characters defined
+	// > in Portable Character Set and do not begin with a digit.
+	// > Other characters may be permitted by an implementation; applications shall tolerate the presence of such names.
+	r := regexp.MustCompile("^([a-zA-Z_][a-zA-Z0-9_]*)=(.*)$")
+
+	lines := strings.Split(output, "\n")
+
+	envs := make(map[string]string)
+	lastKey := ""
+	for _, line := range lines {
+		match := r.FindStringSubmatch(line)
+
+		// If no env is mathced, treat the line as the continuation of the env in the previous line.
+		// `env` command output does not distinguish between a new env in a new line and
+		// and environment value containing newline character.
+		// Newline can be added for example: **  myenv=A$'\n'B env  ** (bash/zsh only)
+		// If called from a script step, the content of the script contains newlines:
+		/*
+			content=#!/usr/bin/env bash
+			set -ex
+			current_envman="..."
+			# ...
+			go test -v ./_tests/integration/..."
+		*/
+		if match == nil {
+			if lastKey != "" {
+				envs[lastKey] += "\n" + line
+			}
+			continue
+		}
+
+		// If match not nil, must have 3 mathces at this point (the matched string and its subexpressions)
+		if len(match) != 3 {
+			return nil, fmt.Errorf("parseEnvRawOut() failed, match (%s) length is not 3 for line (%s).", match, line)
+		}
+
+		lastKey = match[1]
+		envs[match[1]] = match[2]
+	}
+
+	return envs, nil
+}
+
+func Test_parseEnvRawOut(t *testing.T) {
+	tests := []struct {
+		name   string
+		output string
+		want   map[string]string
+	}{
+		{
+			output: `RBENV_SHELL=zsh
+_=/usr/local/bin/go
+#!/bin/env bash
+echo "ff"
+A=`,
+			want: map[string]string{
+				"RBENV_SHELL": "zsh",
+				"_": `/usr/local/bin/go
+#!/bin/env bash
+echo "ff"`,
+				"A": "",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := parseEnvRawOut(tt.output)
+			require.NoError(t, err, "parseEnvRawOut()")
+			require.Equal(t, got, tt.want)
+		})
+	}
+}

_tests/integration/utils.go

@@ -0,0 +1,101 @@
+package integration
+
+import (
+	"os"
+	"os/exec"
+	"strings"
+
+	"github.com/bitrise-io/envman/models"
+	"github.com/bitrise-io/go-utils/command"
+)
+
+// EnvmanInitAtPath ...
+func EnvmanInitAtPath(envstorePth string) error {
+	const logLevel = "debug"
+	args := []string{"--loglevel", logLevel, "--path", envstorePth, "init", "--clear"}
+	return command.RunCommand(binPath(), args...)
+}
+
+// EnvmanAdd ...
+func EnvmanAdd(envstorePth, key, value string, expand, skipIfEmpty bool) error {
+	const logLevel = "debug"
+	args := []string{"--loglevel", logLevel, "--path", envstorePth, "add", "--key", key, "--append"}
+	if !expand {
+		args = append(args, "--no-expand")
+	}
+	if skipIfEmpty {
+		args = append(args, "--skip-if-empty")
+	}
+
+	envman := exec.Command(binPath(), args...)
+	envman.Stdin = strings.NewReader(value)
+	envman.Stdout = os.Stdout
+	envman.Stderr = os.Stderr
+	return envman.Run()
+}
+
+// EnvmanAdd ...
+func EnvmanUnset(envstorePth, key, value string, expand, skipIfEmpty bool) error {
+	const logLevel = "debug"
+	args := []string{"--loglevel", logLevel, "--path", envstorePth, "unset", "--key", key}
+	if !expand {
+		args = append(args, "--no-expand")
+	}
+	if skipIfEmpty {
+		args = append(args, "--skip-if-empty")
+	}
+
+	envman := exec.Command(binPath(), args...)
+	envman.Stdin = strings.NewReader(value)
+	envman.Stdout = os.Stdout
+	envman.Stderr = os.Stderr
+	return envman.Run()
+}
+
+// ExportEnvironmentsList ...
+func ExportEnvironmentsList(envstorePth string, envsList []models.EnvironmentItemModel) error {
+	for _, env := range envsList {
+		key, value, err := env.GetKeyValuePair()
+		if err != nil {
+			return err
+		}
+
+		opts, err := env.GetOptions()
+		if err != nil {
+			return err
+		}
+
+		isExpand := models.DefaultIsExpand
+		if opts.IsExpand != nil {
+			isExpand = *opts.IsExpand
+		}
+
+		skipIfEmpty := models.DefaultSkipIfEmpty
+		if opts.SkipIfEmpty != nil {
+			skipIfEmpty = *opts.SkipIfEmpty
+		}
+
+		if opts.Unset != nil && *opts.Unset {
+			if err := EnvmanUnset(envstorePth, key, value, isExpand, skipIfEmpty); err != nil {
+				return err
+			}
+			continue
+		}
+
+		if err := EnvmanAdd(envstorePth, key, value, isExpand, skipIfEmpty); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// EnvmanRun runs a command through envman.
+func EnvmanRun(envstorePth, workDir string, cmdArgs []string) (string, error) {
+	const logLevel = "panic"
+	args := []string{"--loglevel", logLevel, "--path", envstorePth, "run"}
+	args = append(args, cmdArgs...)
+
+	cmd := command.New(binPath(), args...).SetDir(workDir)
+
+	return cmd.RunAndReturnTrimmedCombinedOutput()
+}

bitrise.yml

@@ -31,6 +31,10 @@ workflows:
 
             export PR="" PULL_REQUEST_ID=""
             export INTEGRATION_TEST_BINARY_PATH="$current_envman"
+
+            # prevent the env var content (with the content of this script)
+            # being added to the test process environment
+            unset content
             go test -v ./_tests/integration/...
 
   create-binaries:

cli/run.go

@@ -1,10 +1,10 @@
 package cli
 
 import (
-	"fmt"
 	"os"
 
 	log "github.com/Sirupsen/logrus"
+	"github.com/bitrise-io/envman/env"
 	"github.com/bitrise-io/envman/envman"
 	"github.com/bitrise-io/envman/models"
 	"github.com/bitrise-io/go-utils/command"
@@ -22,40 +22,18 @@ func expandEnvsInString(inp string) string {
 	return os.ExpandEnv(inp)
 }
 
-func commandEnvs(envs []models.EnvironmentItemModel) ([]string, error) {
-	for _, env := range envs {
-		key, value, err := env.GetKeyValuePair()
-		if err != nil {
-			return []string{}, err
-		}
-
-		opts, err := env.GetOptions()
-		if err != nil {
-			return []string{}, err
-		}
-
-		if opts.Unset != nil && *opts.Unset {
-			if err := os.Unsetenv(key); err != nil {
-				return []string{}, fmt.Errorf("unset env (%s): %s", key, err)
-			}
-			continue
-		}
-
-		if *opts.SkipIfEmpty && value == "" {
-			continue
-		}
-
-		var valueStr string
-		if *opts.IsExpand {
-			valueStr = expandEnvsInString(value)
-		} else {
-			valueStr = value
-		}
+func commandEnvs(newEnvs []models.EnvironmentItemModel) ([]string, error) {
+	result, err := env.GetDeclarationsSideEffects(newEnvs, &env.DefaultEnvironmentSource{})
+	if err != nil {
+		return nil, err
+	}
 
-		if err := os.Setenv(key, valueStr); err != nil {
-			return []string{}, err
+	for _, command := range result.CommandHistory {
+		if err := env.ExecuteCommand(command); err != nil {
+			return nil, err
 		}
 	}
+
 	return os.Environ(), nil
 }