Skip to content

Commit

Permalink
trusts builds from private repo and non-PR
Browse files Browse the repository at this point in the history
  • Loading branch information
Ke Zhu committed Jul 16, 2015
1 parent cf578b3 commit 0ef85cb
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 37 deletions.
35 changes: 18 additions & 17 deletions server/worker/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,6 @@ func (d *Docker) Do(c context.Context, r *worker.Work) {
log.Printf("Error parsing YAML for %s/%s, Err: %s", r.Repo.Owner, r.Repo.Name, err.Error())
}

// append private parameters to the environment
// variable section of the .drone.yml file, iff
// this is not a pull request (for security purposes)
if params != nil && (r.Repo.Private || len(r.Commit.PullRequest) == 0) {
for k, v := range params {
script.Env = append(script.Env, k+"="+v)
}
}

// TODO: handle error better?
buildNumber, err := datastore.GetBuildNumber(c, r.Commit)
if err != nil {
Expand All @@ -121,13 +112,23 @@ func (d *Docker) Do(c context.Context, r *worker.Work) {

path := r.Repo.Host + "/" + r.Repo.Owner + "/" + r.Repo.Name
repo := &repo.Repo{
Name: path,
Path: r.Repo.CloneURL,
Branch: r.Commit.Branch,
Commit: r.Commit.Sha,
PR: r.Commit.PullRequest,
Dir: filepath.Join("/var/cache/drone/src", git.GitPath(script.Git, path)),
Depth: git.GitDepth(script.Git),
Name: path,
Path: r.Repo.CloneURL,
Branch: r.Commit.Branch,
Commit: r.Commit.Sha,
PR: r.Commit.PullRequest,
Private: r.Repo.Private,
Dir: filepath.Join("/var/cache/drone/src", git.GitPath(script.Git, path)),
Depth: git.GitDepth(script.Git),
}

// append private parameters to the environment
// variable section of the .drone.yml file, if
// this is trusted
if params != nil && repo.IsTrusted() {
for k, v := range params {
script.Env = append(script.Env, k+"="+v)
}
}

priorCommit, _ := datastore.GetCommitPrior(c, r.Commit)
Expand All @@ -152,7 +153,7 @@ func (d *Docker) Do(c context.Context, r *worker.Work) {
builder.Timeout = time.Duration(r.Repo.Timeout) * time.Second
builder.Privileged = r.Repo.Privileged

if r.Repo.Private || len(r.Commit.PullRequest) == 0 {
if repo.IsTrusted() {
builder.Key = []byte(r.Repo.PrivateKey)
}

Expand Down
2 changes: 1 addition & 1 deletion shared/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ func (b *Builder) run() error {

// configure if Docker should run in privileged mode
host := docker.HostConfig{
Privileged: (b.Privileged && script.DockerPrivileged(b.Build.Docker)),
Privileged: (b.Privileged && b.Repo.IsTrusted()),
}

if host.Privileged {
Expand Down
28 changes: 23 additions & 5 deletions shared/build/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,21 +412,39 @@ func TestRunPrivileged(t *testing.T) {
t.Errorf("Expected container NOT started in Privileged mode")
}

// now lets try to enable priviliged mode
// now lets set priviliged mode
b.Privileged = true
b.run()

if conf.Privileged != true {
t.Errorf("Expected container IS started in Privileged mode")
}

// now lets set priviliged mode but for a pull request
b.Privileged = true
b.Repo.PR = "55"
b.run()

if conf.Privileged != false {
t.Errorf("Expected container NOT started in Privileged mode when PR")
}

// now lets set priviliged mode for a pull request from public repo
b.Privileged = true
b.Repo.Private = false
b.run()

if conf.Privileged != false {
t.Errorf("Expected container NOT started in Privileged mode when no setup in build script")
t.Errorf("Expected container NOT started in Privileged mode when PR from public repo")
}

// now lets set priviliged mode in build script
// now lets set priviliged mode for a pull request from private repo
b.Privileged = true
b.Build = &script.Build{Docker: &script.Docker{Privileged: true}}
b.Repo.Private = true
b.run()

if conf.Privileged != true {
t.Errorf("Expected container IS started in Privileged mode when setup privileged mode in build script")
t.Errorf("Expected container started in Privileged mode when PR from private repo")
}
}

Expand Down
8 changes: 8 additions & 0 deletions shared/build/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ type Repo struct {
// checkout when the Repository is cloned.
PR string

// Private specifies if a git repo is private or not
Private bool

// (optional) The filesystem path that the repository
// will be cloned into (or copied to) inside the
// host system (Docker Container).
Expand Down Expand Up @@ -125,3 +128,8 @@ func (r *Repo) Commands() []string {

return cmds
}

// IsTrusted returns if a repo is trusted to run under privileged mode
func (r *Repo) IsTrusted() bool {
return r.Private || len(r.PR) == 0
}
20 changes: 20 additions & 0 deletions shared/build/repo/repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,23 @@ func TestIsGit(t *testing.T) {
}
}
}

func TestIsTrusted(t *testing.T) {
repos := []struct {
private bool
PR string
trusted bool
}{
{true, "1", true},
{false, "1", false},
{true, "", true},
{false, "", true},
}

for _, r := range repos {
repo := Repo{Private: r.private, PR: r.PR}
if trusted := repo.IsTrusted(); trusted != r.trusted {
t.Errorf("IsTrusted was %v, expected %v", trusted, r.trusted)
}
}
}
15 changes: 1 addition & 14 deletions shared/build/script/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@ type Docker struct {

// Allocate a pseudo-TTY (also known as `--tty` option)
TTY bool `yaml:"tty,omitempty"`

// Privileged means enabling all devices on the host in container
// (also known as `--privileged=true` option)
Privileged bool `yaml:"privileged,omitempty"`
}

// DockerNetworkMode returns DefaultNetworkMode
Expand All @@ -34,7 +30,7 @@ func DockerNetworkMode(d *Docker) string {
return *d.NetworkMode
}

// DockerNetworkMode returns empty string
// DockerHostname returns empty string
// when Docker.NetworkMode is empty.
// DockerNetworkMode returns Docker.NetworkMode
// when it is not empty.
Expand All @@ -53,12 +49,3 @@ func DockerTty(d *Docker) bool {
}
return d.TTY
}

// DockerPrivileged returns true if the build
// should have privileged mode
func DockerPrivileged(d *Docker) bool {
if d == nil {
return false
}
return d.Privileged
}

0 comments on commit 0ef85cb

Please sign in to comment.