Skip to content

Commit 61b2f08

Browse files
torcolvinbbrks
andcommitted
[3.2.3 backport] CBG-4480: fail test harness if dev time assertions hit (#7313)
* CBG-4340 fail test harness if dev time assertions hit If a panic occurs in a goroutine, like ServeHTTP handler, it might be swallowed. * remove unreachable code * Update rest/handler.go --------- Co-authored-by: Ben Brooks <[email protected]>
1 parent 7024628 commit 61b2f08

File tree

6 files changed

+25
-5
lines changed

6 files changed

+25
-5
lines changed

base/audit_types.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
package base
1010

1111
import (
12+
"context"
1213
"encoding/json"
1314
"fmt"
1415
"reflect"
@@ -150,9 +151,9 @@ func (ed *EventDescriptor) expandOptionalFieldGroups(groups []fieldGroup) {
150151
}
151152
}
152153

153-
func (i AuditID) MustValidateFields(f AuditFields) {
154+
func (i AuditID) MustValidateFields(ctx context.Context, f AuditFields) {
154155
if err := i.ValidateFields(f); err != nil {
155-
panic(fmt.Errorf("audit event %q (%s) invalid:\n%v", i, AuditEvents[i].Name, err))
156+
AssertfCtx(ctx, "audit event %q (%s) invalid:\n%v", i, AuditEvents[i].Name, err)
156157
}
157158
}
158159

base/devmode.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@
88

99
package base
1010

11-
import "context"
11+
import (
12+
"context"
13+
"sync/atomic"
14+
)
1215

1316
// IsDevMode returns true when compiled with the `cb_sg_devmode` build tag, and false otherwise.
1417
//
@@ -18,6 +21,9 @@ func IsDevMode() bool {
1821
return cbSGDevModeBuildTagSet
1922
}
2023

24+
// DevModeAssertionFailures is a counter of the number of assertion failures that have occurred in dev mode. This will always be zero in non-dev mode.
25+
var DevModeAssertionFailures atomic.Uint32
26+
2127
// AssertfCtx panics when compiled with the `cb_sg_devmode` build tag, and just warns otherwise.
2228
// Callers must be aware that they are responsible for handling returns to cover the non-devmode warn case.
2329
func AssertfCtx(ctx context.Context, format string, args ...any) {

base/devmode_on.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,13 @@
1111

1212
package base
1313

14+
import (
15+
"context"
16+
)
17+
1418
const cbSGDevModeBuildTagSet = true
1519

16-
var assertLogFn logFn = PanicfCtx
20+
var assertLogFn logFn = func(ctx context.Context, format string, args ...any) {
21+
DevModeAssertionFailures.Add(1)
22+
PanicfCtx(ctx, format, args...)
23+
}

base/logger_audit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ func Audit(ctx context.Context, id AuditID, additionalData AuditFields) {
128128
globalFields = logger.globalFields
129129
}
130130
fields = expandFields(id, ctx, globalFields, additionalData)
131-
id.MustValidateFields(fields)
131+
id.MustValidateFields(ctx, fields)
132132
}
133133

134134
if !logger.shouldLog(id, ctx) {

base/main_test_bucket_pool.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -725,6 +725,11 @@ func TestBucketPoolMain(ctx context.Context, m *testing.M, bucketReadierFunc TBP
725725
GTestBucketPool = NewTestBucketPoolWithOptions(ctx, bucketReadierFunc, bucketInitFunc, options)
726726
teardownFuncs = append(teardownFuncs, func() { GTestBucketPool.Close(ctx) })
727727

728+
teardownFuncs = append(teardownFuncs, func() {
729+
if DevModeAssertionFailures.Load() > 0 {
730+
panic("Test harness failed due to failures from -tag cb_sg_devmode. Look at logs for panic statements.")
731+
}
732+
})
728733
// must be the last teardown function added to the list to correctly detect leaked goroutines
729734
teardownFuncs = append(teardownFuncs, SetUpTestGoroutineDump(m))
730735

rest/handler.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -965,6 +965,7 @@ func (h *handler) checkPublicAuth(dbCtx *db.DatabaseContext) (err error) {
965965
provider := dbCtx.Options.OIDCOptions.Providers.GetProviderForIssuer(h.ctx(), issuerUrlForDB(h, dbCtx.Name), testProviderAudiences)
966966
if provider != nil && provider.ValidationKey != nil {
967967
if base.StringDefault(provider.ClientID, "") == username && *provider.ValidationKey == password {
968+
auditFields = base.AuditFields{base.AuditFieldAuthMethod: "basic"}
968969
return nil
969970
}
970971
}

0 commit comments

Comments
 (0)