From bfd7cf73b3076d2fa7c89ac396945047820a5f9c Mon Sep 17 00:00:00 2001 From: Joe Kralicky Date: Mon, 4 Nov 2024 14:02:40 -0500 Subject: [PATCH] bugfixes --- config/envoyconfig/bootstrap_test.go | 2 +- config/envoyconfig/clusters_envoy_admin.go | 5 ++++- config/envoyconfig/protocols_int_test.go | 2 ++ config/envoyconfig/testdata/clusters.json | 1 + internal/benchmarks/latency_bench_test.go | 19 ++++++++++++++++--- internal/log/debug.go | 2 ++ internal/testenv/environment.go | 8 ++++++-- internal/testenv/logs.go | 14 +++++++------- pkg/cryptutil/tls.go | 8 ++++---- 9 files changed, 43 insertions(+), 18 deletions(-) diff --git a/config/envoyconfig/bootstrap_test.go b/config/envoyconfig/bootstrap_test.go index 2df7034b4..b7a285327 100644 --- a/config/envoyconfig/bootstrap_test.go +++ b/config/envoyconfig/bootstrap_test.go @@ -25,7 +25,7 @@ func TestBuilder_BuildBootstrapAdmin(t *testing.T) { "address": { "pipe": { "mode": 384, - "path": "`+envoyAdminAddressSockName+`" + "path": "/tmp/`+envoyAdminAddressSockName+`" } } } diff --git a/config/envoyconfig/clusters_envoy_admin.go b/config/envoyconfig/clusters_envoy_admin.go index 6e7c9d8b7..e0144c1e1 100644 --- a/config/envoyconfig/clusters_envoy_admin.go +++ b/config/envoyconfig/clusters_envoy_admin.go @@ -2,6 +2,8 @@ package envoyconfig import ( "context" + "os" + "path/filepath" envoy_config_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3" envoy_config_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" @@ -23,7 +25,8 @@ func (b *Builder) buildEnvoyAdminCluster(_ context.Context, _ *config.Config) (* Address: &envoy_config_core_v3.Address{ Address: &envoy_config_core_v3.Address_Pipe{ Pipe: &envoy_config_core_v3.Pipe{ - Path: envoyAdminAddressSockName, + Path: filepath.Join(os.TempDir(), envoyAdminAddressSockName), + Mode: uint32(envoyAdminAddressMode), }, }, }, diff --git a/config/envoyconfig/protocols_int_test.go b/config/envoyconfig/protocols_int_test.go index 454fc2096..9fd321eb7 100644 --- a/config/envoyconfig/protocols_int_test.go +++ b/config/envoyconfig/protocols_int_test.go @@ -22,6 +22,7 @@ import ( "github.com/pomerium/pomerium/config" "github.com/pomerium/pomerium/internal/testenv" "github.com/pomerium/pomerium/internal/testenv/scenarios" + "github.com/pomerium/pomerium/internal/testenv/snippets" "github.com/pomerium/pomerium/internal/testenv/upstreams" "github.com/pomerium/pomerium/internal/testenv/values" "github.com/pomerium/pomerium/pkg/cmd/pomerium" @@ -48,6 +49,7 @@ func TestH2C_v2(t *testing.T) { env.AddUpstream(up) env.Start() + snippets.WaitStartupComplete(env) t.Run("h2c", func(t *testing.T) { t.Parallel() diff --git a/config/envoyconfig/testdata/clusters.json b/config/envoyconfig/testdata/clusters.json index 4e8a159f6..e42f5b5ee 100644 --- a/config/envoyconfig/testdata/clusters.json +++ b/config/envoyconfig/testdata/clusters.json @@ -280,6 +280,7 @@ "endpoint": { "address": { "pipe": { + "mode": 384, "path": "/tmp/pomerium-envoy-admin.sock" } } diff --git a/internal/benchmarks/latency_bench_test.go b/internal/benchmarks/latency_bench_test.go index 735a7d202..f936d7d8d 100644 --- a/internal/benchmarks/latency_bench_test.go +++ b/internal/benchmarks/latency_bench_test.go @@ -15,10 +15,14 @@ import ( "github.com/stretchr/testify/assert" ) -var numRoutes int +var ( + numRoutes int + dumpErrLogs bool +) func init() { flag.IntVar(&numRoutes, "routes", 100, "number of routes") + flag.BoolVar(&dumpErrLogs, "dump-err-logs", false, "if the test fails, write all captured logs to a file (testdata/)") } func TestRequestLatency(t *testing.T) { @@ -49,13 +53,21 @@ func TestRequestLatency(t *testing.T) { snippets.WaitStartupComplete(env) out := testing.Benchmark(func(b *testing.B) { + b.ReportAllocs() b.RunParallel(func(pb *testing.PB) { - // rec := env.NewLogRecorder(testenv.WithSkipCloseDelay()) + var rec *testenv.LogRecorder + if dumpErrLogs { + rec = env.NewLogRecorder(testenv.WithSkipCloseDelay()) + } for pb.Next() { idx := rand.IntN(numRoutes) resp, err := up.Get(routes[idx], upstreams.AuthenticateAs(fmt.Sprintf("user%d@example.com", idx))) if !assert.NoError(b, err) { - // rec.DumpToFile(filepath.Join("testdata", strings.ReplaceAll(b.Name(), "/", "_"))) + filename := "TestRequestLatency_err.log" + if dumpErrLogs { + rec.DumpToFile(filename) + b.Logf("test logs written to %s", filename) + } return } @@ -67,6 +79,7 @@ func TestRequestLatency(t *testing.T) { } }) }) + t.Log(out) t.Logf("req/s: %f", float64(out.N)/out.T.Seconds()) diff --git a/internal/log/debug.go b/internal/log/debug.go index 1d4f38bb7..0afa5bc56 100644 --- a/internal/log/debug.go +++ b/internal/log/debug.go @@ -7,4 +7,6 @@ var ( DebugDisableZapLogger atomic.Bool // Debug option to suppress global warnings DebugDisableGlobalWarnings atomic.Bool + // Debug option to suppress global (non-warning) messages + DebugDisableGlobalMessages atomic.Bool ) diff --git a/internal/testenv/environment.go b/internal/testenv/environment.go index f79545ab8..df7691212 100644 --- a/internal/testenv/environment.go +++ b/internal/testenv/environment.go @@ -269,13 +269,17 @@ func New(t testing.TB, opts ...EnvironmentOption) Environment { writer := log.NewMultiWriter() silent := options.forceSilent || isSilent(t) if silent { + // this sets the global zap level to fatal, then resets the global zerolog + // level to debug log.SetLevel(zerolog.FatalLevel) zerolog.SetGlobalLevel(zerolog.DebugLevel) - log.DebugDisableGlobalWarnings.Store(true) - log.DebugDisableZapLogger.Store(true) } else { + log.SetLevel(zerolog.InfoLevel) writer.Add(os.Stdout) } + log.DebugDisableGlobalWarnings.Store(silent) + log.DebugDisableGlobalMessages.Store(silent) + log.DebugDisableZapLogger.Store(silent) setGrpcLoggerOnce.Do(func() { grpclog.SetLoggerV2(grpclog.NewLoggerV2WithVerbosity(io.Discard, io.Discard, io.Discard, 0)) }) diff --git a/internal/testenv/logs.go b/internal/testenv/logs.go index c2159bcf0..73789f10d 100644 --- a/internal/testenv/logs.go +++ b/internal/testenv/logs.go @@ -31,8 +31,8 @@ type LogRecorder struct { buf *buffer recordedLogs []map[string]any - closeOnce func() - collectLogsOnce sync.Once + removeGlobalWriterOnce func() + collectLogsOnce sync.Once } type LogRecorderOptions struct { @@ -132,14 +132,14 @@ func (e *environment) NewLogRecorder(opts ...LogRecorderOption) *LogRecorder { buf: newBuffer(), } e.logWriter.Add(lr.buf) - lr.closeOnce = sync.OnceFunc(func() { + lr.removeGlobalWriterOnce = sync.OnceFunc(func() { // wait for envoy access logs, which flush on a 1 second interval if !lr.skipCloseDelay { time.Sleep(1100 * time.Millisecond) } e.logWriter.Remove(lr.buf) }) - context.AfterFunc(e.ctx, lr.closeOnce) + context.AfterFunc(e.ctx, lr.removeGlobalWriterOnce) return lr } @@ -156,13 +156,13 @@ type ( // Close stops the log recorder. After calling this method, Logs() or Match() // can be called to inspect the logs that were captured. func (lr *LogRecorder) Close() { - lr.closeOnce() + lr.removeGlobalWriterOnce() } func (lr *LogRecorder) collectLogs(shouldClose bool) { if shouldClose { + lr.removeGlobalWriterOnce() lr.buf.Close() - lr.closeOnce() } lr.collectLogsOnce.Do(func() { recordedLogs := []map[string]any{} @@ -201,7 +201,7 @@ func (lr *LogRecorder) WaitForMatch(expectedLog map[string]any, timeout ...time. go func() { defer close(done) lr.collectLogs(false) - lr.closeOnce() + lr.removeGlobalWriterOnce() }() if len(timeout) != 0 { select { diff --git a/pkg/cryptutil/tls.go b/pkg/cryptutil/tls.go index 7900e9b69..0d8075686 100644 --- a/pkg/cryptutil/tls.go +++ b/pkg/cryptutil/tls.go @@ -1,7 +1,6 @@ package cryptutil import ( - "context" "crypto/tls" "crypto/x509" "encoding/base64" @@ -15,10 +14,9 @@ import ( // GetCertPool gets a cert pool for the given CA or CAFile. func GetCertPool(ca, caFile string) (*x509.CertPool, error) { - ctx := context.TODO() rootCAs, err := x509.SystemCertPool() if err != nil { - log.Ctx(ctx).Error().Err(err).Msg("pkg/cryptutil: failed getting system cert pool making new one") + log.Error().Err(err).Msg("pkg/cryptutil: failed getting system cert pool making new one") rootCAs = x509.NewCertPool() } if ca == "" && caFile == "" { @@ -40,7 +38,9 @@ func GetCertPool(ca, caFile string) (*x509.CertPool, error) { if ok := rootCAs.AppendCertsFromPEM(data); !ok { return nil, fmt.Errorf("failed to append any PEM-encoded certificates") } - log.Ctx(ctx).Debug().Msg("pkg/cryptutil: added custom certificate authority") + if !log.DebugDisableGlobalMessages.Load() { + log.Debug().Msg("pkg/cryptutil: added custom certificate authority") + } return rootCAs, nil }