scottk/vault
1
0
Fork 0
mirror of https://github.com/optim-enterprises-bv/vault.git synced 2025-11-02 11:38:02 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix incorrect use of loop variable (#16872)

Browse Source 
This fixes a couple of references to loop variables in parallel tests
and deferred functions. When running a parallel test (calling
`t.Parallel()`) combined with the table-driven pattern, it's necessary
to copy the test case loop variable, otherwise only the last test case
is exercised. This is documented in the `testing` package:

https://pkg.go.dev/testing#hdr-Subtests_and_Sub_benchmarks

`defer` statements that invoke a closure should also not reference a
loop variable directly as the referenced value will change in each
iteration of the loop.

Issues were automatically found with the `loopvarcapture` linter.
This commit is contained in:
Renato Costa
2022-10-04 09:23:37 -04:00
committed by GitHub
parent 61707d56d5
commit eb338de819
5 changed files with 36 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
changelog/16872.txt Normal file
View File

@@ -0,0 +1,3 @@
```release-note:improvement
agent: fix incorrectly used loop variables in parallel tests and when finalizing seals
```

1
command/agent/approle_end_to_end_test.go
View File

@@ -54,6 +54,7 @@ func TestAppRoleEndToEnd(t *testing.T) {
if tc.removeSecretIDFile {
secretFileAction = "remove"
}
tc := tc // capture range variable
t.Run(fmt.Sprintf("%s_secret_id_file bindSecretID=%v secretIDLess=%v expectToken=%v", secretFileAction, tc.bindSecretID, tc.secretIDLess, tc.expectToken), func(t *testing.T) {
t.Parallel()
testAppRoleEndToEnd(t, tc.removeSecretIDFile, tc.bindSecretID, tc.secretIDLess, tc.expectToken)

1
command/kv_test.go
View File

@@ -716,6 +716,7 @@ func TestKVPatchCommand_ArgValidation(t *testing.T) {
}
for _, tc := range cases {
tc := tc // capture range variable
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

3
command/operator_diagnose.go
View File

@@ -447,13 +447,13 @@ func (c *OperatorDiagnoseCommand) offlineDiagnostics(ctx context.Context) error
goto SEALFAIL
}
if seals != nil {
for _, seal := range seals {
// There is always one nil seal. We need to skip it so we don't start an empty Finalize-Seal-Shamir
// section.
if seal == nil {
continue
}
seal := seal // capture range variable
// Ensure that the seal finalizer is called, even if using verify-only
defer func(seal *vault.Seal) {
sealType := diagnose.CapitalizeFirstLetter((*seal).BarrierType().String())
@@ -467,7 +467,6 @@ func (c *OperatorDiagnoseCommand) offlineDiagnostics(ctx context.Context) error
finalizeSealSpan.End()
}(&seal)
}
}
if barrierSeal == nil {
diagnose.Fail(sealcontext, "Could not create barrier seal. No error was generated, but it is likely that the seal stanza is misconfigured. For guidance, see Vault's configuration documentation on the seal stanza.")

8
command/server.go
View File

@@ -1337,8 +1337,13 @@ func (c *ServerCommand) Run(args []string) int {
return 1
}
if seals != nil {
for _, seal := range seals {
// There is always one nil seal. We need to skip it so we don't start an empty Finalize-Seal-Shamir
// section.
if seal == nil {
continue
}
seal := seal // capture range variable
// Ensure that the seal finalizer is called, even if using verify-only
defer func(seal *vault.Seal) {
err = (*seal).Finalize(context.Background())
@@ -1347,7 +1352,6 @@ func (c *ServerCommand) Run(args []string) int {
}
}(&seal)
}
}
if barrierSeal == nil {
c.UI.Error("Could not create barrier seal! Most likely proper Seal configuration information was not set, but no error was generated.")