From d1aea317869fc752a3177f32addf38489b76b452 Mon Sep 17 00:00:00 2001 From: umohnani8 Date: Thu, 12 Oct 2017 14:14:42 -0400 Subject: [PATCH] Follow up changes on secrets patch Deleted mounts.conf file and moved the secrets mount paths to a list (default-mounts) in crio.conf Signed-off-by: umohnani8 --- cmd/crio/config.go | 5 +++-- cmd/crio/main.go | 10 +++++----- docs/crio.conf.5.md | 3 +++ libkpod/config.go | 10 +++------- server/container_create.go | 27 +++++++++++---------------- server/secrets.go | 28 ++-------------------------- test/crio_secrets.bats | 10 ---------- test/helpers.bash | 14 ++++++++------ 8 files changed, 35 insertions(+), 72 deletions(-) diff --git a/cmd/crio/config.go b/cmd/crio/config.go index af1af302..76e3361d 100644 --- a/cmd/crio/config.go +++ b/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_path = "{{ .DefaultMountsPath }}" +# default_mounts is the mounts list to be mounted for the container when created +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 }} diff --git a/cmd/crio/main.go b/cmd/crio/main.go index 1059d60f..2446bdf6 100644 --- a/cmd/crio/main.go +++ b/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") { - config.DefaultMountsPath = ctx.GlobalString("default-mounts-path") + if ctx.GlobalIsSet("default-mounts") { + 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{ - Name: "default-mounts-path", - Usage: "set the default mounts file path", + cli.StringSliceFlag{ + Name: "default-mounts", + Usage: "add one or more default mount paths in the form host:container", Hidden: true, }, cli.BoolFlag{ diff --git a/docs/crio.conf.5.md b/docs/crio.conf.5.md index ced28c37..32cac7a4 100644 --- a/docs/crio.conf.5.md +++ b/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** diff --git a/libkpod/config.go b/libkpod/config.go index 1fd86a3b..687b4b38 100644 --- a/libkpod/config.go +++ b/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 - DefaultMountsPath string `toml:"default_mounts_path"` + // DefaultMounts is the list of mounts to be mounted for each container + // 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, diff --git a/server/container_create.go b/server/container_create.go index 1e036554..18b33f2e 100644 --- a/server/container_create.go +++ b/server/container_create.go @@ -385,22 +385,15 @@ func ensureSaneLogPath(logPath string) error { } // 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} +func addSecretsBindMounts(mountLabel, ctrRunDir string, defaultMounts []string, specgen generate.Generator) error { + containerMounts := specgen.Spec().Mounts + mounts, err := secretMounts(defaultMounts, mountLabel, ctrRunDir, containerMounts) + if err != nil { + return err } - 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) + for _, m := range mounts { + specgen.AddBindMount(m.Source, m.Destination, nil) - } } return nil } @@ -932,8 +925,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) + 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) diff --git a/server/secrets.go b/server/secrets.go index 34236c1f..c512aede 100644 --- a/server/secrets.go +++ b/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) - if err != nil { - return nil, err - } - for _, path := range mountPaths { + for _, path := range defaultMountsPaths { hostDir, ctrDir, err := getMountsMap(path) if err != nil { return nil, err diff --git a/test/crio_secrets.bats b/test/crio_secrets.bats index 83945aea..fe1e5230 100644 --- a/test/crio_secrets.bats +++ b/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 diff --git a/test/helpers.bash b/test/helpers.bash index 8d38f1fd..03079865 100644 --- a/test/helpers.bash +++ b/test/helpers.bash @@ -69,12 +69,14 @@ 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" +# Setup default secrets mounts +MOUNT_PATH="$TESTDIR/secrets" +mkdir ${MOUNT_PATH} +MOUNT_FILE="${MOUNT_PATH}/test.txt" +touch ${MOUNT_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