Commit f4645bfb authored by Kamil Trzciński's avatar Kamil Trzciński 🔴

Strong validate that `GIT_CLONE_PATH` is used within `CI_BUILDS_DIR`.

parent c9bd231f
...@@ -321,6 +321,7 @@ func TestDefaultExecutorConfiguration(t *testing.T) { ...@@ -321,6 +321,7 @@ func TestDefaultExecutorConfiguration(t *testing.T) {
Port: "22", Port: "22",
IdentityFile: "/home/user/.ssh/id_rsa", IdentityFile: "/home/user/.ssh/id_rsa",
}, },
CustomBuildDir: nil,
}, },
}, },
}, },
...@@ -341,6 +342,7 @@ func TestDefaultExecutorConfiguration(t *testing.T) { ...@@ -341,6 +342,7 @@ func TestDefaultExecutorConfiguration(t *testing.T) {
Parallels: &common.ParallelsConfig{ Parallels: &common.ParallelsConfig{
BaseName: "my-parallels-vm", BaseName: "my-parallels-vm",
}, },
CustomBuildDir: nil,
}, },
}, },
}, },
...@@ -362,6 +364,7 @@ func TestDefaultExecutorConfiguration(t *testing.T) { ...@@ -362,6 +364,7 @@ func TestDefaultExecutorConfiguration(t *testing.T) {
VirtualBox: &common.VirtualBoxConfig{ VirtualBox: &common.VirtualBoxConfig{
BaseName: "my-virtualbox-vm", BaseName: "my-virtualbox-vm",
}, },
CustomBuildDir: nil,
}, },
}, },
}, },
......
...@@ -160,24 +160,42 @@ func (b *Build) TmpProjectDir() string { ...@@ -160,24 +160,42 @@ func (b *Build) TmpProjectDir() string {
return helpers.ToSlash(b.BuildDir) + ".tmp" return helpers.ToSlash(b.BuildDir) + ".tmp"
} }
func (b *Build) getCustomBuildDir(rootDir, overrideKey string, customDirAllowed, sharedDir bool) (string, error) {
dir := b.GetAllVariables().Get(overrideKey)
if dir != "" {
if !customDirAllowed {
return "", MakeBuildError("setting %s is not allowed, enable `custom_build_dir` feature", overrideKey)
}
if !strings.HasPrefix(dir, rootDir) {
return "", MakeBuildError("the %s=%q has to be within %q",
overrideKey, dir, rootDir)
}
}
if dir == "" {
dir = path.Join(rootDir, b.ProjectUniqueDir(sharedDir))
}
return dir, nil
}
func (b *Build) StartBuild(rootDir, cacheDir string, customDirAllowed, sharedDir bool) error { func (b *Build) StartBuild(rootDir, cacheDir string, customDirAllowed, sharedDir bool) error {
var err error
// We set RootDir and invalidate variables
// to be able to use CI_BUILDS_DIR
b.RootDir = rootDir b.RootDir = rootDir
b.CacheDir = path.Join(cacheDir, b.ProjectUniqueDir(false)) b.CacheDir = path.Join(cacheDir, b.ProjectUniqueDir(false))
b.refreshAllVariables()
// Job Specific build dir b.BuildDir, err = b.getCustomBuildDir(b.RootDir, "GIT_CLONE_PATH", customDirAllowed, sharedDir)
b.BuildDir = b.GetAllVariables().Get("GIT_CLONE_PATH") if err != nil {
if b.BuildDir != "" && !customDirAllowed { return err
return errors.New("setting GIT_CLONE_PATH is not allowed, enable `custom_build_dir` feature")
}
if b.BuildDir == "" {
b.BuildDir = path.Join(rootDir, b.ProjectUniqueDir(sharedDir))
} }
// invalidate variables cache: // We invalidate variables to be able to use
// as some variables are based on dynamic // CI_CACHE_DIR and CI_PROJECT_DIR
// state after build starts b.refreshAllVariables()
b.allVariables = nil
return nil return nil
} }
...@@ -546,6 +564,7 @@ func (b *Build) String() string { ...@@ -546,6 +564,7 @@ func (b *Build) String() string {
func (b *Build) GetDefaultVariables() JobVariables { func (b *Build) GetDefaultVariables() JobVariables {
return JobVariables{ return JobVariables{
{Key: "CI_BUILDS_DIR", Value: filepath.FromSlash(b.RootDir), Public: true, Internal: true, File: false},
{Key: "CI_PROJECT_DIR", Value: filepath.FromSlash(b.FullProjectDir()), Public: true, Internal: true, File: false}, {Key: "CI_PROJECT_DIR", Value: filepath.FromSlash(b.FullProjectDir()), Public: true, Internal: true, File: false},
{Key: "CI_CONCURRENT_ID", Value: strconv.Itoa(b.RunnerID), Public: true, Internal: true, File: false}, {Key: "CI_CONCURRENT_ID", Value: strconv.Itoa(b.RunnerID), Public: true, Internal: true, File: false},
{Key: "CI_CONCURRENT_PROJECT_ID", Value: strconv.Itoa(b.ProjectRunnerID), Public: true, Internal: true, File: false}, {Key: "CI_CONCURRENT_PROJECT_ID", Value: strconv.Itoa(b.ProjectRunnerID), Public: true, Internal: true, File: false},
...@@ -616,6 +635,10 @@ func (b *Build) IsSharedEnv() bool { ...@@ -616,6 +635,10 @@ func (b *Build) IsSharedEnv() bool {
return b.ExecutorFeatures.Shared return b.ExecutorFeatures.Shared
} }
func (b *Build) refreshAllVariables() {
b.allVariables = nil
}
func (b *Build) GetAllVariables() JobVariables { func (b *Build) GetAllVariables() JobVariables {
if b.allVariables != nil { if b.allVariables != nil {
return b.allVariables return b.allVariables
......
...@@ -909,16 +909,14 @@ func TestStartBuild(t *testing.T) { ...@@ -909,16 +909,14 @@ func TestStartBuild(t *testing.T) {
sharedDir bool sharedDir bool
} }
tests := []struct { tests := map[string]struct {
name string
args startBuildArgs args startBuildArgs
jobVariables JobVariables jobVariables JobVariables
expectedBuildDir string expectedBuildDir string
expectedCacheDir string expectedCacheDir string
expectedError bool expectedError bool
}{ }{
{ "no job specific build dir with no shared dir": {
name: "no job specific build dir with no shared dir",
args: startBuildArgs{ args: startBuildArgs{
rootDir: "/build", rootDir: "/build",
cacheDir: "/cache", cacheDir: "/cache",
...@@ -930,8 +928,7 @@ func TestStartBuild(t *testing.T) { ...@@ -930,8 +928,7 @@ func TestStartBuild(t *testing.T) {
expectedCacheDir: "/cache/test-namespace/test-repo", expectedCacheDir: "/cache/test-namespace/test-repo",
expectedError: false, expectedError: false,
}, },
{ "no job specified build dir with shared dir": {
name: "no job specified build dir with shared dir",
args: startBuildArgs{ args: startBuildArgs{
rootDir: "/builds", rootDir: "/builds",
cacheDir: "/cache", cacheDir: "/cache",
...@@ -943,8 +940,7 @@ func TestStartBuild(t *testing.T) { ...@@ -943,8 +940,7 @@ func TestStartBuild(t *testing.T) {
expectedCacheDir: "/cache/test-namespace/test-repo", expectedCacheDir: "/cache/test-namespace/test-repo",
expectedError: false, expectedError: false,
}, },
{ "valid GIT_CLONE_PATH was specified": {
name: "job specific build dir with no shared dir",
args: startBuildArgs{ args: startBuildArgs{
rootDir: "/builds", rootDir: "/builds",
cacheDir: "/cache", cacheDir: "/cache",
...@@ -952,29 +948,27 @@ func TestStartBuild(t *testing.T) { ...@@ -952,29 +948,27 @@ func TestStartBuild(t *testing.T) {
sharedDir: false, sharedDir: false,
}, },
jobVariables: JobVariables{ jobVariables: JobVariables{
{Key: "GIT_CLONE_PATH", Value: "/go/src/gitlab.com/test-namespace/test-repo", Public: true}, {Key: "GIT_CLONE_PATH", Value: "/builds/go/src/gitlab.com/test-namespace/test-repo", Public: true},
}, },
expectedBuildDir: "/go/src/gitlab.com/test-namespace/test-repo", expectedBuildDir: "/builds/go/src/gitlab.com/test-namespace/test-repo",
expectedCacheDir: "/cache/test-namespace/test-repo", expectedCacheDir: "/cache/test-namespace/test-repo",
expectedError: false, expectedError: false,
}, },
{ "valid GIT_CLONE_PATH using CI_BUILDS_DIR was specified": {
name: "job specific build dir with shared dir",
args: startBuildArgs{ args: startBuildArgs{
rootDir: "/builds", rootDir: "/builds",
cacheDir: "/cache", cacheDir: "/cache",
customBuildDir: true, customBuildDir: true,
sharedDir: true, sharedDir: false,
}, },
jobVariables: JobVariables{ jobVariables: JobVariables{
{Key: "GIT_CLONE_PATH", Value: "/go/src/gitlab.com/test-namespace/test-repo", Public: true}, {Key: "GIT_CLONE_PATH", Value: "$CI_BUILDS_DIR/go/src/gitlab.com/test-namespace/test-repo", Public: true},
}, },
expectedBuildDir: "/go/src/gitlab.com/test-namespace/test-repo", expectedBuildDir: "/builds/go/src/gitlab.com/test-namespace/test-repo",
expectedCacheDir: "/cache/test-namespace/test-repo", expectedCacheDir: "/cache/test-namespace/test-repo",
expectedError: false, expectedError: false,
}, },
{ "custom build disabled": {
name: "custom build disabled",
args: startBuildArgs{ args: startBuildArgs{
rootDir: "/builds", rootDir: "/builds",
cacheDir: "/cache", cacheDir: "/cache",
...@@ -982,22 +976,34 @@ func TestStartBuild(t *testing.T) { ...@@ -982,22 +976,34 @@ func TestStartBuild(t *testing.T) {
sharedDir: false, sharedDir: false,
}, },
jobVariables: JobVariables{ jobVariables: JobVariables{
{Key: "GIT_CLONE_PATH", Value: "/go/src/gitlab.com/test-namespace/test-repo", Public: true}, {Key: "GIT_CLONE_PATH", Value: "/builds/go/src/gitlab.com/test-namespace/test-repo", Public: true},
}, },
expectedBuildDir: "/builds/test-namespace/test-repo", expectedBuildDir: "/builds/test-namespace/test-repo",
expectedCacheDir: "/cache/test-namespace/test-repo", expectedCacheDir: "/cache/test-namespace/test-repo",
expectedError: true, expectedError: true,
}, },
"invalid GIT_CLONE_PATH was specified": {
args: startBuildArgs{
rootDir: "/builds",
cacheDir: "/cache",
customBuildDir: true,
sharedDir: false,
},
jobVariables: JobVariables{
{Key: "GIT_CLONE_PATH", Value: "/go/src/gitlab.com/test-namespace/test-repo", Public: true},
},
expectedError: true,
},
} }
for _, tt := range tests { for name, test := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(name, func(t *testing.T) {
build := Build{ build := Build{
JobResponse: JobResponse{ JobResponse: JobResponse{
GitInfo: GitInfo{ GitInfo: GitInfo{
RepoURL: "https://gitlab.com/test-namespace/test-repo.git", RepoURL: "https://gitlab.com/test-namespace/test-repo.git",
}, },
Variables: tt.jobVariables, Variables: test.jobVariables,
}, },
Runner: &RunnerConfig{ Runner: &RunnerConfig{
RunnerCredentials: RunnerCredentials{ RunnerCredentials: RunnerCredentials{
...@@ -1006,16 +1012,16 @@ func TestStartBuild(t *testing.T) { ...@@ -1006,16 +1012,16 @@ func TestStartBuild(t *testing.T) {
}, },
} }
err := build.StartBuild(tt.args.rootDir, tt.args.cacheDir, tt.args.customBuildDir, tt.args.sharedDir) err := build.StartBuild(test.args.rootDir, test.args.cacheDir, test.args.customBuildDir, test.args.sharedDir)
if tt.expectedError { if test.expectedError {
assert.Error(t, err) assert.Error(t, err)
return return
} }
assert.NoError(t, err) assert.NoError(t, err)
assert.Equal(t, tt.expectedBuildDir, build.BuildDir) assert.Equal(t, test.expectedBuildDir, build.BuildDir)
assert.Equal(t, tt.args.rootDir, build.RootDir) assert.Equal(t, test.args.rootDir, build.RootDir)
assert.Equal(t, tt.expectedCacheDir, build.CacheDir) assert.Equal(t, test.expectedCacheDir, build.CacheDir)
}) })
} }
} }
...@@ -1254,11 +1260,11 @@ func TestDefaultVariables(t *testing.T) { ...@@ -1254,11 +1260,11 @@ func TestDefaultVariables(t *testing.T) {
{ {
name: "get overwritten CI_PROJECT_DIR value", name: "get overwritten CI_PROJECT_DIR value",
jobVariables: JobVariables{ jobVariables: JobVariables{
{Key: "GIT_CLONE_PATH", Value: "/go/src/gitlab.com/gitlab-org/gitlab-runner", Public: true}, {Key: "GIT_CLONE_PATH", Value: "/builds/go/src/gitlab.com/gitlab-org/gitlab-runner", Public: true},
}, },
rootDir: "/builds", rootDir: "/builds",
key: "CI_PROJECT_DIR", key: "CI_PROJECT_DIR",
expectedValue: "/go/src/gitlab.com/gitlab-org/gitlab-runner", expectedValue: "/builds/go/src/gitlab.com/gitlab-org/gitlab-runner",
}, },
} }
......
...@@ -66,6 +66,12 @@ func (b *BuildError) Error() string { ...@@ -66,6 +66,12 @@ func (b *BuildError) Error() string {
return b.Inner.Error() return b.Inner.Error()
} }
func MakeBuildError(format string, args ...interface{}) error {
return &BuildError{
Inner: fmt.Errorf(format, args...),
}
}
var executors map[string]ExecutorProvider var executors map[string]ExecutorProvider
func validateExecutorProvider(provider ExecutorProvider) error { func validateExecutorProvider(provider ExecutorProvider) error {
......
...@@ -746,19 +746,23 @@ before upgrading the Runner, otherwise the jobs will start failing with a "No su ...@@ -746,19 +746,23 @@ before upgrading the Runner, otherwise the jobs will start failing with a "No su
## The `[runners.custom_build_dir]` section ## The `[runners.custom_build_dir]` section
NOTE: **Note:** NOTE: **Note:**
[Introduced][gitlab-runner-876] in Gitlab Runner 11.1 [Introduced][gitlab-runner-1267] in Gitlab Runner 11.10
This section defines [custom build directories][custom-build-dir-docs] parameters. This section defines [custom build directories][https://docs.gitlab.com/ee/ci/yaml/README.html#custom-build-directories] parameters.
Please notice, that the feature - if not configured explicitly - will be Please notice, that the feature - if not configured explicitly - will be
enabled by default for `kubernetes`, `docker`, `docker-ssh`, `docker+machine` enabled by default for `kubernetes`, `docker`, `docker-ssh`, `docker+machine`
and `docker-ssh+machine` executors. It will be disabled by default for all other and `docker-ssh+machine` executors. It will be disabled by default for all other
executors. executors.
The feature will be also disabled when _shared_ environments (`shell`, `ssh` This feature requires that `GIT_CLONE_PATH` is within a path defined
executors and all `docker*` executors when working directory is mounted within `runners.builds_dir`. For the ease of using `builds_dir` the
as host volume shared between jobs) will be used together with `concurrent = 1` `$CI_BUILDS_DIR` variable can be used.
set in global section.
The feature is by default enabled only for `docker` and `kubernetes` executors
as they provide a good way to separate resources. This feature can be
explicitly enabled for any executor, but special care should be taken when using
with executors that share `builds_dir` and have `concurrent > 1`.
| Parameter | Type | Description | | Parameter | Type | Description |
|-----------|---------|-------------| |-----------|---------|-------------|
...@@ -787,4 +791,3 @@ It depends on what you'd like to do. ...@@ -787,4 +791,3 @@ It depends on what you'd like to do.
[variable]: https://docs.gitlab.com/ee/ci/variables/#variables [variable]: https://docs.gitlab.com/ee/ci/variables/#variables
[cronvendor]: https://github.com/gorhill/cronexpr#implementation [cronvendor]: https://github.com/gorhill/cronexpr#implementation
[gitlab-runner-876]: https://gitlab.com/gitlab-org/gitlab-runner/merge_requests/876 [gitlab-runner-876]: https://gitlab.com/gitlab-org/gitlab-runner/merge_requests/876
[custom-build-dir-docs]: https://docs.gitlab.com/ee/ci/yaml/README.html#custom-build-directories
...@@ -509,7 +509,7 @@ func (e *executor) createBuildVolume() error { ...@@ -509,7 +509,7 @@ func (e *executor) createBuildVolume() error {
// Cache Git sources: // Cache Git sources:
// use a `BuildsDir` // use a `BuildsDir`
if !path.IsAbs(e.Build.RootDir) || e.Build.RootDir == "/" { if !path.IsAbs(e.Build.RootDir) || e.Build.RootDir == "/" {
return errors.New("build directory needs to be absolute and non-root path") return common.MakeBuildError("build directory needs to be absolute and non-root path")
} }
if e.isHostMountedVolume(e.Build.RootDir, e.Config.Docker.Volumes...) { if e.isHostMountedVolume(e.Build.RootDir, e.Config.Docker.Volumes...) {
......
...@@ -45,6 +45,53 @@ func TestDockerCommandSuccessRun(t *testing.T) { ...@@ -45,6 +45,53 @@ func TestDockerCommandSuccessRun(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
} }
func TestDockerCommandUsingCustomClonePath(t *testing.T) {
if helpers.SkipIntegrationTests(t, "docker", "info") {
return
}
jobResponse, err := common.GetRemoteBuildResponse(
"ls -al $CI_BUILDS_DIR/go/src/gitlab.com/gitlab-org/repo")
require.NoError(t, err)
tests := map[string]struct {
clonePath string
expectedErrorType interface{}
}{
"uses custom clone path": {
clonePath: "$CI_BUILDS_DIR/go/src/gitlab.com/gitlab-org/repo",
expectedErrorType: nil,
},
"path has to be within CI_BUILDS_DIR": {
clonePath: "/unknown/go/src/gitlab.com/gitlab-org/repo",
expectedErrorType: &common.BuildError{},
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
build := &common.Build{
JobResponse: jobResponse,
Runner: &common.RunnerConfig{
RunnerSettings: common.RunnerSettings{
Executor: "docker",
Docker: &common.DockerConfig{
Image: common.TestAlpineImage,
PullPolicy: common.PullPolicyIfNotPresent,
},
Environment: []string{
"GIT_CLONE_PATH=" + test.clonePath,
},
},
},
}
err = build.Run(&common.Config{}, &common.Trace{Writer: os.Stdout})
assert.IsType(t, test.expectedErrorType, err)
})
}
}
func TestDockerCommandNoRootImage(t *testing.T) { func TestDockerCommandNoRootImage(t *testing.T) {
if helpers.SkipIntegrationTests(t, "docker", "info") { if helpers.SkipIntegrationTests(t, "docker", "info") {
return return
......
...@@ -26,9 +26,9 @@ import ( ...@@ -26,9 +26,9 @@ import (
var ( var (
executorOptions = executors.ExecutorOptions{ executorOptions = executors.ExecutorOptions{
DefaultCustomBuildsDirEnabled: true,
DefaultBuildsDir: "/builds", DefaultBuildsDir: "/builds",
DefaultCacheDir: "/cache", DefaultCacheDir: "/cache",
DefaultCustomBuildsDirEnabled: true,
SharedBuildsDir: false, SharedBuildsDir: false,
Shell: common.ShellScriptInfo{ Shell: common.ShellScriptInfo{
Shell: "bash", Shell: "bash",
......
...@@ -1543,6 +1543,7 @@ func TestKubernetesNoRootImage(t *testing.T) { ...@@ -1543,6 +1543,7 @@ func TestKubernetesNoRootImage(t *testing.T) {
RunnerSettings: common.RunnerSettings{ RunnerSettings: common.RunnerSettings{
Executor: "kubernetes", Executor: "kubernetes",
Kubernetes: &common.KubernetesConfig{ Kubernetes: &common.KubernetesConfig{
Image: common.TestAlpineImage,
PullPolicy: common.PullPolicyIfNotPresent, PullPolicy: common.PullPolicyIfNotPresent,
}, },
}, },
...@@ -1553,6 +1554,52 @@ func TestKubernetesNoRootImage(t *testing.T) { ...@@ -1553,6 +1554,52 @@ func TestKubernetesNoRootImage(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
} }
func TestKubernetesCustomClonePath(t *testing.T) {
if helpers.SkipIntegrationTests(t, "kubectl", "cluster-info") {
return
}
jobResponse, err := common.GetRemoteBuildResponse(
"ls -al $CI_BUILDS_DIR/go/src/gitlab.com/gitlab-org/repo")
require.NoError(t, err)
tests := map[string]struct {
clonePath string
expectedErrorType interface{}
}{
"uses custom clone path": {
clonePath: "$CI_BUILDS_DIR/go/src/gitlab.com/gitlab-org/repo",
expectedErrorType: nil,
},
"path has to be within CI_BUILDS_DIR": {
clonePath: "/unknown/go/src/gitlab.com/gitlab-org/repo",
expectedErrorType: &common.BuildError{},
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
build := &common.Build{
JobResponse: jobResponse,
Runner: &common.RunnerConfig{
RunnerSettings: common.RunnerSettings{
Executor: "kubernetes",
Kubernetes: &common.KubernetesConfig{
Image: common.TestAlpineImage,
PullPolicy: common.PullPolicyIfNotPresent,
},
Environment: []string{
"GIT_CLONE_PATH=" + test.clonePath,
},
},
},
}
err = build.Run(&common.Config{}, &common.Trace{Writer: os.Stdout})
assert.IsType(t, test.expectedErrorType, err)
})
}
}
func TestKubernetesBuildFail(t *testing.T) { func TestKubernetesBuildFail(t *testing.T) {
if helpers.SkipIntegrationTests(t, "kubectl", "cluster-info") { if helpers.SkipIntegrationTests(t, "kubectl", "cluster-info") {
return return
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment