From 61282888e1ff2eb9148259f05414638791da106e Mon Sep 17 00:00:00 2001 From: Brad Rydzewski Date: Fri, 21 Nov 2014 22:41:32 -0800 Subject: [PATCH 1/4] improve permission logic --- server/datastore/perm.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/server/datastore/perm.go b/server/datastore/perm.go index ffc431aa..6e257f65 100644 --- a/server/datastore/perm.go +++ b/server/datastore/perm.go @@ -23,7 +23,7 @@ type Permstore interface { // GetPerm retrieves the User's permission from // the datastore for the given repository. func GetPerm(c context.Context, user *model.User, repo *model.Repo) (*model.Perm, error) { - // if the user is a gues they should only be granted + // if the user is a guest they should only be granted // read access to public repositories. switch { case user == nil && repo.Private: @@ -43,8 +43,8 @@ func GetPerm(c context.Context, user *model.User, repo *model.Repo) (*model.Perm // if the user is authenticated we'll retireive the // permission details from the database. perm, err := FromContext(c).GetPerm(user, repo) - if err == nil && perm.ID != 0 { - return perm, err + if perm.ID == 0 { + perm.Guest = true } switch { @@ -53,12 +53,10 @@ func GetPerm(c context.Context, user *model.User, repo *model.Repo) (*model.Perm perm.Read = true perm.Write = true perm.Admin = true - perm.Guest = true // if the repo is public, grant read access only. case repo.Private == false: perm.Read = true - perm.Guest = true } return perm, err } From 76fd8b0d6662b4ae2e7eaf60dc5a59eb47e46c0d Mon Sep 17 00:00:00 2001 From: Brad Rydzewski Date: Tue, 30 Dec 2014 10:35:44 -0800 Subject: [PATCH 2/4] only inject ssk key if private repository or pull request --- server/worker/docker/docker.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/worker/docker/docker.go b/server/worker/docker/docker.go index d00f7752..39df49d9 100644 --- a/server/worker/docker/docker.go +++ b/server/worker/docker/docker.go @@ -122,10 +122,13 @@ func (d *Docker) Do(c context.Context, r *worker.Work) { builder.Build = script builder.Repo = repo builder.Stdout = buf - builder.Key = []byte(r.Repo.PrivateKey) builder.Timeout = time.Duration(r.Repo.Timeout) * time.Second builder.Privileged = r.Repo.Privileged + if r.Repo.Private || len(r.Commit.PullRequest) == 0 { + builder.Key = []byte(r.Repo.PrivateKey) + } + // run the build err = builder.Run() From c3e00e26088ab2e20fbcb6c80ab5681ee0bcce0e Mon Sep 17 00:00:00 2001 From: Brad Rydzewski Date: Tue, 30 Dec 2014 13:09:05 -0800 Subject: [PATCH 3/4] improved error messaging for failed build due to Docker errors --- shared/build/build.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/shared/build/build.go b/shared/build/build.go index 2226cc4f..9fc28420 100644 --- a/shared/build/build.go +++ b/shared/build/build.go @@ -238,7 +238,7 @@ func (b *Builder) setup() error { if _, err := b.dockerClient.Images.Inspect(b.Build.Image); err == docker.ErrNotFound { // download the image if it doesn't exist if err := b.dockerClient.Images.Pull(b.Build.Image); err != nil { - return err + return fmt.Errorf("Error: Unable to pull image %s. %s", b.Build.Image, err) } } else if err != nil { log.Errf("failed to inspect image %s", b.Build.Image) @@ -392,7 +392,7 @@ func (b *Builder) run() error { // create the container from the image run, err := b.dockerClient.Containers.Create(&conf) if err != nil { - return err + return fmt.Errorf("Error: Failed to create build container. %s", err) } // cache instance of docker.Run @@ -407,7 +407,7 @@ func (b *Builder) run() error { if err := b.dockerClient.Containers.Start(run.ID, &host); err != nil { b.BuildState.ExitCode = 1 b.BuildState.Finished = time.Now().UTC().Unix() - return err + return fmt.Errorf("Error: Failed to start build container. %s", err) } // wait for the container to stop @@ -415,7 +415,7 @@ func (b *Builder) run() error { if err != nil { b.BuildState.ExitCode = 1 b.BuildState.Finished = time.Now().UTC().Unix() - return err + return fmt.Errorf("Error: Failed to wait for build container. %s", err) } // set completion time @@ -503,7 +503,9 @@ func (b *Builder) writeBuildScript(dir string) error { f.WriteHost(mapping) } - f.WriteFile("$HOME/.ssh/id_rsa", b.Key, 600) + if len(b.Key) != 0 { + f.WriteFile("$HOME/.ssh/id_rsa", b.Key, 600) + } // if the repository is remote then we should // add the commands to the build script to From f890d420192ba29bab25c7d9cb746348c8eac1ca Mon Sep 17 00:00:00 2001 From: Brad Rydzewski Date: Tue, 30 Dec 2014 13:17:54 -0800 Subject: [PATCH 4/4] fixed unit test --- shared/build/build_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/shared/build/build_test.go b/shared/build/build_test.go index e1eaea07..f9930efd 100644 --- a/shared/build/build_test.go +++ b/shared/build/build_test.go @@ -422,8 +422,7 @@ func TestRunErrorCreate(t *testing.T) { b.image = &docker.Image{ID: "c3ab8ff137"} b.Build = &script.Build{} b.Repo = &repo.Repo{} - - if err := b.run(); err != docker.ErrBadRequest { + if err := b.run(); err == nil || err.Error() != "Error: Failed to create build container. Bad Request" { t.Errorf("Expected error when trying to create build container") } } @@ -456,7 +455,7 @@ func TestRunErrorStart(t *testing.T) { b.Build = &script.Build{} b.Repo = &repo.Repo{} - if err := b.run(); err != docker.ErrBadRequest { + if err := b.run(); err == nil || err.Error() != "Error: Failed to start build container. Bad Request" { t.Errorf("Expected error when trying to start build container") }