Merge pull request #942 from umohnani8/secrets_patch

Add secrets support to crio

cmd/crio/config.go

@@ -108,6 +108,9 @@ cgroup_manager = "{{ .CgroupManager }}"
 # hooks_dir_path is the oci hooks directory for automatically executed hooks
 hooks_dir_path = "{{ .HooksDirPath }}"
 
+# default_mounts_path is the secrets mounts file path
+default_mounts_path = "{{ .DefaultMountsPath }}"
+
 # pids_limit is the number of processes allowed in a container
 pids_limit = {{ .PidsLimit }}
 

cmd/crio/main.go

@@ -127,6 +127,9 @@ func mergeConfig(config *server.Config, ctx *cli.Context) error {
 	if ctx.GlobalIsSet("hooks-dir-path") {
 		config.HooksDirPath = ctx.GlobalString("hooks-dir-path")
 	}
+	if ctx.GlobalIsSet("default-mounts-path") {
+		config.DefaultMountsPath = ctx.GlobalString("default-mounts-path")
+	}
 	if ctx.GlobalIsSet("pids-limit") {
 		config.PidsLimit = ctx.GlobalInt64("pids-limit")
 	}
@@ -322,6 +325,11 @@ func main() {
 			Value:  libkpod.DefaultHooksDirPath,
 			Hidden: true,
 		},
+		cli.StringFlag{
+			Name:   "default-mounts-path",
+			Usage:  "set the default mounts file path",
+			Hidden: true,
+		},
 		cli.BoolFlag{
 			Name:  "profile",
 			Usage: "enable pprof remote profiler on localhost:6060",

libkpod/config.go

@@ -24,6 +24,10 @@ const (
 	cgroupManager       = oci.CgroupfsCgroupsManager
 	lockPath            = "/run/crio.lock"
 	containerExitsDir   = oci.ContainerExitsDir
+	// DefaultMountsFile holds the default mount paths in the form "host:container"
+	DefaultMountsFile = "/usr/share/containers/mounts.conf"
+	// OverrideMountsFile holds the override mount paths in the form "host:container"
+	OverrideMountsFile = "/etc/containers/mounts.conf"
 )
 
 // Config represents the entire set of configuration values that can be set for
@@ -145,6 +149,9 @@ type RuntimeConfig struct {
 	// HooksDirPath location of oci hooks config files
 	HooksDirPath string `toml:"hooks_dir_path"`
 
+	// DefaultMountsPath location of the default mounts file
+	DefaultMountsPath string `toml:"default_mounts_path"`
+
 	// Hooks List of hooks to run with container
 	Hooks map[string]HookParams
 
@@ -288,6 +295,7 @@ func DefaultConfig() *Config {
 			ContainerExitsDir: containerExitsDir,
 			HooksDirPath:      DefaultHooksDirPath,
 			LogSizeMax:        DefaultLogSizeMax,
+			DefaultMountsPath: DefaultMountsFile,
 		},
 		ImageConfig: ImageConfig{
 			DefaultTransport:    defaultTransport,

server/container_create.go

@@ -384,6 +384,27 @@ func ensureSaneLogPath(logPath string) error {
 	return nil
 }
 
+// addSecretsBindMounts mounts user defined secrets to the container
+func addSecretsBindMounts(mountLabel, ctrRunDir, configDefaultMountsPath string, specgen generate.Generator) error {
+	mountPaths := []string{libkpod.OverrideMountsFile, libkpod.DefaultMountsFile}
+	// configDefaultMountsPath is used to override the mount file path for testing purposes only when set in the runtime config
+	if configDefaultMountsPath != "" {
+		mountPaths = []string{configDefaultMountsPath}
+	}
+	for _, path := range mountPaths {
+		containerMounts := specgen.Spec().Mounts
+		mounts, err := secretMounts(mountLabel, path, ctrRunDir, containerMounts)
+		if err != nil {
+			return err
+		}
+		for _, m := range mounts {
+			specgen.AddBindMount(m.Source, m.Destination, nil)
+
+		}
+	}
+	return nil
+}
+
 // CreateContainer creates a new container in specified PodSandbox
 func (s *Server) CreateContainer(ctx context.Context, req *pb.CreateContainerRequest) (res *pb.CreateContainerResponse, err error) {
 	logrus.Debugf("CreateContainerRequest %+v", req)
@@ -911,6 +932,10 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string,
 		return nil, err
 	}
 
+	if err = addSecretsBindMounts(mountLabel, containerInfo.RunDir, s.config.DefaultMountsPath, specgen); err != nil {
+		return nil, fmt.Errorf("failed to mount secrets: %v", err)
+	}
+
 	mountPoint, err := s.StorageRuntimeServer().StartContainer(containerID)
 	if err != nil {
 		return nil, fmt.Errorf("failed to mount container %s(%s): %v", containerName, containerID, err)

server/secrets.go

@@ -0,0 +1,186 @@
+package server
+
+import (
+	"bufio"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"strings"
+
+	rspec "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/opencontainers/selinux/go-selinux/label"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+)
+
+// SecretData info
+type SecretData struct {
+	Name string
+	Data []byte
+}
+
+// SaveTo saves secret data to given directory
+func (s SecretData) SaveTo(dir string) error {
+	path := filepath.Join(dir, s.Name)
+	if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil && !os.IsExist(err) {
+		return err
+	}
+	return ioutil.WriteFile(path, s.Data, 0700)
+}
+
+// readMountFile returns a list of the host:container paths
+func readMountFile(mountFilePath string) ([]string, error) {
+	var mountPaths []string
+	file, err := os.Open(mountFilePath)
+	if err != nil {
+		logrus.Warnf("file doesn't exist %q", mountFilePath)
+		return nil, nil
+	}
+	defer file.Close()
+
+	scanner := bufio.NewScanner(file)
+	scanner.Split(bufio.ScanLines)
+	for scanner.Scan() {
+		mountPaths = append(mountPaths, scanner.Text())
+	}
+
+	return mountPaths, nil
+}
+
+func readAll(root, prefix string) ([]SecretData, error) {
+	path := filepath.Join(root, prefix)
+
+	data := []SecretData{}
+
+	files, err := ioutil.ReadDir(path)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return data, nil
+		}
+
+		return nil, err
+	}
+
+	for _, f := range files {
+		fileData, err := readFile(root, filepath.Join(prefix, f.Name()))
+		if err != nil {
+			// If the file did not exist, might be a dangling symlink
+			// Ignore the error
+			if os.IsNotExist(err) {
+				continue
+			}
+			return nil, err
+		}
+		data = append(data, fileData...)
+	}
+
+	return data, nil
+}
+
+func readFile(root, name string) ([]SecretData, error) {
+	path := filepath.Join(root, name)
+
+	s, err := os.Stat(path)
+	if err != nil {
+		return nil, err
+	}
+
+	if s.IsDir() {
+		dirData, err := readAll(root, name)
+		if err != nil {
+			return nil, err
+		}
+		return dirData, nil
+	}
+	bytes, err := ioutil.ReadFile(path)
+	if err != nil {
+		return nil, err
+	}
+	return []SecretData{{Name: name, Data: bytes}}, nil
+}
+
+// getHostAndCtrDir separates the host:container paths
+func getMountsMap(path string) (string, string, error) {
+	arr := strings.SplitN(path, ":", 2)
+	if len(arr) == 2 {
+		return arr[0], arr[1], nil
+	}
+	return "", "", errors.Errorf("unable to get host and container dir")
+}
+
+func getHostSecretData(hostDir string) ([]SecretData, error) {
+	var allSecrets []SecretData
+	hostSecrets, err := readAll(hostDir, "")
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to read secrets from %q", hostDir)
+	}
+	return append(allSecrets, hostSecrets...), nil
+}
+
+// secretMount copies the contents of host directory to container directory
+// and returns a list of mounts
+func secretMounts(mountLabel, mountFilePath, containerWorkingDir string, runtimeMounts []rspec.Mount) ([]rspec.Mount, error) {
+	var mounts []rspec.Mount
+	mountPaths, err := readMountFile(mountFilePath)
+	if err != nil {
+		return nil, err
+	}
+	for _, path := range mountPaths {
+		hostDir, ctrDir, err := getMountsMap(path)
+		if err != nil {
+			return nil, err
+		}
+		// skip if the hostDir path doesn't exist
+		if _, err := os.Stat(hostDir); os.IsNotExist(err) {
+			logrus.Warnf("%q doesn't exist, skipping", hostDir)
+			continue
+		}
+
+		ctrDir = filepath.Join(containerWorkingDir, ctrDir)
+		// skip if ctrDir has already been mounted by caller
+		if isAlreadyMounted(runtimeMounts, ctrDir) {
+			logrus.Warnf("%q has already been mounted; cannot override mount", ctrDir)
+			continue
+		}
+
+		if err := os.RemoveAll(ctrDir); err != nil {
+			return nil, fmt.Errorf("remove container directory failed: %v", err)
+		}
+
+		if err := os.MkdirAll(ctrDir, 0755); err != nil {
+			return nil, fmt.Errorf("making container directory failed: %v", err)
+		}
+
+		hostDir, err = resolveSymbolicLink(hostDir)
+		if err != nil {
+			return nil, err
+		}
+
+		data, err := getHostSecretData(hostDir)
+		if err != nil {
+			return nil, errors.Wrapf(err, "getting host secret data failed")
+		}
+		for _, s := range data {
+			s.SaveTo(ctrDir)
+		}
+		label.Relabel(ctrDir, mountLabel, false)
+
+		m := rspec.Mount{
+			Source:      hostDir,
+			Destination: ctrDir,
+		}
+
+		mounts = append(mounts, m)
+	}
+	return mounts, nil
+}
+
+func isAlreadyMounted(mounts []rspec.Mount, mountPath string) bool {
+	for _, mount := range mounts {
+		if mount.Destination == mountPath {
+			return true
+		}
+	}
+	return false
+}

test/crio_secrets.bats

@@ -0,0 +1,44 @@
+#!/usr/bin/env bats
+
+load helpers
+
+IMAGE="redis:alpine"
+
+function teardown() {
+	cleanup_test
+}
+
+function setup() {
+    MOUNT_PATH="$TESTDIR/secrets"
+    mkdir ${MOUNT_PATH}
+    MOUNT_FILE="${MOUNT_PATH}/test.txt"
+    touch ${MOUNT_FILE}
+    echo "Testing secrets mounts!" > ${MOUNT_FILE}
+
+    echo "${MOUNT_PATH}:/container/path1" > ${DEFAULT_MOUNTS_FILE}
+}
+
+@test "bind secrets mounts to container" {
+    start_crio
+    run crioctl pod run --config "$TESTDATA"/sandbox_config.json
+    echo "$output"
+    [ "$status" -eq 0 ]
+    pod_id="$output"
+    run crioctl image pull "$IMAGE"
+    [ "$status" -eq 0 ]
+    run crioctl ctr create --config "$TESTDATA"/container_redis.json --pod "$pod_id"
+    echo "$output"
+    [ "$status" -eq 0 ]
+    ctr_id="$output"
+    run crioctl ctr execsync --id "$ctr_id" mount
+    echo "$output"
+    [ "$status" -eq 0 ]
+    mount_info="$output"
+    grep $ctr_id/userdata/container/path1 <<< "$mount_info"
+    echo "$output"
+    [ "$status" -eq 0 ]
+    rm -rf MOUNT_PATH
+    cleanup_ctrs
+    cleanup_pods
+    stop_crio
+}

test/helpers.bash

@@ -69,6 +69,13 @@ HOOKSDIR=$TESTDIR/hooks
 mkdir ${HOOKSDIR}
 HOOKS_OPTS="--hooks-dir-path=$HOOKSDIR"
 
+# Setup default secrets mounts file
+MOUNTS_DIR="$TESTDIR/containers"
+mkdir ${MOUNTS_DIR}
+DEFAULT_MOUNTS_FILE="${MOUNTS_DIR}/mounts.conf"
+touch ${DEFAULT_MOUNTS_FILE}
+DEFAULT_MOUNTS_OPTS="--default-mounts-path=$DEFAULT_MOUNTS_FILE"
+
 # We may need to set some default storage options.
 case "$(stat -f -c %T ${TESTDIR})" in
     aufs)
@@ -235,7 +242,7 @@ function start_crio() {
 	"$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTS --runroot "$TESTDIR/crio-run" --image-name=mrunalp/image-volume-test --import-from=dir:"$ARTIFACTS_PATH"/image-volume-test-image --add-name=docker.io/library/mrunalp/image-volume-test --signature-policy="$INTEGRATION_ROOT"/policy.json
 	"$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTS --runroot "$TESTDIR/crio-run" --image-name=busybox:latest --import-from=dir:"$ARTIFACTS_PATH"/busybox-image --add-name=docker.io/library/busybox:latest --signature-policy="$INTEGRATION_ROOT"/policy.json
 	"$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTS --runroot "$TESTDIR/crio-run" --image-name=runcom/stderr-test:latest --import-from=dir:"$ARTIFACTS_PATH"/stderr-test --add-name=docker.io/runcom/stderr-test:latest --signature-policy="$INTEGRATION_ROOT"/policy.json
-	"$CRIO_BINARY" ${HOOKS_OPTS} --conmon "$CONMON_BINARY" --listen "$CRIO_SOCKET" --cgroup-manager "$CGROUP_MANAGER" --registry "docker.io" --runtime "$RUNTIME_BINARY" --root "$TESTDIR/crio" --runroot "$TESTDIR/crio-run" $STORAGE_OPTS --seccomp-profile "$seccomp" --apparmor-profile "$apparmor" --cni-config-dir "$CRIO_CNI_CONFIG" --cni-plugin-dir "$CRIO_CNI_PLUGIN" --signature-policy "$INTEGRATION_ROOT"/policy.json --image-volumes "$IMAGE_VOLUMES" --pids-limit "$PIDS_LIMIT" --log-size-max "$LOG_SIZE_MAX_LIMIT" --config /dev/null config >$CRIO_CONFIG
+	"$CRIO_BINARY" ${DEFAULT_MOUNTS_OPTS} ${HOOKS_OPTS} --conmon "$CONMON_BINARY" --listen "$CRIO_SOCKET" --cgroup-manager "$CGROUP_MANAGER" --registry "docker.io" --runtime "$RUNTIME_BINARY" --root "$TESTDIR/crio" --runroot "$TESTDIR/crio-run" $STORAGE_OPTS --seccomp-profile "$seccomp" --apparmor-profile "$apparmor" --cni-config-dir "$CRIO_CNI_CONFIG" --signature-policy "$INTEGRATION_ROOT"/policy.json --image-volumes "$IMAGE_VOLUMES" --pids-limit "$PIDS_LIMIT" --log-size-max "$LOG_SIZE_MAX_LIMIT" --config /dev/null config >$CRIO_CONFIG
 
 	# Prepare the CNI configuration files, we're running with non host networking by default
 	if [[ -n "$4" ]]; then