Backport Fix a test sdk bug relating to joining nodes when using autoseal into ce/main (#14427)

Co-authored-by: Nick Cabatoff <ncabatoff@hashicorp.com>

helper/testhelpers/seal/sealhelper.go

@@ -4,8 +4,10 @@
 package sealhelper
 
 import (
+	"fmt"
 	"path"
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/hashicorp/vault/api"
@@ -14,9 +16,12 @@ import (
 	"github.com/hashicorp/vault/helper/testhelpers/teststorage"
 	"github.com/hashicorp/vault/http"
 	"github.com/hashicorp/vault/internalshared/configutil"
+	"github.com/hashicorp/vault/sdk/helper/testcluster"
+	"github.com/hashicorp/vault/sdk/helper/testcluster/docker"
 	"github.com/hashicorp/vault/sdk/logical"
 	"github.com/hashicorp/vault/vault"
 	"github.com/hashicorp/vault/vault/seal"
+	"github.com/stretchr/testify/require"
 )
 
 type TransitSealServer struct {
@@ -78,3 +83,62 @@ func (tss *TransitSealServer) MakeSeal(t testing.TB, key string) (vault.Seal, er
 	}
 	return vault.NewAutoSeal(access), nil
 }
+
+type TransitDockerSealServer struct {
+	cluster *docker.DockerCluster
+	t       *testing.T
+}
+
+func NewTransitDockerSealServer(t *testing.T) *TransitDockerSealServer {
+	opts := docker.DefaultOptions(t)
+	opts.NumCores = 1
+	opts.ImageRepo, opts.ImageTag = "hashicorp/vault", "latest"
+	opts.VaultNodeConfig.StorageOptions = map[string]string{
+		"performance_multiplier": "1",
+	}
+	opts.DisableTLS = true // simplify, this way we don't have to deal with ca
+	opts.ClusterName = strings.ReplaceAll(t.Name()+"-transit", "/", "-")
+	return &TransitDockerSealServer{t: t, cluster: docker.NewTestDockerCluster(t, opts)}
+}
+
+func (tc *TransitDockerSealServer) APIClient() *api.Client {
+	return tc.cluster.Nodes()[0].APIClient()
+}
+
+func (tc *TransitDockerSealServer) SealWithPriorityAndDisabled(name string, idx int, disabled bool, priority int) testcluster.VaultNodeSealConfig {
+	seal := tc.Seal(name, idx)
+	seal.Config["disabled"] = strconv.FormatBool(disabled)
+	seal.Config["priority"] = strconv.Itoa(priority)
+	return seal
+}
+
+// Seal creates a seal using the given mount name and an idx that identifies a key.
+// The mount and key will be created.
+func (tc *TransitDockerSealServer) Seal(name string, idx int) testcluster.VaultNodeSealConfig {
+	client := tc.cluster.Nodes()[0].APIClient()
+	if m, _ := client.Sys().GetMount(name); m == nil {
+		require.NoError(tc.t, client.Sys().Mount(name, &api.MountInput{
+			Type: "transit",
+		}))
+	}
+
+	keyName := fmt.Sprintf("transit-seal-%d", idx+1)
+
+	_, err := client.Logical().Write(path.Join(name, "keys", keyName), nil)
+	require.NoError(tc.t, err)
+
+	return testcluster.VaultNodeSealConfig{
+		Type: "transit",
+		Config: map[string]string{
+			// For another docker container to talk to this cluster they
+			// must use the real api address, not the remapped localhost
+			// address test code uses.
+			"address":    tc.cluster.Nodes()[0].(*docker.DockerClusterNode).RealAPIAddr,
+			"token":      tc.cluster.GetRootToken(),
+			"mount_path": name,
+			"key_name":   keyName,
+			"name":       strings.ReplaceAll(name, " ", "_") + "-" + keyName,
+			"priority":   "1",
+		},
+	}
+}

sdk/helper/testcluster/docker/environment.go

@@ -1323,13 +1323,13 @@ func (dc *DockerCluster) setupDockerCluster(ctx context.Context, opts *DockerClu
 		if opts.SkipInit {
 			continue
 		}
+		hasSealConfig := opts.VaultNodeConfig != nil && len(opts.VaultNodeConfig.Seal) > 0
 		if i == 0 {
-			hasSealConfig := opts.VaultNodeConfig != nil && len(opts.VaultNodeConfig.Seal) > 0
 			if err := dc.setupNode0(ctx, hasSealConfig); err != nil {
 				return err
 			}
 		} else {
-			if err := dc.joinNode(ctx, i, 0, false); err != nil {
+			if err := dc.joinNode(ctx, i, 0, hasSealConfig); err != nil {
 				return err
 			}
 		}

vault/external_tests/raft/raft_binary/raft_test.go

@@ -17,6 +17,7 @@ import (
 	autopilot "github.com/hashicorp/raft-autopilot"
 	"github.com/hashicorp/vault/api"
 	"github.com/hashicorp/vault/helper/testhelpers"
+	sealhelper "github.com/hashicorp/vault/helper/testhelpers/seal"
 	"github.com/hashicorp/vault/sdk/helper/testcluster"
 	"github.com/hashicorp/vault/sdk/helper/testcluster/docker"
 	rafttest "github.com/hashicorp/vault/vault/external_tests/raft"
@@ -27,36 +28,48 @@ import (
 // uses docker containers for the vault nodes.
 func TestRaft_Configuration_Docker(t *testing.T) {
 	t.Parallel()
-	binary := os.Getenv("VAULT_BINARY")
-	if binary == "" {
-		t.Skip("only running docker test when $VAULT_BINARY present")
-	}
-	opts := &docker.DockerClusterOptions{
-		ImageRepo:    "hashicorp/vault",
-		DisableMlock: true,
-		// We're replacing the binary anyway, so we're not too particular about
-		// the docker image version tag.
-		ImageTag:    "latest",
-		VaultBinary: binary,
-		ClusterOptions: testcluster.ClusterOptions{
-			VaultNodeConfig: &testcluster.VaultNodeConfig{
-				LogLevel: "TRACE",
-				// If you want the test to run faster locally, you could
-				// uncomment this performance_multiplier change.
-				//StorageOptions: map[string]string{
-				//	"performance_multiplier": "1",
-				//},
-			},
-		},
-	}
-	cluster := docker.NewTestDockerCluster(t, opts)
-	defer cluster.Cleanup()
-	rafttest.Raft_Configuration_Test(t, cluster)
+	transit := sealhelper.NewTransitDockerSealServer(t)
 
-	if err := cluster.AddNode(context.Background(), opts); err != nil {
-		t.Fatal(err)
+	for _, tc := range []struct {
+		name  string
+		seals []testcluster.VaultNodeSealConfig
+	}{
+		{"shamir", nil},
+		{"autoseal", []testcluster.VaultNodeSealConfig{transit.Seal("test", 1)}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			binary := os.Getenv("VAULT_BINARY")
+			if binary == "" {
+				t.Skip("only running docker test when $VAULT_BINARY present")
+			}
+			opts := &docker.DockerClusterOptions{
+				ImageRepo:    "hashicorp/vault",
+				DisableMlock: true,
+				// We're replacing the binary anyway, so we're not too particular about
+				// the docker image version tag.
+				ImageTag:    "latest",
+				VaultBinary: binary,
+				ClusterOptions: testcluster.ClusterOptions{
+					VaultNodeConfig: &testcluster.VaultNodeConfig{
+						Seal:     tc.seals,
+						LogLevel: "TRACE",
+						// If you want the test to run faster locally, you could
+						// uncomment this performance_multiplier change.
+						//StorageOptions: map[string]string{
+						//	"performance_multiplier": "1",
+						//},
+					},
+				},
+			}
+			cluster := docker.NewTestDockerCluster(t, opts)
+			rafttest.Raft_Configuration_Test(t, cluster)
+
+			if err := cluster.AddNode(context.Background(), opts); err != nil {
+				t.Fatal(err)
+			}
+			rafttest.Raft_Configuration_Test(t, cluster)
+		})
 	}
-	rafttest.Raft_Configuration_Test(t, cluster)
 }
 
 // removeRaftNode removes a node from the raft configuration using the leader client

vault/external_tests/seal_binary/seal_docker_util.go

@@ -11,7 +11,6 @@ import (
 	"net/url"
 	"os"
 	"path"
-	"strconv"
 	"strings"
 	"sync"
 	"testing"
@@ -19,10 +18,8 @@ import (
 	"github.com/hashicorp/go-uuid"
 	"github.com/hashicorp/vault/api"
 	dockhelper "github.com/hashicorp/vault/sdk/helper/docker"
-	"github.com/hashicorp/vault/sdk/helper/testcluster"
 	"github.com/hashicorp/vault/sdk/helper/testcluster/docker"
 	client "github.com/moby/moby/client"
-	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -368,56 +365,6 @@ func dockerOptions(t *testing.T, repo, tag string) *docker.DockerClusterOptions
 	return opts
 }
 
-type transitCluster struct {
-	cluster *docker.DockerCluster
-	t       *testing.T
-}
-
-func newTransitCluster(t *testing.T) *transitCluster {
-	opts := dockerOptions(t, "hashicorp/vault", "latest")
-	opts.DisableTLS = true // simplify, this way we don't have to deal with ca
-	opts.ClusterName = strings.ReplaceAll(t.Name()+"-transit", "/", "-")
-	return &transitCluster{t: t, cluster: docker.NewTestDockerCluster(t, opts)}
-}
-
-func (tc *transitCluster) SealWithPriorityAndDisabled(name string, idx int, disabled bool, priority int) testcluster.VaultNodeSealConfig {
-	seal := tc.Seal(name, idx)
-	seal.Config["disabled"] = strconv.FormatBool(disabled)
-	seal.Config["priority"] = strconv.Itoa(priority)
-	return seal
-}
-
-// Seal creates a seal using the given mount name and an idx that identifies a key.
-// The mount and key will be created.
-func (tc *transitCluster) Seal(name string, idx int) testcluster.VaultNodeSealConfig {
-	client := tc.cluster.Nodes()[0].APIClient()
-	if m, _ := client.Sys().GetMount(name); m == nil {
-		require.NoError(tc.t, client.Sys().Mount(name, &api.MountInput{
-			Type: "transit",
-		}))
-	}
-
-	keyName := fmt.Sprintf("transit-seal-%d", idx+1)
-
-	_, err := client.Logical().Write(path.Join(name, "keys", keyName), nil)
-	require.NoError(tc.t, err)
-
-	return testcluster.VaultNodeSealConfig{
-		Type: "transit",
-		Config: map[string]string{
-			// For another docker container to talk to this cluster they
-			// must use the real api address, not the remapped localhost
-			// address test code uses.
-			"address":    tc.cluster.Nodes()[0].(*docker.DockerClusterNode).RealAPIAddr,
-			"token":      tc.cluster.GetRootToken(),
-			"mount_path": name,
-			"key_name":   keyName,
-			"name":       strings.ReplaceAll(name, " ", "_") + "-" + keyName,
-			"priority":   "1",
-		},
-	}
-}
-
 type logScanner struct {
 	wg   sync.WaitGroup
 	l    sync.Mutex