diff --git a/pkg/api/v1alpha1/genericcontainer_types.go b/pkg/api/v1alpha1/genericcontainer_types.go index 6d22204f8..60b8db72d 100644 --- a/pkg/api/v1alpha1/genericcontainer_types.go +++ b/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. diff --git a/pkg/container/api.go b/pkg/container/api.go index d0457e310..79e30158a 100644 --- a/pkg/container/api.go +++ b/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 + } +} diff --git a/pkg/container/api_test.go b/pkg/container/api_test.go index dcb17e295..a7ee0c7a7 100644 --- a/pkg/container/api_test.go +++ b/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) + }) + } +} diff --git a/pkg/k8s/kubeconfig/builder.go b/pkg/k8s/kubeconfig/builder.go index f75318f6a..efb91fb71 100644 --- a/pkg/k8s/kubeconfig/builder.go +++ b/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 { diff --git a/pkg/k8s/kubeconfig/errors.go b/pkg/k8s/kubeconfig/errors.go index 12ad3a68c..b37909037 100644 --- a/pkg/k8s/kubeconfig/errors.go +++ b/pkg/k8s/kubeconfig/errors.go @@ -40,11 +40,11 @@ func IsErrAllSourcesFailedErr(err error) bool { return ok } -// ErrUknownKubeconfigSourceType returned type of kubeconfig source is unknown -type ErrUknownKubeconfigSourceType struct { +// ErrUnknownKubeconfigSourceType returned type of kubeconfig source is unknown +type ErrUnknownKubeconfigSourceType struct { Type string } -func (e *ErrUknownKubeconfigSourceType) Error() string { +func (e *ErrUnknownKubeconfigSourceType) Error() string { return fmt.Sprintf("unknown source type %s", e.Type) } diff --git a/pkg/k8s/kubeconfig/kubeconfig.go b/pkg/k8s/kubeconfig/kubeconfig.go index 05745b036..bd84b81c2 100644 --- a/pkg/k8s/kubeconfig/kubeconfig.go +++ b/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) - if err != nil { - return nil, err - } + expandedPath := util.ExpandTilde(path) if fSys == nil { fSys = fs.NewDocumentFs() } diff --git a/pkg/phase/executors/container.go b/pkg/phase/executors/container.go index 7fa20019e..88e85df69 100644 --- a/pkg/phase/executors/container.go +++ b/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 diff --git a/pkg/phase/executors/container_test.go b/pkg/phase/executors/container_test.go index be7ffa438..754a0d9a9 100644 --- a/pkg/phase/executors/container_test.go +++ b/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) WriteTempFile(dumpRoot string) (string, kubeconfig.Cleanup, error) { +func (k fakeKubeConfig) WriteFile(_ string) error { return nil } +func (k fakeKubeConfig) WriteTempFile(_ string) (string, kubeconfig.Cleanup, error) { return k.getFile() } diff --git a/pkg/util/errors.go b/pkg/util/errors.go deleted file mode 100644 index e596a2a82..000000000 --- a/pkg/util/errors.go +++ /dev/null @@ -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) -} diff --git a/pkg/util/homedir.go b/pkg/util/homedir.go index 197c4ddd0..09835d316 100644 --- a/pkg/util/homedir.go +++ b/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 -// returned as-is. -// Original source code: https://github.com/mitchellh/go-homedir/blob/master/homedir.go#L55-L77 -func ExpandTilde(path string) (string, error) { - if len(path) == 0 { - return path, nil +// is prefixed with `~`. If it isn't prefixed with `~` or has no slash after tilde, +// the path is returned as-is. +func ExpandTilde(path string) string { + // Just tilde - return current $HOME dir + if path == "~" { + return UserHomeDir() + } + // If path starts with ~/ - expand it + if strings.HasPrefix(path, "~/") { + return filepath.Join(UserHomeDir(), path[1:]) } - if path[0] != '~' { - return path, nil - } - - if len(path) > 1 && path[1] != '/' { - return "", &ErrCantExpandTildePath{} - } - - return filepath.Join(UserHomeDir(), path[1:]), nil + // empty strings, absolute paths, ~<(dir/file)name> return as-is + return path } diff --git a/pkg/util/homedir_test.go b/pkg/util/homedir_test.go index df99f40ec..804948a84 100644 --- a/pkg/util/homedir_test.go +++ b/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) { - airshipDir := ".airship" - dir := filepath.Join("~", airshipDir) - expandedDir, err := util.ExpandTilde(dir) - assert.NoError(t, err) - homedir := path.Join(util.UserHomeDir(), airshipDir) - assert.Equal(t, homedir, expandedDir) - }) + tests := []struct { + name string + input string + expectedOut string + }{ + { + 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) { - dir := "/home/ubuntu/.airship" - expandedDir, err := util.ExpandTilde(dir) - assert.NoError(t, err) - 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) - }) + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.expectedOut, util.ExpandTilde(tt.input)) + }) + } }