Merge pull request #295 from gravitational/fred/env-loader/bugfixes-1

env-loader: bug fixes

tools/env-loader/cmd/env-loader.go

@@ -18,8 +18,10 @@ package main
 
 import (
 	"fmt"
+	"io"
 	"log"
 	"maps"
+	"os"
 	"slices"
 
 	"github.com/alecthomas/kingpin/v2"
@@ -30,6 +32,9 @@ import (
 
 const EnvVarPrefix = "ENV_LOADER_"
 
+// This is a package-level var to assist with capturing stdout in tests
+var outputWriter io.Writer = os.Stdout
+
 type config struct {
 	EnvironmentsDirectory string
 	Environment           string
@@ -38,12 +43,12 @@ type config struct {
 	Writer                string
 }
 
-func parseCLI() *config {
+func parseCLI(args []string) *config {
 	c := &config{}
 
 	kingpin.Flag("environments-directory", "Path to the directory containing all environments, defaulting to the repo root").
 		Short('d').
-		Envar(EnvVarPrefix + "ENVIRONMENT").
+		Envar(EnvVarPrefix + "ENVIRONMENTS_DIRECTORY").
 		StringVar(&c.EnvironmentsDirectory)
 
 	kingpin.Flag("environment", "Name of the environment containing the values to load").
@@ -68,12 +73,11 @@ func parseCLI() *config {
 		Default("dotenv").
 		EnumVar(&c.Writer, slices.Collect(maps.Keys(writers.FromName))...)
 
-	kingpin.Parse()
-
+	kingpin.MustParse(kingpin.CommandLine.Parse(args))
 	return c
 }
 
-func run(c *config) error {
+func getRequestedEnvValues(c *config) (map[string]string, error) {
 	// Load in values
 	var envValues map[string]string
 	var err error
@@ -84,28 +88,43 @@ func run(c *config) error {
 	}
 
 	if err != nil {
-		return trace.Wrap(err, "failed to load all environment values")
+		return nil, trace.Wrap(err, "failed to load all environment values")
 	}
 
 	// Filter out values not requested
-	maps.DeleteFunc(envValues, func(key, _ string) bool {
-		return !slices.Contains(c.Values, key)
-	})
+	if len(c.Values) > 0 {
+		maps.DeleteFunc(envValues, func(key, _ string) bool {
+			return !slices.Contains(c.Values, key)
+		})
+	}
+
+	return envValues, nil
+}
+
+func run(c *config) error {
+	envValues, err := getRequestedEnvValues(c)
+	if err != nil {
+		return trace.Wrap(err, "failed to get requested environment values")
+	}
 
 	// Build the output string
 	writer := writers.FromName[c.Writer]
-	envValueOutput, err := writer.FormatEnvironmentValues(map[string]string{})
+	envValueOutput, err := writer.FormatEnvironmentValues(envValues)
 	if err != nil {
 		return trace.Wrap(err, "failed to format output values with writer %q", c.Writer)
 	}
 
 	// Write it to stdout
-	fmt.Print(envValueOutput)
+	_, err = fmt.Fprint(outputWriter, envValueOutput)
+	if err != nil {
+		return trace.Wrap(err, "failed to print output %q", envValueOutput)
+	}
+
 	return nil
 }
 
 func main() {
-	c := parseCLI()
+	c := parseCLI(os.Args[1:])
 
 	err := run(c)
 	if err != nil {

tools/env-loader/cmd/env-loader_test.go

@@ -0,0 +1,129 @@
+package main
+
+import (
+	"bytes"
+	"path/filepath"
+	"slices"
+	"testing"
+
+	"github.com/alecthomas/kingpin/v2"
+	"github.com/stretchr/testify/require"
+)
+
+func TestParseCli(t *testing.T) {
+	parseCLI([]string{})
+
+	flags := kingpin.CommandLine.Model().Flags
+	flagEnvVars := make([]string, 0, len(flags))
+	for _, flag := range flags {
+		if flag.Envar != "" {
+			flagEnvVars = append(flagEnvVars, flag.Envar)
+		}
+	}
+
+	uniqueFlagEnvVars := slices.Compact(slices.Clone(flagEnvVars))
+
+	require.ElementsMatch(t, flagEnvVars, uniqueFlagEnvVars, "not all flag env vars are unique")
+}
+
+func TestGetRequestedEnvValues(t *testing.T) {
+	tests := []struct {
+		desc           string
+		c              *config
+		expectedValues map[string]string
+	}{
+		{
+			desc: "specific values",
+			c: &config{
+				EnvironmentsDirectory: filepath.Join("..", "pkg", "testdata", "repos", "basic repo", ".environments"),
+				Environment:           "env1",
+				ValueSets: []string{
+					"testing1",
+				},
+				Values: []string{
+					"setLevel",
+					"envLevelCommon1",
+				},
+			},
+			expectedValues: map[string]string{
+				"setLevel":        "set level",
+				"envLevelCommon1": "env level",
+			},
+		},
+		{
+			desc: "full value set",
+			c: &config{
+				EnvironmentsDirectory: filepath.Join("..", "pkg", "testdata", "repos", "basic repo", ".environments"),
+				Environment:           "env1",
+				ValueSets: []string{
+					"testing1",
+				},
+			},
+			expectedValues: map[string]string{
+				"setLevel":        "set level",
+				"setLevelCommon":  "testing1 level",
+				"envLevelCommon1": "env level",
+				"envLevelCommon2": "set level",
+				"topLevelCommon1": "top level",
+				"topLevelCommon2": "env level",
+			},
+		},
+		{
+			desc: "specific env",
+			c: &config{
+				EnvironmentsDirectory: filepath.Join("..", "pkg", "testdata", "repos", "basic repo", ".environments"),
+				Environment:           "env1",
+			},
+			expectedValues: map[string]string{
+				"envLevelCommon1": "env level",
+				"envLevelCommon2": "env level",
+				"topLevelCommon1": "top level",
+				"topLevelCommon2": "env level",
+			},
+		},
+	}
+
+	for _, test := range tests {
+		actualValues, err := getRequestedEnvValues(test.c)
+		require.NoError(t, err)
+		require.EqualValues(t, test.expectedValues, actualValues)
+	}
+}
+
+func TestRun(t *testing.T) {
+	tests := []struct {
+		desc           string
+		c              *config
+		expectedOutput string
+	}{
+		{
+			desc: "specific values",
+			c: &config{
+				EnvironmentsDirectory: filepath.Join("..", "pkg", "testdata", "repos", "basic repo", ".environments"),
+				Environment:           "env1",
+				ValueSets: []string{
+					"testing1",
+				},
+				Values: []string{
+					"setLevel",
+					"envLevelCommon1",
+				},
+				Writer: "dotenv",
+			},
+			expectedOutput: "envLevelCommon1=env level\nsetLevel=set level\n",
+		},
+	}
+
+	for _, test := range tests {
+		// Setup to capture stdout
+		var outputBytes bytes.Buffer
+		outputWriter = &outputBytes
+
+		err := run(test.c)
+
+		output := outputBytes.String()
+
+		require.NoError(t, err)
+		require.Equal(t, test.expectedOutput, output)
+	}
+}

tools/env-loader/pkg/envloader.go

@@ -38,6 +38,8 @@ const EnvironmentNameDirectorySeparator = "/"
 // value files.
 const CommonFileGlob = "common.*"
 
+const gitFakeLinkFileIdentifier = "gitdir: "
+
 func findGitRepoRoot() (string, error) {
 	cwd, err := os.Getwd()
 	if err != nil {
@@ -47,20 +49,37 @@ func findGitRepoRoot() (string, error) {
 	// Walk upwards until a '.git' directory is found, or root is reached
 	path := filepath.Clean(cwd)
 	for {
-		fileInfo, err := os.Lstat(filepath.Join(path, ".git"))
+		gitFsObjectPath := filepath.Join(path, ".git")
+		fileInfo, err := os.Lstat(gitFsObjectPath)
 		// If failed to stat the fs object and it exists
 		if err != nil && !os.IsNotExist(err) {
-			return "", trace.Wrap(err, "failed to read file information for %q", path)
+			return "", trace.Wrap(err, "failed to read file information for %q", gitFsObjectPath)
 		}
 
 		// If the .git fs object was found and it is a directory
-		if err == nil && fileInfo.IsDir() {
-			absPath, err := filepath.Abs(path)
-			if err != nil {
-				return "", trace.Wrap(err, "failed to get absolute path for git repo at %q", path)
+		if err == nil {
+			isCurrentPathAGitDirectory := fileInfo.IsDir()
+
+			// Perform some rudimentary checking to see if the .git directory
+			// exists elsewhere, as is the case with submodules:
+			// https://git-scm.com/docs/git-init#Documentation/git-init.txt-code--separate-git-dircodeemltgit-dirgtem
+			if fileInfo.Mode().IsRegular() {
+				fileContents, err := os.ReadFile(gitFsObjectPath)
+				if err != nil {
+					return "", trace.Wrap(err, "failed to read .git file at %q", gitFsObjectPath)
+				}
+
+				isCurrentPathAGitDirectory = strings.HasPrefix(string(fileContents), gitFakeLinkFileIdentifier)
 			}
 
-			return absPath, nil
+			if isCurrentPathAGitDirectory {
+				absPath, err := filepath.Abs(path)
+				if err != nil {
+					return "", trace.Wrap(err, "failed to get absolute path for git repo at %q", path)
+				}
+
+				return absPath, nil
+			}
 		}
 
 		// If the .git fs object was found and is not a directory, or it wasn't
@@ -90,8 +109,7 @@ func findCommonFilesInPath(basePath, relativeSubdirectoryPath string) ([]string,
 	var commonFilePaths []string
 	currentDirectoryPath := basePath
 	for _, directoryNameToCheck := range subdirectoryNames {
-		currentDirectoryPath := filepath.Join(currentDirectoryPath, directoryNameToCheck)
-
+		currentDirectoryPath = filepath.Join(currentDirectoryPath, directoryNameToCheck)
 		fileInfo, err := os.Lstat(currentDirectoryPath)
 		if err != nil {
 			return nil, trace.Wrap(err, "failed to lstat %q", currentDirectoryPath)

tools/env-loader/pkg/envloader_test.go

@@ -45,6 +45,12 @@ func ensureGitFSObjectsExist(t *testing.T) {
 		return os.WriteFile(gitPath, nil, 0400)
 	})
 
+	ensureGitFSObjectExist(t, "git repo with submodule", "directory", createGitFile)
+	ensureGitFSObjectExist(t, filepath.Join("git repo with submodule", "submodule"), "file", func(gitPath string) error {
+		// This path doesn't (currently) actually need to be created
+		return os.WriteFile(gitPath, []byte("gitdir: ../.git/modules/submodule\n"), 0400)
+	})
+
 	ensureGitFSObjectExist(t, "nested repos", "directory", createGitFile)
 	ensureGitFSObjectExist(t, filepath.Join("nested repos", "subdirectory"), "directory", createGitFile)
 }
@@ -101,6 +107,11 @@ func TestFindGitRepoRoot(t *testing.T) {
 			workingDirectory: filepath.Join("nested repos", "subdirectory"),
 			expectedRoot:     filepath.Join("nested repos", "subdirectory"),
 		},
+		{
+			desc:             "from submodule",
+			workingDirectory: filepath.Join("git repo with submodule", "submodule"),
+			expectedRoot:     filepath.Join("git repo with submodule", "submodule"),
+		},
 	}
 
 	reposDirectory := getTestDataDir(t, "repos")