From f7377bb5b502deec8a7299e2eb2cd3dcdeb0625a Mon Sep 17 00:00:00 2001 From: Chris Capurso <1036769+ccapurso@users.noreply.github.com> Date: Wed, 16 Nov 2022 14:39:44 -0500 Subject: [PATCH 01/11] add logger->log-level str func --- helper/logging/logger.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/helper/logging/logger.go b/helper/logging/logger.go index ec42ef0e1111..6e87fde5387b 100644 --- a/helper/logging/logger.go +++ b/helper/logging/logger.go @@ -112,6 +112,8 @@ func ParseLogFormat(format string) (LogFormat, error) { } } +// ParseLogLevel returns the hclog.Level that corresponds with the provided level string. +// This differs hclog.LevelFromString in that it supports additional level strings. func ParseLogLevel(logLevel string) (log.Level, error) { var result log.Level logLevel = strings.ToLower(strings.TrimSpace(logLevel)) @@ -133,3 +135,24 @@ func ParseLogLevel(logLevel string) (log.Level, error) { return result, nil } + +// TranslateLoggerLevel returns the string that corresponds with logging level of the hclog.Logger. +func TranslateLoggerLevel(logger log.Logger) (string, error) { + var result string + + if logger.IsTrace() { + result = "trace" + } else if logger.IsDebug() { + result = "debug" + } else if logger.IsInfo() { + result = "info" + } else if logger.IsWarn() { + result = "warn" + } else if logger.IsError() { + result = "error" + } else { + return "", fmt.Errorf("unknown log level") + } + + return result, nil +} From a63bcb807dc443ebbb725759df3b5e6d126d569b Mon Sep 17 00:00:00 2001 From: Chris Capurso <1036769+ccapurso@users.noreply.github.com> Date: Wed, 16 Nov 2022 14:44:26 -0500 Subject: [PATCH 02/11] ensure SetLogLevelByName accounts for duplicates --- vault/core.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/vault/core.go b/vault/core.go index f6f7c75b654d..c053ac8ee66d 100644 --- a/vault/core.go +++ b/vault/core.go @@ -2860,6 +2860,7 @@ func (c *Core) AddLogger(logger log.Logger) { c.allLoggers = append(c.allLoggers, logger) } +// SetLogLevel sets logging level for all tracked loggers to the level provided func (c *Core) SetLogLevel(level log.Level) { c.allLoggersLock.RLock() defer c.allLoggersLock.RUnlock() @@ -2868,17 +2869,22 @@ func (c *Core) SetLogLevel(level log.Level) { } } -func (c *Core) SetLogLevelByName(name string, level log.Level) error { +// SetLogLevelByName sets the logging level of named logger to level provided +// if it exists. Core.allLoggers is a slice and as such it is entirely possible +// that multiple entries exist for the same name. Each instance will be modified. +func (c *Core) SetLogLevelByName(name string, level log.Level) bool { c.allLoggersLock.RLock() defer c.allLoggersLock.RUnlock() + + found := false for _, logger := range c.allLoggers { if logger.Name() == name { logger.SetLevel(level) - return nil + found = true } } - return fmt.Errorf("logger %q does not exist", name) + return found } // SetConfig sets core's config object to the newly provided config. From 89aed100defd25332c5cd989251659d19ae47583 Mon Sep 17 00:00:00 2001 From: Chris Capurso <1036769+ccapurso@users.noreply.github.com> Date: Wed, 16 Nov 2022 14:46:36 -0500 Subject: [PATCH 03/11] add read handlers for sys/loggers endpoints --- vault/logical_system.go | 105 +++++++++++++++++++++++----------- vault/logical_system_paths.go | 8 +++ 2 files changed, 80 insertions(+), 33 deletions(-) diff --git a/vault/logical_system.go b/vault/logical_system.go index 118762712e0c..be938a283114 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -20,9 +20,6 @@ import ( "time" "unicode" - "github.com/hashicorp/vault/helper/versions" - "golang.org/x/crypto/sha3" - "github.com/hashicorp/errwrap" log "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-memdb" @@ -32,10 +29,12 @@ import ( semver "github.com/hashicorp/go-version" "github.com/hashicorp/vault/helper/hostutil" "github.com/hashicorp/vault/helper/identity" + "github.com/hashicorp/vault/helper/logging" "github.com/hashicorp/vault/helper/metricsutil" "github.com/hashicorp/vault/helper/monitor" "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/helper/random" + "github.com/hashicorp/vault/helper/versions" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/sdk/helper/jsonutil" @@ -44,6 +43,7 @@ import ( "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/sdk/version" "github.com/mitchellh/mapstructure" + "golang.org/x/crypto/sha3" ) const ( @@ -4828,28 +4828,29 @@ func (b *SystemBackend) handleVersionHistoryList(ctx context.Context, req *logic return logical.ListResponseWithInfo(respKeys, respKeyInfo), nil } -// getLogLevel returns the hclog.Level that corresponds with the provided level string. -// This differs hclog.LevelFromString in that it supports additional level strings so -// that in remains consistent with the handling found in the "vault server" command. -func getLogLevel(logLevel string) (log.Level, error) { - var level log.Level - - switch logLevel { - case "trace": - level = log.Trace - case "debug": - level = log.Debug - case "notice", "info", "": - level = log.Info - case "warn", "warning": - level = log.Warn - case "err", "error": - level = log.Error - default: - return level, fmt.Errorf("unrecognized log level %q", logLevel) +func (b *SystemBackend) handleLoggersRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + b.Core.allLoggersLock.RLock() + defer b.Core.allLoggersLock.RUnlock() + + loggers := make(map[string]interface{}) + warnings := make([]string, 0) + + for _, logger := range b.Core.allLoggers { + loggerName := logger.Name() + logLevel, err := logging.TranslateLoggerLevel(logger) + if err != nil { + warnings = append(warnings, fmt.Sprintf("cannot translate level for %q: %s", loggerName, err.Error())) + } else { + loggers[loggerName] = logLevel + } } - return level, nil + resp := &logical.Response{ + Data: loggers, + Warnings: warnings, + } + + return resp, nil } func (b *SystemBackend) handleLoggersWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { @@ -4864,7 +4865,7 @@ func (b *SystemBackend) handleLoggersWrite(ctx context.Context, req *logical.Req return logical.ErrorResponse("level is empty"), nil } - level, err := getLogLevel(logLevel) + level, err := logging.ParseLogLevel(logLevel) if err != nil { return logical.ErrorResponse(fmt.Sprintf("invalid level provided: %s", err.Error())), nil } @@ -4875,7 +4876,7 @@ func (b *SystemBackend) handleLoggersWrite(ctx context.Context, req *logical.Req } func (b *SystemBackend) handleLoggersDelete(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - level, err := getLogLevel(b.Core.logLevel) + level, err := logging.ParseLogLevel(b.Core.logLevel) if err != nil { return logical.ErrorResponse(fmt.Sprintf("log level from config is invalid: %s", err.Error())), nil } @@ -4885,6 +4886,42 @@ func (b *SystemBackend) handleLoggersDelete(ctx context.Context, req *logical.Re return nil, nil } +func (b *SystemBackend) handleLoggersByNameRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + nameRaw, nameOk := d.GetOk("name") + if !nameOk { + return logical.ErrorResponse("name is required"), nil + } + + name := nameRaw.(string) + + b.Core.allLoggersLock.RLock() + defer b.Core.allLoggersLock.RUnlock() + + loggers := make(map[string]interface{}) + warnings := make([]string, 0) + + for _, logger := range b.Core.allLoggers { + if logger.Name() == name { + logLevel, err := logging.TranslateLoggerLevel(logger) + + if err != nil { + warnings = append(warnings, fmt.Sprintf("cannot translate level for %q: %s", name, err.Error())) + } else { + loggers[name] = logLevel + } + + break + } + } + + resp := &logical.Response{ + Data: loggers, + Warnings: warnings, + } + + return resp, nil +} + func (b *SystemBackend) handleLoggersByNameWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { nameRaw, nameOk := d.GetOk("name") if !nameOk { @@ -4902,14 +4939,15 @@ func (b *SystemBackend) handleLoggersByNameWrite(ctx context.Context, req *logic return logical.ErrorResponse("level is empty"), nil } - level, err := getLogLevel(logLevel) + level, err := logging.ParseLogLevel(logLevel) if err != nil { return logical.ErrorResponse(fmt.Sprintf("invalid level provided: %s", err.Error())), nil } - err = b.Core.SetLogLevelByName(nameRaw.(string), level) - if err != nil { - return logical.ErrorResponse(fmt.Sprintf("invalid params: %s", err.Error())), nil + name := nameRaw.(string) + success := b.Core.SetLogLevelByName(name, level) + if !success { + return logical.ErrorResponse(fmt.Sprintf("logger %q not found", name)), nil } return nil, nil @@ -4921,14 +4959,15 @@ func (b *SystemBackend) handleLoggersByNameDelete(ctx context.Context, req *logi return logical.ErrorResponse("name is required"), nil } - level, err := getLogLevel(b.Core.logLevel) + level, err := logging.ParseLogLevel(b.Core.logLevel) if err != nil { return logical.ErrorResponse(fmt.Sprintf("log level from config is invalid: %s", err.Error())), nil } - err = b.Core.SetLogLevelByName(nameRaw.(string), level) - if err != nil { - return logical.ErrorResponse(fmt.Sprintf("invalid params: %s", err.Error())), nil + name := nameRaw.(string) + success := b.Core.SetLogLevelByName(name, level) + if !success { + return logical.ErrorResponse(fmt.Sprintf("logger %q not found", name)), nil } return nil, nil diff --git a/vault/logical_system_paths.go b/vault/logical_system_paths.go index fbeb9d541b1e..4bd65b9595a2 100644 --- a/vault/logical_system_paths.go +++ b/vault/logical_system_paths.go @@ -298,6 +298,10 @@ func (b *SystemBackend) configPaths() []*framework.Path { }, }, Operations: map[logical.Operation]framework.OperationHandler{ + logical.ReadOperation: &framework.PathOperation{ + Callback: b.handleLoggersRead, + Summary: "Read the log level for all existing loggers.", + }, logical.UpdateOperation: &framework.PathOperation{ Callback: b.handleLoggersWrite, Summary: "Modify the log level for all existing loggers.", @@ -322,6 +326,10 @@ func (b *SystemBackend) configPaths() []*framework.Path { }, }, Operations: map[logical.Operation]framework.OperationHandler{ + logical.ReadOperation: &framework.PathOperation{ + Callback: b.handleLoggersByNameRead, + Summary: "Read the log level for a single logger.", + }, logical.UpdateOperation: &framework.PathOperation{ Callback: b.handleLoggersByNameWrite, Summary: "Modify the log level of a single logger.", From 0936449da6071055e265ca4ec962616a3c31ff22 Mon Sep 17 00:00:00 2001 From: Chris Capurso <1036769+ccapurso@users.noreply.github.com> Date: Wed, 16 Nov 2022 14:57:25 -0500 Subject: [PATCH 04/11] add changelog entry --- changelog/17979.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/17979.txt diff --git a/changelog/17979.txt b/changelog/17979.txt new file mode 100644 index 000000000000..81a5c023c961 --- /dev/null +++ b/changelog/17979.txt @@ -0,0 +1,3 @@ +```release-note:improvement +core: Add read support to `sys/loggers` and `sys/loggers/:name` endpoints +``` From 0a2ee78fb9a77e6a66f88e29b51340fae77c58f0 Mon Sep 17 00:00:00 2001 From: Chris Capurso <1036769+ccapurso@users.noreply.github.com> Date: Wed, 16 Nov 2022 15:12:22 -0500 Subject: [PATCH 05/11] update docs --- website/content/api-docs/system/loggers.mdx | 52 +++++++++++++++++++-- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/website/content/api-docs/system/loggers.mdx b/website/content/api-docs/system/loggers.mdx index 73d2c53ef433..d9990f72550a 100644 --- a/website/content/api-docs/system/loggers.mdx +++ b/website/content/api-docs/system/loggers.mdx @@ -8,15 +8,15 @@ description: The `/sys/loggers` endpoint is used modify the verbosity level of l The `/sys/loggers` endpoint is used modify the verbosity level of logging. -!> **NOTE:** Changes made to the log level using this endpoint are not persisted and will be restored -to either the default log level (info) or the level specified using `log_level` in vault.hcl or the `VAULT_LOG_LEVEL` +!> **NOTE:** Changes made to the log level using this endpoint are not persisted and will be restored +to either the default log level (info) or the level specified using `log_level` in vault.hcl or the `VAULT_LOG_LEVEL` environment variable once the Vault service is reloaded or restarted. ## Modify verbosity level of all loggers | Method | Path | | :------ | :------------- | -| `POST` | `/sys/loggers` | +| `POST` | `/sys/loggers` |'' ### Parameters @@ -71,6 +71,52 @@ $ curl \ http://127.0.0.1:8200/v1/sys/loggers/core ``` +## Read verbosity level of all loggers + +| Method | Path | +| :----- | :------------- | +| `GET` | `/sys/loggers` | + +### Sample Request + +```shell-session +$ curl \ + --header "X-Vault-Token: ..." \ + https://127.0.0.1:8200/v1/sys/loggers +``` + +### Sample Response + +```json +{ + "audit": "trace", + "core": "info", + "policy": "debug" +} +``` + +## Read verbosity level of a single logger + +| Method | Path | +| :----- | :------------------- | +| `GET` | `/sys/loggers/:name` | + +### Sample Request + +```shell-session +$ curl \ + --header "X-Vault-Token: ..." \ + https://127.0.0.1:8200/v1/sys/loggers/core +``` + +### Sample Response + +```json +{ + "core": "info" +} +``` + ## Revert verbosity of all loggers to configured level | Method | Path | From 0050e8b0682fb614421725da5d470fe9f5f2faf7 Mon Sep 17 00:00:00 2001 From: Chris Capurso <1036769+ccapurso@users.noreply.github.com> Date: Wed, 16 Nov 2022 15:38:40 -0500 Subject: [PATCH 06/11] ignore base logger --- vault/logical_system.go | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/vault/logical_system.go b/vault/logical_system.go index be938a283114..8ac39d9fe877 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -4837,6 +4837,12 @@ func (b *SystemBackend) handleLoggersRead(ctx context.Context, req *logical.Requ for _, logger := range b.Core.allLoggers { loggerName := logger.Name() + + // ignore base logger + if loggerName == "" { + continue + } + logLevel, err := logging.TranslateLoggerLevel(logger) if err != nil { warnings = append(warnings, fmt.Sprintf("cannot translate level for %q: %s", loggerName, err.Error())) @@ -4893,6 +4899,9 @@ func (b *SystemBackend) handleLoggersByNameRead(ctx context.Context, req *logica } name := nameRaw.(string) + if name == "" { + return logical.ErrorResponse("name is empty"), nil + } b.Core.allLoggersLock.RLock() defer b.Core.allLoggersLock.RUnlock() @@ -4901,13 +4910,20 @@ func (b *SystemBackend) handleLoggersByNameRead(ctx context.Context, req *logica warnings := make([]string, 0) for _, logger := range b.Core.allLoggers { - if logger.Name() == name { + loggerName := logger.Name() + + // ignore base logger + if loggerName == "" { + continue + } + + if loggerName == name { logLevel, err := logging.TranslateLoggerLevel(logger) if err != nil { - warnings = append(warnings, fmt.Sprintf("cannot translate level for %q: %s", name, err.Error())) + warnings = append(warnings, fmt.Sprintf("cannot translate level for %q: %s", loggerName, err.Error())) } else { - loggers[name] = logLevel + loggers[loggerName] = logLevel } break @@ -4928,6 +4944,11 @@ func (b *SystemBackend) handleLoggersByNameWrite(ctx context.Context, req *logic return logical.ErrorResponse("name is required"), nil } + name := nameRaw.(string) + if name == "" { + return logical.ErrorResponse("name is empty"), nil + } + logLevelRaw, logLevelOk := d.GetOk("level") if !logLevelOk { @@ -4944,7 +4965,6 @@ func (b *SystemBackend) handleLoggersByNameWrite(ctx context.Context, req *logic return logical.ErrorResponse(fmt.Sprintf("invalid level provided: %s", err.Error())), nil } - name := nameRaw.(string) success := b.Core.SetLogLevelByName(name, level) if !success { return logical.ErrorResponse(fmt.Sprintf("logger %q not found", name)), nil From 2b400762937f4b5c5885f878fa4474196fdb8b64 Mon Sep 17 00:00:00 2001 From: Chris Capurso <1036769+ccapurso@users.noreply.github.com> Date: Wed, 16 Nov 2022 16:18:13 -0500 Subject: [PATCH 07/11] fix docs formatting issue --- website/content/api-docs/system/loggers.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/content/api-docs/system/loggers.mdx b/website/content/api-docs/system/loggers.mdx index d9990f72550a..3647701409db 100644 --- a/website/content/api-docs/system/loggers.mdx +++ b/website/content/api-docs/system/loggers.mdx @@ -16,7 +16,7 @@ environment variable once the Vault service is reloaded or restarted. | Method | Path | | :------ | :------------- | -| `POST` | `/sys/loggers` |'' +| `POST` | `/sys/loggers` | ### Parameters From f7b46b184302ba2539f9176246d898c1019ac46b Mon Sep 17 00:00:00 2001 From: Chris Capurso <1036769+ccapurso@users.noreply.github.com> Date: Fri, 18 Nov 2022 19:27:41 -0500 Subject: [PATCH 08/11] add ReadOperation support to TestSystemBackend_Loggers --- vault/logical_system_test.go | 63 ++++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 7 deletions(-) diff --git a/vault/logical_system_test.go b/vault/logical_system_test.go index a890e1c17e17..efffc240a217 100644 --- a/vault/logical_system_test.go +++ b/vault/logical_system_test.go @@ -4791,47 +4791,58 @@ func validateLevel(level string, logger hclog.Logger) bool { func TestSystemBackend_Loggers(t *testing.T) { testCases := []struct { - level string - expectError bool + level string + expectedLevel string + expectError bool }{ { + "trace", "trace", false, }, { + "debug", "debug", false, }, { "notice", + "info", false, }, { + "info", "info", false, }, { + "warn", "warn", false, }, { "warning", + "warn", false, }, { "err", + "error", false, }, { + "error", "error", false, }, { "", + "info", true, }, { "invalid", + "", true, }, } @@ -4844,6 +4855,10 @@ func TestSystemBackend_Loggers(t *testing.T) { core, b, _ := testCoreSystemBackend(t) + // Test core will have core.logLevel set to an empty string + // by default which ultimately is Info but let's make it explicit + core.logLevel = "info" + req := &logical.Request{ Path: "loggers", Operation: logical.UpdateOperation, @@ -4864,15 +4879,32 @@ func TestSystemBackend_Loggers(t *testing.T) { } if !tc.expectError { + req = &logical.Request{ + Path: "loggers", + Operation: logical.ReadOperation, + } + + resp, err = b.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("unexpected error, err: %v, resp: %#v", err, resp) + } + for _, logger := range core.allLoggers { - if !validateLevel(tc.level, logger) { - t.Fatalf("expected logger %q to be %q", logger.Name(), tc.level) + loggerName := logger.Name() + levelRaw, ok := resp.Data[loggerName] + + if !ok { + t.Errorf("logger %q not found in response", loggerName) + } + + if levelStr := levelRaw.(string); levelStr != tc.expectedLevel { + t.Errorf("unexpected level of logger %q, expected: %s, actual: %s", loggerName, tc.expectedLevel, levelStr) } } } req = &logical.Request{ - Path: fmt.Sprintf("loggers"), + Path: "loggers", Operation: logical.DeleteOperation, } @@ -4881,9 +4913,26 @@ func TestSystemBackend_Loggers(t *testing.T) { t.Fatalf("unexpected error, err: %v, resp: %#v", err, resp) } + req = &logical.Request{ + Path: "loggers", + Operation: logical.ReadOperation, + } + + resp, err = b.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("unexpected error, err: %v, resp: %#v", err, resp) + } + for _, logger := range core.allLoggers { - if !validateLevel(core.logLevel, logger) { - t.Errorf("expected level of logger %q to match original config", logger.Name()) + loggerName := logger.Name() + levelRaw, ok := resp.Data[loggerName] + + if !ok { + t.Errorf("logger %q not found in response", loggerName) + } + + if levelStr := levelRaw.(string); levelStr != core.logLevel { + t.Errorf("expected level of logger %q to match original config, expected: %s, actual: %s", loggerName, core.logLevel, levelStr) } } }) From 92cc8796df0abdd36ded28a759f164aa84f6ec3d Mon Sep 17 00:00:00 2001 From: Chris Capurso <1036769+ccapurso@users.noreply.github.com> Date: Mon, 21 Nov 2022 09:38:58 -0500 Subject: [PATCH 09/11] add more robust checks to TestSystemBackend_Loggers --- vault/logical_system_test.go | 45 ++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/vault/logical_system_test.go b/vault/logical_system_test.go index efffc240a217..76ac12f5486c 100644 --- a/vault/logical_system_test.go +++ b/vault/logical_system_test.go @@ -4855,11 +4855,33 @@ func TestSystemBackend_Loggers(t *testing.T) { core, b, _ := testCoreSystemBackend(t) - // Test core will have core.logLevel set to an empty string - // by default which ultimately is Info but let's make it explicit - core.logLevel = "info" - + // Test core overrides logging level outside of config, + // an initial delete will ensure that we an initial read + // to get expected values is based off of config and not + // the test override that is hidden from this test req := &logical.Request{ + Path: "loggers", + Operation: logical.DeleteOperation, + } + + resp, err := b.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("unexpected error, err: %v, resp: %#v", err, resp) + } + + req = &logical.Request{ + Path: "loggers", + Operation: logical.ReadOperation, + } + + resp, err = b.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("unexpected error, err: %v, resp: %#v", err, resp) + } + + initialLoggers := resp.Data + + req = &logical.Request{ Path: "loggers", Operation: logical.UpdateOperation, Data: map[string]interface{}{ @@ -4867,7 +4889,7 @@ func TestSystemBackend_Loggers(t *testing.T) { }, } - resp, err := b.HandleRequest(namespace.RootContext(nil), req) + resp, err = b.HandleRequest(namespace.RootContext(nil), req) respIsError := resp != nil && resp.IsError() if err != nil || (!tc.expectError && respIsError) { @@ -4925,14 +4947,17 @@ func TestSystemBackend_Loggers(t *testing.T) { for _, logger := range core.allLoggers { loggerName := logger.Name() - levelRaw, ok := resp.Data[loggerName] + levelRaw, currentOk := resp.Data[loggerName] + initialLevelRaw, initialOk := initialLoggers[loggerName] - if !ok { - t.Errorf("logger %q not found in response", loggerName) + if !currentOk || !initialOk { + t.Errorf("logger %q not found", loggerName) } - if levelStr := levelRaw.(string); levelStr != core.logLevel { - t.Errorf("expected level of logger %q to match original config, expected: %s, actual: %s", loggerName, core.logLevel, levelStr) + levelStr := levelRaw.(string) + initialLevelStr := initialLevelRaw.(string) + if levelStr != initialLevelStr { + t.Errorf("expected level of logger %q to match original config, expected: %s, actual: %s", loggerName, initialLevelStr, levelStr) } } }) From e7562af11d81d06215d88c34095a8601242b1882 Mon Sep 17 00:00:00 2001 From: Chris Capurso <1036769+ccapurso@users.noreply.github.com> Date: Mon, 21 Nov 2022 09:50:47 -0500 Subject: [PATCH 10/11] add more robust checks to TestSystemBackend_LoggersByName --- vault/logical_system_test.go | 117 ++++++++++++++++++++++++++--------- 1 file changed, 87 insertions(+), 30 deletions(-) diff --git a/vault/logical_system_test.go b/vault/logical_system_test.go index 76ac12f5486c..582c6435cf81 100644 --- a/vault/logical_system_test.go +++ b/vault/logical_system_test.go @@ -4772,23 +4772,6 @@ func TestProcessLimit(t *testing.T) { } } -func validateLevel(level string, logger hclog.Logger) bool { - switch level { - case "trace": - return logger.IsTrace() - case "debug": - return logger.IsDebug() - case "notice", "info", "": - return logger.IsInfo() - case "warn", "warning": - return logger.IsWarn() - case "err", "error": - return logger.IsError() - } - - return false -} - func TestSystemBackend_Loggers(t *testing.T) { testCases := []struct { level string @@ -4968,78 +4951,91 @@ func TestSystemBackend_LoggersByName(t *testing.T) { testCases := []struct { logger string level string + expectedLevel string expectWriteError bool expectDeleteError bool }{ { "core", "trace", + "trace", false, false, }, { "token", "debug", + "debug", false, false, }, { "audit", "notice", + "info", false, false, }, { "expiration", "info", + "info", false, false, }, { "policy", "warn", + "warn", false, false, }, { "activity", "warning", + "warn", false, false, }, { "identity", "err", + "error", false, false, }, { "rollback", "error", + "error", false, false, }, { "system", "", + "does-not-matter", true, false, }, { "quotas", "invalid", + "does-not-matter", true, false, }, { "", "info", + "does-not-matter", true, true, }, { "does_not_exist", "error", + "does-not-matter", true, true, }, @@ -5053,16 +5049,41 @@ func TestSystemBackend_LoggersByName(t *testing.T) { core, b, _ := testCoreSystemBackend(t) + // Test core overrides logging level outside of config, + // an initial delete will ensure that we an initial read + // to get expected values is based off of config and not + // the test override that is hidden from this test req := &logical.Request{ + Path: "loggers", + Operation: logical.DeleteOperation, + } + + resp, err := b.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("unexpected error, err: %v, resp: %#v", err, resp) + } + + req = &logical.Request{ + Path: "loggers", + Operation: logical.ReadOperation, + } + + resp, err = b.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("unexpected error, err: %v, resp: %#v", err, resp) + } + + initialLoggers := resp.Data + + req = &logical.Request{ Path: fmt.Sprintf("loggers/%s", tc.logger), Operation: logical.UpdateOperation, Data: map[string]interface{}{ - "name": tc.logger, "level": tc.level, }, } - resp, err := b.HandleRequest(namespace.RootContext(nil), req) + resp, err = b.HandleRequest(namespace.RootContext(nil), req) respIsError := resp != nil && resp.IsError() if err != nil || (!tc.expectWriteError && respIsError) { @@ -5074,13 +5095,34 @@ func TestSystemBackend_LoggersByName(t *testing.T) { } if !tc.expectWriteError { + req = &logical.Request{ + Path: "loggers", + Operation: logical.ReadOperation, + } + + resp, err = b.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("unexpected error, err: %v, resp: %#v", err, resp) + } + for _, logger := range core.allLoggers { - if logger.Name() != tc.logger && !validateLevel(core.logLevel, logger) { - t.Errorf("expected level of logger %q to be unchanged", logger.Name()) + loggerName := logger.Name() + levelRaw, currentOk := resp.Data[loggerName] + initialLevelRaw, initialOk := initialLoggers[loggerName] + + if !currentOk || !initialOk { + t.Errorf("logger %q not found", loggerName) + } + + levelStr := levelRaw.(string) + initialLevelStr := initialLevelRaw.(string) + + if loggerName == tc.logger && levelStr != tc.expectedLevel { + t.Fatalf("expected logger %q to be %q, actual: %s", loggerName, tc.expectedLevel, levelStr) } - if !validateLevel(tc.level, logger) { - t.Fatalf("expected logger %q to be %q", logger.Name(), tc.level) + if loggerName != tc.logger && levelStr != initialLevelStr { + t.Errorf("expected level of logger %q to be unchanged, exepcted: %s, actual: %s", loggerName, initialLevelStr, levelStr) } } } @@ -5088,9 +5130,6 @@ func TestSystemBackend_LoggersByName(t *testing.T) { req = &logical.Request{ Path: fmt.Sprintf("loggers/%s", tc.logger), Operation: logical.DeleteOperation, - Data: map[string]interface{}{ - "name": tc.logger, - }, } resp, err = b.HandleRequest(namespace.RootContext(nil), req) @@ -5105,10 +5144,28 @@ func TestSystemBackend_LoggersByName(t *testing.T) { } if !tc.expectDeleteError { - for _, logger := range core.allLoggers { - if !validateLevel(core.logLevel, logger) { - t.Errorf("expected level of logger %q to match original config", logger.Name()) - } + req = &logical.Request{ + Path: fmt.Sprintf("loggers/%s", tc.logger), + Operation: logical.ReadOperation, + } + + resp, err = b.HandleRequest(namespace.RootContext(nil), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("unexpected error, err: %v, resp: %#v", err, resp) + } + + currentLevel, ok := resp.Data[tc.logger].(string) + if !ok { + t.Fatalf("expected resp to include %q, resp: %#v", tc.logger, resp) + } + + initialLevel, ok := initialLoggers[tc.logger].(string) + if !ok { + t.Fatalf("expected initial loggers to include %q, resp: %#v", tc.logger, initialLoggers) + } + + if currentLevel != initialLevel { + t.Errorf("expected level of logger %q to match original config, expected: %s, actual: %s", tc.logger, initialLevel, currentLevel) } } }) From 30a5508576c17b34bc6cee2d9e9eaf28da731e03 Mon Sep 17 00:00:00 2001 From: Chris Capurso <1036769+ccapurso@users.noreply.github.com> Date: Mon, 21 Nov 2022 14:23:28 -0500 Subject: [PATCH 11/11] check for empty name in delete handler --- vault/logical_system.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/vault/logical_system.go b/vault/logical_system.go index 8ac39d9fe877..d8d0e03e914f 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -4985,6 +4985,10 @@ func (b *SystemBackend) handleLoggersByNameDelete(ctx context.Context, req *logi } name := nameRaw.(string) + if name == "" { + return logical.ErrorResponse("name is empty"), nil + } + success := b.Core.SetLogLevelByName(name, level) if !success { return logical.ErrorResponse(fmt.Sprintf("logger %q not found", name)), nil