Skip to content

Commit 2b0ec25

Browse files
hc-github-team-secure-vault-coreVioletHynes
hc-github-team-secure-vault-core
and
VioletHynes
authored
VAULT-43444 Addressed races in tests (#13278) (#13285)
* VAULT-43444 Addressed races in tests * more cleanup * make note better Co-authored-by: Violet Hynes <violet.hynes@hashicorp.com>
1 parent 7e587fd commit 2b0ec25

4 files changed

Lines changed: 67 additions & 51 deletions

File tree

‎vault/consumption_billing_util_test.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,6 @@ func TestHWMTotpKeyCounts(t *testing.T) {
566566

567567
// TestTransitDataProtectionCallCounts tests that we correctly store and track the transit data protection call counts
568568
func TestTransitDataProtectionCallCounts(t *testing.T) {
569-
t.Parallel()
570569
coreConfig := &CoreConfig{
571570
LogicalBackends: map[string]logical.Factory{
572571
"transit": transit.Factory,
@@ -819,7 +818,6 @@ func TestSSHCertCounts(t *testing.T) {
819818
// Round to 4 decimal places
820819
expectedCertUnit := math.Round(units*10000) / 10000
821820

822-
t.Parallel()
823821
coreConfig := &CoreConfig{
824822
LogicalBackends: map[string]logical.Factory{
825823
"ssh": ssh.Factory,
@@ -945,7 +943,6 @@ func TestSSHCertCounts(t *testing.T) {
945943
func TestSSHOTPCounts(t *testing.T) {
946944
expectedOTPUnit := 0.0014
947945

948-
t.Parallel()
949946
coreConfig := &CoreConfig{
950947
LogicalBackends: map[string]logical.Factory{
951948
"ssh": ssh.Factory,

‎vault/identity_store_group_aliases_test.go‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
credUserpass "github.com/hashicorp/vault/builtin/credential/userpass"
1212
"github.com/hashicorp/vault/helper/identity"
1313
"github.com/hashicorp/vault/helper/namespace"
14+
"github.com/hashicorp/vault/helper/testhelpers/corehelpers"
1415
"github.com/hashicorp/vault/sdk/logical"
1516
"github.com/kr/pretty"
1617
)
@@ -91,17 +92,14 @@ func TestIdentityStore_CaseInsensitiveGroupAliasName(t *testing.T) {
9192
}
9293

9394
func TestIdentityStore_EnsureNoDanglingGroupAlias(t *testing.T) {
94-
err := AddTestCredentialBackend("userpass", credUserpass.Factory)
95-
if err != nil {
96-
t.Fatal(err)
97-
}
98-
99-
err = AddTestCredentialBackend("ldap", credLdap.Factory)
100-
if err != nil {
101-
t.Fatal(err)
95+
conf := &CoreConfig{
96+
BuiltinRegistry: corehelpers.NewMockBuiltinRegistry(),
97+
CredentialBackends: map[string]logical.Factory{
98+
"ldap": credLdap.Factory,
99+
"userpass": credUserpass.Factory,
100+
},
102101
}
103-
104-
c, _, _ := TestCoreUnsealed(t)
102+
c, _, _ := TestCoreUnsealedWithConfig(t, conf)
105103

106104
ctx := namespace.RootContext(nil)
107105

@@ -111,7 +109,7 @@ func TestIdentityStore_EnsureNoDanglingGroupAlias(t *testing.T) {
111109
Type: "userpass",
112110
Description: "userpass",
113111
}
114-
err = c.enableCredential(ctx, userpassMe)
112+
err := c.enableCredential(ctx, userpassMe)
115113
if err != nil {
116114
t.Fatal(err)
117115
}

‎vault/identity_store_test.go‎

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -549,11 +549,13 @@ func TestIdentityStore_TokenEntityInheritance(t *testing.T) {
549549
}
550550

551551
func TestIdentityStore_MergeConflictingAliases(t *testing.T) {
552-
err := AddTestCredentialBackend("github", credGithub.Factory)
553-
if err != nil {
554-
t.Fatalf("err: %s", err)
552+
conf := &CoreConfig{
553+
BuiltinRegistry: corehelpers.NewMockBuiltinRegistry(),
554+
CredentialBackends: map[string]logical.Factory{
555+
"github": credGithub.Factory,
556+
},
555557
}
556-
c, _, _ := TestCoreUnsealed(t)
558+
c, _, _ := TestCoreUnsealedWithConfig(t, conf)
557559

558560
meGH := &MountEntry{
559561
Table: credentialTableType,
@@ -562,7 +564,7 @@ func TestIdentityStore_MergeConflictingAliases(t *testing.T) {
562564
Description: "github auth",
563565
}
564566

565-
err = c.enableCredential(namespace.RootContext(nil), meGH)
567+
err := c.enableCredential(namespace.RootContext(nil), meGH)
566568
if err != nil {
567569
t.Fatal(err)
568570
}
@@ -666,13 +668,13 @@ func testIdentityStoreWithGithubAuth(ctx context.Context, t *testing.T) (*Identi
666668
// backend to assist with testing aliases and entities that require an valid
667669
// mount accessor of an auth backend.
668670
func testIdentityStoreWithGithubAuthRoot(ctx context.Context, t *testing.T) (*IdentityStore, string, *Core, string) {
669-
// Add github credential factory to core config
670-
err := AddTestCredentialBackend("github", credGithub.Factory)
671-
if err != nil {
672-
t.Fatalf("err: %s", err)
671+
conf := &CoreConfig{
672+
BuiltinRegistry: corehelpers.NewMockBuiltinRegistry(),
673+
CredentialBackends: map[string]logical.Factory{
674+
"github": credGithub.Factory,
675+
},
673676
}
674-
675-
c, _, root := TestCoreUnsealed(t)
677+
c, _, root := TestCoreUnsealedWithConfig(t, conf)
676678

677679
meGH := &MountEntry{
678680
Table: credentialTableType,
@@ -681,27 +683,21 @@ func testIdentityStoreWithGithubAuthRoot(ctx context.Context, t *testing.T) (*Id
681683
Description: "github auth",
682684
}
683685

684-
err = c.enableCredential(ctx, meGH)
685-
if err != nil {
686-
t.Fatal(err)
687-
}
686+
err := c.enableCredential(ctx, meGH)
687+
require.NoError(t, err)
688688

689689
return c.identityStore, meGH.Accessor, c, root
690690
}
691691

692692
func testIdentityStoreWithGithubUserpassAuth(ctx context.Context, t *testing.T) (*IdentityStore, string, string, *Core) {
693-
// Setup 2 auth backends, github and userpass
694-
err := AddTestCredentialBackend("github", credGithub.Factory)
695-
if err != nil {
696-
t.Fatalf("err: %s", err)
697-
}
698-
699-
err = AddTestCredentialBackend("userpass", credUserpass.Factory)
700-
if err != nil {
701-
t.Fatalf("err: %s", err)
693+
conf := &CoreConfig{
694+
BuiltinRegistry: corehelpers.NewMockBuiltinRegistry(),
695+
CredentialBackends: map[string]logical.Factory{
696+
"github": credGithub.Factory,
697+
"userpass": credUserpass.Factory,
698+
},
702699
}
703-
704-
c, _, _ := TestCoreUnsealed(t)
700+
c, _, _ := TestCoreUnsealedWithConfig(t, conf)
705701

706702
githubMe := &MountEntry{
707703
Table: credentialTableType,
@@ -710,7 +706,7 @@ func testIdentityStoreWithGithubUserpassAuth(ctx context.Context, t *testing.T)
710706
Description: "github auth",
711707
}
712708

713-
err = c.enableCredential(ctx, githubMe)
709+
err := c.enableCredential(ctx, githubMe)
714710
if err != nil {
715711
t.Fatal(err)
716712
}
@@ -774,13 +770,13 @@ func expectSingleCount(t *testing.T, sink *metrics.InmemSink, keyPrefix string)
774770
}
775771

776772
func TestIdentityStore_NewEntityCounter(t *testing.T) {
777-
// Add github credential factory to core config
778-
err := AddTestCredentialBackend("github", credGithub.Factory)
779-
if err != nil {
780-
t.Fatalf("err: %s", err)
773+
conf := &CoreConfig{
774+
BuiltinRegistry: corehelpers.NewMockBuiltinRegistry(),
775+
CredentialBackends: map[string]logical.Factory{
776+
"github": credGithub.Factory,
777+
},
781778
}
782-
783-
c, _, _, sink := TestCoreUnsealedWithMetrics(t)
779+
c, _, _, sink := TestCoreUnsealedWithMetricsAndConfig(t, conf)
784780

785781
meGH := &MountEntry{
786782
Table: credentialTableType,
@@ -790,7 +786,7 @@ func TestIdentityStore_NewEntityCounter(t *testing.T) {
790786
}
791787

792788
ctx := namespace.RootContext(nil)
793-
err = c.enableCredential(ctx, meGH)
789+
err := c.enableCredential(ctx, meGH)
794790
if err != nil {
795791
t.Fatal(err)
796792
}

‎vault/testing.go‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,9 @@ oOyBJU/HMVvBfv4g+OVFLVgSwwm6owwsouZ0+D/LasbuHqYyqYqdyPJQYzWA2Y+F
102102
)
103103

104104
// TestCore returns a pure in-memory, uninitialized core for testing.
105+
//
106+
// NOTE: Writing your test using NewTestCluster in a new package is strongly
107+
// preferable to writing a test in the vault package utilizing the TestCore functions.
105108
func TestCore(t testing.TB) *Core {
106109
return TestCoreWithSeal(t, nil, false)
107110
}
@@ -121,6 +124,9 @@ func TestCoreNewSeal(t testing.TB) *Core {
121124

122125
// TestCoreWithConfig returns a pure in-memory, uninitialized core with the
123126
// specified core configurations overridden for testing.
127+
//
128+
// NOTE: Writing your test using NewTestCluster in a new package is strongly
129+
// preferable to writing a test in the vault package utilizing the TestCore functions.
124130
func TestCoreWithConfig(t testing.TB, conf *CoreConfig) *Core {
125131
return TestCoreWithSealAndUI(t, conf)
126132
}
@@ -387,6 +393,9 @@ func TestCoreSeal(core *Core) error {
387393

388394
// TestCoreUnsealed returns a pure in-memory core that is already
389395
// initialized and unsealed.
396+
//
397+
// NOTE: Writing your test using NewTestCluster in a new package is strongly
398+
// preferable to writing a test in the vault package utilizing the TestCore functions.
390399
func TestCoreUnsealed(t testing.TB) (*Core, [][]byte, string) {
391400
t.Helper()
392401
core := TestCore(t)
@@ -420,14 +429,20 @@ func TestCoreUnsealedWithMetricsAndConfig(t testing.TB, conf *CoreConfig) (*Core
420429

421430
// TestCoreUnsealedRaw returns a pure in-memory core that is already
422431
// initialized, unsealed, and with raw endpoints enabled.
432+
//
433+
// NOTE: Writing your test using NewTestCluster in a new package is strongly
434+
// preferable to writing a test in the vault package utilizing the TestCore functions.
423435
func TestCoreUnsealedRaw(t testing.TB) (*Core, [][]byte, string) {
424436
t.Helper()
425437
core := TestCoreRaw(t)
426438
return testCoreUnsealed(t, core)
427439
}
428440

429441
// TestCoreUnsealedWithConfig returns a pure in-memory core that is already
430-
// initialized, unsealed, with the any provided core config values overridden.
442+
// initialized, unsealed, with any provided core config values overridden.
443+
//
444+
// NOTE: Writing your test using NewTestCluster in a new package is strongly
445+
// preferable to writing a test in the vault package utilizing the TestCore functions.
431446
func TestCoreUnsealedWithConfig(t testing.TB, conf *CoreConfig) (*Core, [][]byte, string) {
432447
t.Helper()
433448
core := TestCoreWithConfig(t, conf)
@@ -559,8 +574,13 @@ var (
559574
testCredentialBackends = map[string]logical.Factory{}
560575
)
561576

562-
// This adds a credential backend for the test core. This needs to be
577+
// AddTestCredentialBackend adds a credential backend for the test core. This needs to be
563578
// invoked before the test core is created.
579+
//
580+
// Deprecated: Relies upon global test variables, and prone to race conditions,
581+
// and tests using this function cannot run in parallel.
582+
// A test utilizing NewTestCluster living outside the Vault package should
583+
// be strongly preferred instead.
564584
func AddTestCredentialBackend(name string, factory logical.Factory) error {
565585
if name == "" {
566586
return fmt.Errorf("missing backend name")
@@ -572,8 +592,13 @@ func AddTestCredentialBackend(name string, factory logical.Factory) error {
572592
return nil
573593
}
574594

575-
// This adds a logical backend for the test core. This needs to be
595+
// AddTestLogicalBackend adds a logical backend for the test core. This needs to be
576596
// invoked before the test core is created.
597+
//
598+
// Deprecated: Relies upon global test variables, and prone to race conditions,
599+
// and tests using this function cannot run in parallel.
600+
// A test utilizing NewTestCluster living outside the Vault package should
601+
// be strongly preferred instead.
577602
func AddTestLogicalBackend(name string, factory logical.Factory) error {
578603
if name == "" {
579604
return fmt.Errorf("missing backend name")

0 commit comments

Comments
 (0)
⚡