entrypoint: be overly pendantic about envVars

To address concerns about potential debug trama, all envVars for command
execution are now put to a []string. This ensures that some random Go
routine won't change a variable.

Signed-off-by: Ben Howard <[email protected]>

Author: Ben Howard

entrypoint/cmd/build.go

@@ -43,6 +43,8 @@ func runOCP(c *cobra.Command, args []string) {
 		log.Info("Jobspec will apply to templated commands.")
 	}
 
+	entryEnvVars = append(entryEnvVars, b.EnvVars...)
+
 	b.Exec(func(v []string) error {
 		return runScripts(c, v)
 	})

entrypoint/cmd/entry.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"text/template"
 
@@ -33,6 +34,9 @@ var (
 	spec     rhjobspec.JobSpec
 	specFile string
 
+	// entryEnvars are set for command execution
+	entryEnvVars []string
+
 	// shellCmd is the default command to execute commands.
 	shellCmd = []string{"/bin/bash", "-x"}
 
@@ -79,6 +83,8 @@ func init() {
 		}
 	}
 
+	entryEnvVars = os.Environ()
+
 	log.SetOutput(os.Stdout)
 	log.SetLevel(log.DebugLevel)
 	newPath := fmt.Sprintf("%s:%s", cosaDir, os.Getenv("PATH"))
