Merge pull request #431 from kmala/reg

kmala · web-flow · commit f010a60eb3b0 · 2016-10-03T18:25:05.000-06:00
ref(controller): use the env config instead of directly getting from environment
diff --git a/boot.go b/boot.go
@@ -81,7 +81,7 @@ func main() {
 				log.Printf("Starting health check server on port %d", cnf.HealthSrvPort)
 				healthSrvCh := make(chan error)
 				go func() {
-					if err := healthsrv.Start(cnf.HealthSrvPort, kubeClient.Namespaces(), storageDriver, circ); err != nil {
+					if err := healthsrv.Start(cnf, kubeClient.Namespaces(), storageDriver, circ); err != nil {
 						healthSrvCh <- err
 					}
 				}()
@@ -96,7 +96,7 @@ func main() {
 				log.Printf("Starting SSH server on %s:%d", cnf.SSHHostIP, cnf.SSHHostPort)
 				sshCh := make(chan int)
 				go func() {
-					sshCh <- pkg.RunBuilder(cnf.SSHHostIP, cnf.SSHHostPort, gitHomeDir, circ, pushLock)
+					sshCh <- pkg.RunBuilder(cnf, gitHomeDir, circ, pushLock)
 				}()
 
 				select {
diff --git a/pkg/builder.go b/pkg/builder.go
@@ -25,9 +25,9 @@ const (
 // Git.
 //
 // Run returns on of the Status* status code constants.
-func RunBuilder(sshHostIP string, sshHostPort int, gitHomeDir string, sshServerCircuit *sshd.Circuit, pushLock sshd.RepositoryLock) int {
-	address := fmt.Sprintf("%s:%d", sshHostIP, sshHostPort)
-	cfg, err := sshd.Configure()
+func RunBuilder(cnf *sshd.Config, gitHomeDir string, sshServerCircuit *sshd.Circuit, pushLock sshd.RepositoryLock) int {
+	address := fmt.Sprintf("%s:%d", cnf.SSHHostIP, cnf.SSHHostPort)
+	cfg, err := sshd.Configure(cnf)
 	if err != nil {
 		log.Err("SSH server configuration failed: %s", err)
 		return StatusLocalError
diff --git a/pkg/conf/config.go b/pkg/conf/config.go
@@ -10,7 +10,7 @@ import (
 )
 
 const (
-	builderKeyLocation  = "/var/run/secrets/api/auth/builder-key"
+	BuilderKeyLocation  = "/var/run/secrets/api/auth/builder-key"
 	storageCredLocation = "/var/run/secrets/deis/objectstore/creds/"
 	minioHostEnvVar     = "DEIS_MINIO_SERVICE_HOST"
 	minioPortEnvVar     = "DEIS_MINIO_SERVICE_PORT"
@@ -36,10 +36,10 @@ func EnvConfig(appName string, conf interface{}) error {
 }
 
 // GetBuilderKey returns the key to be used as token to interact with deis-controller
-func GetBuilderKey() (string, error) {
-	builderKeyBytes, err := ioutil.ReadFile(builderKeyLocation)
+func GetBuilderKey(builderKeyPath string) (string, error) {
+	builderKeyBytes, err := ioutil.ReadFile(builderKeyPath)
 	if err != nil {
-		return "", fmt.Errorf("couldn't get builder key from %s (%s)", builderKeyLocation, err)
+		return "", fmt.Errorf("couldn't get builder key from %s (%s)", builderKeyPath, err)
 	}
 	builderKey := strings.Trim(string(builderKeyBytes), "\n")
 	return builderKey, nil
diff --git a/pkg/conf/config_test.go b/pkg/conf/config_test.go
@@ -6,6 +6,7 @@ import (
 	"os/user"
 	"testing"
 
+	"github.com/arschles/assert"
 	"github.com/deis/builder/pkg/sys"
 )
 
@@ -64,3 +65,27 @@ func TestGetStorageParams(t *testing.T) {
 		t.Errorf("received error while retrieving storage params: %v", err)
 	}
 }
+
+func TestGetControllerClient(t *testing.T) {
+	tmpDir, err := ioutil.TempDir("", "tmpdir")
+	if err != nil {
+		t.Fatalf("error creating temp directory (%s)", err)
+	}
+	data := []byte("testbuilderkey")
+	if err := ioutil.WriteFile(tmpDir+"/builder-key", data, 0644); err != nil {
+		t.Fatalf("error creating %s/builder-key (%s)", tmpDir, err)
+	}
+	defer func() {
+		if err := os.RemoveAll(tmpDir); err != nil {
+			t.Fatalf("failed to remove builder-key from %s (%s)", tmpDir, err)
+		}
+	}()
+	key, err := GetBuilderKey(tmpDir + "/builder-key")
+	assert.NoErr(t, err)
+	assert.Equal(t, key, string(data), "data")
+}
+
+func TestGetBuilderKeyError(t *testing.T) {
+	_, err := GetBuilderKey("/builder-key")
+	assert.True(t, err != nil, "no error received when there should have been")
+}
diff --git a/pkg/controller/utils.go b/pkg/controller/utils.go
@@ -2,30 +2,22 @@ package controller
 
 import (
 	"fmt"
-	"os"
 
 	"github.com/deis/builder/pkg/conf"
 	deis "github.com/deis/controller-sdk-go"
 	"github.com/deis/pkg/log"
 )
 
-const (
-	hostEnvName = "DEIS_CONTROLLER_SERVICE_HOST"
-	portEnvName = "DEIS_CONTROLLER_SERVICE_PORT"
-)
-
 // New creates a new SDK client configured as the builder.
-func New() (*deis.Client, error) {
-	host := os.Getenv(hostEnvName)
-	port := os.Getenv(portEnvName)
+func New(host, port, builderKeyPath string) (*deis.Client, error) {
 
 	client, err := deis.New(true, fmt.Sprintf("http://%s:%s/", host, port), "")
 	if err != nil {
 		return client, err
 	}
 	client.UserAgent = "deis-builder"
 
-	builderKey, err := conf.GetBuilderKey()
+	builderKey, err := conf.GetBuilderKey(builderKeyPath)
 	if err != nil {
 		return client, err
 	}
diff --git a/pkg/controller/utils_test.go b/pkg/controller/utils_test.go
@@ -0,0 +1,39 @@
+package controller
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"testing"
+
+	"github.com/arschles/assert"
+)
+
+func TestGetControllerClient(t *testing.T) {
+	tmpDir, err := ioutil.TempDir("", "tmpdir")
+	if err != nil {
+		t.Fatalf("error creating temp directory (%s)", err)
+	}
+	data := []byte("testbuilderkey")
+	if err := ioutil.WriteFile(tmpDir+"/builder-key", data, 0644); err != nil {
+		t.Fatalf("error creating %s/builder-key (%s)", tmpDir, err)
+	}
+	defer func() {
+		if err := os.RemoveAll(tmpDir); err != nil {
+			t.Fatalf("failed to remove builder-key from %s (%s)", tmpDir, err)
+		}
+	}()
+	host := "127.0.0.1"
+	port := "80"
+	cli, err := New(host, port, tmpDir+"/builder-key")
+	assert.NoErr(t, err)
+	assert.Equal(t, cli.ControllerURL.String(), fmt.Sprintf("http://%s:%s/", host, port), "data")
+	assert.Equal(t, cli.HooksToken, string(data), "data")
+}
+
+func TestGetControllerClientError(t *testing.T) {
+	host := "127.0.0.1"
+	port := "80"
+	_, err := New(host, port, "/builder-key")
+	assert.True(t, err != nil, "no error received when there should have been")
+}
diff --git a/pkg/gitreceive/build.go b/pkg/gitreceive/build.go
@@ -11,6 +11,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	builderconf "github.com/deis/builder/pkg/conf"
 	"github.com/deis/builder/pkg/controller"
 	"github.com/deis/builder/pkg/git"
 	"github.com/deis/builder/pkg/k8s"
@@ -90,7 +91,7 @@ func build(
 		}
 	}()
 
-	client, err := controller.New()
+	client, err := controller.New(conf.ControllerHost, conf.ControllerPort, builderconf.BuilderKeyLocation)
 	if err != nil {
 		return err
 	}
diff --git a/pkg/gitreceive/run.go b/pkg/gitreceive/run.go
@@ -27,7 +27,7 @@ func readLine(line string) (string, string, string, error) {
 func Run(conf *Config, fs sys.FS, env sys.Env, storageDriver storagedriver.StorageDriver) error {
 	log.Debug("Running git hook")
 
-	builderKey, err := builderconf.GetBuilderKey()
+	builderKey, err := builderconf.GetBuilderKey(builderconf.BuilderKeyLocation)
 	if err != nil {
 		return err
 	}
diff --git a/pkg/healthsrv/server.go b/pkg/healthsrv/server.go
@@ -4,21 +4,22 @@ import (
 	"fmt"
 	"net/http"
 
+	builderconf "github.com/deis/builder/pkg/conf"
 	"github.com/deis/builder/pkg/controller"
 	"github.com/deis/builder/pkg/sshd"
 )
 
 // Start starts the healthcheck server on :$port and blocks. It only returns if the server fails,
 // with the indicative error.
-func Start(port int, nsLister NamespaceLister, bLister BucketLister, sshServerCircuit *sshd.Circuit) error {
+func Start(cnf *sshd.Config, nsLister NamespaceLister, bLister BucketLister, sshServerCircuit *sshd.Circuit) error {
 	mux := http.NewServeMux()
-	client, err := controller.New()
+	client, err := controller.New(cnf.ControllerHost, cnf.ControllerPort, builderconf.BuilderKeyLocation)
 	if err != nil {
 		return err
 	}
 	mux.Handle("/healthz", healthZHandler(bLister, sshServerCircuit))
 	mux.Handle("/readiness", readinessHandler(client, nsLister))
 
-	hostStr := fmt.Sprintf(":%d", port)
+	hostStr := fmt.Sprintf(":%d", cnf.HealthSrvPort)
 	return http.ListenAndServe(hostStr, mux)
 }
diff --git a/pkg/sshd/config.go b/pkg/sshd/config.go
@@ -6,6 +6,8 @@ import (
 
 // Config represents the required SSH server configuration.
 type Config struct {
+	ControllerHost               string `envconfig:"DEIS_CONTROLLER_SERVICE_HOST" required:"true"`
+	ControllerPort               string `envconfig:"DEIS_CONTROLLER_SERVICE_PORT" required:"true"`
 	SSHHostIP                    string `envconfig:"SSH_HOST_IP" default:"0.0.0.0" required:"true"`
 	SSHHostPort                  int    `envconfig:"SSH_HOST_PORT" default:"2223" required:"true"`
 	HealthSrvPort                int    `envconfig:"HEALTH_SERVER_PORT" default:"8092"`
diff --git a/pkg/sshd/server.go b/pkg/sshd/server.go
@@ -11,6 +11,7 @@ import (
 	"net"
 	"strings"
 
+	builderconf "github.com/deis/builder/pkg/conf"
 	"github.com/deis/builder/pkg/controller"
 	"github.com/deis/builder/pkg/git"
 	"github.com/deis/controller-sdk-go/hooks"
@@ -34,9 +35,9 @@ var errDirPerm = errors.New("Cannot change directory in file name.")
 var errDirCreatePerm = errors.New("Empty repo name.")
 
 // AuthKey authenticates based on a public key.
-func AuthKey(key ssh.PublicKey) (*ssh.Permissions, error) {
+func AuthKey(key ssh.PublicKey, cnf *Config) (*ssh.Permissions, error) {
 	log.Info("Starting ssh authentication")
-	client, err := controller.New()
+	client, err := controller.New(cnf.ControllerHost, cnf.ControllerPort, builderconf.BuilderKeyLocation)
 	if err != nil {
 		return nil, err
 	}
@@ -72,10 +73,10 @@ func AuthKey(key ssh.PublicKey) (*ssh.Permissions, error) {
 //
 // Returns:
 //  An *ssh.ServerConfig
-func Configure() (*ssh.ServerConfig, error) {
+func Configure(cnf *Config) (*ssh.ServerConfig, error) {
 	cfg := &ssh.ServerConfig{
 		PublicKeyCallback: func(m ssh.ConnMetadata, k ssh.PublicKey) (*ssh.Permissions, error) {
-			return AuthKey(k)
+			return AuthKey(k, cnf)
 		},
 	}
 	hostKeyTypes := []string{"rsa", "dsa", "ecdsa"}

Original file line number	Diff line number	Diff line change
`@@ -11,6 +11,7 @@ import (`
`11`	`11`	`"path/filepath"`
`12`	`12`	`"strings"`
`13`	`13`
	`14`	`+ builderconf "github.com/deis/builder/pkg/conf"`
`14`	`15`	`"github.com/deis/builder/pkg/controller"`
`15`	`16`	`"github.com/deis/builder/pkg/git"`
`16`	`17`	`"github.com/deis/builder/pkg/k8s"`
`@@ -90,7 +91,7 @@ func build(`
`90`	`91`	`}`
`91`	`92`	`}()`
`92`	`93`
`93`		`- client, err := controller.New()`
	`94`	`+ client, err := controller.New(conf.ControllerHost, conf.ControllerPort, builderconf.BuilderKeyLocation)`
`94`	`95`	`if err != nil {`
`95`	`96`	`return err`
`96`	`97`	`}`
Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ func readLine(line string) (string, string, string, error) {`
`27`	`27`	`func Run(conf *Config, fs sys.FS, env sys.Env, storageDriver storagedriver.StorageDriver) error {`
`28`	`28`	`log.Debug("Running git hook")`
`29`	`29`
`30`		`- builderKey, err := builderconf.GetBuilderKey()`
	`30`	`+ builderKey, err := builderconf.GetBuilderKey(builderconf.BuilderKeyLocation)`
`31`	`31`	`if err != nil {`
`32`	`32`	`return err`
`33`	`33`	`}`