From 58dcbb2bbe598b5d887d591877d2344d9fab2585 Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Thu, 28 Mar 2024 23:45:19 +0100 Subject: [PATCH 1/2] expand environment variables for services --- internals/overlord/servstate/handlers.go | 75 +++++++++++++++++------- 1 file changed, 55 insertions(+), 20 deletions(-) diff --git a/internals/overlord/servstate/handlers.go b/internals/overlord/servstate/handlers.go index 2bcdd6945..1ff5cfbf9 100644 --- a/internals/overlord/servstate/handlers.go +++ b/internals/overlord/servstate/handlers.go @@ -8,6 +8,7 @@ import ( "os/exec" "os/user" "strconv" + "strings" "syscall" "time" @@ -350,22 +351,12 @@ func logError(err error) { // command. It assumes the caller has ensures the service is in a valid state, // and it sets s.cmd and other relevant fields. func (s *serviceData) startInternal() error { - base, extra, err := s.config.ParseCommand() - if err != nil { - return err - } - args := append(base, extra...) - s.cmd = exec.Command(args[0], args[1:]...) - s.cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - // Copy environment to avoid updating original. - environment := make(map[string]string) + serviceEnvironment := make(map[string]string) for k, v := range s.config.Environment { - environment[k] = v + serviceEnvironment[k] = v } - s.cmd.Dir = s.config.WorkingDir - // Start as another user if specified in plan. uid, gid, err := osutil.NormalizeUidGid(s.config.UserID, s.config.GroupID, s.config.User, s.config.Group) if err != nil { @@ -384,26 +375,70 @@ func (s *serviceData) startInternal() error { } // Also set HOME and USER if not explicitly specified in config. - if environment["HOME"] == "" || environment["USER"] == "" { + if serviceEnvironment["HOME"] == "" || serviceEnvironment["USER"] == "" { u, err := user.LookupId(strconv.Itoa(*uid)) if err != nil { logger.Noticef("Cannot look up user %d: %v", *uid, err) } else { - if environment["HOME"] == "" { - environment["HOME"] = u.HomeDir + if serviceEnvironment["HOME"] == "" { + serviceEnvironment["HOME"] = u.HomeDir } - if environment["USER"] == "" { - environment["USER"] = u.Username + if serviceEnvironment["USER"] == "" { + serviceEnvironment["USER"] = u.Username } } } } - // Pass service description's environment variables to child process. - s.cmd.Env = os.Environ() + // Get system environment variables for command expansion + getEnvironment := func(data []string, getkeyval func(item string) (key, val string)) map[string]string { + items := make(map[string]string) + for _, item := range data { + key, val := getkeyval(item) + items[key] = val + } + return items + } + environment := getEnvironment(os.Environ(), func(item string) (key, val string) { + splits := strings.Split(item, "=") + key = splits[0] + val = splits[1] + return + }) + + // Merge service description's environment variables to system environment variables. + for k, v := range serviceEnvironment { + environment[k] = v + } + + // Assemble the overall environment variables. + var env []string for k, v := range environment { - s.cmd.Env = append(s.cmd.Env, k+"="+v) + env = append(env, k+"="+v) + } + + // Parse and obtain command tokens + base, extra, err := s.config.ParseCommand() + if err != nil { + return err } + args := append(base, extra...) + + // Replace environment variables in the command with its actual value + for i, v := range args { + if strings.HasPrefix(v, "$") { + s := strings.TrimLeft(v, "$") + s = strings.TrimLeft(s, "{") + s = strings.TrimRight(s, "}") + args[i] = environment[s] + } + } + + // Execute command with expanded environment variables + s.cmd = exec.Command(args[0], args[1:]...) + s.cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + s.cmd.Dir = s.config.WorkingDir + s.cmd.Env = env // Set up stdout and stderr to write to log ring buffer. var outputIterator servicelog.Iterator From faef20298b51f2cc38ea64aeb495268d941ee95d Mon Sep 17 00:00:00 2001 From: Zhijie Yang Date: Fri, 29 Mar 2024 09:29:09 +0100 Subject: [PATCH 2/2] fix setting command credentials before exec --- internals/overlord/servstate/handlers.go | 39 +++++++++++++----------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/internals/overlord/servstate/handlers.go b/internals/overlord/servstate/handlers.go index 1ff5cfbf9..2704ae7fa 100644 --- a/internals/overlord/servstate/handlers.go +++ b/internals/overlord/servstate/handlers.go @@ -357,24 +357,15 @@ func (s *serviceData) startInternal() error { serviceEnvironment[k] = v } - // Start as another user if specified in plan. + // Get uid and gid to set user/group later. uid, gid, err := osutil.NormalizeUidGid(s.config.UserID, s.config.GroupID, s.config.User, s.config.Group) if err != nil { return err } - if uid != nil && gid != nil { - isCurrent, err := osutil.IsCurrent(*uid, *gid) - if err != nil { - logger.Debugf("Cannot determine if uid %d gid %d is current user", *uid, *gid) - } - if !isCurrent { - setCmdCredential(s.cmd, &syscall.Credential{ - Uid: uint32(*uid), - Gid: uint32(*gid), - }) - } + idsNotNil := (uid != nil && gid != nil) - // Also set HOME and USER if not explicitly specified in config. + // Also set HOME and USER if not explicitly specified in config. + if idsNotNil { if serviceEnvironment["HOME"] == "" || serviceEnvironment["USER"] == "" { u, err := user.LookupId(strconv.Itoa(*uid)) if err != nil { @@ -390,7 +381,7 @@ func (s *serviceData) startInternal() error { } } - // Get system environment variables for command expansion + // Get system environment variables for command expansion. getEnvironment := func(data []string, getkeyval func(item string) (key, val string)) map[string]string { items := make(map[string]string) for _, item := range data { @@ -417,14 +408,14 @@ func (s *serviceData) startInternal() error { env = append(env, k+"="+v) } - // Parse and obtain command tokens + // Parse and obtain command tokens. base, extra, err := s.config.ParseCommand() if err != nil { return err } args := append(base, extra...) - // Replace environment variables in the command with its actual value + // Expand environment variables in the command with its actual value. for i, v := range args { if strings.HasPrefix(v, "$") { s := strings.TrimLeft(v, "$") @@ -434,12 +425,26 @@ func (s *serviceData) startInternal() error { } } - // Execute command with expanded environment variables + // Execute command with expanded environment variables. s.cmd = exec.Command(args[0], args[1:]...) s.cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} s.cmd.Dir = s.config.WorkingDir s.cmd.Env = env + // Start as another user if specified in plan. + if idsNotNil { + isCurrent, err := osutil.IsCurrent(*uid, *gid) + if err != nil { + logger.Debugf("Cannot determine if uid %d gid %d is current user", *uid, *gid) + } + if !isCurrent { + setCmdCredential(s.cmd, &syscall.Credential{ + Uid: uint32(*uid), + Gid: uint32(*gid), + }) + } + } + // Set up stdout and stderr to write to log ring buffer. var outputIterator servicelog.Iterator if s.manager.serviceOutput != nil {