Allow relative and ~ path for container mount

This patch allow to specify relative and home (~/) path for container
mounts.src field. In case if specified path is not absolute, it will be
created by following pattern: 'targetPath+mounts.src'; in case if path
contains ~/ - it will be properly expanded (it will allow us to mount
~/.airship working directory).

Change-Id: I878094371a2bc4e48216b1d076e466e3d29a86f6
Signed-off-by: Ruslan Aliev <raliev@mirantis.com>
Relates-To: #484
Closes: #484

pkg/api/v1alpha1/genericcontainer_types.go

@@ -47,7 +47,7 @@ type GenericContainer struct {
 	// Holds container configuration
 	Spec GenericContainerSpec `json:"spec,omitempty"`
 
-	// Config will be passed to stdin of the container togather with other objects
+	// Config will be passed to stdin of the container together with other objects
 	// more information on easy ways to consume the config can be found here
 	// https://googlecontainertools.github.io/kpt/guides/producer/functions/golang/
 	Config string `json:"config,omitempty"`
@@ -68,7 +68,7 @@ type GenericContainerSpec struct {
 	// Supported types are "airship" and "krm"
 	Type GenericContainerType `json:"type,omitempty"`
 
-	// Ariship container spec
+	// Airship container spec
 	Airship AirshipContainerSpec `json:"airship,omitempty"`
 
 	// KRM container function spec
@@ -121,6 +121,9 @@ type StorageMount struct {
 	// For named volumes, this is the name of the volume.
 	// For anonymous volumes, this field is omitted (empty string).
 	// For bind mounts, this is the path to the file or directory on the host.
+	// If provided path is relative, it will be expanded to absolute one by following patterns:
+	// - if starts with '~/' or contains only '~' : $HOME + Src
+	// - in other cases : TargetPath + Src
 	Src string `json:"src,omitempty" yaml:"src,omitempty"`
 
 	// The path where the file or directory is mounted in the container.

pkg/container/api.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"path/filepath"
 	"strings"
 
 	// TODO this small library needs to be moved to airshipctl and extended
@@ -33,6 +34,7 @@ import (
 
 	"opendev.org/airship/airshipctl/pkg/api/v1alpha1"
 	"opendev.org/airship/airshipctl/pkg/log"
+	"opendev.org/airship/airshipctl/pkg/util"
 )
 
 // ClientV1Alpha1 provides airship generic container API
@@ -46,13 +48,15 @@ type ClientV1Alpha1FactoryFunc func(
 	resultsDir string,
 	input io.Reader,
 	output io.Writer,
-	conf *v1alpha1.GenericContainer) ClientV1Alpha1
+	conf *v1alpha1.GenericContainer,
+	targetPath string) ClientV1Alpha1
 
 type clientV1Alpha1 struct {
 	resultsDir string
 	input      io.Reader
 	output     io.Writer
 	conf       *v1alpha1.GenericContainer
+	targetPath string
 
 	containerFunc containerFunc
 }
@@ -64,18 +68,22 @@ func NewClientV1Alpha1(
 	resultsDir string,
 	input io.Reader,
 	output io.Writer,
-	conf *v1alpha1.GenericContainer) ClientV1Alpha1 {
+	conf *v1alpha1.GenericContainer,
+	targetPath string) ClientV1Alpha1 {
 	return &clientV1Alpha1{
 		resultsDir:    resultsDir,
 		output:        output,
 		input:         input,
 		conf:          conf,
 		containerFunc: NewContainer,
+		targetPath:    targetPath,
 	}
 }
 
-// Run will peform container run action based on the configuration
+// Run will perform container run action based on the configuration
 func (c *clientV1Alpha1) Run() error {
+	// expand Src paths for mount if they are relative
+	ExpandSourceMounts(c.conf.Spec.StorageMounts, c.targetPath)
 	// set default runtime
 	switch c.conf.Spec.Type {
 	case v1alpha1.GenericContainerTypeAirship, "":
@@ -83,7 +91,7 @@ func (c *clientV1Alpha1) Run() error {
 	case v1alpha1.GenericContainerTypeKrm:
 		return c.runKRM()
 	default:
-		return fmt.Errorf("uknown generic container type %s", c.conf.Spec.Type)
+		return fmt.Errorf("unknown generic container type %s", c.conf.Spec.Type)
 	}
 }
 
@@ -275,3 +283,16 @@ func convertDockerMount(airMounts []v1alpha1.StorageMount) (mounts []Mount) {
 	}
 	return mounts
 }
+
+// ExpandSourceMounts converts relative paths into absolute ones
+func ExpandSourceMounts(storageMounts []v1alpha1.StorageMount, targetPath string) {
+	for i, mount := range storageMounts {
+		// Try to expand Src path
+		path := util.ExpandTilde(mount.Src)
+		// If still relative - add targetPath prefix
+		if !filepath.IsAbs(path) {
+			path = filepath.Join(targetPath, mount.Src)
+		}
+		storageMounts[i].Src = path
+	}
+}

pkg/container/api_test.go

@@ -19,6 +19,7 @@ import (
 	"context"
 	"io"
 	"io/ioutil"
+	"path/filepath"
 	"testing"
 
 	"github.com/docker/docker/api/types"
@@ -29,6 +30,7 @@ import (
 	"opendev.org/airship/airshipctl/pkg/api/v1alpha1"
 	"opendev.org/airship/airshipctl/pkg/document"
 	"opendev.org/airship/airshipctl/pkg/phase/ifc"
+	"opendev.org/airship/airshipctl/pkg/util"
 )
 
 func bundlePathToInput(t *testing.T, bundlePath string) io.Reader {
@@ -56,7 +58,7 @@ func TestGenericContainer(t *testing.T) {
 	}{
 		{
 			name:        "error unknown container type",
-			expectedErr: "uknown generic container type",
+			expectedErr: "unknown generic container type",
 			containerAPI: &v1alpha1.GenericContainer{
 				Spec: v1alpha1.GenericContainerSpec{
 					Type: "unknown",
@@ -65,7 +67,7 @@ func TestGenericContainer(t *testing.T) {
 			execFunc: NewContainer,
 		},
 		{
-			name: "error kyaml cant parse config",
+			name: "error kyaml can't parse config",
 			containerAPI: &v1alpha1.GenericContainer{
 				Spec: v1alpha1.GenericContainerSpec{
 					Type: v1alpha1.GenericContainerTypeKrm,
@@ -145,6 +147,11 @@ func TestGenericContainer(t *testing.T) {
 							Src:       "test",
 							DstPath:   "/mount",
 						},
+						{
+							MountType: "bind",
+							Src:       "~/test",
+							DstPath:   "/mnt",
+						},
 					},
 				},
 				Config: `kind: ConfigMap`,
@@ -234,6 +241,42 @@ func TestGenericContainer(t *testing.T) {
 
 // Dummy test to keep up with coverage.
 func TestNewClientV1alpha1(t *testing.T) {
-	client := NewClientV1Alpha1("", nil, nil, v1alpha1.DefaultGenericContainer())
+	client := NewClientV1Alpha1("", nil, nil, v1alpha1.DefaultGenericContainer(), "")
 	require.NotNil(t, client)
 }
+
+func TestExpandSourceMounts(t *testing.T) {
+	tests := []struct {
+		name           string
+		inputMounts    []v1alpha1.StorageMount
+		targetPath     string
+		expectedMounts []v1alpha1.StorageMount
+	}{
+		{
+			name:           "absolute path",
+			inputMounts:    []v1alpha1.StorageMount{{Src: "/test"}},
+			targetPath:     "/target-path",
+			expectedMounts: []v1alpha1.StorageMount{{Src: "/test"}},
+		},
+		{
+			name:           "tilde path",
+			inputMounts:    []v1alpha1.StorageMount{{Src: "~/test"}},
+			targetPath:     "/target-path",
+			expectedMounts: []v1alpha1.StorageMount{{Src: util.ExpandTilde("~/test")}},
+		},
+		{
+			name:           "relative path",
+			inputMounts:    []v1alpha1.StorageMount{{Src: "test"}},
+			targetPath:     "/target-path",
+			expectedMounts: []v1alpha1.StorageMount{{Src: filepath.Join("/target-path", "test")}},
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			ExpandSourceMounts(tt.inputMounts, tt.targetPath)
+			require.Equal(t, tt.expectedMounts, tt.inputMounts)
+		})
+	}
+}

pkg/k8s/kubeconfig/builder.go

@@ -187,7 +187,7 @@ func (b *Builder) trySource(clusterID, dstContext string, source v1alpha1.Kubeco
 		getter = b.fromClusterAPI(clusterID, source.ClusterAPI)
 	default:
 		// TODO add validation for fast fails to clustermap interface instead of this
-		return nil, &ErrUknownKubeconfigSourceType{Type: string(source.Type)}
+		return nil, &ErrUnknownKubeconfigSourceType{Type: string(source.Type)}
 	}
 	kubeBytes, err := getter()
 	if err != nil {

pkg/k8s/kubeconfig/errors.go

@@ -40,11 +40,11 @@ func IsErrAllSourcesFailedErr(err error) bool {
 	return ok
 }
 
-// ErrUknownKubeconfigSourceType returned type of kubeconfig source is unknown
+// ErrUnknownKubeconfigSourceType returned type of kubeconfig source is unknown
-type ErrUknownKubeconfigSourceType struct {
+type ErrUnknownKubeconfigSourceType struct {
 	Type string
 }
 
-func (e *ErrUknownKubeconfigSourceType) Error() string {
+func (e *ErrUnknownKubeconfigSourceType) Error() string {
 	return fmt.Sprintf("unknown source type %s", e.Type)
 }

pkg/k8s/kubeconfig/kubeconfig.go

@@ -113,10 +113,7 @@ func FromSecret(c client.Interface, o *client.GetKubeconfigOptions) KubeSourceFu
 // FromFile returns KubeSource type, uses path to kubeconfig on FS as source to construct kubeconfig object
 func FromFile(path string, fSys fs.FileSystem) KubeSourceFunc {
 	return func() ([]byte, error) {
-		expandedPath, err := util.ExpandTilde(path)
+		expandedPath := util.ExpandTilde(path)
-		if err != nil {
-			return nil, err
-		}
 		if fSys == nil {
 			fSys = fs.NewDocumentFs()
 		}

pkg/phase/executors/container.go

@@ -124,7 +124,7 @@ func (c *ContainerExecutor) Run(evtCh chan events.Event, opts ifc.RunOptions) {
 		return
 	}
 
-	err = c.ClientFunc(c.ResultsDir, input, output, c.Container).Run()
+	err = c.ClientFunc(c.ResultsDir, input, output, c.Container, c.Helper.TargetPath()).Run()
 	if err != nil {
 		handleError(evtCh, err)
 		return

pkg/phase/executors/container_test.go

@@ -48,11 +48,15 @@ const (
       src: /home/ubuntu/mounts
       dst: /my-mounts
       rw: true
+    - type: bind
+      src: ~/mounts
+      dst: /my-tilde-mount
+      rw: true
   config: |
     apiVersion: v1
     kind: ConfigMap
     metadata:
-      name: my-srange-name
+      name: my-strange-name
     data:
       cmd: encrypt
       unencrypted-regex: '^(kind|apiVersion|group|metadata)$'`
@@ -114,7 +118,7 @@ func TestGenericContainer(t *testing.T) {
 	}{
 		{
 			name:        "error unknown container type",
-			expectedErr: "uknown generic container type",
+			expectedErr: "unknown generic container type",
 			containerAPI: &v1alpha1.GenericContainer{
 				Spec: v1alpha1.GenericContainerSpec{
 					Type: "unknown",
@@ -202,7 +206,7 @@ type: Opaque
 			ch := make(chan events.Event)
 			go container.Run(ch, tt.runOptions)
 
-			var actualEvt []events.Event
+			actualEvt := make([]events.Event, 0)
 			for evt := range ch {
 				actualEvt = append(actualEvt, evt)
 			}
@@ -275,7 +279,7 @@ type fakeKubeConfig struct {
 
 func (k fakeKubeConfig) GetFile() (string, kubeconfig.Cleanup, error) { return k.getFile() }
 func (k fakeKubeConfig) Write(_ io.Writer) error                      { return nil }
-func (k fakeKubeConfig) WriteFile(path string) error                  { return nil }
+func (k fakeKubeConfig) WriteFile(_ string) error                     { return nil }
-func (k fakeKubeConfig) WriteTempFile(dumpRoot string) (string, kubeconfig.Cleanup, error) {
+func (k fakeKubeConfig) WriteTempFile(_ string) (string, kubeconfig.Cleanup, error) {
 	return k.getFile()
 }

pkg/util/errors.go

@@ -1,26 +0,0 @@
-/*
- Licensed under the Apache License, Version 2.0 (the "License");
- you may not use this file except in compliance with the License.
- You may obtain a copy of the License at
-
-     https://www.apache.org/licenses/LICENSE-2.0
-
- Unless required by applicable law or agreed to in writing, software
- distributed under the License is distributed on an "AS IS" BASIS,
- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- See the License for the specific language governing permissions and
- limitations under the License.
-*/
-
-package util
-
-import "fmt"
-
-// ErrCantExpandTildePath returned when malformed path is passed to ExpandTilde function
-type ErrCantExpandTildePath struct {
-	Path string
-}
-
-func (e *ErrCantExpandTildePath) Error() string {
-	return fmt.Sprintf("cannot expand user-specific home dir: %s", e.Path)
-}

pkg/util/homedir.go

@@ -17,6 +17,7 @@ package util
 import (
 	"os"
 	"path/filepath"
+	"strings"
 )
 
 // UserHomeDir is a utility function that wraps os.UserHomeDir and returns no
@@ -31,21 +32,18 @@ func UserHomeDir() string {
 }
 
 // ExpandTilde expands the path to include the home directory if the path
-// is prefixed with `~`. If it isn't prefixed with `~`, the path is
+// is prefixed with `~`. If it isn't prefixed with `~` or has no slash after tilde,
-// returned as-is.
+// the path is returned as-is.
-// Original source code: https://github.com/mitchellh/go-homedir/blob/master/homedir.go#L55-L77
+func ExpandTilde(path string) string {
-func ExpandTilde(path string) (string, error) {
+	// Just tilde - return current $HOME dir
-	if len(path) == 0 {
+	if path == "~" {
-		return path, nil
+		return UserHomeDir()
+	}
+	// If path starts with ~/ - expand it
+	if strings.HasPrefix(path, "~/") {
+		return filepath.Join(UserHomeDir(), path[1:])
 	}
 
-	if path[0] != '~' {
+	// empty strings, absolute paths, ~<(dir/file)name> return as-is
-		return path, nil
+	return path
-	}
-
-	if len(path) > 1 && path[1] != '/' {
-		return "", &ErrCantExpandTildePath{}
-	}
-
-	return filepath.Join(UserHomeDir(), path[1:]), nil
 }

pkg/util/homedir_test.go

@@ -16,11 +16,11 @@ package util_test
 
 import (
 	"os"
-	"path"
 	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"opendev.org/airship/airshipctl/pkg/util"
 	"opendev.org/airship/airshipctl/testutil"
@@ -45,33 +45,42 @@ func setHome(path string) (resetHome func()) {
 }
 
 func TestExpandTilde(t *testing.T) {
-	t.Run("success home path", func(t *testing.T) {
+	tests := []struct {
-		airshipDir := ".airship"
+		name        string
-		dir := filepath.Join("~", airshipDir)
+		input       string
-		expandedDir, err := util.ExpandTilde(dir)
+		expectedOut string
-		assert.NoError(t, err)
+	}{
-		homedir := path.Join(util.UserHomeDir(), airshipDir)
+		{
-		assert.Equal(t, homedir, expandedDir)
+			name:        "expand home path",
-	})
+			input:       "~/.airship",
+			expectedOut: filepath.Join(util.UserHomeDir(), "~/.airship"[1:]),
+		},
+		{
+			name:        "expand just ~",
+			input:       "~",
+			expectedOut: util.UserHomeDir(),
+		},
+		{
+			name:        "empty path",
+			input:       "",
+			expectedOut: "",
+		},
+		{
+			name:        "absolute path",
+			input:       "/.airship",
+			expectedOut: "/.airship",
+		},
+		{
+			name:        "tilde path",
+			input:       "~.airship",
+			expectedOut: "~.airship",
+		},
+	}
 
-	t.Run("success nothing to expand", func(t *testing.T) {
+	for _, tt := range tests {
-		dir := "/home/ubuntu/.airship"
+		tt := tt
-		expandedDir, err := util.ExpandTilde(dir)
+		t.Run(tt.name, func(t *testing.T) {
-		assert.NoError(t, err)
+			require.Equal(t, tt.expectedOut, util.ExpandTilde(tt.input))
-		assert.Equal(t, dir, expandedDir)
+		})
-	})
+	}
-
-	t.Run("error malformed path", func(t *testing.T) {
-		malformedDir := "~.home/ubuntu/.airship"
-		expandedDir, err := util.ExpandTilde(malformedDir)
-		assert.Error(t, err)
-		assert.Equal(t, "", expandedDir)
-	})
-
-	t.Run("success empty path", func(t *testing.T) {
-		emptyDir := ""
-		expandedDir, err := util.ExpandTilde(emptyDir)
-		assert.NoError(t, err)
-		assert.Equal(t, emptyDir, expandedDir)
-	})
 }