tests now use an external vault process

Replace use of core vault package for testing with an external vault process. Previously each set of tests that needed vault would create a vault service in the test process and all tests would use the default 'secret' kv path. Now it uses a separate kv secrets path for each set of tests, setting the version for each. The code for starting up the Vault process was based off what was already used for Consul via its sdk/testutils package. In the process various test conflicts were addressed and an external process (consul and vault) process leak was fixed [1]. [1] the leak was caused by use of log.Fatal which calls os.Exit which exist the process without running any cleanup code.
hashicorp · Jul 16, 2019 · 38a4740 · 38a4740
1 parent cb2ee2f
commit 38a4740
Show file tree

Hide file tree

Showing 9 changed files with 158 additions and 173 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -3,6 +3,7 @@ jobs:
   build:
     environment:
       CONSUL_VERSION: 1.5.1
+      VAULT_VERSION: 1.1.3
     docker:
       - image: circleci/golang:latest
     working_directory: /go/src/github.com/hashicorp/consul-template
@@ -12,4 +13,8 @@ jobs:
           sudo curl -sLo consul.zip https://releases.hashicorp.com/consul/${CONSUL_VERSION}/consul_${CONSUL_VERSION}_linux_amd64.zip
           unzip consul.zip
           sudo cp consul /usr/local/bin/
+      - run: |
+          sudo curl -sLo vault.zip https://releases.hashicorp.com/vault/${VAULT_VERSION}/vault_${VAULT_VERSION}_linux_amd64.zip
+          unzip vault.zip
+          sudo cp vault /usr/local/bin/
       - run: make test
diff --git a/README.md b/README.md
@@ -2227,7 +2227,7 @@ following to generate all binaries:
 $ make build
 ```
 
-If you want to run the tests, first [install consul locally](https://www.consul.io/docs/install/index.html), then:
+If you want to run the tests, first install [consul](https://www.consul.io/docs/install/index.html) and [vault](https://www.vaultproject.io/docs/install/) locally, then:
 
 ```shell
 $ make test

