Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the file sync destination more consistent #3662

Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Make the file sync destination more consistent
Signed-off-by: John Collier <John.J.Collier@ibm.com>
johnmcollier committed Jul 30, 2020
commit 00b21bac94360931adc7fc98157ba6354f82642d
15 changes: 13 additions & 2 deletions pkg/devfile/adapters/common/utils.go
Original file line number Diff line number Diff line change
@@ -61,8 +61,8 @@ const (
// Default volume size for volumes defined in a devfile
volumeSize = "5Gi"

// EnvCheProjectsRoot is the env defined for /projects where component mountSources=true
EnvCheProjectsRoot = "CHE_PROJECTS_ROOT"
// EnvProjectsRoot is the env defined for /projects where component mountSources=true
EnvProjectsRoot = "PROJECTS_ROOT"

// EnvOdoCommandRunWorkingDir is the env defined in the runtime component container which holds the work dir for the run command
EnvOdoCommandRunWorkingDir = "ODO_COMMAND_RUN_WORKING_DIR"
@@ -235,3 +235,14 @@ func GetCommandsMap(commands []common.DevfileCommand) map[string]common.DevfileC
}
return commandsMap
}

// GetComponentEnvVar returns true if a list of env vars contains the specified env var
// If the env exists, it returns the value of it
func GetComponentEnvVar(env string, envs []common.Env) string {
for _, envVar := range envs {
if envVar.Name == env {
return envVar.Value
}
}
return ""
}
65 changes: 65 additions & 0 deletions pkg/devfile/adapters/common/utils_test.go
Original file line number Diff line number Diff line change
@@ -587,3 +587,68 @@ func TestGetCommandsMap(t *testing.T) {
}

}

