airship/airshipctl
Code Issues Proposed changes

Merge "Allow relative and ~ path for container mount"

Browse Source
This commit is contained in:
Zuul 2021-03-26 21:57:56 +00:00 committed by Gerrit Code Review
commit bde1998648
11 changed files with 142 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
pkg/api/v1alpha1/genericcontainer_types.go
View File

@ -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.

29
pkg/container/api.go
View File

@ -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
}
}

49
pkg/container/api_test.go
View File

@ -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)
})
}
}

2
pkg/k8s/kubeconfig/builder.go
View File

@ -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 {

6
pkg/k8s/kubeconfig/errors.go
View File

@ -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)
}

5
pkg/k8s/kubeconfig/kubeconfig.go
View File

@ -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()
}

2
pkg/phase/executors/container.go
View File

@ -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

14
pkg/phase/executors/container_test.go
View File

@ -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()
}

26
pkg/util/errors.go
View File

@ -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)
}

28
pkg/util/homedir.go
View File

@ -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
}

65
pkg/util/homedir_test.go
View File

@ -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))
})
}
}