diff --git a/dependency/client_set_test.go b/dependency/client_set_test.go
@@ -7,46 +7,29 @@ import (
 )
 
 func TestClientSet_unwrapVaultToken(t *testing.T) {
-	t.Parallel()
+	// Don't use t.Parallel() here as the SetWrappingLookupFunc is a global
+	// setting and breaks other tests if run in parallel
 
-	clients, server := testVaultServer(t)
-	defer server.Stop()
-
-	vault := clients.vault.client
-
-	// Grab the original token
-	originalToken, err := vault.Auth().Token().LookupSelf()
-	if err != nil {
-		t.Fatal(err)
-	}
+	vault := testClients.Vault()
 
 	// Create a wrapped token
 	vault.SetWrappingLookupFunc(func(operation, path string) string {
 		return "30s"
 	})
+	defer vault.SetWrappingLookupFunc(nil)
+
 	wrappedToken, err := vault.Auth().Token().Create(&api.TokenCreateRequest{
 		Lease: "1h",
 	})
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	if err := clients.CreateVaultClient(&CreateVaultClientInput{
-		Address:     server.Address,
-		Token:       wrappedToken.WrapInfo.Token,
-		UnwrapToken: true,
-	}); err != nil {
-		t.Fatal(err)
-	}
-
-	newToken := clients.vault.client.Token()
-
-	if newToken == originalToken.Data["id"] {
-		t.Errorf("expected %q to not be %q", newToken, originalToken.Data["id"])
-	}
+	token := vault.Token()
 
-	if newToken == wrappedToken.WrapInfo.Token {
-		t.Errorf("expected %q to not be %q", newToken, wrappedToken.WrapInfo.Token)
+	if token == wrappedToken.WrapInfo.Token {
+		t.Errorf("expected %q to not be %q", token,
+			wrappedToken.WrapInfo.Token)
 	}
 
 	if _, err := vault.Auth().Token().LookupSelf(); err != nil {

diff --git a/dependency/dependency_test.go b/dependency/dependency_test.go
@@ -1,51 +1,44 @@
 package dependency
 
 import (
-	"context"
 	"fmt"
 	"io/ioutil"
 	"log"
-	"net"
 	"os"
+	"os/exec"
 	"reflect"
+	"runtime"
 	"testing"
-	"time"
 
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/testutil"
-	"github.com/hashicorp/vault/builtin/logical/pki"
-	"github.com/hashicorp/vault/builtin/logical/transit"
-	"github.com/hashicorp/vault/helper/namespace"
-	"github.com/hashicorp/vault/http"
-	"github.com/hashicorp/vault/logical"
-	"github.com/hashicorp/vault/physical/inmem"
-	"github.com/hashicorp/vault/vault"
-
-	hclog "github.com/hashicorp/go-hclog"
-	logicalKv "github.com/hashicorp/vault-plugin-secrets-kv"
+	vapi "github.com/hashicorp/vault/api"
 )
 
+const vaultAddr = "http://127.0.0.1:8200"
+const vaultToken = "a_token"
+
 var testConsul *testutil.TestServer
+var testVault *vaultServer
 var testClients *ClientSet
 
 func TestMain(m *testing.M) {
-	consul, err := testutil.NewTestServerConfig(func(c *testutil.TestServerConfig) {
-		c.LogLevel = "warn"
-		c.Stdout = ioutil.Discard
-		c.Stderr = ioutil.Discard
-	})
 	log.SetOutput(ioutil.Discard)
-	if err != nil {
-		log.Fatal(fmt.Errorf("failed to start consul server: %v", err))
-	}
-	testConsul = consul
-
+	runTestConsul()
+	runTestVault()
 	clients := NewClientSet()
 	if err := clients.CreateConsulClient(&CreateConsulClientInput{
 		Address: testConsul.HTTPAddr,
 	}); err != nil {
 		testConsul.Stop()
-		log.Fatal(err)
+		Fatalf("failed to create consul client: %v\n", err)
+	}
+	if err := clients.CreateVaultClient(&CreateVaultClientInput{
+		Address: vaultAddr,
+		Token:   vaultToken,
+	}); err != nil {
+		testVault.Stop()
+		Fatalf("failed to create vault client: %v\n", err)
 	}
 	testClients = clients
 
@@ -58,18 +51,20 @@ func TestMain(m *testing.M) {
 		},
 	}
 
-	if err := testClients.consul.client.Agent().ServiceRegister(serviceMetaService); err != nil {
-		panic(err)
+	consul_agent := testClients.consul.client.Agent()
+	if err := consul_agent.ServiceRegister(serviceMetaService); err != nil {
+		Fatalf("%v", err)
 	}
 
 	exitCh := make(chan int, 1)
 	func() {
 		defer func() {
-			// Attempt to recover from a panic and stop the server. If we don't stop
-			// it, the panic will cause the server to remain running in the
-			// background. Here we catch the panic and the re-raise it.
+			// Attempt to recover from a panic and stop the server. If we don't
+			// stop it, the panic will cause the server to remain running in
+			// the background. Here we catch the panic and the re-raise it.
 			if r := recover(); r != nil {
 				testConsul.Stop()
+				testVault.Stop()
 				panic(r)
 			}
 		}()
@@ -80,9 +75,79 @@ func TestMain(m *testing.M) {
 	exit := <-exitCh
 
 	testConsul.Stop()
+	testVault.Stop()
 	os.Exit(exit)
 }
 
+func runTestConsul() {
+	consul, err := testutil.NewTestServerConfig(
+		func(c *testutil.TestServerConfig) {
+			c.LogLevel = "warn"
+			c.Stdout = ioutil.Discard
+			c.Stderr = ioutil.Discard
+		})
+	if err != nil {
+		Fatalf("failed to start consul server: %v", err)
+	}
+	testConsul = consul
+}
+
+type vaultServer struct {
+	secrets_path string
+	cmd          *exec.Cmd
+}
+
+func runTestVault() {
+	path, err := exec.LookPath("vault")
+	if err != nil || path == "" {
+		Fatalf("vault not found on $PATH")
+	}
+	args := []string{"server", "-dev", "-dev-root-token-id", vaultToken}
+	cmd := exec.Command("vault", args...)
+	cmd.Stdout = ioutil.Discard
+	cmd.Stderr = ioutil.Discard
+
+	if err := cmd.Start(); err != nil {
+		Fatalf("vault failed to start: %v", err)
+	}
+	testVault = &vaultServer{
+		cmd: cmd,
+	}
+}
+
+func (v vaultServer) Stop() error {
+	if v.cmd != nil && v.cmd.Process != nil {
+		return v.cmd.Process.Signal(os.Interrupt)
+	}
+	return nil
+}
+
+func testVaultServer(secrets_path, version string) (*ClientSet, *vaultServer) {
+	vc := testClients.Vault()
+	if err := vc.Sys().Mount(secrets_path, &vapi.MountInput{
+		Type:        "kv",
+		Description: "test mount",
+		Options:     map[string]string{"version": version},
+	}); err != nil {
+		fmt.Println(err)
+		Fatalf("Error creating secrets engine: %s", err)
+	}
+	return testClients, &vaultServer{secrets_path: secrets_path}
+}
+
+func (v *vaultServer) CreateSecret(path string, data map[string]interface{},
+) error {
+	q, err := NewVaultWriteQuery(v.secrets_path+"/"+path, data)
+	if err != nil {
+		return err
+	}
+	_, err = q.writeSecret(testClients, &QueryOptions{})
+	if err != nil {
+		fmt.Println(err)
+	}
+	return err
+}
+
 func TestCanShare(t *testing.T) {
 	t.Parallel()
 
@@ -114,83 +179,7 @@ func TestDeepCopyAndSortTags(t *testing.T) {
 	}
 }
 
-type vaultServer struct {
-	Address string
-	Token   string
-
-	core *vault.Core
-	ln   net.Listener
-}
-
-func (s *vaultServer) Stop() {
-	s.ln.Close()
-}
-
-func (s *vaultServer) CreateSecret(path string, data map[string]interface{}) error {
-	req := &logical.Request{
-		Operation:   logical.UpdateOperation,
-		Path:        fmt.Sprintf("secret/%s", path),
-		Data:        data,
-		ClientToken: s.Token,
-	}
-	_, err := s.core.HandleRequest(namespace.RootContext(context.Background()), req)
-	return err
-}
-
-// testVaultServer is a helper for creating a Vault server and returning the
-// appropriate client to connect to it.
-func testVaultServer(t *testing.T) (*ClientSet, *vaultServer) {
-	inm, err := inmem.NewInmem(nil, hclog.NewNullLogger())
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	core, err := vault.NewCore(&vault.CoreConfig{
-		DisableMlock:    true,
-		DisableCache:    true,
-		DefaultLeaseTTL: 2 * time.Second,
-		MaxLeaseTTL:     3 * time.Second,
-		Logger:          hclog.NewNullLogger(),
-		Physical:        inm,
-		LogicalBackends: map[string]logical.Factory{
-			"pki":     pki.Factory,
-			"transit": transit.Factory,
-			"kv":      logicalKv.Factory,
-		},
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	keys, token := vault.TestCoreInit(t, core)
-
-	for _, key := range keys {
-		if _, err := vault.TestCoreUnseal(core, vault.TestKeyCopy(key)); err != nil {
-			t.Fatal(err)
-		}
-	}
-
-	sealed := core.Sealed()
-	if sealed {
-		t.Fatal("vault should not be sealed")
-	}
-
-	ln, addr := http.TestServer(t, core)
-	clients := NewClientSet()
-	if err := clients.CreateVaultClient(&CreateVaultClientInput{
-		Address: addr,
-		Token:   token,
-	}); err != nil {
-		ln.Close()
-		t.Fatal(err)
-	}
-
-	server := &vaultServer{
-		Address: addr,
-		Token:   token,
-		core:    core,
-		ln:      ln,
-	}
-
-	return clients, server
+func Fatalf(format string, args ...interface{}) {
+	fmt.Printf(format, args...)
+	runtime.Goexit()
 }
diff --git a/dependency/vault_agent_token_test.go b/dependency/vault_agent_token_test.go
@@ -2,7 +2,6 @@ package dependency
 
 import (
 	"io/ioutil"
-	"log"
 	"os"
 	"strings"
 	"testing"
@@ -12,12 +11,18 @@ import (
 )
 
 func TestVaultAgentTokenQuery_Fetch(t *testing.T) {
-	t.Parallel()
+	// Don't use t.Parallel() here as the SetToken() calls are global and break
+	// other tests if run in parallel
+
+	// reset token back to original
+	vc := testClients.Vault()
+	token := vc.Token()
+	defer vc.SetToken(token)
 
 	// Set up the Vault token file.
 	tokenFile, err := ioutil.TempFile("", "token1")
 	if err != nil {
-		log.Fatal(err)
+		t.Fatal(err)
 	}
 	defer os.Remove(tokenFile.Name())
 	renderer.AtomicWrite(tokenFile.Name(), false, []byte("token"), 0644, false)
@@ -27,8 +32,7 @@ func TestVaultAgentTokenQuery_Fetch(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	clientSet, vault := testVaultServer(t)
-	defer vault.Stop()
+	clientSet := testClients
 	_, _, err = d.Fetch(clientSet, nil)
 	if err != nil {
 		t.Fatal(err)