From 6b0f24cc116c3d2f40dd85d053aa77e81d36cf07 Mon Sep 17 00:00:00 2001 From: Don Olmstead Date: Tue, 18 Aug 2015 17:33:42 -0700 Subject: [PATCH 1/3] 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 758ddbcc..38dc4d1b 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 6324c2b6..c24d6f72 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 27a07ed0..4584991b 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 d446f684..7bc13493 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 a6a84953..3758a15e 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 2d29ba24..85cca9a9 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")) + }) }) } From b7635ec844bbf5e96ae8976334d8b9153f255f19 Mon Sep 17 00:00:00 2001 From: Don Olmstead Date: Tue, 18 Aug 2015 18:28:35 -0700 Subject: [PATCH 2/3] Fixing build and formatting issues --- cmd/drone-build/main.go | 18 +++++++++--------- cmd/drone-build/run.go | 2 -- pkg/runner/builtin/runner.go | 4 ++-- pkg/yaml/lint_test.go | 8 ++++---- pkg/yaml/transform/transform.go | 6 +++--- pkg/yaml/transform/transform_test.go | 12 ++++++------ 6 files changed, 24 insertions(+), 26 deletions(-) diff --git a/cmd/drone-build/main.go b/cmd/drone-build/main.go index 38dc4d1b..11c6dc19 100644 --- a/cmd/drone-build/main.go +++ b/cmd/drone-build/main.go @@ -5,9 +5,7 @@ import ( "encoding/json" "flag" "fmt" - "net/url" "os" - "path/filepath" "strings" log "github.com/drone/drone/Godeps/_workspace/src/github.com/Sirupsen/logrus" @@ -68,7 +66,7 @@ func main() { return } createClone(ctx) - + var execs []execFunc if *clone { execs = append(execs, execClone) @@ -130,13 +128,15 @@ func createClone(c *Context) error { // to the clone object for merge requests from bitbucket, gitlab, et al // if len(c.Commit.PullRequest) != 0 { // } - - url_, err := url.Parse(c.Repo.Link) - if err != nil { - return err + pathv, ok := c.Conf.Clone.Config["path"] + if ok { + path, ok := pathv.(string) + if ok { + c.Clone.Dir = path + return nil + } } - c.Clone.Dir, _ = c.Conf.Clone.Config["path"] - return nil + return fmt.Errorf("Workspace path not found") } func parseContext() (*Context, error) { diff --git a/cmd/drone-build/run.go b/cmd/drone-build/run.go index c24d6f72..a87ca913 100644 --- a/cmd/drone-build/run.go +++ b/cmd/drone-build/run.go @@ -1,8 +1,6 @@ package main import ( - "path/filepath" - "github.com/drone/drone/Godeps/_workspace/src/github.com/samalba/dockerclient" common "github.com/drone/drone/pkg/types" "github.com/drone/drone/pkg/yaml" diff --git a/pkg/runner/builtin/runner.go b/pkg/runner/builtin/runner.go index 13d47744..92d41986 100644 --- a/pkg/runner/builtin/runner.go +++ b/pkg/runner/builtin/runner.go @@ -2,14 +2,14 @@ package builtin import ( "bytes" + "crypto/tls" + "crypto/x509" "encoding/json" "fmt" "io" "io/ioutil" "os" "time" - "crypto/tls" - "crypto/x509" "github.com/drone/drone/Godeps/_workspace/src/github.com/samalba/dockerclient" "github.com/drone/drone/pkg/docker" diff --git a/pkg/yaml/lint_test.go b/pkg/yaml/lint_test.go index 7bc13493..cbdbcf5c 100644 --- a/pkg/yaml/lint_test.go +++ b/pkg/yaml/lint_test.go @@ -116,7 +116,7 @@ func Test_Linter(t *testing.T) { g.It("Should pass with cache path inside workspace", func() { c := &common.Config{ Build: &common.Step{ - Cache: []string{".git","/.git","/.git/../.git/../.git"}, + Cache: []string{".git", "/.git", "/.git/../.git/../.git"}, }, } g.Assert(expectCacheInWorkspace(c) == nil).IsTrue() @@ -125,7 +125,7 @@ func Test_Linter(t *testing.T) { g.It("Should fail with cache path outside workspace", func() { c := &common.Config{ Build: &common.Step{ - Cache: []string{".git","/.git","../../.git"}, + Cache: []string{".git", "/.git", "../../.git"}, }, } g.Assert(expectCacheInWorkspace(c) != nil).IsTrue() @@ -134,7 +134,7 @@ func Test_Linter(t *testing.T) { g.It("Should fail when caching workspace directory", func() { c := &common.Config{ Build: &common.Step{ - Cache: []string{".git",".git/../"}, + Cache: []string{".git", ".git/../"}, }, } g.Assert(expectCacheInWorkspace(c) != nil).IsTrue() @@ -143,7 +143,7 @@ func Test_Linter(t *testing.T) { g.It("Should fail when : is in the cache path", func() { c := &common.Config{ Build: &common.Step{ - Cache: []string{".git",".git:/../"}, + Cache: []string{".git", ".git:/../"}, }, } g.Assert(expectCacheInWorkspace(c) != nil).IsTrue() diff --git a/pkg/yaml/transform/transform.go b/pkg/yaml/transform/transform.go index 3758a15e..2db8f0f2 100644 --- a/pkg/yaml/transform/transform.go +++ b/pkg/yaml/transform/transform.go @@ -71,7 +71,7 @@ func RemovePrivileged(c *common.Config) { // Repo executes all transformers that rely on repository // information. func Repo(c *common.Config, r *common.Repo) { - transformWorkspace(c, r) + transformWorkspace(c, r) transformCache(c, r) } @@ -227,7 +227,7 @@ func transformWorkspace(c *common.Config, r *common.Repo) { func transformCache(c *common.Config, r *common.Repo) { cacheCount := len(c.Build.Cache) - if cacheCount == 0 { + if cacheCount == 0 { return } @@ -287,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(buildRoot, 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 85cca9a9..a01214c1 100644 --- a/pkg/yaml/transform/transform_test.go +++ b/pkg/yaml/transform/transform_test.go @@ -151,20 +151,20 @@ func Test_Transform(t *testing.T) { Config: map[string]interface{}{}, }, Build: &common.Step{ - Cache: []string{".git","foo","bar"}, + Cache: []string{".git", "foo", "bar"}, }, Notify: map[string]*common.Step{}, Deploy: map[string]*common.Step{}, Publish: map[string]*common.Step{}, } r := &common.Repo{ - Link: "https://github.com/drone/drone", + Link: "https://github.com/drone/drone", FullName: "drone/drone", } transformWorkspace(c, r) transformCache(c, r) - cacheCount := len(c.Build.Cache) + cacheCount := len(c.Build.Cache) test := func(s *common.Step) { g.Assert(len(s.Volumes)).Equal(cacheCount) @@ -176,7 +176,7 @@ func Test_Transform(t *testing.T) { } } - test(c.Setup) + test(c.Setup) test(c.Clone) test(c.Build) testRange(c.Publish) @@ -192,7 +192,7 @@ func Test_Transform(t *testing.T) { }, } r := &common.Repo{ - Link: "https://github.com/drone/drone", + Link: "https://github.com/drone/drone", FullName: "drone/drone", } transformWorkspace(c, r) @@ -209,7 +209,7 @@ func Test_Transform(t *testing.T) { }, } r := &common.Repo{ - Link: "https://github.com/drone/drone", + Link: "https://github.com/drone/drone", FullName: "drone/drone", } transformWorkspace(c, r) From 257ea9986a474208824b10ce40cda496ec24c89a Mon Sep 17 00:00:00 2001 From: Joshua Anderson Date: Tue, 18 Aug 2015 21:57:35 -0700 Subject: [PATCH 3/3] Change Postgres to MySQL in MySQL docs. --- doc/setup/mysql.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/setup/mysql.md b/doc/setup/mysql.md index 73428f12..c12ca72c 100644 --- a/doc/setup/mysql.md +++ b/doc/setup/mysql.md @@ -2,7 +2,7 @@ # MySQL -Drone comes with support for MySQL as an alternate database engine. To enable Postgres, you should specify the following environment variables: +Drone comes with support for MySQL as an alternate database engine. To enable MySQL, you should specify the following environment variables: ```bash DATABASE_DRIVER="mysql"