From b948180b4e951e41bac3a6673e670875bc310774 Mon Sep 17 00:00:00 2001 From: jola5 Date: Sat, 28 Jan 2017 22:29:49 +0100 Subject: [PATCH 1/7] Abort serve command if rootDir is inaccesible --- cmd/api_serve.go | 5 +++++ cmd/serve.go | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/cmd/api_serve.go b/cmd/api_serve.go index fd4c765b..6cd571bf 100644 --- a/cmd/api_serve.go +++ b/cmd/api_serve.go @@ -6,6 +6,7 @@ import ( "github.com/smira/commander" "github.com/smira/flag" "net/http" + "golang.org/x/sys/unix" ) func aptlyAPIServe(cmd *commander.Command, args []string) error { @@ -18,6 +19,10 @@ func aptlyAPIServe(cmd *commander.Command, args []string) error { return commander.ErrCommandError } + if unix.Access(context.Config().RootDir, unix.W_OK) != nil { + return fmt.Errorf("Configured rootDir '%s' inaccesible, check access rights", context.Config().RootDir) + } + listen := context.Flags().Lookup("listen").Value.String() fmt.Printf("\nStarting web server at: %s (press Ctrl+C to quit)...\n", listen) diff --git a/cmd/serve.go b/cmd/serve.go index d1c19e03..b053f244 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -12,6 +12,7 @@ import ( "os" "sort" "strings" + "golang.org/x/sys/unix" ) func aptlyServe(cmd *commander.Command, args []string) error { @@ -22,6 +23,10 @@ func aptlyServe(cmd *commander.Command, args []string) error { return commander.ErrCommandError } + if unix.Access(context.Config().RootDir, unix.W_OK) != nil { + return fmt.Errorf("Configured rootDir '%s' inaccesible, check access rights", context.Config().RootDir) + } + if context.CollectionFactory().PublishedRepoCollection().Len() == 0 { fmt.Printf("No published repositories, unable to serve.\n") return nil From 3040e7360ac0e058419902ddcabaaa83c6d26eca Mon Sep 17 00:00:00 2001 From: jola5 Date: Sat, 28 Jan 2017 22:44:18 +0100 Subject: [PATCH 2/7] Fix golang.org/x/sys/unix dependency issue --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 6174f0df..4fcc813f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -27,6 +27,7 @@ before_install: - mkdir -p $GOPATH/src/github.com/smira - ln -s $TRAVIS_BUILD_DIR $GOPATH/src/github.com/smira || true - cd $GOPATH/src/github.com/smira/aptly + - go get golang.org/x/sys/unix install: - make prepare From edffa24658315d7c72e46e6eb33c58812274b6ce Mon Sep 17 00:00:00 2001 From: jola5 Date: Sun, 29 Jan 2017 00:05:25 +0100 Subject: [PATCH 3/7] Test startup checks for serve command --- cmd/api_serve.go | 2 +- cmd/serve.go | 2 +- system/t07_serve/RootDirInaccessible_gold | 1 + system/t07_serve/__init__.py | 32 +++++++++++++++++++++++ 4 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 system/t07_serve/RootDirInaccessible_gold diff --git a/cmd/api_serve.go b/cmd/api_serve.go index 6cd571bf..d2eebaed 100644 --- a/cmd/api_serve.go +++ b/cmd/api_serve.go @@ -20,7 +20,7 @@ func aptlyAPIServe(cmd *commander.Command, args []string) error { } if unix.Access(context.Config().RootDir, unix.W_OK) != nil { - return fmt.Errorf("Configured rootDir '%s' inaccesible, check access rights", context.Config().RootDir) + return fmt.Errorf("Configured rootDir '%s' is inaccessible, check access rights", context.Config().RootDir) } listen := context.Flags().Lookup("listen").Value.String() diff --git a/cmd/serve.go b/cmd/serve.go index b053f244..346fa4e5 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -24,7 +24,7 @@ func aptlyServe(cmd *commander.Command, args []string) error { } if unix.Access(context.Config().RootDir, unix.W_OK) != nil { - return fmt.Errorf("Configured rootDir '%s' inaccesible, check access rights", context.Config().RootDir) + return fmt.Errorf("Configured rootDir '%s' is inaccessible, check access rights", context.Config().RootDir) } if context.CollectionFactory().PublishedRepoCollection().Len() == 0 { diff --git a/system/t07_serve/RootDirInaccessible_gold b/system/t07_serve/RootDirInaccessible_gold new file mode 100644 index 00000000..3d57b5c6 --- /dev/null +++ b/system/t07_serve/RootDirInaccessible_gold @@ -0,0 +1 @@ +ERROR: Configured rootDir '/rootDir/does/not/exist' is inaccessible, check access rights diff --git a/system/t07_serve/__init__.py b/system/t07_serve/__init__.py index 063f72da..f7513745 100644 --- a/system/t07_serve/__init__.py +++ b/system/t07_serve/__init__.py @@ -8,8 +8,40 @@ import signal import subprocess import shlex import time +import errno from lib import BaseTest +from socket import error as socket_error + +class RootDirInaccessible(BaseTest): + """ + serve command aborts if rootDir is inaccessible + """ + fixtureDB = False + fixturePool = False + configOverride = { "rootDir": "/rootDir/does/not/exist" } + runCmd = "aptly serve -listen=127.0.0.1:8765" + + def run(self): + try: + proc = subprocess.Popen(shlex.split(self.runCmd), stderr=subprocess.STDOUT, stdout=subprocess.PIPE, bufsize=0) + time.sleep(1) + + conn = httplib.HTTPConnection("127.0.0.1", 8765) + conn.request("GET", "/") + r = conn.getresponse() + self.http_response = r.read() + output = os.read(proc.stdout.fileno(), 8192) + + except socket_error as serr: + if serr.errno != errno.ECONNREFUSED: + raise serr + + finally: + self.output, err = proc.communicate() + + def check(self): + self.check_output() class Serve1Test(BaseTest): From 970b1a424a0e3a2cb42e9b86948236414e96cda6 Mon Sep 17 00:00:00 2001 From: jola5 Date: Sun, 29 Jan 2017 10:38:39 +0100 Subject: [PATCH 4/7] Fix bugged implementation --- cmd/api_serve.go | 19 +++++++++++++++++-- cmd/serve.go | 18 ++++++++++++++++-- system/t07_serve/RootDirInaccessible_gold | 2 +- system/t07_serve/__init__.py | 5 ++++- 4 files changed, 38 insertions(+), 6 deletions(-) diff --git a/cmd/api_serve.go b/cmd/api_serve.go index d2eebaed..c008b355 100644 --- a/cmd/api_serve.go +++ b/cmd/api_serve.go @@ -6,6 +6,7 @@ import ( "github.com/smira/commander" "github.com/smira/flag" "net/http" + "os" "golang.org/x/sys/unix" ) @@ -19,8 +20,22 @@ func aptlyAPIServe(cmd *commander.Command, args []string) error { return commander.ErrCommandError } - if unix.Access(context.Config().RootDir, unix.W_OK) != nil { - return fmt.Errorf("Configured rootDir '%s' is inaccessible, check access rights", context.Config().RootDir) + // There are only two working options for aptly's rootDir: + // 1. rootDir does not exist, then we'll create it + // 2. rootDir exists and is writable + // anything else must fail. + // E.g.: Running the service under a different user may lead to a rootDir + // that exists but is not usable due to access permissions. + // Config loads and returns current configuration + _, err = os.Stat(context.Config().RootDir); + if err != nil { + if ! os.IsNotExist(err) { + return fmt.Errorf("Something went wrong, %v", err) + } + } else { + if unix.Access(context.Config().RootDir, unix.W_OK) != nil { + return fmt.Errorf("Configured rootDir '%s' is inaccessible, check access rights", context.Config().RootDir) + } } listen := context.Flags().Lookup("listen").Value.String() diff --git a/cmd/serve.go b/cmd/serve.go index 346fa4e5..f2679204 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -23,8 +23,22 @@ func aptlyServe(cmd *commander.Command, args []string) error { return commander.ErrCommandError } - if unix.Access(context.Config().RootDir, unix.W_OK) != nil { - return fmt.Errorf("Configured rootDir '%s' is inaccessible, check access rights", context.Config().RootDir) + // There are only two working options for aptly's rootDir: + // 1. rootDir does not exist, then we'll create it + // 2. rootDir exists and is writable + // anything else must fail. + // E.g.: Running the service under a different user may lead to a rootDir + // that exists but is not usable due to access permissions. + // Config loads and returns current configuration + _, err = os.Stat(context.Config().RootDir); + if err != nil { + if ! os.IsNotExist(err) { + return fmt.Errorf("Something went wrong, %v", err) + } + } else { + if unix.Access(context.Config().RootDir, unix.W_OK) != nil { + return fmt.Errorf("Configured rootDir '%s' is inaccessible, check access rights", context.Config().RootDir) + } } if context.CollectionFactory().PublishedRepoCollection().Len() == 0 { diff --git a/system/t07_serve/RootDirInaccessible_gold b/system/t07_serve/RootDirInaccessible_gold index 3d57b5c6..be33d2e7 100644 --- a/system/t07_serve/RootDirInaccessible_gold +++ b/system/t07_serve/RootDirInaccessible_gold @@ -1 +1 @@ -ERROR: Configured rootDir '/rootDir/does/not/exist' is inaccessible, check access rights +ERROR: Configured rootDir '/root' is inaccessible, check access rights diff --git a/system/t07_serve/__init__.py b/system/t07_serve/__init__.py index f7513745..9e7bcedc 100644 --- a/system/t07_serve/__init__.py +++ b/system/t07_serve/__init__.py @@ -19,7 +19,10 @@ class RootDirInaccessible(BaseTest): """ fixtureDB = False fixturePool = False - configOverride = { "rootDir": "/rootDir/does/not/exist" } + + configOverride = { + "rootDir": "/root" # any directory that exists but is not writable + } runCmd = "aptly serve -listen=127.0.0.1:8765" def run(self): From 4456f8da57876361e55d498637849a0eb04665c2 Mon Sep 17 00:00:00 2001 From: jola5 Date: Sun, 29 Jan 2017 12:25:38 +0100 Subject: [PATCH 5/7] Refactor --- cmd/api_serve.go | 14 +++----------- cmd/serve.go | 12 ++---------- system/t07_serve/RootDirInaccessible_gold | 2 +- utils/utils.go | 21 +++++++++++++++++++++ 4 files changed, 27 insertions(+), 22 deletions(-) diff --git a/cmd/api_serve.go b/cmd/api_serve.go index c008b355..7064a1aa 100644 --- a/cmd/api_serve.go +++ b/cmd/api_serve.go @@ -3,11 +3,10 @@ package cmd import ( "fmt" "github.com/smira/aptly/api" + "github.com/smira/aptly/utils" "github.com/smira/commander" "github.com/smira/flag" "net/http" - "os" - "golang.org/x/sys/unix" ) func aptlyAPIServe(cmd *commander.Command, args []string) error { @@ -26,16 +25,9 @@ func aptlyAPIServe(cmd *commander.Command, args []string) error { // anything else must fail. // E.g.: Running the service under a different user may lead to a rootDir // that exists but is not usable due to access permissions. - // Config loads and returns current configuration - _, err = os.Stat(context.Config().RootDir); + err = utils.DirIsAccessible(context.Config().RootDir) if err != nil { - if ! os.IsNotExist(err) { - return fmt.Errorf("Something went wrong, %v", err) - } - } else { - if unix.Access(context.Config().RootDir, unix.W_OK) != nil { - return fmt.Errorf("Configured rootDir '%s' is inaccessible, check access rights", context.Config().RootDir) - } + return err } listen := context.Flags().Lookup("listen").Value.String() diff --git a/cmd/serve.go b/cmd/serve.go index f2679204..03aabdf1 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -12,7 +12,6 @@ import ( "os" "sort" "strings" - "golang.org/x/sys/unix" ) func aptlyServe(cmd *commander.Command, args []string) error { @@ -29,16 +28,9 @@ func aptlyServe(cmd *commander.Command, args []string) error { // anything else must fail. // E.g.: Running the service under a different user may lead to a rootDir // that exists but is not usable due to access permissions. - // Config loads and returns current configuration - _, err = os.Stat(context.Config().RootDir); + err = utils.DirIsAccessible(context.Config().RootDir) if err != nil { - if ! os.IsNotExist(err) { - return fmt.Errorf("Something went wrong, %v", err) - } - } else { - if unix.Access(context.Config().RootDir, unix.W_OK) != nil { - return fmt.Errorf("Configured rootDir '%s' is inaccessible, check access rights", context.Config().RootDir) - } + return err } if context.CollectionFactory().PublishedRepoCollection().Len() == 0 { diff --git a/system/t07_serve/RootDirInaccessible_gold b/system/t07_serve/RootDirInaccessible_gold index be33d2e7..8933b9cb 100644 --- a/system/t07_serve/RootDirInaccessible_gold +++ b/system/t07_serve/RootDirInaccessible_gold @@ -1 +1 @@ -ERROR: Configured rootDir '/root' is inaccessible, check access rights +ERROR: '/root' is inaccessible, check access rights diff --git a/utils/utils.go b/utils/utils.go index d4d17a23..96d11f57 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -1,2 +1,23 @@ // Package utils collects various services: simple operations, compression, etc. package utils + +import ( + "fmt" + "os" + "golang.org/x/sys/unix" +) + +// check if directory exists and is accessible +func DirIsAccessible(filename string) error { + _, err := os.Stat(filename); + if err != nil { + if ! os.IsNotExist(err) { + return fmt.Errorf("Something went wrong, %v", err) + } + } else { + if unix.Access(filename, unix.W_OK) != nil { + return fmt.Errorf("'%s' is inaccessible, check access rights", filename) + } + } + return nil +} From 38a99178158f855e8f9a82b3ca6ae66a85fc963b Mon Sep 17 00:00:00 2001 From: jola5 Date: Thu, 9 Feb 2017 10:19:38 +0100 Subject: [PATCH 6/7] Handle dependencies in gomfile --- .travis.yml | 1 - Gomfile | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 4fcc813f..6174f0df 100644 --- a/.travis.yml +++ b/.travis.yml @@ -27,7 +27,6 @@ before_install: - mkdir -p $GOPATH/src/github.com/smira - ln -s $TRAVIS_BUILD_DIR $GOPATH/src/github.com/smira || true - cd $GOPATH/src/github.com/smira/aptly - - go get golang.org/x/sys/unix install: - make prepare diff --git a/Gomfile b/Gomfile index 929235a6..c96908e4 100644 --- a/Gomfile +++ b/Gomfile @@ -26,6 +26,7 @@ gom 'github.com/ugorji/go/codec', :commit => '71c2886f5a673a35f909803f38ece58101 gom 'github.com/vaughan0/go-ini', :commit => 'a98ad7ee00ec53921f08832bc06ecf7fd600e6a1' gom 'github.com/wsxiaoys/terminal/color', :commit => '5668e431776a7957528361f90ce828266c69ed08' gom 'golang.org/x/crypto/ssh/terminal', :commit => 'a7ead6ddf06233883deca151dffaef2effbf498f' +gom 'golang.org/x/sys/unix', :commit => '7a6e5648d140666db5d920909e082ca00a87ba2c' group :test do gom 'gopkg.in/check.v1' From 5a71847b7ff807b242e754bb6aed5d92f524e4fd Mon Sep 17 00:00:00 2001 From: jola5 Date: Wed, 15 Feb 2017 20:18:30 +0100 Subject: [PATCH 7/7] Simplify test implementation --- system/t07_serve/__init__.py | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/system/t07_serve/__init__.py b/system/t07_serve/__init__.py index 9e7bcedc..1fb6e655 100644 --- a/system/t07_serve/__init__.py +++ b/system/t07_serve/__init__.py @@ -23,29 +23,9 @@ class RootDirInaccessible(BaseTest): configOverride = { "rootDir": "/root" # any directory that exists but is not writable } + runCmd = "aptly serve -listen=127.0.0.1:8765" - - def run(self): - try: - proc = subprocess.Popen(shlex.split(self.runCmd), stderr=subprocess.STDOUT, stdout=subprocess.PIPE, bufsize=0) - time.sleep(1) - - conn = httplib.HTTPConnection("127.0.0.1", 8765) - conn.request("GET", "/") - r = conn.getresponse() - self.http_response = r.read() - output = os.read(proc.stdout.fileno(), 8192) - - except socket_error as serr: - if serr.errno != errno.ECONNREFUSED: - raise serr - - finally: - self.output, err = proc.communicate() - - def check(self): - self.check_output() - + expectedCode = 1 class Serve1Test(BaseTest): """