[#1587] Add unit tests

backend/appcfg/stork/cli.go

@@ -37,6 +37,13 @@ type CLIParser struct {
 }
 
 // Constructs CLI parser.
+// Accepts the main parser. It should be configured with the application
+// settings.
+// The application name is used to construct the namespaces for the CLI flags
+// and environment variables. It should be either 'server' or 'agent'.
+// The callback is called when the environment file is loaded. Its purpose is
+// to allow reconfiguring the logging using the new environment variables as
+// soon as they are available.
 func NewCLIParser(parser *flags.Parser, app string, onLoadEnvironmentFileCallback func()) *CLIParser {
 	if app != "server" && app != "agent" {
 		// Programming error.
@@ -312,8 +319,43 @@ func (p *CLIParser) verifySystemEnvironmentVariables() {
 		}
 	}
 
-	// The prefix that is used for the Stork-specific environment variables.
-	applicationPrefix := fmt.Sprintf("STORK_%s_", strings.ToUpper(p.application))
+	// Contains the prefixes of the Stork-specific environment variables.
+	// Stork environment variables starts with the 'STORK' part and then
+	// the context-specific part e.g.:
+	//
+	// - STORK_SERVER (server-specific environment variable)
+	// - STORK_AGENT (agent-specific environment variable)
+	// - STORK_DATABASE (database-specific environment variable)
+	//
+	// This function analyzes the system-wide environment variables. We cannot
+	// assume that there are only environment variables related to one of the
+	// Stork components. It should be allowed to have the agent, server, and
+	// tool environment variables set in the same shell.
+	//
+	// The below code block allows us to ignore the environment variables from
+	// other Stork components. There is an assumption that all components have
+	// exactly the same environment variables for a given prefix/namespace.
+	// For example, if the application utilizes the environment variables
+	// prefixed with 'STORK_DATABASE_' then its settings specify exactly the
+	// same environment variables as the other components.
+	// I hope it is fair enough.
+	var prefixes []string
+
+	for environmentVariable := range knownEnvironmentVariables {
+		parts := strings.SplitN(environmentVariable, "_", 3)
+		if len(parts) < 3 {
+			// The environment variable doesn't have the context-specific part.
+			// The naming convention is violated.
+			continue
+		}
+		if parts[0] != "STORK" {
+			// The environment variable doesn't start with the 'STORK' part.
+			// The naming convention is violated.
+			continue
+		}
+
+		prefixes = append(prefixes, fmt.Sprintf("%s_%s_", parts[0], parts[1]))
+	}
 
 	// Iterate over all system-wide environment variables.
 	for _, env := range os.Environ() {
@@ -323,7 +365,14 @@ func (p *CLIParser) verifySystemEnvironmentVariables() {
 			continue
 		}
 
-		if !strings.HasPrefix(key, applicationPrefix) {
+		var isApplicationEnvironmentVariable bool
+		for _, prefix := range prefixes {
+			if strings.HasPrefix(key, prefix) {
+				isApplicationEnvironmentVariable = true
+				break
+			}
+		}
+		if !isApplicationEnvironmentVariable {
 			// Not a Stork-specific environment variable.
 			continue
 		}

backend/appcfg/stork/cli_test.go

@@ -491,14 +491,19 @@ func TestVerifySystemEnvironmentVariables(t *testing.T) {
 
 	type settings struct {
 		TLSCert string `long:"tls-cert" env:"STORK_SERVER_TLS_CERT" description:"The path to the TLS certificate"`
+		DBHost  string `long:"db-host" env:"STORK_DATABASE_HOST" description:"The host name, IP address or socket where database is available"`
 	}
 	data := &settings{}
 
 	parser := NewCLIParser(flags.NewParser(data, flags.Default), "server", func() {})
 
 	os.Setenv("STORK_SERVER_UNKNOWN", "unknown")
 	os.Setenv("STORK_SERVER_TLS_CERT", "tlsCert")
+	os.Setenv("STORK_DATABASE_HOST", "databaseHost")
+	os.Setenv("STORK_DATABASE_UNKNOWN", "databaseUnknown")
 	os.Setenv("FOOBAR", "foobar")
+	// Broken naming convention.
+	os.Setenv("STORK_UNKNOWN", "unknown")
 
 	// Act
 	stdout, _, captureErr := testutil.CaptureOutput(func() {
@@ -510,6 +515,111 @@ func TestVerifySystemEnvironmentVariables(t *testing.T) {
 
 	expectedLog := `Unknown environment variable: 'STORK_SERVER_UNKNOWN' set in a shell`
 	require.Contains(t, string(stdout), expectedLog)
+	require.Contains(t, string(stdout), "Unknown environment variable: 'STORK_DATABASE_UNKNOWN' set in a shell")
 	require.NotContains(t, string(stdout), "TLS_CERT")
 	require.NotContains(t, string(stdout), "FOOBAR")
+	require.NotContains(t, string(stdout), "DATABASE_HOST")
+	require.NotContains(t, string(stdout), "STORK_UNKNOWN")
+}
+
+// Test that the environment variables from another Stork application set in
+// the shell don't raise a warning.
+func TestVerifySystemEnvironmentVariablesFromAnotherApplication(t *testing.T) {
+	// Arrange
+	restore := testutil.CreateEnvironmentRestorePoint()
+	defer restore()
+
+	type settings struct {
+		TLSCert string `long:"tls-cert" env:"STORK_SERVER_TLS_CERT" description:"The path to the TLS certificate"`
+	}
+	data := &settings{}
+
+	parser := NewCLIParser(flags.NewParser(data, flags.Default), "server", func() {})
+
+	// Act
+	os.Setenv("STORK_AGENT_TLS_CERT", "tlsCert")
+
+	stdout, _, captureErr := testutil.CaptureOutput(func() {
+		parser.verifySystemEnvironmentVariables()
+	})
+
+	// Assert
+	require.NoError(t, captureErr)
+	require.NotContains(t, string(stdout), "TLS_CERT")
+}
+
+// Test that the callback is called when the environment file is loaded.
+func TestOnEnvironmentFileLoadedCallbackIsCalled(t *testing.T) {
+	// Arrange
+	restore := testutil.CreateEnvironmentRestorePoint()
+	defer restore()
+
+	sandbox := testutil.NewSandbox()
+	defer sandbox.Close()
+
+	envPath, _ := sandbox.Write("file.env", `
+		STORK_SERVER_TLS_CERT=tlsCert
+	`)
+
+	isCalled := false
+	callback := func() {
+		isCalled = true
+	}
+
+	parser := NewCLIParser(flags.NewParser(&struct{}{}, flags.Default), "server", callback)
+
+	// Act
+	err := parser.loadEnvironmentFile(&environmentFileSettings{
+		EnvFile:    envPath,
+		UseEnvFile: true,
+	})
+
+	// Assert
+	require.NoError(t, err)
+	require.True(t, isCalled)
+}
+
+// Test the callback is called when the environment file is loaded exactly once.
+func TestOnEnvironmentFileLoadedCallbackIsCalledOnce(t *testing.T) {
+	// Arrange
+	restorePoint := testutil.CreateEnvironmentRestorePoint()
+	defer restorePoint()
+	sandbox := testutil.NewSandbox()
+	defer sandbox.Close()
+
+	envPath, _ := sandbox.Write("file.env", `
+		STORK_DATABASE_HOST=foo
+		STORK_SERVER_HOOK_DIRECTORY=bar
+		STORK_REST_HOST=baz
+	`)
+
+	defer testutil.CreateOsArgsRestorePoint()()
+	os.Args = []string{
+		"program-name",
+		"--use-env-file",
+		"--env-file", envPath,
+	}
+
+	type settings struct {
+		DBHost   string `long:"db-host" description:"The host name, IP address or socket where database is available" env:"STORK_DATABASE_HOST" default:""`
+		RESTHost string `long:"rest-host" description:"The IP to listen on" default:"" env:"STORK_REST_HOST"`
+	}
+
+	data := &settings{}
+
+	flagParser := flags.NewParser(data, flags.Default)
+
+	parser := NewCLIParser(flagParser, "server", func() {})
+
+	// Act
+	hookDirSettings, hookFlags, isHelp, err := parser.Parse()
+
+	// Assert
+	require.NoError(t, err)
+	require.False(t, isHelp)
+	require.Empty(t, hookFlags)
+
+	require.Equal(t, "foo", data.DBHost)
+	require.Equal(t, "bar", hookDirSettings.HookDirectory)
+	require.Equal(t, "baz", data.RESTHost)
 }