Merge pull request #1004 from umohnani8/secrets_patch

Follow up changes on secrets patch

cmd/crio/config.go

@@ -108,8 +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 is the mounts list to be mounted for the container when created
-default_mounts_path = "{{ .DefaultMountsPath }}"
+default_mounts = [
+{{ range $mount := .DefaultMounts }}{{ printf "\t%q, \n" $mount }}{{ end }}]
 
 # pids_limit is the number of processes allowed in a container
 pids_limit = {{ .PidsLimit }}

cmd/crio/main.go

@@ -127,8 +127,8 @@ 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") {
+	if ctx.GlobalIsSet("default-mounts") {
-		config.DefaultMountsPath = ctx.GlobalString("default-mounts-path")
+		config.DefaultMounts = ctx.GlobalStringSlice("default-mounts")
 	}
 	if ctx.GlobalIsSet("pids-limit") {
 		config.PidsLimit = ctx.GlobalInt64("pids-limit")
@@ -325,9 +325,9 @@ func main() {
 			Value:  libkpod.DefaultHooksDirPath,
 			Hidden: true,
 		},
-		cli.StringFlag{
+		cli.StringSliceFlag{
-			Name:   "default-mounts-path",
+			Name:   "default-mounts",
-			Usage:  "set the default mounts file path",
+			Usage:  "add one or more default mount paths in the form host:container",
 			Hidden: true,
 		},
 		cli.BoolFlag{

docs/crio.conf.5.md

@@ -105,6 +105,9 @@ Example:
 **no_pivot**=*true*|*false*
   Instructs the runtime to not use pivot_root, but instead use MS_MOVE
 
+**default_mounts**=[]
+  List of mount points, in the form host:container, to be mounted in every container
+
 ## CRIO.IMAGE TABLE
 
 **default_transport**

libkpod/config.go

@@ -24,10 +24,6 @@ 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
@@ -149,8 +145,9 @@ type RuntimeConfig struct {
 	// HooksDirPath location of oci hooks config files
 	HooksDirPath string `toml:"hooks_dir_path"`
 
-	// DefaultMountsPath location of the default mounts file
+	// DefaultMounts is the list of mounts to be mounted for each container
-	DefaultMountsPath string `toml:"default_mounts_path"`
+	// The format of each mount is "host-path:container-path"
+	DefaultMounts []string `toml:"default_mounts"`
 
 	// Hooks List of hooks to run with container
 	Hooks map[string]HookParams
@@ -295,7 +292,6 @@ func DefaultConfig() *Config {
 			ContainerExitsDir: containerExitsDir,
 			HooksDirPath:      DefaultHooksDirPath,
 			LogSizeMax:        DefaultLogSizeMax,
-			DefaultMountsPath: DefaultMountsFile,
 		},
 		ImageConfig: ImageConfig{
 			DefaultTransport:    defaultTransport,

server/container_create.go

@@ -385,15 +385,9 @@ func ensureSaneLogPath(logPath string) error {
 }
 
 // addSecretsBindMounts mounts user defined secrets to the container
-func addSecretsBindMounts(mountLabel, ctrRunDir, configDefaultMountsPath string, specgen generate.Generator) error {
+func addSecretsBindMounts(mountLabel, ctrRunDir string, defaultMounts []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)
+	mounts, err := secretMounts(defaultMounts, mountLabel, ctrRunDir, containerMounts)
 	if err != nil {
 		return err
 	}
@@ -401,7 +395,6 @@ func addSecretsBindMounts(mountLabel, ctrRunDir, configDefaultMountsPath string,
 		specgen.AddBindMount(m.Source, m.Destination, nil)
 
 	}
-	}
 	return nil
 }
 
@@ -932,9 +925,11 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string,
 		return nil, err
 	}
 
-	if err = addSecretsBindMounts(mountLabel, containerInfo.RunDir, s.config.DefaultMountsPath, specgen); err != nil {
+	if len(s.config.DefaultMounts) > 0 {
+		if err = addSecretsBindMounts(mountLabel, containerInfo.RunDir, s.config.DefaultMounts, specgen); err != nil {
 			return nil, fmt.Errorf("failed to mount secrets: %v", err)
 		}
+	}
 
 	mountPoint, err := s.StorageRuntimeServer().StartContainer(containerID)
 	if err != nil {

server/secrets.go

@@ -1,7 +1,6 @@
 package server
 
 import (
-	"bufio"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -29,25 +28,6 @@ func (s SecretData) SaveTo(dir string) error {
 	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)
 
@@ -120,13 +100,9 @@ func getHostSecretData(hostDir string) ([]SecretData, error) {
 
 // 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) {
+func secretMounts(defaultMountsPaths []string, mountLabel, containerWorkingDir string, runtimeMounts []rspec.Mount) ([]rspec.Mount, error) {
 	var mounts []rspec.Mount
-	mountPaths, err := readMountFile(mountFilePath)
+	for _, path := range defaultMountsPaths {
-	if err != nil {
-		return nil, err
-	}
-	for _, path := range mountPaths {
 		hostDir, ctrDir, err := getMountsMap(path)
 		if err != nil {
 			return nil, err

test/crio_secrets.bats

@@ -8,16 +8,6 @@ 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

test/helpers.bash

@@ -69,12 +69,14 @@ HOOKSDIR=$TESTDIR/hooks
 mkdir ${HOOKSDIR}
 HOOKS_OPTS="--hooks-dir-path=$HOOKSDIR"
 
-# Setup default secrets mounts file
+# Setup default secrets mounts
-MOUNTS_DIR="$TESTDIR/containers"
+MOUNT_PATH="$TESTDIR/secrets"
-mkdir ${MOUNTS_DIR}
+mkdir ${MOUNT_PATH}
-DEFAULT_MOUNTS_FILE="${MOUNTS_DIR}/mounts.conf"
+MOUNT_FILE="${MOUNT_PATH}/test.txt"
-touch ${DEFAULT_MOUNTS_FILE}
+touch ${MOUNT_FILE}
-DEFAULT_MOUNTS_OPTS="--default-mounts-path=$DEFAULT_MOUNTS_FILE"
+echo "Testing secrets mounts!" > ${MOUNT_FILE}
+
+DEFAULT_MOUNTS_OPTS="--default-mounts=${MOUNT_PATH}:/container/path1"
 
 # We may need to set some default storage options.
 case "$(stat -f -c %T ${TESTDIR})" in