func TestGetComponentEnvVar(t *testing.T) {

tests := []struct {
name string
env string
envs []common.Env
want string
}{
{
name: "Case 1: No env vars",
env: "test",
envs: nil,
want: "",
},
{
name: "Case 2: Has env",
env: "PROJECTS_ROOT",
envs: []common.Env{
{
Name: "SOME_ENV",
Value: "test",
},
{
Name: "TESTER",
Value: "tester",
},
{
Name: "PROJECTS_ROOT",
Value: "/test",
},
},
want: "/test",
},
{
name: "Case 3: No env, multiple values",
env: "PROJECTS_ROOT",
envs: []common.Env{
{
Name: "TESTER",
Value: "fake",
},
{
Name: "FAKE",
Value: "fake",
},
{
Name: "ENV",
Value: "fake",
},
},
want: "",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
value := GetComponentEnvVar(tt.env, tt.envs)
if value != tt.want {
t.Errorf("TestGetComponentEnvVar error: env value mismatch, expected: %v got: %v", tt.want, value)
}

})
}

}
15 changes: 9 additions & 6 deletions pkg/devfile/adapters/docker/component/utils.go
Original file line number Diff line number Diff line change
@@ -200,15 +200,18 @@ func (a Adapter) startComponent(mounts []mount.Mount, comp versionsCommon.Devfil

// If the component set `mountSources` to true, add the source volume and env CHE_PROJECTS_ROOT to it
if comp.Container.MountSources {
var syncFolder, projectsRoot string
if comp.Container.SourceMapping != "" {
utils.AddVolumeToContainer(a.projectVolumeName, comp.Container.SourceMapping, &hostConfig)
syncFolder = comp.Container.SourceMapping
} else if projectsRoot = common.GetComponentEnvVar(common.EnvProjectsRoot, comp.Container.Env); projectsRoot != "" {
syncFolder = projectsRoot
} else {
utils.AddVolumeToContainer(a.projectVolumeName, lclient.OdoSourceVolumeMount, &hostConfig)
syncFolder = lclient.OdoSourceVolumeMount
}

if !common.IsEnvPresent(comp.Container.Env, common.EnvCheProjectsRoot) {
envName := common.EnvCheProjectsRoot
envValue := lclient.OdoSourceVolumeMount
utils.AddVolumeToContainer(a.projectVolumeName, syncFolder, &hostConfig)
if projectsRoot != "" {
envName := common.EnvProjectsRoot
envValue := syncFolder
comp.Container.Env = append(comp.Container.Env, versionsCommon.Env{
Name: envName,
Value: envValue,
8 changes: 5 additions & 3 deletions pkg/devfile/adapters/kubernetes/utils/utils.go
Original file line number Diff line number Diff line change
@@ -83,9 +83,11 @@ func GetContainers(devfileObj devfileParser.DevfileObj) ([]corev1.Container, err
// If `mountSources: true` was set, add an empty dir volume to the container to sync the source to
// Sync to `Container.SourceMapping` if set
if comp.Container.MountSources {
var syncFolder string
var syncFolder, projectsRoot string
if comp.Container.SourceMapping != "" {
syncFolder = comp.Container.SourceMapping
} else if projectsRoot = adaptersCommon.GetComponentEnvVar(adaptersCommon.EnvProjectsRoot, comp.Container.Env); projectsRoot != "" {
syncFolder = projectsRoot
} else {
syncFolder = kclient.OdoSourceVolumeMount
}
@@ -96,10 +98,10 @@ func GetContainers(devfileObj devfileParser.DevfileObj) ([]corev1.Container, err
})

// only add the env if it is not set by the devfile
if !isEnvPresent(container.Env, adaptersCommon.EnvCheProjectsRoot) {
if projectsRoot == "" {
container.Env = append(container.Env,
corev1.EnvVar{
Name: adaptersCommon.EnvCheProjectsRoot,
Name: adaptersCommon.EnvProjectsRoot,
Value: syncFolder,
})
}
38 changes: 1 addition & 37 deletions pkg/sync/adapter.go
Original file line number Diff line number Diff line change
@@ -7,7 +7,6 @@ import (
"strings"

"github.com/openshift/odo/pkg/devfile/adapters/common"
versionsCommon "github.com/openshift/odo/pkg/devfile/parser/data/common"
"github.com/openshift/odo/pkg/exec"
"github.com/openshift/odo/pkg/kclient"
"github.com/openshift/odo/pkg/log"
@@ -157,17 +156,7 @@ func (a Adapter) pushLocal(path string, files []string, delFiles []string, isFor
s := log.Spinner("Syncing files to the component")
defer s.End(false)

// Determine which folder we need to sync to
var syncFolder string
if compInfo.SourceMount != kclient.OdoSourceVolumeMount {
syncFolder = compInfo.SourceMount
} else {
// If there's only one project defined in the devfile, sync to `/projects/project-name`, otherwise sync to /projects
syncFolder, err = getSyncFolder(a.Devfile.Data.GetProjects())
if err != nil {
return errors.Wrapf(err, "unable to determine sync folder")
}
}
syncFolder := compInfo.SourceMount

if syncFolder != kclient.OdoSourceVolumeMount {
// Need to make sure the folder already exists on the component or else sync will fail
@@ -210,31 +199,6 @@ func (a Adapter) pushLocal(path string, files []string, delFiles []string, isFor
return nil
}

// getSyncFolder returns the folder that we need to sync the source files to
// If there's exactly one project defined in the devfile, and clonePath isn't set return `/projects/<projectName>`
// If there's exactly one project, and clonePath is set, return `/projects/<clonePath>`
// If the clonePath is an absolute path or contains '..', return an error
// Otherwise (zero projects or many), return `/projects`
func getSyncFolder(projects []versionsCommon.DevfileProject) (string, error) {
if len(projects) == 1 {
project := projects[0]
// If the clonepath is set to a value, set it to be the sync folder
// As some devfiles rely on the code being synced to the folder in the clonepath
if project.ClonePath != "" {
if strings.HasPrefix(project.ClonePath, "/") {
return "", fmt.Errorf("the clonePath in the devfile must be a relative path")
}
if strings.Contains(project.ClonePath, "..") {
return "", fmt.Errorf("the clonePath in the devfile cannot escape the projects root. Don't use .. to try and do that")
}
return filepath.ToSlash(filepath.Join(kclient.OdoSourceVolumeMount, project.ClonePath)), nil
}
return filepath.ToSlash(filepath.Join(kclient.OdoSourceVolumeMount, projects[0].Name)), nil
}
return kclient.OdoSourceVolumeMount, nil

}

// getCmdToCreateSyncFolder returns the command used to create the remote sync folder on the running container
func getCmdToCreateSyncFolder(syncFolder string) []string {
return []string{"mkdir", "-p", syncFolder}
126 changes: 0 additions & 126 deletions pkg/sync/adapter_test.go
Original file line number Diff line number Diff line change
@@ -17,132 +17,6 @@ import (
"github.com/openshift/odo/tests/helper"
)

func TestGetSyncFolder(t *testing.T) {
projectNames := []string{"some-name", "another-name"}
projectRepos := []string{"https://github.com/some/repo.git", "https://github.com/another/repo.git"}
projectClonePath := "src/github.com/golang/example/"
invalidClonePaths := []string{"/var", "../var", "pkg/../../var"}

tests := []struct {
name string
projects []versionsCommon.DevfileProject
want string
wantErr bool
}{
{
name: "Case 1: No projects",
projects: []versionsCommon.DevfileProject{},
want: kclient.OdoSourceVolumeMount,
wantErr: false,
},
{
name: "Case 2: One project",
projects: []versionsCommon.DevfileProject{
{
Name: projectNames[0],
Git: &versionsCommon.Git{
Location: projectRepos[0],
},
},
},
want: filepath.ToSlash(filepath.Join(kclient.OdoSourceVolumeMount, projectNames[0])),
wantErr: false,
},
{
name: "Case 3: Multiple projects",
projects: []versionsCommon.DevfileProject{
{
Name: projectNames[0],
Git: &versionsCommon.Git{
Location: projectRepos[0],
},
},
{
Name: projectNames[1],
Github: &versionsCommon.Github{
Location: projectRepos[1],
},
},
{
Name: projectNames[1],
Zip: &versionsCommon.Zip{
Location: projectRepos[1],
},
},
},
want: kclient.OdoSourceVolumeMount,
wantErr: false,
},
{
name: "Case 4: Clone path set",
projects: []versionsCommon.DevfileProject{
{
ClonePath: projectClonePath,
Name: projectNames[0],
Zip: &versionsCommon.Zip{
Location: projectRepos[0],
},
},
},
want: filepath.ToSlash(filepath.Join(kclient.OdoSourceVolumeMount, projectClonePath)),
wantErr: false,
},
{
name: "Case 5: Invalid clone path, set with absolute path",
projects: []versionsCommon.DevfileProject{
{
ClonePath: invalidClonePaths[0],
Name: projectNames[0],
Github: &versionsCommon.Github{
Location: projectRepos[0],
},
},
},
want: "",
wantErr: true,
},
{
name: "Case 6: Invalid clone path, starts with ..",
projects: []versionsCommon.DevfileProject{
{
ClonePath: invalidClonePaths[1],
Name: projectNames[0],
Git: &versionsCommon.Git{
Location: projectRepos[0],
},
},
},
want: "",
wantErr: true,
},
{
name: "Case 7: Invalid clone path, contains ..",
projects: []versionsCommon.DevfileProject{
{
ClonePath: invalidClonePaths[2],
Name: projectNames[0],
Zip: &versionsCommon.Zip{
Location: projectRepos[0],
},
},
},
want: "",
wantErr: true,
},
}
for _, tt := range tests {
syncFolder, err := getSyncFolder(tt.projects)

if !tt.wantErr == (err != nil) {
t.Errorf("expected %v, actual %v", tt.wantErr, err)
}

if syncFolder != tt.want {
t.Errorf("expected %s, actual %s", tt.want, syncFolder)
}
}
}

func TestGetCmdToCreateSyncFolder(t *testing.T) {
tests := []struct {
name string