fix incorrect use of loop variable (#16872)

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 ff8d8338cc
commit ea1257be2e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 36 additions and 28 deletions

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
```

View File

@ -54,6 +54,7 @@ func TestAppRoleEndToEnd(t *testing.T) {
if tc.removeSecretIDFile { if tc.removeSecretIDFile {
secretFileAction = "remove" 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.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() t.Parallel()
testAppRoleEndToEnd(t, tc.removeSecretIDFile, tc.bindSecretID, tc.secretIDLess, tc.expectToken) testAppRoleEndToEnd(t, tc.removeSecretIDFile, tc.bindSecretID, tc.secretIDLess, tc.expectToken)

View File

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

View File

@ -447,26 +447,25 @@ func (c *OperatorDiagnoseCommand) offlineDiagnostics(ctx context.Context) error
goto SEALFAIL goto SEALFAIL
} }
if seals != nil { for _, seal := range seals {
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
// There is always one nil seal. We need to skip it so we don't start an empty Finalize-Seal-Shamir // section.
// section. if seal == nil {
if seal == nil { continue
continue
}
// Ensure that the seal finalizer is called, even if using verify-only
defer func(seal *vault.Seal) {
sealType := diagnose.CapitalizeFirstLetter((*seal).BarrierType().String())
finalizeSealContext, finalizeSealSpan := diagnose.StartSpan(ctx, "Finalize "+sealType+" Seal")
err = (*seal).Finalize(finalizeSealContext)
if err != nil {
diagnose.Fail(finalizeSealContext, "Error finalizing seal.")
diagnose.Advise(finalizeSealContext, "This likely means that the barrier is still in use; therefore, finalizing the seal timed out.")
finalizeSealSpan.End()
}
finalizeSealSpan.End()
}(&seal)
} }
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())
finalizeSealContext, finalizeSealSpan := diagnose.StartSpan(ctx, "Finalize "+sealType+" Seal")
err = (*seal).Finalize(finalizeSealContext)
if err != nil {
diagnose.Fail(finalizeSealContext, "Error finalizing seal.")
diagnose.Advise(finalizeSealContext, "This likely means that the barrier is still in use; therefore, finalizing the seal timed out.")
finalizeSealSpan.End()
}
finalizeSealSpan.End()
}(&seal)
} }
if barrierSeal == nil { if barrierSeal == nil {

View File

@ -1337,16 +1337,20 @@ func (c *ServerCommand) Run(args []string) int {
return 1 return 1
} }
if seals != nil { for _, seal := range seals {
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
// Ensure that the seal finalizer is called, even if using verify-only // section.
defer func(seal *vault.Seal) { if seal == nil {
err = (*seal).Finalize(context.Background()) continue
if err != nil {
c.UI.Error(fmt.Sprintf("Error finalizing seals: %v", err))
}
}(&seal)
} }
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())
if err != nil {
c.UI.Error(fmt.Sprintf("Error finalizing seals: %v", err))
}
}(&seal)
} }
if barrierSeal == nil { if barrierSeal == nil {