Skip to content

Commit a22473d

Browse files
arschlesarschles
committed
Merge pull request #229 from arschles/storage-testcov
fix(*): creating and using interfaces for system tasks
2 parents 6955570 + fcb6977 commit a22473d

15 files changed

Lines changed: 300 additions & 21 deletions

File tree

‎boot.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/deis/builder/pkg/gitreceive/storage"
1515
"github.com/deis/builder/pkg/healthsrv"
1616
"github.com/deis/builder/pkg/sshd"
17+
"github.com/deis/builder/pkg/sys"
1718
pkglog "github.com/deis/pkg/log"
1819
kcl "k8s.io/kubernetes/pkg/client/unversioned"
1920
)
@@ -48,10 +49,12 @@ func main() {
4849
pkglog.Err("getting config for %s [%s]", serverConfAppName, err)
4950
os.Exit(1)
5051
}
52+
env := sys.RealEnv()
53+
fs := sys.RealFS()
5154
pushLock := sshd.NewInMemoryRepositoryLock()
5255
circ := sshd.NewCircuit()
5356

54-
s3Client, err := storage.GetClient(cnf.HealthSrvTestStorageRegion)
57+
s3Client, err := storage.GetClient(cnf.HealthSrvTestStorageRegion, fs, env)
5558
if err != nil {
5659
log.Printf("Error getting s3 client (%s)", err)
5760
os.Exit(1)
@@ -106,8 +109,10 @@ func main() {
106109
os.Exit(1)
107110
}
108111
cnf.CheckDurations()
112+
fs := sys.RealFS()
113+
env := sys.RealEnv()
109114

110-
if err := gitreceive.Run(cnf); err != nil {
115+
if err := gitreceive.Run(cnf, fs, env); err != nil {
111116
log.Printf("Error running git receive hook [%s]", err)
112117
os.Exit(1)
113118
}

‎pkg/gitreceive/build.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/deis/builder/pkg"
1616
"github.com/deis/builder/pkg/gitreceive/git"
1717
"github.com/deis/builder/pkg/gitreceive/storage"
18+
"github.com/deis/builder/pkg/sys"
1819
"github.com/deis/pkg/log"
1920
"gopkg.in/yaml.v2"
2021

@@ -41,7 +42,7 @@ func run(cmd *exec.Cmd) error {
4142
return cmd.Run()
4243
}
4344

44-
func build(conf *Config, s3Client *s3.S3, kubeClient *client.Client, builderKey, rawGitSha string) error {
45+
func build(conf *Config, s3Client *s3.S3, kubeClient *client.Client, fs sys.FS, env sys.Env, builderKey, rawGitSha string) error {
4546
repo := conf.Repository
4647
gitSha, err := git.NewSha(rawGitSha)
4748
if err != nil {
@@ -125,7 +126,7 @@ func build(conf *Config, s3Client *s3.S3, kubeClient *client.Client, builderKey,
125126
return fmt.Errorf("uploading %s to %s/%s (%v)", absAppTgz, conf.Bucket, slugBuilderInfo.TarKey(), err)
126127
}
127128

128-
creds := storage.CredsOK()
129+
creds := storage.CredsOK(fs)
129130

130131
var pod *api.Pod
131132
var buildPodName string

‎pkg/gitreceive/run.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"strings"
88

99
"github.com/deis/builder/pkg/gitreceive/storage"
10+
"github.com/deis/builder/pkg/sys"
1011
"github.com/deis/pkg/log"
1112

1213
builderconf "github.com/deis/builder/pkg/conf"
@@ -22,15 +23,15 @@ func readLine(line string) (string, string, string, error) {
2223
return spl[0], spl[1], spl[2], nil
2324
}
2425

25-
func Run(conf *Config) error {
26+
func Run(conf *Config, fs sys.FS, env sys.Env) error {
2627
log.Debug("Running git hook")
2728

2829
builderKey, err := builderconf.GetBuilderKey()
2930
if err != nil {
3031
return err
3132
}
3233

33-
s3Client, err := storage.GetClient(conf.StorageRegion)
34+
s3Client, err := storage.GetClient(conf.StorageRegion, fs, env)
3435
if err != nil {
3536
return fmt.Errorf("configuring S3 client (%s)", err)
3637
}
@@ -56,7 +57,7 @@ func Run(conf *Config) error {
5657
}
5758
// if we're processing a receive-pack on an existing repo, run a build
5859
if strings.HasPrefix(conf.SSHOriginalCommand, "git-receive-pack") {
59-
if err := build(conf, s3Client, kubeClient, builderKey, newRev); err != nil {
60+
if err := build(conf, s3Client, kubeClient, fs, env, builderKey, newRev); err != nil {
6061
return err
6162
}
6263
}

‎pkg/gitreceive/storage/auth.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ package storage
22

33
import (
44
"fmt"
5-
"io/ioutil"
65
"os"
76
"strings"
87

98
"github.com/aws/aws-sdk-go/aws/credentials"
9+
"github.com/deis/builder/pkg/sys"
1010
)
1111

1212
const (
@@ -24,9 +24,9 @@ var (
2424
// if a key exists but not a secret, or vice-versa, returns an error.
2525
// if both don't exist returns emptyAuth.
2626
// otherwise returns a valid auth
27-
func getAuth() (*credentials.Credentials, error) {
28-
accessKeyIDBytes, accessKeyErr := ioutil.ReadFile(accessKeyIDFile)
29-
accessSecretKeyBytes, accessSecretKeyErr := ioutil.ReadFile(accessSecretKeyFile)
27+
func getAuth(fs sys.FS) (*credentials.Credentials, error) {
28+
accessKeyIDBytes, accessKeyErr := fs.ReadFile(accessKeyIDFile)
29+
accessSecretKeyBytes, accessSecretKeyErr := fs.ReadFile(accessSecretKeyFile)
3030
if accessKeyErr == os.ErrNotExist && accessSecretKeyErr == os.ErrNotExist {
3131
return emptyAuth, nil
3232
}
@@ -43,8 +43,8 @@ func getAuth() (*credentials.Credentials, error) {
4343
}
4444

4545
// CredsOK checks if the required credentials to make a request exist
46-
func CredsOK() bool {
47-
cred, err := getAuth()
46+
func CredsOK(fs sys.FS) bool {
47+
cred, err := getAuth(fs)
4848
if err != nil {
4949
return false
5050
}

‎pkg/gitreceive/storage/auth_test.go‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
package storage
2+
3+
import (
4+
"testing"
5+
6+
"github.com/arschles/assert"
7+
"github.com/aws/aws-sdk-go/aws/credentials"
8+
"github.com/deis/builder/pkg/sys"
9+
)
10+
11+
func TestGetAuthEmptyAuth(t *testing.T) {
12+
fs := sys.NewFakeFS()
13+
creds, err := getAuth(fs)
14+
assert.NoErr(t, err)
15+
assert.Equal(t, creds, credentials.AnonymousCredentials, "returned credentials")
16+
}
17+
18+
func TestGetAuthMissingSecret(t *testing.T) {
19+
fs := sys.NewFakeFS()
20+
fs.Files[accessSecretKeyFile] = []byte("hello world")
21+
creds, err := getAuth(fs)
22+
assert.Err(t, err, errMissingKey)
23+
assert.True(t, creds == nil, "returned credentials were not nil")
24+
}
25+
26+
func TestGetAuthMissingKey(t *testing.T) {
27+
fs := sys.NewFakeFS()
28+
fs.Files[accessKeyIDFile] = []byte("hello world")
29+
creds, err := getAuth(fs)
30+
assert.Err(t, err, errMissingSecret)
31+
assert.True(t, creds == nil, "returned credentials were not nil")
32+
}
33+
34+
func TestGetAuthSuccess(t *testing.T) {
35+
fs := sys.NewFakeFS()
36+
fs.Files[accessKeyIDFile] = []byte("stuff")
37+
fs.Files[accessSecretKeyFile] = []byte("other stuff")
38+
creds, err := getAuth(fs)
39+
assert.NoErr(t, err)
40+
assert.True(t, creds != nil, "creds were nil when they shouldn't have been")
41+
}
42+
43+
func TestCredsOKFail(t *testing.T) {
44+
fs := sys.NewFakeFS()
45+
assert.False(t, CredsOK(fs), "true returned when there were no credentials")
46+
}
47+
48+
func TestCredsOKSuccess(t *testing.T) {
49+
fs := sys.NewFakeFS()
50+
fs.Files[accessKeyIDFile] = []byte("stuff")
51+
fs.Files[accessSecretKeyFile] = []byte("other stuff")
52+
assert.True(t, CredsOK(fs), "false returned when there were valid credentials")
53+
}

‎pkg/gitreceive/storage/client.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,17 @@ import (
44
"github.com/aws/aws-sdk-go/aws"
55
"github.com/aws/aws-sdk-go/aws/session"
66
"github.com/aws/aws-sdk-go/service/s3"
7+
"github.com/deis/builder/pkg/sys"
78
)
89

910
// GetClient returns a S3 API compatible storage client
10-
func GetClient(regionStr string) (*s3.S3, error) {
11-
auth, err := getAuth()
11+
func GetClient(regionStr string, fs sys.FS, env sys.Env) (*s3.S3, error) {
12+
auth, err := getAuth(fs)
1213
if err != nil {
1314
return nil, err
1415
}
1516

16-
endpoint, err := getEndpoint()
17+
endpoint, err := getEndpoint(env)
1718
if err != nil {
1819
return nil, err
1920
}

‎pkg/gitreceive/storage/client_test.go‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package storage
2+
3+
import (
4+
"fmt"
5+
"testing"
6+
7+
"github.com/arschles/assert"
8+
"github.com/deis/builder/pkg/sys"
9+
)
10+
11+
type getClientTestCase struct {
12+
fileSystem map[string][]byte
13+
envs map[string]string
14+
expectClient bool
15+
expectErr bool
16+
}
17+
18+
func TestGetClientEmptyAuth(t *testing.T) {
19+
testCases := []getClientTestCase{
20+
// no auth & outside storage
21+
getClientTestCase{
22+
fileSystem: make(map[string][]byte),
23+
envs: map[string]string{"DEIS_OUTSIDE_STORAGE": "http://outside.com"},
24+
expectClient: true,
25+
expectErr: false,
26+
},
27+
// invalid auth - missing secret
28+
getClientTestCase{
29+
fileSystem: map[string][]byte{accessKeyIDFile: []byte("access key")},
30+
envs: map[string]string{"DEIS_OUTSIDE_STORAGE": "http://outside.com"},
31+
expectClient: false,
32+
expectErr: true,
33+
},
34+
// invalid auth - missing key
35+
getClientTestCase{
36+
fileSystem: map[string][]byte{accessSecretKeyFile: []byte("access secret")},
37+
envs: map[string]string{"DEIS_OUTSIDE_STORAGE": "http://outside.com"},
38+
expectClient: false,
39+
expectErr: true,
40+
},
41+
// invalid endpoint
42+
getClientTestCase{
43+
fileSystem: make(map[string][]byte),
44+
envs: make(map[string]string),
45+
expectClient: false,
46+
expectErr: true,
47+
},
48+
// valid auth, outside storage
49+
getClientTestCase{
50+
fileSystem: map[string][]byte{accessKeyIDFile: []byte("access key"), accessSecretKeyFile: []byte("access secret")},
51+
envs: map[string]string{"DEIS_OUTSIDE_STORAGE": "http://outside.com"},
52+
expectClient: true,
53+
expectErr: false,
54+
},
55+
}
56+
for i, testCase := range testCases {
57+
fs := sys.NewFakeFS()
58+
fs.Files = testCase.fileSystem
59+
env := sys.NewFakeEnv()
60+
env.Envs = testCase.envs
61+
cl, err := GetClient("myrevion", fs, env)
62+
assert.Equal(t, testCase.expectClient, cl != nil, fmt.Sprintf("returned client %d", i))
63+
assert.Equal(t, testCase.expectErr, err != nil, fmt.Sprintf("returned error %d", i))
64+
}
65+
}

‎pkg/gitreceive/storage/endpoint.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ package storage
22

33
import (
44
"fmt"
5-
"os"
5+
6+
"github.com/deis/builder/pkg/sys"
67
)
78

89
const (
@@ -20,10 +21,10 @@ var (
2021
)
2122
)
2223

23-
func getEndpoint() (string, error) {
24-
mHost := os.Getenv(minioHostEnvVar)
25-
mPort := os.Getenv(minioPortEnvVar)
26-
S3EP := os.Getenv(outsideStorageEndpoint)
24+
func getEndpoint(env sys.Env) (string, error) {
25+
mHost := env.Get(minioHostEnvVar)
26+
mPort := env.Get(minioPortEnvVar)
27+
S3EP := env.Get(outsideStorageEndpoint)
2728
if S3EP != "" {
2829
return S3EP, nil
2930
} else if mHost != "" && mPort != "" {

‎pkg/gitreceive/storage/endpoint_test.go‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package storage
2+
3+
import (
4+
"testing"
5+
6+
"github.com/arschles/assert"
7+
"github.com/deis/builder/pkg/sys"
8+
)
9+
10+
type getEndpointTestCase struct {
11+
envVars map[string]string
12+
expectedOut string
13+
expectedErr error
14+
}
15+
16+
func TestGetEndpoint(t *testing.T) {
17+
testCases := []getEndpointTestCase{
18+
getEndpointTestCase{
19+
envVars: map[string]string{"DEIS_OUTSIDE_STORAGE": "http://outside.storage.com"},
20+
expectedOut: "http://outside.storage.com",
21+
},
22+
getEndpointTestCase{
23+
envVars: map[string]string{"DEIS_OUTSIDE_STORAGE": "https://outside.com"},
24+
expectedOut: "https://outside.com",
25+
},
26+
getEndpointTestCase{
27+
envVars: map[string]string{
28+
"DEIS_OUTSIDE_STORAGE": "outside.com",
29+
"DEIS_MINIO_SERVICE_HOST": "minio.com",
30+
"DEIS_MINIO_SERVICE_PORT": "8888",
31+
},
32+
expectedOut: "outside.com",
33+
},
34+
getEndpointTestCase{
35+
envVars: map[string]string{
36+
"DEIS_MINIO_SERVICE_HOST": "minio.com",
37+
"DEIS_MINIO_SERVICE_PORT": "8888",
38+
},
39+
expectedOut: "http://minio.com:8888",
40+
},
41+
getEndpointTestCase{
42+
envVars: map[string]string{
43+
"DEIS_MINIO_SERVICE_HOST": "minio.com",
44+
},
45+
expectedErr: errNoStorageConfig,
46+
},
47+
getEndpointTestCase{
48+
envVars: map[string]string{
49+
"DEIS_MINIO_SERVICE_PORT": "9999",
50+
},
51+
expectedErr: errNoStorageConfig,
52+
},
53+
}
54+
for _, testCase := range testCases {
55+
fe := sys.NewFakeEnv()
56+
fe.Envs = testCase.envVars
57+
str, err := getEndpoint(fe)
58+
assert.Equal(t, str, testCase.expectedOut, "output")
59+
if testCase.expectedErr == nil {
60+
assert.NoErr(t, err)
61+
} else {
62+
assert.Equal(t, err, testCase.expectedErr, "error")
63+
}
64+
}
65+
}

0 commit comments

Comments
 (0)