@@ -132,7 +138,10 @@ func runScripts(c *cobra.Command, args []string) error {
 	log.Infof("Executing %d script(s)", len(rendered))
 	for i, v := range rendered {
 		log.WithFields(log.Fields{"script": i}).Info("Startig script")
-		rc, err := ee.RunCmds(append(shellCmd, v.Name()))
+		cArgs := append(shellCmd, v.Name())
+		cmd := exec.Command(cArgs[0], cArgs[1:]...)
+		cmd.Env = entryEnvVars
+		rc, err := ee.RunCmds(cmd)
 		if rc != 0 {
 			return fmt.Errorf("Script exited with return code %d", rc)
 		}
@@ -162,7 +171,9 @@ func runSingle(c *cobra.Command, args []string) {
 	}
 
 	log.Infof("Executing commands: %v", args)
-	rc, err := ee.RunCmds(args)
+	cmd := exec.Command(args[0], args[1:]...)
+	cmd.Env = entryEnvVars
+	rc, err := ee.RunCmds(cmd)
 	if rc != 0 || err != nil {
 		log.WithFields(log.Fields{
 			"return code": rc,

entrypoint/exec/run.go

@@ -2,24 +2,22 @@ package exec
 
 import (
 	"context"
-	"os"
+	"errors"
 	"os/exec"
 	"sync"
 )
 
 // RunCmds runs args as a command and ensures that each
-func RunCmds(args []string) (int, error) {
-	if len(args) <= 1 {
-		os.Exit(0)
+func RunCmds(cmd *exec.Cmd) (int, error) {
+	if cmd == nil {
+		return 1, errors.New("No command to execute")
 	}
-
 	ctx, cancel := context.WithCancel(context.Background())
 	var wg sync.WaitGroup
 	wg.Add(1)
 	go RemoveZombies(ctx, &wg)
 
 	var rc int
-	cmd := exec.Command(args[0], args[1:]...)
 	err := Run(cmd)
 	if err != nil {
 		rc = 1

entrypoint/ocp/build_secrets.go

@@ -17,7 +17,7 @@ import (
 
 // copySecret is a hack. OpenShift presents secrets with 0400.
 // Since syscall.SetUid is blocked, we need to use sudo read the file.
-func copySecret(inDir, name, from string) error {
+func copySecret(inDir, name, from string, ret *[]string) error {
 	sDir := filepath.Join(inDir, "secrets", name)
 	if err := os.MkdirAll(sDir, 0755); err != nil {
 		return err
@@ -53,8 +53,7 @@ func copySecret(inDir, name, from string) error {
 			return fmt.Errorf("%s envVar should be in format 'K=V', not %s", e, ev)
 		}
 		if v[1] == baseName {
-			os.Setenv(v[0], to)
-			log.Debugf("Set envVar: %s=%s", v[0], to)
+			*ret = append(*ret, fmt.Sprintf("%s=%s", v[0], to))
 		}
 	}
 
@@ -65,13 +64,14 @@ func copySecret(inDir, name, from string) error {
 // able to read them. Secrets on 3.x clusters are mapped to 0400 and owned by root.
 // To allow the non-privilaged build user access, they need to be copied before
 // /usr/lib/coreos-assembler sudo's to the builder user.
-func buildSecretsSetup(contextDir string) error {
+func buildSecretsSetup(contextDir string) ([]string, error) {
+	var ret []string
 	if apiBuild.Spec.Source.Secrets == nil {
-		return nil
+		return ret, nil
 	}
 	secrets := apiBuild.Spec.Source.Secrets
 	if len(secrets) == 0 {
-		return nil
+		return ret, nil
 	}
 
 	log.Infof("Build has defined %d secrets.", len(secrets))
@@ -83,16 +83,16 @@ func buildSecretsSetup(contextDir string) error {
 		fs, err := filepath.Glob(fmt.Sprintf("%s/%s/*/*", ocpSecretDir, s.Secret.Name))
 		if err != nil {
 			log.Errorf("Failed to get file listing: %v", err)
-			return err
+			return ret, err
 		}
 
 		for _, f := range fs {
 			log.Infof("Processing secret: %s at %s", s.Secret.Name, f)
-			if err := copySecret(contextDir, s.Secret.Name, f); err != nil {
-				return err
+			if err := copySecret(contextDir, s.Secret.Name, f, &ret); err != nil {
+				return ret, err
 			}
 		}
 	}
 
-	return nil
+	return ret, nil
 }

entrypoint/ocp/builder.go

@@ -41,6 +41,9 @@ type Builder struct {
 	JobSpecFile   string `envVar:"COSA_JOBSPEC_FILE"`
 	CosaCmds      string `envVar:"COSA_CMDS"`
 
+	// EnvVars is a listing of specific envVars to set
+	EnvVars []string
+
 	// Internal copy of the JobSpec
 	JobSpec *spec.JobSpec
 }
@@ -73,8 +76,13 @@ func NewBuilder() (*Builder, error) {
 	if buildAPIErr != nil && buildAPIErr != ErrNoOCPBuildSpec {
 		log.Errorf("Failed to initalized the OpenShift Build API Client: %v", buildAPIErr)
 		return nil, buildAPIErr
+	} else {
+		v.EnvVars = append(
+			os.Environ(),
+			"COSA_SKIP_OVERLAY=skip",
+			"FORCE_UNPRIVILEGED=1",
+		)
 	}
-
 	// Builder requires either a Build API Client or Kuberneres Cluster client.
 	if k8sAPIErr != nil && buildAPIErr != nil {
 		return nil, ErrInvalidOCPMode
@@ -95,15 +103,27 @@ func (o *Builder) PrepareEnv() error {
 
 	// Load secrets directly from the Kubernetes API that are
 	// are "well-known" secrets.
-	if err := kubernetesSecretsSetup(cosaSrvDir); err != nil {
+	ks, err := kubernetesSecretsSetup(cosaSrvDir)
+	if err != nil {
 		log.Errorf("Failed to setup Service Account Secrets: %v", err)
+	} else {
+		log.Infof("Mapped %d secrets from Kubernetes", len(ks))
 	}
 
 	// Read setup the secrets locally.
-	if err := buildSecretsSetup(cosaSrvDir); err != nil {
+	bs, err := buildSecretsSetup(cosaSrvDir)
+	if err != nil {
 		log.Errorf("Failed to setup OCP Build Secrets: %v", err)
+	} else {
+		log.Infof("Mapped %d secrets from Kubernetes", len(bs))
 	}
 
+	preCount := len(o.EnvVars)
+	o.EnvVars = append(o.EnvVars, bs...)
+	o.EnvVars = append(o.EnvVars, ks...)
+	addedCount := len(o.EnvVars) - preCount
+	log.Infof("Added %d secret envVar mappings", addedCount)
+
 	// Extract any binary sources first. If a binary payload is delivered,
 	// then blindly execute any script ending in .cosa.sh
 	bin, err := extractInputBinary(cosaSrvDir)
@@ -121,7 +141,7 @@ func (o *Builder) PrepareEnv() error {
 	// If there is no binary payload, then init COSA
 	// With OCP its either binary _or_ source.
 	if !bin {
-		if err := cosaInit(); err != ErrNoSourceInput {
+		if err := cosaInit(o.EnvVars); err != ErrNoSourceInput {
 			return err
 		}
 		log.Info("No source input, relying solely on envVars...this won't end well.")

entrypoint/ocp/cosa_init.go

@@ -3,6 +3,7 @@ package ocp
 import (
 	"errors"
 	"os"
+	"os/exec"
 	"strings"
 
 	ee "github.com/coreos/entrypoint/exec"
@@ -13,7 +14,7 @@ import (
 // based builds first, check the API client, then check envVars. The use of envVars
 // in this case is *safe*; `SOURCE_{URI,REF} == apiBuild.Spec.Source.Git.{URI,REF}`. That
 // is, SOURCE_* envVars will always match the apiBuild.Spec.Source.Git.* values.
-func cosaInit() error {
+func cosaInit(envVars []string) error {
 	var gitURI, gitRef string
 	if apiBuild.Spec.Source.Git != nil {
 		gitURI = apiBuild.Spec.Source.Git.URI
@@ -27,16 +28,18 @@ func cosaInit() error {
 		return ErrNoSourceInput
 	}
 
-	initCmd := []string{"cosa", "init"}
+	args := []string{"init"}
 	if gitRef != "" {
-		initCmd = append(initCmd, "--force", "--branch", gitRef)
+		args = append(args, "--force", "--branch", gitRef)
 	}
-	initCmd = append(initCmd, gitURI)
-	log.Infof("running '%v'", strings.Join(initCmd, " "))
-	rc, err := ee.RunCmds(initCmd)
+	args = append(args, gitURI)
+	log.Infof("running 'git %v'", strings.Join(args, " "))
+	cmd := exec.Command("cosa", args...)
+	cmd.Env = envVars
+	rc, err := ee.RunCmds(cmd)
 	if rc != 0 || err != nil {
 		log.WithFields(log.Fields{
-			"cmd":         initCmd,
+			"cmd":         args,
 			"return code": rc,
 			"error":       err,
 		}).Error("Failed to checkout respository")

entrypoint/ocp/ocp.go

@@ -50,11 +50,6 @@ func ocpBuildClient() error {
 	}
 	log.Info("Host is running as an OpenShift custom strategy builder.")
 
-	// BuildConfigs do not use overlays. And when running in Buildconfig
-	// mode, force running as unprivileged.
-	os.Setenv("COSA_SKIP_OVERLAY", "skip")
-	os.Setenv("FORCE_UNPRIVILEGED", "1")
-
 	// Check to make sure that we have a valid contextDir
 	// Almost _always_ this should be in /srv for COSA.
 	cDir := apiBuild.Spec.Source.ContextDir

entrypoint/ocp/sa_secrets.go

@@ -1,6 +1,7 @@
 package ocp
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -96,55 +97,52 @@ func getSecretMapping(s string) (*secretMap, bool) {
 	return nil, false
 }
 
-func (sm *secretMap) writeSecretEnvVars(d map[string][]byte) (int, error) {
-	count := 0
+func (sm *secretMap) writeSecretEnvVars(d map[string][]byte, ret *[]string) error {
 	for k, v := range d {
 		envKey, ok := sm.envVarMap[k]
 		if !ok {
 			continue
 		}
 		log.Debugf("Set envVar %s from secret", envKey)
-		os.Setenv(envKey, strings.TrimSuffix(string(v), "\n"))
-		count++
+		*ret = append(*ret, fmt.Sprintf("%s=%s", envKey, strings.TrimSuffix(string(v), "\n")))
 	}
-	return count, nil
+	return nil
 }
 
-func (sm *secretMap) writeSecretFiles(toDir, name string, d map[string][]byte) (int, error) {
+func (sm *secretMap) writeSecretFiles(toDir, name string, d map[string][]byte, ret *[]string) error {
 	sDir := filepath.Join(toDir, name)
 	if err := os.MkdirAll(sDir, 0755); err != nil {
-		return 0, err
+		return err
 	}
-	count := 0
 	for k, v := range d {
 		eKey, ok := sm.fileVarMap[k]
 		if !ok {
 			continue
 		}
 		f := filepath.Join(sDir, k)
 		if err := ioutil.WriteFile(k, v, 0555); err != nil {
-			return count, err
+			return err
 		}
-		os.Setenv(eKey, f)
-		log.Debugf("Set envVar %s to filepath %s", eKey, f)
-		count++
+		*ret = append(*ret, fmt.Sprintf("%s=%s", eKey, f))
 	}
-	return count, nil
+	return nil
 }
 
 // kubernetesSecretSetup looks for matching secrets in the environment matching
 // 'coreos-assembler.coreos.com/secret=k' and then maps the secret
 // automatically in. "k" must be in the "known" secrets type to be mapped
 // automatically.
-func kubernetesSecretsSetup(toDir string) error {
+func kubernetesSecretsSetup(toDir string) ([]string, error) {
 	lo := metav1.ListOptions{
 		LabelSelector: secretLabelName,
 		Limit:         100,
 	}
 
+	var ret []string
+
 	secrets, err := apiClient.Secrets(projectNamespace).List(lo)
 	if err != nil {
-		return err
+		return ret, err
 	}
 	log.Infof("Found %d secrets to consider", len(secrets.Items))
 
@@ -162,18 +160,14 @@ func kubernetesSecretsSetup(toDir string) error {
 			}
 			log.Infof("Known secret type for %s found, mapping automatically", v)
 
-			c, err := m.writeSecretEnvVars(secret.Data)
-			if err != nil {
+			if err := m.writeSecretEnvVars(secret.Data, &ret); err != nil {
 				log.Errorf("Failed to set envVars for %s: %s", sName, err)
 			}
-			log.Infof("Set %d envVars from %s", c, sName)
 
-			c, err = m.writeSecretFiles(toDir, sName, secret.Data)
-			if err != nil {
+			if err := m.writeSecretFiles(toDir, sName, secret.Data, &ret); err != nil {
 				log.Errorf("Failed to set files envVars for %s: %s", sName, err)
 			}
-			log.Infof("Set %d envVars to files from %s", c, sName)
 		}
 	}
-	return nil
+	return ret, nil
 }

entrypoint/spec/clone.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"io/ioutil"
 	"os"
+	"os/exec"
 	"path/filepath"
 
 	ee "github.com/coreos/entrypoint/exec"
@@ -29,12 +30,13 @@ func cloneJobSpec(url, ref, specFile string) (*JobSpec, error) {
 
 	// Clone the JobSpec Repo
 	jsD := filepath.Join(tmpd, "jobspec")
-	gitCmd := []string{"git", "clone"}
+	args := []string{"clone"}
 	if ref != "" {
-		gitCmd = append(gitCmd, "--branch", ref)
+		args = append(args, "--branch", ref)
 	}
-	gitCmd = append(gitCmd, url, jsD)
-	rc, err := ee.RunCmds(gitCmd)
+	args = append(args, url, jsD)
+	cmd := exec.Command("git", args...)
+	rc, err := ee.RunCmds(cmd)
 	if rc != 0 {
 		if err == nil {
 			err = errors.New("non-zero exit from command")