Merge pull request #3598 from ashley-cui/envs

Add support for env var secret sources

define/types.go

@@ -93,6 +93,13 @@ type IDMappingOptions struct {
 	GIDMap         []specs.LinuxIDMapping
 }
 
+// Secret is a secret source that can be used in a RUN
+type Secret struct {
+	ID         string
+	Source     string
+	SourceType string
+}
+
 // TempDirForURL checks if the passed-in string looks like a URL or -.  If it is,
 // TempDirForURL creates a temporary directory, arranges for its contents to be
 // the contents of that URL, and returns the temporary directory's path, along

imagebuildah/executor.go

@@ -123,7 +123,7 @@ type Executor struct {
 	imageInfoCache                 map[string]imageTypeAndHistoryAndDiffIDs
 	fromOverride                   string
 	manifest                       string
-	secrets                        map[string]string
+	secrets                        map[string]define.Secret
 	sshsources                     map[string]*sshagent.Source
 	logPrefix                      string
 }

pkg/parse/parse.go

@@ -1212,35 +1212,60 @@ func GetTempDir() string {
 }
 
 // Secrets parses the --secret flag
-func Secrets(secrets []string) (map[string]string, error) {
-	parsed := make(map[string]string)
-	invalidSyntax := errors.Errorf("incorrect secret flag format: should be --secret id=foo,src=bar")
+func Secrets(secrets []string) (map[string]define.Secret, error) {
+	invalidSyntax := errors.Errorf("incorrect secret flag format: should be --secret id=foo,src=bar[,env=ENV,type=file|env]")
+	parsed := make(map[string]define.Secret)
 	for _, secret := range secrets {
-		split := strings.Split(secret, ",")
-		if len(split) > 2 {
+		tokens := strings.Split(secret, ",")
+		var id, src, typ string
+		for _, val := range tokens {
+			kv := strings.SplitN(val, "=", 2)
+			switch kv[0] {
+			case "id":
+				id = kv[1]
+			case "src":
+				src = kv[1]
+			case "env":
+				src = kv[1]
+				typ = "env"
+			case "type":
+				if kv[1] != "file" && kv[1] != "env" {
+					return nil, errors.New("invalid secret type, must be file or env")
+				}
+				typ = kv[1]
+			}
+		}
+		if id == "" {
 			return nil, invalidSyntax
 		}
-		if len(split) == 2 {
-			id := strings.Split(split[0], "=")
-			src := strings.Split(split[1], "=")
-			if len(split) == 2 && strings.ToLower(id[0]) == "id" && strings.ToLower(src[0]) == "src" {
-				fullPath, err := filepath.Abs(src[1])
-				if err != nil {
-					return nil, err
-				}
-				_, err = os.Stat(fullPath)
-				if err == nil {
-					parsed[id[1]] = fullPath
-				}
-				if err != nil {
-					return nil, errors.Wrap(err, "could not parse secrets")
-				}
+		if src == "" {
+			src = id
+		}
+		if typ == "" {
+			if _, ok := os.LookupEnv(id); ok {
+				typ = "env"
 			} else {
-				return nil, invalidSyntax
+				typ = "file"
 			}
-		} else {
-			return nil, invalidSyntax
 		}
+
+		if typ == "file" {
+			fullPath, err := filepath.Abs(src)
+			if err != nil {
+				return nil, errors.Wrap(err, "could not parse secrets")
+			}
+			_, err = os.Stat(fullPath)
+			if err != nil {
+				return nil, errors.Wrap(err, "could not parse secrets")
+			}
+			src = fullPath
+		}
+		newSecret := define.Secret{
+			Source:     src,
+			SourceType: typ,
+		}
+		parsed[id] = newSecret
+
 	}
 	return parsed, nil
 }

run.go

@@ -141,7 +141,7 @@ type RunOptions struct {
 	// Devices are the additional devices to add to the containers
 	Devices define.ContainerDevices
 	// Secrets are the available secrets to use in a RUN
-	Secrets map[string]string
+	Secrets map[string]define.Secret
 	// SSHSources is the available ssh agents to use in a RUN
 	SSHSources map[string]*sshagent.Source `json:"-"`
 	// RunMounts are mounts for this run. RunMounts for this run
@@ -153,6 +153,8 @@ type RunOptions struct {
 type runMountArtifacts struct {
 	// RunMountTargets are the run mount targets inside the container
 	RunMountTargets []string
+	// TmpFiles are artifacts that need to be removed outside the container
+	TmpFiles []string
 	// Agents are the ssh agents started
 	Agents []*sshagent.AgentServer
 	// SSHAuthSock is the path to the ssh auth sock inside the container

run_linux.go

@@ -415,7 +415,7 @@ func runSetupBuiltinVolumes(mountLabel, mountPoint, containerDir string, builtin
 	return mounts, nil
 }
 
-func (b *Builder) setupMounts(mountPoint string, spec *specs.Spec, bundlePath string, optionMounts []specs.Mount, bindFiles map[string]string, builtinVolumes, volumeMounts []string, shmSize string, namespaceOptions define.NamespaceOptions, secrets map[string]string, sshSources map[string]*sshagent.Source, runFileMounts []string, contextDir string) (*runMountArtifacts, error) {
+func (b *Builder) setupMounts(mountPoint string, spec *specs.Spec, bundlePath string, optionMounts []specs.Mount, bindFiles map[string]string, builtinVolumes, volumeMounts []string, shmSize string, namespaceOptions define.NamespaceOptions, secrets map[string]define.Secret, sshSources map[string]*sshagent.Source, runFileMounts []string, contextDir string) (*runMountArtifacts, error) {
 	// Start building a new list of mounts.
 	var mounts []specs.Mount
 	haveMount := func(destination string) bool {
@@ -2375,8 +2375,9 @@ func init() {
 }
 
 // runSetupRunMounts sets up mounts that exist only in this RUN, not in subsequent runs
-func (b *Builder) runSetupRunMounts(mounts []string, secrets map[string]string, sshSources map[string]*sshagent.Source, containerWorkingDir string, contextDir string, uidmap []spec.LinuxIDMapping, gidmap []spec.LinuxIDMapping, rootUID int, rootGID int, processUID int, processGID int) ([]spec.Mount, *runMountArtifacts, error) {
-	mountTargets := make([]string, 0, 10)
+func (b *Builder) runSetupRunMounts(mounts []string, secrets map[string]define.Secret, sshSources map[string]*sshagent.Source, containerWorkingDir string, contextDir string, uidmap []spec.LinuxIDMapping, gidmap []spec.LinuxIDMapping, rootUID int, rootGID int, processUID int, processGID int) ([]spec.Mount, *runMountArtifacts, error) {
+	mountTargets := make([]string, 0, len(mounts))
+	tmpFiles := make([]string, 0, len(mounts))
 	finalMounts := make([]specs.Mount, 0, len(mounts))
 	agents := make([]*sshagent.AgentServer, 0, len(mounts))
 	sshCount := 0
@@ -2395,14 +2396,16 @@ func (b *Builder) runSetupRunMounts(mounts []string, secrets map[string]string,
 		// For now, we only support type secret.
 		switch kv[1] {
 		case "secret":
-			mount, err := getSecretMount(tokens, secrets, b.MountLabel, containerWorkingDir, uidmap, gidmap)
+			mount, envFile, err := getSecretMount(tokens, secrets, b.MountLabel, containerWorkingDir, uidmap, gidmap)
 			if err != nil {
 				return nil, nil, err
 			}
 			if mount != nil {
 				finalMounts = append(finalMounts, *mount)
 				mountTargets = append(mountTargets, mount.Destination)
-
+				if envFile != "" {
+					tmpFiles = append(tmpFiles, envFile)
+				}
 			}
 		case "ssh":
 			mount, agent, err := b.getSSHMount(tokens, sshCount, sshSources, b.MountLabel, uidmap, gidmap, b.ProcessLabel)
@@ -2446,6 +2449,7 @@ func (b *Builder) runSetupRunMounts(mounts []string, secrets map[string]string,
 	}
 	artifacts := &runMountArtifacts{
 		RunMountTargets: mountTargets,
+		TmpFiles:        tmpFiles,
 		Agents:          agents,
 		SSHAuthSock:     defaultSSHSock,
 	}
@@ -2497,10 +2501,10 @@ func (b *Builder) getCacheMount(tokens []string, rootUID, rootGID, processUID, p
 	return &volumes[0], nil
 }
 
-func getSecretMount(tokens []string, secrets map[string]string, mountlabel string, containerWorkingDir string, uidmap []spec.LinuxIDMapping, gidmap []spec.LinuxIDMapping) (*spec.Mount, error) {
+func getSecretMount(tokens []string, secrets map[string]define.Secret, mountlabel string, containerWorkingDir string, uidmap []spec.LinuxIDMapping, gidmap []spec.LinuxIDMapping) (*spec.Mount, string, error) {
 	errInvalidSyntax := errors.New("secret should have syntax id=id[,target=path,required=bool,mode=uint,uid=uint,gid=uint")
 	if len(tokens) == 0 {
-		return nil, errInvalidSyntax
+		return nil, "", errInvalidSyntax
 	}
 	var err error
 	var id, target string
@@ -2517,84 +2521,102 @@ func getSecretMount(tokens []string, secrets map[string]string, mountlabel strin
 		case "required":
 			required, err = strconv.ParseBool(kv[1])
 			if err != nil {
-				return nil, errInvalidSyntax
+				return nil, "", errInvalidSyntax
 			}
 		case "mode":
 			mode64, err := strconv.ParseUint(kv[1], 8, 32)
 			if err != nil {
-				return nil, errInvalidSyntax
+				return nil, "", errInvalidSyntax
 			}
 			mode = uint32(mode64)
 		case "uid":
 			uid64, err := strconv.ParseUint(kv[1], 10, 32)
 			if err != nil {
-				return nil, errInvalidSyntax
+				return nil, "", errInvalidSyntax
 			}
 			uid = uint32(uid64)
 		case "gid":
 			gid64, err := strconv.ParseUint(kv[1], 10, 32)
 			if err != nil {
-				return nil, errInvalidSyntax
+				return nil, "", errInvalidSyntax
 			}
 			gid = uint32(gid64)
 		default:
-			return nil, errInvalidSyntax
+			return nil, "", errInvalidSyntax
 		}
 	}
 
 	if id == "" {
-		return nil, errInvalidSyntax
+		return nil, "", errInvalidSyntax
 	}
 	// Default location for secretis is /run/secrets/id
 	if target == "" {
 		target = "/run/secrets/" + id
 	}
 
-	src, ok := secrets[id]
+	secr, ok := secrets[id]
 	if !ok {
 		if required {
-			return nil, errors.Errorf("secret required but no secret with id %s found", id)
+			return nil, "", errors.Errorf("secret required but no secret with id %s found", id)
 		}
-		return nil, nil
+		return nil, "", nil
 	}
+	var data []byte
+	var envFile string
+	var ctrFileOnHost string
 
-	// Copy secrets to container working dir, since we need to chmod, chown and relabel it
-	// for the container user and we don't want to mess with the original file
-	ctrFileOnHost := filepath.Join(containerWorkingDir, "secrets", id)
-	_, err = os.Stat(ctrFileOnHost)
-	if os.IsNotExist(err) {
-		data, err := ioutil.ReadFile(src)
+	switch secr.SourceType {
+	case "env":
+		data = []byte(os.Getenv(secr.Source))
+		tmpFile, err := ioutil.TempFile("/dev/shm", "buildah*")
 		if err != nil {
-			return nil, err
+			return nil, "", err
 		}
-		if err := os.MkdirAll(filepath.Dir(ctrFileOnHost), 0644); err != nil {
-			return nil, err
+		envFile = tmpFile.Name()
+		ctrFileOnHost = tmpFile.Name()
+	case "file":
+		data, err = ioutil.ReadFile(secr.Source)
+		if err != nil {
+			return nil, "", err
 		}
-		if err := ioutil.WriteFile(ctrFileOnHost, data, 0644); err != nil {
-			return nil, err
+		ctrFileOnHost = filepath.Join(containerWorkingDir, "secrets", id)
+		_, err = os.Stat(ctrFileOnHost)
+		if !os.IsNotExist(err) {
+			return nil, "", err
 		}
+	default:
+		return nil, "", errors.New("invalid source secret type")
+	}
+
+	// Copy secrets to container working dir (or tmp dir if it's an env), since we need to chmod,
+	// chown and relabel it for the container user and we don't want to mess with the original file
+	if err := os.MkdirAll(filepath.Dir(ctrFileOnHost), 0644); err != nil {
+		return nil, "", err
+	}
+	if err := ioutil.WriteFile(ctrFileOnHost, data, 0644); err != nil {
+		return nil, "", err
 	}
 
 	if err := label.Relabel(ctrFileOnHost, mountlabel, false); err != nil {
-		return nil, err
+		return nil, "", err
 	}
 	hostUID, hostGID, err := util.GetHostIDs(uidmap, gidmap, uid, gid)
 	if err != nil {
-		return nil, err
+		return nil, "", err
 	}
 	if err := os.Lchown(ctrFileOnHost, int(hostUID), int(hostGID)); err != nil {
-		return nil, err
+		return nil, "", err
 	}
 	if err := os.Chmod(ctrFileOnHost, os.FileMode(mode)); err != nil {
-		return nil, err
+		return nil, "", err
 	}
 	newMount := specs.Mount{
 		Destination: target,
 		Type:        "bind",
 		Source:      ctrFileOnHost,
 		Options:     []string{"bind", "rprivate", "ro"},
 	}
-	return &newMount, nil
+	return &newMount, envFile, nil
 }
 
 // getSSHMount parses the --mount type=ssh flag in the Containerfile, checks if there's an ssh source provided, and creates and starts an ssh-agent to be forwarded into the container
@@ -2730,5 +2752,15 @@ func cleanupRunMounts(mountpoint string, artifacts *runMountArtifacts) error {
 			return err
 		}
 	}
-	return nil
+	var prevErr error
+	for _, path := range artifacts.TmpFiles {
+		err := os.Remove(path)
+		if !os.IsNotExist(err) {
+			if prevErr != nil {
+				logrus.Error(prevErr)
+			}
+			prevErr = err
+		}
+	}
+	return prevErr
 }

tests/bud.bats

@@ -3329,6 +3329,38 @@ _EOF
   expect_output --substring "secret required but no secret with id mysecret found"
 }
 
+@test "bud with containerfile env secret" {
+  export MYSECRET=SOMESECRETDATA
+  run_buildah build --secret=id=mysecret,src=MYSECRET,type=env --signature-policy ${TESTSDIR}/policy.json  -t secretimg -f ${TESTSDIR}/bud/run-mounts/Dockerfile.secret ${TESTSDIR}/bud/run-mounts
+  expect_output --substring "SOMESECRETDATA"
+
+  run_buildah from secretimg
+  run_buildah 1 run secretimg-working-container cat /run/secrets/mysecret
+  expect_output --substring "cat: can't open '/run/secrets/mysecret': No such file or directory"
+  run_buildah rm -a
+
+  run_buildah build --secret=id=mysecret,env=MYSECRET --signature-policy ${TESTSDIR}/policy.json  -t secretimg -f ${TESTSDIR}/bud/run-mounts/Dockerfile.secret ${TESTSDIR}/bud/run-mounts
+  expect_output --substring "SOMESECRETDATA"
+
+  run_buildah from secretimg
+  run_buildah 1 run secretimg-working-container cat /run/secrets/mysecret
+  expect_output --substring "cat: can't open '/run/secrets/mysecret': No such file or directory"
+  run_buildah rm -a
+}
+
+@test "bud with containerfile env secret priority" {
+  _prefetch alpine
+  mytmpdir=${TESTDIR}/my-dir1
+  mkdir -p ${mytmpdir}
+  cat > $mytmpdir/mysecret << _EOF
+SOMESECRETDATA
+_EOF
+
+  export mysecret=ENVDATA
+  run_buildah build --secret=id=mysecret --signature-policy ${TESTSDIR}/policy.json  -t secretimg -f ${TESTSDIR}/bud/run-mounts/Dockerfile.secret ${TESTSDIR}/bud/run-mounts
+  expect_output --substring "ENVDATA"
+}
+
 @test "bud-multiple-platform-values" {
   outputlist=testlist
   # check if we can run a couple of 32-bit versions of an image, and if we can,