fix: pebble exec and "exec" health checks inherit environment from daemon

internals/daemon/api_exec_test.go

@@ -108,6 +108,27 @@ func (s *execSuite) TestEnvironment(c *C) {
 	c.Check(stderr, Equals, "")
 }
 
+func (s *execSuite) TestEnvironmentInheritedFromDaemon(c *C) {
+	restore := fakeEnv("FOO", "bar")
+	defer restore()
+
+	stdout, stderr, waitErr := s.exec(c, "", &client.ExecOptions{
+		Command: []string{"/bin/sh", "-c", "echo FOO=$FOO"},
+	})
+	c.Check(waitErr, IsNil)
+	c.Check(stdout, Equals, "FOO=bar\n")
+	c.Check(stderr, Equals, "")
+
+	// Check that requested environment takes precedence.
+	stdout, stderr, waitErr = s.exec(c, "", &client.ExecOptions{
+		Command:     []string{"/bin/sh", "-c", "echo FOO=$FOO"},
+		Environment: map[string]string{"FOO": "foo"},
+	})
+	c.Check(waitErr, IsNil)
+	c.Check(stdout, Equals, "FOO=foo\n")
+	c.Check(stderr, Equals, "")
+}
+
 func (s *execSuite) TestWorkingDir(c *C) {
 	workingDir := c.MkDir()
 	stdout, stderr, waitErr := s.exec(c, "", &client.ExecOptions{

internals/daemon/api_test.go

@@ -18,6 +18,7 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"os"
 
 	"gopkg.in/check.v1"
 
@@ -101,3 +102,22 @@ func (s *apiSuite) TestSysInfo(c *check.C) {
 	c.Check(rsp.Type, check.Equals, ResponseTypeSync)
 	c.Check(rsp.Result, check.DeepEquals, expected)
 }
+
+func fakeEnv(key, value string) (restore func()) {
+	oldEnv, envWasSet := os.LookupEnv(key)
+	err := os.Setenv(key, value)
+	if err != nil {
+		panic(err)
+	}
+	return func() {
+		var err error
+		if envWasSet {
+			err = os.Setenv(key, oldEnv)
+		} else {
+			err = os.Unsetenv(key)
+		}
+		if err != nil {
+			panic(err)
+		}
+	}
+}

internals/osutil/env.go

@@ -0,0 +1,38 @@
+// Copyright (c) 2014-2023 Canonical Ltd
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License version 3 as
+// published by the Free Software Foundation.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package osutil
+
+import (
+	"os"
+	"strings"
+)
+
+var osEnviron = os.Environ
+
+// Environ returns a map representing the environment.
+// It converts the slice from os.Environ() into a map.
+func Environ() map[string]string {
+	env := make(map[string]string)
+	for _, kv := range osEnviron() {
+		parts := strings.SplitN(kv, "=", 2)
+		key := parts[0]
+		val := ""
+		if len(parts) == 2 {
+			val = parts[1]
+		}
+		env[key] = val
+	}
+	return env
+}

internals/osutil/env_test.go

@@ -0,0 +1,40 @@
+// Copyright (c) 2014-2023 Canonical Ltd
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License version 3 as
+// published by the Free Software Foundation.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package osutil_test
+
+import (
+	"github.com/canonical/pebble/internals/osutil"
+	. "gopkg.in/check.v1"
+)
+
+type envSuite struct{}
+
+var _ = Suite(&envSuite{})
+
+func (s *envSuite) TestEnviron(c *C) {
+	restore := osutil.FakeEnviron(func() []string {
+		return []string{"FOO=bar", "BAR=", "TEMP"}
+	})
+	defer restore()
+
+	env := osutil.Environ()
+
+	c.Assert(len(env), Equals, 3)
+	c.Assert(env, DeepEquals, map[string]string{
+		"FOO":  "bar",
+		"BAR":  "",
+		"TEMP": "",
+	})
+}

internals/osutil/export_test.go

@@ -111,3 +111,11 @@ func FakeSyscallGetpgid(f func(int) (int, error)) (restore func()) {
 		syscallGetpgid = oldSyscallGetpgid
 	}
 }
+
+func FakeEnviron(f func() []string) (restore func()) {
+	oldEnviron := osEnviron
+	osEnviron = f
+	return func() {
+		osEnviron = oldEnviron
+	}
+}

internals/overlord/checkstate/checkers.go

@@ -128,9 +128,16 @@ func (c *execChecker) check(ctx context.Context) error {
 		return fmt.Errorf("cannot parse check command: %v", err)
 	}
 
-	cmd := exec.CommandContext(ctx, args[0], args[1:]...)
-	cmd.Env = make([]string, 0, len(c.environment)) // avoid nil to ensure we don't inherit parent env
+	// Similar to services and exec, inherit the daemon's environment.
+	environment := osutil.Environ()
 	for k, v := range c.environment {
+		// Requested environment takes precedence.
+		environment[k] = v
+	}
+
+	cmd := exec.CommandContext(ctx, args[0], args[1:]...)
+	cmd.Env = make([]string, 0, len(environment)) // avoid additional allocations
+	for k, v := range environment {
 		cmd.Env = append(cmd.Env, k+"="+v)
 	}
 	cmd.Dir = c.workingDir

internals/overlord/checkstate/checkers_test.go

@@ -193,7 +193,7 @@ func (s *CheckersSuite) TestExec(c *C) {
 	c.Assert(ok, Equals, true)
 	c.Assert(detailsErr.Details(), Equals, "Foo, meet Bar.")
 
-	// Does not inherit environment when no environment vars set
+	// Inherits environment when no environment vars set
 	os.Setenv("PEBBLE_TEST_CHECKERS_EXEC", "parent")
 	chk = &execChecker{
 		command: "/bin/sh -c 'echo $PEBBLE_TEST_CHECKERS_EXEC; exit 1'",
@@ -202,9 +202,9 @@ func (s *CheckersSuite) TestExec(c *C) {
 	c.Assert(err, ErrorMatches, "exit status 1")
 	detailsErr, ok = err.(*detailsError)
 	c.Assert(ok, Equals, true)
-	c.Assert(detailsErr.Details(), Equals, "")
+	c.Assert(detailsErr.Details(), Equals, "parent")
 
-	// Does not inherit environment when some environment vars set
+	// Inherits environment when some environment vars set
 	os.Setenv("PEBBLE_TEST_CHECKERS_EXEC", "parent")
 	chk = &execChecker{
 		command:     "/bin/sh -c 'echo FOO=$FOO test=$PEBBLE_TEST_CHECKERS_EXEC; exit 1'",
@@ -214,7 +214,7 @@ func (s *CheckersSuite) TestExec(c *C) {
 	c.Assert(err, ErrorMatches, "exit status 1")
 	detailsErr, ok = err.(*detailsError)
 	c.Assert(ok, Equals, true)
-	c.Assert(detailsErr.Details(), Equals, "FOO=foo test=")
+	c.Assert(detailsErr.Details(), Equals, "FOO=foo test=parent")
 
 	// Working directory is passed through
 	workingDir := c.MkDir()

internals/overlord/cmdstate/request.go

@@ -23,6 +23,7 @@ import (
 	"time"
 
 	"github.com/canonical/pebble/internals/logger"
+	"github.com/canonical/pebble/internals/osutil"
 	"github.com/canonical/pebble/internals/overlord/state"
 )
 
@@ -69,8 +70,10 @@ func Exec(st *state.State, args *ExecArgs) (*state.Task, ExecMetadata, error) {
 		return nil, ExecMetadata{}, errors.New("cannot use interactive mode without a terminal")
 	}
 
-	environment := map[string]string{}
+	// Inherit the pebble daemon environment.
+	environment := osutil.Environ()
 	for k, v := range args.Environment {
+		// Requested environment takes precedence.
 		environment[k] = v
 	}