From 68dd2a66ba4327e89fe6e23b97a18dbd257d347e Mon Sep 17 00:00:00 2001 From: Jim Date: Sun, 1 Aug 2021 14:41:13 -0400 Subject: [PATCH] Remove unneeded hclog dependencies as we move forward with events (#1433) --- internal/auth/additional_verification_test.go | 1 - internal/cmd/base/dev.go | 2 +- internal/cmd/base/initial_resources.go | 10 +- internal/cmd/base/listener.go | 17 +- internal/cmd/base/profiling_off.go | 4 +- internal/cmd/base/profiling_on.go | 13 +- internal/cmd/base/servers.go | 2 +- internal/cmd/commands/dev/dev.go | 2 +- internal/cmd/commands/server/server.go | 2 +- .../server/worker_shutdown_reload_test.go | 11 +- internal/credential/vault/jobs.go | 43 ++--- internal/credential/vault/jobs_test.go | 179 ++++++------------ internal/kms/kms.go | 7 +- internal/kms/options.go | 9 - internal/libs/alpnmux/mux.go | 55 +----- internal/libs/alpnmux/mux_test.go | 7 +- internal/scheduler/scheduler.go | 10 +- internal/scheduler/scheduler_test.go | 18 +- internal/scheduler/testing.go | 3 +- internal/servers/controller/auth/auth.go | 4 - internal/servers/controller/auth/auth_test.go | 4 +- internal/servers/controller/auth/testing.go | 2 +- .../common/scopeids/scope_ids_test.go | 1 - internal/servers/controller/controller.go | 8 +- internal/servers/controller/handler.go | 6 +- internal/servers/controller/handler_noui.go | 5 +- internal/servers/controller/handler_ui.go | 2 +- .../authmethods/authmethod_service.go | 8 +- .../handlers/authmethods/oidc_test.go | 1 - .../authtokens/authtoken_service_test.go | 10 +- .../servers/controller/handlers/errors.go | 3 +- .../controller/handlers/errors_test.go | 3 +- .../controller/handlers/option_test.go | 10 - .../servers/controller/handlers/options.go | 9 - .../handlers/sessions/session_service_test.go | 4 +- .../handlers/targets/target_service_test.go | 7 +- .../handlers/workers/worker_service.go | 4 - internal/servers/controller/listeners.go | 2 +- .../servers/controller/session_cleanup_job.go | 6 - .../controller/session_cleanup_job_test.go | 15 +- internal/servers/worker/worker.go | 9 +- .../tests/cluster/session_cleanup_test.go | 12 +- internal/tests/helper/testing_helper.go | 38 ++-- 43 files changed, 147 insertions(+), 421 deletions(-) diff --git a/internal/auth/additional_verification_test.go b/internal/auth/additional_verification_test.go index 77b94b88b..a73c21f4b 100644 --- a/internal/auth/additional_verification_test.go +++ b/internal/auth/additional_verification_test.go @@ -88,7 +88,6 @@ func TestFetchActionSetForId(t *testing.T) { req := require.New(t) ctx := auth.NewVerifierContext( context.Background(), - tc.Logger(), iamRepoFn, authTokenRepoFn, serversRepoFn, diff --git a/internal/cmd/base/dev.go b/internal/cmd/base/dev.go index 20ddacdf6..6ecaec91b 100644 --- a/internal/cmd/base/dev.go +++ b/internal/cmd/base/dev.go @@ -332,7 +332,7 @@ func (b *Server) createInitialOidcAuthMethod(ctx context.Context) (*oidc.AuthMet if err != nil { return nil, fmt.Errorf("error creating kms repository: %w", err) } - kmsCache, err := kms.NewKms(kmsRepo, kms.WithLogger(b.Logger.Named("kms"))) + kmsCache, err := kms.NewKms(kmsRepo) if err != nil { return nil, fmt.Errorf("error creating kms cache: %w", err) } diff --git a/internal/cmd/base/initial_resources.go b/internal/cmd/base/initial_resources.go index ba25538a9..ecf2e34f6 100644 --- a/internal/cmd/base/initial_resources.go +++ b/internal/cmd/base/initial_resources.go @@ -23,7 +23,7 @@ func (b *Server) CreateInitialLoginRole(ctx context.Context) (*iam.Role, error) if err != nil { return nil, fmt.Errorf("error creating kms repository: %w", err) } - kmsCache, err := kms.NewKms(kmsRepo, kms.WithLogger(b.Logger.Named("kms"))) + kmsCache, err := kms.NewKms(kmsRepo) if err != nil { return nil, fmt.Errorf("error creating kms cache: %w", err) } @@ -80,7 +80,7 @@ func (b *Server) CreateInitialPasswordAuthMethod(ctx context.Context) (*password if err != nil { return nil, nil, fmt.Errorf("error creating kms repository: %w", err) } - kmsCache, err := kms.NewKms(kmsRepo, kms.WithLogger(b.Logger.Named("kms"))) + kmsCache, err := kms.NewKms(kmsRepo) if err != nil { return nil, nil, fmt.Errorf("error creating kms cache: %w", err) } @@ -278,7 +278,7 @@ func (b *Server) CreateInitialScopes(ctx context.Context) (*iam.Scope, *iam.Scop if err != nil { return nil, nil, fmt.Errorf("error creating kms repository: %w", err) } - kmsCache, err := kms.NewKms(kmsRepo, kms.WithLogger(b.Logger.Named("kms"))) + kmsCache, err := kms.NewKms(kmsRepo) if err != nil { return nil, nil, fmt.Errorf("error creating kms cache: %w", err) } @@ -358,7 +358,7 @@ func (b *Server) CreateInitialHostResources(ctx context.Context) (*static.HostCa if err != nil { return nil, nil, nil, fmt.Errorf("error creating kms repository: %w", err) } - kmsCache, err := kms.NewKms(kmsRepo, kms.WithLogger(b.Logger.Named("kms"))) + kmsCache, err := kms.NewKms(kmsRepo) if err != nil { return nil, nil, nil, fmt.Errorf("error creating kms cache: %w", err) } @@ -468,7 +468,7 @@ func (b *Server) CreateInitialTarget(ctx context.Context) (target.Target, error) if err != nil { return nil, fmt.Errorf("error creating kms repository: %w", err) } - kmsCache, err := kms.NewKms(kmsRepo, kms.WithLogger(b.Logger.Named("kms"))) + kmsCache, err := kms.NewKms(kmsRepo) if err != nil { return nil, fmt.Errorf("error creating kms cache: %w", err) } diff --git a/internal/cmd/base/listener.go b/internal/cmd/base/listener.go index 86d835dea..cde813fe4 100644 --- a/internal/cmd/base/listener.go +++ b/internal/cmd/base/listener.go @@ -5,7 +5,6 @@ import ( "fmt" "net" "net/http" - "os" "strings" "time" @@ -14,7 +13,6 @@ import ( _ "crypto/sha512" "github.com/hashicorp/boundary/internal/libs/alpnmux" - "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-secure-stdlib/listenerutil" "github.com/hashicorp/go-secure-stdlib/reloadutil" "github.com/mitchellh/cli" @@ -39,7 +37,7 @@ type WorkerAuthInfo struct { } // Factory is the factory function to create a listener. -type ListenerFactory func(string, *listenerutil.ListenerConfig, hclog.Logger, cli.Ui) (string, net.Listener, error) +type ListenerFactory func(string, *listenerutil.ListenerConfig, cli.Ui) (string, net.Listener, error) // BuiltinListeners is the list of built-in listener types. var BuiltinListeners = map[string]ListenerFactory{ @@ -49,7 +47,7 @@ var BuiltinListeners = map[string]ListenerFactory{ // New creates a new listener of the given type with the given // configuration. The type is looked up in the BuiltinListeners map. -func NewListener(l *listenerutil.ListenerConfig, logger hclog.Logger, ui cli.Ui) (*alpnmux.ALPNMux, map[string]string, reloadutil.ReloadFunc, error) { +func NewListener(l *listenerutil.ListenerConfig, ui cli.Ui) (*alpnmux.ALPNMux, map[string]string, reloadutil.ReloadFunc, error) { f, ok := BuiltinListeners[l.Type] if !ok { return nil, nil, nil, fmt.Errorf("unknown listener type: %q", l.Type) @@ -69,7 +67,7 @@ func NewListener(l *listenerutil.ListenerConfig, logger hclog.Logger, ui cli.Ui) l.TLSDisable = true } - finalAddr, ln, err := f(purpose, l, logger, ui) + finalAddr, ln, err := f(purpose, l, ui) if err != nil { return nil, nil, nil, err } @@ -83,10 +81,7 @@ func NewListener(l *listenerutil.ListenerConfig, logger hclog.Logger, ui cli.Ui) "addr": finalAddr, } - if _, ok := os.LookupEnv("BOUNDARY_LOG_CONNECTION_MUXING"); !ok { - logger = nil - } - alpnMux := alpnmux.New(ln, logger) + alpnMux := alpnmux.New(ln) if l.TLSDisable { return alpnMux, props, nil, nil @@ -115,7 +110,7 @@ func NewListener(l *listenerutil.ListenerConfig, logger hclog.Logger, ui cli.Ui) return alpnMux, props, reloadFunc, nil } -func tcpListenerFactory(purpose string, l *listenerutil.ListenerConfig, logger hclog.Logger, ui cli.Ui) (string, net.Listener, error) { +func tcpListenerFactory(purpose string, l *listenerutil.ListenerConfig, ui cli.Ui) (string, net.Listener, error) { if l.Address == "" { switch purpose { case "cluster": @@ -180,7 +175,7 @@ func tcpListenerFactory(purpose string, l *listenerutil.ListenerConfig, logger h return finalListenAddr, ln, nil } -func unixListenerFactory(purpose string, l *listenerutil.ListenerConfig, logger hclog.Logger, ui cli.Ui) (string, net.Listener, error) { +func unixListenerFactory(purpose string, l *listenerutil.ListenerConfig, ui cli.Ui) (string, net.Listener, error) { var uConfig *listenerutil.UnixSocketsConfig if l.SocketMode != "" && l.SocketUser != "" && diff --git a/internal/cmd/base/profiling_off.go b/internal/cmd/base/profiling_off.go index c1578a630..9b8d341e6 100644 --- a/internal/cmd/base/profiling_off.go +++ b/internal/cmd/base/profiling_off.go @@ -2,7 +2,7 @@ package base -import "github.com/hashicorp/go-hclog" +import "context" -func StartMemProfiler(_ hclog.Logger) { +func StartMemProfiler(_ context.Context) { } diff --git a/internal/cmd/base/profiling_on.go b/internal/cmd/base/profiling_on.go index e48731d2f..e16cdc827 100644 --- a/internal/cmd/base/profiling_on.go +++ b/internal/cmd/base/profiling_on.go @@ -8,14 +8,13 @@ import ( "runtime" "runtime/pprof" "time" - - "github.com/hashicorp/go-hclog" ) -func StartMemProfiler(logger hclog.Logger) { +func StartMemProfiler(ctx context.Context) { + const op = "base.StartMemProfiler" profileDir := filepath.Join(os.TempDir(), "boundaryprof") if err := os.MkdirAll(profileDir, 0o700); err != nil { - logger.Debug("could not create profile directory", "error", err) + event.WriteError(ctx, op, err,"could not create profile directory") return } @@ -24,14 +23,14 @@ func StartMemProfiler(logger hclog.Logger) { filename := filepath.Join(profileDir, time.Now().UTC().Format("20060102_150405")) + ".pprof" f, err := os.Create(filename) if err != nil { - logger.Debug("could not create memory profile", "error", err) + event.WriteError(ctx, op, err, event.WithInfo("could not create memory profile")) } runtime.GC() if err := pprof.WriteHeapProfile(f); err != nil { - logger.Debug("could not write memory profile", "error", err) + event.WriteError(ctx, op, err, "could not write memory profile")) } f.Close() - logger.Debug("wrote memory profile", "filename", filename) + event.WriteSysEvent(ctx, op, "wrote memory profile", "filename", filename) time.Sleep(5 * time.Minute) } }() diff --git a/internal/cmd/base/servers.go b/internal/cmd/base/servers.go index 3524e9583..9f8309bdf 100644 --- a/internal/cmd/base/servers.go +++ b/internal/cmd/base/servers.go @@ -395,7 +395,7 @@ func (b *Server) SetupListeners(ui cli.Ui, config *configutil.SharedConfig, allo } } - lnMux, props, reloadFunc, err := NewListener(lnConfig, b.Logger, ui) + lnMux, props, reloadFunc, err := NewListener(lnConfig, ui) if err != nil { return fmt.Errorf("Error initializing listener of type %s: %w", lnConfig.Type, err) } diff --git a/internal/cmd/commands/dev/dev.go b/internal/cmd/commands/dev/dev.go index edbb3d411..dafd979ad 100644 --- a/internal/cmd/commands/dev/dev.go +++ b/internal/cmd/commands/dev/dev.go @@ -499,7 +499,7 @@ func (c *Command) Run(args []string) int { // here) c.SetStatusGracePeriodDuration(0) - base.StartMemProfiler(c.Logger) + base.StartMemProfiler(ctx) if c.flagRecoveryKey != "" { c.Config.DevRecoveryKey = c.flagRecoveryKey diff --git a/internal/cmd/commands/server/server.go b/internal/cmd/commands/server/server.go index 29cf554ca..f36a01d44 100644 --- a/internal/cmd/commands/server/server.go +++ b/internal/cmd/commands/server/server.go @@ -178,7 +178,7 @@ func (c *Command) Run(args []string) int { // here) c.SetStatusGracePeriodDuration(0) - base.StartMemProfiler(c.Logger) + base.StartMemProfiler(ctx) if err := c.SetupKMSes(c.UI, c.Config); err != nil { c.UI.Error(err.Error()) diff --git a/internal/cmd/commands/server/worker_shutdown_reload_test.go b/internal/cmd/commands/server/worker_shutdown_reload_test.go index ee6a8af18..d349a73ad 100644 --- a/internal/cmd/commands/server/worker_shutdown_reload_test.go +++ b/internal/cmd/commands/server/worker_shutdown_reload_test.go @@ -16,7 +16,6 @@ import ( "github.com/hashicorp/boundary/internal/cmd/config" "github.com/hashicorp/boundary/internal/session" "github.com/hashicorp/boundary/internal/tests/helper" - "github.com/hashicorp/go-hclog" "github.com/mitchellh/cli" "github.com/stretchr/testify/require" "go.uber.org/atomic" @@ -86,10 +85,6 @@ listener "tcp" { ` func TestServer_ShutdownWorker(t *testing.T) { - logger := hclog.New(&hclog.LoggerOptions{ - Name: t.Name(), - Level: hclog.Trace, - }) require := require.New(t) controllerKey, workerAuthKey, _ := config.DevKeyGeneration() @@ -144,7 +139,7 @@ func TestServer_ShutdownWorker(t *testing.T) { require.NotNil(tgt) // Create test server, update default port on target - ts := helper.NewTestTcpServer(t, logger) + ts := helper.NewTestTcpServer(t) require.NotNil(ts) defer ts.Close() tgt, err = tcl.Update(ctx, tgt.Item.Id, tgt.Item.Version, targets.WithTcpTargetDefaultPort(ts.Port())) @@ -152,11 +147,11 @@ func TestServer_ShutdownWorker(t *testing.T) { require.NotNil(tgt) // Authorize and connect - sess := helper.NewTestSession(ctx, t, logger, tcl, "ttcp_1234567890") + sess := helper.NewTestSession(ctx, t, tcl, "ttcp_1234567890") sConn := sess.Connect(ctx, t) // Run initial send/receive test, make sure things are working - logger.Debug("running initial send/recv test") + t.Log("running initial send/recv test") sConn.TestSendRecvAll(t) // Now, shut the worker down. diff --git a/internal/credential/vault/jobs.go b/internal/credential/vault/jobs.go index aead405f7..29977dac6 100644 --- a/internal/credential/vault/jobs.go +++ b/internal/credential/vault/jobs.go @@ -11,7 +11,6 @@ import ( "github.com/hashicorp/boundary/internal/observability/event" "github.com/hashicorp/boundary/internal/oplog" "github.com/hashicorp/boundary/internal/scheduler" - "github.com/hashicorp/go-hclog" vault "github.com/hashicorp/vault/api" ua "go.uber.org/atomic" ) @@ -28,37 +27,37 @@ const ( renewalWindow = 10 * time.Minute ) -func RegisterJobs(ctx context.Context, scheduler *scheduler.Scheduler, r db.Reader, w db.Writer, kms *kms.Kms, logger hclog.Logger) error { +func RegisterJobs(ctx context.Context, scheduler *scheduler.Scheduler, r db.Reader, w db.Writer, kms *kms.Kms) error { const op = "vault.RegisterJobs" - tokenRenewal, err := newTokenRenewalJob(r, w, kms, logger) + tokenRenewal, err := newTokenRenewalJob(r, w, kms) if err != nil { return errors.Wrap(ctx, err, op) } if err = scheduler.RegisterJob(ctx, tokenRenewal); err != nil { return errors.Wrap(ctx, err, op, errors.WithMsg("token renewal job")) } - tokenRevoke, err := newTokenRevocationJob(r, w, kms, logger) + tokenRevoke, err := newTokenRevocationJob(r, w, kms) if err != nil { return errors.Wrap(ctx, err, op) } if err = scheduler.RegisterJob(ctx, tokenRevoke); err != nil { return errors.Wrap(ctx, err, op, errors.WithMsg("token revocation job")) } - credRenewal, err := newCredentialRenewalJob(r, w, kms, logger) + credRenewal, err := newCredentialRenewalJob(r, w, kms) if err != nil { return errors.Wrap(ctx, err, op) } if err = scheduler.RegisterJob(ctx, credRenewal); err != nil { return errors.Wrap(ctx, err, op, errors.WithMsg("credential renewal job")) } - credRevoke, err := newCredentialRevocationJob(r, w, kms, logger) + credRevoke, err := newCredentialRevocationJob(r, w, kms) if err != nil { return errors.Wrap(ctx, err, op) } if err = scheduler.RegisterJob(ctx, credRevoke); err != nil { return errors.Wrap(ctx, err, op, errors.WithMsg("credential revocation job")) } - credStoreCleanup, err := newCredentialStoreCleanupJob(r, w, kms, logger) + credStoreCleanup, err := newCredentialStoreCleanupJob(r, w, kms) if err != nil { return errors.Wrap(ctx, err, op) } @@ -82,7 +81,6 @@ type TokenRenewalJob struct { reader db.Reader writer db.Writer kms *kms.Kms - logger hclog.Logger limit int running ua.Bool @@ -93,7 +91,7 @@ type TokenRenewalJob struct { // newTokenRenewalJob creates a new in-memory TokenRenewalJob. // // WithLimit is the only supported option. -func newTokenRenewalJob(r db.Reader, w db.Writer, kms *kms.Kms, logger hclog.Logger, opt ...Option) (*TokenRenewalJob, error) { +func newTokenRenewalJob(r db.Reader, w db.Writer, kms *kms.Kms, opt ...Option) (*TokenRenewalJob, error) { const op = "vault.newTokenRenewalJob" switch { case r == nil: @@ -102,8 +100,6 @@ func newTokenRenewalJob(r db.Reader, w db.Writer, kms *kms.Kms, logger hclog.Log return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing db.Writer") case kms == nil: return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing kms") - case logger == nil: - return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing logger") } opts := getOpts(opt...) @@ -115,7 +111,6 @@ func newTokenRenewalJob(r db.Reader, w db.Writer, kms *kms.Kms, logger hclog.Log reader: r, writer: w, kms: kms, - logger: logger, limit: opts.withLimit, }, nil } @@ -308,7 +303,6 @@ type TokenRevocationJob struct { reader db.Reader writer db.Writer kms *kms.Kms - logger hclog.Logger limit int running ua.Bool @@ -319,7 +313,7 @@ type TokenRevocationJob struct { // newTokenRevocationJob creates a new in-memory TokenRevocationJob. // // WithLimit is the only supported option. -func newTokenRevocationJob(r db.Reader, w db.Writer, kms *kms.Kms, logger hclog.Logger, opt ...Option) (*TokenRevocationJob, error) { +func newTokenRevocationJob(r db.Reader, w db.Writer, kms *kms.Kms, opt ...Option) (*TokenRevocationJob, error) { const op = "vault.newTokenRevocationJob" switch { case r == nil: @@ -328,8 +322,6 @@ func newTokenRevocationJob(r db.Reader, w db.Writer, kms *kms.Kms, logger hclog. return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing db.Writer") case kms == nil: return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing kms") - case logger == nil: - return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing logger") } opts := getOpts(opt...) @@ -341,7 +333,6 @@ func newTokenRevocationJob(r db.Reader, w db.Writer, kms *kms.Kms, logger hclog. reader: r, writer: w, kms: kms, - logger: logger, limit: opts.withLimit, }, nil } @@ -476,7 +467,6 @@ type CredentialRenewalJob struct { reader db.Reader writer db.Writer kms *kms.Kms - logger hclog.Logger limit int running ua.Bool @@ -487,7 +477,7 @@ type CredentialRenewalJob struct { // newCredentialRenewalJob creates a new in-memory CredentialRenewalJob. // // WithLimit is the only supported option. -func newCredentialRenewalJob(r db.Reader, w db.Writer, kms *kms.Kms, logger hclog.Logger, opt ...Option) (*CredentialRenewalJob, error) { +func newCredentialRenewalJob(r db.Reader, w db.Writer, kms *kms.Kms, opt ...Option) (*CredentialRenewalJob, error) { const op = "vault.newCredentialRenewalJob" switch { case r == nil: @@ -496,8 +486,6 @@ func newCredentialRenewalJob(r db.Reader, w db.Writer, kms *kms.Kms, logger hclo return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing db.Writer") case kms == nil: return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing kms") - case logger == nil: - return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing logger") } opts := getOpts(opt...) @@ -509,7 +497,6 @@ func newCredentialRenewalJob(r db.Reader, w db.Writer, kms *kms.Kms, logger hclo reader: r, writer: w, kms: kms, - logger: logger, limit: opts.withLimit, }, nil } @@ -644,7 +631,6 @@ type CredentialRevocationJob struct { reader db.Reader writer db.Writer kms *kms.Kms - logger hclog.Logger limit int running ua.Bool @@ -655,7 +641,7 @@ type CredentialRevocationJob struct { // newCredentialRevocationJob creates a new in-memory CredentialRevocationJob. // // WithLimit is the only supported option. -func newCredentialRevocationJob(r db.Reader, w db.Writer, kms *kms.Kms, logger hclog.Logger, opt ...Option) (*CredentialRevocationJob, error) { +func newCredentialRevocationJob(r db.Reader, w db.Writer, kms *kms.Kms, opt ...Option) (*CredentialRevocationJob, error) { const op = "vault.newCredentialRevocationJob" switch { case r == nil: @@ -664,8 +650,6 @@ func newCredentialRevocationJob(r db.Reader, w db.Writer, kms *kms.Kms, logger h return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing db.Writer") case kms == nil: return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing kms") - case logger == nil: - return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing logger") } opts := getOpts(opt...) @@ -677,7 +661,6 @@ func newCredentialRevocationJob(r db.Reader, w db.Writer, kms *kms.Kms, logger h reader: r, writer: w, kms: kms, - logger: logger, limit: opts.withLimit, }, nil } @@ -790,7 +773,6 @@ type CredentialStoreCleanupJob struct { reader db.Reader writer db.Writer kms *kms.Kms - logger hclog.Logger limit int running ua.Bool @@ -801,7 +783,7 @@ type CredentialStoreCleanupJob struct { // newCredentialStoreCleanupJob creates a new in-memory CredentialStoreCleanupJob. // // No options are supported. -func newCredentialStoreCleanupJob(r db.Reader, w db.Writer, kms *kms.Kms, logger hclog.Logger, opt ...Option) (*CredentialStoreCleanupJob, error) { +func newCredentialStoreCleanupJob(r db.Reader, w db.Writer, kms *kms.Kms, opt ...Option) (*CredentialStoreCleanupJob, error) { const op = "vault.newCredentialStoreCleanupJob" switch { case r == nil: @@ -810,8 +792,6 @@ func newCredentialStoreCleanupJob(r db.Reader, w db.Writer, kms *kms.Kms, logger return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing db.Writer") case kms == nil: return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing kms") - case logger == nil: - return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing logger") } opts := getOpts(opt...) @@ -823,7 +803,6 @@ func newCredentialStoreCleanupJob(r db.Reader, w db.Writer, kms *kms.Kms, logger reader: r, writer: w, kms: kms, - logger: logger, limit: opts.withLimit, }, nil } diff --git a/internal/credential/vault/jobs_test.go b/internal/credential/vault/jobs_test.go index 33fe78a69..3dfc1c24c 100644 --- a/internal/credential/vault/jobs_test.go +++ b/internal/credential/vault/jobs_test.go @@ -16,7 +16,6 @@ import ( "github.com/hashicorp/boundary/internal/scheduler" "github.com/hashicorp/boundary/internal/session" "github.com/hashicorp/boundary/internal/target" - "github.com/hashicorp/go-hclog" wrapping "github.com/hashicorp/go-kms-wrapping" vault "github.com/hashicorp/vault/api" "github.com/jinzhu/gorm" @@ -154,10 +153,9 @@ func TestNewTokenRenewalJob(t *testing.T) { kmsCache := kms.TestKms(t, conn, wrapper) type args struct { - r db.Reader - w db.Writer - kms *kms.Kms - logger hclog.Logger + r db.Reader + w db.Writer + kms *kms.Kms } tests := []struct { name string @@ -190,32 +188,20 @@ func TestNewTokenRenewalJob(t *testing.T) { wantErrCode: errors.InvalidParameter, }, { - name: "nil logger", + name: "valid-no-options", args: args{ r: rw, w: rw, kms: kmsCache, }, - wantErr: true, - wantErrCode: errors.InvalidParameter, - }, - { - name: "valid-no-options", - args: args{ - r: rw, - w: rw, - kms: kmsCache, - logger: hclog.L(), - }, wantLimit: db.DefaultLimit, }, { name: "valid-with-limit", args: args{ - r: rw, - w: rw, - kms: kmsCache, - logger: hclog.L(), + r: rw, + w: rw, + kms: kmsCache, }, options: []Option{WithLimit(100)}, wantLimit: 100, @@ -226,7 +212,7 @@ func TestNewTokenRenewalJob(t *testing.T) { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - got, err := newTokenRenewalJob(tt.args.r, tt.args.w, tt.args.kms, tt.args.logger, tt.options...) + got, err := newTokenRenewalJob(tt.args.r, tt.args.w, tt.args.kms, tt.options...) if tt.wantErr { require.Error(err) assert.Nil(got) @@ -238,7 +224,6 @@ func TestNewTokenRenewalJob(t *testing.T) { assert.Equal(tt.args.r, got.reader) assert.Equal(tt.args.w, got.writer) assert.Equal(tt.args.kms, got.kms) - require.NotNil(got.logger) assert.Equal(tt.wantLimit, got.limit) }) } @@ -313,7 +298,7 @@ func TestTokenRenewalJob_RunLimits(t *testing.T) { require.NoError(err) assert.Equal(1, numRows) - r, err := newTokenRenewalJob(rw, rw, kmsCache, hclog.L(), tt.opts...) + r, err := newTokenRenewalJob(rw, rw, kmsCache, tt.opts...) require.NoError(err) err = r.Run(context.Background()) @@ -349,7 +334,7 @@ func TestTokenRenewalJob_Run(t *testing.T) { cs, err := repo.CreateCredentialStore(context.Background(), in) require.NoError(err) - r, err := newTokenRenewalJob(rw, rw, kmsCache, hclog.L()) + r, err := newTokenRenewalJob(rw, rw, kmsCache) require.NoError(err) err = sche.RegisterJob(context.Background(), r) @@ -455,7 +440,7 @@ func TestTokenRenewalJob_RunExpired(t *testing.T) { assert.NoError(err) require.NotNil(in) - r, err := newTokenRenewalJob(rw, rw, kmsCache, hclog.L()) + r, err := newTokenRenewalJob(rw, rw, kmsCache) require.NoError(err) err = sche.RegisterJob(context.Background(), r) @@ -568,7 +553,7 @@ func TestTokenRenewalJob_NextRunIn(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - r, err := newTokenRenewalJob(rw, rw, kmsCache, hclog.L()) + r, err := newTokenRenewalJob(rw, rw, kmsCache) assert.NoError(err) require.NotNil(r) @@ -612,10 +597,9 @@ func TestNewTokenRevocationJob(t *testing.T) { kmsCache := kms.TestKms(t, conn, wrapper) type args struct { - r db.Reader - w db.Writer - kms *kms.Kms - logger hclog.Logger + r db.Reader + w db.Writer + kms *kms.Kms } tests := []struct { name string @@ -648,32 +632,20 @@ func TestNewTokenRevocationJob(t *testing.T) { wantErrCode: errors.InvalidParameter, }, { - name: "nil logger", + name: "valid-no-options", args: args{ r: rw, w: rw, kms: kmsCache, }, - wantErr: true, - wantErrCode: errors.InvalidParameter, - }, - { - name: "valid-no-options", - args: args{ - r: rw, - w: rw, - kms: kmsCache, - logger: hclog.L(), - }, wantLimit: db.DefaultLimit, }, { name: "valid-with-limit", args: args{ - r: rw, - w: rw, - kms: kmsCache, - logger: hclog.L(), + r: rw, + w: rw, + kms: kmsCache, }, options: []Option{WithLimit(100)}, wantLimit: 100, @@ -684,7 +656,7 @@ func TestNewTokenRevocationJob(t *testing.T) { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - got, err := newTokenRevocationJob(tt.args.r, tt.args.w, tt.args.kms, tt.args.logger, tt.options...) + got, err := newTokenRevocationJob(tt.args.r, tt.args.w, tt.args.kms, tt.options...) if tt.wantErr { require.Error(err) assert.Nil(got) @@ -696,7 +668,6 @@ func TestNewTokenRevocationJob(t *testing.T) { assert.Equal(tt.args.r, got.reader) assert.Equal(tt.args.w, got.writer) assert.Equal(tt.args.kms, got.kms) - require.NotNil(got.logger) assert.Equal(tt.wantLimit, got.limit) }) } @@ -773,7 +744,7 @@ func TestTokenRevocationJob_RunLimits(t *testing.T) { require.NoError(err) assert.Equal(1, numRows) - r, err := newTokenRevocationJob(rw, rw, kmsCache, hclog.L(), tt.opts...) + r, err := newTokenRevocationJob(rw, rw, kmsCache, tt.opts...) require.NoError(err) err = r.Run(context.Background()) @@ -808,7 +779,7 @@ func TestTokenRevocationJob_Run(t *testing.T) { cs, err := repo.CreateCredentialStore(context.Background(), in) require.NoError(err) - r, err := newTokenRevocationJob(rw, rw, kmsCache, hclog.L()) + r, err := newTokenRevocationJob(rw, rw, kmsCache) require.NoError(err) err = sche.RegisterJob(context.Background(), r) @@ -927,10 +898,9 @@ func TestNewCredentialRenewalJob(t *testing.T) { kmsCache := kms.TestKms(t, conn, wrapper) type args struct { - r db.Reader - w db.Writer - kms *kms.Kms - logger hclog.Logger + r db.Reader + w db.Writer + kms *kms.Kms } tests := []struct { name string @@ -963,32 +933,20 @@ func TestNewCredentialRenewalJob(t *testing.T) { wantErrCode: errors.InvalidParameter, }, { - name: "nil logger", + name: "valid-no-options", args: args{ r: rw, w: rw, kms: kmsCache, }, - wantErr: true, - wantErrCode: errors.InvalidParameter, - }, - { - name: "valid-no-options", - args: args{ - r: rw, - w: rw, - kms: kmsCache, - logger: hclog.L(), - }, wantLimit: db.DefaultLimit, }, { name: "valid-with-limit", args: args{ - r: rw, - w: rw, - kms: kmsCache, - logger: hclog.L(), + r: rw, + w: rw, + kms: kmsCache, }, options: []Option{WithLimit(100)}, wantLimit: 100, @@ -999,7 +957,7 @@ func TestNewCredentialRenewalJob(t *testing.T) { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - got, err := newCredentialRenewalJob(tt.args.r, tt.args.w, tt.args.kms, tt.args.logger, tt.options...) + got, err := newCredentialRenewalJob(tt.args.r, tt.args.w, tt.args.kms, tt.options...) if tt.wantErr { require.Error(err) assert.Nil(got) @@ -1011,7 +969,6 @@ func TestNewCredentialRenewalJob(t *testing.T) { assert.Equal(tt.args.r, got.reader) assert.Equal(tt.args.w, got.writer) assert.Equal(tt.args.kms, got.kms) - require.NotNil(got.logger) assert.Equal(tt.wantLimit, got.limit) }) } @@ -1112,7 +1069,7 @@ func TestCredentialRenewalJob_RunLimits(t *testing.T) { testVaultCred(t, conn, v, cl, sess, credsToken, status, 5*time.Minute) } - r, err := newCredentialRenewalJob(rw, rw, kmsCache, hclog.L(), tt.opts...) + r, err := newCredentialRenewalJob(rw, rw, kmsCache, tt.opts...) require.NoError(err) err = r.Run(context.Background()) @@ -1174,7 +1131,7 @@ func TestCredentialRenewalJob_Run(t *testing.T) { csToken := allocToken() require.NoError(rw.LookupWhere(context.Background(), &csToken, "token_hmac = ?", []interface{}{cs.outputToken.TokenHmac})) - credRenewal, err := newCredentialRenewalJob(rw, rw, kmsCache, hclog.L()) + credRenewal, err := newCredentialRenewalJob(rw, rw, kmsCache) require.NoError(err) err = credRenewal.Run(context.Background()) @@ -1280,7 +1237,7 @@ func TestCredentialRenewalJob_RunExpired(t *testing.T) { repoToken := allocToken() require.NoError(rw.LookupWhere(context.Background(), &repoToken, "token_hmac = ?", []interface{}{cs.outputToken.TokenHmac})) - credRenewal, err := newCredentialRenewalJob(rw, rw, kmsCache, hclog.L()) + credRenewal, err := newCredentialRenewalJob(rw, rw, kmsCache) require.NoError(err) _, cred := testVaultCred(t, conn, v, cl, sess, repoToken, ActiveCredential, time.Minute) @@ -1439,7 +1396,7 @@ func TestCredentialRenewalJob_NextRunIn(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - r, err := newCredentialRenewalJob(rw, rw, kmsCache, hclog.L()) + r, err := newCredentialRenewalJob(rw, rw, kmsCache) assert.NoError(err) require.NotNil(r) @@ -1468,10 +1425,9 @@ func TestNewCredentialRevocationJob(t *testing.T) { kmsCache := kms.TestKms(t, conn, wrapper) type args struct { - r db.Reader - w db.Writer - kms *kms.Kms - logger hclog.Logger + r db.Reader + w db.Writer + kms *kms.Kms } tests := []struct { name string @@ -1504,32 +1460,20 @@ func TestNewCredentialRevocationJob(t *testing.T) { wantErrCode: errors.InvalidParameter, }, { - name: "nil logger", + name: "valid-no-options", args: args{ r: rw, w: rw, kms: kmsCache, }, - wantErr: true, - wantErrCode: errors.InvalidParameter, - }, - { - name: "valid-no-options", - args: args{ - r: rw, - w: rw, - kms: kmsCache, - logger: hclog.L(), - }, wantLimit: db.DefaultLimit, }, { name: "valid-with-limit", args: args{ - r: rw, - w: rw, - kms: kmsCache, - logger: hclog.L(), + r: rw, + w: rw, + kms: kmsCache, }, options: []Option{WithLimit(100)}, wantLimit: 100, @@ -1540,7 +1484,7 @@ func TestNewCredentialRevocationJob(t *testing.T) { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - got, err := newCredentialRevocationJob(tt.args.r, tt.args.w, tt.args.kms, tt.args.logger, tt.options...) + got, err := newCredentialRevocationJob(tt.args.r, tt.args.w, tt.args.kms, tt.options...) if tt.wantErr { require.Error(err) assert.Nil(got) @@ -1552,7 +1496,6 @@ func TestNewCredentialRevocationJob(t *testing.T) { assert.Equal(tt.args.r, got.reader) assert.Equal(tt.args.w, got.writer) assert.Equal(tt.args.kms, got.kms) - require.NotNil(got.logger) assert.Equal(tt.wantLimit, got.limit) }) } @@ -1655,7 +1598,7 @@ func TestCredentialRevocationJob_RunLimits(t *testing.T) { testVaultCred(t, conn, v, cl, sess, repoToken, status, 5*time.Minute) } - r, err := newCredentialRevocationJob(rw, rw, kmsCache, hclog.L(), tt.opts...) + r, err := newCredentialRevocationJob(rw, rw, kmsCache, tt.opts...) require.NoError(err) err = r.Run(context.Background()) @@ -1717,7 +1660,7 @@ func TestCredentialRevocationJob_Run(t *testing.T) { repoToken := allocToken() require.NoError(rw.LookupWhere(context.Background(), &repoToken, "token_hmac = ?", []interface{}{cs.outputToken.TokenHmac})) - r, err := newCredentialRevocationJob(rw, rw, kmsCache, hclog.L()) + r, err := newCredentialRevocationJob(rw, rw, kmsCache) require.NoError(err) err = r.Run(context.Background()) @@ -1807,7 +1750,7 @@ func TestCredentialRevocationJob_RunDeleted(t *testing.T) { repoToken := allocToken() require.NoError(rw.LookupWhere(context.Background(), &repoToken, "token_hmac = ?", []interface{}{cs.outputToken.TokenHmac})) - r, err := newCredentialRevocationJob(rw, rw, kmsCache, hclog.L()) + r, err := newCredentialRevocationJob(rw, rw, kmsCache) require.NoError(err) secret, cred := testVaultCred(t, conn, v, cl, sess, repoToken, ActiveCredential, 5*time.Hour) @@ -1872,10 +1815,9 @@ func TestNewCredentialStoreCleanupJob(t *testing.T) { kmsCache := kms.TestKms(t, conn, wrapper) type args struct { - r db.Reader - w db.Writer - kms *kms.Kms - logger hclog.Logger + r db.Reader + w db.Writer + kms *kms.Kms } tests := []struct { name string @@ -1908,32 +1850,20 @@ func TestNewCredentialStoreCleanupJob(t *testing.T) { wantErrCode: errors.InvalidParameter, }, { - name: "nil logger", + name: "valid-no-options", args: args{ r: rw, w: rw, kms: kmsCache, }, - wantErr: true, - wantErrCode: errors.InvalidParameter, - }, - { - name: "valid-no-options", - args: args{ - r: rw, - w: rw, - kms: kmsCache, - logger: hclog.L(), - }, wantLimit: db.DefaultLimit, }, { name: "valid-with-limit", args: args{ - r: rw, - w: rw, - kms: kmsCache, - logger: hclog.L(), + r: rw, + w: rw, + kms: kmsCache, }, options: []Option{WithLimit(100)}, wantLimit: 100, @@ -1944,7 +1874,7 @@ func TestNewCredentialStoreCleanupJob(t *testing.T) { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - got, err := newCredentialStoreCleanupJob(tt.args.r, tt.args.w, tt.args.kms, tt.args.logger, tt.options...) + got, err := newCredentialStoreCleanupJob(tt.args.r, tt.args.w, tt.args.kms, tt.options...) if tt.wantErr { require.Error(err) assert.Nil(got) @@ -1956,7 +1886,6 @@ func TestNewCredentialStoreCleanupJob(t *testing.T) { assert.Equal(tt.args.r, got.reader) assert.Equal(tt.args.w, got.writer) assert.Equal(tt.args.kms, got.kms) - require.NotNil(got.logger) assert.Equal(tt.wantLimit, got.limit) }) } @@ -2000,7 +1929,7 @@ func TestCredentialStoreCleanupJob_Run(t *testing.T) { // create second token on cs2 secondToken := testVaultToken(t, conn, wrapper, v, cs2, MaintainingToken, time.Hour) - r, err := newCredentialStoreCleanupJob(rw, rw, kmsCache, hclog.L()) + r, err := newCredentialStoreCleanupJob(rw, rw, kmsCache) require.NoError(err) err = sche.RegisterJob(context.Background(), r) diff --git a/internal/kms/kms.go b/internal/kms/kms.go index 566d5ed30..11b2ddcdd 100644 --- a/internal/kms/kms.go +++ b/internal/kms/kms.go @@ -10,7 +10,6 @@ import ( "github.com/hashicorp/boundary/internal/db" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/types/scope" - "github.com/hashicorp/go-hclog" wrapping "github.com/hashicorp/go-kms-wrapping" "github.com/hashicorp/go-kms-wrapping/wrappers/aead" "github.com/hashicorp/go-kms-wrapping/wrappers/multiwrapper" @@ -51,7 +50,6 @@ func (e *ExternalWrappers) Recovery() wrapping.Wrapper { // never change, only be added or (eventually) removed, it opportunistically // caches, going to the database as needed. type Kms struct { - logger hclog.Logger // scopePurposeCache holds a per-scope-purpose multiwrapper containing the // current encrypting key and all previous key versions, for decryption @@ -65,17 +63,14 @@ type Kms struct { repo *Repository } -// NewKms takes in a repo and returns a Kms. Supported options: WithLogger. +// NewKms takes in a repo and returns a Kms. func NewKms(repo *Repository, opt ...Option) (*Kms, error) { const op = "kms.NewKms" if repo == nil { return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing underlying repo") } - opts := getOpts(opt...) - return &Kms{ - logger: opts.withLogger, externalScopeCache: make(map[string]*ExternalWrappers), repo: repo, }, nil diff --git a/internal/kms/options.go b/internal/kms/options.go index 4a19fb6e3..4efae20b5 100644 --- a/internal/kms/options.go +++ b/internal/kms/options.go @@ -2,7 +2,6 @@ package kms import ( "github.com/hashicorp/boundary/internal/db" - "github.com/hashicorp/go-hclog" wrapping "github.com/hashicorp/go-kms-wrapping" ) @@ -21,7 +20,6 @@ type Option func(*options) // options = how options are represented type options struct { withLimit int - withLogger hclog.Logger withRootWrapper wrapping.Wrapper withWorkerAuthWrapper wrapping.Wrapper withRecoveryWrapper wrapping.Wrapper @@ -43,13 +41,6 @@ func WithLimit(limit int) Option { } } -// WithLogger provides a logger to be used when needed -func WithLogger(l hclog.Logger) Option { - return func(o *options) { - o.withLogger = l - } -} - // WithRootWrapper sets the external root wrapper for a given scope func WithRootWrapper(w wrapping.Wrapper) Option { return func(o *options) { diff --git a/internal/libs/alpnmux/mux.go b/internal/libs/alpnmux/mux.go index 69d94a413..1615085c4 100644 --- a/internal/libs/alpnmux/mux.go +++ b/internal/libs/alpnmux/mux.go @@ -11,7 +11,6 @@ import ( "sync" "github.com/hashicorp/boundary/internal/observability/event" - "github.com/hashicorp/go-hclog" ) const ( @@ -50,16 +49,14 @@ type muxedListener struct { type ALPNMux struct { ctx context.Context baseLn net.Listener - log hclog.Logger cancel context.CancelFunc muxMap *sync.Map } -func New(baseLn net.Listener, log hclog.Logger) *ALPNMux { +func New(baseLn net.Listener) *ALPNMux { ctx, cancel := context.WithCancel(context.Background()) ret := &ALPNMux{ ctx: ctx, - log: log, cancel: cancel, muxMap: new(sync.Map), baseLn: baseLn, @@ -76,13 +73,8 @@ func (l *ALPNMux) Close() error { return l.baseLn.Close() } -func (l *ALPNMux) SetLogger(log hclog.Logger) { - l.log = log -} - func (l *ALPNMux) RegisterProto(proto string, tlsConf *tls.Config) (net.Listener, error) { const op = "alpnmux.(ALPNMux).RegisterProto" - ctx := context.TODO() switch proto { case NoProto: if tlsConf != nil { @@ -112,16 +104,11 @@ func (l *ALPNMux) RegisterProto(proto string, tlsConf *tls.Config) (net.Listener go l.UnregisterProto(proto) } - if l.log != nil && l.log.IsDebug() { - event.WriteSysEvent(ctx, op, "registered", "proto", proto) - } - return sub, nil } func (l *ALPNMux) UnregisterProto(proto string) { const op = "alpnmux.(ALPNMux).UnregisterProto" - ctx := context.TODO() val, ok := l.muxMap.Load(proto) if !ok { return @@ -134,9 +121,6 @@ func (l *ALPNMux) UnregisterProto(proto string) { close(ml.connCh) }) l.muxMap.Delete(proto) - if l.log != nil && l.log.IsDebug() { - event.WriteSysEvent(ctx, op, "unregistered", "proto", proto) - } } func (l *ALPNMux) GetListener(proto string) net.Listener { @@ -152,13 +136,8 @@ func (l *ALPNMux) GetListener(proto string) net.Listener { func (l *ALPNMux) getConfigForClient(hello *tls.ClientHelloInfo) (*tls.Config, error) { const op = "alpnmux.(ALPNMux).getConfigForClient" - ctx := context.TODO() var ret *tls.Config - if l.log != nil && l.log.IsTrace() { - event.WriteSysEvent(ctx, op, "getting config for client", "supported_protos", hello.SupportedProtos, "server_name", hello.ServerName) - } - supportedProtos := hello.SupportedProtos if len(hello.SupportedProtos) == 0 { supportedProtos = append(supportedProtos, "") @@ -205,9 +184,6 @@ func (l *ALPNMux) accept() { if conn == nil { continue } - if l.log != nil && l.log.IsTrace() { - event.WriteSysEvent(ctx, op, "got connection", "addr", conn.RemoteAddr()) - } // Do the rest in a goroutine so that a timeout in e.g. handshaking // doesn't block acceptance of the next connection @@ -218,9 +194,6 @@ func (l *ALPNMux) accept() { } peeked, err := bufConn.buffer.Peek(3) if err != nil { - if l.log != nil && l.log.IsDebug() { - event.WriteError(ctx, op, err, event.WithInfoMsg("error peeking connection", "addr", conn.RemoteAddr())) - } bufConn.Close() return } @@ -228,20 +201,11 @@ func (l *ALPNMux) accept() { // First byte should always be a handshake, second byte a 3, and // third can be 3 or 1 depending on the implementation case peeked[0] != 0x16 || peeked[1] != 0x03 || (peeked[2] != 0x03 && peeked[2] != 0x01): - if l.log != nil && l.log.IsTrace() { - event.WriteSysEvent(ctx, op, "connection is not tls", "addr", conn.RemoteAddr()) - } val, ok := l.muxMap.Load(NoProto) if !ok { - if l.log != nil && l.log.IsTrace() { - event.WriteSysEvent(ctx, op, "no non-tls registration found", "addr", conn.RemoteAddr()) - } bufConn.Close() return } - if l.log != nil && l.log.IsTrace() { - event.WriteSysEvent(ctx, op, "found noproto handler", "addr", conn.RemoteAddr()) - } ml := val.(*muxedListener) ml.connMutex.RLock() if !ml.closed { @@ -250,38 +214,23 @@ func (l *ALPNMux) accept() { ml.connMutex.RUnlock() default: - if l.log != nil && l.log.IsTrace() { - event.WriteSysEvent(ctx, op, "connection is tls", "addr", conn.RemoteAddr()) - } tlsConn := tls.Server(bufConn, baseTLSConf) - if l.log != nil && l.log.IsTrace() { - event.WriteSysEvent(ctx, op, "handshaking", "addr", conn.RemoteAddr()) - } if err := tlsConn.Handshake(); err != nil { closeErr := tlsConn.Close() - if l.log != nil && l.log.IsDebug() { + if closeErr != nil { event.WriteError(ctx, op, err, event.WithInfoMsg("error handshaking connection", "addr", conn.RemoteAddr(), "close_error", closeErr)) } return } negProto := tlsConn.ConnectionState().NegotiatedProtocol - if l.log != nil && l.log.IsTrace() { - event.WriteSysEvent(ctx, op, "tls negotiated", "addr", conn.RemoteAddr(), "proto", negProto) - } val, ok := l.muxMap.Load(negProto) if !ok { val, ok = l.muxMap.Load(DefaultProto) if !ok { - if l.log != nil && l.log.IsTrace() { - event.WriteSysEvent(ctx, op, "no handler found", "addr", conn.RemoteAddr(), "proto", negProto) - } tlsConn.Close() return } } - if l.log != nil && l.log.IsTrace() { - event.WriteSysEvent(ctx, op, "found tls handler", "addr", conn.RemoteAddr(), "proto", negProto) - } ml := val.(*muxedListener) ml.connMutex.RLock() if !ml.closed { diff --git a/internal/libs/alpnmux/mux_test.go b/internal/libs/alpnmux/mux_test.go index 782d6fbd8..8ef7bad9f 100644 --- a/internal/libs/alpnmux/mux_test.go +++ b/internal/libs/alpnmux/mux_test.go @@ -27,7 +27,7 @@ func TestListenCloseErrMsg(t *testing.T) { func TestRegistrationErrors(t *testing.T) { listener := getListener(t) defer listener.Close() - mux := New(listener, nil) + mux := New(listener) p1config := getTestTLS(t, []string{"p1"}) if _, err := mux.RegisterProto("p1", nil); err.Error() != "nil tls config given" { t.Fatal(err) @@ -78,10 +78,7 @@ func TestListening(t *testing.T) { } listener := getListener(t) - logger := hclog.Default() - logger.SetLevel(hclog.Trace) - // log.SetOutput(logger.StandardWriter(new(hclog.StandardLoggerOptions))) - mux := New(listener, logger) + mux := New(listener) defer mux.Close() emptyconns := atomic.NewUint32(0) diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index 5a8a0d546..00daa022e 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -9,7 +9,6 @@ import ( "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/observability/event" "github.com/hashicorp/boundary/internal/scheduler/job" - "github.com/hashicorp/go-hclog" ua "go.uber.org/atomic" ) @@ -25,7 +24,6 @@ type runningJob struct { type Scheduler struct { serverId string jobRepoFn jobRepoFactory - logger hclog.Logger registeredJobs *sync.Map runningJobs *sync.Map @@ -43,11 +41,9 @@ type Scheduler struct { // // • jobRepoFn must be provided and is a function that returns the job repository // -// • logger must be provided and is used to log errors that occur during scheduling and running of jobs -// // WithRunJobsLimit, WithRunJobsInterval, WithMonitorInterval and WithInterruptThreshold are // the only valid options. -func New(serverId string, jobRepoFn jobRepoFactory, logger hclog.Logger, opt ...Option) (*Scheduler, error) { +func New(serverId string, jobRepoFn jobRepoFactory, opt ...Option) (*Scheduler, error) { const op = "scheduler.New" if serverId == "" { return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing server id") @@ -55,15 +51,11 @@ func New(serverId string, jobRepoFn jobRepoFactory, logger hclog.Logger, opt ... if jobRepoFn == nil { return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing job repo function") } - if logger == nil { - return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing logger") - } opts := getOpts(opt...) return &Scheduler{ serverId: serverId, jobRepoFn: jobRepoFn, - logger: logger, registeredJobs: new(sync.Map), runningJobs: new(sync.Map), runJobsLimit: opts.withRunJobsLimit, diff --git a/internal/scheduler/scheduler_test.go b/internal/scheduler/scheduler_test.go index 05fca0aab..ce40b9cc8 100644 --- a/internal/scheduler/scheduler_test.go +++ b/internal/scheduler/scheduler_test.go @@ -11,7 +11,6 @@ import ( "github.com/hashicorp/boundary/internal/iam" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/scheduler/job" - "github.com/hashicorp/go-hclog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -31,7 +30,6 @@ func TestScheduler_New(t *testing.T) { type args struct { serverId string jobRepo jobRepoFactory - logger hclog.Logger runLimit uint runInterval time.Duration } @@ -59,22 +57,11 @@ func TestScheduler_New(t *testing.T) { wantErrCode: errors.InvalidParameter, wantErrMsg: "scheduler.New: missing job repo function: parameter violation: error #100", }, - { - name: "with-no-logger", - args: args{ - serverId: "test-server", - jobRepo: jobRepoFn, - }, - wantErr: true, - wantErrCode: errors.InvalidParameter, - wantErrMsg: "scheduler.New: missing logger: parameter violation: error #100", - }, { name: "valid", args: args{ serverId: "test-server", jobRepo: jobRepoFn, - logger: hclog.L(), }, want: args{ serverId: "test-server", @@ -87,7 +74,6 @@ func TestScheduler_New(t *testing.T) { args: args{ serverId: "test-server", jobRepo: jobRepoFn, - logger: hclog.L(), }, opts: []Option{ WithRunJobsInterval(time.Hour), @@ -103,7 +89,6 @@ func TestScheduler_New(t *testing.T) { args: args{ serverId: "test-server", jobRepo: jobRepoFn, - logger: hclog.L(), }, opts: []Option{ WithRunJobsLimit(20), @@ -119,7 +104,6 @@ func TestScheduler_New(t *testing.T) { args: args{ serverId: "test-server", jobRepo: jobRepoFn, - logger: hclog.L(), }, opts: []Option{ WithRunJobsInterval(time.Hour), @@ -137,7 +121,7 @@ func TestScheduler_New(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - got, err := New(tt.args.serverId, tt.args.jobRepo, tt.args.logger, tt.opts...) + got, err := New(tt.args.serverId, tt.args.jobRepo, tt.opts...) if tt.wantErr { require.Error(err) assert.Truef(errors.Match(errors.T(tt.wantErrCode), err), "Unexpected error %s", err) diff --git a/internal/scheduler/testing.go b/internal/scheduler/testing.go index ccf4a4022..8de49b3ce 100644 --- a/internal/scheduler/testing.go +++ b/internal/scheduler/testing.go @@ -11,7 +11,6 @@ import ( "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/scheduler/job" "github.com/hashicorp/boundary/internal/servers" - "github.com/hashicorp/go-hclog" wrapping "github.com/hashicorp/go-kms-wrapping" "github.com/hashicorp/go-uuid" "github.com/jinzhu/gorm" @@ -47,7 +46,7 @@ func TestScheduler(t *testing.T, conn *gorm.DB, wrapper wrapping.Wrapper, opt .. return job.NewRepository(rw, rw, kmsCache) } - s, err := New(controller.PrivateId, jobRepoFn, hclog.L(), opt...) + s, err := New(controller.PrivateId, jobRepoFn, opt...) require.NoError(t, err) return s diff --git a/internal/servers/controller/auth/auth.go b/internal/servers/controller/auth/auth.go index da5998910..e7dd86ff2 100644 --- a/internal/servers/controller/auth/auth.go +++ b/internal/servers/controller/auth/auth.go @@ -22,7 +22,6 @@ import ( "github.com/hashicorp/boundary/internal/types/resource" "github.com/hashicorp/boundary/internal/types/scope" "github.com/hashicorp/boundary/sdk/recovery" - "github.com/hashicorp/go-hclog" wrapping "github.com/hashicorp/go-kms-wrapping" "github.com/mr-tron/base58" "google.golang.org/protobuf/proto" @@ -105,7 +104,6 @@ type VerifyResults struct { } type verifier struct { - logger hclog.Logger iamRepoFn common.IamRepoFactory authTokenRepoFn common.AuthTokenRepoFactory serversRepoFn common.ServersRepoFactory @@ -122,14 +120,12 @@ type verifier struct { // HTTP handler and should exist for every request that reaches the service // handlers. func NewVerifierContext(ctx context.Context, - logger hclog.Logger, iamRepoFn common.IamRepoFactory, authTokenRepoFn common.AuthTokenRepoFactory, serversRepoFn common.ServersRepoFactory, kms *kms.Kms, requestInfo RequestInfo) context.Context { return context.WithValue(ctx, verifierKey, &verifier{ - logger: logger, iamRepoFn: iamRepoFn, authTokenRepoFn: authTokenRepoFn, serversRepoFn: serversRepoFn, diff --git a/internal/servers/controller/auth/auth_test.go b/internal/servers/controller/auth/auth_test.go index 981a6767a..574ac0cb3 100644 --- a/internal/servers/controller/auth/auth_test.go +++ b/internal/servers/controller/auth/auth_test.go @@ -13,7 +13,6 @@ import ( "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/servers" "github.com/hashicorp/boundary/internal/servers/controller/handlers" - "github.com/hashicorp/go-hclog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -21,7 +20,6 @@ import ( func TestAuthTokenAuthenticator(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") rw := db.New(conn) - logger := hclog.New(nil) wrapper := db.TestWrapper(t) kms := kms.TestKms(t, conn, wrapper) tokenRepo, err := authtoken.NewRepository(rw, rw, kms) @@ -119,7 +117,7 @@ func TestAuthTokenAuthenticator(t *testing.T) { if tc.userId == "" { return } - ctx := NewVerifierContext(context.Background(), logger, iamRepoFn, tokenRepoFn, serversRepoFn, kms, requestInfo) + ctx := NewVerifierContext(context.Background(), iamRepoFn, tokenRepoFn, serversRepoFn, kms, requestInfo) v, ok := ctx.Value(verifierKey).(*verifier) require.True(t, ok) diff --git a/internal/servers/controller/auth/testing.go b/internal/servers/controller/auth/testing.go index 9c2a68001..731da8bd2 100644 --- a/internal/servers/controller/auth/testing.go +++ b/internal/servers/controller/auth/testing.go @@ -22,5 +22,5 @@ func DisabledAuthTestContext(iamRepoFn common.IamRepoFactory, scopeId string, op reqInfo.userIdOverride = "u_auth" } requestContext := context.WithValue(context.Background(), requests.ContextRequestInformationKey, &requests.RequestContext{}) - return NewVerifierContext(requestContext, nil, iamRepoFn, nil, nil, opts.withKms, reqInfo) + return NewVerifierContext(requestContext, iamRepoFn, nil, nil, opts.withKms, reqInfo) } diff --git a/internal/servers/controller/common/scopeids/scope_ids_test.go b/internal/servers/controller/common/scopeids/scope_ids_test.go index 49743cd39..570098e9d 100644 --- a/internal/servers/controller/common/scopeids/scope_ids_test.go +++ b/internal/servers/controller/common/scopeids/scope_ids_test.go @@ -225,7 +225,6 @@ func TestListingScopeIds(t *testing.T) { o, p := iam.TestScopes(t, iamRepo) at := authtoken.TestAuthToken(t, conn, kms, o.GetPublicId()) ctx := auth.NewVerifierContext(context.Background(), - nil, iamRepoFn, authTokenRepoFn, serversRepoFn, diff --git a/internal/servers/controller/controller.go b/internal/servers/controller/controller.go index 693edd768..0e389e3b2 100644 --- a/internal/servers/controller/controller.go +++ b/internal/servers/controller/controller.go @@ -105,7 +105,7 @@ func New(ctx context.Context, conf *Config) (*Controller, error) { if err != nil { return nil, fmt.Errorf("error creating kms repository: %w", err) } - c.kms, err = kms.NewKms(kmsRepo, kms.WithLogger(c.logger.Named("kms"))) + c.kms, err = kms.NewKms(kmsRepo) if err != nil { return nil, fmt.Errorf("error creating kms cache: %w", err) } @@ -121,7 +121,7 @@ func New(ctx context.Context, conf *Config) (*Controller, error) { } // TODO: the RunJobsLimit is temporary until a better fix gets in. This // currently caps the scheduler at running 10 jobs per interval. - c.scheduler, err = scheduler.New(c.conf.RawConfig.Controller.Name, jobRepoFn, c.logger, scheduler.WithRunJobsLimit(10)) + c.scheduler, err = scheduler.New(c.conf.RawConfig.Controller.Name, jobRepoFn, scheduler.WithRunJobsLimit(10)) if err != nil { return nil, fmt.Errorf("error creating new scheduler: %w", err) } @@ -203,7 +203,7 @@ func (c *Controller) Start() error { func (c *Controller) registerJobs() error { rw := db.New(c.conf.Database) - if err := vault.RegisterJobs(c.baseContext, c.scheduler, rw, rw, c.kms, c.logger); err != nil { + if err := vault.RegisterJobs(c.baseContext, c.scheduler, rw, rw, c.kms); err != nil { return err } @@ -217,7 +217,7 @@ func (c *Controller) registerJobs() error { // registerSessionCleanupJob is a helper method to abstract // registering the session cleanup job specifically. func (c *Controller) registerSessionCleanupJob() error { - sessionCleanupJob, err := newSessionCleanupJob(c.logger, c.SessionRepoFn, int(c.conf.StatusGracePeriodDuration.Seconds())) + sessionCleanupJob, err := newSessionCleanupJob(c.SessionRepoFn, int(c.conf.StatusGracePeriodDuration.Seconds())) if err != nil { return fmt.Errorf("error creating session cleanup job: %w", err) } diff --git a/internal/servers/controller/handler.go b/internal/servers/controller/handler.go index ecd0e64e2..0bd972914 100644 --- a/internal/servers/controller/handler.go +++ b/internal/servers/controller/handler.go @@ -82,7 +82,7 @@ func handleGrpcGateway(c *Controller, props HandlerProperties) (http.Handler, er runtime.WithMarshalerOption(runtime.MIMEWildcard, &runtime.HTTPBodyMarshaler{ Marshaler: handlers.JSONMarshaler(), }), - runtime.WithErrorHandler(handlers.ErrorHandler(c.logger)), + runtime.WithErrorHandler(handlers.ErrorHandler()), runtime.WithForwardResponseOption(handlers.OutgoingInterceptor), ) hcs, err := host_catalogs.NewService(c.StaticHostRepoFn, c.IamRepoFn) @@ -113,7 +113,7 @@ func handleGrpcGateway(c *Controller, props HandlerProperties) (http.Handler, er if err := services.RegisterAccountServiceHandlerServer(ctx, mux, accts); err != nil { return nil, fmt.Errorf("failed to register account service handler: %w", err) } - authMethods, err := authmethods.NewService(c.kms, c.PasswordAuthRepoFn, c.OidcRepoFn, c.IamRepoFn, c.AuthTokenRepoFn, handlers.WithLogger(c.logger.Named("authmethod_service"))) + authMethods, err := authmethods.NewService(c.kms, c.PasswordAuthRepoFn, c.OidcRepoFn, c.IamRepoFn, c.AuthTokenRepoFn) if err != nil { return nil, fmt.Errorf("failed to create auth method handler service: %w", err) } @@ -243,7 +243,7 @@ func wrapHandlerWithCommonFuncs(h http.Handler, c *Controller, props HandlerProp } requestInfo.PublicId, requestInfo.EncryptedToken, requestInfo.TokenFormat = auth.GetTokenFromRequest(ctx, c.kms, r) - ctx = auth.NewVerifierContext(ctx, c.logger, c.IamRepoFn, c.AuthTokenRepoFn, c.ServersRepoFn, c.kms, requestInfo) + ctx = auth.NewVerifierContext(ctx, c.IamRepoFn, c.AuthTokenRepoFn, c.ServersRepoFn, c.kms, requestInfo) // Add general request information to the context. The information from // the auth verifier context is pretty specifically curated to diff --git a/internal/servers/controller/handler_noui.go b/internal/servers/controller/handler_noui.go index c7bf5ca0c..20bc64689 100644 --- a/internal/servers/controller/handler_noui.go +++ b/internal/servers/controller/handler_noui.go @@ -6,10 +6,9 @@ import ( "path/filepath" "github.com/hashicorp/boundary/internal/observability/event" - "github.com/hashicorp/go-hclog" ) -func devPassthroughHandler(logger hclog.Logger, passthroughDir string) http.Handler { +func devPassthroughHandler(passthroughDir string) http.Handler { const op = "controller.devPassthroughHandler" ctx := context.TODO() // Panic may not be ideal but this is never a production call and it'll @@ -28,7 +27,7 @@ func devPassthroughHandler(logger hclog.Logger, passthroughDir string) http.Hand var handleUi = func(c *Controller) http.Handler { if c.conf.RawConfig.PassthroughDirectory != "" { - return devPassthroughHandler(c.logger, c.conf.RawConfig.PassthroughDirectory) + return devPassthroughHandler(c.conf.RawConfig.PassthroughDirectory) } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNotFound) diff --git a/internal/servers/controller/handler_ui.go b/internal/servers/controller/handler_ui.go index f766606b2..94cd9e7aa 100644 --- a/internal/servers/controller/handler_ui.go +++ b/internal/servers/controller/handler_ui.go @@ -16,7 +16,7 @@ func init() { func handleUiWithAssets(c *Controller) http.Handler { var nextHandler http.Handler if c.conf.RawConfig.PassthroughDirectory != "" { - nextHandler = devPassthroughHandler(c.logger, c.conf.RawConfig.PassthroughDirectory) + nextHandler = devPassthroughHandler(c.conf.RawConfig.PassthroughDirectory) } else { nextHandler = ui.Handler() } diff --git a/internal/servers/controller/handlers/authmethods/authmethod_service.go b/internal/servers/controller/handlers/authmethods/authmethod_service.go index 25e4d916c..e8feaab90 100644 --- a/internal/servers/controller/handlers/authmethods/authmethod_service.go +++ b/internal/servers/controller/handlers/authmethods/authmethod_service.go @@ -29,7 +29,6 @@ import ( "github.com/hashicorp/boundary/internal/types/resource" "github.com/hashicorp/boundary/internal/types/scope" "github.com/hashicorp/boundary/internal/types/subtypes" - "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-secure-stdlib/strutil" "google.golang.org/grpc/codes" "google.golang.org/protobuf/types/known/structpb" @@ -74,8 +73,6 @@ type Service struct { oidcRepoFn common.OidcAuthRepoFactory iamRepoFn common.IamRepoFactory atRepoFn common.AuthTokenRepoFactory - - oidcLogger hclog.Logger } // NewService returns a auth method service which handles auth method related requests to boundary. @@ -97,10 +94,7 @@ func NewService(kms *kms.Kms, pwRepoFn common.PasswordAuthRepoFactory, oidcRepoF return Service{}, fmt.Errorf("nil auth token repository provided") } s := Service{kms: kms, pwRepoFn: pwRepoFn, oidcRepoFn: oidcRepoFn, iamRepoFn: iamRepoFn, atRepoFn: atRepoFn} - opts := handlers.GetOpts(opt...) - if opts.WithLogger != nil { - s.oidcLogger = opts.WithLogger.Named("oidc") - } + return s, nil } diff --git a/internal/servers/controller/handlers/authmethods/oidc_test.go b/internal/servers/controller/handlers/authmethods/oidc_test.go index 166521348..292425288 100644 --- a/internal/servers/controller/handlers/authmethods/oidc_test.go +++ b/internal/servers/controller/handlers/authmethods/oidc_test.go @@ -203,7 +203,6 @@ func TestList_FilterNonPublic(t *testing.T) { reqCtx: func() context.Context { at := authtoken.TestAuthToken(t, conn, kmsCache, o.GetPublicId()) return auth.NewVerifierContext(requests.NewRequestContext(context.Background()), - nil, iamRepoFn, atRepoFn, serversRepoFn, diff --git a/internal/servers/controller/handlers/authtokens/authtoken_service_test.go b/internal/servers/controller/handlers/authtokens/authtoken_service_test.go index 2d73d6a9f..be0f63922 100644 --- a/internal/servers/controller/handlers/authtokens/authtoken_service_test.go +++ b/internal/servers/controller/handlers/authtokens/authtoken_service_test.go @@ -21,7 +21,6 @@ import ( "github.com/hashicorp/boundary/internal/servers/controller/handlers" "github.com/hashicorp/boundary/internal/servers/controller/handlers/authtokens" "github.com/hashicorp/boundary/internal/types/scope" - "github.com/hashicorp/go-hclog" "google.golang.org/grpc/codes" "google.golang.org/protobuf/testing/protocmp" @@ -35,7 +34,6 @@ func TestGetSelf(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") rw := db.New(conn) wrap := db.TestWrapper(t) - logger := hclog.New(nil) kms := kms.TestKms(t, conn, wrap) iamRepoFn := func() (*iam.Repository, error) { @@ -97,7 +95,7 @@ func TestGetSelf(t *testing.T) { Token: tc.token.GetToken(), } - ctx := auth.NewVerifierContext(context.Background(), logger, iamRepoFn, tokenRepoFn, serversRepoFn, kms, requestInfo) + ctx := auth.NewVerifierContext(context.Background(), iamRepoFn, tokenRepoFn, serversRepoFn, kms, requestInfo) ctx = context.WithValue(ctx, requests.ContextRequestInformationKey, &requests.RequestContext{}) got, err := a.GetAuthToken(ctx, &pbs.GetAuthTokenRequest{Id: tc.readId}) if tc.err != nil { @@ -193,7 +191,6 @@ func TestList_Self(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") rw := db.New(conn) wrap := db.TestWrapper(t) - logger := hclog.New(nil) kms := kms.TestKms(t, conn, wrap) iamRepo := iam.TestRepo(t, conn, wrap) @@ -247,7 +244,7 @@ func TestList_Self(t *testing.T) { Token: tc.requester.GetToken(), } - ctx := auth.NewVerifierContext(context.Background(), logger, iamRepoFn, tokenRepoFn, serversRepoFn, kms, requestInfo) + ctx := auth.NewVerifierContext(context.Background(), iamRepoFn, tokenRepoFn, serversRepoFn, kms, requestInfo) got, err := a.ListAuthTokens(ctx, &pbs.ListAuthTokensRequest{ScopeId: o.GetPublicId()}) require.NoError(err) require.Len(got.Items, 1) @@ -418,7 +415,6 @@ func TestDeleteSelf(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") rw := db.New(conn) wrap := db.TestWrapper(t) - logger := hclog.New(nil) kms := kms.TestKms(t, conn, wrap) iamRepo := iam.TestRepo(t, conn, wrap) @@ -494,7 +490,7 @@ func TestDeleteSelf(t *testing.T) { Token: tc.token.GetToken(), } - ctx := auth.NewVerifierContext(context.Background(), logger, iamRepoFn, tokenRepoFn, serversRepoFn, kms, requestInfo) + ctx := auth.NewVerifierContext(context.Background(), iamRepoFn, tokenRepoFn, serversRepoFn, kms, requestInfo) got, err := a.DeleteAuthToken(ctx, &pbs.DeleteAuthTokenRequest{Id: tc.deleteId}) if tc.err != nil { require.EqualError(err, tc.err.Error()) diff --git a/internal/servers/controller/handlers/errors.go b/internal/servers/controller/handlers/errors.go index 12ee34251..1628de364 100644 --- a/internal/servers/controller/handlers/errors.go +++ b/internal/servers/controller/handlers/errors.go @@ -12,7 +12,6 @@ import ( "github.com/hashicorp/boundary/internal/errors" pb "github.com/hashicorp/boundary/internal/gen/controller/api" "github.com/hashicorp/boundary/internal/observability/event" - "github.com/hashicorp/go-hclog" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -185,7 +184,7 @@ func backendErrorToApiError(inErr error) *apiError { } } -func ErrorHandler(logger hclog.Logger) runtime.ErrorHandlerFunc { +func ErrorHandler() runtime.ErrorHandlerFunc { const op = "handlers.ErrorHandler" const errorFallback = `{"error": "failed to marshal error message"}` return func(ctx context.Context, _ *runtime.ServeMux, mar runtime.Marshaler, w http.ResponseWriter, r *http.Request, inErr error) { diff --git a/internal/servers/controller/handlers/errors_test.go b/internal/servers/controller/handlers/errors_test.go index 73743b5c9..f4b2f34fc 100644 --- a/internal/servers/controller/handlers/errors_test.go +++ b/internal/servers/controller/handlers/errors_test.go @@ -13,7 +13,6 @@ import ( "github.com/grpc-ecosystem/grpc-gateway/v2/runtime" "github.com/hashicorp/boundary/internal/errors" pb "github.com/hashicorp/boundary/internal/gen/controller/api" - "github.com/hashicorp/go-hclog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" @@ -28,7 +27,7 @@ func TestApiErrorHandler(t *testing.T) { mux := runtime.NewServeMux() inMarsh, outMarsh := runtime.MarshalerForRequest(mux, req) - tested := ErrorHandler(hclog.L()) + tested := ErrorHandler() testCases := []struct { name string diff --git a/internal/servers/controller/handlers/option_test.go b/internal/servers/controller/handlers/option_test.go index 6f5eb5dd3..17701a6e4 100644 --- a/internal/servers/controller/handlers/option_test.go +++ b/internal/servers/controller/handlers/option_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/hashicorp/boundary/internal/perms" - "github.com/hashicorp/go-hclog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -29,15 +28,6 @@ func Test_GetOpts(t *testing.T) { testOpts.withDiscardUnknownFields = false assert.Equal(opts, testOpts) }) - t.Run("WithLogger", func(t *testing.T) { - assert := assert.New(t) - - opts := GetOpts() - assert.Nil(opts.WithLogger) - - opts = GetOpts(WithLogger(hclog.New(nil))) - assert.NotNil(opts.WithLogger) - }) t.Run("WithUserIsAnonymous", func(t *testing.T) { assert := assert.New(t) diff --git a/internal/servers/controller/handlers/options.go b/internal/servers/controller/handlers/options.go index 3735a5ddd..c283b8b5a 100644 --- a/internal/servers/controller/handlers/options.go +++ b/internal/servers/controller/handlers/options.go @@ -3,7 +3,6 @@ package handlers import ( "github.com/hashicorp/boundary/internal/gen/controller/api/resources/scopes" "github.com/hashicorp/boundary/internal/perms" - "github.com/hashicorp/go-hclog" "google.golang.org/protobuf/types/known/structpb" ) @@ -24,7 +23,6 @@ type Option func(*options) // options = how options are represented type options struct { withDiscardUnknownFields bool - WithLogger hclog.Logger WithUserIsAnonymous bool WithOutputFields *perms.OutputFieldsMap WithScope *scopes.ScopeInfo @@ -47,13 +45,6 @@ func WithDiscardUnknownFields(discard bool) Option { } } -// WithLogger provides an option include a logger -func WithLogger(logger hclog.Logger) Option { - return func(o *options) { - o.WithLogger = logger - } -} - // DEPRECATED: Superceded by WithOutputFields. Will be removed once all handlers // have been migrated to that. func WithUserIsAnonymous(anonListing bool) Option { diff --git a/internal/servers/controller/handlers/sessions/session_service_test.go b/internal/servers/controller/handlers/sessions/session_service_test.go index ab5c10325..9f4c41b5e 100644 --- a/internal/servers/controller/handlers/sessions/session_service_test.go +++ b/internal/servers/controller/handlers/sessions/session_service_test.go @@ -24,7 +24,6 @@ import ( "github.com/hashicorp/boundary/internal/session" "github.com/hashicorp/boundary/internal/target" "github.com/hashicorp/boundary/internal/types/scope" - "github.com/hashicorp/go-hclog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" @@ -148,7 +147,6 @@ func TestList_Self(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") wrap := db.TestWrapper(t) kms := kms.TestKms(t, conn, wrap) - logger := hclog.New(nil) iamRepo := iam.TestRepo(t, conn, wrap) rw := db.New(conn) @@ -220,7 +218,7 @@ func TestList_Self(t *testing.T) { Token: tc.requester.GetToken(), } - ctx := auth.NewVerifierContext(context.Background(), logger, iamRepoFn, tokenRepoFn, serversRepoFn, kms, requestInfo) + ctx := auth.NewVerifierContext(context.Background(), iamRepoFn, tokenRepoFn, serversRepoFn, kms, requestInfo) got, err := s.ListSessions(ctx, &pbs.ListSessionsRequest{ScopeId: pWithSessions.GetPublicId()}) require.NoError(t, err) assert.Equal(t, tc.count, len(got.GetItems()), got.GetItems()) diff --git a/internal/servers/controller/handlers/targets/target_service_test.go b/internal/servers/controller/handlers/targets/target_service_test.go index fe2344d2b..85896b57e 100644 --- a/internal/servers/controller/handlers/targets/target_service_test.go +++ b/internal/servers/controller/handlers/targets/target_service_test.go @@ -36,7 +36,6 @@ import ( "github.com/hashicorp/boundary/internal/session" "github.com/hashicorp/boundary/internal/target" "github.com/hashicorp/boundary/internal/types/scope" - "github.com/hashicorp/go-hclog" wrapping "github.com/hashicorp/go-kms-wrapping" "github.com/jinzhu/gorm" "github.com/stretchr/testify/assert" @@ -2497,7 +2496,6 @@ func TestAuthorizeSession(t *testing.T) { org, proj := iam.TestScopes(t, iamRepo) at := authtoken.TestAuthToken(t, conn, kms, org.GetPublicId()) ctx := auth.NewVerifierContext(requests.NewRequestContext(context.Background()), - nil, iamRepoFn, atRepoFn, serversRepoFn, @@ -2556,7 +2554,7 @@ func TestAuthorizeSession(t *testing.T) { require.NoError(t, err) // Tell our DB that there is a worker ready to serve the data - workerService := workers.NewWorkerServiceServer(hclog.Default(), serversRepoFn, sessionRepoFn, &sync.Map{}, kms) + workerService := workers.NewWorkerServiceServer(serversRepoFn, sessionRepoFn, &sync.Map{}, kms) _, err = workerService.Status(ctx, &spbs.StatusRequest{ Worker: &spb.Server{ PrivateId: "testworker", @@ -2665,7 +2663,6 @@ func TestAuthorizeSession_Errors(t *testing.T) { // Authorized user gets full permissions at := authtoken.TestAuthToken(t, conn, kms, org.GetPublicId()) ctx := auth.NewVerifierContext(requests.NewRequestContext(context.Background()), - nil, iamRepoFn, atRepoFn, serversRepoFn, @@ -2685,7 +2682,7 @@ func TestAuthorizeSession_Errors(t *testing.T) { store := vault.TestCredentialStore(t, conn, wrapper, proj.GetPublicId(), v.Addr, tok, sec.Auth.Accessor) workerExists := func(tar *target.TcpTarget) (version uint32) { - workerService := workers.NewWorkerServiceServer(hclog.Default(), serversRepoFn, sessionRepoFn, &sync.Map{}, kms) + workerService := workers.NewWorkerServiceServer(serversRepoFn, sessionRepoFn, &sync.Map{}, kms) _, err := workerService.Status(context.Background(), &spbs.StatusRequest{ Worker: &spb.Server{ PrivateId: "testworker", diff --git a/internal/servers/controller/handlers/workers/worker_service.go b/internal/servers/controller/handlers/workers/worker_service.go index 0390489f0..c228525dc 100644 --- a/internal/servers/controller/handlers/workers/worker_service.go +++ b/internal/servers/controller/handlers/workers/worker_service.go @@ -17,7 +17,6 @@ import ( "github.com/hashicorp/boundary/internal/session" "github.com/hashicorp/boundary/internal/types/resource" "github.com/hashicorp/go-bexpr" - "github.com/hashicorp/go-hclog" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -26,7 +25,6 @@ type workerServiceServer struct { pbs.UnimplementedServerCoordinationServiceServer pbs.UnimplementedSessionServiceServer - logger hclog.Logger serversRepoFn common.ServersRepoFactory sessionRepoFn common.SessionRepoFactory updateTimes *sync.Map @@ -34,13 +32,11 @@ type workerServiceServer struct { } func NewWorkerServiceServer( - logger hclog.Logger, serversRepoFn common.ServersRepoFactory, sessionRepoFn common.SessionRepoFactory, updateTimes *sync.Map, kms *kms.Kms) *workerServiceServer { return &workerServiceServer{ - logger: logger, serversRepoFn: serversRepoFn, sessionRepoFn: sessionRepoFn, updateTimes: updateTimes, diff --git a/internal/servers/controller/listeners.go b/internal/servers/controller/listeners.go index 5442a0b58..58996e1a4 100644 --- a/internal/servers/controller/listeners.go +++ b/internal/servers/controller/listeners.go @@ -119,7 +119,7 @@ func (c *Controller) startListeners() error { grpc.MaxRecvMsgSize(math.MaxInt32), grpc.MaxSendMsgSize(math.MaxInt32), ) - workerService := workers.NewWorkerServiceServer(c.logger.Named("worker-handler"), c.ServersRepoFn, c.SessionRepoFn, c.workerStatusUpdateTimes, c.kms) + workerService := workers.NewWorkerServiceServer(c.ServersRepoFn, c.SessionRepoFn, c.workerStatusUpdateTimes, c.kms) pbs.RegisterServerCoordinationServiceServer(workerServer, workerService) pbs.RegisterSessionServiceServer(workerServer, workerService) diff --git a/internal/servers/controller/session_cleanup_job.go b/internal/servers/controller/session_cleanup_job.go index e8f108d08..cff63e790 100644 --- a/internal/servers/controller/session_cleanup_job.go +++ b/internal/servers/controller/session_cleanup_job.go @@ -11,7 +11,6 @@ import ( "github.com/hashicorp/boundary/internal/scheduler" "github.com/hashicorp/boundary/internal/servers/controller/common" "github.com/hashicorp/boundary/internal/session" - "github.com/hashicorp/go-hclog" ) // sessionCleanupJob defines a periodic job that monitors workers for @@ -24,7 +23,6 @@ import ( // the controller will win out and order that the connections be // closed on the worker. type sessionCleanupJob struct { - logger hclog.Logger sessionRepoFn common.SessionRepoFactory // The amount of time to give disconnected workers before marking @@ -37,14 +35,11 @@ type sessionCleanupJob struct { // newSessionCleanupJob instantiates the session cleanup job. func newSessionCleanupJob( - logger hclog.Logger, sessionRepoFn common.SessionRepoFactory, gracePeriod int, ) (*sessionCleanupJob, error) { const op = "controller.newNewSessionCleanupJob" switch { - case logger == nil: - return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing logger") case sessionRepoFn == nil: return nil, errors.NewDeprecated(errors.InvalidParameter, op, "missing sessionRepoFn") case gracePeriod < session.DeadWorkerConnCloseMinGrace: @@ -53,7 +48,6 @@ func newSessionCleanupJob( } return &sessionCleanupJob{ - logger: logger, sessionRepoFn: sessionRepoFn, gracePeriod: gracePeriod, }, nil diff --git a/internal/servers/controller/session_cleanup_job_test.go b/internal/servers/controller/session_cleanup_job_test.go index 9365392da..803fc9e15 100644 --- a/internal/servers/controller/session_cleanup_job_test.go +++ b/internal/servers/controller/session_cleanup_job_test.go @@ -13,7 +13,6 @@ import ( "github.com/hashicorp/boundary/internal/scheduler" "github.com/hashicorp/boundary/internal/servers" "github.com/hashicorp/boundary/internal/session" - "github.com/hashicorp/go-hclog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -96,7 +95,6 @@ func TestSessionCleanupJob(t *testing.T) { // Create the job. job, err := newSessionCleanupJob( - hclog.New(&hclog.LoggerOptions{Level: hclog.Trace}), func() (*session.Repository, error) { return sessionRepo, nil }, session.DeadWorkerConnCloseMinGrace, ) @@ -145,16 +143,7 @@ func TestSessionCleanupJobNewJobErr(t *testing.T) { const op = "controller.newNewSessionCleanupJob" require := require.New(t) - job, err := newSessionCleanupJob(nil, nil, 0) - require.Equal(err, errors.E( - ctx, - errors.WithCode(errors.InvalidParameter), - errors.WithOp(op), - errors.WithMsg("missing logger"), - )) - require.Nil(job) - - job, err = newSessionCleanupJob(hclog.New(nil), nil, 0) + job, err := newSessionCleanupJob(nil, 0) require.Equal(err, errors.E( ctx, errors.WithCode(errors.InvalidParameter), @@ -163,7 +152,7 @@ func TestSessionCleanupJobNewJobErr(t *testing.T) { )) require.Nil(job) - job, err = newSessionCleanupJob(hclog.New(nil), func() (*session.Repository, error) { return nil, nil }, 0) + job, err = newSessionCleanupJob(func() (*session.Repository, error) { return nil, nil }, 0) require.Equal(err, errors.E( ctx, errors.WithCode(errors.InvalidParameter), diff --git a/internal/servers/worker/worker.go b/internal/servers/worker/worker.go index c354413d9..79680d5ba 100644 --- a/internal/servers/worker/worker.go +++ b/internal/servers/worker/worker.go @@ -135,6 +135,7 @@ func (w *Worker) Start() error { // doable, but work for later. func (w *Worker) Shutdown(skipListeners bool) error { const op = "worker.(Worker).Shutdown" + ctx := context.TODO() if !w.started.Load() { event.WriteSysEvent(context.TODO(), op, "already shut down, skipping") return nil @@ -142,31 +143,27 @@ func (w *Worker) Shutdown(skipListeners bool) error { // Stop listeners first to prevent new connections to the // controller. - w.logger.Debug("beginning shutdown") defer w.started.Store(false) w.Resolver().UpdateState(resolver.State{Addresses: []resolver.Address{}}) w.baseCancel() if !skipListeners { - w.logger.Debug("stopping listeners") if err := w.stopListeners(); err != nil { return fmt.Errorf("error stopping worker listeners: %w", err) } } // Shut down all connections. - w.logger.Debug("shutting down all connections due to worker shutdown or reload") w.cleanupConnections(w.baseContext, true) // Wait for next status request to succeed. Don't wait too long; // wrap the base context in a timeout equal to our status grace // period. - w.logger.Debug("waiting for next status report to controller") waitStatusStart := time.Now() nextStatusCtx, nextStatusCancel := context.WithTimeout(w.baseContext, w.conf.StatusGracePeriodDuration) defer nextStatusCancel() for { if err := nextStatusCtx.Err(); err != nil { - w.logger.Error("error waiting for next status report to controller", "err", err) + event.WriteError(ctx, op, err, event.WithInfoMsg("error waiting for next status report to controller")) break } @@ -178,7 +175,6 @@ func (w *Worker) Shutdown(skipListeners bool) error { } // Proceed with remainder of shutdown. - w.logger.Debug("canceling base context and shutting down connection to controller") w.baseCancel() w.Resolver().UpdateState(resolver.State{Addresses: []resolver.Address{}}) @@ -190,7 +186,6 @@ func (w *Worker) Shutdown(skipListeners bool) error { } } - w.logger.Debug("shutdown successful") return nil } diff --git a/internal/tests/cluster/session_cleanup_test.go b/internal/tests/cluster/session_cleanup_test.go index 2ba8ce0d9..1b3749c33 100644 --- a/internal/tests/cluster/session_cleanup_test.go +++ b/internal/tests/cluster/session_cleanup_test.go @@ -146,7 +146,7 @@ func testWorkerSessionCleanupSingle(burdenCase timeoutBurdenType) func(t *testin require.NotNil(tgt) // Create test server, update default port on target - ts := helper.NewTestTcpServer(t, logger) + ts := helper.NewTestTcpServer(t) require.NotNil(t, ts) defer ts.Close() tgt, err = tcl.Update(ctx, tgt.Item.Id, tgt.Item.Version, targets.WithTcpTargetDefaultPort(ts.Port()), targets.WithSessionConnectionLimit(-1)) @@ -154,7 +154,7 @@ func testWorkerSessionCleanupSingle(burdenCase timeoutBurdenType) func(t *testin require.NotNil(tgt) // Authorize and connect - sess := helper.NewTestSession(ctx, t, logger, tcl, "ttcp_1234567890") + sess := helper.NewTestSession(ctx, t, tcl, "ttcp_1234567890") sConn := sess.Connect(ctx, t) // Run initial send/receive test, make sure things are working @@ -222,7 +222,7 @@ func testWorkerSessionCleanupSingle(burdenCase timeoutBurdenType) func(t *testin // Proceed with new connection test event.WriteSysEvent(ctx, op, "connecting to new session after resuming controller/worker link") - sess = helper.NewTestSession(ctx, t, logger, tcl, "ttcp_1234567890") // re-assign, other connection will close in t.Cleanup() + sess = helper.NewTestSession(ctx, t, tcl, "ttcp_1234567890") // re-assign, other connection will close in t.Cleanup() sConn = sess.Connect(ctx, t) sConn.TestSendRecvAll(t) } @@ -329,7 +329,7 @@ func testWorkerSessionCleanupMulti(burdenCase timeoutBurdenType) func(t *testing require.NotNil(tgt) // Create test server, update default port on target - ts := helper.NewTestTcpServer(t, logger) + ts := helper.NewTestTcpServer(t) require.NotNil(ts) defer ts.Close() tgt, err = tcl.Update(ctx, tgt.Item.Id, tgt.Item.Version, targets.WithTcpTargetDefaultPort(ts.Port()), targets.WithSessionConnectionLimit(-1)) @@ -337,7 +337,7 @@ func testWorkerSessionCleanupMulti(burdenCase timeoutBurdenType) func(t *testing require.NotNil(tgt) // Authorize and connect - sess := helper.NewTestSession(ctx, t, logger, tcl, "ttcp_1234567890") + sess := helper.NewTestSession(ctx, t, tcl, "ttcp_1234567890") sConn := sess.Connect(ctx, t) // Run initial send/receive test, make sure things are working @@ -424,7 +424,7 @@ func testWorkerSessionCleanupMulti(burdenCase timeoutBurdenType) func(t *testing // Proceed with new connection test event.WriteSysEvent(ctx, op, "connecting to new session after resuming controller/worker link") - sess = helper.NewTestSession(ctx, t, logger, tcl, "ttcp_1234567890") // re-assign, other connection will close in t.Cleanup() + sess = helper.NewTestSession(ctx, t, tcl, "ttcp_1234567890") // re-assign, other connection will close in t.Cleanup() sConn = sess.Connect(ctx, t) sConn.TestSendRecvAll(t) } diff --git a/internal/tests/helper/testing_helper.go b/internal/tests/helper/testing_helper.go index 95ff0808a..ca669a2d1 100644 --- a/internal/tests/helper/testing_helper.go +++ b/internal/tests/helper/testing_helper.go @@ -24,7 +24,6 @@ import ( "github.com/hashicorp/boundary/internal/servers/worker" "github.com/hashicorp/boundary/internal/session" "github.com/hashicorp/go-cleanhttp" - "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-secure-stdlib/base62" "github.com/mr-tron/base58" "github.com/stretchr/testify/assert" @@ -60,7 +59,6 @@ type TestSession struct { transport *http.Transport tofuToken string connectionsLeft int32 - logger hclog.Logger } // NewTestSession authorizes a session and creates all of the data @@ -68,7 +66,6 @@ type TestSession struct { func NewTestSession( ctx context.Context, t *testing.T, - logger hclog.Logger, tcl *targets.Client, targetId string, ) *TestSession { @@ -80,7 +77,6 @@ func NewTestSession( s := &TestSession{ sessionId: sar.Item.SessionId, - logger: logger, } authzString := sar.GetItem().(*targets.SessionAuthorization).AuthorizationToken marshaled, err := base58.FastBase58Decoding(authzString) @@ -232,7 +228,7 @@ func (s *TestSession) ExpectConnectionStateOnController( // Assert require.Equal(expectStates, actualStates) - s.logger.Debug("successfully asserted all connection states on controller", "expected_states", expectStates, "actual_states", actualStates) + t.Log("successfully asserted all connection states on controller", "expected_states", expectStates, "actual_states", actualStates) } // ExpectConnectionStateOnWorker waits until all connections in a @@ -288,7 +284,7 @@ func (s *TestSession) ExpectConnectionStateOnWorker( // Assert require.Equal(expectStates, actualStates) - s.logger.Debug("successfully asserted all connection states on worker", "expected_states", expectStates, "actual_states", actualStates) + t.Log("successfully asserted all connection states on worker", "expected_states", expectStates, "actual_states", actualStates) } func (s *TestSession) testWorkerConnectionInfo(t *testing.T, tw *worker.TestWorker) map[string]worker.TestConnectionInfo { @@ -319,8 +315,7 @@ func (s *TestSession) testWorkerConnectionIds(t *testing.T, tw *worker.TestWorke // TestSessionConnection abstracts a connected session. type TestSessionConnection struct { - conn net.Conn - logger hclog.Logger + conn net.Conn } // Connect returns a TestSessionConnection for a TestSession. Check @@ -339,8 +334,7 @@ func (s *TestSession) Connect( require.NotNil(conn) return &TestSessionConnection{ - conn: conn, - logger: s.logger, + conn: conn, } } @@ -364,7 +358,7 @@ func (c *TestSessionConnection) testSendRecv(t *testing.T) bool { // Shuttle over the sequence number as base64. err := binary.Write(c.conn, binary.LittleEndian, i) if err != nil { - c.logger.Debug("received error during write", "err", err) + t.Log("received error during write", "err", err) if errors.Is(err, net.ErrClosed) || errors.Is(err, io.EOF) || errors.Is(err, websocket.CloseError{Code: websocket.StatusPolicyViolation, Reason: "timed out"}) { @@ -378,7 +372,7 @@ func (c *TestSessionConnection) testSendRecv(t *testing.T) bool { var j uint32 err = binary.Read(c.conn, binary.LittleEndian, &j) if err != nil { - c.logger.Debug("received error during read", "err", err, "num_successfully_sent", i) + t.Log("received error during read", "err", err, "num_successfully_sent", i) if errors.Is(err, net.ErrClosed) || errors.Is(err, io.EOF) || errors.Is(err, websocket.CloseError{Code: websocket.StatusPolicyViolation, Reason: "timed out"}) { @@ -394,7 +388,7 @@ func (c *TestSessionConnection) testSendRecv(t *testing.T) bool { // time.Sleep(time.Second) } - c.logger.Debug("finished send/recv successfully", "num_successfully_sent", testSendRecvSendMax) + t.Log("finished send/recv successfully", "num_successfully_sent", testSendRecvSendMax) return true } @@ -403,7 +397,7 @@ func (c *TestSessionConnection) testSendRecv(t *testing.T) bool { func (c *TestSessionConnection) TestSendRecvAll(t *testing.T) { t.Helper() require.True(t, c.testSendRecv(t)) - c.logger.Debug("successfully asserted send/recv as passing") + t.Log("successfully asserted send/recv as passing") } // TestSendRecvFail asserts that we were able to send/recv all pings @@ -411,15 +405,15 @@ func (c *TestSessionConnection) TestSendRecvAll(t *testing.T) { func (c *TestSessionConnection) TestSendRecvFail(t *testing.T) { t.Helper() require.False(t, c.testSendRecv(t)) - c.logger.Debug("successfully asserted send/recv as failing") + t.Log("successfully asserted send/recv as failing") } // TestTcpServer defines a simple testing TCP server. Data is simply // copied from its input to its output. type TestTcpServer struct { - logger hclog.Logger - ln net.Listener - conns map[string]net.Conn + t *testing.T + ln net.Listener + conns map[string]net.Conn } // Port returns the port number for the test server listener. @@ -458,7 +452,7 @@ func (ts *TestTcpServer) run() { conn, err := ts.ln.Accept() if err != nil { if !errors.Is(err, net.ErrClosed) { - ts.logger.Error("Accept() error in TestTcpServer", "err", err) + ts.t.Error("Accept() error in TestTcpServer", "err", err) } return @@ -474,12 +468,12 @@ func (ts *TestTcpServer) run() { } // NewTestTcpServer creates a TestTcpServer and runs it. -func NewTestTcpServer(t *testing.T, logger hclog.Logger) *TestTcpServer { +func NewTestTcpServer(t *testing.T) *TestTcpServer { t.Helper() require := require.New(t) ts := &TestTcpServer{ - logger: logger, - conns: make(map[string]net.Conn), + t: t, + conns: make(map[string]net.Conn), } var err error ts.ln, err = net.Listen("tcp", ":0")