From 6b0f24cc116c3d2f40dd85d053aa77e81d36cf07 Mon Sep 17 00:00:00 2001 From: Don Olmstead Date: Tue, 18 Aug 2015 17:33:42 -0700 Subject: [PATCH] Adding support for setting the workspace directly in the config --- cmd/drone-build/main.go | 6 ++--- cmd/drone-build/run.go | 11 -------- pkg/yaml/lint.go | 30 +++++++++++++++++++++- pkg/yaml/lint_test.go | 31 ++++++++++++++++++++--- pkg/yaml/transform/transform.go | 23 ++++++++++++++--- pkg/yaml/transform/transform_test.go | 38 +++++++++++++++++++++++++++- 6 files changed, 116 insertions(+), 23 deletions(-) diff --git a/cmd/drone-build/main.go b/cmd/drone-build/main.go index 758ddbcc1..38dc4d1b1 100644 --- a/cmd/drone-build/main.go +++ b/cmd/drone-build/main.go @@ -37,7 +37,6 @@ func main() { os.Exit(1) return } - createClone(ctx) // creates the Docker client, connecting to the // linked Docker daemon @@ -68,7 +67,8 @@ func main() { os.Exit(1) return } - + createClone(ctx) + var execs []execFunc if *clone { execs = append(execs, execClone) @@ -135,7 +135,7 @@ func createClone(c *Context) error { if err != nil { return err } - c.Clone.Dir = filepath.Join("/drone/src", url_.Host, c.Repo.FullName) + c.Clone.Dir, _ = c.Conf.Clone.Config["path"] return nil } diff --git a/cmd/drone-build/run.go b/cmd/drone-build/run.go index 6324c2b6c..c24d6f724 100644 --- a/cmd/drone-build/run.go +++ b/cmd/drone-build/run.go @@ -64,17 +64,6 @@ func setup(c *Context) error { c.Conf.Build.Environment = append(c.Conf.Build.Environment, env) } - // attempt to extract the clone path. i'm not a huge fan of - // this, by the way, but for now we'll keep it. - // TODO consider moving this to a transform? - pathv, ok := c.Conf.Clone.Config["path"] - if ok { - path, ok := pathv.(string) - if ok { - c.Clone.Dir = filepath.Join("/drone/src", path) - } - } - return nil } diff --git a/pkg/yaml/lint.go b/pkg/yaml/lint.go index 27a07ed06..4584991b6 100644 --- a/pkg/yaml/lint.go +++ b/pkg/yaml/lint.go @@ -13,7 +13,7 @@ import ( // fails it should return an error message. type lintRule func(*common.Config) error -var lintRules = [...]lintRule{ +var lintRules = []lintRule{ expectBuild, expectImage, expectCommand, @@ -22,6 +22,7 @@ var lintRules = [...]lintRule{ expectTrustedPublish, expectTrustedDeploy, expectTrustedNotify, + expectCloneInWorkspace, expectCacheInWorkspace, } @@ -106,6 +107,33 @@ func expectTrustedNotify(c *common.Config) error { return nil } +// lint rule that fails if the clone directory is not contained +// in the root workspace. +func expectCloneInWorkspace(c *common.Config) error { + pathv, ok := c.Clone.Config["path"] + var path string + + if ok { + path, _ = pathv.(string) + } + if len(path) == 0 { + // This should only happen if the transformer was not run + return fmt.Errorf("No workspace specified") + } + + relative, relOk := filepath.Rel("/drone/src", path) + if relOk != nil { + return fmt.Errorf("Path is not relative to root") + } + + cleaned := filepath.Clean(relative) + if strings.Index(cleaned, "../") != -1 { + return fmt.Errorf("Cannot clone above the root") + } + + return nil +} + // lint rule that fails if the cache directories are not contained // in the workspace. func expectCacheInWorkspace(c *common.Config) error { diff --git a/pkg/yaml/lint_test.go b/pkg/yaml/lint_test.go index d446f684a..7bc134935 100644 --- a/pkg/yaml/lint_test.go +++ b/pkg/yaml/lint_test.go @@ -79,6 +79,9 @@ func Test_Linter(t *testing.T) { c.Build.Image = "golang" c.Build.Config = map[string]interface{}{} c.Build.Config["commands"] = []string{"go build", "go test"} + c.Clone = &common.Step{} + c.Clone.Config = map[string]interface{}{} + c.Clone.Config["path"] = "/drone/src/foo/bar" c.Publish = map[string]*common.Step{} c.Publish["docker"] = &common.Step{Image: "docker"} c.Deploy = map[string]*common.Step{} @@ -88,7 +91,29 @@ func Test_Linter(t *testing.T) { g.Assert(Lint(c) == nil).IsTrue() }) - g.It("Should pass with path inside workspace", func() { + g.It("Should pass with clone path inside workspace", func() { + c := &common.Config{ + Clone: &common.Step{ + Config: map[string]interface{}{ + "path": "/drone/src/foo/bar", + }, + }, + } + g.Assert(expectCloneInWorkspace(c) == nil).IsTrue() + }) + + g.It("Should fail with clone path outside workspace", func() { + c := &common.Config{ + Clone: &common.Step{ + Config: map[string]interface{}{ + "path": "/foo/bar", + }, + }, + } + g.Assert(expectCloneInWorkspace(c) != nil).IsTrue() + }) + + g.It("Should pass with cache path inside workspace", func() { c := &common.Config{ Build: &common.Step{ Cache: []string{".git","/.git","/.git/../.git/../.git"}, @@ -97,7 +122,7 @@ func Test_Linter(t *testing.T) { g.Assert(expectCacheInWorkspace(c) == nil).IsTrue() }) - g.It("Should fail with path outside workspace", func() { + g.It("Should fail with cache path outside workspace", func() { c := &common.Config{ Build: &common.Step{ Cache: []string{".git","/.git","../../.git"}, @@ -115,7 +140,7 @@ func Test_Linter(t *testing.T) { g.Assert(expectCacheInWorkspace(c) != nil).IsTrue() }) - g.It("Should fail when : is in the path", func() { + g.It("Should fail when : is in the cache path", func() { c := &common.Config{ Build: &common.Step{ Cache: []string{".git",".git:/../"}, diff --git a/pkg/yaml/transform/transform.go b/pkg/yaml/transform/transform.go index a6a84953f..3758a15e7 100644 --- a/pkg/yaml/transform/transform.go +++ b/pkg/yaml/transform/transform.go @@ -9,6 +9,12 @@ import ( common "github.com/drone/drone/pkg/types" ) +// buildRoot is the root build directory. +// +// If this changes then the matching value in lint.go needs +// to be modified as well. +const buildRoot = "/drone/src" + // transformRule applies a check or transformation rule // to the build configuration. type transformRule func(*common.Config) @@ -203,7 +209,17 @@ func rmNetwork(c *common.Config) { // directory to the configuration based on the repository // information. func transformWorkspace(c *common.Config, r *common.Repo) { - //c.Clone.Dir = workspaceRoot(r) + pathv, ok := c.Clone.Config["path"] + var path string + + if ok { + path, _ = pathv.(string) + } + if len(path) == 0 { + path = repoPath(r) + } + + c.Clone.Config["path"] = filepath.Join(buildRoot, path) } // transformCache is a transformer that adds volumes @@ -218,14 +234,13 @@ func transformCache(c *common.Config, r *common.Repo) { volumes := make([]string, cacheCount) cache := cacheRoot(r) - workspace := workspaceRoot(r) + workspace := c.Clone.Config["path"].(string) for i, dir := range c.Build.Cache { cacheDir := filepath.Join(cache, dir) workspaceDir := filepath.Join(workspace, dir) volumes[i] = fmt.Sprintf("%s:%s", cacheDir, workspaceDir) - fmt.Printf("Volume %s", volumes[i]) } c.Setup.Volumes = append(c.Setup.Volumes, volumes...) @@ -272,7 +287,7 @@ func imageNameDefault(name, defaultName string) string { // workspaceRoot is a helper function that determines the // default workspace the build runs in. func workspaceRoot(r *common.Repo) string { - return filepath.Join("/drone/src", repoPath(r)) + return filepath.Join(buildRoot, repoPath(r)) } // cacheRoot is a helper function that deteremines the diff --git a/pkg/yaml/transform/transform_test.go b/pkg/yaml/transform/transform_test.go index 2d29ba24c..85cca9a92 100644 --- a/pkg/yaml/transform/transform_test.go +++ b/pkg/yaml/transform/transform_test.go @@ -1,6 +1,7 @@ package transform import ( + "path/filepath" "testing" "github.com/drone/drone/Godeps/_workspace/src/github.com/franela/goblin" @@ -146,7 +147,9 @@ func Test_Transform(t *testing.T) { g.It("Should have cached volumes", func() { c := &common.Config{ Setup: &common.Step{}, - Clone: &common.Step{}, + Clone: &common.Step{ + Config: map[string]interface{}{}, + }, Build: &common.Step{ Cache: []string{".git","foo","bar"}, }, @@ -158,6 +161,7 @@ func Test_Transform(t *testing.T) { Link: "https://github.com/drone/drone", FullName: "drone/drone", } + transformWorkspace(c, r) transformCache(c, r) cacheCount := len(c.Build.Cache) @@ -180,5 +184,37 @@ func Test_Transform(t *testing.T) { testRange(c.Notify) testRange(c.Compose) }) + + g.It("Should have default workspace directory", func() { + c := &common.Config{ + Clone: &common.Step{ + Config: map[string]interface{}{}, + }, + } + r := &common.Repo{ + Link: "https://github.com/drone/drone", + FullName: "drone/drone", + } + transformWorkspace(c, r) + + g.Assert(c.Clone.Config["path"]).Equal(workspaceRoot(r)) + }) + + g.It("Should use path for working directory", func() { + c := &common.Config{ + Clone: &common.Step{ + Config: map[string]interface{}{ + "path": "foo/bar", + }, + }, + } + r := &common.Repo{ + Link: "https://github.com/drone/drone", + FullName: "drone/drone", + } + transformWorkspace(c, r) + + g.Assert(c.Clone.Config["path"]).Equal(filepath.Join(buildRoot, "foo/bar")) + }) }) }