From 1cd1b5568284119a34b3e77196c5c14d0aed31aa Mon Sep 17 00:00:00 2001 From: MagnumOpus21 Date: Thu, 5 Jul 2018 22:04:29 -0400 Subject: [PATCH 1/5] Agent/Proxy : Properly passes env variables to child --- agent/proxy/manager.go | 2 +- agent/proxy/manager_test.go | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/agent/proxy/manager.go b/agent/proxy/manager.go index e3c6dd7796..e22079b186 100644 --- a/agent/proxy/manager.go +++ b/agent/proxy/manager.go @@ -432,7 +432,7 @@ func (m *Manager) newProxy(mp *local.ManagedProxy) (Proxy, error) { if err := m.configureLogDir(id, &cmd); err != nil { return nil, fmt.Errorf("error configuring proxy logs: %s", err) } - + cmd.Env = os.Environ() // Build the daemon structure proxy.Command = &cmd proxy.ProxyID = id diff --git a/agent/proxy/manager_test.go b/agent/proxy/manager_test.go index 42286493c7..b3269bf10c 100644 --- a/agent/proxy/manager_test.go +++ b/agent/proxy/manager_test.go @@ -261,6 +261,46 @@ func TestManagerRun_daemonPid(t *testing.T) { require.NotEmpty(pidRaw) } +// Test to check if the parent and the child processes +// have the same environmental variables + +func TestEnvironproxy(t *testing.T) { + t.Parallel() + + require := require.New(t) + state := local.TestState(t) + m, closer := testManager(t) + defer closer() + m.State = state + defer m.Kill() + + // Add the proxy + td, closer := testTempDir(t) + defer closer() + path := filepath.Join(td, "env-variables") + d := &Daemon{ + Command: helperProcess("parent", path), + Logger: testLogger, + } + var currentEnviron []string + currentEnviron = os.Environ() + // Environmental variable of the helper + if err := d.Start(); err != nil { + t.Error("Daemon failed to start") + defer d.Stop() + envProcess := d.Command.Env + var envData []byte + for _, data := range envProcess { + envData = append(envData, []byte(data)...) + envData = append(envData, []byte("\n")...) + } + t.Logf("Env Data:%s", envProcess) + //Get the parent environmental variables in a file + + require.Equal(currentEnviron, envProcess) + } +} + // Test the Snapshot/Restore works. func TestManagerRun_snapshotRestore(t *testing.T) { t.Parallel() From 6cecf2961d4b9dc5dbdc23e5f9d4e13750259ab9 Mon Sep 17 00:00:00 2001 From: MagnumOpus21 Date: Thu, 5 Jul 2018 22:04:29 -0400 Subject: [PATCH 2/5] Agent/Proxy : Properly passes env variables to child --- agent/proxy/manager.go | 2 +- agent/proxy/manager_test.go | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/agent/proxy/manager.go b/agent/proxy/manager.go index e3c6dd7796..e22079b186 100644 --- a/agent/proxy/manager.go +++ b/agent/proxy/manager.go @@ -432,7 +432,7 @@ func (m *Manager) newProxy(mp *local.ManagedProxy) (Proxy, error) { if err := m.configureLogDir(id, &cmd); err != nil { return nil, fmt.Errorf("error configuring proxy logs: %s", err) } - + cmd.Env = os.Environ() // Build the daemon structure proxy.Command = &cmd proxy.ProxyID = id diff --git a/agent/proxy/manager_test.go b/agent/proxy/manager_test.go index 42286493c7..b3269bf10c 100644 --- a/agent/proxy/manager_test.go +++ b/agent/proxy/manager_test.go @@ -261,6 +261,46 @@ func TestManagerRun_daemonPid(t *testing.T) { require.NotEmpty(pidRaw) } +// Test to check if the parent and the child processes +// have the same environmental variables + +func TestEnvironproxy(t *testing.T) { + t.Parallel() + + require := require.New(t) + state := local.TestState(t) + m, closer := testManager(t) + defer closer() + m.State = state + defer m.Kill() + + // Add the proxy + td, closer := testTempDir(t) + defer closer() + path := filepath.Join(td, "env-variables") + d := &Daemon{ + Command: helperProcess("parent", path), + Logger: testLogger, + } + var currentEnviron []string + currentEnviron = os.Environ() + // Environmental variable of the helper + if err := d.Start(); err != nil { + t.Error("Daemon failed to start") + defer d.Stop() + envProcess := d.Command.Env + var envData []byte + for _, data := range envProcess { + envData = append(envData, []byte(data)...) + envData = append(envData, []byte("\n")...) + } + t.Logf("Env Data:%s", envProcess) + //Get the parent environmental variables in a file + + require.Equal(currentEnviron, envProcess) + } +} + // Test the Snapshot/Restore works. func TestManagerRun_snapshotRestore(t *testing.T) { t.Parallel() From 94e8ff55cfea24a7a78cd6e67037a10478ffe12f Mon Sep 17 00:00:00 2001 From: MagnumOpus21 Date: Mon, 9 Jul 2018 12:27:34 -0400 Subject: [PATCH 3/5] Proxy/Tests: Added test cases to check env variables --- agent/proxy/manager_test.go | 49 ++++++++++++++++++++++--------------- agent/proxy/proxy_test.go | 45 +++++++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 21 deletions(-) diff --git a/agent/proxy/manager_test.go b/agent/proxy/manager_test.go index b3269bf10c..43a0c0f24a 100644 --- a/agent/proxy/manager_test.go +++ b/agent/proxy/manager_test.go @@ -5,6 +5,8 @@ import ( "os" "os/exec" "path/filepath" + "reflect" + "sort" "testing" "time" @@ -274,31 +276,38 @@ func TestEnvironproxy(t *testing.T) { m.State = state defer m.Kill() - // Add the proxy + // Add Proxy for the test td, closer := testTempDir(t) defer closer() path := filepath.Join(td, "env-variables") - d := &Daemon{ - Command: helperProcess("parent", path), - Logger: testLogger, - } - var currentEnviron []string - currentEnviron = os.Environ() - // Environmental variable of the helper - if err := d.Start(); err != nil { - t.Error("Daemon failed to start") - defer d.Stop() - envProcess := d.Command.Env - var envData []byte - for _, data := range envProcess { - envData = append(envData, []byte(data)...) - envData = append(envData, []byte("\n")...) - } - t.Logf("Env Data:%s", envProcess) - //Get the parent environmental variables in a file + testStateProxy(t, state, "environTest", helperProcess("environ", path)) - require.Equal(currentEnviron, envProcess) + //Run the manager + go m.Run() + + //Get the environmental variables from the OS + //envCheck := os.Environ() + var fileContent Bytes + var err error + var data Bytes + envData := os.Environ() + sort.Strings(envData) + for _, envVariable := range envData { + data = append(data, envVariable...) + data = append(data, "\n"...) } + // t.Log(data) + retry.Run(t, func(r *retry.R) { + if fileContent, err = ioutil.ReadFile(path); err != nil { + r.Fatalf("No file ya dummy") + } + }) + + t.Logf("Len (data) : %d , Len (fileContent) : %d", len(data), len(fileContent)) + t.Logf("Type (data) : %s Type (fileContent) : %s", reflect.TypeOf(data), reflect.TypeOf(fileContent)) + // t.Logf("File content: \n %s", fileContent) + // t.Logf("Data content: \n %s", data) + require.Equal(fileContent, data) } // Test the Snapshot/Restore works. diff --git a/agent/proxy/proxy_test.go b/agent/proxy/proxy_test.go index 0a92373192..4394433fe7 100644 --- a/agent/proxy/proxy_test.go +++ b/agent/proxy/proxy_test.go @@ -7,7 +7,9 @@ import ( "os" "os/exec" "os/signal" + "sort" "strconv" + "strings" "syscall" "testing" "time" @@ -17,6 +19,22 @@ import ( // *log.Logger instance. var testLogger = log.New(os.Stderr, "logger: ", log.LstdFlags) +//SOrting interface +type Bytes []byte + +//Length method for our Byte interface +func (b Bytes) Len() int { + return len(b) +} + +func (b Bytes) Swap(i, j int) { + b[i], b[j] = b[j], b[i] +} + +func (b Bytes) Less(i, j int) bool { + return b[i] < b[j] +} + // testTempDir returns a temporary directory and a cleanup function. func testTempDir(t *testing.T) (string, func()) { t.Helper() @@ -124,7 +142,6 @@ func TestHelperProcess(t *testing.T) { default: } } - case "stop-kill": // Setup listeners so it is ignored ch := make(chan os.Signal, 1) @@ -139,6 +156,32 @@ func TestHelperProcess(t *testing.T) { } time.Sleep(25 * time.Millisecond) } + // Check if the external process can access the enivironmental variables + case "environ": + stop := make(chan os.Signal, 1) + signal.Notify(stop, os.Interrupt) + defer signal.Stop(stop) + + //Write the environmental variables into the file + path := args[0] + var data Bytes + // sort.Sort(data) + envData := os.Environ() + sort.Strings(envData) + for _, envVariable := range envData { + if strings.Contains(envVariable, "CONNECT_PROXY") { + continue + } + data = append(data, envVariable...) + data = append(data, "\n"...) + } + if err := ioutil.WriteFile(path, data, 0644); err != nil { + t.Fatalf("[Error] File write failed : %s", err) + } + + // Clean up after we receive the signal to exit + defer os.Remove(path) + <-stop case "output": fmt.Fprintf(os.Stdout, "hello stdout\n") From 300330e24b72f51105fcf0854e2e996d0a69baad Mon Sep 17 00:00:00 2001 From: MagnumOpus21 Date: Mon, 9 Jul 2018 12:46:10 -0400 Subject: [PATCH 4/5] Agent/Proxy: Formatting and test cases fix --- agent/consul/server.go | 2 +- agent/proxy/manager.go | 3 +++ agent/proxy/manager_test.go | 14 +++++--------- agent/proxy/proxy_test.go | 28 ++++++++-------------------- 4 files changed, 17 insertions(+), 30 deletions(-) diff --git a/agent/consul/server.go b/agent/consul/server.go index 4520b11114..e153f9b28d 100644 --- a/agent/consul/server.go +++ b/agent/consul/server.go @@ -427,7 +427,7 @@ func NewServerLogger(config *Config, logger *log.Logger, tokens *token.Store) (* } go s.Flood(nil, portFn, s.serfWAN) } - + // Start enterprise specific functionality if err := s.startEnterprise(); err != nil { s.Shutdown() diff --git a/agent/proxy/manager.go b/agent/proxy/manager.go index e22079b186..724ed39b4d 100644 --- a/agent/proxy/manager.go +++ b/agent/proxy/manager.go @@ -432,7 +432,10 @@ func (m *Manager) newProxy(mp *local.ManagedProxy) (Proxy, error) { if err := m.configureLogDir(id, &cmd); err != nil { return nil, fmt.Errorf("error configuring proxy logs: %s", err) } + + // Pass in the environmental variables for this proxy process cmd.Env = os.Environ() + // Build the daemon structure proxy.Command = &cmd proxy.ProxyID = id diff --git a/agent/proxy/manager_test.go b/agent/proxy/manager_test.go index 43a0c0f24a..10234e90d2 100644 --- a/agent/proxy/manager_test.go +++ b/agent/proxy/manager_test.go @@ -5,7 +5,6 @@ import ( "os" "os/exec" "path/filepath" - "reflect" "sort" "testing" "time" @@ -286,27 +285,24 @@ func TestEnvironproxy(t *testing.T) { go m.Run() //Get the environmental variables from the OS - //envCheck := os.Environ() - var fileContent Bytes + var fileContent []byte var err error - var data Bytes + var data []byte envData := os.Environ() sort.Strings(envData) for _, envVariable := range envData { data = append(data, envVariable...) data = append(data, "\n"...) } - // t.Log(data) + + // Check if the file written to from the spawned process + // has the necessary environmental variable data retry.Run(t, func(r *retry.R) { if fileContent, err = ioutil.ReadFile(path); err != nil { r.Fatalf("No file ya dummy") } }) - t.Logf("Len (data) : %d , Len (fileContent) : %d", len(data), len(fileContent)) - t.Logf("Type (data) : %s Type (fileContent) : %s", reflect.TypeOf(data), reflect.TypeOf(fileContent)) - // t.Logf("File content: \n %s", fileContent) - // t.Logf("Data content: \n %s", data) require.Equal(fileContent, data) } diff --git a/agent/proxy/proxy_test.go b/agent/proxy/proxy_test.go index 4394433fe7..0ac0446a8f 100644 --- a/agent/proxy/proxy_test.go +++ b/agent/proxy/proxy_test.go @@ -19,22 +19,6 @@ import ( // *log.Logger instance. var testLogger = log.New(os.Stderr, "logger: ", log.LstdFlags) -//SOrting interface -type Bytes []byte - -//Length method for our Byte interface -func (b Bytes) Len() int { - return len(b) -} - -func (b Bytes) Swap(i, j int) { - b[i], b[j] = b[j], b[i] -} - -func (b Bytes) Less(i, j int) bool { - return b[i] < b[j] -} - // testTempDir returns a temporary directory and a cleanup function. func testTempDir(t *testing.T) (string, func()) { t.Helper() @@ -162,14 +146,17 @@ func TestHelperProcess(t *testing.T) { signal.Notify(stop, os.Interrupt) defer signal.Stop(stop) - //Write the environmental variables into the file + //Get the path for the file to be written to path := args[0] - var data Bytes - // sort.Sort(data) + var data []byte + + //Get the environmental variables envData := os.Environ() + + //Sort the env data for easier comparison sort.Strings(envData) for _, envVariable := range envData { - if strings.Contains(envVariable, "CONNECT_PROXY") { + if strings.HasPrefix(envVariable, "CONSUL") || strings.HasPrefix(envVariable, "CONNECT") { continue } data = append(data, envVariable...) @@ -181,6 +168,7 @@ func TestHelperProcess(t *testing.T) { // Clean up after we receive the signal to exit defer os.Remove(path) + <-stop case "output": From 371f0c3d5f792287e9b36972f7ea6beda7317fc3 Mon Sep 17 00:00:00 2001 From: MagnumOpus21 Date: Mon, 9 Jul 2018 13:18:57 -0400 Subject: [PATCH 5/5] Tests/Proxy : Changed function name to match the system being tested. --- agent/proxy/manager_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/proxy/manager_test.go b/agent/proxy/manager_test.go index 10234e90d2..d9e63b6c63 100644 --- a/agent/proxy/manager_test.go +++ b/agent/proxy/manager_test.go @@ -265,7 +265,7 @@ func TestManagerRun_daemonPid(t *testing.T) { // Test to check if the parent and the child processes // have the same environmental variables -func TestEnvironproxy(t *testing.T) { +func TestManagerPassesEnvironment(t *testing.T) { t.Parallel() require := require.New(t)